[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 Fri, Jul 20, 2012 at 09:26:48PM +0800, 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?

Just raise an error if it is not supported, as we do for any other
virsh option the user requests that isn't supported


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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