Re: [Qemu-devel] [PATCH 5/6] add-cow: support snapshot_blkdev

[Dong Xu Wang]

On Thu, Jun 14, 2012 at 7:33 PM, Kevin Wolf <address@hidden> wrote:
> Am 14.06.2012 13:18, schrieb Paolo Bonzini:
>> Il 14/06/2012 12:59, Kevin Wolf ha scritto:
>>> Am 13.06.2012 16:36, schrieb Dong Xu Wang:
>>>> add-cow will let raw file support snapshot_blkdev indirectly.
>>>>
>>>> Signed-off-by: Dong Xu Wang <address@hidden>
>>>
>>> Paolo, what do you think about this magic?
>>
>> Besides the obvious layering violation (it would be better to add a new
>> method bdrv_ext_snapshot_create perhaps) I don't see very much a problem
>> in it.  Passing image creation options sounds like a good idea that we
>> can add in the future as an extension.
>>
>> But honestly, I don't really see the point of add-cow in general...  The
>> raw image is anyway not usable without a pass through qemu-io convert,
>> and mirroring will also allow collapsing an image to raw (with the
>> persistent dirty bitmap playing the role of the add-cow metadata).
>
> Well, the idea was that you stream into add-cow and once the streaming
> has completed, you can just drop the bitmap.
>
> There might be some overlap with mirroring, though when we discussed
> introducing add-cow, mirrors were not yet on the table. One difference
> that remains is that with streaming the guest only writes to the target
> image and can't have any problem with convergence.
>
>>> I think I can see its use especially for HMP because it's quite
>>> convenient, but on the other hand it assumes a fixed image path for the
>>> new raw image. This isn't going to work for block devices, for example.
>>
>> True, but then probably you would use mode='existing', because you need
>> to pre-create the logical volume.
>
> I think it might be convenient to have the raw volume precreated (you
> need to do that anyway), but create the COW bitmap only during the
> snapshot command. But yeah, not really important.
>
>>> If we don't do it this way, we need to allow passing image creation
>>> options to the snapshotting command so that you can pass a precreated
>>> image file.
>>
>> This sounds like a useful extension anyway, except that passing an
>> unstructured string for image creation options is ugly...  Perhaps we
>> can base a better implementation of options on Laszlo's QemuOpts visitor.
>
> Yes, I wouldn't want to use a string here, we should use something
> structured. Image creation still uses the old-style options instead of
> QemuOpts, but maybe this is the opportunity to convert it.

Kevin, do you mean I need to replace QEMUOptionParameter with QemuOpts?

If true, other image formats should also be changed, I noticed Stefan
has an un-comleted patch:
http://repo.or.cz/w/qemu/stefanha.git/commitdiff/b49babb2c8b476a36357cfd7276ca45a11039ca5

then I can work based on Stefan's patch.


>
> Kevin
>