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

[John Ferlan]

On 3/7/19 9:20 AM, Ján Tomko wrote:
> 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
> 

<sigh>, true, but we could go with virSnapState to ensure we match the
typedef nomenclature with the enum symbol names or change the _SNAP_ to
_SNAPSHOT_ w/ virSnapshotState for the typedef - whatever is easiest.

John

>>               "nostate",
>>               "running",
>>               "blocked",
> 
> Jano
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>