On Wed, Feb 1, 2012 at 2:02 PM, Daniel P. Berrange <berrange@xxxxxxxxxx> wrote: > On Wed, Feb 01, 2012 at 12:03:43PM +0100, Christophe Fergeau wrote: >> On Wed, Feb 01, 2012 at 02:27:28AM +0200, Zeeshan Ali (Khattak) wrote: >> > From: "Zeeshan Ali (Khattak)" <zeeshanak@xxxxxxxxx> >> > >> > Add wrapper for virStorageVolResize(). >> > --- >> > libvirt-gobject/libvirt-gobject-storage-vol.c | 45 +++++++++++++++++++++++++ >> > libvirt-gobject/libvirt-gobject-storage-vol.h | 20 +++++++++++ >> > libvirt-gobject/libvirt-gobject.sym | 1 + >> > 3 files changed, 66 insertions(+), 0 deletions(-) >> > >> > diff --git a/libvirt-gobject/libvirt-gobject-storage-vol.c b/libvirt-gobject/libvirt-gobject-storage-vol.c >> > index 491e2e6..f6f0c50 100644 >> > --- a/libvirt-gobject/libvirt-gobject-storage-vol.c >> > +++ b/libvirt-gobject/libvirt-gobject-storage-vol.c >> > @@ -299,3 +299,48 @@ gboolean gvir_storage_vol_delete(GVirStorageVol *vol, >> > >> > return TRUE; >> > } >> > + >> > +/** >> > + * gvir_storage_vol_resize: >> > + * @vol: the storage volume to resize >> > + * @capacity: the new capacity of the volume >> > + * @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. OTOH, not specifying the exact type increases the chances of developer passing wrong values. > 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. On C-level this will only mean a slightly non-intuitive API (as you can't see from the signature what the hell is supposed to be passed to flags) but in high-level languages it might even mean that developer have to cast the enums to guint64 explicitly (which would be annoying as the reason for this won't be obvious to most). Anyway, I am strong against what you guys are suggesting as I prefer intuitive API over caring about such corner cases but in the end its up to you (Daniel) so just say 'nay' in reply and I'll change it to guint64. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list