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

Osier Yang jyang at redhat.com
Tue Sep 11 10:10:42 UTC 2012


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:
>>>> 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.
>
> 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
    VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL), which are defined in
    *_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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list




More information about the libvir-list mailing list