Re: [Qemu-devel] [PATCH 4/6 v11] add-cow: support snapshot_blkde

[Stefan Hajnoczi]

On Thu, Aug 2, 2012 at 8:28 AM, Dong Xu Wang <address@hidden> wrote:
> On Wed, Aug 1, 2012 at 11:37 PM, Stefan Hajnoczi <address@hidden> wrote:
>> On Tue, Jul 31, 2012 at 5:51 PM, Dong Xu Wang
>> <address@hidden> wrote:
>>> add-cow will let raw file support snapshot_blkdev indirectly.
>>>
>>> Signed-off-by: Dong Xu Wang <address@hidden>
>>> ---
>>>  blockdev.c              |   45 
>>> +++++++++++++++++++++++++++++++++++++--------
>>>  docs/live-block-ops.txt |   11 ++++++++++-
>>>  2 files changed, 47 insertions(+), 9 deletions(-)
>>
>> This patch would need to update qapi-schema.json and qmp-commands.hx
>> to make use of the new arguments.
>>
>> I don't think QEMU compiles cleanly after this patch.  It's important
>> to keep the build clean after every patch so that git-bisect(1) can be
>> used (if you hit a broken build during a bisect it makes things
>> harder).
>>
>> However, I think the QMP/HMP changes should not be included in this
>> series unless you are sure the interface is stable.  When you convert
>> QEMUOptionsParameter will transaction or snapshot-blkdev-sync change?
>> If so, please only send the add-cow image format and leave the QMP/HMP
>> changes until you have the QemuOpts solution.
>
> Ah, yes, sorry, the patch must be compiled with 5/6, HMP  now works like:
> [-n] device [new-image-file] [format] [image-file] [image-format]
>
> snapshot_blockdev /dev/ide-id0 1.add-cow add-cow t.raw raw
>
> Also qmp's args_type:
> .args_type  = 
> "device:B,snapshot-file:s,format:s?,mode:s?,image-file:s?,image-format:s?",
>
> Before  converting QEMUOptionsParameter, can the code be like this?

I think it's cleanest to submit just the image format without
snapshot_blockdev interface changes.  It's definitely not a good idea
to temporarily change the QMP/HMP interface if the
QEMUOptionsParameter conversion will change it again.

Stefan