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

Re: [libvirt] [PATCH 6/7] list: Use virConnectListAllNetworks in virsh



On Mon, Sep 10, 2012 at 12:29:43PM -0400, Laine Stump wrote:
> On 09/04/2012 11:55 AM, Osier Yang wrote:
> > tools/virsh-network.c:
> >   * vshNetworkSorter to sort networks by name
> >
> >   * vshNetworkListFree to free the network objects list.
> >
> >   * vshNetworkListCollect to collect the network objects, trying
> >     to use new API first, fall back to older APIs if it's not supported.
> >
> >   * New options --persistent, --transient, --autostart, --no-autostart,
> >     for net-list, and new field 'Persistent' for its output.
> >
> > tools/virsh.pod:
> >   * Add documents for the new options.
> > ---
> >  tools/virsh-network.c |  352 ++++++++++++++++++++++++++++++++++++------------
> >  tools/virsh.pod       |   12 ++-
> >  2 files changed, 275 insertions(+), 89 deletions(-)
> >
> > diff --git a/tools/virsh-network.c b/tools/virsh-network.c
> > index db204af..f6623ff 100644
> > --- a/tools/virsh-network.c
> > +++ b/tools/virsh-network.c
> > @@ -36,6 +36,7 @@
> >  #include "memory.h"
> >  #include "util.h"
> >  #include "xml.h"
> > +#include "conf/network_conf.h"
> 
> 
> I've gotta say that (as discussed before) I don't like including
> something from the conf directory here. I think it's the case that this
> is only being done so that virsh can provide the new functionality even
> when only the old API is available, but I think it should be done in a
> self-contained manner, at least partly because people will look to virsh
> as an example of how to use the libvirt API. I guess I'm okay with
> leaving it this way for now, but I think it really needs to be cleaned up.

I don't see why the fallback code needs to use this include either,
since it must surely still be just using older public APIs, not
internal code.


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]