| Message ID | 1345724888-30204-2-git-send-email-benoit@irqsave.net |
|---|---|
| State | New |
| Headers | show |
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
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 :|
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
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 :| >
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 :| >
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 :| > > >
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);
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(-)