Re: [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls

[Benoît Canet]

Le Thursday 23 Aug 2012 à 14:48:19 (+0200), Benoît Canet a écrit :
> Le Thursday 23 Aug 2012 à 13:34:01 (+0100), Daniel P. Berrange a écrit :
> > On Thu, Aug 23, 2012 at 02:28:07PM +0200, Benoît Canet wrote:
> > > Usage:
> > > (qemu) migrate file:/path/to/vm_statefile
> > > 
> > > Signed-off-by: Benoit Canet <address@hidden>
> > > ---
> > >  migration-fd.c |    4 ++--
> > >  migration.c    |   20 +++++++++++++++++++-
> > >  migration.h    |    2 +-
> > >  3 files changed, 22 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/migration-fd.c b/migration-fd.c
> > > index 50138ed..d39e44a 100644
> > > --- a/migration-fd.c
> > > +++ b/migration-fd.c
> > > @@ -73,9 +73,9 @@ static int fd_close(MigrationState *s)
> > >      return 0;
> > >  }
> > >  
> > > -int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
> > > +int fd_start_outgoing_migration(MigrationState *s, const char *fdname, 
> > > int fd)
> > >  {
> > > -    s->fd = monitor_get_fd(cur_mon, fdname);
> > > +    s->fd = fd ? fd : monitor_get_fd(cur_mon, fdname);
> > >      if (s->fd == -1) {
> > >          DPRINTF("fd_migration: invalid file descriptor identifier\n");
> > >          goto err_after_get_fd;
> > > diff --git a/migration.c b/migration.c
> > > index 1edeec5..679847d 100644
> > > --- a/migration.c
> > > +++ b/migration.c
> > > @@ -239,9 +239,14 @@ void 
> > > qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> > >  static int migrate_fd_cleanup(MigrationState *s)
> > >  {
> > >      int ret = 0;
> > > +    struct stat st;
> > >  
> > >      qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
> > >  
> > > +    if (!fstat(s->fd, &st) && S_ISREG(st.st_mode)) {
> > > +        fsync(s->fd);
> > > +    }
> > > +
> > >      if (s->file) {
> > >          DPRINTF("closing file\n");
> > >          ret = qemu_fclose(s->file);
> > > @@ -475,6 +480,17 @@ void migrate_del_blocker(Error *reason)
> > >      migration_blockers = g_slist_remove(migration_blockers, reason);
> > >  }
> > >  
> > > +static int file_start_outgoing_migration(MigrationState *s,
> > > +                                         const char *filename)
> > > +{
> > > +    int fd;
> > > +    fd = open(filename, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR);
> > > +    if (fd < 0) {
> > > +        return -errno;
> > > +    }
> > > +    return fd_start_outgoing_migration(s, NULL, fd);
> > 
> > 'fd_start_outgoing_migration' requires that the FD you give it
> > supports non-blocking I/O. File descriptors opened from plain
> > files or block devices do not honour that requirement. So this
> > proposed code will cause the entire QEMU process to block while
> > migration is taking place. This is why no on has ever implemented
> > the 'file:' protocol in QEMU before.
> 
> When I run the code it appear it does not block all the time.
> (guest is still interactive most of the time needed to complete migration)
> How can it be ?
> 
> Benoît
> 
> > 
> > To deal with this issue you'd either have to use the POSIX
> > async I/O APIs (or QEMU's internal equivalent)
I don't see how AIO could fit in this scheme.

>, or spawn a
> > separate 'dd' helper process and give QEMU a pipe FD instead.
> > The latter is what libvirt does to implement migrate to file.

I could subvert a exec_start_outgoing_migration clone to be
fork_start_outgoing_migration doing a for then writing to disk
the incoming data.

Does any QEMU developper have an idea on this ?

Benoît


> > 
> > Daniel
> > -- 
> > |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ 
> > :|
> > |: http://libvirt.org              -o-             http://virt-manager.org 
> > :|
> > |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ 
> > :|
> > |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc 
> > :|
>