qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] hmp: add hmp command for incre


From: John Snow
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] hmp: add hmp command for incremental backup
Date: Thu, 21 Jan 2016 15:47:48 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0


On 01/21/2016 11:39 AM, Eric Blake wrote:
> On 01/21/2016 04:22 AM, Rudy Zhang wrote:
>> Add hmp command for incremental backup in drive-backup.
>> It need a bitmap to backup data from drive-image to incremental image,
>> so before it need add bitmap for this device to track io.
>> Usage:
>>      drive_backup [-n] [-f] device target [bitmap] [format]
>>
>> Signed-off-by: Rudy Zhang <address@hidden>
>> ---
>>  hmp-commands.hx |  5 +++--
>>  hmp.c           | 16 ++++++++++++++--
>>  2 files changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index bb52e4d..7378aaa 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1180,12 +1180,13 @@ ETEXI
>>  
>>      {
>>          .name       = "drive_backup",
>> -        .args_type  = "reuse:-n,full:-f,device:B,target:s,format:s?",
>> -        .params     = "[-n] [-f] device target [format]",
>> +        .args_type  = 
>> "reuse:-n,full:-f,device:B,target:s,bitmap:s?,format:s?",
>> +        .params     = "[-n] [-f] device target [bitmap] [format]",
> 
> This is HMP, so it may not matter, but this is not backwards compatible.
>  Scripts targetting the old style of passing a format will now have that
> format string interpreted as a bitmap name with no format.  Better would
> be to stick [bitmap] at the end, not the middle.
> 

I'd like to also point out that the method of intuiting the backup mode
based on the parameters present won't expand very well as we add more
modes, especially if there's ever an addition for a differential backup
mode.

Maybe it's fine because it's HMP and backwards compatibility is not a
real concern ...

> 
>> @@ -1098,6 +1100,17 @@ void hmp_drive_backup(Monitor *mon, const QDict 
>> *qdict)
>>          return;
>>      }
>>  
>> +    if (full && bitmap) {
>> +        error_setg(&err, "Parameter 'bitmap' if conflict with '-f'");
> 
> s/if conflict/conflicts/
> 
>> +        hmp_handle_error(mon, &err);
>> +        return;
>> +    } else if (full)
>> +        sync = MIRROR_SYNC_MODE_FULL;
> 
> Needs {}.  Run your patch through scripts/checkpatch.pl, to flag this
> and other style violations.
> 

-- 
—js



reply via email to

[Prev in Thread] Current Thread [Next in Thread]