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

[Anthony Liguori]

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.
>
>