This is to list the interface objects, supported filtering flags are: active|inactive. include/libvirt/libvirt.h.in: Declare enum virConnectListAllInterfaceFlags and virConnectListAllInterfaces. python/generator.py: Skip auto-generating src/driver.h: (virDrvConnectListAllInterfaces) src/libvirt.c: Implement the public API src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in | 13 +++++++ python/generator.py | 1 + src/driver.h | 5 +++ src/libvirt.c | 77 ++++++++++++++++++++++++++++++++++++++++- src/libvirt_public.syms | 1 + 5 files changed, 95 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 50d865c..9226f3e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2386,6 +2386,19 @@ int virConnectNumOfDefinedInterfaces (virConnectPtr conn); int virConnectListDefinedInterfaces (virConnectPtr conn, char **const names, int maxnames); +/* + * virConnectListAllInterfaces: + * + * Flags used to filter the returned interfaces. + */ +typedef enum { + VIR_CONNECT_LIST_INTERFACES_INACTIVE = 1 << 0, + VIR_CONNECT_LIST_INTERFACES_ACTIVE = 1 << 1, +} virConnectListAllInterfacesFlags; + +int virConnectListAllInterfaces (virConnectPtr conn, + virInterfacePtr **ifaces, + unsigned int flags); virInterfacePtr virInterfaceLookupByName (virConnectPtr conn, const char *name); diff --git a/python/generator.py b/python/generator.py index a9a401b..8f6e455 100755 --- a/python/generator.py +++ b/python/generator.py @@ -463,6 +463,7 @@ skip_function = ( 'virConnectListAllStoragePools', # overridden in virConnect.py 'virStoragePoolListAllVolumes', # overridden in virStoragePool.py 'virConnectListAllNetworks', # overridden in virConnect.py + 'virConnectListAllInterfaces', # overridden in virConnect.py 'virStreamRecvAll', # Pure python libvirt-override-virStream.py 'virStreamSendAll', # Pure python libvirt-override-virStream.py diff --git a/src/driver.h b/src/driver.h index 534da05..518e9d4 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1173,6 +1173,10 @@ typedef int (*virDrvListDefinedInterfaces) (virConnectPtr conn, char **const names, int maxnames); +typedef int + (*virDrvListAllInterfaces) (virConnectPtr conn, + virInterfacePtr **ifaces, + unsigned int flags); typedef virInterfacePtr (*virDrvInterfaceLookupByName) (virConnectPtr conn, const char *name); @@ -1231,6 +1235,7 @@ struct _virInterfaceDriver { virDrvListInterfaces listInterfaces; virDrvNumOfDefinedInterfaces numOfDefinedInterfaces; virDrvListDefinedInterfaces listDefinedInterfaces; + virDrvListAllInterfaces listAllInterfaces; virDrvInterfaceLookupByName interfaceLookupByName; virDrvInterfaceLookupByMACString interfaceLookupByMACString; virDrvInterfaceGetXMLDesc interfaceGetXMLDesc; diff --git a/src/libvirt.c b/src/libvirt.c index 686af8a..90ed7ff 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -10727,6 +10727,67 @@ virInterfaceGetConnect (virInterfacePtr iface) } /** + * virConnectListAllInterfaces: + * @conn: Pointer to the hypervisor connection. + * @ifaces: Pointer to a variable to store the array containing the interface + * objects or NULL if the list is not required (just returns number + * of interfaces). + * @flags: bitwise-OR of virConnectListAllInterfacesFlags. + * + * Collect the list of interfaces, and allocate an array to store those + * objects. This API solves the race inherent between virConnectListInterfaces + * and virConnectListDefinedInterfaces. + * + * Normally, all interfaces are returned; however, @flags can be used to + * filter the results for a smaller list of targeted interfaces. The valid + * flags are divided into groups, where each group contains bits that + * describe mutually exclusive attributes of a interface, and where all bits + * within a group describe all possible interfaces. + * + * The only one group of @flags

is VIR_CONNECT_LIST_INTERFACES_ACTIVE (up) and + * VIR_CONNECT_LIST_INTERFACES_INACTIVE (down) to fitler the interfaces by state.

+ * + * Returns the number of interfaces found or -1 and sets @ifaces to NULL in case + * of error. On success, the array stored into @ifaces is guaranteed to have an + * extra allocated element set to NULL but not included in the return count, + * to make iteration easier. The caller is responsible for calling + * virStorageInterfaceFree() on each array element, then calling free() on @ifaces. + */ +int +virConnectListAllInterfaces(virConnectPtr conn, + virInterfacePtr **ifaces, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, ifaces=%p, flags=%x", conn, ifaces, flags); + + virResetLastError(); + + if (ifaces) + *ifaces = NULL; + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (conn->interfaceDriver && + conn->interfaceDriver->listAllInterfaces) { + int ret; + ret = conn->interfaceDriver->listAllInterfaces(conn, ifaces, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} + +/** * virConnectNumOfInterfaces: * @conn: pointer to the hypervisor connection * @@ -10771,7 +10832,13 @@ error: * Collect the list of active physical host interfaces, * and store their names in @names * - * Returns the number of interfaces found or -1 in case of error + * For more control over the results, see virConnectListAllInterfaces(). + * + * Returns the number of interfaces found or -1 in case of error. Note that + * this command is inherently racy; a interface can be started between a call + * to virConnectNumOfInterfaces() and this call; you are only guaranteed that + * all currently active interfaces were listed if the return is less than + * @maxnames. */ int virConnectListInterfaces(virConnectPtr conn, char **const names, int maxnames) @@ -10849,7 +10916,13 @@ error: * Collect the list of defined (inactive) physical host interfaces, * and store their names in @names. * - * Returns the number of interfaces found or -1 in case of error + * For more control over the results, see virConnectListAllInterfaces(). + * + * Returns the number of names provided in the array or -1 in case of error. + * Note that this command is inherently racy; a interface can be defined between + * a call to virConnectNumOfDefinedInterfaces() and this call; you are only + * guaranteed that all currently defined interfaces were listed if the return + * is less than @maxnames. The client must call free() on each returned name. */ int virConnectListDefinedInterfaces(virConnectPtr conn, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 6ff5a77..8dda48b 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -556,6 +556,7 @@ LIBVIRT_0.10.0 { LIBVIRT_0.10.2 { global: + virConnectListAllInterfaces; virConnectListAllNetworks; virConnectListAllStoragePools; virStoragePoolListAllVolumes;

The RPC generator doesn't support returning list of object yet, this patch do the work manually. * daemon/remote.c: Implemente the server side handler remoteDispatchConnectListAllInterfaces. * src/remote/remote_driver.c: Add remote driver handler remoteConnectListAllInterfaces. * src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES and structs to represent the args and ret for it. * src/remote_protocol-structs: Likewise. --- daemon/remote.c | 54 +++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 13 ++++++++- src/remote_protocol-structs | 12 ++++++++ 4 files changed, 142 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index dba51a7..fe2f9dd 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4265,6 +4265,60 @@ cleanup: return rv; } +static int +remoteDispatchConnectListAllInterfaces(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_connect_list_all_interfaces_args *args, + remote_connect_list_all_interfaces_ret *ret) +{ + virInterfacePtr *ifaces = NULL; + int nifaces = 0; + int i; + int rv = -1; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if ((nifaces = virConnectListAllInterfaces(priv->conn, + args->need_results ? &ifaces : NULL, + args->flags)) < 0) + goto cleanup; + + if (ifaces && nifaces) { + if (VIR_ALLOC_N(ret->ifaces.ifaces_val, nifaces) < 0) { + virReportOOMError(); + goto cleanup; + } + + ret->ifaces.ifaces_len = nifaces; + + for (i = 0; i < nifaces; i++) + make_nonnull_interface(ret->ifaces.ifaces_val + i, ifaces[i]); + } else { + ret->ifaces.ifaces_len = 0; + ret->ifaces.ifaces_val = NULL; + } + + ret->ret = nifaces; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (ifaces) { + for (i = 0; i < nifaces; i++) + virInterfaceFree(ifaces[i]); + VIR_FREE(ifaces); + } + return rv; +} + /*----- Helpers. -----*/ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 56e50a6..5c97061 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2781,6 +2781,69 @@ done: return rv; } +static int +remoteConnectListAllInterfaces(virConnectPtr conn, + virInterfacePtr **ifaces, + unsigned int flags) +{ + int rv = -1; + int i; + virInterfacePtr *tmp_ifaces = NULL; + remote_connect_list_all_interfaces_args args; + remote_connect_list_all_interfaces_ret ret; + + struct private_data *priv = conn->privateData; + + remoteDriverLock(priv); + + args.need_results = !!ifaces; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(conn, + priv, + 0, + REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES, + (xdrproc_t) xdr_remote_connect_list_all_interfaces_args, + (char *) &args, + (xdrproc_t) xdr_remote_connect_list_all_interfaces_ret, + (char *) &ret) == -1) + goto done; + + if (ifaces) { + if (VIR_ALLOC_N(tmp_ifaces, ret.ifaces.ifaces_len + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0; i < ret.ifaces.ifaces_len; i++) { + tmp_ifaces[i] = get_nonnull_interface (conn, ret.ifaces.ifaces_val[i]); + if (!tmp_ifaces[i]) { + virReportOOMError(); + goto cleanup; + } + } + *ifaces = tmp_ifaces; + tmp_ifaces = NULL; + } + + rv = ret.ret; + +cleanup: + if (tmp_ifaces) { + for (i = 0; i < ret.ifaces.ifaces_len; i++) + if (tmp_ifaces[i]) + virInterfaceFree(tmp_ifaces[i]); + VIR_FREE(tmp_ifaces); + }

+ + xdr_free((xdrproc_t) xdr_remote_connect_list_all_interfaces_ret, (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + /*----------------------------------------------------------------------*/ @@ -5783,6 +5846,7 @@ static virInterfaceDriver interface_driver = { .listInterfaces = remoteListInterfaces, /* 0.7.2 */ .numOfDefinedInterfaces = remoteNumOfDefinedInterfaces, /* 0.7.2 */ .listDefinedInterfaces = remoteListDefinedInterfaces, /* 0.7.2 */ + .listAllInterfaces = remoteConnectListAllInterfaces, /* 0.10.0 */

.interfaceLookupByName = remoteInterfaceLookupByName, /* 0.7.2 */ .interfaceLookupByMACString = remoteInterfaceLookupByMACString, /* 0.7.2 */ .interfaceGetXMLDesc = remoteInterfaceGetXMLDesc, /* 0.7.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 12ecfd7..54054af 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2589,6 +2589,16 @@ struct remote_connect_list_all_networks_ret { unsigned int ret; }; +struct remote_connect_list_all_interfaces_args { + int need_results; + unsigned int flags; +}; + +struct remote_connect_list_all_interfaces_ret { + remote_nonnull_interface ifaces<>; + unsigned int ret; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -2922,7 +2932,8 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS = 281, /* skipgen skipgen priority:high */ REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES = 282, /* skipgen skipgen priority:high */ - REMOTE_PROC_CONNECT_LIST_ALL_NETWORKS = 283 /* skipgen skipgen priority:high */ + REMOTE_PROC_CONNECT_LIST_ALL_NETWORKS = 283, /* skipgen skipgen priority:high */ + REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES = 284 /* skipgen skipgen priority:high */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 45262f8..4e41c67 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2045,6 +2045,17 @@ struct remote_list_all_networks_ret { } nets; u_int ret; }; +struct remote_connect_list_all_interfaces_args { + int need_results; + u_int flags; +}; +struct remote_connect_list_all_interfaces_ret { + struct { + u_int ifaces_len; + remote_nonnull_interface * ifaces_val; + } pools; + u_int ret; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2329,4 +2340,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS = 281, REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES = 282, REMOTE_PROC_CONNECT_LIST_ALL_NETWORKS = 283, + REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES = 284, };

This is not that ideal as API for other objects, as it's still O(n).

Because interface driver uses netcf APIs to manage the stuffs, instead of by itself. And netcf APIs don't return a object. It provides APIs like old libvirt APIs: ncf_number_of_interfaces ncf_list_interfaces ncf_lookup_by_name ......

Perhaps we should further hack netcf to let it provide an API

to return the object, but it could be a later patch. And anyway, we will still befinit from the new API for the simplification,

and no race like the old APIs.

src/interface/netcf_driver.c: Implement listAllInterfaces --- src/interface/netcf_driver.c | 135 ++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 135 insertions(+), 0 deletions(-) diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c index 935be66..1dae99b 100644 --- a/src/interface/netcf_driver.c +++ b/src/interface/netcf_driver.c @@ -259,6 +259,140 @@ static int interfaceListDefinedInterfaces(virConnectPtr conn, char **const names } +static int +interfaceListAllInterfaces(virConnectPtr conn, + virInterfacePtr **ifaces, + unsigned int flags) +{ + struct interface_driver *driver = conn->interfacePrivateData; + int count; + int i; + struct netcf_if *iface = NULL; + virInterfacePtr *tmp_iface_objs = NULL; + virInterfacePtr iface_obj = NULL; + unsigned int status; + int niface_objs = 0; + int ret = -1; + char **names; + + virCheckFlags(VIR_CONNECT_LIST_INTERFACES_ACTIVE | + VIR_CONNECT_LIST_INTERFACES_INACTIVE, -1); + + interfaceDriverLock(driver); + + /* List all interfaces, in case of we might support new filter flags + * except active|inactive in future. + */ + count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_ACTIVE | + NETCF_IFACE_INACTIVE); + if (count < 0) { + const char *errmsg, *details; + int errcode = ncf_error(driver->netcf, &errmsg, &details); + virReportError(netcf_to_vir_err(errcode), + _("failed to get number of host interfaces: %s%s%s"), + errmsg, details ? " - " : "", + details ? details : ""); + return -1; + } + + if (count == 0) + return 0; + + if (VIR_ALLOC_N(names, count) < 0) { + virReportOOMError(); + return -1; + }

+ + if ((count = ncf_list_interfaces(driver->netcf, count, names, + NETCF_IFACE_ACTIVE | + NETCF_IFACE_INACTIVE)) < 0) { + const char *errmsg, *details; + int errcode = ncf_error(driver->netcf, &errmsg, &details); + virReportError(netcf_to_vir_err(errcode), + _("failed to list host interfaces: %s%s%s"), + errmsg, details ? " - " : "", + details ? details : ""); + goto cleanup; + } + + if (ifaces) { + if (VIR_ALLOC_N(tmp_iface_objs, count + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + } + + for (i = 0; i < count; i++) { + iface = ncf_lookup_by_name(driver->netcf, names[i]); + if (!iface) { + const char *errmsg, *details; + int errcode = ncf_error(driver->netcf, &errmsg, &details); + if (errcode != NETCF_NOERROR) { + virReportError(netcf_to_vir_err(errcode), + _("couldn't find interface named '%s': %s%s%s"), + names[i], errmsg, + details ? " - " : "", details ? details : ""); + } else { + virReportError(VIR_ERR_NO_INTERFACE, + _("couldn't find interface named '%s'"), names[i]);

+ } + goto cleanup; + } + + if (ncf_if_status(iface, &status) < 0) { + const char *errmsg, *details; + int errcode = ncf_error(driver->netcf, &errmsg, &details); + virReportError(netcf_to_vir_err(errcode), + _("failed to get status of interface %s: %s%s%s"), + names[i], errmsg, details ? " - " : "", + details ? details : ""); + goto cleanup; + } + + /* XXX: Filter the result, need to be splitted once new fitler 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 (tmp_iface_objs) { + /* trim the array to the final size */ + ignore_value(VIR_REALLOC_N(tmp_iface_objs, niface_objs + 1)); + *ifaces = tmp_iface_objs; + tmp_iface_objs = NULL; + } + + ret = niface_objs; + +cleanup: + ncf_if_free(iface); + + for (i = 0; i < count; i++) + VIR_FREE(names[i]); + VIR_FREE(names); + + if (tmp_iface_objs) { + for (i = 0; i < niface_objs; i++) { + if (tmp_iface_objs[i]) + virInterfaceFree(tmp_iface_objs[i]); + } + } + + interfaceDriverUnlock(driver); + return ret; +} + + static virInterfacePtr interfaceLookupByName(virConnectPtr conn, const char *name) { @@ -642,6 +776,7 @@ static virInterfaceDriver interfaceDriver = { .listInterfaces = interfaceListInterfaces, /* 0.7.0 */ .numOfDefinedInterfaces = interfaceNumOfDefinedInterfaces, /* 0.7.0 */ .listDefinedInterfaces = interfaceListDefinedInterfaces, /* 0.7.0 */ + .listAllInterfaces = interfaceListAllInterfaces, /* 0.10.0 */

.interfaceLookupByName = interfaceLookupByName, /* 0.7.0 */ .interfaceLookupByMACString = interfaceLookupByMACString, /* 0.7.0 */ .interfaceGetXMLDesc = interfaceGetXMLDesc, /* 0.7.0 */

tools/virsh-interface.c: * vshInterfaceSorter to sort interfaces by name * vshInterfaceListFree to free the interface objects list. * vshInterfaceListCollect to collect the interface objects, trying to use new API first, fall back to older APIs if it's not supported. --- tools/virsh-interface.c | 257 +++++++++++++++++++++++++++++++++-------------- 1 files changed, 183 insertions(+), 74 deletions(-) diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 28e9d8c..7446ca3 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -131,6 +131,174 @@ cleanup: return ret; } +static int +vshInterfaceSorter(const void *a, const void *b) +{ + virInterfacePtr *ia = (virInterfacePtr *) a; + virInterfacePtr *ib = (virInterfacePtr *) b; + + if (*ia && !*ib) + return -1; + + if (!*ia) + return *ib != NULL; + + return vshStrcasecmp(virInterfaceGetName(*ia), + virInterfaceGetName(*ib)); +} + +struct vshInterfaceList { + virInterfacePtr *ifaces; + size_t nifaces; +}; +typedef struct vshInterfaceList *vshInterfaceListPtr;

+ +static void +vshInterfaceListFree(vshInterfaceListPtr list) +{ + int i; + + if (list && list->nifaces) { + for (i = 0; i < list->nifaces; i++) { + if (list->ifaces[i]) + virInterfaceFree(list->ifaces[i]); + } + VIR_FREE(list->ifaces); + } + VIR_FREE(list); +} + +static vshInterfaceListPtr +vshInterfaceListCollect(vshControl *ctl, + unsigned int flags) +{ + vshInterfaceListPtr list = vshMalloc(ctl, sizeof(*list)); + int i; + int ret; + char **activeNames = NULL; + char **inactiveNames = NULL; + virInterfacePtr iface; + bool success = false; + size_t deleted = 0; + int nActiveIfaces = 0; + int nInactiveIfaces = 0; + int nAllIfaces = 0; + + /* try the list with flags support (0.10.0 and later) */ + if ((ret = virConnectListAllInterfaces(ctl->conn, + &list->ifaces, + flags)) >= 0) { + list->nifaces = ret; + goto finished; + } + + /* check if the command is actually supported */ + if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) { + vshResetLibvirtError(); + goto fallback; + } + + /* there was an error during the first or second call */ + vshError(ctl, "%s", _("Failed to list interfaces")); + goto cleanup; + + +fallback: + /* fall back to old method (0.9.13 and older) */

+ vshResetLibvirtError();

+ + if (flags & VIR_CONNECT_LIST_INTERFACES_ACTIVE) { + nActiveIfaces = virConnectNumOfInterfaces(ctl->conn); + if (nActiveIfaces < 0) { + vshError(ctl, "%s", _("Failed to list active interfaces")); + goto cleanup; + } + if (nActiveIfaces) { + activeNames = vshMalloc(ctl, sizeof(char *) * nActiveIfaces); + + if ((nActiveIfaces = virConnectListInterfaces(ctl->conn, activeNames, + nActiveIfaces)) < 0) { + vshError(ctl, "%s", _("Failed to list active interfaces")); + goto cleanup; + } + } + } + + if (flags & VIR_CONNECT_LIST_INTERFACES_INACTIVE) { + nInactiveIfaces = virConnectNumOfDefinedInterfaces(ctl->conn); + if (nInactiveIfaces < 0) { + vshError(ctl, "%s", _("Failed to list inactive interfaces")); + goto cleanup; + } + if (nInactiveIfaces) { + inactiveNames = vshMalloc(ctl, sizeof(char *) * nInactiveIfaces); + + if ((nInactiveIfaces = + virConnectListDefinedInterfaces(ctl->conn, inactiveNames, + nInactiveIfaces)) < 0) { + vshError(ctl, "%s", _("Failed to list inactive interfaces")); + goto cleanup; + } + } + } + + nAllIfaces = nActiveIfaces + nInactiveIfaces; + if (nAllIfaces == 0) { + VIR_FREE(activeNames); + VIR_FREE(inactiveNames); + return list; + } + + list->ifaces = vshMalloc(ctl, sizeof(virInterfacePtr) * (nAllIfaces)); + list->nifaces = 0; + + /* get active interfaces */ + for (i = 0; i < nActiveIfaces; i++) { + if (!(iface = virInterfaceLookupByName(ctl->conn, activeNames[i])))

+ continue; + list->ifaces[list->nifaces++] = iface; + } + + /* get inactive interfaces */ + for (i = 0; i < nInactiveIfaces; i++) { + if (!(iface = virInterfaceLookupByName(ctl->conn, inactiveNames[i])))

+ continue; + list->ifaces[list->nifaces++] = iface; + } + + /* truncate interfaces that weren't found */ + deleted = nAllIfaces - list->nifaces; + +finished: + /* sort the list */ + if (list->ifaces && list->nifaces) + qsort(list->ifaces, list->nifaces, + sizeof(*list->ifaces), vshInterfaceSorter); + + /* truncate the list if filter simulation deleted entries */ + if (deleted) + VIR_SHRINK_N(list->ifaces, list->nifaces, deleted); + + success = true; + +cleanup: + for (i = 0; i < nActiveIfaces; i++) + VIR_FREE(activeNames[i]); + + for (i = 0; i < nInactiveIfaces; i++) + VIR_FREE(inactiveNames[i]); + + VIR_FREE(activeNames); + VIR_FREE(inactiveNames); + + if (!success) { + vshInterfaceListFree(list); + list = NULL; + } + + return list; +} + /* * "iface-list" command */ @@ -145,99 +313,40 @@ static const vshCmdOptDef opts_interface_list[] = { {"all", VSH_OT_BOOL, 0, N_("list inactive & active interfaces")}, {NULL, 0, 0, NULL} }; + static bool cmdInterfaceList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { bool inactive = vshCommandOptBool(cmd, "inactive"); bool all = vshCommandOptBool(cmd, "all"); - bool active = !inactive || all; - int maxactive = 0, maxinactive = 0, i; - char **activeNames = NULL, **inactiveNames = NULL; - inactive |= all; - - if (active) { - maxactive = virConnectNumOfInterfaces(ctl->conn); - if (maxactive < 0) { - vshError(ctl, "%s", _("Failed to list active interfaces")); - return false; - } - if (maxactive) { - activeNames = vshMalloc(ctl, sizeof(char *) * maxactive); + unsigned int flags = VIR_CONNECT_LIST_INTERFACES_ACTIVE; + vshInterfaceListPtr list = NULL; + int i; - if ((maxactive = virConnectListInterfaces(ctl->conn, activeNames, - maxactive)) < 0) { - vshError(ctl, "%s", _("Failed to list active interfaces")); - VIR_FREE(activeNames); - return false; - } - - qsort(&activeNames[0], maxactive, sizeof(char *), vshNameSorter); - } - } - if (inactive) { - maxinactive = virConnectNumOfDefinedInterfaces(ctl->conn); - if (maxinactive < 0) { - vshError(ctl, "%s", _("Failed to list inactive interfaces")); - VIR_FREE(activeNames); - return false; - } - if (maxinactive) { - inactiveNames = vshMalloc(ctl, sizeof(char *) * maxinactive); + if (inactive) + flags = VIR_CONNECT_LIST_INTERFACES_INACTIVE; + if (all) + flags = VIR_CONNECT_LIST_INTERFACES_INACTIVE | + VIR_CONNECT_LIST_INTERFACES_ACTIVE; - if ((maxinactive = - virConnectListDefinedInterfaces(ctl->conn, inactiveNames, - maxinactive)) < 0) { - vshError(ctl, "%s", _("Failed to list inactive interfaces")); - VIR_FREE(activeNames); - VIR_FREE(inactiveNames); - return false; - } + if (!(list = vshInterfaceListCollect(ctl, flags))) + return false; - qsort(&inactiveNames[0], maxinactive, sizeof(char*), vshNameSorter); - } - } vshPrintExtra(ctl, "%-20s %-10s %s\n", _("Name"), _("State"), _("MAC Address")); vshPrintExtra(ctl, "--------------------------------------------\n"); - for (i = 0; i < maxactive; i++) { - virInterfacePtr iface = - virInterfaceLookupByName(ctl->conn, activeNames[i]); - - /* this kind of work with interfaces is not atomic */ - if (!iface) { - VIR_FREE(activeNames[i]); - continue; - } + for (i = 0; i < list->nifaces; i++) { + virInterfacePtr iface = list->ifaces[i]; vshPrint(ctl, "%-20s %-10s %s\n", virInterfaceGetName(iface), - _("active"), + virInterfaceIsActive(iface) ? _("active") : _("inactive"), virInterfaceGetMACString(iface)); - virInterfaceFree(iface); - VIR_FREE(activeNames[i]); } - for (i = 0; i < maxinactive; i++) { - virInterfacePtr iface = - virInterfaceLookupByName(ctl->conn, inactiveNames[i]); - /* this kind of work with interfaces is not atomic */ - if (!iface) { - VIR_FREE(inactiveNames[i]); - continue; - } - - vshPrint(ctl, "%-20s %-10s %s\n", - virInterfaceGetName(iface), - _("inactive"), - virInterfaceGetMACString(iface)); - virInterfaceFree(iface); - VIR_FREE(inactiveNames[i]); - } - VIR_FREE(activeNames); - VIR_FREE(inactiveNames); + vshInterfaceListFree(list); return true; - } /*

On 09/11/2012 11:37 PM, Osier Yang wrote: > This is not that ideal as API for other objects, as it's still > O(n). Because interface driver uses netcf APIs to manage the > stuffs, instead of by itself. And netcf APIs don't return a object. > It provides APIs like old libvirt APIs: > > ncf_number_of_interfaces > ncf_list_interfaces > ncf_lookup_by_name > ...... > > Perhaps we should further improve netcf to let it provide an API > to return the object, but it could be a later patch. And anyway, > we will still benefit from the new API for the simplification, > and no race like the old APIs. > > src/interface/netcf_driver.c: Implement listAllInterfaces > > v4 - v5: > * Ignore the NETCF_NOERROR, in case of the iface could be deleted > by other management apps as a race. > > * Fix the memory leak caught by Laine. > > * The version number fix. > > --- > src/interface/netcf_driver.c | 143 ++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 143 insertions(+), 0 deletions(-) > > diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c > index 935be66..26d25d7 100644 > --- a/src/interface/netcf_driver.c > +++ b/src/interface/netcf_driver.c > @@ -30,6 +30,7 @@ > #include "netcf_driver.h" > #include "interface_conf.h" > #include "memory.h" > +#include "logging.h" > > #define VIR_FROM_THIS VIR_FROM_INTERFACE > > @@ -259,6 +260,147 @@ static int interfaceListDefinedInterfaces(virConnectPtr conn, char **const names > > } > > +static int > +interfaceListAllInterfaces(virConnectPtr conn, > + virInterfacePtr **ifaces, > + unsigned int flags) > +{ > + struct interface_driver *driver = conn->interfacePrivateData; > + int count; > + int i; > + struct netcf_if *iface = NULL; > + virInterfacePtr *tmp_iface_objs = NULL; > + virInterfacePtr iface_obj = NULL; > + unsigned int status; > + int niface_objs = 0; > + int ret = -1; > + char **names; > + > + virCheckFlags(VIR_CONNECT_LIST_INTERFACES_ACTIVE | > + VIR_CONNECT_LIST_INTERFACES_INACTIVE, -1); > + > + interfaceDriverLock(driver); > + > + /* List all interfaces, in case of we might support new filter flags > + * except active|inactive in future. > + */ > + count = ncf_num_of_interfaces(driver->netcf, NETCF_IFACE_ACTIVE | > + NETCF_IFACE_INACTIVE); > + if (count< 0) { > + const char *errmsg, *details; > + int errcode = ncf_error(driver->netcf,&errmsg,&details); > + virReportError(netcf_to_vir_err(errcode), > + _("failed to get number of host interfaces: %s%s%s"), > + errmsg, details ? " - " : "", > + details ? details : ""); > + return -1; Ooh! I just noticed that you have several return paths here that don't call interfaceDriverUnlock()!! If you initialize names = NULL, then qualify the loop to free names with "if (names) { ... }", you should be able to just to "ret = X; goto cleanup;" instead of return.

tools/virsh-interface.c: * vshInterfaceSorter to sort interfaces by name * vshInterfaceListFree to free the interface objects list. * vshInterfaceListCollect to collect the interface objects, trying to use new API first, fall back to older APIs if it's not supported. v4 - v5: * The version number fix * Remove redudant error reset when goto "fallback" * Add vshResetLibvirtError when virInterfaceLookupByName fails --- tools/virsh-interface.c | 259 +++++++++++++++++++++++++++++++++-------------- 1 files changed, 185 insertions(+), 74 deletions(-) diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 28e9d8c..f2d3897 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -131,6 +131,176 @@ cleanup: return ret; } +static int +vshInterfaceSorter(const void *a, const void *b) +{ + virInterfacePtr *ia = (virInterfacePtr *) a; + virInterfacePtr *ib = (virInterfacePtr *) b; + + if (*ia && !*ib) + return -1; + + if (!*ia) + return *ib != NULL; + + return vshStrcasecmp(virInterfaceGetName(*ia), + virInterfaceGetName(*ib)); +} + +struct vshInterfaceList { + virInterfacePtr *ifaces; + size_t nifaces; +}; +typedef struct vshInterfaceList *vshInterfaceListPtr; + +static void +vshInterfaceListFree(vshInterfaceListPtr list) +{ + int i; + + if (list && list->nifaces) { + for (i = 0; i < list->nifaces; i++) { + if (list->ifaces[i]) + virInterfaceFree(list->ifaces[i]); + } + VIR_FREE(list->ifaces); + } + VIR_FREE(list); +} + +static vshInterfaceListPtr +vshInterfaceListCollect(vshControl *ctl, + unsigned int flags) +{ + vshInterfaceListPtr list = vshMalloc(ctl, sizeof(*list)); + int i; + int ret; + char **activeNames = NULL; + char **inactiveNames = NULL; + virInterfacePtr iface; + bool success = false; + size_t deleted = 0; + int nActiveIfaces = 0; + int nInactiveIfaces = 0; + int nAllIfaces = 0; + + /* try the list with flags support (0.10.2 and later) */ + if ((ret = virConnectListAllInterfaces(ctl->conn, + &list->ifaces, + flags)) >= 0) { + list->nifaces = ret; + goto finished; + } + + /* check if the command is actually supported */ + if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) + goto fallback; + + /* there was an error during the first or second call */ + vshError(ctl, "%s", _("Failed to list interfaces")); + goto cleanup; + + +fallback: + /* fall back to old method (0.10.1 and older) */ + vshResetLibvirtError(); + + if (flags & VIR_CONNECT_LIST_INTERFACES_ACTIVE) { + nActiveIfaces = virConnectNumOfInterfaces(ctl->conn); + if (nActiveIfaces < 0) { + vshError(ctl, "%s", _("Failed to list active interfaces")); + goto cleanup; + } + if (nActiveIfaces) { + activeNames = vshMalloc(ctl, sizeof(char *) * nActiveIfaces); + + if ((nActiveIfaces = virConnectListInterfaces(ctl->conn, activeNames, + nActiveIfaces)) < 0) { + vshError(ctl, "%s", _("Failed to list active interfaces")); + goto cleanup; + } + } + } + + if (flags & VIR_CONNECT_LIST_INTERFACES_INACTIVE) { + nInactiveIfaces = virConnectNumOfDefinedInterfaces(ctl->conn); + if (nInactiveIfaces < 0) { + vshError(ctl, "%s", _("Failed to list inactive interfaces")); + goto cleanup; + } + if (nInactiveIfaces) { + inactiveNames = vshMalloc(ctl, sizeof(char *) * nInactiveIfaces); + + if ((nInactiveIfaces = + virConnectListDefinedInterfaces(ctl->conn, inactiveNames, + nInactiveIfaces)) < 0) { + vshError(ctl, "%s", _("Failed to list inactive interfaces")); + goto cleanup; + } + } + } + + nAllIfaces = nActiveIfaces + nInactiveIfaces; + if (nAllIfaces == 0) { + VIR_FREE(activeNames); + VIR_FREE(inactiveNames); + return list; + } + + list->ifaces = vshMalloc(ctl, sizeof(virInterfacePtr) * (nAllIfaces)); + list->nifaces = 0; + + /* get active interfaces */ + for (i = 0; i < nActiveIfaces; i++) { + if (!(iface = virInterfaceLookupByName(ctl->conn, activeNames[i]))) { + vshResetLibvirtError(); + continue; + } + list->ifaces[list->nifaces++] = iface; + } + + /* get inactive interfaces */ + for (i = 0; i < nInactiveIfaces; i++) { + if (!(iface = virInterfaceLookupByName(ctl->conn, inactiveNames[i]))) { + vshResetLibvirtError(); + continue; + } + list->ifaces[list->nifaces++] = iface; + } + + /* truncate interfaces that weren't found */ + deleted = nAllIfaces - list->nifaces; + +finished: + /* sort the list */ + if (list->ifaces && list->nifaces) + qsort(list->ifaces, list->nifaces, + sizeof(*list->ifaces), vshInterfaceSorter); + + /* truncate the list if filter simulation deleted entries */ + if (deleted) + VIR_SHRINK_N(list->ifaces, list->nifaces, deleted); + + success = true; + +cleanup: + for (i = 0; i < nActiveIfaces; i++) + VIR_FREE(activeNames[i]); + + for (i = 0; i < nInactiveIfaces; i++) + VIR_FREE(inactiveNames[i]); + + VIR_FREE(activeNames); + VIR_FREE(inactiveNames); + + if (!success) { + vshInterfaceListFree(list); + list = NULL; + } + + return list; +} + /* * "iface-list" command */ @@ -145,99 +315,40 @@ static const vshCmdOptDef opts_interface_list[] = { {"all", VSH_OT_BOOL, 0, N_("list inactive & active interfaces")}, {NULL, 0, 0, NULL} }; + static bool cmdInterfaceList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { bool inactive = vshCommandOptBool(cmd, "inactive"); bool all = vshCommandOptBool(cmd, "all"); - bool active = !inactive || all; - int maxactive = 0, maxinactive = 0, i; - char **activeNames = NULL, **inactiveNames = NULL; - inactive |= all; - - if (active) { - maxactive = virConnectNumOfInterfaces(ctl->conn); - if (maxactive < 0) { - vshError(ctl, "%s", _("Failed to list active interfaces")); - return false; - } - if (maxactive) { - activeNames = vshMalloc(ctl, sizeof(char *) * maxactive); - - if ((maxactive = virConnectListInterfaces(ctl->conn, activeNames, - maxactive)) < 0) { - vshError(ctl, "%s", _("Failed to list active interfaces")); - VIR_FREE(activeNames); - return false; - } + unsigned int flags = VIR_CONNECT_LIST_INTERFACES_ACTIVE; + vshInterfaceListPtr list = NULL; + int i; - qsort(&activeNames[0], maxactive, sizeof(char *), vshNameSorter); - } - } - if (inactive) { - maxinactive = virConnectNumOfDefinedInterfaces(ctl->conn); - if (maxinactive < 0) { - vshError(ctl, "%s", _("Failed to list inactive interfaces")); - VIR_FREE(activeNames); - return false; - } - if (maxinactive) { - inactiveNames = vshMalloc(ctl, sizeof(char *) * maxinactive); + if (inactive) + flags = VIR_CONNECT_LIST_INTERFACES_INACTIVE; + if (all) + flags = VIR_CONNECT_LIST_INTERFACES_INACTIVE | + VIR_CONNECT_LIST_INTERFACES_ACTIVE; - if ((maxinactive = - virConnectListDefinedInterfaces(ctl->conn, inactiveNames, - maxinactive)) < 0) { - vshError(ctl, "%s", _("Failed to list inactive interfaces")); - VIR_FREE(activeNames); - VIR_FREE(inactiveNames); - return false; - } + if (!(list = vshInterfaceListCollect(ctl, flags))) + return false; - qsort(&inactiveNames[0], maxinactive, sizeof(char*), vshNameSorter); - } - } vshPrintExtra(ctl, "%-20s %-10s %s\n", _("Name"), _("State"), _("MAC Address")); vshPrintExtra(ctl, "--------------------------------------------\n"); - for (i = 0; i < maxactive; i++) { - virInterfacePtr iface = - virInterfaceLookupByName(ctl->conn, activeNames[i]); - - /* this kind of work with interfaces is not atomic */ - if (!iface) { - VIR_FREE(activeNames[i]); - continue; - } + for (i = 0; i < list->nifaces; i++) { + virInterfacePtr iface = list->ifaces[i]; vshPrint(ctl, "%-20s %-10s %s\n", virInterfaceGetName(iface), - _("active"), + virInterfaceIsActive(iface) ? _("active") : _("inactive"), virInterfaceGetMACString(iface)); - virInterfaceFree(iface); - VIR_FREE(activeNames[i]); } - for (i = 0; i < maxinactive; i++) { - virInterfacePtr iface = - virInterfaceLookupByName(ctl->conn, inactiveNames[i]); - - /* this kind of work with interfaces is not atomic */ - if (!iface) { - VIR_FREE(inactiveNames[i]); - continue; - } - vshPrint(ctl, "%-20s %-10s %s\n", - virInterfaceGetName(iface), - _("inactive"), - virInterfaceGetMACString(iface)); - virInterfaceFree(iface); - VIR_FREE(inactiveNames[i]); - } - VIR_FREE(activeNames); - VIR_FREE(inactiveNames); + vshInterfaceListFree(list); return true; - } /*

The implementation is done manually as the generator does not support wrapping lists of C pointers into Python objects.

python/libvirt-override-api.xml: Document python/libvirt-override-virConnect.py: * New file, includes implementation of listAllInterfaces. python/libvirt-override.c: Implementation for the wrapper. --- python/libvirt-override-api.xml | 6 ++++ python/libvirt-override-virConnect.py | 12 ++++++++ python/libvirt-override.c | 48 +++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 0 deletions(-) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 5f51fc7..ab6f407 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -416,6 +416,12 @@ <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> <return type='str *' info='the list of Names of None in case of error'/> </function> + <function name='virConnectListAllInterfaces' file='python'> + <info>returns list of all interfaces</info> + <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> + <arg name='flags' type='unsigned int' info='optional flags'/> + <return type='interface *' info='the list of interfaces or None in case of error'/> + </function> <function name='virConnectBaselineCPU' file='python'> <info>Computes the most feature-rich CPU which is compatible with all given host CPUs.</info> <return type='char *' info='XML description of the computed CPU or NULL on error.'/> diff --git a/python/libvirt-override-virConnect.py b/python/libvirt-override-virConnect.py index 85db5fe..ffa1a3c 100644 --- a/python/libvirt-override-virConnect.py +++ b/python/libvirt-override-virConnect.py @@ -230,3 +230,15 @@ retlist.append(virNetwork(self, _obj=netptr)) return retlist + + def listAllInterfaces(self, flags): + """Returns a list of interface objects""" + ret = libvirtmod.virConnectListAllInterfaces(self._o, flags) + if ret is None: + raise libvirtError("virConnectListAllInterfaces() failed", conn=self) + + retlist = list() + for ifaceptr in ret: + retlist.append(virInterface(self, _obj=ifaceptr)) + + return retlist diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 3426560..0675545 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3824,6 +3824,53 @@ libvirt_virConnectListDefinedInterfaces(PyObject *self ATTRIBUTE_UNUSED, static PyObject * +libvirt_virConnectListAllInterfaces(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *pyobj_conn; + PyObject *py_retval = NULL; + PyObject *tmp = NULL; + virConnectPtr conn; + virInterfacePtr *ifaces = NULL; + int c_retval = 0; + int i; + unsigned int flags; + + if (!PyArg_ParseTuple(args, (char *)"Oi:virConnectListAllInterfaces", + &pyobj_conn, &flags)) + return NULL; + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virConnectListAllInterfaces(conn, &ifaces, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_NONE; + + if (!(py_retval = PyList_New(c_retval))) + goto cleanup; + + for (i = 0; i < c_retval; i++) { + if (!(tmp = libvirt_virInterfacePtrWrap(ifaces[i])) || + PyList_SetItem(py_retval, i, tmp) < 0) { + Py_XDECREF(tmp); + Py_DECREF(py_retval); + py_retval = NULL; + goto cleanup; + } + /* python steals the pointer */ + ifaces[i] = NULL; + } + +cleanup: + for (i = 0; i < c_retval; i++) + if (ifaces[i]) + virInterfaceFree(ifaces[i]); + VIR_FREE(ifaces); + return py_retval; +} + +static PyObject * libvirt_virConnectBaselineCPU(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *pyobj_conn; @@ -6049,6 +6096,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virConnectListNWFilters", libvirt_virConnectListNWFilters, METH_VARARGS, NULL}, {(char *) "virConnectListInterfaces", libvirt_virConnectListInterfaces, METH_VARARGS, NULL}, {(char *) "virConnectListDefinedInterfaces", libvirt_virConnectListDefinedInterfaces, METH_VARARGS, NULL}, + {(char *) "virConnectListAllInterfaces", libvirt_virConnectListAllInterfaces, METH_VARARGS, NULL}, {(char *) "virConnectBaselineCPU", libvirt_virConnectBaselineCPU, METH_VARARGS, NULL}, {(char *) "virDomainGetJobInfo", libvirt_virDomainGetJobInfo, METH_VARARGS, NULL}, {(char *) "virDomainSnapshotListNames", libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL},