Re: [libvirt] [PATCH v3 02/18] snapshot: Rework virDomainSnapshotState enum
On 3/7/19 9:26 AM, John Ferlan wrote:
>> + 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/
D'oh - it's a bad sign when I miss incorporating my own review comments.
>>
>> /* 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
>
Indeed, but it is also longer, and runs into line-wrap issues (I'm not
opposed to it, though).
<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.
I concur with having consistency one way or another. Unless I hear a
strong preference, I'll go ahead and spell the values out in full (and
wrap lines) rather than trying to abbreviate the values but not the enum
name.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)