Re: [libvirt] [PATCH 1/2] resize: add virStorageVolResize() API
On 2012年06月06日 20:58, Eric Blake wrote:
On 06/06/2012 06:12 AM, Osier Yang wrote:
> On 2012年01月28日 08:28, Eric Blake wrote:
>> From: "Zeeshan Ali (Khattak)"<zeeshanak(a)gnome.org>
>>
>> Add a new function to allow changing of capacity of storage volumes.
>> Plan out several flags, even if not all of them will be implemented
>> up front.
>>
>
> Late response, but it seems not good idea to expose the options
> which are actually not implemented underlying yet. One would get
> error like:
>
> # virsh vol-resize /var/lib/libvirt/images/test.img 60 --shrink
> error: invalid argument: storageVolumeResize: unsupported flags (0x4)
>
> With the magic number "0x4", one'd never known what it is. It can
> be improved roundabout, but it should be avoided in the beginning.
Improving the error message would indeed be nice, if you can find a way
to do it across the board for all APIs that call virCheckFlags(). But
your general objection to avoiding flag values that are not implemented
across all drivers is impossible - as we add new APIs and new flags to
existing APIs, we are stuck with the fact that not all hypervisors can
implement all those flags simultaneously (if even at all). I see no
problem with defining a libvirt API with flags that some drivers cannot
implement yet,
Sure, it's fine for an API to have a flag not implemented underlying.
what I'm objecting is not to expose the options in virsh commands
if none of the drivers support them yet.
But yeah, all the problems are not problem if we could find a way to
improve the error message with some work on virCheckFlags. I don't
have a good idea in mind yet, though. Converting the magic number (flag)
to string doesn't make sense as the user won't known the mapping
between them, and it smells like overkilling.
Regards,
Osier