[libvirt] [PATCH 4/6 v5] list: Implement listAllInterfaces

[Osier Yang]

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