The existing virDomainSnapshotState is a superset of virDomainState,
adding one more state (disk-snapshot) on top of valid domain states.
But as written, the enum cannot be used for gcc validation that all
enum values are covered in a strongly-typed switch condition, because
the enum does not explicitly include the values it is adding to.
Copy the style used in qemu_blockjob.h of creating new enum names
for every inherited value, and update all clients to use the new
enum names anywhere snapshot state is referenced. Qemu code gains
a fixme comment about some odd casts (which will be cleaned up in
separate patches); the rest of the patch is mechanical.
Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
src/conf/snapshot_conf.h | 21 ++++++++++++++++++---
src/conf/snapshot_conf.c | 28 ++++++++++++++--------------
src/qemu/qemu_driver.c | 27 ++++++++++++++++-----------
src/test/test_driver.c | 20 ++++++++++----------
src/vbox/vbox_common.c | 4 ++--
5 files changed, 60 insertions(+), 40 deletions(-)
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 7a175dfc96..9084f5fb8b 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -36,11 +36,26 @@ typedef enum {
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
+ * for snapshots.
+ */
typedef enum {
- /* Inherit the VIR_DOMAIN_* states from virDomainState. */
- VIR_DOMAIN_DISK_SNAPSHOT = VIR_DOMAIN_LAST,
- VIR_DOMAIN_SNAPSHOT_STATE_LAST
+ /* Mapped to public enum */
+ VIR_SNAP_STATE_NOSTATE = VIR_DOMAIN_NOSTATE,
+ VIR_SNAP_STATE_RUNNING = VIR_DOMAIN_RUNNING,
+ VIR_SNAP_STATE_BLOCKED = VIR_DOMAIN_BLOCKED,
+ VIR_SNAP_STATE_PAUSED = VIR_DOMAIN_PAUSED,
+ VIR_SNAP_STATE_SHUTDOWN = VIR_DOMAIN_SHUTDOWN,
+ VIR_SNAP_STATE_SHUTOFF = VIR_DOMAIN_SHUTOFF,
+ VIR_SNAP_STATE_CRASHED = VIR_DOMAIN_CRASHED,
+ VIR_SNAP_STATE_PMSUSPENDED = VIR_DOMAIN_PMSUSPENDED,
+ /* Additional enum values local to qemu */
+ VIR_SNAP_STATE_DISK_SNAPSHOT,
+ VIR_SNAP_STATE_LAST
} virDomainSnapshotState;
+verify((int)VIR_SNAP_STATE_DISK_SNAPSHOT == VIR_DOMAIN_LAST);
/* Stores disk-snapshot information */
typedef struct _virDomainSnapshotDiskDef virDomainSnapshotDiskDef;
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 41236d9932..299fc2101b 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -58,7 +58,7 @@ VIR_ENUM_IMPL(virDomainSnapshotLocation, VIR_DOMAIN_SNAPSHOT_LOCATION_LAST,
);
/* virDomainSnapshotState is really virDomainState plus one extra state */
-VIR_ENUM_IMPL(virDomainSnapshotState, VIR_DOMAIN_SNAPSHOT_STATE_LAST,
+VIR_ENUM_IMPL(virDomainSnapshotState, VIR_SNAP_STATE_LAST,
"nostate",
"running",
"blocked",
@@ -257,8 +257,8 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
state);
goto cleanup;
}
- offline = (def->state == VIR_DOMAIN_SHUTOFF ||
- def->state == VIR_DOMAIN_DISK_SNAPSHOT);
+ offline = (def->state == VIR_SNAP_STATE_SHUTOFF ||
+ def->state == VIR_SNAP_STATE_DISK_SNAPSHOT);
/* Older snapshots were created with just <domain>/<uuid>, and
* lack domain/@type. In that case, leave dom NULL, and
@@ -879,14 +879,14 @@ static int virDomainSnapshotObjListCopyNames(void *payload,
if (data->flags & VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS) {
if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE) &&
- obj->def->state == VIR_DOMAIN_SHUTOFF)
+ obj->def->state == VIR_SNAP_STATE_SHUTOFF)
return 0;
if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) &&
- obj->def->state == VIR_DOMAIN_DISK_SNAPSHOT)
+ obj->def->state == VIR_SNAP_STATE_DISK_SNAPSHOT)
return 0;
if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE) &&
- obj->def->state != VIR_DOMAIN_SHUTOFF &&
- obj->def->state != VIR_DOMAIN_DISK_SNAPSHOT)
+ obj->def->state != VIR_SNAP_STATE_SHUTOFF &&
+ obj->def->state != VIR_SNAP_STATE_DISK_SNAPSHOT)
return 0;
}
@@ -1225,7 +1225,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
bool align_match = true;
virDomainSnapshotObjPtr other;
- bool external = def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
+ bool external = def->state == VIR_SNAP_STATE_DISK_SNAPSHOT ||
virDomainSnapshotDefIsExternal(def);
/* Prevent circular chains */
@@ -1282,10 +1282,10 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
other = virDomainSnapshotFindByName(vm->snapshots, def->name);
if (other) {
- if ((other->def->state == VIR_DOMAIN_RUNNING ||
- other->def->state == VIR_DOMAIN_PAUSED) !=
- (def->state == VIR_DOMAIN_RUNNING ||
- def->state == VIR_DOMAIN_PAUSED)) {
+ if ((other->def->state == VIR_SNAP_STATE_RUNNING ||
+ other->def->state == VIR_SNAP_STATE_PAUSED) !=
+ (def->state == VIR_SNAP_STATE_RUNNING ||
+ def->state == VIR_SNAP_STATE_PAUSED)) {
virReportError(VIR_ERR_INVALID_ARG,
_("cannot change between online and offline "
"snapshot state in snapshot %s"),
@@ -1293,8 +1293,8 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
goto cleanup;
}
- if ((other->def->state == VIR_DOMAIN_DISK_SNAPSHOT) !=
- (def->state == VIR_DOMAIN_DISK_SNAPSHOT)) {
+ if ((other->def->state == VIR_SNAP_STATE_DISK_SNAPSHOT) !=
+ (def->state == VIR_SNAP_STATE_DISK_SNAPSHOT)) {
virReportError(VIR_ERR_INVALID_ARG,
_("cannot change between disk only and "
"full system in snapshot %s"),
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a8ac9b59ee..34014ba9c7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15022,7 +15022,7 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL:
found_internal = true;
- if (def->state == VIR_DOMAIN_DISK_SNAPSHOT && active) {
+ if (def->state == VIR_SNAP_STATE_DISK_SNAPSHOT && active) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("active qemu domains require external disk "
"snapshots; disk %s requested internal"),
@@ -15099,7 +15099,7 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
}
/* disk snapshot requires at least one disk */
- if (def->state == VIR_DOMAIN_DISK_SNAPSHOT && !external) {
+ if (def->state == VIR_SNAP_STATE_DISK_SNAPSHOT && !external) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("disk-only snapshots require at least "
"one disk to be selected for snapshot"));
@@ -15844,9 +15844,9 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
align_match = false;
if (virDomainObjIsActive(vm))
- def->state = VIR_DOMAIN_DISK_SNAPSHOT;
+ def->state = VIR_SNAP_STATE_DISK_SNAPSHOT;
else
- def->state = VIR_DOMAIN_SHUTOFF;
+ def->state = VIR_SNAP_STATE_SHUTOFF;
def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
} else if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
def->state = virDomainObjGetState(vm, NULL);
@@ -15863,7 +15863,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
goto endjob;
}
- def->memory = (def->state == VIR_DOMAIN_SHUTOFF ?
+ def->memory = (def->state == VIR_SNAP_STATE_SHUTOFF ?
VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL);
}
@@ -16426,8 +16426,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
goto endjob;
if (!vm->persistent &&
- snap->def->state != VIR_DOMAIN_RUNNING &&
- snap->def->state != VIR_DOMAIN_PAUSED &&
+ snap->def->state != VIR_SNAP_STATE_RUNNING &&
+ snap->def->state != VIR_SNAP_STATE_PAUSED &&
(flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) == 0) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -16450,8 +16450,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
goto endjob;
}
if (virDomainObjIsActive(vm) &&
- !(snap->def->state == VIR_DOMAIN_RUNNING
- || snap->def->state == VIR_DOMAIN_PAUSED) &&
+ !(snap->def->state == VIR_SNAP_STATE_RUNNING ||
+ snap->def->state == VIR_SNAP_STATE_PAUSED) &&
(flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
@@ -16487,6 +16487,11 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
cookie = (qemuDomainSaveCookiePtr) snap->def->cookie;
+ /* FIXME: This cast should be to virDomainSnapshotState, with
+ * better handling of VIR_SNAP_STATE_DISK_SNAPSHOT (the only enum
+ * value added beyond what virDomainState supports). But for now
+ * it doesn't matter, because of the above rejection of revert to
+ * external snapshots. */
switch ((virDomainState) snap->def->state) {
case VIR_DOMAIN_RUNNING:
case VIR_DOMAIN_PAUSED:
@@ -16628,7 +16633,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
/* Touch up domain state. */
if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
- (snap->def->state == VIR_DOMAIN_PAUSED ||
+ (snap->def->state == VIR_SNAP_STATE_PAUSED ||
(flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
/* Transitions 3, 6, 9 */
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
@@ -16735,7 +16740,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid target domain state '%s'. Refusing "
"snapshot reversion"),
- virDomainStateTypeToString(snap->def->state));
+ virDomainSnapshotStateTypeToString(snap->def->state));
goto endjob;
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index ce0df1f8e3..4a6555e0ea 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -6271,9 +6271,9 @@ testDomainSnapshotAlignDisks(virDomainObjPtr vm,
align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
align_match = false;
if (virDomainObjIsActive(vm))
- def->state = VIR_DOMAIN_DISK_SNAPSHOT;
+ def->state = VIR_SNAP_STATE_DISK_SNAPSHOT;
else
- def->state = VIR_DOMAIN_SHUTOFF;
+ def->state = VIR_SNAP_STATE_SHUTOFF;
def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
} else if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
def->state = virDomainObjGetState(vm, NULL);
@@ -6281,7 +6281,7 @@ testDomainSnapshotAlignDisks(virDomainObjPtr vm,
align_match = false;
} else {
def->state = virDomainObjGetState(vm, NULL);
- def->memory = def->state == VIR_DOMAIN_SHUTOFF ?
+ def->memory = def->state == VIR_SNAP_STATE_SHUTOFF ?
VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
}
@@ -6572,8 +6572,8 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
goto cleanup;
if (!vm->persistent &&
- snap->def->state != VIR_DOMAIN_RUNNING &&
- snap->def->state != VIR_DOMAIN_PAUSED &&
+ snap->def->state != VIR_SNAP_STATE_RUNNING &&
+ snap->def->state != VIR_SNAP_STATE_PAUSED &&
(flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) == 0) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -6590,8 +6590,8 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
goto cleanup;
}
if (virDomainObjIsActive(vm) &&
- !(snap->def->state == VIR_DOMAIN_RUNNING
- || snap->def->state == VIR_DOMAIN_PAUSED) &&
+ !(snap->def->state == VIR_SNAP_STATE_RUNNING ||
+ snap->def->state == VIR_SNAP_STATE_PAUSED) &&
(flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
@@ -6612,8 +6612,8 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
if (!config)
goto cleanup;
- if (snap->def->state == VIR_DOMAIN_RUNNING ||
- snap->def->state == VIR_DOMAIN_PAUSED) {
+ if (snap->def->state == VIR_SNAP_STATE_RUNNING ||
+ snap->def->state == VIR_SNAP_STATE_PAUSED) {
/* Transitions 2, 3, 5, 6, 8, 9 */
bool was_running = false;
bool was_stopped = false;
@@ -6672,7 +6672,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
/* Touch up domain state. */
if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
- (snap->def->state == VIR_DOMAIN_PAUSED ||
+ (snap->def->state == VIR_SNAP_STATE_PAUSED ||
(flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
/* Transitions 3, 6, 9 */
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index d95fe7c7ae..407c932019 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -6314,9 +6314,9 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
goto cleanup;
}
if (online)
- def->state = VIR_DOMAIN_RUNNING;
+ def->state = VIR_SNAP_STATE_RUNNING;
else
- def->state = VIR_DOMAIN_SHUTOFF;
+ def->state = VIR_SNAP_STATE_SHUTOFF;
virUUIDFormat(dom->uuid, uuidstr);
memcpy(def->dom->uuid, dom->uuid, VIR_UUID_BUFLEN);
--
2.20.1
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
