[libvirt] [PATCH] interface: list all interfaces with flags == 0
Jiri Denemark
jdenemar at redhat.com
Tue May 21 12:15:22 UTC 2013
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
More information about the libvir-list
mailing list