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

Peter Krempa pkrempa at redhat.com
Tue Nov 12 17:33:04 UTC 2013


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.


> 
>> +    case VIR_STORAGE_VOL_FILE:
>> +        return N_("file");
>> +
>> +    case VIR_STORAGE_VOL_BLOCK:
>> +        return N_("block");
>> +
>> +    case VIR_STORAGE_VOL_DIR:
>> +        return N_("dir");
>> +
>> +    case VIR_STORAGE_VOL_NETWORK:
>> +        return N_("network");
>> +
>> +    default:
>> +        return N_("unknown");
>> +    }
> 
> drop the default: case, and instead use:
> 
>     VIR_STORAGE_VOL_LAST:
>         break;
>     }
>     return N_("unknown");
> 
> which will then let the compiler enforce us to expand the list if we
> ever add another type.

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131112/848bf54e/attachment-0001.sig>


More information about the libvir-list mailing list