10:12 a.m.
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
10:12 a.m.
New subject: [libvirt] [PATCH v2 1/9] qemu: snapshot: Fix image lock handling when taking a snapshot
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(a)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
10:12 a.m.
New subject: [libvirt] [PATCH v2 2/9] qemu: snapshot: Save status and config XMLs only on success
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(a)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
10:12 a.m.
New subject: [libvirt] [PATCH v2 3/9] qemu: snapshot: Move error preservation to qemuDomainSnapshotDiskDataCleanup
Make qemuDomainSnapshotDiskDataCleanup cleanup section friendly by
moving the error preservation code inside it.
Signed-off-by: Peter Krempa <pkrempa(a)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
10:12 a.m.
New subject: [libvirt] [PATCH v2 4/9] qemu: snapshot: Rename external disk snapshot handling functions
Fix and unify the naming of external snapshot preparation functions.
Signed-off-by: Peter Krempa <pkrempa(a)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
10:12 a.m.
New subject: [libvirt] [PATCH v2 5/9] qemu: Disband qemuDomainSnapshotCreateSingleDiskActive
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(a)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
10:12 a.m.
New subject: [libvirt] [PATCH v2 6/9] qemu: Merge use of 'reuse' flag in qemuDomainSnapshotDiskPrepareOne
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
Reviewed-by: Ján Tomko <jtomko(a)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
10:12 a.m.
New subject: [libvirt] [PATCH v2 7/9] qemu: snapshot: Skip overlay file creation/interogation if unsupported
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(a)redhat.com>
Reviewed-by: Ján Tomko <jtomko(a)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
10:12 a.m.
New subject: [libvirt] [PATCH v2 8/9] qemu: Add -blockdev support for external snapshots
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(a)redhat.com>
Reviewed-by: Ján Tomko <jtomko(a)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
10:12 a.m.
New subject: [libvirt] [PATCH v2 9/9] qemu: Defer support checks for external active snapshots to blockdev code or qemu
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(a)redhat.com>
Reviewed-by: Ján Tomko <jtomko(a)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
1968
days inactive
1968
days old
9 comments
1 participants
participants (1)
-
Peter Krempa