6:40 a.m.
Cleanup patches extracted from my upcoming series for handling snapshots
with outsourced storage.
Peter Krempa (14):
virDomainDiskDefFormat: Refactor to virXMLFormatElement
qemuSnapshotDiskPrepareActiveExternal: Handle only external snapshots
qemuSnapshotCreateAlignDisks: Rewrite logic for selecting default
memory snapshot mode
virDomainSnapshotDiskDefParseXML: Automatically free temporary
variables and remove cleanup
virStorageSource: Convert 'type' to proper enum
conf: Move definition of 'virDomainSnapshotLocation'
Rename VIR_DOMAIN_SNAPSHOT_LOCATION_NONE to
VIR_DOMAIN_SNAPSHOT_LOCATION_NO
qemuDomainSnapshotForEachQcow2Raw: Act only on internal snapshots
conf: snapshot: Remove VIR_DOMAIN_SNAPSHOT_PARSE_DISKS flag
virDomainSnapshotDefParse: Refactor cleanup
virDomainSnapshotDefParse: Avoid 'memoryfile' temporary variable
virDomainSnapshotDefParse: Decouple parsing of memory snapshot config
conf: snapshot: Use proper types for snapshot location
qemuSnapshotCreateActiveExternal: Remove duplicit assignment
src/ch/ch_monitor.c | 1 +
src/conf/domain_conf.c | 122 ++++++------
src/conf/domain_conf.h | 14 +-
src/conf/domain_validate.c | 2 +-
src/conf/snapshot_conf.c | 255 ++++++++++++--------------
src/conf/snapshot_conf.h | 23 +--
src/conf/storage_source_conf.h | 2 +-
src/libvirt_private.syms | 4 +-
src/qemu/qemu_domain.c | 2 +-
src/qemu/qemu_driver.c | 1 -
src/qemu/qemu_snapshot.c | 23 ++-
src/test/test_driver.c | 7 +-
src/vbox/vbox_common.c | 4 +-
src/vz/vz_driver.c | 2 +-
src/vz/vz_sdk.c | 2 +-
tests/qemudomainsnapshotxml2xmltest.c | 2 +-
16 files changed, 221 insertions(+), 245 deletions(-)
--
2.35.1
6:40 a.m.
New subject: [PATCH 01/14] virDomainDiskDefFormat: Refactor to virXMLFormatElement
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 79 ++++++++++++++++++++----------------------
1 file changed, 38 insertions(+), 41 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 34fec887a3..69f61aadc1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23568,6 +23568,8 @@ virDomainDiskDefFormat(virBuffer *buf,
const char *device = virDomainDiskDeviceTypeToString(def->device);
const char *bus = virDomainDiskBusTypeToString(def->bus);
const char *sgio = virDomainDeviceSGIOTypeToString(def->sgio);
+ g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
+ g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
if (!type || !def->src->type) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -23590,39 +23592,35 @@ virDomainDiskDefFormat(virBuffer *buf,
return -1;
}
- virBufferAsprintf(buf,
- "<disk type='%s' device='%s'",
- type, device);
+ virBufferAsprintf(&attrBuf, " type='%s' device='%s'",
type, device);
if (def->model) {
- virBufferAsprintf(buf, " model='%s'",
+ virBufferAsprintf(&attrBuf, " model='%s'",
virDomainDiskModelTypeToString(def->model));
}
if (def->rawio) {
- virBufferAsprintf(buf, " rawio='%s'",
+ virBufferAsprintf(&attrBuf, " rawio='%s'",
virTristateBoolTypeToString(def->rawio));
}
if (def->sgio)
- virBufferAsprintf(buf, " sgio='%s'", sgio);
+ virBufferAsprintf(&attrBuf, " sgio='%s'", sgio);
if (def->snapshot &&
!(def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE &&
def->src->readonly))
- virBufferAsprintf(buf, " snapshot='%s'",
+ virBufferAsprintf(&attrBuf, " snapshot='%s'",
virDomainSnapshotLocationTypeToString(def->snapshot));
- virBufferAddLit(buf, ">\n");
- virBufferAdjustIndent(buf, 2);
- virDomainDiskDefFormatDriver(buf, def);
+ virDomainDiskDefFormatDriver(&childBuf, def);
/* Format as child of <disk> if defined there; otherwise,
* if defined as child of <source>, then format later */
if (def->src->auth && def->diskElementAuth)
- virStorageAuthDefFormat(buf, def->src->auth);
+ virStorageAuthDefFormat(&childBuf, def->src->auth);
- if (virDomainDiskSourceFormat(buf, def->src, "source",
def->startupPolicy,
+ if (virDomainDiskSourceFormat(&childBuf, def->src, "source",
def->startupPolicy,
true, flags,
def->diskElementAuth, def->diskElementEnc,
xmlopt) < 0)
@@ -23630,75 +23628,74 @@ virDomainDiskDefFormat(virBuffer *buf,
/* Don't format backingStore to inactive XMLs until the code for
* persistent storage of backing chains is ready. */
- if (virDomainDiskBackingStoreFormat(buf, def->src, xmlopt, flags) < 0)
+ if (virDomainDiskBackingStoreFormat(&childBuf, def->src, xmlopt, flags) <
0)
return -1;
- virBufferEscapeString(buf, "<backenddomain name='%s'/>\n",
def->domain_name);
+ virBufferEscapeString(&childBuf, "<backenddomain
name='%s'/>\n", def->domain_name);
- virDomainDiskGeometryDefFormat(buf, def);
- virDomainDiskBlockIoDefFormat(buf, def);
+ virDomainDiskGeometryDefFormat(&childBuf, def);
+ virDomainDiskBlockIoDefFormat(&childBuf, def);
- if (virDomainDiskDefFormatMirror(buf, def, flags, xmlopt) < 0)
+ if (virDomainDiskDefFormatMirror(&childBuf, def, flags, xmlopt) < 0)
return -1;
- virBufferAsprintf(buf, "<target dev='%s' bus='%s'",
+ virBufferAsprintf(&childBuf, "<target dev='%s'
bus='%s'",
def->dst, bus);
if ((def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
def->device == VIR_DOMAIN_DISK_DEVICE_CDROM) &&
def->tray_status != VIR_DOMAIN_DISK_TRAY_CLOSED)
- virBufferAsprintf(buf, " tray='%s'",
+ virBufferAsprintf(&childBuf, " tray='%s'",
virDomainDiskTrayTypeToString(def->tray_status));
if (def->bus == VIR_DOMAIN_DISK_BUS_USB &&
def->removable != VIR_TRISTATE_SWITCH_ABSENT) {
- virBufferAsprintf(buf, " removable='%s'",
+ virBufferAsprintf(&childBuf, " removable='%s'",
virTristateSwitchTypeToString(def->removable));
}
if (def->rotation_rate)
- virBufferAsprintf(buf, " rotation_rate='%u'",
def->rotation_rate);
- virBufferAddLit(buf, "/>\n");
+ virBufferAsprintf(&childBuf, " rotation_rate='%u'",
def->rotation_rate);
+ virBufferAddLit(&childBuf, "/>\n");
- virDomainDiskDefFormatIotune(buf, def);
+ virDomainDiskDefFormatIotune(&childBuf, def);
if (def->src->readonly)
- virBufferAddLit(buf, "<readonly/>\n");
+ virBufferAddLit(&childBuf, "<readonly/>\n");
if (def->src->shared)
- virBufferAddLit(buf, "<shareable/>\n");
+ virBufferAddLit(&childBuf, "<shareable/>\n");
if (def->transient) {
- virBufferAddLit(buf, "<transient");
+ virBufferAddLit(&childBuf, "<transient");
if (def->transientShareBacking == VIR_TRISTATE_BOOL_YES)
- virBufferAddLit(buf, " shareBacking='yes'");
- virBufferAddLit(buf, "/>\n");
+ virBufferAddLit(&childBuf, " shareBacking='yes'");
+ virBufferAddLit(&childBuf, "/>\n");
}
- virBufferEscapeString(buf, "<serial>%s</serial>\n",
def->serial);
- virBufferEscapeString(buf, "<wwn>%s</wwn>\n", def->wwn);
- virBufferEscapeString(buf, "<vendor>%s</vendor>\n",
def->vendor);
- virBufferEscapeString(buf, "<product>%s</product>\n",
def->product);
+ virBufferEscapeString(&childBuf, "<serial>%s</serial>\n",
def->serial);
+ virBufferEscapeString(&childBuf, "<wwn>%s</wwn>\n",
def->wwn);
+ virBufferEscapeString(&childBuf, "<vendor>%s</vendor>\n",
def->vendor);
+ virBufferEscapeString(&childBuf, "<product>%s</product>\n",
def->product);
/* If originally found as a child of <disk>, then format thusly;
* otherwise, will be formatted as child of <source> */
if (def->src->encryption && def->diskElementEnc &&
- virStorageEncryptionFormat(buf, def->src->encryption) < 0)
+ virStorageEncryptionFormat(&childBuf, def->src->encryption) < 0)
return -1;
- virDomainDeviceInfoFormat(buf, &def->info, flags |
VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT);
+ virDomainDeviceInfoFormat(&childBuf, &def->info, flags |
VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT);
- if (virDomainDiskDefFormatPrivateData(buf, def, flags, xmlopt) < 0)
+ if (virDomainDiskDefFormatPrivateData(&childBuf, def, flags, xmlopt) < 0)
return -1;
/* format diskElementAuth and diskElementEnc into status XML to preserve
* formatting */
if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) {
- g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
+ g_auto(virBuffer) secretPlacementAttrBuf = VIR_BUFFER_INITIALIZER;
if (def->diskElementAuth)
- virBufferAddLit(&attrBuf, " auth='true'");
+ virBufferAddLit(&secretPlacementAttrBuf, "
auth='true'");
if (def->diskElementEnc)
- virBufferAddLit(&attrBuf, " enc='true'");
+ virBufferAddLit(&secretPlacementAttrBuf, "
enc='true'");
- virXMLFormatElement(buf, "diskSecretsPlacement", &attrBuf, NULL);
+ virXMLFormatElement(&childBuf, "diskSecretsPlacement",
&secretPlacementAttrBuf, NULL);
}
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "</disk>\n");
+ virXMLFormatElement(buf, "disk", &attrBuf, &childBuf);
return 0;
}
--
2.35.1
6:40 a.m.
New subject: [PATCH 02/14] qemuSnapshotDiskPrepareActiveExternal: Handle only external snapshots
Preparation steps ensure that the 'snapshot' field can only be
'VIR_DOMAIN_SNAPSHOT_LOCATION_NONE' or
VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL' at this point, but upcoming
patches will change that. Handle only external snapshots.
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 cb2a7eb739..f92b571b06 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1165,7 +1165,7 @@ qemuSnapshotDiskPrepareActiveExternal(virDomainObj *vm,
snapctxt = qemuSnapshotDiskContextNew(snapdef->ndisks, vm, asyncJob);
for (i = 0; i < snapdef->ndisks; i++) {
- if (snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)
+ if (snapdef->disks[i].snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
continue;
if (qemuSnapshotDiskPrepareOne(snapctxt,
--
2.35.1
6:40 a.m.
New subject: [PATCH 03/14] qemuSnapshotCreateAlignDisks: Rewrite logic for selecting default memory snapshot mode
Use an if/else branch rather than a expression with a ternary operator.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index f92b571b06..355a2fd782 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1672,9 +1672,10 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
return -1;
}
- def->memory = (def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF ?
- VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
- VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL);
+ if (def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF)
+ def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
+ else
+ def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
}
if (virDomainSnapshotAlignDisks(def, NULL, align_location, true) < 0)
return -1;
--
2.35.1
6:40 a.m.
New subject: [PATCH 04/14] virDomainSnapshotDiskDefParseXML: Automatically free temporary variables and remove cleanup
Refactor the function to avoid the cleanup section used to just free
memory associated with the parsed object.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/snapshot_conf.c | 58 ++++++++++++++++++----------------------
1 file changed, 26 insertions(+), 32 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 79a7b891a2..a2e45632bf 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -138,21 +138,20 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
unsigned int flags,
virDomainXMLOption *xmlopt)
{
- int ret = -1;
- char *snapshot = NULL;
- char *type = NULL;
- char *driver = NULL;
+ g_autofree char *snapshot = NULL;
+ g_autofree char *type = NULL;
+ g_autofree char *driver = NULL;
+ g_autofree char *name = NULL;
+ g_autoptr(virStorageSource) src = virStorageSourceNew();
xmlNodePtr cur;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
ctxt->node = node;
- def->src = virStorageSourceNew();
- def->name = virXMLPropString(node, "name");
- if (!def->name) {
+ if (!(name = virXMLPropString(node, "name"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing name from disk snapshot element"));
- goto cleanup;
+ return -1;
}
snapshot = virXMLPropString(node, "snapshot");
@@ -162,59 +161,54 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown disk snapshot setting '%s'"),
snapshot);
- goto cleanup;
+ return -1;
}
}
if ((type = virXMLPropString(node, "type"))) {
- if ((def->src->type = virStorageTypeFromString(type)) <= 0 ||
- def->src->type == VIR_STORAGE_TYPE_VOLUME ||
- def->src->type == VIR_STORAGE_TYPE_DIR) {
+ if ((src->type = virStorageTypeFromString(type)) <= 0 ||
+ src->type == VIR_STORAGE_TYPE_VOLUME ||
+ src->type == VIR_STORAGE_TYPE_DIR) {
virReportError(VIR_ERR_XML_ERROR,
_("unknown disk snapshot type '%s'"),
type);
- goto cleanup;
+ return -1;
}
} else {
- def->src->type = VIR_STORAGE_TYPE_FILE;
+ src->type = VIR_STORAGE_TYPE_FILE;
}
if ((cur = virXPathNode("./source", ctxt)) &&
- virDomainStorageSourceParse(cur, ctxt, def->src, flags, xmlopt) < 0)
- goto cleanup;
+ virDomainStorageSourceParse(cur, ctxt, src, flags, xmlopt) < 0)
+ return -1;
if ((driver = virXPathString("string(./driver/@type)", ctxt)) &&
- (def->src->format = virStorageFileFormatTypeFromString(driver)) <= 0) {
+ (src->format = virStorageFileFormatTypeFromString(driver)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown disk snapshot driver '%s'"),
driver);
- goto cleanup;
+ return -1;
}
if (virParseScaledValue("./driver/metadata_cache/max_size", NULL,
ctxt,
- &def->src->metadataCacheMaxSize,
+ &src->metadataCacheMaxSize,
1, ULLONG_MAX, false) < 0)
- goto cleanup;
+ return -1;
/* validate that the passed path is absolute */
- if (virStorageSourceIsRelative(def->src)) {
+ if (virStorageSourceIsRelative(src)) {
virReportError(VIR_ERR_XML_ERROR,
_("disk snapshot image path '%s' must be
absolute"),
- def->src->path);
- goto cleanup;
+ src->path);
+ return -1;
}
- if (!def->snapshot && (def->src->path || def->src->format))
+ if (!def->snapshot && (src->path || src->format))
def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
- ret = 0;
- cleanup:
+ def->name = g_steal_pointer(&name);
+ def->src = g_steal_pointer(&src);
- VIR_FREE(driver);
- VIR_FREE(snapshot);
- VIR_FREE(type);
- if (ret < 0)
- virDomainSnapshotDiskDefClear(def);
- return ret;
+ return 0;
}
/* flags is bitwise-or of virDomainSnapshotParseFlags.
--
2.35.1
6:40 a.m.
New subject: [PATCH 05/14] virStorageSource: Convert 'type' to proper enum
Use 'virStorageType' as type for the 'type' member and convert the code
to work properly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/ch/ch_monitor.c | 1 +
src/conf/domain_conf.c | 27 ++++++++++++++-------------
src/conf/snapshot_conf.c | 24 +++++++++++++-----------
src/conf/storage_source_conf.h | 2 +-
4 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 60905e36c2..84085b7991 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -198,6 +198,7 @@ virCHMonitorBuildDiskJson(virJSONValue *disks, virDomainDiskDef
*diskdef)
case VIR_STORAGE_TYPE_VOLUME:
case VIR_STORAGE_TYPE_NVME:
case VIR_STORAGE_TYPE_VHOST_USER:
+ case VIR_STORAGE_TYPE_LAST:
default:
virReportEnumRangeError(virStorageType, diskdef->src->type);
return -1;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 69f61aadc1..40ff71d7db 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8502,11 +8502,15 @@ virDomainStorageSourceParseBase(const char *type,
src = virStorageSourceNew();
src->type = VIR_STORAGE_TYPE_FILE;
- if (type &&
- (src->type = virStorageTypeFromString(type)) <= 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("unknown storage source type '%s'"), type);
- return NULL;
+ if (type) {
+ int tmp;
+ if ((tmp = virStorageTypeFromString(type)) <= 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unknown storage source type '%s'"),
type);
+ return NULL;
+ }
+
+ src->type = tmp;
}
if (format &&
@@ -9055,19 +9059,16 @@ virDomainDiskDefParseSourceXML(virDomainXMLOption *xmlopt,
{
g_autoptr(virStorageSource) src = virStorageSourceNew();
VIR_XPATH_NODE_AUTORESTORE(ctxt)
- g_autofree char *type = NULL;
xmlNodePtr tmp;
ctxt->node = node;
- src->type = VIR_STORAGE_TYPE_FILE;
-
- if ((type = virXMLPropString(node, "type")) &&
- (src->type = virStorageTypeFromString(type)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown disk type '%s'"), type);
+ if (virXMLPropEnumDefault(node, "type",
+ virStorageTypeFromString,
+ VIR_XML_PROP_NONZERO,
+ &src->type,
+ VIR_STORAGE_TYPE_FILE) < 0)
return NULL;
- }
if ((tmp = virXPathNode("./source[1]", ctxt))) {
if (virDomainStorageSourceParse(tmp, ctxt, src, flags, xmlopt) < 0)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index a2e45632bf..812cca2ed1 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -139,7 +139,6 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
virDomainXMLOption *xmlopt)
{
g_autofree char *snapshot = NULL;
- g_autofree char *type = NULL;
g_autofree char *driver = NULL;
g_autofree char *name = NULL;
g_autoptr(virStorageSource) src = virStorageSourceNew();
@@ -165,16 +164,19 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
}
}
- if ((type = virXMLPropString(node, "type"))) {
- if ((src->type = virStorageTypeFromString(type)) <= 0 ||
- src->type == VIR_STORAGE_TYPE_VOLUME ||
- src->type == VIR_STORAGE_TYPE_DIR) {
- virReportError(VIR_ERR_XML_ERROR,
- _("unknown disk snapshot type '%s'"),
type);
- return -1;
- }
- } else {
- src->type = VIR_STORAGE_TYPE_FILE;
+ if (virXMLPropEnumDefault(node, "type",
+ virStorageTypeFromString,
+ VIR_XML_PROP_NONZERO,
+ &src->type,
+ VIR_STORAGE_TYPE_FILE) < 0)
+ return -1;
+
+ if (src->type == VIR_STORAGE_TYPE_VOLUME ||
+ src->type == VIR_STORAGE_TYPE_DIR) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unsupported disk snapshot type '%s'"),
+ virStorageTypeToString(src->type));
+ return -1;
}
if ((cur = virXPathNode("./source", ctxt)) &&
diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
index c720d093be..e984421e3d 100644
--- a/src/conf/storage_source_conf.h
+++ b/src/conf/storage_source_conf.h
@@ -269,7 +269,7 @@ struct _virStorageSource {
virObject parent;
unsigned int id; /* backing chain identifier, 0 is unset */
- int type; /* virStorageType */
+ virStorageType type;
char *path;
int protocol; /* virStorageNetProtocol */
char *volume; /* volume name for remote storage */
--
2.35.1
6:40 a.m.
New subject: [PATCH 06/14] conf: Move definition of 'virDomainSnapshotLocation'
The snapshot location enum is also needed for the disk definition so if
we house it inside domain_conf we can use the proper type for it.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 10 +++++++++-
src/conf/domain_conf.h | 14 +++++++++++++-
src/conf/snapshot_conf.c | 8 --------
src/conf/snapshot_conf.h | 12 ------------
src/libvirt_private.syms | 4 ++--
5 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 40ff71d7db..721623129c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1402,6 +1402,14 @@ VIR_ENUM_IMPL(virDomainIBS,
"fixed-na",
);
+VIR_ENUM_IMPL(virDomainSnapshotLocation,
+ VIR_DOMAIN_SNAPSHOT_LOCATION_LAST,
+ "default",
+ "no",
+ "internal",
+ "external",
+);
+
/* Internal mapping: subset of block job types that can be present in
* <mirror> XML (remaining types are not two-phase). */
VIR_ENUM_DECL(virDomainBlockJob);
@@ -23608,7 +23616,7 @@ virDomainDiskDefFormat(virBuffer *buf,
if (def->sgio)
virBufferAsprintf(&attrBuf, " sgio='%s'", sgio);
- if (def->snapshot &&
+ if (def->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
!(def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE &&
def->src->readonly))
virBufferAsprintf(&attrBuf, " snapshot='%s'",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9fcf842ee7..5f6b508e89 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -537,6 +537,18 @@ typedef enum {
} virDomainMemoryAllocation;
+typedef enum {
+ VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT = 0,
+ VIR_DOMAIN_SNAPSHOT_LOCATION_NONE,
+ VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL,
+ VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL,
+
+ VIR_DOMAIN_SNAPSHOT_LOCATION_LAST
+} virDomainSnapshotLocation;
+
+VIR_ENUM_DECL(virDomainSnapshotLocation);
+
+
/* Stores the virtual disk configuration */
struct _virDomainDiskDef {
virStorageSource *src; /* non-NULL. XXX Allow NULL for empty cdrom? */
@@ -581,7 +593,7 @@ struct _virDomainDiskDef {
virTristateSwitch ioeventfd;
virTristateSwitch event_idx;
virTristateSwitch copy_on_read;
- unsigned int snapshot; /* virDomainSnapshotLocation, snapshot_conf.h */
+ virDomainSnapshotLocation snapshot;
virDomainStartupPolicy startupPolicy;
bool transient;
virTristateBool transientShareBacking;
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 812cca2ed1..a4b3cd8c2b 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -67,14 +67,6 @@ virDomainSnapshotOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virDomainSnapshot);
-VIR_ENUM_IMPL(virDomainSnapshotLocation,
- VIR_DOMAIN_SNAPSHOT_LOCATION_LAST,
- "default",
- "no",
- "internal",
- "external",
-);
-
/* virDomainSnapshotState is really virDomainState plus one extra state */
VIR_ENUM_IMPL(virDomainSnapshotState,
VIR_DOMAIN_SNAPSHOT_LAST,
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index c8997c710c..b7e0d441ff 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -26,17 +26,6 @@
#include "moment_conf.h"
#include "virenum.h"
-/* Items related to snapshot state */
-
-typedef enum {
- VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT = 0,
- VIR_DOMAIN_SNAPSHOT_LOCATION_NONE,
- VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL,
- VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL,
-
- VIR_DOMAIN_SNAPSHOT_LOCATION_LAST
-} virDomainSnapshotLocation;
-
/**
* This enum has to map all known domain states from the public enum
* virDomainState, before adding one additional state possible only
@@ -139,5 +128,4 @@ int virDomainSnapshotRedefinePrep(virDomainObj *vm,
virDomainXMLOption *xmlopt,
unsigned int flags);
-VIR_ENUM_DECL(virDomainSnapshotLocation);
VIR_ENUM_DECL(virDomainSnapshotState);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6f0d72ca38..01503083ef 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -637,6 +637,8 @@ virDomainSmartcardTypeFromString;
virDomainSmartcardTypeToString;
virDomainSmbiosModeTypeFromString;
virDomainSmbiosModeTypeToString;
+virDomainSnapshotLocationTypeFromString;
+virDomainSnapshotLocationTypeToString;
virDomainSoundDefFind;
virDomainSoundDefFree;
virDomainSoundDefRemove;
@@ -1024,8 +1026,6 @@ virDomainSnapshotDiskDefFree;
virDomainSnapshotDiskDefParseXML;
virDomainSnapshotFormatConvertXMLFlags;
virDomainSnapshotIsExternal;
-virDomainSnapshotLocationTypeFromString;
-virDomainSnapshotLocationTypeToString;
virDomainSnapshotRedefinePrep;
virDomainSnapshotStateTypeFromString;
virDomainSnapshotStateTypeToString;
--
2.35.1
6:40 a.m.
New subject: [PATCH 07/14] Rename VIR_DOMAIN_SNAPSHOT_LOCATION_NONE to VIR_DOMAIN_SNAPSHOT_LOCATION_NO
The string value associated to the enum is "no". Rename the enum
accordingly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 6 +++---
src/conf/domain_conf.h | 2 +-
src/conf/domain_validate.c | 2 +-
src/conf/snapshot_conf.c | 12 ++++++------
src/qemu/qemu_domain.c | 2 +-
src/qemu/qemu_snapshot.c | 12 ++++++------
src/test/test_driver.c | 4 ++--
src/vz/vz_sdk.c | 2 +-
8 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 721623129c..58fe24a8c1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5521,7 +5521,7 @@ virDomainDiskDefPostParse(virDomainDiskDef *disk,
if (disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
disk->src->readonly)
- disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
+ disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NO;
if (disk->src->type == VIR_STORAGE_TYPE_NETWORK &&
disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI) {
@@ -5536,7 +5536,7 @@ virDomainDiskDefPostParse(virDomainDiskDef *disk,
/* vhost-user doesn't allow us to snapshot, disable snapshots by default */
if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER &&
disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) {
- disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
+ disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NO;
}
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
@@ -23617,7 +23617,7 @@ virDomainDiskDefFormat(virBuffer *buf,
virBufferAsprintf(&attrBuf, " sgio='%s'", sgio);
if (def->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
- !(def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE &&
+ !(def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NO &&
def->src->readonly))
virBufferAsprintf(&attrBuf, " snapshot='%s'",
virDomainSnapshotLocationTypeToString(def->snapshot));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5f6b508e89..a4de46773c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -539,7 +539,7 @@ typedef enum {
typedef enum {
VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT = 0,
- VIR_DOMAIN_SNAPSHOT_LOCATION_NONE,
+ VIR_DOMAIN_SNAPSHOT_LOCATION_NO,
VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL,
VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL,
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index f0b8aa2655..d6869e8fd8 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -317,7 +317,7 @@ virDomainDiskVhostUserValidate(const virDomainDiskDef *disk)
return -1;
}
- if (disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) {
+ if (disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_NO) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("only snapshot=no is supported with vhostuser
disk"));
return -1;
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index a4b3cd8c2b..f5ae2ee212 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -337,11 +337,11 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
} else if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
def->memory = (offline ?
- VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
+ VIR_DOMAIN_SNAPSHOT_LOCATION_NO :
VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL);
}
if (offline && def->memory &&
- def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) {
+ def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_NO) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("memory state cannot be saved with offline or "
"disk-only snapshot"));
@@ -681,15 +681,15 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) {
if (domdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
(!require_match ||
- domdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)) {
+ domdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NO)) {
snapdisk->snapshot = domdisk->snapshot;
} else {
snapdisk->snapshot = default_snapshot;
}
} else if (require_match &&
snapdisk->snapshot != default_snapshot &&
- !(snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE
&&
- domdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)) {
+ !(snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NO &&
+ domdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NO)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk '%s' must use snapshot mode
'%s'"),
snapdisk->name,
@@ -728,7 +728,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
/* Don't snapshot empty drives */
if (virStorageSourceIsEmpty(domdef->disks[i]->src))
- snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
+ snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NO;
else
snapdisk->snapshot = domdef->disks[i]->snapshot;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6b61fefb8f..936a0c8a35 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6886,7 +6886,7 @@ qemuDomainSnapshotForEachQcow2Raw(virQEMUDriver *driver,
/* FIXME: we also need to handle LVM here */
if (def->disks[i]->device != VIR_DOMAIN_DISK_DEVICE_DISK ||
- snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)
+ snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NO)
continue;
if (!virStorageSourceIsLocalStorage(def->disks[i]->src)) {
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 355a2fd782..c5fc7dcf67 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -692,7 +692,7 @@ qemuSnapshotPrepare(virDomainObj *vm,
virDomainSnapshotDiskDef *disk = &def->disks[i];
virDomainDiskDef *dom_disk = vm->def->disks[i];
- if (disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE &&
+ if (disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_NO &&
qemuDomainDiskBlockJobIsActive(dom_disk))
return -1;
@@ -757,7 +757,7 @@ qemuSnapshotPrepare(virDomainObj *vm,
external++;
break;
- case VIR_DOMAIN_SNAPSHOT_LOCATION_NONE:
+ case VIR_DOMAIN_SNAPSHOT_LOCATION_NO:
/* Remember seeing a disk that has snapshot disabled */
if (!virStorageSourceIsEmpty(dom_disk->src) &&
!dom_disk->src->readonly)
@@ -773,7 +773,7 @@ qemuSnapshotPrepare(virDomainObj *vm,
}
if (!found_internal && !external &&
- def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) {
+ def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_NO) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("nothing selected for snapshot"));
return -1;
@@ -1657,7 +1657,7 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
def->state = VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT;
else
def->state = VIR_DOMAIN_SNAPSHOT_SHUTOFF;
- def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
+ def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NO;
} else if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
def->state = virDomainObjGetState(vm, NULL);
align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
@@ -1665,7 +1665,7 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
def->state = virDomainObjGetState(vm, NULL);
if (virDomainObjIsActive(vm) &&
- def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) {
+ def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_NO) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("internal snapshot of a running VM "
"must include the memory state"));
@@ -1673,7 +1673,7 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
}
if (def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF)
- def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
+ def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NO;
else
def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 4eca5c4a65..da1c6c8f6e 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -8723,14 +8723,14 @@ testDomainSnapshotAlignDisks(virDomainObj *vm,
def->state = VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT;
else
def->state = VIR_DOMAIN_SNAPSHOT_SHUTOFF;
- def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
+ def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NO;
} else if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
def->state = virDomainObjGetState(vm, NULL);
align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
} else {
def->state = virDomainObjGetState(vm, NULL);
def->memory = def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF ?
- VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
+ VIR_DOMAIN_SNAPSHOT_LOCATION_NO :
VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
}
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f149360c03..43d528820e 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4640,7 +4640,7 @@ prlsdkParseSnapshotTree(const char *treexml)
def->parent.description = virXPathString("string(./Description)",
ctxt);
- def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
+ def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NO;
xmlstr = virXPathString("string(./@state)", ctxt);
if (!xmlstr) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
--
2.35.1
6:40 a.m.
New subject: [PATCH 08/14] qemuDomainSnapshotForEachQcow2Raw: Act only on internal snapshots
Similarly to the external snapshot code the internal inactive snapshot
creation helper should act only when an internal snapshot of the disk is
required. For now the callers ensure that it's either _INTERNAL or _NO
when control reaches this function.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 936a0c8a35..f61509d00b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6886,7 +6886,7 @@ qemuDomainSnapshotForEachQcow2Raw(virQEMUDriver *driver,
/* FIXME: we also need to handle LVM here */
if (def->disks[i]->device != VIR_DOMAIN_DISK_DEVICE_DISK ||
- snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NO)
+ snapdef->disks[i].snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL)
continue;
if (!virStorageSourceIsLocalStorage(def->disks[i]->src)) {
--
2.35.1
6:40 a.m.
New subject: [PATCH 09/14] conf: snapshot: Remove VIR_DOMAIN_SNAPSHOT_PARSE_DISKS flag
All callers except the one in the 'esx' driver pass the flag. The 'esx'
driver has a check that 'def->ndisks' is zero after parsing the
definition. This means that we can simply always parse the disks.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/snapshot_conf.c | 22 ++++++++--------------
src/conf/snapshot_conf.h | 7 +++----
src/qemu/qemu_driver.c | 1 -
src/qemu/qemu_snapshot.c | 2 +-
src/test/test_driver.c | 3 +--
src/vbox/vbox_common.c | 4 +---
src/vz/vz_driver.c | 2 +-
tests/qemudomainsnapshotxml2xmltest.c | 2 +-
8 files changed, 16 insertions(+), 27 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f5ae2ee212..f477b67785 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -359,21 +359,15 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
if ((n = virXPathNodeSet("./disks/*", ctxt, &nodes)) < 0)
goto cleanup;
- if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_DISKS) {
- if (n)
- def->disks = g_new0(virDomainSnapshotDiskDef, n);
- def->ndisks = n;
- for (i = 0; i < def->ndisks; i++) {
- if (virDomainSnapshotDiskDefParseXML(nodes[i], ctxt, &def->disks[i],
- flags, xmlopt) < 0)
- goto cleanup;
- }
- VIR_FREE(nodes);
- } else if (n) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("unable to handle disk requests in snapshot"));
- goto cleanup;
+ if (n)
+ def->disks = g_new0(virDomainSnapshotDiskDef, n);
+ def->ndisks = n;
+ for (i = 0; i < def->ndisks; i++) {
+ if (virDomainSnapshotDiskDefParseXML(nodes[i], ctxt, &def->disks[i],
+ flags, xmlopt) < 0)
+ goto cleanup;
}
+ VIR_FREE(nodes);
if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL) {
if (!current) {
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index b7e0d441ff..8823af1ac1 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -84,10 +84,9 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSnapshotDef, virObjectUnref);
typedef enum {
VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE = 1 << 0,
- VIR_DOMAIN_SNAPSHOT_PARSE_DISKS = 1 << 1,
- VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL = 1 << 2,
- VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE = 1 << 3,
- VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE = 1 << 4,
+ VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL = 1 << 1,
+ VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE = 1 << 2,
+ VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE = 1 << 3,
} virDomainSnapshotParseFlags;
typedef enum {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c1b3bd8536..de7d2b93b9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -347,7 +347,6 @@ qemuDomainSnapshotLoad(virDomainObj *vm,
virDomainMomentObj *current = NULL;
bool cur;
unsigned int flags = (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE |
- VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL);
int ret = -1;
int direrr;
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index c5fc7dcf67..3fb1d3abe0 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1530,7 +1530,7 @@ qemuSnapshotCreateXMLParse(virDomainObj *vm,
unsigned int flags)
{
qemuDomainObjPrivate *priv = vm->privateData;
- unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
+ unsigned int parse_flags = 0;
if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)
parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index da1c6c8f6e..9ceb0b45c8 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -978,7 +978,6 @@ testParseDomainSnapshots(testDriver *privconn,
privconn->xmlopt,
NULL,
&cur,
- VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL |
VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
if (!def)
@@ -8777,7 +8776,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
virObjectEvent *event = NULL;
bool update_current = true;
bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
- unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
+ unsigned int parse_flags = 0;
g_autoptr(virDomainSnapshotDef) def = NULL;
/*
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 36db6e06be..acd18494d3 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -5364,8 +5364,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
nsresult rc;
resultCodeUnion result;
virDomainSnapshotPtr ret = NULL;
- unsigned int parse_flags = (VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
- VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
+ unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE;
g_autoptr(virDomainSnapshotDef) def = NULL;
if (!data->vboxObj)
@@ -6783,7 +6782,6 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
}
def = virDomainSnapshotDefParseString(defXml,
data->xmlopt, NULL, NULL,
- VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
if (!def) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index fdd776282e..2107785ab2 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -2580,7 +2580,7 @@ vzDomainSnapshotCreateXML(virDomainPtr domain,
virDomainObj *dom;
struct _vzConn *privconn = domain->conn->privateData;
struct _vzDriver *driver = privconn->driver;
- unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
+ unsigned int parse_flags = 0;
virDomainSnapshotObjList *snapshots = NULL;
virDomainMomentObj *current;
bool job = false;
diff --git a/tests/qemudomainsnapshotxml2xmltest.c
b/tests/qemudomainsnapshotxml2xmltest.c
index 735d0e21a1..3fa932e755 100644
--- a/tests/qemudomainsnapshotxml2xmltest.c
+++ b/tests/qemudomainsnapshotxml2xmltest.c
@@ -34,7 +34,7 @@ testCompareXMLToXMLFiles(const char *inxml,
g_autofree char *inXmlData = NULL;
g_autofree char *outXmlData = NULL;
g_autofree char *actual = NULL;
- unsigned int parseflags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
+ unsigned int parseflags = 0;
unsigned int formatflags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE;
bool cur = false;
g_autoptr(virDomainSnapshotDef) def = NULL;
--
2.35.1
6:40 a.m.
New subject: [PATCH 10/14] virDomainSnapshotDefParse: Refactor cleanup
Use automatic memory cleanup, decrease scope of variables and remove the
'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/snapshot_conf.c | 74 +++++++++++++++++-----------------------
1 file changed, 32 insertions(+), 42 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f477b67785..f3b264d2e1 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -216,17 +216,12 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
bool *current,
unsigned int flags)
{
- virDomainSnapshotDef *def = NULL;
- virDomainSnapshotDef *ret = NULL;
- xmlNodePtr *nodes = NULL;
- xmlNodePtr inactiveDomNode = NULL;
+ g_autoptr(virDomainSnapshotDef) def = NULL;
+ g_autofree xmlNodePtr *diskNodes = NULL;
size_t i;
int n;
- char *state = NULL;
- int active;
- char *tmp;
- char *memorySnapshot = NULL;
- char *memoryFile = NULL;
+ g_autofree char *memorySnapshot = NULL;
+ g_autofree char *memoryFile = NULL;
bool offline = !!(flags & VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE);
virSaveCookieCallbacks *saveCookie = virDomainXMLOptionGetSaveCookie(xmlopt);
int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
@@ -240,18 +235,22 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("a redefined snapshot must have a name"));
- goto cleanup;
+ return NULL;
}
}
def->parent.description = virXPathString("string(./description)",
ctxt);
if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
+ g_autofree char *state = NULL;
+ g_autofree char *domtype = NULL;
+ xmlNodePtr inactiveDomNode = NULL;
+
if (virXPathLongLong("string(./creationTime)", ctxt,
&def->parent.creationTime) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing creationTime from existing snapshot"));
- goto cleanup;
+ return NULL;
}
def->parent.parent_name = virXPathString("string(./parent/name)",
ctxt);
@@ -263,14 +262,14 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
*/
virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing state from existing snapshot"));
- goto cleanup;
+ return NULL;
}
def->state = virDomainSnapshotStateTypeFromString(state);
if (def->state <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Invalid state '%s' in domain snapshot
XML"),
state);
- goto cleanup;
+ return NULL;
}
offline = (def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF ||
def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT);
@@ -279,20 +278,19 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
* lack domain/@type. In that case, leave dom NULL, and
* clients will have to decide between best effort
* initialization or outright failure. */
- if ((tmp = virXPathString("string(./domain/@type)", ctxt))) {
+ if ((domtype = virXPathString("string(./domain/@type)", ctxt))) {
xmlNodePtr domainNode = virXPathNode("./domain", ctxt);
- VIR_FREE(tmp);
if (!domainNode) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing domain in snapshot"));
- goto cleanup;
+ return NULL;
}
def->parent.dom = virDomainDefParseNode(ctxt->node->doc,
domainNode,
xmlopt, parseOpaque,
domainflags);
if (!def->parent.dom)
- goto cleanup;
+ return NULL;
} else {
VIR_WARN("parsing older snapshot that lacks domain");
}
@@ -304,10 +302,10 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
def->parent.inactiveDom = virDomainDefParseNode(ctxt->node->doc,
inactiveDomNode,
xmlopt, NULL, domainflags);
if (!def->parent.inactiveDom)
- goto cleanup;
+ return NULL;
}
} else if (virDomainXMLOptionRunMomentPostParse(xmlopt, &def->parent) < 0)
{
- goto cleanup;
+ return NULL;
}
memorySnapshot = virXPathString("string(./memory/@snapshot)", ctxt);
@@ -318,20 +316,20 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown memory snapshot setting '%s'"),
memorySnapshot);
- goto cleanup;
+ return NULL;
}
if (memoryFile &&
def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
virReportError(VIR_ERR_XML_ERROR,
_("memory filename '%s' requires external
snapshot"),
memoryFile);
- goto cleanup;
+ return NULL;
}
if (!memoryFile &&
def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("external memory snapshots require a filename"));
- goto cleanup;
+ return NULL;
}
} else if (memoryFile) {
def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
@@ -345,7 +343,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
virReportError(VIR_ERR_XML_ERROR, "%s",
_("memory state cannot be saved with offline or "
"disk-only snapshot"));
- goto cleanup;
+ return NULL;
}
def->memorysnapshotfile = g_steal_pointer(&memoryFile);
@@ -354,48 +352,40 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
virReportError(VIR_ERR_XML_ERROR,
_("memory snapshot file path (%s) must be absolute"),
def->memorysnapshotfile);
- goto cleanup;
+ return NULL;
}
- if ((n = virXPathNodeSet("./disks/*", ctxt, &nodes)) < 0)
- goto cleanup;
+ if ((n = virXPathNodeSet("./disks/*", ctxt, &diskNodes)) < 0)
+ return NULL;
if (n)
def->disks = g_new0(virDomainSnapshotDiskDef, n);
def->ndisks = n;
for (i = 0; i < def->ndisks; i++) {
- if (virDomainSnapshotDiskDefParseXML(nodes[i], ctxt, &def->disks[i],
+ if (virDomainSnapshotDiskDefParseXML(diskNodes[i], ctxt, &def->disks[i],
flags, xmlopt) < 0)
- goto cleanup;
+ return NULL;
}
- VIR_FREE(nodes);
if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL) {
+ int active;
+
if (!current) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("internal parse requested with NULL current"));
- goto cleanup;
+ return NULL;
}
if (virXPathInt("string(./active)", ctxt, &active) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Could not find 'active' element"));
- goto cleanup;
+ return NULL;
}
*current = active != 0;
}
if (!offline && virSaveCookieParse(ctxt, &def->cookie, saveCookie)
< 0)
- goto cleanup;
-
- ret = g_steal_pointer(&def);
-
- cleanup:
- VIR_FREE(state);
- VIR_FREE(nodes);
- VIR_FREE(memorySnapshot);
- VIR_FREE(memoryFile);
- virObjectUnref(def);
+ return NULL;
- return ret;
+ return g_steal_pointer(&def);
}
virDomainSnapshotDef *
--
2.35.1
6:40 a.m.
New subject: [PATCH 11/14] virDomainSnapshotDefParse: Avoid 'memoryfile' temporary variable
Assign directly into the definition. The cleanup code can deal with
that.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/snapshot_conf.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f3b264d2e1..7f98ae6aec 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -221,7 +221,6 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
size_t i;
int n;
g_autofree char *memorySnapshot = NULL;
- g_autofree char *memoryFile = NULL;
bool offline = !!(flags & VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE);
virSaveCookieCallbacks *saveCookie = virDomainXMLOptionGetSaveCookie(xmlopt);
int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
@@ -309,7 +308,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
}
memorySnapshot = virXPathString("string(./memory/@snapshot)", ctxt);
- memoryFile = virXPathString("string(./memory/@file)", ctxt);
+ def->memorysnapshotfile = virXPathString("string(./memory/@file)",
ctxt);
if (memorySnapshot) {
def->memory = virDomainSnapshotLocationTypeFromString(memorySnapshot);
if (def->memory <= 0) {
@@ -318,20 +317,20 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
memorySnapshot);
return NULL;
}
- if (memoryFile &&
+ if (def->memorysnapshotfile &&
def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
virReportError(VIR_ERR_XML_ERROR,
_("memory filename '%s' requires external
snapshot"),
- memoryFile);
+ def->memorysnapshotfile);
return NULL;
}
- if (!memoryFile &&
+ if (!def->memorysnapshotfile &&
def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("external memory snapshots require a filename"));
return NULL;
}
- } else if (memoryFile) {
+ } else if (def->memorysnapshotfile) {
def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
} else if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
def->memory = (offline ?
@@ -345,7 +344,6 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
"disk-only snapshot"));
return NULL;
}
- def->memorysnapshotfile = g_steal_pointer(&memoryFile);
/* verify that memory path is absolute */
if (def->memorysnapshotfile &&
!g_path_is_absolute(def->memorysnapshotfile)) {
--
2.35.1
6:40 a.m.
New subject: [PATCH 12/14] virDomainSnapshotDefParse: Decouple parsing of memory snapshot config
Separate the steps of parsing the memory snapshot config from the
post-processing and validation code. The upcoming patch refactoring the
parsing will be simpler.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/snapshot_conf.c | 50 ++++++++++++++++++++++++----------------
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 7f98ae6aec..594492ccd2 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -317,27 +317,37 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
memorySnapshot);
return NULL;
}
- if (def->memorysnapshotfile &&
- def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
- virReportError(VIR_ERR_XML_ERROR,
- _("memory filename '%s' requires external
snapshot"),
- def->memorysnapshotfile);
- return NULL;
- }
- if (!def->memorysnapshotfile &&
- def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("external memory snapshots require a filename"));
- return NULL;
+ }
+
+ if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) {
+ if (def->memorysnapshotfile) {
+ def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
+ } else if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
+ if (offline) {
+ def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NO;
+ } else {
+ def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
+ }
}
- } else if (def->memorysnapshotfile) {
- def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
- } else if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
- def->memory = (offline ?
- VIR_DOMAIN_SNAPSHOT_LOCATION_NO :
- VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL);
- }
- if (offline && def->memory &&
+ }
+
+ if (def->memorysnapshotfile &&
+ def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("memory filename '%s' requires external
snapshot"),
+ def->memorysnapshotfile);
+ return NULL;
+ }
+
+ if (!def->memorysnapshotfile &&
+ def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("external memory snapshots require a filename"));
+ return NULL;
+ }
+
+ if (offline &&
+ def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_NO) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("memory state cannot be saved with offline or "
--
2.35.1
6:40 a.m.
New subject: [PATCH 13/14] conf: snapshot: Use proper types for snapshot location
Refactor the code to use proper types for the memory and disk snapshot
location and fix the parsing code to be compatible with an unsigned
type.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/snapshot_conf.c | 39 +++++++++++++++++----------------------
src/conf/snapshot_conf.h | 4 ++--
2 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 594492ccd2..e2442441d0 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -130,7 +130,6 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
unsigned int flags,
virDomainXMLOption *xmlopt)
{
- g_autofree char *snapshot = NULL;
g_autofree char *driver = NULL;
g_autofree char *name = NULL;
g_autoptr(virStorageSource) src = virStorageSourceNew();
@@ -145,16 +144,12 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
return -1;
}
- snapshot = virXMLPropString(node, "snapshot");
- if (snapshot) {
- def->snapshot = virDomainSnapshotLocationTypeFromString(snapshot);
- if (def->snapshot <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown disk snapshot setting '%s'"),
- snapshot);
- return -1;
- }
- }
+ if (virXMLPropEnumDefault(node, "snapshot",
+ virDomainSnapshotLocationTypeFromString,
+ VIR_XML_PROP_NONZERO,
+ &def->snapshot,
+ VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) < 0)
+ return -1;
if (virXMLPropEnumDefault(node, "type",
virStorageTypeFromString,
@@ -196,7 +191,8 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
return -1;
}
- if (!def->snapshot && (src->path || src->format))
+ if (def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
+ (src->path || src->format))
def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
def->name = g_steal_pointer(&name);
@@ -220,7 +216,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
g_autofree xmlNodePtr *diskNodes = NULL;
size_t i;
int n;
- g_autofree char *memorySnapshot = NULL;
+ xmlNodePtr memoryNode = NULL;
bool offline = !!(flags & VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE);
virSaveCookieCallbacks *saveCookie = virDomainXMLOptionGetSaveCookie(xmlopt);
int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
@@ -307,16 +303,15 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
return NULL;
}
- memorySnapshot = virXPathString("string(./memory/@snapshot)", ctxt);
- def->memorysnapshotfile = virXPathString("string(./memory/@file)",
ctxt);
- if (memorySnapshot) {
- def->memory = virDomainSnapshotLocationTypeFromString(memorySnapshot);
- if (def->memory <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown memory snapshot setting '%s'"),
- memorySnapshot);
+ if ((memoryNode = virXPathNode("./memory", ctxt))) {
+ def->memorysnapshotfile = virXMLPropString(memoryNode, "file");
+
+ if (virXMLPropEnumDefault(memoryNode, "snapshot",
+ virDomainSnapshotLocationTypeFromString,
+ VIR_XML_PROP_NONZERO,
+ &def->memory,
+ VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) < 0)
return NULL;
- }
}
if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) {
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 8823af1ac1..1f787f1a94 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -51,7 +51,7 @@ G_STATIC_ASSERT((int)VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT ==
VIR_DOMAIN_LAST);
typedef struct _virDomainSnapshotDiskDef virDomainSnapshotDiskDef;
struct _virDomainSnapshotDiskDef {
char *name; /* name matching the <target dev='...' of the domain */
- int snapshot; /* virDomainSnapshotLocation */
+ virDomainSnapshotLocation snapshot;
/* details of wrapper external file. src is always non-NULL.
* XXX optimize this to allow NULL for internal snapshots? */
@@ -70,7 +70,7 @@ struct _virDomainSnapshotDef {
/* Additional public XML. */
int state; /* virDomainSnapshotState */
- int memory; /* virDomainMemorySnapshot */
+ virDomainSnapshotLocation memory;
char *memorysnapshotfile; /* memory state file when snapshot is external */
size_t ndisks; /* should not exceed dom->ndisks */
--
2.35.1
6:40 a.m.
New subject: [PATCH 14/14] qemuSnapshotCreateActiveExternal: Remove duplicit assignment
The block of code pausing the VM assigns 'resume' to true but it's
already true because of the previous condition.
The code is deliberately kept in two blocks as upcoming changes will
modify both conditions.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 3fb1d3abe0..a7901779fc 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1400,8 +1400,6 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver,
_("guest unexpectedly quit"));
goto cleanup;
}
-
- resume = true;
}
}
--
2.35.1
7:04 a.m.
On a Thursday in 2022, Peter Krempa wrote:
Cleanup patches extracted from my upcoming series for handling
snapshots
with outsourced storage.
Peter Krempa (14):
virDomainDiskDefFormat: Refactor to virXMLFormatElement
qemuSnapshotDiskPrepareActiveExternal: Handle only external snapshots
qemuSnapshotCreateAlignDisks: Rewrite logic for selecting default
memory snapshot mode
virDomainSnapshotDiskDefParseXML: Automatically free temporary
variables and remove cleanup
virStorageSource: Convert 'type' to proper enum
conf: Move definition of 'virDomainSnapshotLocation'
Rename VIR_DOMAIN_SNAPSHOT_LOCATION_NONE to
VIR_DOMAIN_SNAPSHOT_LOCATION_NO
qemuDomainSnapshotForEachQcow2Raw: Act only on internal snapshots
conf: snapshot: Remove VIR_DOMAIN_SNAPSHOT_PARSE_DISKS flag
virDomainSnapshotDefParse: Refactor cleanup
virDomainSnapshotDefParse: Avoid 'memoryfile' temporary variable
virDomainSnapshotDefParse: Decouple parsing of memory snapshot config
conf: snapshot: Use proper types for snapshot location
qemuSnapshotCreateActiveExternal: Remove duplicit assignment
src/ch/ch_monitor.c | 1 +
[..]
src/vz/vz_driver.c | 2 +-
src/vz/vz_sdk.c | 2 +-
tests/qemudomainsnapshotxml2xmltest.c | 2 +-
16 files changed, 221 insertions(+), 245 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
983
days inactive
983
days old
15 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa