From dd862584c24fde160c46198fe7b2fbddf12ecd495d5ba0d9ede2e2a1d090ef14 Mon Sep 17 00:00:00 2001 From: James Fehlig Date: Fri, 31 Jul 2015 18:53:33 +0000 Subject: [PATCH] Add fix for bsc#936185 to Factory OBS-URL: https://build.opensuse.org/package/show/Virtualization/libvirt?expand=0&rev=473 --- ...ef-counting-of-libxlMigrationDstArgs.patch | 108 ++++++++++++++++++ ...mpt-to-resume-domain-when-suspend-fa.patch | 51 +++++++++ ...job-when-receiving-a-migrating-domai.patch | 61 ++++++++++ libvirt.changes | 9 ++ libvirt.spec | 6 + 5 files changed, 235 insertions(+) create mode 100644 0003-libxl-fix-ref-counting-of-libxlMigrationDstArgs.patch create mode 100644 0004-libxl-don-t-attempt-to-resume-domain-when-suspend-fa.patch create mode 100644 0005-libxl-acquire-a-job-when-receiving-a-migrating-domai.patch diff --git a/0003-libxl-fix-ref-counting-of-libxlMigrationDstArgs.patch b/0003-libxl-fix-ref-counting-of-libxlMigrationDstArgs.patch new file mode 100644 index 0000000..9a9429a --- /dev/null +++ b/0003-libxl-fix-ref-counting-of-libxlMigrationDstArgs.patch @@ -0,0 +1,108 @@ +From 68ccc0077c7f5af07ccf6992192bfc924d0fdd83 Mon Sep 17 00:00:00 2001 +From: Jim Fehlig +Date: Thu, 16 Jul 2015 14:51:30 -0600 +Subject: libxl: fix ref counting of libxlMigrationDstArgs + +This patch fixes some flawed logic around ref counting the +libxlMigrationDstArgs object. + +First, when adding sockets to the event loop with +virNetSocketAddIOCallback(), the generic virObjectFreeCallback() +was registered as a free function, with libxlMigrationDstArgs as +its parameter. A reference was also taken on +libxlMigrationDstArgs for each successful call to +virNetSocketAddIOCallback(). The rational behind this logic was +that the libxlMigrationDstArgs object had to out-live the socket +objects. But virNetSocketAddIOCallback() already takes a +reference on socket objects, ensuring their life until removed +from the event loop and unref'ed in virNetSocketEventFree(). We +only need to ensure libxlMigrationDstArgs lives until +libxlDoMigrateReceive() finishes, which can be done by simply +unref'ing libxlMigrationDstArgs at the end of +libxlDoMigrateReceive(). + +The second flaw was unref'ing the sockets in the failure path of +libxlMigrateReceive() and at the end of libxlDoMigrateReceive(). +As mentioned above, the sockets are already unref'ed by +virNetSocketEventFree() when removed from the event loop. +Attempting to unref the socket a second time resulted in a +libvirtd crash since the socket was previously unref'ed and +disposed. + +Signed-off-by: Jim Fehlig +--- + src/libxl/libxl_migration.c | 18 +++++------------- + 1 file changed, 5 insertions(+), 13 deletions(-) + +Index: libvirt-1.2.17/src/libxl/libxl_migration.c +=================================================================== +--- libvirt-1.2.17.orig/src/libxl/libxl_migration.c ++++ libvirt-1.2.17/src/libxl/libxl_migration.c +@@ -112,11 +112,11 @@ libxlDoMigrateReceive(void *opaque) + virNetSocketUpdateIOCallback(socks[i], 0); + virNetSocketRemoveIOCallback(socks[i]); + virNetSocketClose(socks[i]); +- virObjectUnref(socks[i]); + socks[i] = NULL; + } + args->nsocks = 0; + VIR_FORCE_CLOSE(recvfd); ++ virObjectUnref(args); + } + + +@@ -164,11 +164,11 @@ libxlMigrateReceive(virNetSocketPtr sock + virNetSocketUpdateIOCallback(socks[i], 0); + virNetSocketRemoveIOCallback(socks[i]); + virNetSocketClose(socks[i]); +- virObjectUnref(socks[i]); + socks[i] = NULL; + } + args->nsocks = 0; + VIR_FORCE_CLOSE(recvfd); ++ virObjectUnref(args); + } + + static int +@@ -318,7 +318,7 @@ libxlDomainMigrationPrepare(virConnectPt + virNetSocketPtr *socks = NULL; + size_t nsocks = 0; + int nsocks_listen = 0; +- libxlMigrationDstArgs *args; ++ libxlMigrationDstArgs *args = NULL; + size_t i; + int ret = -1; + +@@ -420,22 +420,12 @@ libxlDomainMigrationPrepare(virConnectPt + VIR_EVENT_HANDLE_READABLE, + libxlMigrateReceive, + args, +- virObjectFreeCallback) < 0) ++ NULL) < 0) + continue; + +- /* +- * Successfully added sock to event loop. Take a ref on args to +- * ensure it is not freed until sock is removed from the event loop. +- * Ref is dropped in virObjectFreeCallback after being removed +- * from the event loop. +- */ +- virObjectRef(args); + nsocks_listen++; + } + +- /* Done with args in this function, drop reference */ +- virObjectUnref(args); +- + if (!nsocks_listen) + goto error; + +@@ -448,6 +438,8 @@ libxlDomainMigrationPrepare(virConnectPt + virObjectUnref(socks[i]); + } + VIR_FREE(socks); ++ virObjectUnref(args); ++ + /* Remove virDomainObj from domain list */ + if (vm) { + virDomainObjListRemove(driver->domains, vm); diff --git a/0004-libxl-don-t-attempt-to-resume-domain-when-suspend-fa.patch b/0004-libxl-don-t-attempt-to-resume-domain-when-suspend-fa.patch new file mode 100644 index 0000000..917c175 --- /dev/null +++ b/0004-libxl-don-t-attempt-to-resume-domain-when-suspend-fa.patch @@ -0,0 +1,51 @@ +From 13d53b7763d3d93339cc33a19845bdd623524b81 Mon Sep 17 00:00:00 2001 +From: Jim Fehlig +Date: Thu, 16 Jul 2015 14:51:31 -0600 +Subject: libxl: don't attempt to resume domain when suspend fails + +Failure of libxl_domain_suspend() does not leave the domain in +a suspended state, so no need to call libxl_domain_resume(), +which btw will fail with "domain not suspended". + +Signed-off-by: Jim Fehlig +--- + src/libxl/libxl_migration.c | 14 -------------- + 1 file changed, 14 deletions(-) + +Index: libvirt-1.2.17/src/libxl/libxl_migration.c +=================================================================== +--- libvirt-1.2.17.orig/src/libxl/libxl_migration.c ++++ libvirt-1.2.17/src/libxl/libxl_migration.c +@@ -178,7 +178,6 @@ libxlDoMigrateSend(libxlDriverPrivatePtr + int sockfd) + { + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); +- virObjectEventPtr event = NULL; + int xl_flags = 0; + int ret; + +@@ -188,24 +187,11 @@ libxlDoMigrateSend(libxlDriverPrivatePtr + ret = libxl_domain_suspend(cfg->ctx, vm->def->id, sockfd, + xl_flags, NULL); + if (ret != 0) { +- /* attempt to resume the domain on failure */ +- if (libxl_domain_resume(cfg->ctx, vm->def->id, 1, 0) != 0) { +- VIR_DEBUG("Failed to resume domain following failed migration"); +- virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, +- VIR_DOMAIN_PAUSED_MIGRATION); +- event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_SUSPENDED, +- VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED); +- ignore_value(virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm)); +- } + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to send migration data to destination host")); + ret = -1; +- goto cleanup; + } + +- cleanup: +- if (event) +- libxlDomainEventQueue(driver, event); + virObjectUnref(cfg); + return ret; + } diff --git a/0005-libxl-acquire-a-job-when-receiving-a-migrating-domai.patch b/0005-libxl-acquire-a-job-when-receiving-a-migrating-domai.patch new file mode 100644 index 0000000..0edd952 --- /dev/null +++ b/0005-libxl-acquire-a-job-when-receiving-a-migrating-domai.patch @@ -0,0 +1,61 @@ +From 710088061fb3caaf3d56888b05ad2d331a740d07 Mon Sep 17 00:00:00 2001 +From: Jim Fehlig +Date: Thu, 16 Jul 2015 14:51:32 -0600 +Subject: libxl: acquire a job when receiving a migrating domain + +Commit f86ae403 moved acquiring a job from libxlDomainStart() +to its callers. One spot missed was in libxlDoMigrateReceive(). +Acquire a job in libxlDoMigrateReceive() before calling +libxlDomainStart(). + +Signed-off-by: Jim Fehlig +--- + src/libxl/libxl_migration.c | 20 +++++++++++++++++--- + 1 file changed, 17 insertions(+), 3 deletions(-) + +Index: libvirt-1.2.17/src/libxl/libxl_migration.c +=================================================================== +--- libvirt-1.2.17.orig/src/libxl/libxl_migration.c ++++ libvirt-1.2.17/src/libxl/libxl_migration.c +@@ -95,17 +95,20 @@ libxlDoMigrateReceive(void *opaque) + int recvfd = args->recvfd; + size_t i; + int ret; ++ bool remove_dom = 0; ++ ++ virObjectLock(vm); ++ if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) ++ goto cleanup; + + /* + * Always start the domain paused. If needed, unpause in the + * finish phase, after transfer of the domain is complete. + */ +- virObjectLock(vm); + ret = libxlDomainStart(driver, vm, true, recvfd); +- virObjectUnlock(vm); + + if (ret < 0 && !vm->persistent) +- virDomainObjListRemove(driver->domains, vm); ++ remove_dom = true; + + /* Remove all listen socks from event handler, and close them. */ + for (i = 0; i < nsocks; i++) { +@@ -117,6 +120,17 @@ libxlDoMigrateReceive(void *opaque) + args->nsocks = 0; + VIR_FORCE_CLOSE(recvfd); + virObjectUnref(args); ++ ++ if (!libxlDomainObjEndJob(driver, vm)) ++ vm = NULL; ++ ++ cleanup: ++ if (remove_dom && vm) { ++ virDomainObjListRemove(driver->domains, vm); ++ vm = NULL; ++ } ++ if (vm) ++ virObjectUnlock(vm); + } + + diff --git a/libvirt.changes b/libvirt.changes index 90fff80..e196cdd 100644 --- a/libvirt.changes +++ b/libvirt.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Fri Jul 31 18:51:38 UTC 2015 - jfehlig@suse.com + +- Fix crash in libxl driver on receiving side + 0003-libxl-fix-ref-counting-of-libxlMigrationDstArgs.patch + 0004-libxl-don-t-attempt-to-resume-domain-when-suspend-fa.patch + 0005-libxl-acquire-a-job-when-receiving-a-migrating-domai.patch + bsc#936185 + ------------------------------------------------------------------- Fri Jul 10 18:35:27 UTC 2015 - jfehlig@suse.com diff --git a/libvirt.spec b/libvirt.spec index d42404d..e53f6c1 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -454,6 +454,9 @@ Patch4: 45697fe5-libxl-support-dom0.patch Patch5: e9c27344-libxl-fix-virDomainObj-state.patch Patch6: 4ffb21c8-libxl-dom0-state-fix.patch # Patches pending upstream review +Patch100: 0003-libxl-fix-ref-counting-of-libxlMigrationDstArgs.patch +Patch101: 0004-libxl-don-t-attempt-to-resume-domain-when-suspend-fa.patch +Patch102: 0005-libxl-acquire-a-job-when-receiving-a-migrating-domai.patch # Need to go upstream Patch150: xen-pv-cdrom.patch Patch151: blockcopy-check-dst-identical-device.patch @@ -991,6 +994,9 @@ Provides a dissector for the libvirt RPC protocol to help debugging it. %patch4 -p1 %patch5 -p1 %patch6 -p1 +%patch100 -p1 +%patch101 -p1 +%patch102 -p1 %patch150 -p1 %patch151 -p1 %patch152 -p1