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@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
>
<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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
