Re: [Qemu-devel] [PATCH v2 1/1] Add QMP bits for blockdev-snapshot-sync.

[Jes Sorensen]

On 04/29/11 15:45, Anthony Liguori wrote:
> On 04/29/2011 08:38 AM, Jes Sorensen wrote:
>> It is exactly the same for the management tool:
>> - Creation of the new image either succeeds or fails
>> - Switchover either succeeds or fails
> 
> Creating an image can be treated as an atomic operation.  It either
> fully succeeds or you assume it failed and throw the image away since
> you can always try again.
> 
> When you combine creating an image with another operation, you create a
> a single operation that cannot be treated as atomic which makes recovery
> from failure much more difficult.

As explained in previous emails, you still have the option to do this
with the current implementation, if you so desire. It's a bad idea, but
for those who insist on doing the wrong thing, sure go ahead.

Just create the image first, and then pass it in as the snapshot file
rather than specifying a snapshot file that doesn't exist already.

>>> 3) Begin switch over to new image
>>> 4) Switch over image.
>>> 5) Receive notification when it's complete
>>
>> Sorry but this isn't an accurate description of the process. Once you
>> have a new image ready, you need to halt writes, flush buffers, and then
>> you can do the switch, which has to be atomic.
> 
> You need to queue writes, you can still let the guest run while the
> remaining writes are sent to disk.
> 
> But if this is a background task, then as a management tool, don't I
> want a notification when it's been completed?
> 
> Don't I want to have a little spinning box in my GUI or something like
> that while this is happening?

I agree that having an interface that can run it in the background
asynchronously isn't a bad thing, and it would be a nice feature.
However it really doesn't qualify as anything but an enhancement in my
book. You already have the option to pre-create the snapshot image if
you so desire.

>>> But combining 1-5 in a single interface creates a command that while
>>> convenient on the command line, is not usable for a robust management
>>> tool.
>>
>> As I explained you can already use an externally created image with the
>> current interface, the only issue that may be worth doing async is the
>> buffer flushing. However once you do that, guest writes are going to
>> stall anyway, and eventually the guest applications will all stall.
> 
> The interface shouldn't have the option to create an image... because if
> you have that, the interface is difficult to use.
> 
> Why present an option to do something that we know is broken?  We can't
> blame management tools for not doing a good job managing KVM if we're
> giving them poor interfaces to work with.

Sorry this is utterly bogus. This is *not* broken, it is the right way
to do things. It is clearly a matter of taste whether you prefer it one
way or another, but it is not broken.

There is nothing wrong with it being an option, especially since you
don't have to do anything magic for creating an image in advance. I
realize it doesn't match your image of how you would like the command to
work, but that is not the same as it being broken.

I really see nothing in your above arguments that backs up the argument
that the current implementation is broken in any way. I am totally in
favor or having an async implementation as well, but I really don't see
it being as big an issue as you are making it.

Regards,
Jes