[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 02/50] list: Expose pool type via virStoragePoolGetInfo



On 07/20/12 15:26, Osier Yang wrote:
On 2012年07月20日 21:23, Daniel P. Berrange wrote:
On Fri, Jul 20, 2012 at 03:21:20PM +0200, Jiri Denemark wrote:
On Fri, Jul 20, 2012 at 21:10:57 +0800, Osier Yang wrote:
On 2012年07月20日 21:01, Daniel P. Berrange wrote:
On Fri, Jul 20, 2012 at 09:00:21PM +0800, Osier Yang wrote:
On 2012年07月20日 20:44, Daniel P. Berrange wrote:
Fortunately no other part of this patch series appears to rely on
this
extra field. Just remove this addition&    the place in
storage_driver.c
which sets it. The rest of this patch series can still be reviewed
as is

The 'type' is used to filter the returned pool objects, so patches
1/50 to 14/50 should be skipped, though there is several patches
in the range not related with storage pool specificly.

Filtering is done inside the storage driver, so I don't see why
this needs to be exposed in the public API.

Patch 12/50 will explain it: if the server side is old enough without
the new API listAllStoragePools, and virsh is new enough to have the
new introduced option "--type". I.e. new virsh talks to old libvirt,
It will need to get the pool type to filter the results in virsh layer.

I guess I'm missing something important here... If libvirtd is old
enough not
to support listAllStoragePools, how it can be new enough to support
the new
API which would return pool type?

Yeah, this just doesn't make any sense. All filtering should be done in
the libvirtd server side for new enough libvirt. Nothing should be done
client side, for old or new libvirtd.


So how about keeping the "--type" option, and ignore it if talking to
old libvirt without the API? perhaps with a warning?

You might retrieve that info from the pool XML. And if you decide not to support it with older servers, you should quit with an error if it's not supported, not ignore it.

Peter

Regards,
Osier

--
libvir-list mailing list
libvir-list redhat com
https://www.redhat.com/mailman/listinfo/libvir-list



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]