Re: [PATCH v3 02/18] snapshot: Rework virDomainSnapshotState enum

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 04, 2019 at 09:34:29PM -0600, Eric Blake wrote:
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 most clients to use the new
enum names anywhere snapshot state is referenced. The exception is
two switch statements in qemu code, which instead gain a fixme
comment about odd type usage (which will be cleaned up in the next
patch). 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   | 34 ++++++++++++++++++++++------------
src/test/test_driver.c   | 20 ++++++++++----------
src/vbox/vbox_common.c   |  4 ++--
5 files changed, 66 insertions(+), 41 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 */

As Eric Blake pointed out in an earlier iteration:
s/qemu/snapshots/

+    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,

VIR_DOMAIN_SNAPSHOT_STATE would be the least surprising prefix

              "nostate",
              "running",
              "blocked",

Jano

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux