On 04/21/2011 10:32 AM, Daniel P. Berrange wrote:
Migration just seems togo from bad to worse. We already had to
s/togo/to go/
This patch attempts to introduce a version 3 that uses the improved 5 step sequence
* Src: Begin - Generate XML to pass to dst - Generate optional cookie to pass to dst
* Dst: Prepare - Get ready to accept incoming VM - Generate optional cookie to pass to src
* Src: Perform - Start migration and wait for send completion - Generate optional cookie to pass to dst
* Dst: Finish - Wait for recv completion and check status - Kill off VM if failed, resume if success - Generate optional cookie to pass to src
* Src: Confirm - Kill off VM if success, resume if failed
I think we've covered that this is a reasonable idiom in other threads. At least, I can't think of anything else to add.
The API is designed to allow both input and output cookies in all methods where applicable. This lets us pass around arbitrary extra driver specific data between src & dst during migration. Combined with the extra 'Begin' method this lets us pass lease information from source to dst at the start of migration
Moving the killing of the source VM out of Perform and into Confirm, means we can now recover if the dst host can't successfully Finish receiving migration data.
I think this also opens the gate to allowing the destination side gracefully abort the receive job, although I'm not sure if your patch series got to that point, yet.
+ * Src: Begin + * - Generate XML to pass to dst + * - Generate optional cookie to pass to dst + * + * Dst: Prepare + * - Get ready to accept incoming VM + * - Generate optional cookie to pass to src + * + * Src: Perform + * - Start migration and wait for send completion + * - Generate optional cookie to pass to dst + * + * Dst: Finish + * - Wait for recv completion and check status + * - Kill off VM if failed, resume if success + * - Generate optional cookie to pass to src + * + * Src: Confirm + * - Kill off VM if success, resume if failed + * + */
+/* + * Not for public use. This function is part of the internal + * implementation of migration in the remote case. + */ +char * +virDomainMigrateBegin3(virDomainPtr domain, + char **cookieout, + int *cookieoutlen, + unsigned long flags, + const char *dname, + unsigned long bandwidth) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "cookieout=%p, cookieoutlen=%p, " + "flags=%lu, dname=%s, bandwidth=%lu", + cookieout, cookieoutlen, flags, + NULLSTR(dname), bandwidth); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN (domain)) {
I noticed spacing is funny here [s/DOMAIN (/DOMAIN(/]. Didn't look closely for it elsewhere.
+++ b/src/libvirt_private.syms @@ -547,6 +547,12 @@ virDomainMigratePerform; virDomainMigratePrepare2; virDomainMigratePrepare; virDomainMigratePrepareTunnel; +virDomainMigrateBegin3; +virDomainMigratePrepare3; +virDomainMigratePrepareTunnel3; +virDomainMigratePerform3; +virDomainMigrateFinish3; +virDomainMigrateConfirm3;
For consistency, sort these lines alphabetically amongst the other functions. You missed stubs for src/libxl/libxl_driver.c. ACK with those nits fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org