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

Ján Tomko jtomko at redhat.com
Thu Mar 7 14:20:57 UTC 2019


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 at redhat.com>
>---
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190307/aea69848/attachment-0001.sig>


More information about the libvir-list mailing list