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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list