[libvirt] [PATCH v2 0/9] qemu: Blockdevize external snapshot creation (blokcdev-add saga)
Patches 1-5 of the original series were already pushed. Patches 1-3 of this series are new and deal with fixing of a image locking bug which was added during the blockdev-add saga as we've changed the handling of the 'readonly' flag of an image. Patch 4 is new and should solve review feedback which complained about function naming. The rest of the series is almost exactly as in previous posting with following exceptions: 1) Review feedback incorporated and R-b's applied. 2) Patch 6/9 has a different summary 3) Patch 8/9 fixes a memory leak/crash as a part of the cleanup was misplaced out of the cleaning loop (while still using the 'i' to dereference arrays). 4) Conflicts with 4/9 resolved. Peter Krempa (9): qemu: snapshot: Fix image lock handling when taking a snapshot qemu: snapshot: Save status and config XMLs only on success qemu: snapshot: Move error preservation to qemuDomainSnapshotDiskDataCleanup qemu: snapshot: Rename external disk snapshot handling functions qemu: Disband qemuDomainSnapshotCreateSingleDiskActive qemu: Merge use of 'reuse' flag in qemuDomainSnapshotDiskPrepareOne qemu: snapshot: Skip overlay file creation/interogation if unsupported qemu: Add -blockdev support for external snapshots qemu: Defer support checks for external active snapshots to blockdev code or qemu src/qemu/qemu_driver.c | 276 +++++++++++++++++++++++++---------------- 1 file changed, 171 insertions(+), 105 deletions(-) -- 2.21.0
When we take a snapshot we must properly remove our locking infrastructure locks. This was broken by commit 3817fa10c4ad9 which attempted to properly track the readonly state for the image as the locking code was executed after this change. Since we forced the image which was locked as read-write to read-only prior to unlocking it the write lock was not dropped. Fix it by moving the locking code prior to modifying the readonly flag. https://bugzilla.redhat.com/show_bug.cgi?id=1745618 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 78f5471b79..a1f4105cc3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15432,6 +15432,13 @@ qemuDomainSnapshotUpdateDiskSources(virQEMUDriverPtr driver, if (qemuSecurityMoveImageMetadata(driver, vm, dd->disk->src, dd->src) < 0) VIR_WARN("Unable to move disk metadata on vm %s", vm->def->name); + /* unlock the write lock on the original image as qemu will no longer write to it */ + virDomainLockImageDetach(driver->lockManager, vm, dd->disk->src); + + /* unlock also the new image if the VM is paused to follow the locking semantics */ + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) + virDomainLockImageDetach(driver->lockManager, vm, dd->src); + /* the old disk image is now readonly */ dd->disk->src->readonly = true; @@ -15550,23 +15557,8 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, ret = 0; cleanup: - if (ret < 0) { + if (ret < 0) virErrorPreserveLast(&orig_err); - } else { - /* on successful snapshot we need to remove locks from the now-old - * disks and if the VM is paused release locks on the images since qemu - * stopped using them*/ - bool paused = virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING; - - for (i = 0; i < ndiskdata; i++) { - if (paused) - virDomainLockImageDetach(driver->lockManager, vm, - diskdata[i].disk->src); - - virDomainLockImageDetach(driver->lockManager, vm, - diskdata[i].disk->src->backingStore); - } - } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0 || (vm->newDef && virDomainSaveConfig(cfg->configDir, driver->caps, -- 2.21.0
We changed to always saving the status and config XMLs to simplify code. After a few more refactors it's now possible to move it to the appropriate place and save the XMLs only on success again. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a1f4105cc3..7af3af7809 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15554,17 +15554,17 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (rc < 0) goto cleanup; + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0 || + (vm->newDef && virDomainSaveConfig(cfg->configDir, driver->caps, + vm->newDef) < 0)) + goto cleanup; + ret = 0; cleanup: if (ret < 0) virErrorPreserveLast(&orig_err); - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0 || - (vm->newDef && virDomainSaveConfig(cfg->configDir, driver->caps, - vm->newDef) < 0)) - ret = -1; - qemuDomainSnapshotDiskDataCleanup(diskdata, ndiskdata, driver, vm); virErrorRestore(&orig_err); -- 2.21.0
Make qemuDomainSnapshotDiskDataCleanup cleanup section friendly by moving the error preservation code inside it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7af3af7809..4f28848dae 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15271,11 +15271,14 @@ qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data, virQEMUDriverPtr driver, virDomainObjPtr vm) { + virErrorPtr orig_err; size_t i; if (!data) return; + virErrorPreserveLast(&orig_err); + for (i = 0; i < ndata; i++) { /* on success of the snapshot the 'src' and 'persistsrc' properties will * be set to NULL by qemuDomainSnapshotUpdateDiskSources */ @@ -15299,6 +15302,7 @@ qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data, } VIR_FREE(data); + virErrorRestore(&orig_err); } @@ -15503,7 +15507,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; qemuDomainSnapshotDiskDataPtr diskdata = NULL; size_t ndiskdata = 0; - virErrorPtr orig_err = NULL; if (virDomainObjCheckActive(vm) < 0) return -1; @@ -15562,12 +15565,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, ret = 0; cleanup: - if (ret < 0) - virErrorPreserveLast(&orig_err); - qemuDomainSnapshotDiskDataCleanup(diskdata, ndiskdata, driver, vm); - virErrorRestore(&orig_err); - return ret; } -- 2.21.0
Fix and unify the naming of external snapshot preparation functions. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 64 +++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4f28848dae..1191a6e233 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15266,10 +15266,10 @@ typedef qemuDomainSnapshotDiskData *qemuDomainSnapshotDiskDataPtr; static void -qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data, - size_t ndata, - virQEMUDriverPtr driver, - virDomainObjPtr vm) +qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, + size_t ndata, + virQEMUDriverPtr driver, + virDomainObjPtr vm) { virErrorPtr orig_err; size_t i; @@ -15281,7 +15281,7 @@ qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data, for (i = 0; i < ndata; i++) { /* on success of the snapshot the 'src' and 'persistsrc' properties will - * be set to NULL by qemuDomainSnapshotUpdateDiskSources */ + * be set to NULL by qemuDomainSnapshotDiskUpdateSource */ if (data[i].src) { if (data[i].created && virStorageFileUnlink(data[i].src) < 0) { @@ -15307,12 +15307,12 @@ qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data, static int -qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainDiskDefPtr disk, - virDomainSnapshotDiskDefPtr snapdisk, - qemuDomainSnapshotDiskDataPtr dd, - bool reuse) +qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virDomainSnapshotDiskDefPtr snapdisk, + qemuDomainSnapshotDiskDataPtr dd, + bool reuse) { char *backingStoreStr; virDomainDiskDefPtr persistdisk; @@ -15363,18 +15363,18 @@ qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr driver, /** - * qemuDomainSnapshotDiskDataCollect: + * qemuDomainSnapshotDiskPrepare: * * Collects and prepares a list of structures that hold information about disks * that are selected for the snapshot. */ static int -qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMomentObjPtr snap, - bool reuse, - qemuDomainSnapshotDiskDataPtr *rdata, - size_t *rndata) +qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMomentObjPtr snap, + bool reuse, + qemuDomainSnapshotDiskDataPtr *rdata, + size_t *rndata) { size_t i; qemuDomainSnapshotDiskDataPtr data; @@ -15389,9 +15389,9 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, if (snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) continue; - if (qemuDomainSnapshotDiskDataCollectOne(driver, vm, vm->def->disks[i], - snapdef->disks + i, - data + ndata++, reuse) < 0) + if (qemuDomainSnapshotDiskPrepareOne(driver, vm, vm->def->disks[i], + snapdef->disks + i, + data + ndata++, reuse) < 0) goto cleanup; } @@ -15400,13 +15400,13 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuDomainSnapshotDiskDataCleanup(data, ndata, driver, vm); + qemuDomainSnapshotDiskCleanup(data, ndata, driver, vm); return ret; } static void -qemuDomainSnapshotUpdateDiskSourcesRenumber(virStorageSourcePtr src) +qemuDomainSnapshotDiskUpdateSourceRenumber(virStorageSourcePtr src) { virStorageSourcePtr next; unsigned int idx = 1; @@ -15417,7 +15417,7 @@ qemuDomainSnapshotUpdateDiskSourcesRenumber(virStorageSourcePtr src) /** - * qemuDomainSnapshotUpdateDiskSources: + * qemuDomainSnapshotDiskUpdateSource: * @driver: QEMU driver * @vm: domain object * @dd: snapshot disk data object @@ -15425,9 +15425,9 @@ qemuDomainSnapshotUpdateDiskSourcesRenumber(virStorageSourcePtr src) * Updates disk definition after a successful snapshot. */ static void -qemuDomainSnapshotUpdateDiskSources(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuDomainSnapshotDiskDataPtr dd) +qemuDomainSnapshotDiskUpdateSource(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainSnapshotDiskDataPtr dd) { /* storage driver access won'd be needed */ if (dd->initialized) @@ -15451,7 +15451,7 @@ qemuDomainSnapshotUpdateDiskSources(virQEMUDriverPtr driver, VIR_STEAL_PTR(dd->disk->src, dd->src); /* fix numbering of disks */ - qemuDomainSnapshotUpdateDiskSourcesRenumber(dd->disk->src); + qemuDomainSnapshotDiskUpdateSourceRenumber(dd->disk->src); if (dd->persistdisk) { VIR_STEAL_PTR(dd->persistsrc->backingStore, dd->persistdisk->src); @@ -15516,8 +15516,8 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, /* prepare a list of objects to use in the vm definition so that we don't * have to roll back later */ - if (qemuDomainSnapshotDiskDataCollect(driver, vm, snap, reuse, - &diskdata, &ndiskdata) < 0) + if (qemuDomainSnapshotDiskPrepare(driver, vm, snap, reuse, + &diskdata, &ndiskdata) < 0) goto cleanup; /* check whether there's anything to do */ @@ -15551,7 +15551,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0); if (rc == 0) - qemuDomainSnapshotUpdateDiskSources(driver, vm, dd); + qemuDomainSnapshotDiskUpdateSource(driver, vm, dd); } if (rc < 0) @@ -15565,7 +15565,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuDomainSnapshotDiskDataCleanup(diskdata, ndiskdata, driver, vm); + qemuDomainSnapshotDiskCleanup(diskdata, ndiskdata, driver, vm); return ret; } -- 2.21.0
After we always assume support for the 'transaction' command (c358adc57113b) and follow-up cleanups qemuDomainSnapshotCreateSingleDiskActive lost its value. Move the code into appropriate helpers and remove the function. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 53 ++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1191a6e233..8ea34e868c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15358,6 +15358,22 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, } } + /* pre-create the image file so that we can label it before handing it to qemu */ + if (!reuse && dd->src->type != VIR_STORAGE_TYPE_BLOCK) { + if (virStorageFileCreate(dd->src) < 0) { + virReportSystemError(errno, _("failed to create image file '%s'"), + NULLSTR(dd->src->path)); + return -1; + } + dd->created = true; + } + + /* set correct security, cgroup and locking options on the new image */ + if (qemuDomainStorageSourceAccessAllow(driver, vm, dd->src, false, true) < 0) + return -1; + + dd->prepared = true; + return 0; } @@ -15460,36 +15476,6 @@ qemuDomainSnapshotDiskUpdateSource(virQEMUDriverPtr driver, } -static int -qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuDomainSnapshotDiskDataPtr dd, - virJSONValuePtr actions, - bool reuse) -{ - if (qemuBlockSnapshotAddLegacy(actions, dd->disk, dd->src, reuse) < 0) - return -1; - - /* pre-create the image file so that we can label it before handing it to qemu */ - if (!reuse && dd->src->type != VIR_STORAGE_TYPE_BLOCK) { - if (virStorageFileCreate(dd->src) < 0) { - virReportSystemError(errno, _("failed to create image file '%s'"), - NULLSTR(dd->src->path)); - return -1; - } - dd->created = true; - } - - /* set correct security, cgroup and locking options on the new image */ - if (qemuDomainStorageSourceAccessAllow(driver, vm, dd->src, false, true) < 0) - return -1; - - dd->prepared = true; - - return 0; -} - - /* The domain is expected to be locked and active. */ static int qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, @@ -15531,9 +15517,10 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, * VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL with a valid file name and * qcow2 format. */ for (i = 0; i < ndiskdata; i++) { - if (qemuDomainSnapshotCreateSingleDiskActive(driver, vm, - &diskdata[i], - actions, reuse) < 0) + if (qemuBlockSnapshotAddLegacy(actions, + diskdata[i].disk, + diskdata[i].src, + reuse) < 0) goto cleanup; } -- 2.21.0
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8ea34e868c..4e0804d505 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15356,16 +15356,16 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, else VIR_FREE(backingStoreStr); } - } - - /* pre-create the image file so that we can label it before handing it to qemu */ - if (!reuse && dd->src->type != VIR_STORAGE_TYPE_BLOCK) { - if (virStorageFileCreate(dd->src) < 0) { - virReportSystemError(errno, _("failed to create image file '%s'"), - NULLSTR(dd->src->path)); - return -1; + } else { + /* pre-create the image file so that we can label it before handing it to qemu */ + if (dd->src->type != VIR_STORAGE_TYPE_BLOCK) { + if (virStorageFileCreate(dd->src) < 0) { + virReportSystemError(errno, _("failed to create image file '%s'"), + NULLSTR(dd->src->path)); + return -1; + } + dd->created = true; } - dd->created = true; } /* set correct security, cgroup and locking options on the new image */ -- 2.21.0
With blockdev we'll be able to support protocols which are not supported by the storage backends in libvirt. This means that we have to be able to skip the creation and relative storage path reading if it's not supported. This will make it impossible to use relative backing for network protocols but that would be almost insane anyways. Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4e0804d505..21bec4f67b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15316,6 +15316,8 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, { char *backingStoreStr; virDomainDiskDefPtr persistdisk; + bool supportsCreate; + bool supportsBacking; dd->disk = disk; @@ -15340,31 +15342,38 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, return -1; } - if (qemuDomainStorageFileInit(driver, vm, dd->src, NULL) < 0) - return -1; - - dd->initialized = true; + supportsCreate = virStorageFileSupportsCreate(dd->src); + supportsBacking = virStorageFileSupportsBackingChainTraversal(dd->src); - /* relative backing store paths need to be updated so that relative - * block commit still works */ - if (reuse) { - if (virStorageFileGetBackingStoreStr(dd->src, &backingStoreStr) < 0) + if (supportsCreate || supportsBacking) { + if (qemuDomainStorageFileInit(driver, vm, dd->src, NULL) < 0) return -1; - if (backingStoreStr != NULL) { - if (virStorageIsRelative(backingStoreStr)) - VIR_STEAL_PTR(dd->relPath, backingStoreStr); - else - VIR_FREE(backingStoreStr); - } - } else { - /* pre-create the image file so that we can label it before handing it to qemu */ - if (dd->src->type != VIR_STORAGE_TYPE_BLOCK) { - if (virStorageFileCreate(dd->src) < 0) { - virReportSystemError(errno, _("failed to create image file '%s'"), - NULLSTR(dd->src->path)); - return -1; + + dd->initialized = true; + + /* relative backing store paths need to be updated so that relative + * block commit still works */ + if (reuse) { + if (supportsBacking) { + if (virStorageFileGetBackingStoreStr(dd->src, &backingStoreStr) < 0) + return -1; + if (backingStoreStr != NULL) { + if (virStorageIsRelative(backingStoreStr)) + VIR_STEAL_PTR(dd->relPath, backingStoreStr); + else + VIR_FREE(backingStoreStr); + } + } + } else { + /* pre-create the image file so that we can label it before handing it to qemu */ + if (supportsCreate && dd->src->type != VIR_STORAGE_TYPE_BLOCK) { + if (virStorageFileCreate(dd->src) < 0) { + virReportSystemError(errno, _("failed to create image file '%s'"), + NULLSTR(dd->src->path)); + return -1; + } + dd->created = true; } - dd->created = true; } } -- 2.21.0
Use the code for creating or attaching new storage source in the snapshot code and switch to 'blockdev-snapshot' for creating the snapshot itself. Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 104 ++++++++++++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 21bec4f67b..0cb54445f2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15256,6 +15256,8 @@ struct _qemuDomainSnapshotDiskData { bool prepared; /* @src was prepared using qemuDomainStorageSourceAccessAllow */ virDomainDiskDefPtr disk; char *relPath; /* relative path component to fill into original disk */ + qemuBlockStorageSourceChainDataPtr crdata; + bool blockdevadded; virStorageSourcePtr persistsrc; virDomainDiskDefPtr persistdisk; @@ -15269,7 +15271,8 @@ static void qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, size_t ndata, virQEMUDriverPtr driver, - virDomainObjPtr vm) + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) { virErrorPtr orig_err; size_t i; @@ -15283,6 +15286,15 @@ qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, /* on success of the snapshot the 'src' and 'persistsrc' properties will * be set to NULL by qemuDomainSnapshotDiskUpdateSource */ if (data[i].src) { + if (data[i].blockdevadded) { + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { + + qemuBlockStorageSourceAttachRollback(qemuDomainGetMonitor(vm), + data[i].crdata->srcdata[0]); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + } + } + if (data[i].created && virStorageFileUnlink(data[i].src) < 0) { VIR_WARN("Unable to remove just-created %s", @@ -15299,6 +15311,7 @@ qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, } virObjectUnref(data[i].persistsrc); VIR_FREE(data[i].relPath); + qemuBlockStorageSourceChainDataFree(data[i].crdata); } VIR_FREE(data); @@ -15309,15 +15322,20 @@ qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, static int qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, virDomainObjPtr vm, + virQEMUDriverConfigPtr cfg, virDomainDiskDefPtr disk, virDomainSnapshotDiskDefPtr snapdisk, qemuDomainSnapshotDiskDataPtr dd, - bool reuse) + bool reuse, + bool blockdev, + qemuDomainAsyncJob asyncJob) { + qemuDomainObjPrivatePtr priv = vm->privateData; char *backingStoreStr; virDomainDiskDefPtr persistdisk; bool supportsCreate; bool supportsBacking; + int rc; dd->disk = disk; @@ -15383,6 +15401,44 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, dd->prepared = true; + if (blockdev) { + /* create a terminator for the snapsot disks so that qemu does not try + * to open them at first */ + virObjectUnref(dd->src->backingStore); + if (!(dd->src->backingStore = virStorageSourceNew())) + return -1; + + if (qemuDomainPrepareStorageSourceBlockdev(dd->disk, dd->src, + priv, cfg) < 0) + return -1; + + if (!(dd->crdata = qemuBuildStorageSourceChainAttachPrepareBlockdevTop(dd->src, + priv->qemuCaps))) + return -1; + + if (reuse) { + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + rc = qemuBlockStorageSourceAttachApply(qemuDomainGetMonitor(vm), + dd->crdata->srcdata[0]); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + return -1; + } else { + if (qemuBlockStorageSourceCreateDetectSize(vm, dd->src, dd->disk->src, + asyncJob) < 0) + return -1; + + if (qemuBlockStorageSourceCreate(vm, dd->src, dd->disk->src, + NULL, dd->crdata->srcdata[0], + asyncJob) < 0) + return -1; + } + + dd->blockdevadded = true; + } + return 0; } @@ -15397,7 +15453,10 @@ static int qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainMomentObjPtr snap, + virQEMUDriverConfigPtr cfg, bool reuse, + bool blockdev, + qemuDomainAsyncJob asyncJob, qemuDomainSnapshotDiskDataPtr *rdata, size_t *rndata) { @@ -15414,9 +15473,10 @@ qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, if (snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) continue; - if (qemuDomainSnapshotDiskPrepareOne(driver, vm, vm->def->disks[i], + if (qemuDomainSnapshotDiskPrepareOne(driver, vm, cfg, vm->def->disks[i], snapdef->disks + i, - data + ndata++, reuse) < 0) + data + ndata++, reuse, blockdev, + asyncJob) < 0) goto cleanup; } @@ -15425,7 +15485,7 @@ qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuDomainSnapshotDiskCleanup(data, ndata, driver, vm); + qemuDomainSnapshotDiskCleanup(data, ndata, driver, vm, asyncJob); return ret; } @@ -15446,13 +15506,15 @@ qemuDomainSnapshotDiskUpdateSourceRenumber(virStorageSourcePtr src) * @driver: QEMU driver * @vm: domain object * @dd: snapshot disk data object + * @blockdev: -blockdev is in use for the VM * * Updates disk definition after a successful snapshot. */ static void qemuDomainSnapshotDiskUpdateSource(virQEMUDriverPtr driver, virDomainObjPtr vm, - qemuDomainSnapshotDiskDataPtr dd) + qemuDomainSnapshotDiskDataPtr dd, + bool blockdev) { /* storage driver access won'd be needed */ if (dd->initialized) @@ -15472,11 +15534,13 @@ qemuDomainSnapshotDiskUpdateSource(virQEMUDriverPtr driver, dd->disk->src->readonly = true; VIR_STEAL_PTR(dd->disk->src->relPath, dd->relPath); + virObjectUnref(dd->src->backingStore); VIR_STEAL_PTR(dd->src->backingStore, dd->disk->src); VIR_STEAL_PTR(dd->disk->src, dd->src); /* fix numbering of disks */ - qemuDomainSnapshotDiskUpdateSourceRenumber(dd->disk->src); + if (!blockdev) + qemuDomainSnapshotDiskUpdateSourceRenumber(dd->disk->src); if (dd->persistdisk) { VIR_STEAL_PTR(dd->persistsrc->backingStore, dd->persistdisk->src); @@ -15502,6 +15566,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; qemuDomainSnapshotDiskDataPtr diskdata = NULL; size_t ndiskdata = 0; + bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); if (virDomainObjCheckActive(vm) < 0) return -1; @@ -15511,8 +15576,8 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, /* prepare a list of objects to use in the vm definition so that we don't * have to roll back later */ - if (qemuDomainSnapshotDiskPrepare(driver, vm, snap, reuse, - &diskdata, &ndiskdata) < 0) + if (qemuDomainSnapshotDiskPrepare(driver, vm, snap, cfg, reuse, blockdev, + asyncJob, &diskdata, &ndiskdata) < 0) goto cleanup; /* check whether there's anything to do */ @@ -15526,11 +15591,18 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, * VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL with a valid file name and * qcow2 format. */ for (i = 0; i < ndiskdata; i++) { - if (qemuBlockSnapshotAddLegacy(actions, - diskdata[i].disk, - diskdata[i].src, - reuse) < 0) - goto cleanup; + if (blockdev) { + if (qemuBlockSnapshotAddBlockdev(actions, + diskdata[i].disk, + diskdata[i].src)) + goto cleanup; + } else { + if (qemuBlockSnapshotAddLegacy(actions, + diskdata[i].disk, + diskdata[i].src, + reuse) < 0) + goto cleanup; + } } if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) @@ -15547,7 +15619,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0); if (rc == 0) - qemuDomainSnapshotDiskUpdateSource(driver, vm, dd); + qemuDomainSnapshotDiskUpdateSource(driver, vm, dd, blockdev); } if (rc < 0) @@ -15561,7 +15633,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuDomainSnapshotDiskCleanup(diskdata, ndiskdata, driver, vm); + qemuDomainSnapshotDiskCleanup(diskdata, ndiskdata, driver, vm, asyncJob); return ret; } -- 2.21.0
Remove libvit's support check for the target of an external snapshot to the blockdev code or qemu. This will potentially require a more complex cleanup but removes a level of hardcoded feature checks. Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0cb54445f2..7876dd7e22 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14917,7 +14917,8 @@ qemuDomainSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdi static int qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr snapdisk, - virDomainDiskDefPtr domdisk) + virDomainDiskDefPtr domdisk, + bool blockdev) { int actualType = virStorageSourceGetActualType(snapdisk->src); @@ -14934,6 +14935,10 @@ qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr snapdisk break; case VIR_STORAGE_TYPE_NETWORK: + /* defer all of the checking to either qemu or libvirt's blockdev code */ + if (blockdev) + break; + switch ((virStorageNetProtocol) snapdisk->src->protocol) { case VIR_STORAGE_NET_PROTOCOL_GLUSTER: break; @@ -14981,7 +14986,8 @@ static int qemuDomainSnapshotPrepareDiskExternal(virDomainDiskDefPtr disk, virDomainSnapshotDiskDefPtr snapdisk, bool active, - bool reuse) + bool reuse, + bool blockdev) { struct stat st; int err; @@ -15004,7 +15010,7 @@ qemuDomainSnapshotPrepareDiskExternal(virDomainDiskDefPtr disk, if (qemuDomainSnapshotPrepareDiskExternalInactive(snapdisk, disk) < 0) return -1; } else { - if (qemuDomainSnapshotPrepareDiskExternalActive(snapdisk, disk) < 0) + if (qemuDomainSnapshotPrepareDiskExternalActive(snapdisk, disk, blockdev) < 0) return -1; } @@ -15105,6 +15111,8 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def, unsigned int *flags) { + qemuDomainObjPrivatePtr priv = vm->privateData; + bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); int ret = -1; size_t i; bool active = virDomainObjIsActive(vm); @@ -15163,7 +15171,7 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, } if (qemuDomainSnapshotPrepareDiskExternal(dom_disk, disk, - active, reuse) < 0) + active, reuse, blockdev) < 0) goto cleanup; external++; -- 2.21.0
participants (1)
-
Peter Krempa