From 925965e6265cc808b8517d69849eb2b45ee8cba9 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Mon, 21 Sep 2015 21:24:48 -0400 Subject: [PATCH] Add sanity checks for drive mirroring (bz #1263438) --- ...ort-qemuProcessFindDomainDiskByAlias.patch | 35 ++ ...-fiddle-with-disk-backing-trees-with.patch | 351 ++++++++++++++++++ ...ncurrent-block-jobs-on-a-single-disk.patch | 175 +++++++++ ...t-Mark-disk-in-block-jobs-only-on-su.patch | 28 ++ libvirt.spec | 10 +- 5 files changed, 598 insertions(+), 1 deletion(-) create mode 100644 0102-qemu-process-Export-qemuProcessFindDomainDiskByAlias.patch create mode 100644 0103-qemu-event-Don-t-fiddle-with-disk-backing-trees-with.patch create mode 100644 0104-qemu-Disallow-concurrent-block-jobs-on-a-single-disk.patch create mode 100644 0105-qemu-block-commit-Mark-disk-in-block-jobs-only-on-su.patch diff --git a/0102-qemu-process-Export-qemuProcessFindDomainDiskByAlias.patch b/0102-qemu-process-Export-qemuProcessFindDomainDiskByAlias.patch new file mode 100644 index 0000000..6b8185e --- /dev/null +++ b/0102-qemu-process-Export-qemuProcessFindDomainDiskByAlias.patch @@ -0,0 +1,35 @@ +From: Peter Krempa +Date: Fri, 13 Mar 2015 16:59:26 +0100 +Subject: [PATCH] qemu: process: Export qemuProcessFindDomainDiskByAlias + +(cherry picked from commit 5c634730b99b53afd6e2cea4b7d2fc2dfc2ee630) +--- + src/qemu/qemu_process.c | 2 +- + src/qemu/qemu_process.h | 3 +++ + 2 files changed, 4 insertions(+), 1 deletion(-) + +diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c +index 46f02b5..1994027 100644 +--- a/src/qemu/qemu_process.c ++++ b/src/qemu/qemu_process.c +@@ -390,7 +390,7 @@ qemuProcessFindDomainDiskByPath(virDomainObjPtr vm, + return NULL; + } + +-static virDomainDiskDefPtr ++virDomainDiskDefPtr + qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm, + const char *alias) + { +diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h +index 2e1d393..fffc926 100644 +--- a/src/qemu/qemu_process.h ++++ b/src/qemu/qemu_process.h +@@ -107,4 +107,7 @@ int qemuProcessReadLog(int fd, char *buf, int buflen, int off, bool skipchar); + int qemuProcessSetSchedParams(int id, pid_t pid, size_t nsp, + virDomainThreadSchedParamPtr sp); + ++virDomainDiskDefPtr qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm, ++ const char *alias); ++ + #endif /* __QEMU_PROCESS_H__ */ diff --git a/0103-qemu-event-Don-t-fiddle-with-disk-backing-trees-with.patch b/0103-qemu-event-Don-t-fiddle-with-disk-backing-trees-with.patch new file mode 100644 index 0000000..6e91c1e --- /dev/null +++ b/0103-qemu-event-Don-t-fiddle-with-disk-backing-trees-with.patch @@ -0,0 +1,351 @@ +From: Peter Krempa +Date: Fri, 13 Mar 2015 17:00:03 +0100 +Subject: [PATCH] qemu: event: Don't fiddle with disk backing trees without a + job + +Surprisingly we did not grab a VM job when a block job finished and we'd +happily rewrite the backing chain data. This made it possible to crash +libvirt when queueing two backing chains tightly and other badness. + +To fix it, add yet another handler to the helper thread that handles +monitor events that require a job. + +(cherry picked from commit 1a92c719101e5bfa6fe2b78006ad04c7f075ea28) +--- + src/qemu/qemu_domain.h | 2 + + src/qemu/qemu_driver.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++ + src/qemu/qemu_process.c | 129 ++++++++----------------------------------- + 3 files changed, 168 insertions(+), 105 deletions(-) + +diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h +index fe3e2b1..a7ebb47 100644 +--- a/src/qemu/qemu_domain.h ++++ b/src/qemu/qemu_domain.h +@@ -196,6 +196,7 @@ typedef enum { + QEMU_PROCESS_EVENT_DEVICE_DELETED, + QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED, + QEMU_PROCESS_EVENT_SERIAL_CHANGED, ++ QEMU_PROCESS_EVENT_BLOCK_JOB, + + QEMU_PROCESS_EVENT_LAST + } qemuProcessEventType; +@@ -204,6 +205,7 @@ struct qemuProcessEvent { + virDomainObjPtr vm; + qemuProcessEventType eventType; + int action; ++ int status; + void *data; + }; + +diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c +index 1009497..2e8c9d8 100644 +--- a/src/qemu/qemu_driver.c ++++ b/src/qemu/qemu_driver.c +@@ -4480,6 +4480,141 @@ processSerialChangedEvent(virQEMUDriverPtr driver, + } + + ++static void ++processBlockJobEvent(virQEMUDriverPtr driver, ++ virDomainObjPtr vm, ++ char *diskAlias, ++ int type, ++ int status) ++{ ++ virObjectEventPtr event = NULL; ++ virObjectEventPtr event2 = NULL; ++ const char *path; ++ virDomainDiskDefPtr disk; ++ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); ++ virDomainDiskDefPtr persistDisk = NULL; ++ bool save = false; ++ ++ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) ++ goto cleanup; ++ ++ if (!virDomainObjIsActive(vm)) { ++ VIR_DEBUG("Domain is not running"); ++ goto endjob; ++ } ++ ++ disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias); ++ ++ if (disk) { ++ /* Have to generate two variants of the event for old vs. new ++ * client callbacks */ ++ if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT && ++ disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) ++ type = disk->mirrorJob; ++ path = virDomainDiskGetSource(disk); ++ event = virDomainEventBlockJobNewFromObj(vm, path, type, status); ++ event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type, ++ status); ++ ++ /* If we completed a block pull or commit, then update the XML ++ * to match. */ ++ switch ((virConnectDomainEventBlockJobStatus) status) { ++ case VIR_DOMAIN_BLOCK_JOB_COMPLETED: ++ if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) { ++ if (vm->newDef) { ++ int indx = virDomainDiskIndexByName(vm->newDef, disk->dst, ++ false); ++ virStorageSourcePtr copy = NULL; ++ ++ if (indx >= 0) { ++ persistDisk = vm->newDef->disks[indx]; ++ copy = virStorageSourceCopy(disk->mirror, false); ++ if (virStorageSourceInitChainElement(copy, ++ persistDisk->src, ++ true) < 0) { ++ VIR_WARN("Unable to update persistent definition " ++ "on vm %s after block job", ++ vm->def->name); ++ virStorageSourceFree(copy); ++ copy = NULL; ++ persistDisk = NULL; ++ } ++ } ++ if (copy) { ++ virStorageSourceFree(persistDisk->src); ++ persistDisk->src = copy; ++ } ++ } ++ ++ /* XXX We want to revoke security labels and disk ++ * lease, as well as audit that revocation, before ++ * dropping the original source. But it gets tricky ++ * if both source and mirror share common backing ++ * files (we want to only revoke the non-shared ++ * portion of the chain); so for now, we leak the ++ * access to the original. */ ++ virStorageSourceFree(disk->src); ++ disk->src = disk->mirror; ++ } else { ++ virStorageSourceFree(disk->mirror); ++ } ++ ++ /* Recompute the cached backing chain to match our ++ * updates. Better would be storing the chain ourselves ++ * rather than reprobing, but we haven't quite completed ++ * that conversion to use our XML tracking. */ ++ disk->mirror = NULL; ++ save = disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE; ++ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; ++ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; ++ ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, ++ true, true)); ++ break; ++ ++ case VIR_DOMAIN_BLOCK_JOB_READY: ++ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; ++ save = true; ++ break; ++ ++ case VIR_DOMAIN_BLOCK_JOB_FAILED: ++ case VIR_DOMAIN_BLOCK_JOB_CANCELED: ++ virStorageSourceFree(disk->mirror); ++ disk->mirror = NULL; ++ disk->mirrorState = status == VIR_DOMAIN_BLOCK_JOB_FAILED ? ++ VIR_DOMAIN_DISK_MIRROR_STATE_ABORT : VIR_DOMAIN_DISK_MIRROR_STATE_NONE; ++ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; ++ save = true; ++ break; ++ ++ case VIR_DOMAIN_BLOCK_JOB_LAST: ++ break; ++ } ++ } ++ ++ if (save) { ++ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) ++ VIR_WARN("Unable to save status on vm %s after block job", ++ vm->def->name); ++ if (persistDisk && virDomainSaveConfig(cfg->configDir, ++ vm->newDef) < 0) ++ VIR_WARN("Unable to update persistent definition on vm %s " ++ "after block job", vm->def->name); ++ } ++ virObjectUnlock(vm); ++ virObjectUnref(cfg); ++ ++ if (event) ++ qemuDomainEventQueue(driver, event); ++ if (event2) ++ qemuDomainEventQueue(driver, event2); ++ ++ endjob: ++ qemuDomainObjEndJob(driver, vm); ++ cleanup: ++ VIR_FREE(diskAlias); ++} ++ ++ + static void qemuProcessEventHandler(void *data, void *opaque) + { + struct qemuProcessEvent *processEvent = data; +@@ -4506,6 +4641,13 @@ static void qemuProcessEventHandler(void *data, void *opaque) + case QEMU_PROCESS_EVENT_SERIAL_CHANGED: + processSerialChangedEvent(driver, vm, processEvent->data, + processEvent->action); ++ break; ++ case QEMU_PROCESS_EVENT_BLOCK_JOB: ++ processBlockJobEvent(driver, vm, ++ processEvent->data, ++ processEvent->action, ++ processEvent->status); ++ break; + case QEMU_PROCESS_EVENT_LAST: + break; + } +diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c +index 1994027..685cec6 100644 +--- a/src/qemu/qemu_process.c ++++ b/src/qemu/qemu_process.c +@@ -1017,123 +1017,42 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + void *opaque) + { + virQEMUDriverPtr driver = opaque; +- virObjectEventPtr event = NULL; +- virObjectEventPtr event2 = NULL; +- const char *path; +- virDomainDiskDefPtr disk; +- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +- virDomainDiskDefPtr persistDisk = NULL; +- bool save = false; ++ struct qemuProcessEvent *processEvent = NULL; ++ char *data; + + virObjectLock(vm); +- disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias); + +- if (disk) { +- /* Have to generate two variants of the event for old vs. new +- * client callbacks */ +- if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT && +- disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) +- type = disk->mirrorJob; +- path = virDomainDiskGetSource(disk); +- event = virDomainEventBlockJobNewFromObj(vm, path, type, status); +- event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type, +- status); +- +- /* If we completed a block pull or commit, then update the XML +- * to match. */ +- switch ((virConnectDomainEventBlockJobStatus) status) { +- case VIR_DOMAIN_BLOCK_JOB_COMPLETED: +- if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) { +- if (vm->newDef) { +- int indx = virDomainDiskIndexByName(vm->newDef, disk->dst, +- false); +- virStorageSourcePtr copy = NULL; +- +- if (indx >= 0) { +- persistDisk = vm->newDef->disks[indx]; +- copy = virStorageSourceCopy(disk->mirror, false); +- if (virStorageSourceInitChainElement(copy, +- persistDisk->src, +- true) < 0) { +- VIR_WARN("Unable to update persistent definition " +- "on vm %s after block job", +- vm->def->name); +- virStorageSourceFree(copy); +- copy = NULL; +- persistDisk = NULL; +- } +- } +- if (copy) { +- virStorageSourceFree(persistDisk->src); +- persistDisk->src = copy; +- } +- } +- +- /* XXX We want to revoke security labels and disk +- * lease, as well as audit that revocation, before +- * dropping the original source. But it gets tricky +- * if both source and mirror share common backing +- * files (we want to only revoke the non-shared +- * portion of the chain); so for now, we leak the +- * access to the original. */ +- virStorageSourceFree(disk->src); +- disk->src = disk->mirror; +- } else { +- virStorageSourceFree(disk->mirror); +- } ++ VIR_DEBUG("Block job for device %s (domain: %p,%s) type %d status %d", ++ diskAlias, vm, vm->def->name, type, status); + +- /* Recompute the cached backing chain to match our +- * updates. Better would be storing the chain ourselves +- * rather than reprobing, but we haven't quite completed +- * that conversion to use our XML tracking. */ +- disk->mirror = NULL; +- save = disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE; +- disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; +- disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; +- ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, +- true, true)); +- break; +- +- case VIR_DOMAIN_BLOCK_JOB_READY: +- disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; +- save = true; +- break; ++ if (VIR_ALLOC(processEvent) < 0) ++ goto error; + +- case VIR_DOMAIN_BLOCK_JOB_FAILED: +- case VIR_DOMAIN_BLOCK_JOB_CANCELED: +- virStorageSourceFree(disk->mirror); +- disk->mirror = NULL; +- disk->mirrorState = status == VIR_DOMAIN_BLOCK_JOB_FAILED ? +- VIR_DOMAIN_DISK_MIRROR_STATE_ABORT : VIR_DOMAIN_DISK_MIRROR_STATE_NONE; +- disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; +- save = true; +- break; ++ processEvent->eventType = QEMU_PROCESS_EVENT_BLOCK_JOB; ++ if (VIR_STRDUP(data, diskAlias) < 0) ++ goto error; ++ processEvent->data = data; ++ processEvent->vm = vm; ++ processEvent->action = type; ++ processEvent->status = status; + +- case VIR_DOMAIN_BLOCK_JOB_LAST: +- break; +- } ++ virObjectRef(vm); ++ if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { ++ ignore_value(virObjectUnref(vm)); ++ goto error; + } + +- if (save) { +- if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) +- VIR_WARN("Unable to save status on vm %s after block job", +- vm->def->name); +- if (persistDisk && virDomainSaveConfig(cfg->configDir, +- vm->newDef) < 0) +- VIR_WARN("Unable to update persistent definition on vm %s " +- "after block job", vm->def->name); +- } ++ cleanup: + virObjectUnlock(vm); +- virObjectUnref(cfg); +- +- if (event) +- qemuDomainEventQueue(driver, event); +- if (event2) +- qemuDomainEventQueue(driver, event2); +- + return 0; ++ error: ++ if (processEvent) ++ VIR_FREE(processEvent->data); ++ VIR_FREE(processEvent); ++ goto cleanup; + } + ++ + static int + qemuProcessHandleGraphics(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, diff --git a/0104-qemu-Disallow-concurrent-block-jobs-on-a-single-disk.patch b/0104-qemu-Disallow-concurrent-block-jobs-on-a-single-disk.patch new file mode 100644 index 0000000..711030a --- /dev/null +++ b/0104-qemu-Disallow-concurrent-block-jobs-on-a-single-disk.patch @@ -0,0 +1,175 @@ +From: Peter Krempa +Date: Fri, 13 Mar 2015 17:22:04 +0100 +Subject: [PATCH] qemu: Disallow concurrent block jobs on a single disk + +While qemu may be prepared to do this libvirt is not. Forbid the block +ops until we fix our code. + +(cherry picked from commit 51f9f03a4ca50b070c0fbfb29748d49f583e15e1) +--- + src/conf/domain_conf.h | 4 ++++ + src/qemu/qemu_domain.c | 23 +++++++++++++++++++++++ + src/qemu/qemu_domain.h | 2 ++ + src/qemu/qemu_driver.c | 28 +++++++++++++--------------- + 4 files changed, 42 insertions(+), 15 deletions(-) + +diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h +index bca1b94..47bdb61 100644 +--- a/src/conf/domain_conf.h ++++ b/src/conf/domain_conf.h +@@ -664,6 +664,10 @@ struct _virDomainDiskDef { + int tray_status; /* enum virDomainDiskTray */ + int removable; /* enum virTristateSwitch */ + ++ /* ideally we want a smarter way to interlock block jobs on single qemu disk ++ * in the future, but for now we just disallow any concurrent job on a ++ * single disk */ ++ bool blockjob; + virStorageSourcePtr mirror; + int mirrorState; /* enum virDomainDiskMirrorState */ + int mirrorJob; /* virDomainBlockJobType */ +diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c +index bd7d8a4..7c2d046 100644 +--- a/src/qemu/qemu_domain.c ++++ b/src/qemu/qemu_domain.c +@@ -2757,6 +2757,29 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, + return ret; + } + ++ ++bool ++qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk) ++{ ++ if (disk->mirror) { ++ virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, ++ _("disk '%s' already in active block job"), ++ disk->dst); ++ ++ return true; ++ } ++ ++ if (disk->blockjob) { ++ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, ++ _("disk '%s' already in active block job"), ++ disk->dst); ++ return true; ++ } ++ ++ return false; ++} ++ ++ + int + qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, + virDomainObjPtr vm, +diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h +index a7ebb47..41e075b 100644 +--- a/src/qemu/qemu_domain.h ++++ b/src/qemu/qemu_domain.h +@@ -414,6 +414,8 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + ++bool qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk); ++ + void qemuDomObjEndAPI(virDomainObjPtr *vm); + + #endif /* __QEMU_DOMAIN_H__ */ +diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c +index 2e8c9d8..3ec057b 100644 +--- a/src/qemu/qemu_driver.c ++++ b/src/qemu/qemu_driver.c +@@ -4569,6 +4569,7 @@ processBlockJobEvent(virQEMUDriverPtr driver, + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; + ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, + true, true)); ++ disk->blockjob = false; + break; + + case VIR_DOMAIN_BLOCK_JOB_READY: +@@ -4584,6 +4585,7 @@ processBlockJobEvent(virQEMUDriverPtr driver, + VIR_DOMAIN_DISK_MIRROR_STATE_ABORT : VIR_DOMAIN_DISK_MIRROR_STATE_NONE; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; + save = true; ++ disk->blockjob = false; + break; + + case VIR_DOMAIN_BLOCK_JOB_LAST: +@@ -15827,6 +15829,7 @@ qemuDomainBlockPivot(virConnectPtr conn, + disk->mirror = NULL; + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; ++ disk->blockjob = false; + } + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + ret = -1; +@@ -15927,12 +15930,9 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, + goto endjob; + disk = vm->def->disks[idx]; + +- if (mode == BLOCK_JOB_PULL && disk->mirror) { +- virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, +- _("disk '%s' already in active block job"), +- disk->dst); ++ if (mode == BLOCK_JOB_PULL && qemuDomainDiskBlockJobIsActive(disk)) + goto endjob; +- } ++ + if (mode == BLOCK_JOB_ABORT) { + if ((flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) && + !(async && disk->mirror)) { +@@ -16017,6 +16017,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, + if (mode == BLOCK_JOB_ABORT && disk->mirror) + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; + goto endjob; ++ } else if (mode == BLOCK_JOB_PULL) { ++ disk->blockjob = true; + } + + waitjob: +@@ -16269,12 +16271,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, + if (!device) + goto endjob; + disk = vm->def->disks[idx]; +- if (disk->mirror) { +- virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, +- _("disk '%s' already in active block job"), +- disk->dst); ++ if (qemuDomainDiskBlockJobIsActive(disk)) + goto endjob; +- } + + if (!(virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_MIRROR) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC))) { +@@ -16396,6 +16394,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, + disk->mirror = mirror; + mirror = NULL; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY; ++ disk->blockjob = true; + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + VIR_WARN("Unable to save status on vm %s after state change", +@@ -16657,12 +16656,9 @@ qemuDomainBlockCommit(virDomainPtr dom, + disk->dst); + goto endjob; + } +- if (disk->mirror) { +- virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, +- _("disk '%s' already in active block job"), +- disk->dst); ++ ++ if (qemuDomainDiskBlockJobIsActive(disk)) + goto endjob; +- } + if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0) + goto endjob; + +@@ -16785,6 +16781,8 @@ qemuDomainBlockCommit(virDomainPtr dom, + goto endjob; + } + ++ disk->blockjob = true; ++ + if (mirror) { + if (ret == 0) { + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); diff --git a/0105-qemu-block-commit-Mark-disk-in-block-jobs-only-on-su.patch b/0105-qemu-block-commit-Mark-disk-in-block-jobs-only-on-su.patch new file mode 100644 index 0000000..5652f99 --- /dev/null +++ b/0105-qemu-block-commit-Mark-disk-in-block-jobs-only-on-su.patch @@ -0,0 +1,28 @@ +From: Peter Krempa +Date: Mon, 16 Mar 2015 16:52:44 +0100 +Subject: [PATCH] qemu: block-commit: Mark disk in block jobs only on + successful command + +Patch 51f9f03a4ca50b070c0fbfb29748d49f583e15e1 introduces a regression +where if a blockCommit operation fails the disk is still marked as being +part of a block job but can't be unmarked later. + +(cherry picked from commit ee744b5b387b5123ee40683c52ab40783ffc3020) +--- + src/qemu/qemu_driver.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c +index 3ec057b..4817e06 100644 +--- a/src/qemu/qemu_driver.c ++++ b/src/qemu/qemu_driver.c +@@ -16781,7 +16781,8 @@ qemuDomainBlockCommit(virDomainPtr dom, + goto endjob; + } + +- disk->blockjob = true; ++ if (ret == 0) ++ disk->blockjob = true; + + if (mirror) { + if (ret == 0) { diff --git a/libvirt.spec b/libvirt.spec index 7a45c3c..1f117a0 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -371,7 +371,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 1.2.13.1 -Release: 2%{?dist}%{?extra_release} +Release: 3%{?dist}%{?extra_release} License: LGPLv2+ Group: Development/Libraries BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root @@ -394,6 +394,11 @@ Patch0008: 0008-virnetdev-fix-moving-of-802.11-phys.patch # polkit: Allow password-less access for 'libvirt' group (bz #957300) Patch0101: 0101-polkit-Allow-password-less-access-for-libvirt-group.patch +# Add sanity checks for drive mirroring (bz #1263438) +Patch0102: 0102-qemu-process-Export-qemuProcessFindDomainDiskByAlias.patch +Patch0103: 0103-qemu-event-Don-t-fiddle-with-disk-backing-trees-with.patch +Patch0104: 0104-qemu-Disallow-concurrent-block-jobs-on-a-single-disk.patch +Patch0105: 0105-qemu-block-commit-Mark-disk-in-block-jobs-only-on-su.patch %if %{with_libvirtd} Requires: libvirt-daemon = %{version}-%{release} @@ -2304,6 +2309,9 @@ exit 0 %doc examples/systemtap %changelog +* Mon Sep 21 2015 Cole Robinson - 1.2.13.1-3 +- Add sanity checks for drive mirroring (bz #1263438) + * Fri Jun 05 2015 Cole Robinson - 1.2.13.1-2 - lxc network fixes (bz #1225591, bz #1225593, bz #1225594) - polkit: Allow password-less access for 'libvirt' group (bz #957300)