[libvirt] [PATCH] virsh-volume: Unify strigification of volume type

[Daniel P. Berrange]

On Tue, Nov 12, 2013 at 06:33:04PM +0100, Peter Krempa wrote:
> On 11/12/13 17:13, Eric Blake wrote:
> > On 11/12/2013 08:16 AM, Peter Krempa wrote:
> >> There were two separate places with that were stringifying type of a
> >> volume. One of the places was out of sync with types implemented
> >> upstream.
> >>
> >> To avoid such problems in the future, this patch adds a common function
> >> to convert the type to string and reuses it across the two said places.
> >> ---
> >>  tools/virsh-volume.c | 59 ++++++++++++++++++++++++----------------------------
> >>  1 file changed, 27 insertions(+), 32 deletions(-)
> > 
> > ACK with one nit:
> > 
> >> +static const char *
> >> +vshVolumeTypeToString(int type)
> >> +{
> >> +    switch (type) {
> > 
> > Please make this "switch ((virStorageVolType) type)"
> 
> I was considering this, but in the VIR_STORAGE_VOL_LAST value defined
> virStorageVolType enum is protected by an ifdef:
> 
> typedef enum {
>     VIR_STORAGE_VOL_FILE = 0,   /* Regular file based volumes */
>     VIR_STORAGE_VOL_BLOCK = 1,  /* Block based volumes */
>     VIR_STORAGE_VOL_DIR = 2,    /* Directory-passthrough based volume */
>     VIR_STORAGE_VOL_NETWORK = 3,/* Network volumes like RBD (RADOS
> Block Device) */
> 
> #ifdef VIR_ENUM_SENTINELS
>     VIR_STORAGE_VOL_LAST		<- here
> #endif
> } virStorageVolType;
> 
> As I don't know what are the conditions that make VIR_ENUM_SENTINELS
> defined I didn't want to risk a broken build.

Applications are expected to #define VIR_ENUM_SENTINELS themselves
before including libvirt.h, once they accept that the _LAST values
are subject to change between releases.

/* General note - throughout this file, any linear enumeration which
 * might be expanded in the future has an optional *_LAST value that
 * gives the size of the enum at the time of compilation, if the user
 * defines VIR_ENUM_SENTINELS.  Enumerations for bit values do not
 * have a *_LAST value, but additional bits may be defined.  */


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|