11:41 a.m.
Peter Krempa (3):
qemuSnapshotForEachQcow2: Don't initialize 'nrollback'
qemu: process: Export qemuPrepareNVRAM for use in snapshot code
qemu: snapshot: Ensure that NVRAM image exists when taking inactive
internal snapshot
src/qemu/qemu_process.c | 26 ++++++++++++++------------
src/qemu/qemu_process.h | 4 ++++
src/qemu/qemu_snapshot.c | 25 ++++++++++++++++++++-----
3 files changed, 38 insertions(+), 17 deletions(-)
--
2.48.1
11:41 a.m.
New subject: [PATCH 1/3] qemuSnapshotForEachQcow2: Don't initialize 'nrollback'
The variable holds the amount of disks to roll back the snapshot for.
The value must be set before the code jumps to the 'rollback:' label so
the best situation is to not initialize it and let the compiler catch
errors rather than initialize the unsigned variable to -1 and let it
crash.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 7ce018b026..e5226db942 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -290,7 +290,7 @@ qemuSnapshotForEachQcow2(virDomainDef *def,
size_t i;
bool skipped = false;
bool create = STREQ(op, "-c");
- size_t nrollback = -1;
+ size_t nrollback;
virErrorPtr orig_err;
/* pre-checks */
--
2.48.1
11:41 a.m.
New subject: [PATCH 2/3] qemu: process: Export qemuPrepareNVRAM for use in snapshot code
Export qemuPrepareNVRAM so that it doesn't require the VM object. The
snapshot code needs in the corner case of creating a snapshot of a
freshly defined VM ensure that the nvram image exists in order to
snapshot it.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_process.c | 26 ++++++++++++++------------
src/qemu/qemu_process.h | 4 ++++
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 34a755a49a..4948801ca1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4608,10 +4608,9 @@ qemuPrepareNVRAMHelper(int dstFD,
static int
-qemuPrepareNVRAMBlock(virDomainObj *vm,
+qemuPrepareNVRAMBlock(virDomainLoaderDef *loader,
bool reset_nvram)
{
- virDomainLoaderDef *loader = vm->def->os.loader;
g_autoptr(virCommand) qemuimg = NULL;
const char *templateFormatStr = "raw";
@@ -4672,13 +4671,12 @@ qemuPrepareNVRAMBlock(virDomainObj *vm,
static int
-qemuPrepareNVRAMFile(virDomainObj *vm,
+qemuPrepareNVRAMFile(virQEMUDriver *driver,
+ virDomainLoaderDef *loader,
bool reset_nvram)
{
- qemuDomainObjPrivate *priv = vm->privateData;
- g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
+ g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
VIR_AUTOCLOSE srcFD = -1;
- virDomainLoaderDef *loader = vm->def->os.loader;
struct qemuPrepareNVRAMHelperData data;
if (virFileExists(loader->nvram->path) && !reset_nvram)
@@ -4720,21 +4718,24 @@ qemuPrepareNVRAMFile(virDomainObj *vm,
}
-static int
-qemuPrepareNVRAM(virDomainObj *vm,
+int
+qemuPrepareNVRAM(virQEMUDriver *driver,
+ virDomainDef *def,
bool reset_nvram)
{
- virDomainLoaderDef *loader = vm->def->os.loader;
+ virDomainLoaderDef *loader = def->os.loader;
if (!loader || !loader->nvram)
return 0;
+ VIR_DEBUG("nvram='%s'", NULLSTR(loader->nvram->path));
+
switch (virStorageSourceGetActualType(loader->nvram)) {
case VIR_STORAGE_TYPE_FILE:
- return qemuPrepareNVRAMFile(vm, reset_nvram);
+ return qemuPrepareNVRAMFile(driver, loader, reset_nvram);
case VIR_STORAGE_TYPE_BLOCK:
- return qemuPrepareNVRAMBlock(vm, reset_nvram);
+ return qemuPrepareNVRAMBlock(loader, reset_nvram);
case VIR_STORAGE_TYPE_DIR:
case VIR_STORAGE_TYPE_NETWORK:
@@ -7399,7 +7400,8 @@ qemuProcessPrepareHost(virQEMUDriver *driver,
qemuProcessMakeDir(driver, vm, priv->channelTargetDir) < 0)
return -1;
- if (qemuPrepareNVRAM(vm, !!(flags & VIR_QEMU_PROCESS_START_RESET_NVRAM)) < 0)
+ if (qemuPrepareNVRAM(driver, vm->def,
+ !!(flags & VIR_QEMU_PROCESS_START_RESET_NVRAM)) < 0)
return -1;
if (vm->def->vsock) {
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 12781673c5..fee00ce53b 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -258,3 +258,7 @@ int qemuProcessSetupEmulator(virDomainObj *vm);
void qemuProcessHandleNbdkitExit(qemuNbdkitProcess *nbdkit,
virDomainObj *vm);
+
+int qemuPrepareNVRAM(virQEMUDriver *driver,
+ virDomainDef *def,
+ bool reset_nvram);
--
2.48.1
11:41 a.m.
New subject: [PATCH 3/3] qemu: snapshot: Ensure that NVRAM image exists when taking inactive internal snapshot
Attempting to take an internal snapshot of a freshly defined VM with
qcow2 backed NVRAM results in failure as the NVRAM image doesn't get
populated until the VM is started for the first time.
Fix this by invoking qemuPrepareNVRAM() when qcow2 nvram is defined.
Resolves: https://issues.redhat.com/browse/RHEL-73315
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index e5226db942..ed140dd41c 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -269,6 +269,7 @@ qemuSnapshotForEachQcow2One(virStorageSource *src,
/**
* qemuSnapshotForEachQcow2:
*
+ * @driver: qemu driver configuration
* @def: domain definition
* @snap: snapshot object
* @op: 'qemu-img snapshot' operation flag, one of "-c",
"-d", "-a"
@@ -282,7 +283,8 @@ qemuSnapshotForEachQcow2One(virStorageSource *src,
* permissive modes.
*/
static int
-qemuSnapshotForEachQcow2(virDomainDef *def,
+qemuSnapshotForEachQcow2(virQEMUDriver *driver,
+ virDomainDef *def,
virDomainMomentObj *snap,
const char *op)
{
@@ -352,6 +354,16 @@ qemuSnapshotForEachQcow2(virDomainDef *def,
if (virStorageSourceIsLocalStorage(nvram) &&
nvram->format == VIR_STORAGE_FILE_QCOW2) {
+ if (create) {
+ /* Ensure that the NVRAM image exists; e.g. when snapshotting
+ * a VM directly after defining it */
+ if (qemuPrepareNVRAM(driver, def, false) < 0) {
+ nrollback = def->ndisks;
+ virErrorPreserveLast(&orig_err);
+ goto rollback;
+ }
+ }
+
if (qemuSnapshotForEachQcow2One(nvram, op, snap->def->name) < 0) {
if (create) {
nrollback = def->ndisks;
@@ -392,7 +404,8 @@ static int
qemuSnapshotCreateInactiveInternal(virDomainObj *vm,
virDomainMomentObj *snap)
{
- return qemuSnapshotForEachQcow2(vm->def, snap, "-c");
+ return qemuSnapshotForEachQcow2(QEMU_DOMAIN_PRIVATE(vm)->driver,
+ vm->def, snap, "-c");
}
@@ -2697,7 +2710,8 @@ qemuSnapshotInternalRevertInactive(virDomainObj *vm,
}
/* Try all disks, but report failure if we skipped any. */
- if (qemuSnapshotForEachQcow2(def, snap, "-a") != 0)
+ if (qemuSnapshotForEachQcow2(QEMU_DOMAIN_PRIVATE(vm)->driver,
+ def, snap, "-a") != 0)
return -1;
return 0;
@@ -4064,7 +4078,8 @@ qemuSnapshotDiscardImpl(virDomainObj *vm,
if (qemuSnapshotDiscardExternal(vm, snap, externalData) < 0)
return -1;
} else {
- if (qemuSnapshotForEachQcow2(def, snap, "-d") < 0)
+ if (qemuSnapshotForEachQcow2(QEMU_DOMAIN_PRIVATE(vm)->driver,
+ def, snap, "-d") < 0)
return -1;
}
} else {
--
2.48.1
10:03 a.m.
On Mon, Feb 03, 2025 at 06:41:08PM +0100, Peter Krempa wrote:
Peter Krempa (3):
qemuSnapshotForEachQcow2: Don't initialize 'nrollback'
qemu: process: Export qemuPrepareNVRAM for use in snapshot code
qemu: snapshot: Ensure that NVRAM image exists when taking inactive
internal snapshot
Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com>
6
days inactive
8
days old
4 comments
2 participants
participants (2)
-
Pavel Hrdina -
Peter Krempa