On 05/21/2013 08:15 PM, Jiri Denemark wrote:
On Tue, May 21, 2013 at 17:05:09 +0800, Guan Nan Ren wrote:
virConnectListAllInterfaces should support to list all of
interfaces when the value of flags is 0. The behaviour is
consistent with other virConnectListAll* APIs
---
diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c
index cbba4fd..c6e069a 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
...
@@ -361,16 +359,17 @@ netcfConnectListAllInterfaces(virConnectPtr conn,
/* XXX: Filter the result, need to be splitted once new filter flags
* except active|inactive are supported.
*/
- if (((status & NETCF_IFACE_ACTIVE) &&
- (flags & VIR_CONNECT_LIST_INTERFACES_ACTIVE)) ||
- ((status & NETCF_IFACE_INACTIVE) &&
- (flags & VIR_CONNECT_LIST_INTERFACES_INACTIVE))) {
- if (ifaces) {
- iface_obj = virGetInterface(conn, ncf_if_name(iface),
- ncf_if_mac_string(iface));
- tmp_iface_objs[niface_objs] = iface_obj;
- }
- niface_objs++;
+ if (MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) &&
+ !((MATCH(VIR_CONNECT_LIST_INTERFACES_ACTIVE) &&
+ (status & NETCF_IFACE_ACTIVE)) ||
+ (MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) &&
+ (status & NETCF_IFACE_INACTIVE))))
+ continue;
IMHO this is much harder to read. I'd just rewrite the original
condition as (i.e., !MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE)
is the part that was missing):
if (!MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) ||
(MATCH(VIR_CONNECT_LIST_INTERFACES_ACTIVE) &&
(status & NETCF_IFACE_ACTIVE)) ||
(MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) &&
(status & NETCF_IFACE_INACTIVE)))
Moreover, your version is wrong as it skips ncf_if_free(iface) in case
iface should be skipped.
yes, currently, there are only one group of flags with two
values(active/inactive),
the way of yours is better to read.
if in future, there are more than one group of flags which are
going to support,
the way of my version will be better. virConnectListAllDomains
use this way to
filter list of domains.
Guannan
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list