[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 2012年09月11日 01:30, Laine Stump wrote:
On 09/10/2012 12:33 PM, Daniel P. Berrange wrote:
On Mon, Sep 10, 2012 at 12:29:43PM -0400, Laine Stump wrote:
On 09/04/2012 11:55 AM, Osier Yang wrote:
   * 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.

   * 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.

I haven't investigated but have an inkling that it's likely used to
avoid retyping some #defines of combined flags or something like that.

Mainly two reaons:

1) To use the filtering macros (such as
   *_conf.h now, should be moved to public when doing cleanup later.

2) To use some helpers, such as virStoragePoolTypeFromString, which
   is used in the list all storage pools series. It's not the first
   breaker, for example, the previous
   virDomainNumatuneMemModeTypeFromString, these all should be cleaned
   up later.

libvir-list mailing list
libvir-list redhat com

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