If any disk of a VM was involved in a (copy) block job we refused to do a snapshot. As not only copy jobs interlock snapshots and the interlocking is applicable to individual disks only we can make the check in a more individual fashion and interlock all block job types supported by libvirt. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1203628 --- src/qemu/qemu_driver.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6700fc9..6bc305e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13928,6 +13928,14 @@ qemuDomainSnapshotPrepare(virConnectPtr conn, virDomainSnapshotDiskDefPtr disk = &def->disks[i]; virDomainDiskDefPtr dom_disk = vm->def->disks[i]; + if (disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE && + dom_disk->blockjob) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk '%s' has an active block job"), + disk->name); + goto cleanup; + } + switch ((virDomainSnapshotLocation) disk->snapshot) { case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL: found_internal = true; @@ -14574,11 +14582,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, "%s", _("domain is marked for auto destroy")); goto cleanup; } - if (virDomainHasDiskMirror(vm)) { - virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s", - _("domain has active block job")); - goto cleanup; - } if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", -- 2.2.2

From: Shanzhi Yu <shyu@redhat.com> virDomainHasDiskMirror() currently detects only jobs that add the mirror elements. Since some operations like migration are interlocked by existing block jobs on the given domain the check needs to be instrumented to check regular jobs too. This patch renames virDomainHasDiskMirror to virDomainHasDiskBlockjob and adds an argument that allows to select that it returns true only for block copy jobs as those interlock making the domain persistent. Other two uses trigger on any block job type. Signed-off-by: Shanzhi Yu <shyu@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 25 ++++++++++++++++++++----- src/conf/domain_conf.h | 3 ++- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 6 +++--- src/qemu/qemu_migration.c | 2 +- 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9324de0..16cdfa2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12263,15 +12263,30 @@ virDomainDiskRemoveByName(virDomainDefPtr def, const char *name) return virDomainDiskRemove(def, idx); } -/* Return true if VM has at least one disk involved in a current block - * copy/commit job (that is, with a <mirror> element in the disk xml). */ +/** + * virDomainHasBlockjob: + * @vm: domain object + * @copy_only: Reject only block copy job + * + * Return true if @vm has at least one disk involved in a current block + * copy/commit/pull job. If @copy_only is true this returns true only if the + * disk is involved in a block copy. + * */ bool -virDomainHasDiskMirror(virDomainObjPtr vm) +virDomainHasBlockjob(virDomainObjPtr vm, + bool copy_only) { size_t i; - for (i = 0; i < vm->def->ndisks; i++) - if (vm->def->disks[i]->mirror) + for (i = 0; i < vm->def->ndisks; i++) { + if (!copy_only && + vm->def->disks[i]->blockjob) + return true; + + if (vm->def->disks[i]->mirror && + vm->def->disks[i]->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) return true; + } + return false; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 608f61f..493fbba 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2654,7 +2654,8 @@ int virDomainDiskSourceParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virStorageSourcePtr src); -bool virDomainHasDiskMirror(virDomainObjPtr vm); +bool virDomainHasBlockjob(virDomainObjPtr vm, + bool copy_only); int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net); virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5716ece..9e71b1a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -297,7 +297,7 @@ virDomainGraphicsTypeFromString; virDomainGraphicsTypeToString; virDomainGraphicsVNCSharePolicyTypeFromString; virDomainGraphicsVNCSharePolicyTypeToString; -virDomainHasDiskMirror; +virDomainHasBlockjob; virDomainHasNet; virDomainHostdevCapsTypeToString; virDomainHostdevDefAlloc; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6bc305e..eac04cf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7389,7 +7389,7 @@ static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml virObjectRef(vm); def = NULL; - if (virDomainHasDiskMirror(vm)) { + if (virDomainHasBlockjob(vm, true)) { virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s", _("domain has active block job")); virDomainObjAssignDef(vm, NULL, false, NULL); @@ -15239,8 +15239,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (virDomainHasDiskMirror(vm)) { - virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s", + if (virDomainHasBlockjob(vm, false)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain has active block job")); goto cleanup; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d34bb02..8c45415 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1977,7 +1977,7 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, } - if (virDomainHasDiskMirror(vm)) { + if (virDomainHasBlockjob(vm, false)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain has an active block job")); return false; -- 2.2.2