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

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.