[libvirt] [libvirt-glib] Add gvir_storage_vol_resize()

[Eric Blake]

On 02/01/2012 12:44 PM, Zeeshan Ali (Khattak) wrote:

>>>> + * @flags: the flags
>>>> + * @err: Return location for errors, or NULL
>>>> + *
>>>> + * Changes the capacity of the storage volume @vol to @capacity.
>>>> + *
>>>> + * Returns: the new capacity of the volume on success, 0 otherwise
>>>> + */
>>>> +gboolean gvir_storage_vol_resize(GVirStorageVol *vol,
>>>> +                                 guint64 capacity,
>>>> +                                 GVirStorageVolResizeFlags flags,
>>>
>>> We're probably already doing that in other places, I may have already
>>> mentioned it, but I'm a bit uncomfortable with using the enum type for
>>> bitfields. If I'm not mistaken, if we use GVirStorageVolResizeFlags,
>>> passing a value which is not one of the enum values (such as DELTA |
>>> SHRINK) is undefined behaviour in the C spec. I'm fine with us deciding
>>> it's not important, but it's still worth mentioning :)
> 
> Seriously, do we have to care about every corner-case imaginable? Why
> would anyone pass anything other than the enums supported when the
> type of the parameter is clearly indicated in function signature? Even
> if for some weird reason, somebody somehow manages to do this, AFAIK
> every modern (enough) compiler will give him/her warning about this.
> And if compiler isn't going to, libvirt is erroring out if you pass
> anything other than support flags.

The problem is that enum virStorageVolResizeFlags in libvirt.h is _not_
a strict enum where no other values are valid, but a definition of bit
values that are designed to be combined together.  That is, the flags
value of 6 (aka VIR_STORAGE_VOL_RESIZE_DELTA |
VIR_STROAGE_VOL_RESIZE_SHRINK) is _not_ part of the C enum, but _is_ a
valid value to pass into the flags field of the C API.

We are insisting on using integer, rather than enum types, for the flags
parameter, precisely because we want to only list N flags rather than
all 2**N valid combinations of the flags in our enum.  Since the bitwise
OR of enums is valid in C, but produces an int value, rather than
maintaining the type of the enum, it is easier to encourage proper
bitwise-OR by giving the flags parameter the type that results from use
of bitwise-OR.

> 
> OTOH, not specifying the exact type increases the chances of developer
> passing wrong values.

Perhaps, but that's why every libvirt API also has a check for all bits
falling within the expected range, and erroring out if any unknown bits
are found (at least that were unknown at the time the .so was compiled).
 That way, if a future .so understands more bits, and you don't know
whether you are talking to a newer or older server, you can try the bit
out, and be guaranteed of a sane failure if you are talking to the older
server, rather than getting in a potentially undefined state where the
server ignored your bit and you think the server is in a different state
than it really is because you assume the bit was acted on.

> 
>> Yeah, for methods which are 'flags', we should use  guint64 as the
>> type. Only use the enum types for things which really are enums,
>> not bitfields.

guint64 may be a bit big, since libvirt.h only uses 32-bit flags; but I
agree with Dan's complaint that we should not be using the flag enum
type as the signature, at least not for any flags parameter designed to
be a bitwise-OR of defined bit values.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

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