[libvirt] [PATCH v2 0/8] Post-copy live migration support
Qemu currently implements pre-copy live migration. VM memory pages are first copied from the source hypervisor to the destination, potentially multiple times as pages get dirtied during transfer, then VCPU state is migrated. Unfortunately, if the VM dirties memory faster than the network bandwidth, then pre-copy cannot finish. `virsh` currently includes an option to suspend a VM after a timeout, so that migration may finish, but at the expense of downtime. A future version of qemu will implement post-copy live migration. The VCPU state is first migrated to the destination hypervisor, then memory pages are pulled from the source hypervisor. Post-copy has the potential to do migration with zero-downtime, despite the VM dirtying pages fast, with minimum performance impact. On the other hand, one post-copy is in progress, any network failure would render the VM unusable, as its memory is partitioned between the source and destination hypervisor. Therefore, post-copy should only be used when necessary. Post-copy migration in qemu will work as follows: (1) The `x-postcopy-ram` migration capability needs to be set. (2) Migration is started. (3) When the user decides so, post-copy migration is activated by sending the `migrate-start-postcopy` command. Qemu acknowledges by setting migration status to `postcopy-active`. v2: - Fixed formatting - Set target version to libvirt 1.2.10 - Only use JSON monitor - Renamed `qemuMigrateStartPostCopy` to `qemuDomainMigrateStartPostCopy` - Added parameter `flags` to domainMigrateStartPostCopy (currently unused) - Misc fixes required for `make check` - Stop perform phase, when post-copy starts - Wait for post-copy completion in confirm phase, before killing source VM Implementation note: `qemuMigrationWaitForCompletion` is overloaded. When called the first time it waits for post-copy to start, when called the second time it waits for post-copy to complete. I did so to reduce code duplication, but am not sure this is the best solution. Cristian Klein (8): Added public API to enable post-copy migration Added public API to start post-copy migration Added low-level API to qemu post-copy migration Implemented VIR_MIGRATE_POSTCOPY in qemu driver Added job type VIR_DOMAIN_JOB_PHASE1_COMPLETED Implemented post-copy migration logic in qemu Implement virDomainMigrateStartPostCopy in qemu virsh: add postcopy-after option to migrate command include/libvirt/libvirt.h.in | 5 +++ src/driver.h | 5 +++ src/libvirt.c | 46 ++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/qemu/qemu_driver.c | 58 ++++++++++++++++++++++++++++++ src/qemu/qemu_migration.c | 85 ++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_migration.h | 3 +- src/qemu/qemu_monitor.c | 24 +++++++++++-- src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 23 +++++++++++- src/qemu/qemu_monitor_json.h | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 ++++++- src/remote_protocol-structs | 5 +++ tools/virsh-domain.c | 75 ++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 5 +++ 16 files changed, 347 insertions(+), 11 deletions(-) -- 1.9.1
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5217ab3..82f3aeb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1225,6 +1225,7 @@ typedef enum { VIR_MIGRATE_ABORT_ON_ERROR = (1 << 12), /* abort migration on I/O errors happened during migration */ VIR_MIGRATE_AUTO_CONVERGE = (1 << 13), /* force convergence */ VIR_MIGRATE_RDMA_PIN_ALL = (1 << 14), /* RDMA memory pinning */ + VIR_MIGRATE_POSTCOPY = (1 << 15), /* enable (but don't start) post-copy */ } virDomainMigrateFlags; diff --git a/src/libvirt.c b/src/libvirt.c index 6dc7c73..2c8ffbe 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5265,6 +5265,7 @@ virDomainMigrateDirect(virDomainPtr domain, * automatically when supported). * VIR_MIGRATE_UNSAFE Force migration even if it is considered unsafe. * VIR_MIGRATE_OFFLINE Migrate offline + * VIR_MIGRATE_POSTCOPY Enable (but don't start) post-copy * * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. * Applications using the VIR_MIGRATE_PEER2PEER flag will probably @@ -5291,6 +5292,12 @@ virDomainMigrateDirect(virDomainPtr domain, * can use either VIR_MIGRATE_NON_SHARED_DISK or * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive. * + * If you want to enable post-copy migration you must set the + * VIR_MIGRATE_POSTCOPY flag. Once migration is active, you may + * start post-copy by calling virDomainMigrateStartPostCopy. + * When to start post-copy is entirely left to the user, libvirt + * only implements the necessary mechanism. + * * In either case it is typically only necessary to specify a * URI if the destination host has multiple interfaces and a * specific interface is required to transmit migration data. -- 1.9.1
On Tue, Sep 30, 2014 at 16:39:22 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 7 +++++++ 2 files changed, 8 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5217ab3..82f3aeb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1225,6 +1225,7 @@ typedef enum { VIR_MIGRATE_ABORT_ON_ERROR = (1 << 12), /* abort migration on I/O errors happened during migration */ VIR_MIGRATE_AUTO_CONVERGE = (1 << 13), /* force convergence */ VIR_MIGRATE_RDMA_PIN_ALL = (1 << 14), /* RDMA memory pinning */ + VIR_MIGRATE_POSTCOPY = (1 << 15), /* enable (but don't start) post-copy */ } virDomainMigrateFlags;
I still think we should add an extra flag to start post copy immediately. To address your concerns about it, I don't think it's implementing a policy in libvirt. It's for apps that want to make sure migration converges without having to spawn another thread and monitor the progress or wait for a timeout. It's a bit similar to migrating a paused domain vs. migrating a running domain and pausing it when it doesn't seem to converge. Yesterday, I talked to VDSM guys and it seems like this extra flag would be what they'd start with when implementing support for post-copy migration. It's already a significant change for them so they'd like to start with a simple usage pattern first. (I'll continue reviewing this series tomorrow.) Jirka
On Tue, Sep 30, 2014 at 05:11:03PM +0200, Jiri Denemark wrote:
On Tue, Sep 30, 2014 at 16:39:22 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 7 +++++++ 2 files changed, 8 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5217ab3..82f3aeb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1225,6 +1225,7 @@ typedef enum { VIR_MIGRATE_ABORT_ON_ERROR = (1 << 12), /* abort migration on I/O errors happened during migration */ VIR_MIGRATE_AUTO_CONVERGE = (1 << 13), /* force convergence */ VIR_MIGRATE_RDMA_PIN_ALL = (1 << 14), /* RDMA memory pinning */ + VIR_MIGRATE_POSTCOPY = (1 << 15), /* enable (but don't start) post-copy */ } virDomainMigrateFlags;
I still think we should add an extra flag to start post copy immediately. To address your concerns about it, I don't think it's implementing a policy in libvirt. It's for apps that want to make sure migration converges without having to spawn another thread and monitor the progress or wait for a timeout. It's a bit similar to migrating a paused domain vs. migrating a running domain and pausing it when it doesn't seem to converge.
Your point about spawning another thread makes me wonder if we should actually look at adding a 'VIR_MIGRATE_ASYNC' method (that would require P2P migration of course). If this flag were set, virDomainMigrateXXX would only block for long enough to start the migration and then return. Callers can use the job info API to monitor progress & success/failure. Then we wouldn't have to keep adding flags like you suggest - apps can just easily call the appropriate API right away with no threads needed Regards, 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 2014-09-30 17:16, Daniel P. Berrange wrote:
On Tue, Sep 30, 2014 at 05:11:03PM +0200, Jiri Denemark wrote:
On Tue, Sep 30, 2014 at 16:39:22 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 7 +++++++ 2 files changed, 8 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5217ab3..82f3aeb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1225,6 +1225,7 @@ typedef enum { VIR_MIGRATE_ABORT_ON_ERROR = (1 << 12), /* abort migration on I/O errors happened during migration */ VIR_MIGRATE_AUTO_CONVERGE = (1 << 13), /* force convergence */ VIR_MIGRATE_RDMA_PIN_ALL = (1 << 14), /* RDMA memory pinning */ + VIR_MIGRATE_POSTCOPY = (1 << 15), /* enable (but don't start) post-copy */ } virDomainMigrateFlags;
I still think we should add an extra flag to start post copy immediately. To address your concerns about it, I don't think it's implementing a policy in libvirt. It's for apps that want to make sure migration converges without having to spawn another thread and monitor the progress or wait for a timeout. It's a bit similar to migrating a paused domain vs. migrating a running domain and pausing it when it doesn't seem to converge.
Your point about spawning another thread makes me wonder if we should actually look at adding a 'VIR_MIGRATE_ASYNC' method (that would require P2P migration of course). If this flag were set, virDomainMigrateXXX would only block for long enough to start the migration and then return.
Callers can use the job info API to monitor progress & success/failure.
Then we wouldn't have to keep adding flags like you suggest - apps can just easily call the appropriate API right away with no threads needed
This would make a lot of sense. The user would call: """ virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY | VIR_MIGRATE_ASYNC) virDomainMigrateStartPostCopy(...) """ Would this be seen as more cumbersome than having a dedicated VIR_MIGRATE_POSTCOPY_AUTOSTART? -- Cristian Klein, PhD Post-doc @ Umeå Universitet http://www8.cs.umu.se/~cklein
On Wed, Oct 01, 2014 at 10:45:33AM +0200, Cristian KLEIN wrote:
On 2014-09-30 17:16, Daniel P. Berrange wrote:
On Tue, Sep 30, 2014 at 05:11:03PM +0200, Jiri Denemark wrote:
On Tue, Sep 30, 2014 at 16:39:22 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 7 +++++++ 2 files changed, 8 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5217ab3..82f3aeb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1225,6 +1225,7 @@ typedef enum { VIR_MIGRATE_ABORT_ON_ERROR = (1 << 12), /* abort migration on I/O errors happened during migration */ VIR_MIGRATE_AUTO_CONVERGE = (1 << 13), /* force convergence */ VIR_MIGRATE_RDMA_PIN_ALL = (1 << 14), /* RDMA memory pinning */ + VIR_MIGRATE_POSTCOPY = (1 << 15), /* enable (but don't start) post-copy */ } virDomainMigrateFlags;
I still think we should add an extra flag to start post copy immediately. To address your concerns about it, I don't think it's implementing a policy in libvirt. It's for apps that want to make sure migration converges without having to spawn another thread and monitor the progress or wait for a timeout. It's a bit similar to migrating a paused domain vs. migrating a running domain and pausing it when it doesn't seem to converge.
Your point about spawning another thread makes me wonder if we should actually look at adding a 'VIR_MIGRATE_ASYNC' method (that would require P2P migration of course). If this flag were set, virDomainMigrateXXX would only block for long enough to start the migration and then return.
Callers can use the job info API to monitor progress & success/failure.
Then we wouldn't have to keep adding flags like you suggest - apps can just easily call the appropriate API right away with no threads needed
This would make a lot of sense. The user would call:
""" virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY | VIR_MIGRATE_ASYNC) virDomainMigrateStartPostCopy(...) """
Would this be seen as more cumbersome than having a dedicated VIR_MIGRATE_POSTCOPY_AUTOSTART?
I think it is still acceptably simple - the root complaint that Jiri had was that he doesn't want apps to have to spawn threads, which this proposal of mine achieves. Regards, 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 Wed, Oct 01, 2014 at 10:45:33 +0200, Cristian KLEIN wrote:
On 2014-09-30 17:16, Daniel P. Berrange wrote:
On Tue, Sep 30, 2014 at 05:11:03PM +0200, Jiri Denemark wrote:
On Tue, Sep 30, 2014 at 16:39:22 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 7 +++++++ 2 files changed, 8 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5217ab3..82f3aeb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1225,6 +1225,7 @@ typedef enum { VIR_MIGRATE_ABORT_ON_ERROR = (1 << 12), /* abort migration on I/O errors happened during migration */ VIR_MIGRATE_AUTO_CONVERGE = (1 << 13), /* force convergence */ VIR_MIGRATE_RDMA_PIN_ALL = (1 << 14), /* RDMA memory pinning */ + VIR_MIGRATE_POSTCOPY = (1 << 15), /* enable (but don't start) post-copy */ } virDomainMigrateFlags;
I still think we should add an extra flag to start post copy immediately. To address your concerns about it, I don't think it's implementing a policy in libvirt. It's for apps that want to make sure migration converges without having to spawn another thread and monitor the progress or wait for a timeout. It's a bit similar to migrating a paused domain vs. migrating a running domain and pausing it when it doesn't seem to converge.
Your point about spawning another thread makes me wonder if we should actually look at adding a 'VIR_MIGRATE_ASYNC' method (that would require P2P migration of course). If this flag were set, virDomainMigrateXXX would only block for long enough to start the migration and then return.
Callers can use the job info API to monitor progress & success/failure.
Then we wouldn't have to keep adding flags like you suggest - apps can just easily call the appropriate API right away with no threads needed
This would make a lot of sense. The user would call:
""" virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY | VIR_MIGRATE_ASYNC) virDomainMigrateStartPostCopy(...) """
Would this be seen as more cumbersome than having a dedicated VIR_MIGRATE_POSTCOPY_AUTOSTART?
The ASYNC flag Daniel suggested makes sense, so I guess you can just ignore my request for a special flag. Although, I don't think the ASYNC stuff needs to be done within this series, let's just focus on the post-copy stuff. Jirka
On 01 Oct 2014, at 12:07 , Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Oct 01, 2014 at 10:45:33 +0200, Cristian KLEIN wrote:
On 2014-09-30 17:16, Daniel P. Berrange wrote:
On Tue, Sep 30, 2014 at 05:11:03PM +0200, Jiri Denemark wrote:
On Tue, Sep 30, 2014 at 16:39:22 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 7 +++++++ 2 files changed, 8 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5217ab3..82f3aeb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1225,6 +1225,7 @@ typedef enum { VIR_MIGRATE_ABORT_ON_ERROR = (1 << 12), /* abort migration on I/O errors happened during migration */ VIR_MIGRATE_AUTO_CONVERGE = (1 << 13), /* force convergence */ VIR_MIGRATE_RDMA_PIN_ALL = (1 << 14), /* RDMA memory pinning */ + VIR_MIGRATE_POSTCOPY = (1 << 15), /* enable (but don't start) post-copy */ } virDomainMigrateFlags;
I still think we should add an extra flag to start post copy immediately. To address your concerns about it, I don't think it's implementing a policy in libvirt. It's for apps that want to make sure migration converges without having to spawn another thread and monitor the progress or wait for a timeout. It's a bit similar to migrating a paused domain vs. migrating a running domain and pausing it when it doesn't seem to converge.
Your point about spawning another thread makes me wonder if we should actually look at adding a 'VIR_MIGRATE_ASYNC' method (that would require P2P migration of course). If this flag were set, virDomainMigrateXXX would only block for long enough to start the migration and then return.
Callers can use the job info API to monitor progress & success/failure.
Then we wouldn't have to keep adding flags like you suggest - apps can just easily call the appropriate API right away with no threads needed
This would make a lot of sense. The user would call:
""" virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY | VIR_MIGRATE_ASYNC) virDomainMigrateStartPostCopy(...) """
Would this be seen as more cumbersome than having a dedicated VIR_MIGRATE_POSTCOPY_AUTOSTART?
The ASYNC flag Daniel suggested makes sense, so I guess you can just ignore my request for a special flag. Although, I don't think the ASYNC stuff needs to be done within this series, let's just focus on the post-copy stuff.
Hi Jirka, I talked to the qemu post-copy guys (Andrea and Dave in CC). Starting post-copy immediately is a bad performance choice: The VM will start on the destination hypervisor before the read-only or kernel memory is there. This means that those pages need to be pulled on-demand, hence a lot of overhead and interruptions in the VM’s execution. Instead, it is better to first do one pass of pre-copy and only then trigger post-copy. In fact, I did an experiment with a video streaming VM and starting post-copy after the first pass of pre-copy (instead of starting post-copy immediately) reduces downtime from 3.5 seconds to under 1 second. Given all above, I propose the following post-copy API in libvirt: virDomainMigrateXXX(..., VIR_MIGRATE_ENABLE_POSTCOPY) virDomainMigrateStartPostCopy(...) // from a different thread This is for those who just need the post-copy mechanism and want to implement a policy themselves. virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY_AFTER_PRECOPY) This is for those who want to use post-copy without caring about any low-level details, offering a good enough policy for most cases. What do you think? Would you accept patches that implement this API? Cristian
Hi everyone, On Thu, Nov 06, 2014 at 09:18:04AM +0200, Cristian Klein wrote: I
talked to the qemu post-copy guys (Andrea and Dave in CC). Starting post-copy immediately is a bad performance choice: The VM will start on the destination hypervisor before the read-only or kernel memory is there. This means that those pages need to be pulled on-demand, hence a lot of overhead and interruptions in the VM’s execution.
Instead, it is better to first do one pass of pre-copy and only then trigger post-copy. In fact, I did an experiment with a video streaming VM and starting post-copy after the first pass of pre-copy (instead of starting post-copy immediately) reduces downtime from 3.5 seconds to under 1 second.
Given all above, I propose the following post-copy API in libvirt:
virDomainMigrateXXX(..., VIR_MIGRATE_ENABLE_POSTCOPY) virDomainMigrateStartPostCopy(...) // from a different thread
This is for those who just need the post-copy mechanism and want to implement a policy themselves.
virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY_AFTER_PRECOPY)
This is for those who want to use post-copy without caring about any low-level details, offering a good enough policy for most cases.
What do you think? Would you accept patches that implement this API?
I agree, even better would be to also pass a parameter to specify how many passes of precopy to run before engaging postcopy, adding at least the number of passes parameter shouldn't be a huge change and in doubt you can just define it to 1. The other things needed are: 1) adding an event that doesn't require libvirt to poll to know when the source node has been stopped (then if postcopy was engaged precopy may not have finished, but the problem remains the same as with pure precopy: we need to know efficiently when exactly the source node has been stopped without adding an average 25msec latency) 2) preparing a second socket for qemu so we can run the out of band requests of postcopy without incurring into artificial latencies created by the socket sendbuffer kept filled by the background transfer (the hack to decrease /proc/sys/net/ipv4/tcp_wmem helps tremendously, but it's unlikely to ever be as efficient as having two sockets, potentially both running on openssl etc..) Thanks, Andrea
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 3 +++ src/driver.h | 5 +++++ src/libvirt.c | 39 +++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 ++++++++++++- src/remote_protocol-structs | 5 +++++ 7 files changed, 70 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 82f3aeb..84cd5a4 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1346,6 +1346,9 @@ int virDomainMigrateToURI3(virDomainPtr domain, unsigned int nparams, unsigned int flags); +int virDomainMigrateStartPostCopy (virDomainPtr domain, + unsigned int flags); + int virDomainMigrateSetMaxDowntime (virDomainPtr domain, unsigned long long downtime, unsigned int flags); diff --git a/src/driver.h b/src/driver.h index dc62a8e..9eb29e2 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1221,6 +1221,10 @@ typedef int unsigned int cellCount, unsigned int flags); +typedef int +(*virDrvDomainMigrateStartPostCopy)(virDomainPtr domain, + unsigned int flags); + typedef struct _virDriver virDriver; typedef virDriver *virDriverPtr; @@ -1445,6 +1449,7 @@ struct _virDriver { virDrvConnectGetDomainCapabilities connectGetDomainCapabilities; virDrvConnectGetAllDomainStats connectGetAllDomainStats; virDrvNodeAllocPages nodeAllocPages; + virDrvDomainMigrateStartPostCopy domainMigrateStartPostCopy; }; diff --git a/src/libvirt.c b/src/libvirt.c index 2c8ffbe..5bb1897 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17803,6 +17803,45 @@ virDomainMigrateSetCompressionCache(virDomainPtr domain, /** + * virDomainMigrateStartPostCopy: + * @domain: a domain object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Starts post-copy migration. This function has to be called while + * migration (initially pre-copy) is in progress. The migration operation + * must be called with the VIR_MIGRATE_POSTCOPY flag. + * + * Returns 0 in case of success, -1 otherwise. + */ +int +virDomainMigrateStartPostCopy(virDomainPtr domain, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainMigrateStartPostCopy) { + if (conn->driver->domainMigrateStartPostCopy(domain, flags) < 0) + goto error; + return 0; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} + + +/** * virDomainMigrateSetMaxSpeed: * @domain: a domain object * @bandwidth: migration bandwidth limit in MiB/s diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 5f95802..190eb11 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -684,4 +684,9 @@ LIBVIRT_1.2.9 { virNodeAllocPages; } LIBVIRT_1.2.8; +LIBVIRT_1.2.10 { + global: + virDomainMigrateStartPostCopy; +} LIBVIRT_1.2.9; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 6c49e49..03257ec 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8239,6 +8239,7 @@ static virDriver remote_driver = { .connectGetDomainCapabilities = remoteConnectGetDomainCapabilities, /* 1.2.7 */ .connectGetAllDomainStats = remoteConnectGetAllDomainStats, /* 1.2.8 */ .nodeAllocPages = remoteNodeAllocPages, /* 1.2.9 */ + .domainMigrateStartPostCopy = remoteDomainMigrateStartPostCopy, /* 1.2.10 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index db12cda..dfbc119 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3111,6 +3111,11 @@ struct remote_connect_get_all_domain_stats_args { struct remote_connect_get_all_domain_stats_ret { remote_domain_stats_record retStats<REMOTE_DOMAIN_LIST_MAX>; }; + +struct remote_domain_migrate_start_post_copy_args { + remote_nonnull_domain dom; + unsigned int flags; +}; /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -5505,5 +5510,11 @@ enum remote_procedure { * @generate: none * @acl: connect:write */ - REMOTE_PROC_NODE_ALLOC_PAGES = 347 + REMOTE_PROC_NODE_ALLOC_PAGES = 347, + + /** + * @generate: both + * @acl: domain:migrate + */ + REMOTE_PROC_DOMAIN_MIGRATE_START_POST_COPY = 348 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 362baf9..fd98c80 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2579,6 +2579,10 @@ struct remote_connect_get_all_domain_stats_ret { remote_domain_stats_record * retStats_val; } retStats; }; +struct remote_domain_migrate_start_post_copy_args { + remote_nonnull_domain dom; + u_int flags; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -2927,4 +2931,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_COPY = 345, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE = 346, REMOTE_PROC_NODE_ALLOC_PAGES = 347, + REMOTE_PROC_DOMAIN_MIGRATE_START_POST_COPY = 348, }; -- 1.9.1
On Tue, Sep 30, 2014 at 16:39:23 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 3 +++ src/driver.h | 5 +++++ src/libvirt.c | 39 +++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 ++++++++++++- src/remote_protocol-structs | 5 +++++ 7 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 82f3aeb..84cd5a4 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1346,6 +1346,9 @@ int virDomainMigrateToURI3(virDomainPtr domain, unsigned int nparams, unsigned int flags);
+int virDomainMigrateStartPostCopy (virDomainPtr domain, + unsigned int flags); + int virDomainMigrateSetMaxDowntime (virDomainPtr domain, unsigned long long downtime, unsigned int flags); ...
Looks good and I also checked the generator in libvirt-python creates correct implementation of this API. No extra manual work is needed here. Jirka
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- src/qemu/qemu_monitor.c | 24 ++++++++++++++++++++++-- src/qemu/qemu_monitor.h | 4 ++++ src/qemu/qemu_monitor_json.c | 23 ++++++++++++++++++++++- src/qemu/qemu_monitor_json.h | 1 + 4 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 543384d..dbd854c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -117,11 +117,11 @@ VIR_ONCE_GLOBAL_INIT(qemuMonitor) VIR_ENUM_IMPL(qemuMonitorMigrationStatus, QEMU_MONITOR_MIGRATION_STATUS_LAST, - "inactive", "active", "completed", "failed", "cancelled", "setup") + "inactive", "active", "completed", "failed", "cancelled", "setup", "postcopy-active") VIR_ENUM_IMPL(qemuMonitorMigrationCaps, QEMU_MONITOR_MIGRATION_CAPS_LAST, - "xbzrle", "auto-converge", "rdma-pin-all") + "xbzrle", "auto-converge", "rdma-pin-all", "x-postcopy-ram") VIR_ENUM_IMPL(qemuMonitorVMStatus, QEMU_MONITOR_VM_STATUS_LAST, @@ -2378,6 +2378,26 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, return ret; } +int qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon) +{ + VIR_DEBUG("mon=%p", mon); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONMigrateStartPostCopy(mon); +} + + int qemuMonitorMigrateCancel(qemuMonitorPtr mon) { int ret; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index adf18ab..e42c6af 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -427,6 +427,7 @@ enum { QEMU_MONITOR_MIGRATION_STATUS_ERROR, QEMU_MONITOR_MIGRATION_STATUS_CANCELLED, QEMU_MONITOR_MIGRATION_STATUS_SETUP, + QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_ACTIVE, QEMU_MONITOR_MIGRATION_STATUS_LAST }; @@ -480,6 +481,7 @@ typedef enum { QEMU_MONITOR_MIGRATION_CAPS_XBZRLE, QEMU_MONITOR_MIGRATION_CAPS_AUTO_CONVERGE, QEMU_MONITOR_MIGRATION_CAPS_RDMA_PIN_ALL, + QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY, QEMU_MONITOR_MIGRATION_CAPS_LAST } qemuMonitorMigrationCaps; @@ -535,6 +537,8 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, unsigned int flags, const char *unixfile); +int qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon); + int qemuMonitorMigrateCancel(qemuMonitorPtr mon); int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index b3b6451..1a9e6e1 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2550,7 +2550,8 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply, status->setup_time_set = true; if (status->status == QEMU_MONITOR_MIGRATION_STATUS_ACTIVE || - status->status == QEMU_MONITOR_MIGRATION_STATUS_COMPLETED) { + status->status == QEMU_MONITOR_MIGRATION_STATUS_COMPLETED || + status->status == QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_ACTIVE) { virJSONValuePtr ram = virJSONValueObjectGet(ret, "ram"); if (!ram) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -2790,6 +2791,26 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon, return ret; } +int qemuMonitorJSONMigrateStartPostCopy(qemuMonitorPtr mon) +{ + int ret; + virJSONValuePtr cmd; + cmd = qemuMonitorJSONMakeCommand("migrate-start-postcopy", NULL); + + virJSONValuePtr reply = NULL; + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) { int ret; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index ef9b552..3fba203 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -149,6 +149,7 @@ int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitorPtr mon, bool *spice_migrated); +int qemuMonitorJSONMigrateStartPostCopy(qemuMonitorPtr mon); int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon); int qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, -- 1.9.1
On Tue, Sep 30, 2014 at 16:39:24 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- src/qemu/qemu_monitor.c | 24 ++++++++++++++++++++++-- src/qemu/qemu_monitor.h | 4 ++++ src/qemu/qemu_monitor_json.c | 23 ++++++++++++++++++++++- src/qemu/qemu_monitor_json.h | 1 + 4 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 543384d..dbd854c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -117,11 +117,11 @@ VIR_ONCE_GLOBAL_INIT(qemuMonitor)
VIR_ENUM_IMPL(qemuMonitorMigrationStatus, QEMU_MONITOR_MIGRATION_STATUS_LAST, - "inactive", "active", "completed", "failed", "cancelled", "setup") + "inactive", "active", "completed", "failed", "cancelled", "setup", "postcopy-active")
VIR_ENUM_IMPL(qemuMonitorMigrationCaps, QEMU_MONITOR_MIGRATION_CAPS_LAST, - "xbzrle", "auto-converge", "rdma-pin-all") + "xbzrle", "auto-converge", "rdma-pin-all", "x-postcopy-ram")
As already mentioned, we don't want to support unstable QEMU interfaces so we will have to wait with this series until the "x-" prefix is removed in QEMU. Otherwise this patch looks good. Jirka
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- src/qemu/qemu_migration.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 3 ++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 284cd5a..4a36946 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1801,6 +1801,49 @@ qemuMigrationSetOffline(virQEMUDriverPtr driver, static int +qemuMigrationSetPostCopy(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob job) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, job) < 0) + return -1; + + ret = qemuMonitorGetMigrationCapability( + priv->mon, + QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY); + + if (ret < 0) { + goto cleanup; + } else if (ret == 0) { + if (job == QEMU_ASYNC_JOB_MIGRATION_IN) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Post-copy migration is not supported by " + "target QEMU binary")); + } else { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Post-copy migration is not supported by " + "source QEMU binary")); + } + ret = -1; + goto cleanup; + } + + /* Post-copy only needs to be enabled on source qemu, + * for target, this function only acts as a capability check */ + if (job == QEMU_ASYNC_JOB_MIGRATION_OUT) { + ret = qemuMonitorSetMigrationCapability( + priv->mon, + QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY); + } + + cleanup: + qemuDomainObjExitMonitor(driver, vm); + return ret; +} +static int qemuMigrationSetCompression(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob job) @@ -2741,6 +2784,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, dataFD[1] = -1; /* 'st' owns the FD now & will close it */ } + if (flags & VIR_MIGRATE_POSTCOPY && + qemuMigrationSetPostCopy(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_IN) < 0) + goto stop; + if (flags & VIR_MIGRATE_COMPRESSED && qemuMigrationSetCompression(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) @@ -3583,6 +3631,18 @@ qemuMigrationRun(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) goto cleanup; + if (flags & VIR_MIGRATE_POSTCOPY) { + if (!(flags & VIR_MIGRATE_LIVE)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Enabling post-copy only makes sense with " + "live migration")); + goto cleanup; + } + if (qemuMigrationSetPostCopy(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) + goto cleanup; + } + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) goto cleanup; diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index e7a90c3..349c9c4 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -41,7 +41,8 @@ VIR_MIGRATE_COMPRESSED | \ VIR_MIGRATE_ABORT_ON_ERROR | \ VIR_MIGRATE_AUTO_CONVERGE | \ - VIR_MIGRATE_RDMA_PIN_ALL) + VIR_MIGRATE_RDMA_PIN_ALL | \ + VIR_MIGRATE_POSTCOPY) /* All supported migration parameters and their types. */ # define QEMU_MIGRATION_PARAMETERS \ -- 1.9.1
On Tue, Sep 30, 2014 at 16:39:25 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- src/qemu/qemu_migration.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 3 ++- 2 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 284cd5a..4a36946 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1801,6 +1801,49 @@ qemuMigrationSetOffline(virQEMUDriverPtr driver,
static int +qemuMigrationSetPostCopy(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob job) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, job) < 0) + return -1; + + ret = qemuMonitorGetMigrationCapability( + priv->mon, + QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY); + + if (ret < 0) { + goto cleanup; + } else if (ret == 0) { + if (job == QEMU_ASYNC_JOB_MIGRATION_IN) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Post-copy migration is not supported by " + "target QEMU binary")); + } else { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Post-copy migration is not supported by " + "source QEMU binary")); + } + ret = -1; + goto cleanup; + } + + /* Post-copy only needs to be enabled on source qemu, + * for target, this function only acts as a capability check */ + if (job == QEMU_ASYNC_JOB_MIGRATION_OUT) { + ret = qemuMonitorSetMigrationCapability( + priv->mon, + QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY); + } + + cleanup: + qemuDomainObjExitMonitor(driver, vm); + return ret; +}
Unless we need to try to set the capability to check whether it is really available (which is not what the current code does), I think it would make more sense to limit this function to source side only and let destination call qemuMonitorGetMigrationCapability directly. Empty lines missing between qemuMigrationSetPostCopy and qemuMigrationSetCompression.
+static int qemuMigrationSetCompression(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob job) @@ -2741,6 +2784,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, dataFD[1] = -1; /* 'st' owns the FD now & will close it */ }
+ if (flags & VIR_MIGRATE_POSTCOPY && + qemuMigrationSetPostCopy(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_IN) < 0) + goto stop; +
Just call qemuMonitorGetMigrationCapability directly here.
if (flags & VIR_MIGRATE_COMPRESSED && qemuMigrationSetCompression(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) @@ -3583,6 +3631,18 @@ qemuMigrationRun(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) goto cleanup;
+ if (flags & VIR_MIGRATE_POSTCOPY) { + if (!(flags & VIR_MIGRATE_LIVE)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Enabling post-copy only makes sense with " + "live migration")); + goto cleanup; + } + if (qemuMigrationSetPostCopy(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) + goto cleanup; + } + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) goto cleanup; diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index e7a90c3..349c9c4 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -41,7 +41,8 @@ VIR_MIGRATE_COMPRESSED | \ VIR_MIGRATE_ABORT_ON_ERROR | \ VIR_MIGRATE_AUTO_CONVERGE | \ - VIR_MIGRATE_RDMA_PIN_ALL) + VIR_MIGRATE_RDMA_PIN_ALL | \ + VIR_MIGRATE_POSTCOPY)
/* All supported migration parameters and their types. */ # define QEMU_MIGRATION_PARAMETERS \
Jirka
Some jobs feature several phases. For example, post-copy migration is composed of a first phase, from migration start to switching to post-copy, and a second phase, to migration completion. This job type allows to flag that the job has completed the first phase, but is not yet fully completed. Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 1 + tools/virsh-domain.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 84cd5a4..81044f0 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4307,6 +4307,7 @@ typedef enum { VIR_DOMAIN_JOB_COMPLETED = 3, /* Job has finished, but isn't cleaned up */ VIR_DOMAIN_JOB_FAILED = 4, /* Job hit error, but isn't cleaned up */ VIR_DOMAIN_JOB_CANCELLED = 5, /* Job was aborted, but isn't cleaned up */ + VIR_DOMAIN_JOB_PHASE1_COMPLETED = 6, /* Job completed first phase, e.g., post-copy activation */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_JOB_LAST diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ce59406..36a6d52 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5407,7 +5407,8 @@ VIR_ENUM_IMPL(vshDomainJob, N_("Unbounded"), N_("Completed"), N_("Failed"), - N_("Cancelled")) + N_("Cancelled"), + N_("InPhase2")) static const char * vshDomainJobToString(int type) -- 1.9.1
On Tue, Sep 30, 2014 at 16:39:26 +0200, Cristian Klein wrote:
Some jobs feature several phases. For example, post-copy migration is composed of a first phase, from migration start to switching to post-copy, and a second phase, to migration completion. This job type allows to flag that the job has completed the first phase, but is not yet fully completed.
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 1 + tools/virsh-domain.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 84cd5a4..81044f0 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4307,6 +4307,7 @@ typedef enum { VIR_DOMAIN_JOB_COMPLETED = 3, /* Job has finished, but isn't cleaned up */ VIR_DOMAIN_JOB_FAILED = 4, /* Job hit error, but isn't cleaned up */ VIR_DOMAIN_JOB_CANCELLED = 5, /* Job was aborted, but isn't cleaned up */ + VIR_DOMAIN_JOB_PHASE1_COMPLETED = 6, /* Job completed first phase, e.g., post-copy activation */
This is not a job type. If we need to advertise this to libvirt clients, we may introduce a new job statistics typed parameter but we should not misuse job type. And I'm not entirely convinced we need to advertise this. To me it seems the only interested thing is whether a domain is still running on the source host or it was already resumed on the destination host. And it's easy to get this kind of information via existing ways, e.g., listening to domain life cycle events or by checking domain's status. Jirka
On 07 Oct 2014, at 22:08 , Jiri Denemark <jdenemar@redhat.com> wrote:
On Tue, Sep 30, 2014 at 16:39:26 +0200, Cristian Klein wrote:
Some jobs feature several phases. For example, post-copy migration is composed of a first phase, from migration start to switching to post-copy, and a second phase, to migration completion. This job type allows to flag that the job has completed the first phase, but is not yet fully completed.
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 1 + tools/virsh-domain.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 84cd5a4..81044f0 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4307,6 +4307,7 @@ typedef enum { VIR_DOMAIN_JOB_COMPLETED = 3, /* Job has finished, but isn't cleaned up */ VIR_DOMAIN_JOB_FAILED = 4, /* Job hit error, but isn't cleaned up */ VIR_DOMAIN_JOB_CANCELLED = 5, /* Job was aborted, but isn't cleaned up */ + VIR_DOMAIN_JOB_PHASE1_COMPLETED = 6, /* Job completed first phase, e.g., post-copy activation */
This is not a job type. If we need to advertise this to libvirt clients, we may introduce a new job statistics typed parameter but we should not misuse job type. And I'm not entirely convinced we need to advertise this. To me it seems the only interested thing is whether a domain is still running on the source host or it was already resumed on the destination host. And it's easy to get this kind of information via existing ways, e.g., listening to domain life cycle events or by checking domain's status.
My rationale for introducing this flag was to prevent a future breakage of an internal API. I felt like none of the previous flags where accurately representing the status of this job after post-copy has started. Sure, I could use `job->status.status` in `qemuMigrationWaitForCompletion`, but I was concerned that a future `libvirt` addition might not take this subtly into account and might break post-copy. If you think I should go ahead, suppress this job type and use `job->status.status` instead, what should I use instead, `VIR_DOMAIN_JOB_UNBOUNDED` or `VIR_DOMAIN_JOB_BOUNDED`? To be honest, I’m not sure to have completely understood the difference between them, except that `VIR_DOMAIN_JOB_BOUNDED` seems to be used to signal that a job is almost complete.
On Wed, Oct 08, 2014 at 23:45:17 +0900, Cristian Klein wrote:
On 07 Oct 2014, at 22:08 , Jiri Denemark <jdenemar@redhat.com> wrote:
On Tue, Sep 30, 2014 at 16:39:26 +0200, Cristian Klein wrote:
Some jobs feature several phases. For example, post-copy migration is composed of a first phase, from migration start to switching to post-copy, and a second phase, to migration completion. This job type allows to flag that the job has completed the first phase, but is not yet fully completed.
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- include/libvirt/libvirt.h.in | 1 + tools/virsh-domain.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 84cd5a4..81044f0 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4307,6 +4307,7 @@ typedef enum { VIR_DOMAIN_JOB_COMPLETED = 3, /* Job has finished, but isn't cleaned up */ VIR_DOMAIN_JOB_FAILED = 4, /* Job hit error, but isn't cleaned up */ VIR_DOMAIN_JOB_CANCELLED = 5, /* Job was aborted, but isn't cleaned up */ + VIR_DOMAIN_JOB_PHASE1_COMPLETED = 6, /* Job completed first phase, e.g., post-copy activation */
This is not a job type. If we need to advertise this to libvirt clients, we may introduce a new job statistics typed parameter but we should not misuse job type. And I'm not entirely convinced we need to advertise this. To me it seems the only interested thing is whether a domain is still running on the source host or it was already resumed on the destination host. And it's easy to get this kind of information via existing ways, e.g., listening to domain life cycle events or by checking domain's status.
My rationale for introducing this flag was to prevent a future breakage of an internal API. I felt like none of the previous flags where accurately representing the status of this job after post-copy has started.
On the other hand, you changed something that is externally visible. Apps would be pretty confused by job type changing from UNBOUNDED to PHASE1_COMPLETED during migration.
Sure, I could use `job->status.status` in `qemuMigrationWaitForCompletion`, but I was concerned that a future `libvirt` addition might not take this subtly into account and might break post-copy.
Well, we have to be careful :-) Also I'm not sure how the new VIR_DOMAIN_JOB_PHASE1_COMPLETED would help...
If you think I should go ahead, suppress this job type and use `job->status.status` instead, what should I use instead, `VIR_DOMAIN_JOB_UNBOUNDED` or `VIR_DOMAIN_JOB_BOUNDED`? To be honest, I’m not sure to have completely understood the difference between them, except that `VIR_DOMAIN_JOB_BOUNDED` seems to be used to signal that a job is almost complete.
virDomainJobType is a bit overloaded since it is mostly used to describe what kind of job is running and also about its state. - VIR_DOMAIN_JOB_NONE - no job is currently running - VIR_DOMAIN_JOB_BOUNDED - there is a job running and we know how much data we will need to process during the job. That is, apps may easily indicate its progress because we report the total amount of data and how much we already processed. Currently we don't have such jobs in libvirt. - VIR_DOMAIN_JOB_UNBOUNDED - there is a job running and we have no idea how much data we will need to process. That is, while we are processing data, something we already processed gets changed and we need to process it again. This is a live migration, once all memory is processed, we need to process the memory that changed in the meantime... - VIR_DOMAIN_JOB_COMPLETED - there was a job running and it has just finished (very hard to be seen externally until recently when we added support for explicitly querying statistics about a completed job) - VIR_DOMAIN_JOB_FAILED - there was a job running and it failed - VIR_DOMAIN_JOB_CANCELLED - there was a job running and it was cancelled either by libvirt itself or by a user/app Apps know VIR_DOMAIN_{UN,}BOUNDED job mean there is a job running. We can't change that without breaking them. Jirka
On 10/09/2014 07:09 AM, Jiri Denemark wrote:
virDomainJobType is a bit overloaded since it is mostly used to describe what kind of job is running and also about its state.
- VIR_DOMAIN_JOB_NONE - no job is currently running - VIR_DOMAIN_JOB_BOUNDED - there is a job running and we know how much data we will need to process during the job. That is, apps may easily indicate its progress because we report the total amount of data and how much we already processed. Currently we don't have such jobs in libvirt.
Technically, some of the block jobs might fit this category; for example, a block commit of an intermediate layer has a finite amount of effort to perform. Active commit might be slightly unbounded, depending on whether the guest is performing I/O faster than the merge can flush it into the backing chain, but even then, it is like all other block jobs in that it provides a x/y completion metric; even if y is growing during the course of the operation, you can estimate progress by how fast x is converging to the value of y. On the other hand, we haven't (yet) merged block jobs and domain jobs into a common framework yet. I'd also like to get to the point where we have storage volume jobs, some of which may be bounded.
- VIR_DOMAIN_JOB_UNBOUNDED - there is a job running and we have no idea how much data we will need to process. That is, while we are processing data, something we already processed gets changed and we need to process it again. This is a live migration, once all memory is processed, we need to process the memory that changed in the meantime...
Hmm, that also sounds like active commit :)
- VIR_DOMAIN_JOB_COMPLETED - there was a job running and it has just finished (very hard to be seen externally until recently when we added support for explicitly querying statistics about a completed job) - VIR_DOMAIN_JOB_FAILED - there was a job running and it failed - VIR_DOMAIN_JOB_CANCELLED - there was a job running and it was cancelled either by libvirt itself or by a user/app
Apps know VIR_DOMAIN_{UN,}BOUNDED job mean there is a job running. We can't change that without breaking them.
I agree that adding new states is not a very good idea at this point, but we already know we need to someday enhance the job system to allow parallel jobs. Having more details about a running job, such as whether it is in first or second stage, may make sense. After all, for both block copy and active commit, we DO report two different job states for whether the job has moved between two stages. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Perform phase stops once migration switched to post-copy. Confirm phase waits for post-copy to finish before killing the VM. Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_migration.c | 25 ++++++++++++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e873d45..3fe2216 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10942,6 +10942,14 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, virCheckFlags(QEMU_MIGRATION_FLAGS, -1); + if (flags & VIR_MIGRATE_POSTCOPY) { + /* post-copy migration does not work with Sequence v2 */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Post-copy migration requested but not " + "supported by v2 protocol")); + goto cleanup; + } + if (flags & VIR_MIGRATE_TUNNELLED) { /* this is a logical error; we never should have gotten here with * VIR_MIGRATE_TUNNELLED set diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4a36946..436b701 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2039,6 +2039,11 @@ qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver, ret = 0; break; + case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_ACTIVE: + jobInfo->type = VIR_DOMAIN_JOB_PHASE1_COMPLETED; + ret = 0; + break; + case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: jobInfo->type = VIR_DOMAIN_JOB_NONE; virReportError(VIR_ERR_OPERATION_FAILED, @@ -2077,6 +2082,7 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, qemuDomainJobInfoPtr jobInfo = priv->job.current; const char *job; int pauseReason; + bool inPhase2 = (jobInfo->type == VIR_DOMAIN_JOB_PHASE1_COMPLETED); switch (priv->job.asyncJob) { case QEMU_ASYNC_JOB_MIGRATION_OUT: @@ -2092,9 +2098,11 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, job = _("job"); } - jobInfo->type = VIR_DOMAIN_JOB_UNBOUNDED; + if (!inPhase2) + jobInfo->type = VIR_DOMAIN_JOB_UNBOUNDED; - while (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) { + while (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED || + (inPhase2 && jobInfo->type == VIR_DOMAIN_JOB_PHASE1_COMPLETED)) { /* Poll every 50ms for progress & to allow cancellation */ struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull }; @@ -2123,7 +2131,8 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, virObjectLock(vm); } - if (jobInfo->type == VIR_DOMAIN_JOB_COMPLETED) { + if (jobInfo->type == VIR_DOMAIN_JOB_COMPLETED || + jobInfo->type == VIR_DOMAIN_JOB_PHASE1_COMPLETED) { qemuDomainJobInfoUpdateDowntime(jobInfo); VIR_FREE(priv->job.completed); if (VIR_ALLOC(priv->job.completed) == 0) @@ -3149,6 +3158,16 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, virCheckFlags(QEMU_MIGRATION_FLAGS, -1); + /* Wait for post-copy to complete */ + if (flags & VIR_MIGRATE_POSTCOPY) { + bool abort_on_error = !!(flags & VIR_MIGRATE_ABORT_ON_ERROR); + rv = qemuMigrationWaitForCompletion(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT, + conn, abort_on_error); + if (rv < 0) + goto cleanup; + } + qemuMigrationJobSetPhase(driver, vm, retcode == 0 ? QEMU_MIGRATION_PHASE_CONFIRM3 -- 1.9.1
On Tue, Sep 30, 2014 at 16:39:27 +0200, Cristian Klein wrote:
Perform phase stops once migration switched to post-copy. Confirm phase waits for post-copy to finish before killing the VM.
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_migration.c | 25 ++++++++++++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e873d45..3fe2216 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10942,6 +10942,14 @@ qemuDomainMigratePrepare2(virConnectPtr dconn,
virCheckFlags(QEMU_MIGRATION_FLAGS, -1);
+ if (flags & VIR_MIGRATE_POSTCOPY) { + /* post-copy migration does not work with Sequence v2 */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Post-copy migration requested but not " + "supported by v2 protocol")); + goto cleanup; + } +
This code should be unreachable. If both source and destination daemons support VIR_MIGRATE_POSTCOPY, they support v3 protocol as well. And a client new enough to specify VIR_MIGRATE_POSTCOPY will also support v3 migration protocol. However, it doesn't hurt to check this to be safe.
if (flags & VIR_MIGRATE_TUNNELLED) { /* this is a logical error; we never should have gotten here with * VIR_MIGRATE_TUNNELLED set diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4a36946..436b701 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2039,6 +2039,11 @@ qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver, ret = 0; break;
+ case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_ACTIVE: + jobInfo->type = VIR_DOMAIN_JOB_PHASE1_COMPLETED; + ret = 0; + break; +
This will need to be dropped after 5/8 is removed. However, it reminds me... enum { QEMU_MONITOR_MIGRATION_STATUS_INACTIVE, QEMU_MONITOR_MIGRATION_STATUS_ACTIVE, QEMU_MONITOR_MIGRATION_STATUS_COMPLETED, QEMU_MONITOR_MIGRATION_STATUS_ERROR, QEMU_MONITOR_MIGRATION_STATUS_CANCELLED, QEMU_MONITOR_MIGRATION_STATUS_SETUP, QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_ACTIVE, QEMU_MONITOR_MIGRATION_STATUS_LAST }; in qemu_monitor.h needs to be turned into a proper typedef so that switch (status.status) { line in qemuMigrationUpdateJobStatus may be changed to explicitly mention the enum so that the compiler may report a warning whenever we add new status but forgot to handle it in this switch. Which means the new state will need to be handled in the same patch it was introduced, i.e, in 3/8.
case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: jobInfo->type = VIR_DOMAIN_JOB_NONE; virReportError(VIR_ERR_OPERATION_FAILED, @@ -2077,6 +2082,7 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, qemuDomainJobInfoPtr jobInfo = priv->job.current; const char *job; int pauseReason; + bool inPhase2 = (jobInfo->type == VIR_DOMAIN_JOB_PHASE1_COMPLETED);
I think it would be cleaner to pass this info in a new parameter for qemuMigrationWaitForCompletion instead of doing a hidden black magic here.
switch (priv->job.asyncJob) { case QEMU_ASYNC_JOB_MIGRATION_OUT: @@ -2092,9 +2098,11 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, job = _("job"); }
- jobInfo->type = VIR_DOMAIN_JOB_UNBOUNDED; + if (!inPhase2) + jobInfo->type = VIR_DOMAIN_JOB_UNBOUNDED;
- while (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) { + while (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED || + (inPhase2 && jobInfo->type == VIR_DOMAIN_JOB_PHASE1_COMPLETED)) {
Just use jobInfo->status.status directly.
/* Poll every 50ms for progress & to allow cancellation */ struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull };
@@ -2123,7 +2131,8 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, virObjectLock(vm); }
- if (jobInfo->type == VIR_DOMAIN_JOB_COMPLETED) { + if (jobInfo->type == VIR_DOMAIN_JOB_COMPLETED || + jobInfo->type == VIR_DOMAIN_JOB_PHASE1_COMPLETED) {
This shouldn't be needed when 5/8 is dropped.
qemuDomainJobInfoUpdateDowntime(jobInfo); VIR_FREE(priv->job.completed); if (VIR_ALLOC(priv->job.completed) == 0) @@ -3149,6 +3158,16 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver,
virCheckFlags(QEMU_MIGRATION_FLAGS, -1);
+ /* Wait for post-copy to complete */ + if (flags & VIR_MIGRATE_POSTCOPY) { + bool abort_on_error = !!(flags & VIR_MIGRATE_ABORT_ON_ERROR); + rv = qemuMigrationWaitForCompletion(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT, + conn, abort_on_error); + if (rv < 0) + goto cleanup; + } +
This has to be done after setting the phase. We may need to add a new so that we can recover from post-copy migrations when libvirtd is restarted.
qemuMigrationJobSetPhase(driver, vm, retcode == 0 ? QEMU_MIGRATION_PHASE_CONFIRM3
Jirka
On 07 Oct 2014, at 23:02 , Jiri Denemark <jdenemar@redhat.com> wrote:
On Tue, Sep 30, 2014 at 16:39:27 +0200, Cristian Klein wrote:
Perform phase stops once migration switched to post-copy. Confirm phase waits for post-copy to finish before killing the VM.
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_migration.c | 25 ++++++++++++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e873d45..3fe2216 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10942,6 +10942,14 @@ qemuDomainMigratePrepare2(virConnectPtr dconn,
virCheckFlags(QEMU_MIGRATION_FLAGS, -1);
+ if (flags & VIR_MIGRATE_POSTCOPY) { + /* post-copy migration does not work with Sequence v2 */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Post-copy migration requested but not " + "supported by v2 protocol")); + goto cleanup; + } +
This code should be unreachable. If both source and destination daemons support VIR_MIGRATE_POSTCOPY, they support v3 protocol as well. And a client new enough to specify VIR_MIGRATE_POSTCOPY will also support v3 migration protocol. However, it doesn't hurt to check this to be safe.
if (flags & VIR_MIGRATE_TUNNELLED) { /* this is a logical error; we never should have gotten here with * VIR_MIGRATE_TUNNELLED set diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4a36946..436b701 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2039,6 +2039,11 @@ qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver, ret = 0; break;
+ case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_ACTIVE: + jobInfo->type = VIR_DOMAIN_JOB_PHASE1_COMPLETED; + ret = 0; + break; +
This will need to be dropped after 5/8 is removed. However, it reminds me...
enum { QEMU_MONITOR_MIGRATION_STATUS_INACTIVE, QEMU_MONITOR_MIGRATION_STATUS_ACTIVE, QEMU_MONITOR_MIGRATION_STATUS_COMPLETED, QEMU_MONITOR_MIGRATION_STATUS_ERROR, QEMU_MONITOR_MIGRATION_STATUS_CANCELLED, QEMU_MONITOR_MIGRATION_STATUS_SETUP, QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_ACTIVE,
QEMU_MONITOR_MIGRATION_STATUS_LAST };
in qemu_monitor.h needs to be turned into a proper typedef so that
switch (status.status) {
line in qemuMigrationUpdateJobStatus may be changed to explicitly mention the enum so that the compiler may report a warning whenever we add new status but forgot to handle it in this switch.
What about `QEMU_MONITOR_MIGRATION_STATUS_LAST`? Should this be included in a switch with an assertion that that code should never be reached?
Which means the new state will need to be handled in the same patch it was introduced, i.e, in 3/8.
case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: jobInfo->type = VIR_DOMAIN_JOB_NONE; virReportError(VIR_ERR_OPERATION_FAILED, @@ -2077,6 +2082,7 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, qemuDomainJobInfoPtr jobInfo = priv->job.current; const char *job; int pauseReason; + bool inPhase2 = (jobInfo->type == VIR_DOMAIN_JOB_PHASE1_COMPLETED);
I think it would be cleaner to pass this info in a new parameter for qemuMigrationWaitForCompletion instead of doing a hidden black magic here.
I agree with you, I wasn’t sure about with this way of coding things either. Is it okey to ask developers to always create a new intermediate parameter to call this function, so as to make its usage easier to read? E.g., “”” bool return_on_postcopy_active = false; rv = qemuMigrationWaitForCompletion(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, conn, abort_on_error, return_on_postcopy_active); “”"
switch (priv->job.asyncJob) { case QEMU_ASYNC_JOB_MIGRATION_OUT: @@ -2092,9 +2098,11 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, job = _("job"); }
- jobInfo->type = VIR_DOMAIN_JOB_UNBOUNDED; + if (!inPhase2) + jobInfo->type = VIR_DOMAIN_JOB_UNBOUNDED;
- while (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) { + while (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED || + (inPhase2 && jobInfo->type == VIR_DOMAIN_JOB_PHASE1_COMPLETED)) {
Just use jobInfo->status.status directly.
/* Poll every 50ms for progress & to allow cancellation */ struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull };
@@ -2123,7 +2131,8 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver, virObjectLock(vm); }
- if (jobInfo->type == VIR_DOMAIN_JOB_COMPLETED) { + if (jobInfo->type == VIR_DOMAIN_JOB_COMPLETED || + jobInfo->type == VIR_DOMAIN_JOB_PHASE1_COMPLETED) {
This shouldn't be needed when 5/8 is dropped.
qemuDomainJobInfoUpdateDowntime(jobInfo); VIR_FREE(priv->job.completed); if (VIR_ALLOC(priv->job.completed) == 0) @@ -3149,6 +3158,16 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver,
virCheckFlags(QEMU_MIGRATION_FLAGS, -1);
+ /* Wait for post-copy to complete */ + if (flags & VIR_MIGRATE_POSTCOPY) { + bool abort_on_error = !!(flags & VIR_MIGRATE_ABORT_ON_ERROR); + rv = qemuMigrationWaitForCompletion(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT, + conn, abort_on_error); + if (rv < 0) + goto cleanup; + } +
This has to be done after setting the phase. We may need to add a new so that we can recover from post-copy migrations when libvirtd is restarted.
Correct me if I’m wrong, but isn’t the phase already set to `QEMU_MIGRATION_PHASE_CONFIRM3` by `qemuMigrationConfirm`, which calls `qemuMigrationConfirmPhase`? Also, I’m not sure we need another phase. If libvirtd is restarted, it should continue with phase CONFIRM3, wait for post-copy migration to finish or observe that the VM has finished post-copy migration, and kill it. Am I missing something?
qemuMigrationJobSetPhase(driver, vm, retcode == 0 ? QEMU_MIGRATION_PHASE_CONFIRM3
Cristian
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- src/qemu/qemu_driver.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3fe2216..1242b77 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11950,6 +11950,55 @@ qemuDomainGetJobStats(virDomainPtr dom, } +static int qemuDomainMigrateStartPostCopy(virDomainPtr dom, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + qemuDomainObjPrivatePtr priv; + + virCheckFlags(0, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainMigrateStartPostCopyEnsureACL(dom->conn, vm->def) < 0) + return -1; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + priv = vm->privateData; + + if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("post-copy can only be started " + "while migration is in progress")); + goto cleanup; + } + + VIR_DEBUG("Starting post-copy"); + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorMigrateStartPostCopy(priv->mon); + qemuDomainObjExitMonitor(driver, vm); + + endjob: + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + static int qemuDomainAbortJob(virDomainPtr dom) { virQEMUDriverPtr driver = dom->conn->privateData; @@ -18419,6 +18468,7 @@ static virDriver qemuDriver = { .connectGetDomainCapabilities = qemuConnectGetDomainCapabilities, /* 1.2.7 */ .connectGetAllDomainStats = qemuConnectGetAllDomainStats, /* 1.2.8 */ .nodeAllocPages = qemuNodeAllocPages, /* 1.2.9 */ + .domainMigrateStartPostCopy = qemuDomainMigrateStartPostCopy, /* 1.2.10 */ }; -- 1.9.1
On Tue, Sep 30, 2014 at 16:39:28 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- src/qemu/qemu_driver.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3fe2216..1242b77 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11950,6 +11950,55 @@ qemuDomainGetJobStats(virDomainPtr dom, }
+static int qemuDomainMigrateStartPostCopy(virDomainPtr dom, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + qemuDomainObjPrivatePtr priv; + + virCheckFlags(0, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup;
return -1;
+ + if (virDomainMigrateStartPostCopyEnsureACL(dom->conn, vm->def) < 0) + return -1;
goto cleanup;
+ + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + priv = vm->privateData; + + if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("post-copy can only be started " + "while migration is in progress")); + goto cleanup;
goto endjob;
+ }
Do we need to explicitly check that post-copy was activated or is the error we get from qemuMonitorMigrateStartPostCopy in such case good enough?
+ + VIR_DEBUG("Starting post-copy"); + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorMigrateStartPostCopy(priv->mon); + qemuDomainObjExitMonitor(driver, vm); + + endjob: + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + static int qemuDomainAbortJob(virDomainPtr dom) { virQEMUDriverPtr driver = dom->conn->privateData; @@ -18419,6 +18468,7 @@ static virDriver qemuDriver = { .connectGetDomainCapabilities = qemuConnectGetDomainCapabilities, /* 1.2.7 */ .connectGetAllDomainStats = qemuConnectGetAllDomainStats, /* 1.2.8 */ .nodeAllocPages = qemuNodeAllocPages, /* 1.2.9 */ + .domainMigrateStartPostCopy = qemuDomainMigrateStartPostCopy, /* 1.2.10 */ };
Jirka
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- tools/virsh-domain.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 5 ++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 36a6d52..395c73c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9251,6 +9251,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_INT, .help = N_("force guest to suspend if live migration exceeds timeout (in seconds)") }, + {.name = "postcopy-after", + .type = VSH_OT_INT, + .help = N_("switch to post-copy migration if live migration exceeds timeout (in seconds)") + }, {.name = "xml", .type = VSH_OT_STRING, .help = N_("filename containing updated XML for the target") @@ -9332,6 +9336,8 @@ doMigrate(void *opaque) VIR_FREE(xml); } + if (vshCommandOptBool(cmd, "postcopy-after")) /* actually an int */ + flags |= VIR_MIGRATE_POSTCOPY; if (vshCommandOptBool(cmd, "live")) flags |= VIR_MIGRATE_LIVE; if (vshCommandOptBool(cmd, "p2p")) @@ -9423,6 +9429,50 @@ vshMigrationTimeout(vshControl *ctl, virDomainSuspend(dom); } +static void +vshMigrationPostCopyAfter(vshControl *ctl, + virDomainPtr dom, + void *opaque ATTRIBUTE_UNUSED) +{ + vshDebug(ctl, VSH_ERR_DEBUG, "starting post-copy\n"); + int rv = virDomainMigrateStartPostCopy(dom, 0); + if (rv < 0) { + vshError(ctl, "%s", _("start post-copy command failed")); + } else { + vshDebug(ctl, VSH_ERR_INFO, "switched to post-copy\n"); + } +} + +/* Parse the --postcopy-after parameter in seconds, but store its + * value in milliseconds. Return -1 on error, 0 if + * no timeout was requested, and 1 if timeout was set. + * Copy-paste-adapted from vshCommandOptTimeoutToMs. + */ +static int +vshCommandOptPostCopyAfterToMs(vshControl *ctl, const vshCmd *cmd, int *postCopyAfter) +{ + int rv = vshCommandOptInt(cmd, "postcopy-after", postCopyAfter); + + if (rv < 0 || (rv > 0 && *postCopyAfter < 0)) { + vshError(ctl, "%s", _("invalid postcopy-after parameter")); + return -1; + } + if (rv > 0) { + /* Ensure that we can multiply by 1000 without overflowing. */ + if (*postCopyAfter > INT_MAX / 1000) { + vshError(ctl, "%s", _("post-copy after parameter is too big")); + return -1; + } + *postCopyAfter *= 1000; + /* 0 is a special value inside virsh, which means no timeout, so + * use 1ms instead for "start post-copy immediately" + */ + if (*postCopyAfter == 0) + *postCopyAfter = 1; + } + return rv; +} + static bool cmdMigrate(vshControl *ctl, const vshCmd *cmd) { @@ -9432,6 +9482,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) bool verbose = false; bool functionReturn = false; int timeout = 0; + int postCopyAfter = 0; bool live_flag = false; vshCtrlData data = { .dconn = NULL }; @@ -9451,6 +9502,18 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) goto cleanup; } + if (vshCommandOptPostCopyAfterToMs(ctl, cmd, &postCopyAfter) < 0) { + goto cleanup; + } else if (postCopyAfter > 0 && !live_flag) { + vshError(ctl, "%s", + _("migrate: Unexpected postcopy-after for offline migration")); + goto cleanup; + } else if (postCopyAfter > 0 && timeout > 0) { + vshError(ctl, "%s", + _("migrate: --postcopy-after is incompatible with --timeout")); + goto cleanup; + } + if (pipe(p) < 0) goto cleanup; @@ -9480,8 +9543,13 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) doMigrate, &data) < 0) goto cleanup; - functionReturn = vshWatchJob(ctl, dom, verbose, p[0], timeout, - vshMigrationTimeout, NULL, _("Migration")); + if (postCopyAfter != 0) { + functionReturn = vshWatchJob(ctl, dom, verbose, p[0], postCopyAfter, + vshMigrationPostCopyAfter, NULL, _("Migration")); + } else { + functionReturn = vshWatchJob(ctl, dom, verbose, p[0], timeout, + vshMigrationTimeout, NULL, _("Migration")); + } virThreadJoin(&workerThread); diff --git a/tools/virsh.pod b/tools/virsh.pod index eae9195..2ae7afa 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1404,6 +1404,7 @@ to the I<uri> namespace is displayed instead of being modified. [I<--compressed>] [I<--abort-on-error>] [I<--auto-converge>] I<domain> I<desturi> [I<migrateuri>] [I<graphicsuri>] [I<listen-address>] [I<dname>] [I<--timeout> B<seconds>] [I<--xml> B<file>] +[I<--postcopy-after> B<seconds>] Migrate domain to another host. Add I<--live> for live migration; <--p2p> for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled> @@ -1451,6 +1452,10 @@ I<--timeout> B<seconds> forces guest to suspend when live migration exceeds that many seconds, and then the migration will complete offline. It can only be used with I<--live>. +I<--postcopy-after> switches to post-copy migration when pre-copy migration +exceeds that many seconds. Zero means start post-copy as soon as possible. +It can only be used with I<--live>. + Running migration can be canceled by interrupting virsh (usually using C<Ctrl-C>) or by B<domjobabort> command sent from another virsh instance. -- 1.9.1
On Tue, Sep 30, 2014 at 16:39:29 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- tools/virsh-domain.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 5 ++++ 2 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 36a6d52..395c73c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9251,6 +9251,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_INT, .help = N_("force guest to suspend if live migration exceeds timeout (in seconds)") }, + {.name = "postcopy-after", + .type = VSH_OT_INT, + .help = N_("switch to post-copy migration if live migration exceeds timeout (in seconds)") + }, {.name = "xml", .type = VSH_OT_STRING, .help = N_("filename containing updated XML for the target")
We also want to add --postcopy option to enable just VIR_MIGRATE_POSTCOPY without an automatic switch to post-copy after a timeout. The --postcopy-after option may turn it on automatically, though, so that users don't have to use --postcopy --postcopy-after. And we also want to add a new migrate-startpostcopy command as a direct wrapper to virDomainMigrateStartPostCopy.
@@ -9332,6 +9336,8 @@ doMigrate(void *opaque) VIR_FREE(xml); }
+ if (vshCommandOptBool(cmd, "postcopy-after")) /* actually an int */ + flags |= VIR_MIGRATE_POSTCOPY; if (vshCommandOptBool(cmd, "live")) flags |= VIR_MIGRATE_LIVE; if (vshCommandOptBool(cmd, "p2p")) @@ -9423,6 +9429,50 @@ vshMigrationTimeout(vshControl *ctl, virDomainSuspend(dom); }
+static void +vshMigrationPostCopyAfter(vshControl *ctl, + virDomainPtr dom, + void *opaque ATTRIBUTE_UNUSED) +{ + vshDebug(ctl, VSH_ERR_DEBUG, "starting post-copy\n"); + int rv = virDomainMigrateStartPostCopy(dom, 0); + if (rv < 0) { + vshError(ctl, "%s", _("start post-copy command failed")); + } else { + vshDebug(ctl, VSH_ERR_INFO, "switched to post-copy\n"); + } +} + +/* Parse the --postcopy-after parameter in seconds, but store its + * value in milliseconds. Return -1 on error, 0 if + * no timeout was requested, and 1 if timeout was set. + * Copy-paste-adapted from vshCommandOptTimeoutToMs. + */ +static int +vshCommandOptPostCopyAfterToMs(vshControl *ctl, const vshCmd *cmd, int *postCopyAfter) +{ + int rv = vshCommandOptInt(cmd, "postcopy-after", postCopyAfter); + + if (rv < 0 || (rv > 0 && *postCopyAfter < 0)) { + vshError(ctl, "%s", _("invalid postcopy-after parameter")); + return -1; + } + if (rv > 0) { + /* Ensure that we can multiply by 1000 without overflowing. */ + if (*postCopyAfter > INT_MAX / 1000) { + vshError(ctl, "%s", _("post-copy after parameter is too big")); + return -1; + } + *postCopyAfter *= 1000; + /* 0 is a special value inside virsh, which means no timeout, so + * use 1ms instead for "start post-copy immediately" + */ + if (*postCopyAfter == 0) + *postCopyAfter = 1; + } + return rv; +} + static bool cmdMigrate(vshControl *ctl, const vshCmd *cmd) { @@ -9432,6 +9482,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) bool verbose = false; bool functionReturn = false; int timeout = 0; + int postCopyAfter = 0; bool live_flag = false; vshCtrlData data = { .dconn = NULL };
@@ -9451,6 +9502,18 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) goto cleanup; }
+ if (vshCommandOptPostCopyAfterToMs(ctl, cmd, &postCopyAfter) < 0) { + goto cleanup; + } else if (postCopyAfter > 0 && !live_flag) { + vshError(ctl, "%s", + _("migrate: Unexpected postcopy-after for offline migration"));
Offline migration is a migration of a domain which is not running at all. We don't have a good name for migration of a paused domain, some refer to it as coma migration. Anyway, this is checked for in the daemon so checking here is redundant. Especially when this check seems to be somewhat artificial. Sure, it doesn't make a lot of sense but post-copy migration of a paused domain should be doable. Moreover, I believe you can call virDomainSuspend during a post-copy migration, which is very close to running migration without --live.
+ goto cleanup; + } else if (postCopyAfter > 0 && timeout > 0) { + vshError(ctl, "%s", + _("migrate: --postcopy-after is incompatible with --timeout")); + goto cleanup; + }
Anyway, handling --postcopy-after seems quite complicated since we have to repeat the logic around -1, 0, 1 return values in two places. Can't we just directly fetch and process the value in cmdMigrate?
+ if (pipe(p) < 0) goto cleanup;
... Jirka
On 07 Oct 2014, at 23:48 , Jiri Denemark <jdenemar@redhat.com> wrote:
On Tue, Sep 30, 2014 at 16:39:29 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- tools/virsh-domain.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 5 ++++ 2 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 36a6d52..395c73c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9251,6 +9251,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_INT, .help = N_("force guest to suspend if live migration exceeds timeout (in seconds)") }, + {.name = "postcopy-after", + .type = VSH_OT_INT, + .help = N_("switch to post-copy migration if live migration exceeds timeout (in seconds)") + }, {.name = "xml", .type = VSH_OT_STRING, .help = N_("filename containing updated XML for the target")
We also want to add --postcopy option to enable just VIR_MIGRATE_POSTCOPY without an automatic switch to post-copy after a timeout. The --postcopy-after option may turn it on automatically, though, so that users don't have to use --postcopy --postcopy-after. And we also want to add a new migrate-startpostcopy command as a direct wrapper to virDomainMigrateStartPostCopy.
I’m still not convinced this is a good idea. To use —postcopy and migrate-startpostcopy, the user would have to launch a second instance of virsh, since the first one is busy monitoring the migration. I feel that this is a bit too low level for a tool like virsh: It would be as if we offered the user a dedicated migrate-cancel command, instead of pressing CTRL+C.
@@ -9332,6 +9336,8 @@ doMigrate(void *opaque) VIR_FREE(xml); }
+ if (vshCommandOptBool(cmd, "postcopy-after")) /* actually an int */ + flags |= VIR_MIGRATE_POSTCOPY; if (vshCommandOptBool(cmd, "live")) flags |= VIR_MIGRATE_LIVE; if (vshCommandOptBool(cmd, "p2p")) @@ -9423,6 +9429,50 @@ vshMigrationTimeout(vshControl *ctl, virDomainSuspend(dom); }
+static void +vshMigrationPostCopyAfter(vshControl *ctl, + virDomainPtr dom, + void *opaque ATTRIBUTE_UNUSED) +{ + vshDebug(ctl, VSH_ERR_DEBUG, "starting post-copy\n"); + int rv = virDomainMigrateStartPostCopy(dom, 0); + if (rv < 0) { + vshError(ctl, "%s", _("start post-copy command failed")); + } else { + vshDebug(ctl, VSH_ERR_INFO, "switched to post-copy\n"); + } +} + +/* Parse the --postcopy-after parameter in seconds, but store its + * value in milliseconds. Return -1 on error, 0 if + * no timeout was requested, and 1 if timeout was set. + * Copy-paste-adapted from vshCommandOptTimeoutToMs. + */ +static int +vshCommandOptPostCopyAfterToMs(vshControl *ctl, const vshCmd *cmd, int *postCopyAfter) +{ + int rv = vshCommandOptInt(cmd, "postcopy-after", postCopyAfter); + + if (rv < 0 || (rv > 0 && *postCopyAfter < 0)) { + vshError(ctl, "%s", _("invalid postcopy-after parameter")); + return -1; + } + if (rv > 0) { + /* Ensure that we can multiply by 1000 without overflowing. */ + if (*postCopyAfter > INT_MAX / 1000) { + vshError(ctl, "%s", _("post-copy after parameter is too big")); + return -1; + } + *postCopyAfter *= 1000; + /* 0 is a special value inside virsh, which means no timeout, so + * use 1ms instead for "start post-copy immediately" + */ + if (*postCopyAfter == 0) + *postCopyAfter = 1; + } + return rv; +} + static bool cmdMigrate(vshControl *ctl, const vshCmd *cmd) { @@ -9432,6 +9482,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) bool verbose = false; bool functionReturn = false; int timeout = 0; + int postCopyAfter = 0; bool live_flag = false; vshCtrlData data = { .dconn = NULL };
@@ -9451,6 +9502,18 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) goto cleanup; }
+ if (vshCommandOptPostCopyAfterToMs(ctl, cmd, &postCopyAfter) < 0) { + goto cleanup; + } else if (postCopyAfter > 0 && !live_flag) { + vshError(ctl, "%s", + _("migrate: Unexpected postcopy-after for offline migration"));
Offline migration is a migration of a domain which is not running at all. We don't have a good name for migration of a paused domain, some refer to it as coma migration. Anyway, this is checked for in the daemon so checking here is redundant. Especially when this check seems to be somewhat artificial. Sure, it doesn't make a lot of sense but post-copy migration of a paused domain should be doable. Moreover, I believe you can call virDomainSuspend during a post-copy migration, which is very close to running migration without —live.
+ goto cleanup; + } else if (postCopyAfter > 0 && timeout > 0) { + vshError(ctl, "%s", + _("migrate: --postcopy-after is incompatible with --timeout")); + goto cleanup; + }
Anyway, handling --postcopy-after seems quite complicated since we have to repeat the logic around -1, 0, 1 return values in two places. Can't we just directly fetch and process the value in cmdMigrate?
+ if (pipe(p) < 0) goto cleanup;
I’ll fix the other issues you have raised. Cristian
On Thu, Oct 09, 2014 at 16:22:54 +0900, Cristian Klein wrote:
On 07 Oct 2014, at 23:48 , Jiri Denemark <jdenemar@redhat.com> wrote:
On Tue, Sep 30, 2014 at 16:39:29 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- tools/virsh-domain.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 5 ++++ 2 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 36a6d52..395c73c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9251,6 +9251,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_INT, .help = N_("force guest to suspend if live migration exceeds timeout (in seconds)") }, + {.name = "postcopy-after", + .type = VSH_OT_INT, + .help = N_("switch to post-copy migration if live migration exceeds timeout (in seconds)") + }, {.name = "xml", .type = VSH_OT_STRING, .help = N_("filename containing updated XML for the target")
We also want to add --postcopy option to enable just VIR_MIGRATE_POSTCOPY without an automatic switch to post-copy after a timeout. The --postcopy-after option may turn it on automatically, though, so that users don't have to use --postcopy --postcopy-after. And we also want to add a new migrate-startpostcopy command as a direct wrapper to virDomainMigrateStartPostCopy.
I’m still not convinced this is a good idea. To use —postcopy and migrate-startpostcopy, the user would have to launch a second instance of virsh, since the first one is busy monitoring the migration. I feel that this is a bit too low level for a tool like virsh: It would be as if we offered the user a dedicated migrate-cancel command, instead of pressing CTRL+C.
I'm not saying we should remove --postcopy-after and only provide --postcopy and migrate-startpostcopy. We should provide both ways of switching to post-copy: the easy one (--postcopy-after) and the powerful one (--postcopy + migrate-startpostcopy). We tend to make every single piece of libvirt API available through virsh so that it has the full power. The ability to call any libvirt API manually with virsh is often useful for developers. You may, for example, try to call migrate-startpostcopy on a domain that's not even running, is not being migrated, or is being migrated without the POSTCOPY flag. In addition to the low level wrappers for libvirt APIs, we sometimes provide easy-to-use wrappers for some functionality, e.g., creating snapshots, aborting long running jobs... Your example with migrate-cancel is actually a perfect one since it is exactly what I'm asking for. We do have the ability to cancel migration via a dedicated virsh command and the command is called domjobabort (it's not limit to migration, it also covers managed-save, dump, and other similar jobs). In addition to this command users can just press Ctrl-C to achieve the same without having to run one more virsh. Jirka
On 10 Oct 2014, at 5:59 , Jiri Denemark <jdenemar@redhat.com> wrote:
On Thu, Oct 09, 2014 at 16:22:54 +0900, Cristian Klein wrote:
On 07 Oct 2014, at 23:48 , Jiri Denemark <jdenemar@redhat.com> wrote:
On Tue, Sep 30, 2014 at 16:39:29 +0200, Cristian Klein wrote:
Signed-off-by: Cristian Klein <cristian.klein@cs.umu.se> --- tools/virsh-domain.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 5 ++++ 2 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 36a6d52..395c73c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9251,6 +9251,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_INT, .help = N_("force guest to suspend if live migration exceeds timeout (in seconds)") }, + {.name = "postcopy-after", + .type = VSH_OT_INT, + .help = N_("switch to post-copy migration if live migration exceeds timeout (in seconds)") + }, {.name = "xml", .type = VSH_OT_STRING, .help = N_("filename containing updated XML for the target")
We also want to add --postcopy option to enable just VIR_MIGRATE_POSTCOPY without an automatic switch to post-copy after a timeout. The --postcopy-after option may turn it on automatically, though, so that users don't have to use --postcopy --postcopy-after. And we also want to add a new migrate-startpostcopy command as a direct wrapper to virDomainMigrateStartPostCopy.
I’m still not convinced this is a good idea. To use —postcopy and migrate-startpostcopy, the user would have to launch a second instance of virsh, since the first one is busy monitoring the migration. I feel that this is a bit too low level for a tool like virsh: It would be as if we offered the user a dedicated migrate-cancel command, instead of pressing CTRL+C.
I'm not saying we should remove --postcopy-after and only provide --postcopy and migrate-startpostcopy. We should provide both ways of switching to post-copy: the easy one (--postcopy-after) and the powerful one (--postcopy + migrate-startpostcopy). We tend to make every single piece of libvirt API available through virsh so that it has the full power. The ability to call any libvirt API manually with virsh is often useful for developers. You may, for example, try to call migrate-startpostcopy on a domain that's not even running, is not being migrated, or is being migrated without the POSTCOPY flag.
In addition to the low level wrappers for libvirt APIs, we sometimes provide easy-to-use wrappers for some functionality, e.g., creating snapshots, aborting long running jobs...
Your example with migrate-cancel is actually a perfect one since it is exactly what I'm asking for. We do have the ability to cancel migration via a dedicated virsh command and the command is called domjobabort (it's not limit to migration, it also covers managed-save, dump, and other similar jobs). In addition to this command users can just press Ctrl-C to achieve the same without having to run one more virsh.
I didn’t know about domjobabort. That’s a pretty compelling argument to add migrate-startpostcopy. Okey, I’ll add it in the next version of the patch series.
participants (6)
-
Andrea Arcangeli -
Cristian KLEIN -
Cristian Klein
-
Daniel P. Berrange -
Eric Blake -
Jiri Denemark