[PATCH] Make NetworkPool use libvirt's virtual network API
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199377368 28800 # Node ID 3a838dfd165b5f721687a2bb0b2e03bb0c8192b7 # Parent 84b72fff34f65ad578d70fcc67b141510e34e24e Make NetworkPool use libvirt's virtual network API This means that you no longer see a NetworkPool for each bridge on the system, but rather one per configured virtual network in libvirt. The id is NetworkPool/NetName, and the Caption property of the instance includes the bridge name for reference. This change removes the need for the contents of hostres.{c,h}, so it is removed here. That makes the net change -85 lines of pure goodness. Changes since last version: - Fixed netpool_member_of() to do extra level of bridge->network mapping Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 84b72fff34f6 -r 3a838dfd165b libxkutil/Makefile.am --- a/libxkutil/Makefile.am Wed Jan 02 09:18:22 2008 -0800 +++ b/libxkutil/Makefile.am Thu Jan 03 08:22:48 2008 -0800 @@ -4,11 +4,11 @@ SUBDIRS = tests CFLAGS += $(CFLAGS_STRICT) -noinst_HEADERS = cs_util.h misc_util.h device_parsing.h hostres.h xmlgen.h +noinst_HEADERS = cs_util.h misc_util.h device_parsing.h xmlgen.h lib_LTLIBRARIES = libxkutil.la AM_LDFLAGS = -lvirt -luuid libxkutil_la_SOURCES = cs_util_instance.c misc_util.c device_parsing.c \ - xmlgen.c hostres.c + xmlgen.c diff -r 84b72fff34f6 -r 3a838dfd165b libxkutil/hostres.c --- a/libxkutil/hostres.c Wed Jan 02 09:18:22 2008 -0800 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,84 +0,0 @@ -/* - * Copyright IBM Corp. 2007 - * - * Authors: - * Dan Smith <danms@us.ibm.com> - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - */ -#include <stdio.h> -#include <stdbool.h> -#include <stdlib.h> -#include <string.h> - -#include <sys/types.h> -#include <sys/stat.h> -#include <dirent.h> - -#include "hostres.h" - -bool is_bridge(const char *name) -{ - char *path = NULL; - int ret; - struct stat s; - - ret = asprintf(&path, "/sys/class/net/%s/bridge", name); - if (ret == -1) - return false; - - if (stat(path, &s) != 0) - return false; - - if (S_ISDIR(s.st_mode)) - return true; - else - return false; -} - -char **list_bridges(void) -{ - char **list = NULL; - DIR *dir = NULL; - struct dirent *de; - int count = 0; - - dir = opendir("/sys/class/net"); - if (dir == NULL) - return NULL; - - while ((de = readdir(dir))) { - if (is_bridge(de->d_name)) { - list = realloc(list, ++count); - list[count-1] = strdup(de->d_name); - } - } - - /* Leave a NULL at the end, no matter what */ - list = realloc(list, count + 1); - list[count] = NULL; - - return list; -} - -/* - * Local Variables: - * mode: C - * c-set-style: "K&R" - * tab-width: 8 - * c-basic-offset: 8 - * indent-tabs-mode: nil - * End: - */ diff -r 84b72fff34f6 -r 3a838dfd165b libxkutil/hostres.h --- a/libxkutil/hostres.h Wed Jan 02 09:18:22 2008 -0800 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,32 +0,0 @@ -/* - * Copyright IBM Corp. 2007 - * - * Authors: - * Dan Smith <danms@us.ibm.com> - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - */ -#ifndef __HOSTRES_H -#define __HOSTRES_H - -#include <stdbool.h> - -bool is_bridge(const char *name); - -/* Return a NULL-terminated array of bridge names on the system. - Result must be free()d, as well as each of the array members */ -char **list_bridges(void); - -#endif diff -r 84b72fff34f6 -r 3a838dfd165b src/Virt_DevicePool.c --- a/src/Virt_DevicePool.c Wed Jan 02 09:18:22 2008 -0800 +++ b/src/Virt_DevicePool.c Thu Jan 03 08:22:48 2008 -0800 @@ -35,7 +35,6 @@ #include "config.h" #include "misc_util.h" -#include "hostres.h" #include "device_parsing.h" #include <libcmpiutil/libcmpiutil.h> @@ -196,6 +195,74 @@ static char *diskpool_member_of(const CM return pool; } +static virNetworkPtr bridge_to_network(virConnectPtr conn, + const char *bridge) +{ + char **networks = NULL; + virNetworkPtr network = NULL; + int num; + int i; + + num = virConnectNumOfNetworks(conn); + if (num < 0) + return NULL; + + networks = calloc(num, sizeof(*networks)); + if (networks == NULL) + return NULL; + + num = virConnectListNetworks(conn, networks, num); + + for (i = 0; i < num; i++) { + char *_bridge; + + network = virNetworkLookupByName(conn, networks[i]); + if (network == NULL) + continue; + + _bridge = virNetworkGetBridgeName(network); + CU_DEBUG("Network `%s' has bridge `%s'", networks[i], _bridge); + if (STREQ(bridge, _bridge)) { + i = num; + } else { + virNetworkFree(network); + network = NULL; + } + + free(_bridge); + } + + free(networks); + + return network; +} + +static char *_netpool_member_of(virConnectPtr conn, + const char *bridge) +{ + virNetworkPtr net = NULL; + const char *netname; + char *pool = NULL; + + net = bridge_to_network(conn, bridge); + if (net == NULL) + goto out; + + netname = virNetworkGetName(net); + if (netname == NULL) + goto out; + + if (asprintf(&pool, "NetworkPool/%s", netname) == -1) + pool = NULL; + + CU_DEBUG("Determined pool: %s (%s, %s)", pool, bridge, netname); + + out: + virNetworkFree(net); + + return pool; +} + static char *netpool_member_of(const CMPIBroker *broker, const char *rasd_id, const char *refcn) @@ -227,11 +294,8 @@ static char *netpool_member_of(const CMP for (i = 0; i < count; i++) { if (STREQ((devs[i].id), dev)) { - ret = asprintf(&result, - "NetworkPool/%s", - devs[i].dev.net.bridge); - if (ret == -1) - result = NULL; + result = _netpool_member_of(conn, + devs[i].dev.net.bridge); break; } } @@ -416,31 +480,54 @@ static CMPIStatus procpool_instance(virC return s; } -static CMPIStatus _netpool_for_bridge(struct inst_list *list, - const char *ns, - const char *bridge, - const char *refcn, - const CMPIBroker *broker) -{ - char *id = NULL; +static CMPIStatus _netpool_for_network(struct inst_list *list, + const char *ns, + virConnectPtr conn, + const char *netname, + const char *refcn, + const CMPIBroker *broker) +{ + char *str = NULL; + char *bridge = NULL; uint16_t type = CIM_RASD_TYPE_NET; CMPIInstance *inst; + virNetworkPtr network = NULL; + + CU_DEBUG("Looking up network `%s'", netname); + network = virNetworkLookupByName(conn, netname); + if (network == NULL) { + CMPIStatus s; + + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "No such NetworkPool: %s", netname); + return s; + } inst = get_typed_instance(broker, refcn, "NetworkPool", ns); - if (asprintf(&id, "NetworkPool/%s", bridge) == -1) + if (asprintf(&str, "NetworkPool/%s", netname) == -1) return (CMPIStatus){CMPI_RC_ERR_FAILED, NULL}; CMSetProperty(inst, "InstanceID", - (CMPIValue *)id, CMPI_chars); + (CMPIValue *)str, CMPI_chars); + free(str); + + bridge = virNetworkGetBridgeName(network); + if (asprintf(&str, "Bridge: %s", bridge) == -1) + return (CMPIStatus){CMPI_RC_ERR_FAILED, NULL}; + + CMSetProperty(inst, "Caption", + (CMPIValue *)str, CMPI_chars); + free(str); + free(bridge); CMSetProperty(inst, "ResourceType", (CMPIValue *)&type, CMPI_uint16); - free(id); inst_list_add(list, inst); @@ -454,39 +541,49 @@ static CMPIStatus netpool_instance(virCo const CMPIBroker *broker) { CMPIStatus s = {CMPI_RC_OK, NULL}; - char **bridges; + char **netnames = NULL; int i; + int nets; if (id != NULL) { - if (!is_bridge(id)) { - cu_statusf(broker, &s, - CMPI_RC_ERR_FAILED, - "No such network pool `%s'", id); - goto out; - } - return _netpool_for_bridge(list, - ns, - id, - pfx_from_conn(conn), - broker); - } - - bridges = list_bridges(); - if (bridges == NULL) - return (CMPIStatus){CMPI_RC_ERR_FAILED, NULL}; - - for (i = 0; bridges[i]; i++) { - _netpool_for_bridge(list, - ns, - bridges[i], - pfx_from_conn(conn), - broker); - free(bridges[i]); - } - - free(bridges); - - out: + return _netpool_for_network(list, + ns, + conn, + id, + pfx_from_conn(conn), + broker); + } + + nets = virConnectNumOfNetworks(conn); + if (nets < 0) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Unable to list networks"); + goto out; + } + + netnames = calloc(nets, sizeof(*netnames)); + if (netnames == NULL) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Failed to allocate memory for %i net names", nets); + goto out; + } + + nets = virConnectListNetworks(conn, netnames, nets); + + for (i = 0; i < nets; i++) { + _netpool_for_network(list, + ns, + conn, + netnames[i], + pfx_from_conn(conn), + broker); + } + + out: + free(netnames); + return s; }
Dan Smith wrote:
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1199377368 28800 # Node ID 3a838dfd165b5f721687a2bb0b2e03bb0c8192b7 # Parent 84b72fff34f65ad578d70fcc67b141510e34e24e Make NetworkPool use libvirt's virtual network API
I ran into one issue testing, but it seems like it might be a libvirt issue. I was testing with a defined guest and saw the following debug output: No bridge, taking default of `xenbr0' Virt_DevicePool.c(224): Network `default' has bridge `virbr0' libvir: Network error : invalid network pointer in virNetworkFree It looks like virNetworkFree() doesn't handle NULL pointers. I think most of the other libvirt functions do though. Otherwise, this patch looks good. -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
KR> It looks like virNetworkFree() doesn't handle NULL pointers. I think KR> most of the other libvirt functions do though. Actually, it looks like it does, it just complains about it. For some reason, virConnectClose() quietly returns on a bad pointer, but virDomainFree() and virNetworkFree() both complain. I'm not sure it's a big deal, or worth making free conditional on network!=NULL. I imagine at some point we'll wrap the libvirt error handler, which would allow us to hide this warning. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
KR> It looks like virNetworkFree() doesn't handle NULL pointers. I think KR> most of the other libvirt functions do though.
Actually, it looks like it does, it just complains about it. For some reason, virConnectClose() quietly returns on a bad pointer, but virDomainFree() and virNetworkFree() both complain. I'm not sure it's a big deal, or worth making free conditional on network!=NULL. I imagine at some point we'll wrap the libvirt error handler, which would allow us to hide this warning.
Oh, interesting. Thanks for taking a look. It's surprising, because it doesn't seem like any of the other Free() functions return this kind of warning. Either that, or I'm not paying enough attention. =) Thanks!
participants (2)
-
Dan Smith -
Kaitlin Rupert