[Qemu-devel] Re: [patch 2/3] Add support for live block copy

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/22/2011 05:09 PM, Marcelo Tosatti wrote:
> On Tue, Feb 22, 2011 at 03:11:06PM -0600, Anthony Liguori wrote:
>    
>> On 02/22/2011 03:07 PM, Marcelo Tosatti wrote:
>>      
>>> On Tue, Feb 22, 2011 at 02:50:09PM -0600, Anthony Liguori wrote:
>>>        
>>>>> +static int write_commit_file(BdrvCopyState *s)
>>>>> +{
>>>>> +    char commit_msg[1400];
>>>>> +    const char *buf = commit_msg;
>>>>> +    int len, ret;
>>>>> +
>>>>> +    sprintf(commit_msg, "commit QEMU block_copy %s ->    %s\n", s->src_device_name,
>>>>> +                        s->dst_filename);
>>>>> +
>>>>> +    len = strlen(commit_msg);
>>>>> +    while (len>    0) {
>>>>> +        ret = write(s->commit_fd, buf, len);
>>>>> +        if (ret == -1&&    errno == EINTR) {
>>>>> +            continue;
>>>>> +        }
>>>>> +        if (ret<= 0) {
>>>>> +            return -errno;
>>>>> +        }
>>>>> +        buf += ret;
>>>>> +        len -= ret;
>>>>> +    }
>>>>> +
>>>>> +    if (fsync(s->commit_fd) == -1) {
>>>>> +        return -errno;
>>>>> +    }
>>>>>
>>>>>            
>>>> This is more or less black magic.  What is this commit file used for
>>>> and why aren't we using something like a QMP event?
>>>>          
>>> The commit file is considered reliable storage for the result of image
>>> switch operation. Think of the following scenario:
>>>
>>> - mgmt app requests live copy of guests ide1-hd0
>>>        
>> >from /a/image.img to /b/image.img.
>>      
>>> - mgmt app dies.
>>> - guest switches to /b/image.img, /a/image.img is outdated.
>>> - guest dies.
>>>
>>> Notifying the switch via QMP would not be reliable in this case.
>>>        
>> But this is true of any number of operations in QEMU such as hotplug
>> where if a management tool dies after requesting hotplug and then
>> the guest dies before the management tool reconnects, it's never
>> known whether it's truly connected or not.
>>      
> This is a different case. The mgmt tool should be able to safely store
> the desired device tree before hotplugging a device.
>
> And even if you consider that should be done by QEMU, in a config file,
> restarting the guest with or without the device is not going to result
> in data corruption/loss.
>    

If a guest started writing to a disk and is storing important data 
there, and then the guest is restarted and the disk (and data) isn't 
there, I'd classify that as data loss.

>> The only way to robustly solve this is for QEMU to maintain a
>> stateful config file.  A one-off for this particular command doesn't
>> seem wise to me.
>>      
> I don't disagree the commit message could be in a config file, but since
> that does not exist, a separate file is a workable solution.

I disagree.  This is something we'll have to maintain forever and what's 
to stop us from having additional one-off mechanisms to deal with this 
same problem.

-drive already ties into the qemuopts infrastructure and we have 
readconfig and writeconfig.  I don't think we're missing any major 
pieces to do this in a more proper fashion.

Regards,

Anthony Liguori

>   Also, the
> separate commit file is not incompatible with future improvements.
>
>