On 2012年09月12日 14:09, Laine Stump wrote:
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.
Oops, now pushed the whole set with this fixed. Thanks for the quick reviewing! Regards, Osier -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list