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. > + > + if (ifaces) { > + iface_obj = virGetInterface(conn, ncf_if_name(iface), > + ncf_if_mac_string(iface)); > + tmp_iface_objs[niface_objs++] = iface_obj; > } > > ncf_if_free(iface); > diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c > index 1fd7d46..242fc15 100644 > --- a/src/interface/interface_backend_udev.c > +++ b/src/interface/interface_backend_udev.c ... > @@ -363,18 +362,15 @@ udevConnectListAllInterfaces(virConnectPtr conn, > status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); > > /* Filter the results */ > - if (status && (flags & VIR_CONNECT_LIST_INTERFACES_ACTIVE)) > - add_to_list = 1; > - else if (!status && (flags & VIR_CONNECT_LIST_INTERFACES_INACTIVE)) > - add_to_list = 1; > + if (MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) && > + !((MATCH(VIR_CONNECT_LIST_INTERFACES_ACTIVE) && status) || > + (MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) && !status))) > + continue; Same bug in skipping udev_device_unref(dev). And similarly hard to read test :-) > > /* If we matched a filter, then add it */ > - if (add_to_list) { > - if (ifaces) { > - iface_obj = virGetInterface(conn, name, macaddr); > - ifaces_list[count] = iface_obj; > - } > - count++; > + if (ifaces) { > + iface_obj = virGetInterface(conn, name, macaddr); > + ifaces_list[count++] = iface_obj; > } > udev_device_unref(dev); > } Jirka -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list