qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] Add file url support to migration
@ 2012-08-23 12:28 Benoît Canet
  2012-08-23 12:28 ` [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls Benoît Canet
  2012-08-23 12:28 ` [Qemu-devel] [PATCH 2/2] migration: Allow -incoming " Benoît Canet
  0 siblings, 2 replies; 9+ messages in thread
From: Benoît Canet @ 2012-08-23 12:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, Benoît Canet

This patchset add support of file: urls allowing a user
to save to and restore from disk the state of a vm in an easy
way.

An fflush was added before closing the fd for this use case
in order to be sure that all data have reached stable storage.

One new usage would be migrating between two virtualizer which
share only the storage but do not see each other on the network.
(cisco pvlan for example)

Benoît Canet (2):
  migration: Allow the migrate command to work on file: urls
  migration: Allow -incoming to work on file: urls

 migration-fd.c |    9 ++++-----
 migration.c    |   34 ++++++++++++++++++++++++++++++++--
 migration.h    |    4 ++--
 3 files changed, 38 insertions(+), 9 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls
  2012-08-23 12:28 [Qemu-devel] [PATCH 0/2] Add file url support to migration Benoît Canet
@ 2012-08-23 12:28 ` Benoît Canet
  2012-08-23 12:34   ` Daniel P. Berrange
  2012-08-23 12:28 ` [Qemu-devel] [PATCH 2/2] migration: Allow -incoming " Benoît Canet
  1 sibling, 1 reply; 9+ messages in thread
From: Benoît Canet @ 2012-08-23 12:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, Benoît Canet

Usage:
(qemu) migrate file:/path/to/vm_statefile

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 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);
+}
+
 void qmp_migrate(const char *uri, bool has_blk, bool blk,
                  bool has_inc, bool inc, bool has_detach, bool detach,
                  Error **errp)
@@ -511,7 +527,9 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     } else if (strstart(uri, "unix:", &p)) {
         ret = unix_start_outgoing_migration(s, p);
     } else if (strstart(uri, "fd:", &p)) {
-        ret = fd_start_outgoing_migration(s, p);
+        ret = fd_start_outgoing_migration(s, p, 0);
+    } else if (strstart(uri, "file:", &p)) {
+        ret = file_start_outgoing_migration(s, p);
 #endif
     } else {
         error_set(errp, QERR_INVALID_PARAMETER_VALUE, "uri", "a valid migration protocol");
diff --git a/migration.h b/migration.h
index a9852fc..d431284 100644
--- a/migration.h
+++ b/migration.h
@@ -69,7 +69,7 @@ int unix_start_outgoing_migration(MigrationState *s, const char *path);
 
 int fd_start_incoming_migration(const char *path);
 
-int fd_start_outgoing_migration(MigrationState *s, const char *fdname);
+int fd_start_outgoing_migration(MigrationState *s, const char *fdname, int fd);
 
 void migrate_fd_error(MigrationState *s);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH 2/2] migration: Allow -incoming to work on file: urls
  2012-08-23 12:28 [Qemu-devel] [PATCH 0/2] Add file url support to migration Benoît Canet
  2012-08-23 12:28 ` [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls Benoît Canet
@ 2012-08-23 12:28 ` Benoît Canet
  1 sibling, 0 replies; 9+ messages in thread
From: Benoît Canet @ 2012-08-23 12:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, Benoît Canet

Usage:
-incoming file:/path/to/vm_statefile

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 migration-fd.c |    5 ++---
 migration.c    |   14 +++++++++++++-
 migration.h    |    2 +-
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/migration-fd.c b/migration-fd.c
index d39e44a..bf500a0 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -108,14 +108,13 @@ static void fd_accept_incoming_migration(void *opaque)
     qemu_fclose(f);
 }
 
-int fd_start_incoming_migration(const char *infd)
+int fd_start_incoming_migration(const char *infd, int fd)
 {
-    int fd;
     QEMUFile *f;
 
     DPRINTF("Attempting to start an incoming migration via fd\n");
 
-    fd = strtol(infd, NULL, 0);
+    fd = fd ? fd : strtol(infd, NULL, 0);
     f = qemu_fdopen(fd, "rb");
     if(f == NULL) {
         DPRINTF("Unable to apply qemu wrapper to file descriptor\n");
diff --git a/migration.c b/migration.c
index 679847d..e4b228e 100644
--- a/migration.c
+++ b/migration.c
@@ -64,6 +64,16 @@ static MigrationState *migrate_get_current(void)
     return &current_migration;
 }
 
+static int file_start_incoming_migration(const char *filename)
+{
+    int fd;
+    fd = open(filename, O_RDONLY);
+    if (fd < 0) {
+        return -errno;
+    }
+    return fd_start_incoming_migration(NULL, fd);
+}
+
 int qemu_start_incoming_migration(const char *uri, Error **errp)
 {
     const char *p;
@@ -77,7 +87,9 @@ int qemu_start_incoming_migration(const char *uri, Error **errp)
     else if (strstart(uri, "unix:", &p))
         ret = unix_start_incoming_migration(p);
     else if (strstart(uri, "fd:", &p))
-        ret = fd_start_incoming_migration(p);
+        ret = fd_start_incoming_migration(p, 0);
+    else if (strstart(uri, "file:", &p))
+        ret = file_start_incoming_migration(p);
 #endif
     else {
         fprintf(stderr, "unknown migration protocol: %s\n", uri);
diff --git a/migration.h b/migration.h
index d431284..dbdaf72 100644
--- a/migration.h
+++ b/migration.h
@@ -67,7 +67,7 @@ int unix_start_incoming_migration(const char *path);
 
 int unix_start_outgoing_migration(MigrationState *s, const char *path);
 
-int fd_start_incoming_migration(const char *path);
+int fd_start_incoming_migration(const char *path, int fd);
 
 int fd_start_outgoing_migration(MigrationState *s, const char *fdname, int fd);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls
  2012-08-23 12:28 ` [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls Benoît Canet
@ 2012-08-23 12:34   ` Daniel P. Berrange
  2012-08-23 12:48     ` Benoît Canet
  2012-08-27  7:36     ` Benoît Canet
  0 siblings, 2 replies; 9+ messages in thread
From: Daniel P. Berrange @ 2012-08-23 12:34 UTC (permalink / raw)
  To: Benoît Canet; +Cc: aliguori, qemu-devel, Benoît Canet

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 <benoit@irqsave.net>
> ---
>  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.

To deal with this issue you'd either have to use the POSIX
async I/O APIs (or QEMU's internal equivalent), 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.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls
  2012-08-23 12:34   ` Daniel P. Berrange
@ 2012-08-23 12:48     ` Benoît Canet
  2012-08-23 12:58       ` Daniel P. Berrange
  2012-08-23 13:34       ` Benoît Canet
  2012-08-27  7:36     ` Benoît Canet
  1 sibling, 2 replies; 9+ messages in thread
From: Benoît Canet @ 2012-08-23 12:48 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: aliguori, Benoît Canet, qemu-devel

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 <benoit@irqsave.net>
> > ---
> >  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), 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.
> 
> 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 :|

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls
  2012-08-23 12:48     ` Benoît Canet
@ 2012-08-23 12:58       ` Daniel P. Berrange
  2012-08-23 13:34       ` Benoît Canet
  1 sibling, 0 replies; 9+ messages in thread
From: Daniel P. Berrange @ 2012-08-23 12:58 UTC (permalink / raw)
  To: Benoît Canet; +Cc: aliguori, Benoît Canet, qemu-devel

On Thu, Aug 23, 2012 at 02:48:19PM +0200, Benoît Canet wrote:
> 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 <benoit@irqsave.net>
> > > ---
> > >  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 ?

The default migration bandwidth limit means qemu throttles the rate at
which it sends data, so you won't notice the flaw with I/O blocking.


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls
  2012-08-23 12:48     ` Benoît Canet
  2012-08-23 12:58       ` Daniel P. Berrange
@ 2012-08-23 13:34       ` Benoît Canet
  1 sibling, 0 replies; 9+ messages in thread
From: Benoît Canet @ 2012-08-23 13:34 UTC (permalink / raw)
  To: Benoît Canet; +Cc: aliguori, Benoît Canet, qemu-devel

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 <benoit@irqsave.net>
> > > ---
> > >  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 :|
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls
  2012-08-23 12:34   ` Daniel P. Berrange
  2012-08-23 12:48     ` Benoît Canet
@ 2012-08-27  7:36     ` Benoît Canet
  2012-08-28 13:11       ` Luiz Capitulino
  1 sibling, 1 reply; 9+ messages in thread
From: Benoît Canet @ 2012-08-27  7:36 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: qemu-devel, lcapitulino

Adding Luiz to the thread since he is concerned by migration.

Luiz do you have any hints on doing this properly ?

Benoît

> 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 <benoit@irqsave.net>
> > ---
> >  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.
> 
> To deal with this issue you'd either have to use the POSIX
> async I/O APIs (or QEMU's internal equivalent), 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.
> 
> 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 :|
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls
  2012-08-27  7:36     ` Benoît Canet
@ 2012-08-28 13:11       ` Luiz Capitulino
  0 siblings, 0 replies; 9+ messages in thread
From: Luiz Capitulino @ 2012-08-28 13:11 UTC (permalink / raw)
  To: Benoît Canet; +Cc: qemu-devel, Juan Jose Quintela Carreira

On Mon, 27 Aug 2012 09:36:05 +0200
Benoît Canet <benoit.canet@irqsave.net> wrote:

> Adding Luiz to the thread since he is concerned by migration.
> 
> Luiz do you have any hints on doing this properly ?

I don't. Juan is a better option though.

But, we use exec migration for this, no?

> 
> Benoît
> 
> > 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 <benoit@irqsave.net>
> > > ---
> > >  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.
> > 
> > To deal with this issue you'd either have to use the POSIX
> > async I/O APIs (or QEMU's internal equivalent), 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.
> > 
> > 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 :|
> > 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-08-28 13:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-23 12:28 [Qemu-devel] [PATCH 0/2] Add file url support to migration Benoît Canet
2012-08-23 12:28 ` [Qemu-devel] [PATCH 1/2] migration: Allow the migrate command to work on file: urls Benoît Canet
2012-08-23 12:34   ` Daniel P. Berrange
2012-08-23 12:48     ` Benoît Canet
2012-08-23 12:58       ` Daniel P. Berrange
2012-08-23 13:34       ` Benoît Canet
2012-08-27  7:36     ` Benoît Canet
2012-08-28 13:11       ` Luiz Capitulino
2012-08-23 12:28 ` [Qemu-devel] [PATCH 2/2] migration: Allow -incoming " Benoît Canet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).