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

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(a)redhat.com&gt; > --- > 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 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list