Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-migrate commands

[Eric Blake]

On 02/24/2012 11:57 AM, Paolo Bonzini wrote:
> On 02/24/2012 06:46 PM, Eric Blake wrote:
>> I think you need to be more explicit that @new-image-file MUST have
>> identical contents as the current image file, for this to be useful, and
>> that qemu does not validate whether the new image met those conditions.
>>  Possible ways to achieve this:
> 
> Not necessarily, you could always do this on a paused machine.

You lost me; I think you snipped too much.  Was it about this statement
in my original?

>> 1. Freeze all guest I/O, so that you know the current image file is
>> stable,

To which I have a counter-question: Is pausing a machine guaranteed to
flush all I/O out to the image prior to returning control to the
controlling app, or do we have to rely on the qemu-ga agent command to
actually freeze I/O within the guest?

Overall, I was arguing that from a usability perspective, if
new-image-file does not have the same contents (from the guest's
perspective) as the current image, then when you resume the machine, you
will be injecting arbitrary disk changes into the guest's disk, probably
with disastrous results.  I'm not arguing that new-image-file has to be
identical to the current image (in fact, converting between raw or qcow2
via drive-reopen seems like great use cases for this new command).  So
documenting this distinction (guest data must be consistent across the
reopen), as part of the contract of the monitor command, would be useful.

> 
>>>> +# @destination: the destination of the block migration.
>> Where do you list what format the destination is?  Shouldn't this have
>> an optional format, defaulting to qcow2?  Does qemu create the
>> destination file, or must it already be existing?
> 
> I think no default, raw makes as much or more sense in the
> non-incremental case.  Anyway either autodetect, or no default.

So if there is no default, then how do I specify the format?  Libvirt
_cannot_ rely on autodetect, ever since we had to solve a CVE where
relying on autodetect would allow the guest to cause libvirt to mislabel
files on the host.

>> I know that this patch is only implementing the case where incremental
>> is true and new-image-file is provided; but I'm not quite sure what
>> semantics are intended if incremental is false.  Is that still a case
>> where this sets up mirroring (writes go to two images) but additionally
>> the contents from the current image are (asynchronously) streamed into
>> the destination?
> 
> Yes.  The image should already be there also in this case, and
> new-image-file will usually be omitted.

If the image to be opened must already exist, then explicitly state that
in the command documentation.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature