[libvirt] PATCH: 0 of 5: Remove all linked lists

I've been doing some proof of concept work to make the libvirtd daemon multi-threaded, and this in turns mean that the QEMU / LXC / OpenVZ drivers need to have some degree of locking on their internal data structures. Unforatunately the internal domain/network/storage APIs make extensive use of linked lists for tracking objects. This makes it incredibly hard to do fine grained locking of individual objects - eg, deleting one object requires locking the object it points to, and one the pointing to it in the linked list, as well as the list itself. This would be so complex we'd inevitably introduce locking bugs. So the series of patches which follow replace all linked lists with explicitly sized arrays. This means I only need 2 levels of locks, one on the driver object, and one of the domain/network/storage object being manipulated. The semantics for this are nice & easy to define. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Due to historical architectural restrictions the virtual network driver is actually part of the QEMU driver. The restrictions have long ago been removed, so its overdue to move the network driver out of the QEMU driver. This patch does this. It is a straight re-factoring with no functional change. The only minor pain point is that the network driver stores it config files under /etc/libvirt/qemu/networks, when ideally it would be in /etc/libvirt/networks. This patch leaves the config files where they already are, but I'm not too happy about that. I'd like to move them to the correct location and just leave a back-compatability symlink or something like that. b/src/network_driver.c | 1193 +++++++++++++++++++++++++++++++++++++++++++++++++ b/src/network_driver.h | 34 + configure.in | 12 src/Makefile.am | 8 src/libvirt.c | 6 src/qemu_conf.c | 15 src/qemu_conf.h | 5 src/qemu_driver.c | 965 --------------------------------------- 8 files changed, 1261 insertions(+), 977 deletions(-) Daniel diff -r f68f3fd13e9d configure.in --- a/configure.in Fri Oct 03 00:07:33 2008 +0100 +++ b/configure.in Fri Oct 03 10:46:41 2008 +0100 @@ -643,6 +643,17 @@ AC_SUBST([LIBVIRT_FEATURES]) +AC_ARG_WITH([network], +[ --with-network with virtual network driver (on)],[],[with_network=yes]) +if test "$with_libvirtd" = "no" ; then + with_network=no +fi +if test "$with_network" = "yes" ; then + AC_DEFINE_UNQUOTED([WITH_NETWORK], 1, [whether network driver is enabled]) +fi +AM_CONDITIONAL([WITH_NETWORK], [test "$with_network" = "yes"]) + + dnl dnl Storage driver checks dnl @@ -1070,6 +1081,7 @@ AC_MSG_NOTICE([ LXC: $with_lxc]) AC_MSG_NOTICE([ Test: $with_test]) AC_MSG_NOTICE([ Remote: $with_remote]) +AC_MSG_NOTICE([ Network: $with_network]) AC_MSG_NOTICE([Libvirtd: $with_libvirtd]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Storage Drivers]) diff -r f68f3fd13e9d src/Makefile.am --- a/src/Makefile.am Fri Oct 03 00:07:33 2008 +0100 +++ b/src/Makefile.am Fri Oct 03 10:46:41 2008 +0100 @@ -102,11 +102,12 @@ openvz_conf.c openvz_conf.h \ openvz_driver.c openvz_driver.h -# XXX network driver should be split out of this QEMU_DRIVER_SOURCES = \ qemu_conf.c qemu_conf.h \ qemu_driver.c qemu_driver.h +NETWORK_DRIVER_SOURCES = \ + network_driver.h network_driver.c # And finally storage backend specific impls STORAGE_DRIVER_SOURCES = \ @@ -163,9 +164,11 @@ libvirt_la_SOURCES += $(OPENVZ_DRIVER_SOURCES) endif - # Drivers usable inside daemon context if WITH_LIBVIRTD +if WITH_NETWORK +libvirt_la_SOURCES += $(NETWORK_DRIVER_SOURCES) +endif libvirt_la_SOURCES += $(STORAGE_DRIVER_SOURCES) libvirt_la_SOURCES += $(STORAGE_DRIVER_FS_SOURCES) @@ -198,6 +201,7 @@ $(QEMU_DRIVER_SOURCES) \ $(LXC_DRIVER_SOURCES) \ $(OPENVZ_DRIVER_SOURCES) \ + $(NETWORK_DRIVER_SOURCES) \ $(STORAGE_DRIVER_SOURCES) \ $(STORAGE_DRIVER_FS_SOURCES) \ $(STORAGE_DRIVER_LVM_SOURCES) \ diff -r f68f3fd13e9d src/libvirt.c --- a/src/libvirt.c Fri Oct 03 00:07:33 2008 +0100 +++ b/src/libvirt.c Fri Oct 03 10:46:41 2008 +0100 @@ -57,6 +57,9 @@ #include "lxc_driver.h" #endif #include "storage_driver.h" +#ifdef WITH_NETWORK +#include "network_driver.h" +#endif /* * TODO: @@ -295,6 +298,9 @@ #endif #ifdef WITH_LXC if (lxcRegister() == -1) return -1; +#endif +#ifdef WITH_NETWORK + if (networkRegister() == -1) return -1; #endif if (storageRegister() == -1) return -1; #ifdef WITH_REMOTE diff -r f68f3fd13e9d src/network_driver.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/network_driver.c Fri Oct 03 10:46:41 2008 +0100 @@ -0,0 +1,1193 @@ +/* + * driver.c: core driver methods for managing qemu guests + * + * Copyright (C) 2006, 2007, 2008 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * 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 + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include <sys/types.h> +#include <sys/poll.h> +#include <dirent.h> +#include <limits.h> +#include <string.h> +#include <stdio.h> +#include <strings.h> +#include <stdarg.h> +#include <stdlib.h> +#include <unistd.h> +#include <errno.h> +#include <sys/utsname.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <signal.h> +#include <paths.h> +#include <pwd.h> +#include <stdio.h> +#include <sys/wait.h> +#include <sys/ioctl.h> + +#include "network_driver.h" +#include "network_conf.h" +#include "driver.h" +#include "c-ctype.h" +#include "event.h" +#include "buf.h" +#include "util.h" +#include "memory.h" +#include "uuid.h" +#include "iptables.h" +#include "bridge.h" + +/* Main driver state */ +struct network_driver { + virNetworkObjPtr networks; + + iptablesContext *iptables; + brControl *brctl; + char *networkConfigDir; + char *networkAutostartDir; + char *logDir; +}; + +static int networkShutdown(void); + +/* networkDebug statements should be changed to use this macro instead. */ +#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__) +#define DEBUG0(msg) VIR_DEBUG(__FILE__, "%s", msg) + +#define networkLog(level, msg...) fprintf(stderr, msg) + +void networkReportError(virConnectPtr conn, + virDomainPtr dom, + virNetworkPtr net, + int code, const char *fmt, ...) + ATTRIBUTE_FORMAT(printf,5,6); + +void networkReportError(virConnectPtr conn, + virDomainPtr dom, + virNetworkPtr net, + int code, const char *fmt, ...) { + va_list args; + char errorMessage[1024]; + const char *virerr; + + if (fmt) { + va_start(args, fmt); + vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args); + va_end(args); + } else { + errorMessage[0] = '\0'; + } + + virerr = __virErrorMsg(code, (errorMessage[0] ? errorMessage : NULL)); + __virRaiseError(conn, dom, net, VIR_FROM_QEMU, code, VIR_ERR_ERROR, + virerr, errorMessage, NULL, -1, -1, virerr, errorMessage); +} + + +static int networkStartNetworkDaemon(virConnectPtr conn, + struct network_driver *driver, + virNetworkObjPtr network); + +static int networkShutdownNetworkDaemon(virConnectPtr conn, + struct network_driver *driver, + virNetworkObjPtr network); + +static struct network_driver *driverState = NULL; + + +static +void networkAutostartConfigs(struct network_driver *driver) { + virNetworkObjPtr network; + + network = driver->networks; + while (network != NULL) { + virNetworkObjPtr next = network->next; + + if (network->autostart && + !virNetworkIsActive(network) && + networkStartNetworkDaemon(NULL, driver, network) < 0) { + virErrorPtr err = virGetLastError(); + networkLog(NETWORK_ERR, _("Failed to autostart network '%s': %s\n"), + network->def->name, err->message); + } + + network = next; + } +} + +/** + * networkStartup: + * + * Initialization function for the QEmu daemon + */ +static int +networkStartup(void) { + uid_t uid = geteuid(); + struct passwd *pw; + char *base = NULL; + + if (VIR_ALLOC(driverState) < 0) + return -1; + + if (!uid) { + if (asprintf(&driverState->logDir, + "%s/log/libvirt/qemu", LOCAL_STATE_DIR) == -1) + goto out_of_memory; + + if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) + goto out_of_memory; + } else { + if (!(pw = getpwuid(uid))) { + networkLog(NETWORK_ERR, _("Failed to find user record for uid '%d': %s\n"), + uid, strerror(errno)); + goto out_of_memory; + } + + if (asprintf(&driverState->logDir, + "%s/.libvirt/qemu/log", pw->pw_dir) == -1) + goto out_of_memory; + + if (asprintf (&base, "%s/.libvirt", pw->pw_dir) == -1) { + networkLog (NETWORK_ERR, + "%s", _("out of memory in asprintf\n")); + goto out_of_memory; + } + } + + /* Configuration paths are either ~/.libvirt/qemu/... (session) or + * /etc/libvirt/qemu/... (system). + */ + if (asprintf (&driverState->networkConfigDir, "%s/qemu/networks", base) == -1) + goto out_of_memory; + + if (asprintf (&driverState->networkAutostartDir, "%s/qemu/networks/autostart", + base) == -1) + goto out_of_memory; + + VIR_FREE(base); + + if (virNetworkLoadAllConfigs(NULL, + &driverState->networks, + driverState->networkConfigDir, + driverState->networkAutostartDir) < 0) { + networkShutdown(); + return -1; + } + networkAutostartConfigs(driverState); + + return 0; + + out_of_memory: + networkLog (NETWORK_ERR, + "%s", _("networkStartup: out of memory\n")); + VIR_FREE(base); + VIR_FREE(driverState); + return -1; +} + +/** + * networkReload: + * + * Function to restart the QEmu daemon, it will recheck the configuration + * files and update its state and the networking + */ +static int +networkReload(void) { + virNetworkLoadAllConfigs(NULL, + &driverState->networks, + driverState->networkConfigDir, + driverState->networkAutostartDir); + + if (driverState->iptables) { + networkLog(NETWORK_INFO, + "%s", _("Reloading iptables rules\n")); + iptablesReloadRules(driverState->iptables); + } + + networkAutostartConfigs(driverState); + + return 0; +} + +/** + * networkActive: + * + * Checks if the QEmu daemon is active, i.e. has an active domain or + * an active network + * + * Returns 1 if active, 0 otherwise + */ +static int +networkActive(void) { + virNetworkObjPtr net = driverState->networks; + + while (net) { + if (virNetworkIsActive(net)) + return 1; + net = net->next; + } + + /* Otherwise we're happy to deal with a shutdown */ + return 0; +} + +/** + * networkShutdown: + * + * Shutdown the QEmu daemon, it will stop all active domains and networks + */ +static int +networkShutdown(void) { + virNetworkObjPtr network; + + if (!driverState) + return -1; + + /* shutdown active networks */ + network = driverState->networks; + while (network) { + virNetworkObjPtr next = network->next; + if (virNetworkIsActive(network)) + networkShutdownNetworkDaemon(NULL, driverState, network); + network = next; + } + + /* free inactive networks */ + network = driverState->networks; + while (network) { + virNetworkObjPtr next = network->next; + virNetworkObjFree(network); + network = next; + } + driverState->networks = NULL; + + VIR_FREE(driverState->logDir); + VIR_FREE(driverState->networkConfigDir); + VIR_FREE(driverState->networkAutostartDir); + + if (driverState->brctl) + brShutdown(driverState->brctl); + if (driverState->iptables) + iptablesContextFree(driverState->iptables); + + VIR_FREE(driverState); + + return 0; +} + + +static int +networkBuildDnsmasqArgv(virConnectPtr conn, + virNetworkObjPtr network, + const char ***argv) { + int i, len, r; + char buf[PATH_MAX]; + + len = + 1 + /* dnsmasq */ + 1 + /* --keep-in-foreground */ + 1 + /* --strict-order */ + 1 + /* --bind-interfaces */ + (network->def->domain?2:0) + /* --domain name */ + 2 + /* --pid-file "" */ + 2 + /* --conf-file "" */ + /*2 + *//* --interface virbr0 */ + 2 + /* --except-interface lo */ + 2 + /* --listen-address 10.0.0.1 */ + 1 + /* --dhcp-leasefile=path */ + (2 * network->def->nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ + /* --dhcp-host 01:23:45:67:89:0a,hostname,10.0.0.3 */ + (2 * network->def->nhosts) + + 1; /* NULL */ + + if (VIR_ALLOC_N(*argv, len) < 0) + goto no_memory; + +#define APPEND_ARG(v, n, s) do { \ + if (!((v)[(n)] = strdup(s))) \ + goto no_memory; \ + } while (0) + + i = 0; + + APPEND_ARG(*argv, i++, DNSMASQ); + + APPEND_ARG(*argv, i++, "--keep-in-foreground"); + /* + * Needed to ensure dnsmasq uses same algorithm for processing + * multiple namedriver entries in /etc/resolv.conf as GLibC. + */ + APPEND_ARG(*argv, i++, "--strict-order"); + APPEND_ARG(*argv, i++, "--bind-interfaces"); + + if (network->def->domain) { + APPEND_ARG(*argv, i++, "--domain"); + APPEND_ARG(*argv, i++, network->def->domain); + } + + APPEND_ARG(*argv, i++, "--pid-file"); + APPEND_ARG(*argv, i++, ""); + + APPEND_ARG(*argv, i++, "--conf-file"); + APPEND_ARG(*argv, i++, ""); + + /* + * XXX does not actually work, due to some kind of + * race condition setting up ipv6 addresses on the + * interface. A sleep(10) makes it work, but that's + * clearly not practical + * + * APPEND_ARG(*argv, i++, "--interface"); + * APPEND_ARG(*argv, i++, network->def->bridge); + */ + APPEND_ARG(*argv, i++, "--listen-address"); + APPEND_ARG(*argv, i++, network->def->ipAddress); + + APPEND_ARG(*argv, i++, "--except-interface"); + APPEND_ARG(*argv, i++, "lo"); + + /* + * NB, dnsmasq command line arg bug means we need to + * use a single arg '--dhcp-leasefile=path' rather than + * two separate args in '--dhcp-leasefile path' style + */ + snprintf(buf, sizeof(buf), "--dhcp-leasefile=%s/lib/libvirt/dhcp-%s.leases", + LOCAL_STATE_DIR, network->def->name); + APPEND_ARG(*argv, i++, buf); + + for (r = 0 ; r < network->def->nranges ; r++) { + snprintf(buf, sizeof(buf), "%s,%s", + network->def->ranges[r].start, + network->def->ranges[r].end); + + APPEND_ARG(*argv, i++, "--dhcp-range"); + APPEND_ARG(*argv, i++, buf); + } + + for (r = 0 ; r < network->def->nhosts ; r++) { + virNetworkDHCPHostDefPtr host = &(network->def->hosts[r]); + if ((host->mac) && (host->name)) { + snprintf(buf, sizeof(buf), "%s,%s,%s", + host->mac, host->name, host->ip); + } else if (host->mac) { + snprintf(buf, sizeof(buf), "%s,%s", + host->mac, host->ip); + } else if (host->name) { + snprintf(buf, sizeof(buf), "%s,%s", + host->name, host->ip); + } else + continue; + + APPEND_ARG(*argv, i++, "--dhcp-host"); + APPEND_ARG(*argv, i++, buf); + } + +#undef APPEND_ARG + + return 0; + + no_memory: + if (argv) { + for (i = 0; (*argv)[i]; i++) + VIR_FREE((*argv)[i]); + VIR_FREE(*argv); + } + networkReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, + "%s", _("failed to allocate space for dnsmasq argv")); + return -1; +} + + +static int +dhcpStartDhcpDaemon(virConnectPtr conn, + virNetworkObjPtr network) +{ + const char **argv; + int ret, i; + + if (network->def->ipAddress == NULL) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot start dhcp daemon without IP address for server")); + return -1; + } + + argv = NULL; + if (networkBuildDnsmasqArgv(conn, network, &argv) < 0) + return -1; + + ret = virExec(conn, argv, NULL, NULL, + &network->dnsmasqPid, -1, NULL, NULL, VIR_EXEC_NONBLOCK); + + for (i = 0; argv[i]; i++) + VIR_FREE(argv[i]); + VIR_FREE(argv); + + return ret; +} + +static int +networkAddMasqueradingIptablesRules(virConnectPtr conn, + struct network_driver *driver, + virNetworkObjPtr network) { + int err; + /* allow forwarding packets from the bridge interface */ + if ((err = iptablesAddForwardAllowOut(driver->iptables, + network->def->network, + network->def->bridge, + network->def->forwardDev))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to allow forwarding from '%s' : %s\n"), + network->def->bridge, strerror(err)); + goto masqerr1; + } + + /* allow forwarding packets to the bridge interface if they are part of an existing connection */ + if ((err = iptablesAddForwardAllowRelatedIn(driver->iptables, + network->def->network, + network->def->bridge, + network->def->forwardDev))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to allow forwarding to '%s' : %s\n"), + network->def->bridge, strerror(err)); + goto masqerr2; + } + + /* enable masquerading */ + if ((err = iptablesAddForwardMasquerade(driver->iptables, + network->def->network, + network->def->forwardDev))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to enable masquerading : %s\n"), + strerror(err)); + goto masqerr3; + } + + return 1; + + masqerr3: + iptablesRemoveForwardAllowRelatedIn(driver->iptables, + network->def->network, + network->def->bridge, + network->def->forwardDev); + masqerr2: + iptablesRemoveForwardAllowOut(driver->iptables, + network->def->network, + network->def->bridge, + network->def->forwardDev); + masqerr1: + return 0; +} + +static int +networkAddRoutingIptablesRules(virConnectPtr conn, + struct network_driver *driver, + virNetworkObjPtr network) { + int err; + /* allow routing packets from the bridge interface */ + if ((err = iptablesAddForwardAllowOut(driver->iptables, + network->def->network, + network->def->bridge, + network->def->forwardDev))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to allow routing from '%s' : %s\n"), + network->def->bridge, strerror(err)); + goto routeerr1; + } + + /* allow routing packets to the bridge interface */ + if ((err = iptablesAddForwardAllowIn(driver->iptables, + network->def->network, + network->def->bridge, + network->def->forwardDev))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to allow routing to '%s' : %s\n"), + network->def->bridge, strerror(err)); + goto routeerr2; + } + + return 1; + + + routeerr2: + iptablesRemoveForwardAllowOut(driver->iptables, + network->def->network, + network->def->bridge, + network->def->forwardDev); + routeerr1: + return 0; +} + +static int +networkAddIptablesRules(virConnectPtr conn, + struct network_driver *driver, + virNetworkObjPtr network) { + int err; + + if (!driver->iptables && !(driver->iptables = iptablesContextNew())) { + networkReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, + "%s", _("failed to allocate space for IP tables support")); + return 0; + } + + + /* allow DHCP requests through to dnsmasq */ + if ((err = iptablesAddTcpInput(driver->iptables, network->def->bridge, 67))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to allow DHCP requests from '%s' : %s"), + network->def->bridge, strerror(err)); + goto err1; + } + + if ((err = iptablesAddUdpInput(driver->iptables, network->def->bridge, 67))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to allow DHCP requests from '%s' : %s"), + network->def->bridge, strerror(err)); + goto err2; + } + + /* allow DNS requests through to dnsmasq */ + if ((err = iptablesAddTcpInput(driver->iptables, network->def->bridge, 53))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to allow DNS requests from '%s' : %s"), + network->def->bridge, strerror(err)); + goto err3; + } + + if ((err = iptablesAddUdpInput(driver->iptables, network->def->bridge, 53))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to allow DNS requests from '%s' : %s"), + network->def->bridge, strerror(err)); + goto err4; + } + + + /* Catch all rules to block forwarding to/from bridges */ + + if ((err = iptablesAddForwardRejectOut(driver->iptables, network->def->bridge))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to block outbound traffic from '%s' : %s"), + network->def->bridge, strerror(err)); + goto err5; + } + + if ((err = iptablesAddForwardRejectIn(driver->iptables, network->def->bridge))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to block inbound traffic to '%s' : %s"), + network->def->bridge, strerror(err)); + goto err6; + } + + /* Allow traffic between guests on the same bridge */ + if ((err = iptablesAddForwardAllowCross(driver->iptables, network->def->bridge))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to add iptables rule to allow cross bridge traffic on '%s' : %s"), + network->def->bridge, strerror(err)); + goto err7; + } + + + /* If masquerading is enabled, set up the rules*/ + if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT && + !networkAddMasqueradingIptablesRules(conn, driver, network)) + goto err8; + /* else if routing is enabled, set up the rules*/ + else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE && + !networkAddRoutingIptablesRules(conn, driver, network)) + goto err8; + + iptablesSaveRules(driver->iptables); + + return 1; + + err8: + iptablesRemoveForwardAllowCross(driver->iptables, + network->def->bridge); + err7: + iptablesRemoveForwardRejectIn(driver->iptables, + network->def->bridge); + err6: + iptablesRemoveForwardRejectOut(driver->iptables, + network->def->bridge); + err5: + iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53); + err4: + iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53); + err3: + iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 67); + err2: + iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67); + err1: + return 0; +} + +static void +networkRemoveIptablesRules(struct network_driver *driver, + virNetworkObjPtr network) { + if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) { + iptablesRemoveForwardMasquerade(driver->iptables, + network->def->network, + network->def->forwardDev); + + if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) + iptablesRemoveForwardAllowRelatedIn(driver->iptables, + network->def->network, + network->def->bridge, + network->def->forwardDev); + else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) + iptablesRemoveForwardAllowIn(driver->iptables, + network->def->network, + network->def->bridge, + network->def->forwardDev); + + iptablesRemoveForwardAllowOut(driver->iptables, + network->def->network, + network->def->bridge, + network->def->forwardDev); + } + iptablesRemoveForwardAllowCross(driver->iptables, network->def->bridge); + iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge); + iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge); + iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53); + iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53); + iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 67); + iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67); + iptablesSaveRules(driver->iptables); +} + +static int +networkEnableIpForwarding(void) +{ +#define PROC_IP_FORWARD "/proc/sys/net/ipv4/ip_forward" + + int fd, ret; + + if ((fd = open(PROC_IP_FORWARD, O_WRONLY|O_TRUNC)) == -1) + return 0; + + if (safewrite(fd, "1\n", 2) < 0) + ret = 0; + + close (fd); + + return 1; + +#undef PROC_IP_FORWARD +} + +static int networkStartNetworkDaemon(virConnectPtr conn, + struct network_driver *driver, + virNetworkObjPtr network) { + int err; + + if (virNetworkIsActive(network)) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("network is already active")); + return -1; + } + + if (!driver->brctl && (err = brInit(&driver->brctl))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot initialize bridge support: %s"), strerror(err)); + return -1; + } + + if ((err = brAddBridge(driver->brctl, &network->def->bridge))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot create bridge '%s' : %s"), + network->def->bridge, strerror(err)); + return -1; + } + + + if (brSetForwardDelay(driver->brctl, network->def->bridge, network->def->delay) < 0) + goto err_delbr; + + if (brSetEnableSTP(driver->brctl, network->def->bridge, network->def->stp ? 1 : 0) < 0) + goto err_delbr; + + if (network->def->ipAddress && + (err = brSetInetAddress(driver->brctl, network->def->bridge, network->def->ipAddress))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot set IP address on bridge '%s' to '%s' : %s"), + network->def->bridge, network->def->ipAddress, strerror(err)); + goto err_delbr; + } + + if (network->def->netmask && + (err = brSetInetNetmask(driver->brctl, network->def->bridge, network->def->netmask))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot set netmask on bridge '%s' to '%s' : %s"), + network->def->bridge, network->def->netmask, strerror(err)); + goto err_delbr; + } + + if (network->def->ipAddress && + (err = brSetInterfaceUp(driver->brctl, network->def->bridge, 1))) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to bring the bridge '%s' up : %s"), + network->def->bridge, strerror(err)); + goto err_delbr; + } + + if (!networkAddIptablesRules(conn, driver, network)) + goto err_delbr1; + + if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE && + !networkEnableIpForwarding()) { + networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to enable IP forwarding : %s"), strerror(err)); + goto err_delbr2; + } + + if (network->def->nranges && + dhcpStartDhcpDaemon(conn, network) < 0) + goto err_delbr2; + + network->active = 1; + + return 0; + + err_delbr2: + networkRemoveIptablesRules(driver, network); + + err_delbr1: + if (network->def->ipAddress && + (err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) { + networkLog(NETWORK_WARN, _("Failed to bring down bridge '%s' : %s\n"), + network->def->bridge, strerror(err)); + } + + err_delbr: + if ((err = brDeleteBridge(driver->brctl, network->def->bridge))) { + networkLog(NETWORK_WARN, _("Failed to delete bridge '%s' : %s\n"), + network->def->bridge, strerror(err)); + } + + return -1; +} + + +static int networkShutdownNetworkDaemon(virConnectPtr conn ATTRIBUTE_UNUSED, + struct network_driver *driver, + virNetworkObjPtr network) { + int err; + + networkLog(NETWORK_INFO, _("Shutting down network '%s'\n"), network->def->name); + + if (!virNetworkIsActive(network)) + return 0; + + if (network->dnsmasqPid > 0) + kill(network->dnsmasqPid, SIGTERM); + + networkRemoveIptablesRules(driver, network); + + if (network->def->ipAddress && + (err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) { + networkLog(NETWORK_WARN, _("Failed to bring down bridge '%s' : %s\n"), + network->def->bridge, strerror(err)); + } + + if ((err = brDeleteBridge(driver->brctl, network->def->bridge))) { + networkLog(NETWORK_WARN, _("Failed to delete bridge '%s' : %s\n"), + network->def->bridge, strerror(err)); + } + + if (network->dnsmasqPid > 0 && + waitpid(network->dnsmasqPid, NULL, WNOHANG) != network->dnsmasqPid) { + kill(network->dnsmasqPid, SIGKILL); + if (waitpid(network->dnsmasqPid, NULL, 0) != network->dnsmasqPid) + networkLog(NETWORK_WARN, + "%s", _("Got unexpected pid for dnsmasq\n")); + } + + network->dnsmasqPid = -1; + network->active = 0; + + if (network->newDef) { + virNetworkDefFree(network->def); + network->def = network->newDef; + network->newDef = NULL; + } + + if (!network->configFile) + virNetworkRemoveInactive(&driver->networks, + network); + + return 0; +} + + +static virNetworkPtr networkLookupByUUID(virConnectPtr conn ATTRIBUTE_UNUSED, + const unsigned char *uuid) { + struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; + virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, uuid); + virNetworkPtr net; + + if (!network) { + networkReportError(conn, NULL, NULL, VIR_ERR_NO_NETWORK, + "%s", _("no network with matching uuid")); + return NULL; + } + + net = virGetNetwork(conn, network->def->name, network->def->uuid); + return net; +} +static virNetworkPtr networkLookupByName(virConnectPtr conn ATTRIBUTE_UNUSED, + const char *name) { + struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; + virNetworkObjPtr network = virNetworkFindByName(driver->networks, name); + virNetworkPtr net; + + if (!network) { + networkReportError(conn, NULL, NULL, VIR_ERR_NO_NETWORK, + "%s", _("no network with matching name")); + return NULL; + } + + net = virGetNetwork(conn, network->def->name, network->def->uuid); + return net; +} + +static virDrvOpenStatus networkOpenNetwork(virConnectPtr conn, + xmlURIPtr uri ATTRIBUTE_UNUSED, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + int flags ATTRIBUTE_UNUSED) { + if (!driverState) + return VIR_DRV_OPEN_DECLINED; + + conn->networkPrivateData = driverState; + return VIR_DRV_OPEN_SUCCESS; +} + +static int networkCloseNetwork(virConnectPtr conn) { + conn->networkPrivateData = NULL; + return 0; +} + +static int networkNumNetworks(virConnectPtr conn) { + int nactive = 0; + struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; + virNetworkObjPtr net = driver->networks; + while (net) { + if (virNetworkIsActive(net)) + nactive++; + net = net->next; + } + return nactive; +} + +static int networkListNetworks(virConnectPtr conn, char **const names, int nnames) { + struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; + virNetworkObjPtr network = driver->networks; + int got = 0, i; + while (network && got < nnames) { + if (virNetworkIsActive(network)) { + if (!(names[got] = strdup(network->def->name))) { + networkReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, + "%s", _("failed to allocate space for VM name string")); + goto cleanup; + } + got++; + } + network = network->next; + } + return got; + + cleanup: + for (i = 0 ; i < got ; i++) + VIR_FREE(names[i]); + return -1; +} + +static int networkNumDefinedNetworks(virConnectPtr conn) { + int ninactive = 0; + struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; + virNetworkObjPtr net = driver->networks; + while (net) { + if (!virNetworkIsActive(net)) + ninactive++; + net = net->next; + } + return ninactive; +} + +static int networkListDefinedNetworks(virConnectPtr conn, char **const names, int nnames) { + struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; + virNetworkObjPtr network = driver->networks; + int got = 0, i; + while (network && got < nnames) { + if (!virNetworkIsActive(network)) { + if (!(names[got] = strdup(network->def->name))) { + networkReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, + "%s", _("failed to allocate space for VM name string")); + goto cleanup; + } + got++; + } + network = network->next; + } + return got; + + cleanup: + for (i = 0 ; i < got ; i++) + VIR_FREE(names[i]); + return -1; +} + +static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) { + struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; + virNetworkDefPtr def; + virNetworkObjPtr network; + virNetworkPtr net; + + if (!(def = virNetworkDefParseString(conn, xml))) + return NULL; + + if (!(network = virNetworkAssignDef(conn, + &driver->networks, + def))) { + virNetworkDefFree(def); + return NULL; + } + + if (networkStartNetworkDaemon(conn, driver, network) < 0) { + virNetworkRemoveInactive(&driver->networks, + network); + return NULL; + } + + net = virGetNetwork(conn, network->def->name, network->def->uuid); + return net; +} + +static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { + struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; + virNetworkDefPtr def; + virNetworkObjPtr network; + + if (!(def = virNetworkDefParseString(conn, xml))) + return NULL; + + if (!(network = virNetworkAssignDef(conn, + &driver->networks, + def))) { + virNetworkDefFree(def); + return NULL; + } + + if (virNetworkSaveConfig(conn, + driver->networkConfigDir, + driver->networkAutostartDir, + network) < 0) { + virNetworkRemoveInactive(&driver->networks, + network); + return NULL; + } + + return virGetNetwork(conn, network->def->name, network->def->uuid); +} + +static int networkUndefine(virNetworkPtr net) { + struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; + virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + + if (!network) { + networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_DOMAIN, + "%s", _("no network with matching uuid")); + return -1; + } + + if (virNetworkIsActive(network)) { + networkReportError(net->conn, NULL, net, VIR_ERR_INTERNAL_ERROR, + "%s", _("network is still active")); + return -1; + } + + if (virNetworkDeleteConfig(net->conn, network) < 0) + return -1; + + virNetworkRemoveInactive(&driver->networks, + network); + + return 0; +} + +static int networkStart(virNetworkPtr net) { + struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; + virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + + if (!network) { + networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, + "%s", _("no network with matching uuid")); + return -1; + } + + return networkStartNetworkDaemon(net->conn, driver, network); +} + +static int networkDestroy(virNetworkPtr net) { + struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; + virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + int ret; + + if (!network) { + networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, + "%s", _("no network with matching uuid")); + return -1; + } + + ret = networkShutdownNetworkDaemon(net->conn, driver, network); + + return ret; +} + +static char *networkDumpXML(virNetworkPtr net, int flags ATTRIBUTE_UNUSED) { + struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; + virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + + if (!network) { + networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, + "%s", _("no network with matching uuid")); + return NULL; + } + + return virNetworkDefFormat(net->conn, network->def); +} + +static char *networkGetBridgeName(virNetworkPtr net) { + struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; + virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + char *bridge; + if (!network) { + networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, + "%s", _("no network with matching id")); + return NULL; + } + + bridge = strdup(network->def->bridge); + if (!bridge) { + networkReportError(net->conn, NULL, net, VIR_ERR_NO_MEMORY, + "%s", _("failed to allocate space for network bridge string")); + return NULL; + } + return bridge; +} + +static int networkGetAutostart(virNetworkPtr net, + int *autostart) { + struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; + virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + + if (!network) { + networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, + "%s", _("no network with matching uuid")); + return -1; + } + + *autostart = network->autostart; + + return 0; +} + +static int networkSetAutostart(virNetworkPtr net, + int autostart) { + struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; + virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + + if (!network) { + networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, + "%s", _("no network with matching uuid")); + return -1; + } + + autostart = (autostart != 0); + + if (network->autostart == autostart) + return 0; + + if (autostart) { + int err; + + if ((err = virFileMakePath(driver->networkAutostartDir))) { + networkReportError(net->conn, NULL, net, VIR_ERR_INTERNAL_ERROR, + _("cannot create autostart directory %s: %s"), + driver->networkAutostartDir, strerror(err)); + return -1; + } + + if (symlink(network->configFile, network->autostartLink) < 0) { + networkReportError(net->conn, NULL, net, VIR_ERR_INTERNAL_ERROR, + _("Failed to create symlink '%s' to '%s': %s"), + network->autostartLink, network->configFile, strerror(errno)); + return -1; + } + } else { + if (unlink(network->autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { + networkReportError(net->conn, NULL, net, VIR_ERR_INTERNAL_ERROR, + _("Failed to delete symlink '%s': %s"), + network->autostartLink, strerror(errno)); + return -1; + } + } + + network->autostart = autostart; + + return 0; +} + + +static virNetworkDriver networkDriver = { + "Network", + networkOpenNetwork, /* open */ + networkCloseNetwork, /* close */ + networkNumNetworks, /* numOfNetworks */ + networkListNetworks, /* listNetworks */ + networkNumDefinedNetworks, /* numOfDefinedNetworks */ + networkListDefinedNetworks, /* listDefinedNetworks */ + networkLookupByUUID, /* networkLookupByUUID */ + networkLookupByName, /* networkLookupByName */ + networkCreate, /* networkCreateXML */ + networkDefine, /* networkDefineXML */ + networkUndefine, /* networkUndefine */ + networkStart, /* networkCreate */ + networkDestroy, /* networkDestroy */ + networkDumpXML, /* networkDumpXML */ + networkGetBridgeName, /* networkGetBridgeName */ + networkGetAutostart, /* networkGetAutostart */ + networkSetAutostart, /* networkSetAutostart */ +}; + +static virStateDriver networkStateDriver = { + networkStartup, + networkShutdown, + networkReload, + networkActive, + NULL +}; + +int networkRegister(void) { + virRegisterNetworkDriver(&networkDriver); + virRegisterStateDriver(&networkStateDriver); + return 0; +} + diff -r f68f3fd13e9d src/network_driver.h --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/network_driver.h Fri Oct 03 10:46:41 2008 +0100 @@ -0,0 +1,34 @@ +/* + * network_driver.h: core driver methods for managing networks + * + * Copyright (C) 2006, 2007 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * 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 + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + + +#ifndef __VIR_NETWORK__DRIVER_H +#define __VIR_NETWORK__DRIVER_H + +#include <config.h> + +#include "internal.h" + +int networkRegister(void); + +#endif /* __VIR_NETWORK__DRIVER_H */ diff -r f68f3fd13e9d src/qemu_conf.c --- a/src/qemu_conf.c Fri Oct 03 00:07:33 2008 +0100 +++ b/src/qemu_conf.c Fri Oct 03 10:46:41 2008 +0100 @@ -537,7 +537,6 @@ virDomainNetDefPtr net, int vlan) { - virNetworkObjPtr network = NULL; char *brname; char tapfdstr[4+3+32+7]; char *retval = NULL; @@ -545,18 +544,24 @@ int tapfd = -1; if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { - if (!(network = virNetworkFindByName(driver->networks, net->data.network.name))) { + virNetworkPtr network = virNetworkLookupByName(conn, + net->data.network.name); + if (!network) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Network '%s' not found"), net->data.network.name); goto error; - } else if (network->def->bridge == NULL) { + } + brname = virNetworkGetBridgeName(network); + + virNetworkFree(network); + + if (brname == NULL) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Network '%s' not active"), + _("Network '%s' is not active"), net->data.network.name); goto error; } - brname = network->def->bridge; } else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { brname = net->data.bridge.brname; } else { diff -r f68f3fd13e9d src/qemu_conf.h --- a/src/qemu_conf.h Fri Oct 03 00:07:33 2008 +0100 +++ b/src/qemu_conf.h Fri Oct 03 10:46:41 2008 +0100 @@ -27,7 +27,6 @@ #include <config.h> #include "internal.h" -#include "iptables.h" #include "bridge.h" #include "capabilities.h" #include "network_conf.h" @@ -53,14 +52,10 @@ int nextvmid; virDomainObjPtr domains; - virNetworkObjPtr networks; brControl *brctl; - iptablesContext *iptables; char *configDir; char *autostartDir; - char *networkConfigDir; - char *networkAutostartDir; char *logDir; unsigned int vncTLS : 1; unsigned int vncTLSx509verify : 1; diff -r f68f3fd13e9d src/qemu_driver.c --- a/src/qemu_driver.c Fri Oct 03 00:07:33 2008 +0100 +++ b/src/qemu_driver.c Fri Oct 03 10:46:41 2008 +0100 @@ -68,9 +68,7 @@ /* For storing short-lived temporary files. */ #define TEMPDIR LOCAL_STATE_DIR "/cache/libvirt" -#ifdef WITH_LIBVIRTD static int qemudShutdown(void); -#endif /* qemudDebug statements should be changed to use this macro instead. */ #define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__) @@ -118,13 +116,6 @@ struct qemud_driver *driver, virDomainObjPtr vm); -static int qemudStartNetworkDaemon(virConnectPtr conn, - struct qemud_driver *driver, - virNetworkObjPtr network); - -static int qemudShutdownNetworkDaemon(virConnectPtr conn, - struct qemud_driver *driver, - virNetworkObjPtr network); static int qemudDomainGetMaxVcpus(virDomainPtr dom); static int qemudMonitorCommand (const struct qemud_driver *driver, @@ -137,23 +128,7 @@ static void qemudAutostartConfigs(struct qemud_driver *driver) { - virNetworkObjPtr network; virDomainObjPtr vm; - - network = driver->networks; - while (network != NULL) { - virNetworkObjPtr next = network->next; - - if (network->autostart && - !virNetworkIsActive(network) && - qemudStartNetworkDaemon(NULL, driver, network) < 0) { - virErrorPtr err = virGetLastError(); - qemudLog(QEMUD_ERR, _("Failed to autostart network '%s': %s\n"), - network->def->name, err->message); - } - - network = next; - } vm = driver->domains; while (vm != NULL) { @@ -171,7 +146,6 @@ } } -#ifdef WITH_LIBVIRTD /** * qemudStartup: * @@ -228,13 +202,6 @@ if (asprintf (&qemu_driver->autostartDir, "%s/qemu/autostart", base) == -1) goto out_of_memory; - if (asprintf (&qemu_driver->networkConfigDir, "%s/qemu/networks", base) == -1) - goto out_of_memory; - - if (asprintf (&qemu_driver->networkAutostartDir, "%s/qemu/networks/autostart", - base) == -1) - goto out_of_memory; - VIR_FREE(base); if ((qemu_driver->caps = qemudCapsInit()) == NULL) @@ -250,13 +217,6 @@ &qemu_driver->domains, qemu_driver->configDir, qemu_driver->autostartDir) < 0) { - qemudShutdown(); - return -1; - } - if (virNetworkLoadAllConfigs(NULL, - &qemu_driver->networks, - qemu_driver->networkConfigDir, - qemu_driver->networkAutostartDir) < 0) { qemudShutdown(); return -1; } @@ -286,17 +246,6 @@ qemu_driver->configDir, qemu_driver->autostartDir); - virNetworkLoadAllConfigs(NULL, - &qemu_driver->networks, - qemu_driver->networkConfigDir, - qemu_driver->networkAutostartDir); - - if (qemu_driver->iptables) { - qemudLog(QEMUD_INFO, - "%s", _("Reloading iptables rules\n")); - iptablesReloadRules(qemu_driver->iptables); - } - qemudAutostartConfigs(qemu_driver); return 0; @@ -313,18 +262,11 @@ static int qemudActive(void) { virDomainObjPtr dom = qemu_driver->domains; - virNetworkObjPtr net = qemu_driver->networks; while (dom) { if (virDomainIsActive(dom)) return 1; dom = dom->next; - } - - while (net) { - if (virNetworkIsActive(net)) - return 1; - net = net->next; } /* Otherwise we're happy to deal with a shutdown */ @@ -339,7 +281,6 @@ static int qemudShutdown(void) { virDomainObjPtr vm; - virNetworkObjPtr network; if (!qemu_driver) return -1; @@ -367,41 +308,18 @@ } qemu_driver->domains = NULL; - /* shutdown active networks */ - network = qemu_driver->networks; - while (network) { - virNetworkObjPtr next = network->next; - if (virNetworkIsActive(network)) - qemudShutdownNetworkDaemon(NULL, qemu_driver, network); - network = next; - } - - /* free inactive networks */ - network = qemu_driver->networks; - while (network) { - virNetworkObjPtr next = network->next; - virNetworkObjFree(network); - network = next; - } - qemu_driver->networks = NULL; - VIR_FREE(qemu_driver->logDir); VIR_FREE(qemu_driver->configDir); VIR_FREE(qemu_driver->autostartDir); - VIR_FREE(qemu_driver->networkConfigDir); - VIR_FREE(qemu_driver->networkAutostartDir); VIR_FREE(qemu_driver->vncTLSx509certdir); if (qemu_driver->brctl) brShutdown(qemu_driver->brctl); - if (qemu_driver->iptables) - iptablesContextFree(qemu_driver->iptables); VIR_FREE(qemu_driver); return 0; } -#endif /* Return -1 for error, 1 to continue reading and 0 for success */ typedef int qemudHandlerMonitorOutput(virConnectPtr conn, @@ -1112,547 +1030,6 @@ if (!vm->persistent) virDomainRemoveInactive(&driver->domains, vm); - return 0; -} - -static int -qemudBuildDnsmasqArgv(virConnectPtr conn, - virNetworkObjPtr network, - const char ***argv) { - int i, len, r; - char buf[PATH_MAX]; - - len = - 1 + /* dnsmasq */ - 1 + /* --keep-in-foreground */ - 1 + /* --strict-order */ - 1 + /* --bind-interfaces */ - (network->def->domain?2:0) + /* --domain name */ - 2 + /* --pid-file "" */ - 2 + /* --conf-file "" */ - /*2 + *//* --interface virbr0 */ - 2 + /* --except-interface lo */ - 2 + /* --listen-address 10.0.0.1 */ - 1 + /* --dhcp-leasefile=path */ - (2 * network->def->nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ - /* --dhcp-host 01:23:45:67:89:0a,hostname,10.0.0.3 */ - (2 * network->def->nhosts) + - 1; /* NULL */ - - if (VIR_ALLOC_N(*argv, len) < 0) - goto no_memory; - -#define APPEND_ARG(v, n, s) do { \ - if (!((v)[(n)] = strdup(s))) \ - goto no_memory; \ - } while (0) - - i = 0; - - APPEND_ARG(*argv, i++, DNSMASQ); - - APPEND_ARG(*argv, i++, "--keep-in-foreground"); - /* - * Needed to ensure dnsmasq uses same algorithm for processing - * multiple namedriver entries in /etc/resolv.conf as GLibC. - */ - APPEND_ARG(*argv, i++, "--strict-order"); - APPEND_ARG(*argv, i++, "--bind-interfaces"); - - if (network->def->domain) { - APPEND_ARG(*argv, i++, "--domain"); - APPEND_ARG(*argv, i++, network->def->domain); - } - - APPEND_ARG(*argv, i++, "--pid-file"); - APPEND_ARG(*argv, i++, ""); - - APPEND_ARG(*argv, i++, "--conf-file"); - APPEND_ARG(*argv, i++, ""); - - /* - * XXX does not actually work, due to some kind of - * race condition setting up ipv6 addresses on the - * interface. A sleep(10) makes it work, but that's - * clearly not practical - * - * APPEND_ARG(*argv, i++, "--interface"); - * APPEND_ARG(*argv, i++, network->def->bridge); - */ - APPEND_ARG(*argv, i++, "--listen-address"); - APPEND_ARG(*argv, i++, network->def->ipAddress); - - APPEND_ARG(*argv, i++, "--except-interface"); - APPEND_ARG(*argv, i++, "lo"); - - /* - * NB, dnsmasq command line arg bug means we need to - * use a single arg '--dhcp-leasefile=path' rather than - * two separate args in '--dhcp-leasefile path' style - */ - snprintf(buf, sizeof(buf), "--dhcp-leasefile=%s/lib/libvirt/dhcp-%s.leases", - LOCAL_STATE_DIR, network->def->name); - APPEND_ARG(*argv, i++, buf); - - for (r = 0 ; r < network->def->nranges ; r++) { - snprintf(buf, sizeof(buf), "%s,%s", - network->def->ranges[r].start, - network->def->ranges[r].end); - - APPEND_ARG(*argv, i++, "--dhcp-range"); - APPEND_ARG(*argv, i++, buf); - } - - for (r = 0 ; r < network->def->nhosts ; r++) { - virNetworkDHCPHostDefPtr host = &(network->def->hosts[r]); - if ((host->mac) && (host->name)) { - snprintf(buf, sizeof(buf), "%s,%s,%s", - host->mac, host->name, host->ip); - } else if (host->mac) { - snprintf(buf, sizeof(buf), "%s,%s", - host->mac, host->ip); - } else if (host->name) { - snprintf(buf, sizeof(buf), "%s,%s", - host->name, host->ip); - } else - continue; - - APPEND_ARG(*argv, i++, "--dhcp-host"); - APPEND_ARG(*argv, i++, buf); - } - -#undef APPEND_ARG - - return 0; - - no_memory: - if (argv) { - for (i = 0; (*argv)[i]; i++) - VIR_FREE((*argv)[i]); - VIR_FREE(*argv); - } - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, - "%s", _("failed to allocate space for dnsmasq argv")); - return -1; -} - - -static int -dhcpStartDhcpDaemon(virConnectPtr conn, - virNetworkObjPtr network) -{ - const char **argv; - int ret, i; - - if (network->def->ipAddress == NULL) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot start dhcp daemon without IP address for server")); - return -1; - } - - argv = NULL; - if (qemudBuildDnsmasqArgv(conn, network, &argv) < 0) - return -1; - - ret = virExec(conn, argv, NULL, NULL, - &network->dnsmasqPid, -1, NULL, NULL, VIR_EXEC_NONBLOCK); - - for (i = 0; argv[i]; i++) - VIR_FREE(argv[i]); - VIR_FREE(argv); - - return ret; -} - -static int -qemudAddMasqueradingIptablesRules(virConnectPtr conn, - struct qemud_driver *driver, - virNetworkObjPtr network) { - int err; - /* allow forwarding packets from the bridge interface */ - if ((err = iptablesAddForwardAllowOut(driver->iptables, - network->def->network, - network->def->bridge, - network->def->forwardDev))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to allow forwarding from '%s' : %s\n"), - network->def->bridge, strerror(err)); - goto masqerr1; - } - - /* allow forwarding packets to the bridge interface if they are part of an existing connection */ - if ((err = iptablesAddForwardAllowRelatedIn(driver->iptables, - network->def->network, - network->def->bridge, - network->def->forwardDev))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to allow forwarding to '%s' : %s\n"), - network->def->bridge, strerror(err)); - goto masqerr2; - } - - /* enable masquerading */ - if ((err = iptablesAddForwardMasquerade(driver->iptables, - network->def->network, - network->def->forwardDev))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to enable masquerading : %s\n"), - strerror(err)); - goto masqerr3; - } - - return 1; - - masqerr3: - iptablesRemoveForwardAllowRelatedIn(driver->iptables, - network->def->network, - network->def->bridge, - network->def->forwardDev); - masqerr2: - iptablesRemoveForwardAllowOut(driver->iptables, - network->def->network, - network->def->bridge, - network->def->forwardDev); - masqerr1: - return 0; -} - -static int -qemudAddRoutingIptablesRules(virConnectPtr conn, - struct qemud_driver *driver, - virNetworkObjPtr network) { - int err; - /* allow routing packets from the bridge interface */ - if ((err = iptablesAddForwardAllowOut(driver->iptables, - network->def->network, - network->def->bridge, - network->def->forwardDev))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to allow routing from '%s' : %s\n"), - network->def->bridge, strerror(err)); - goto routeerr1; - } - - /* allow routing packets to the bridge interface */ - if ((err = iptablesAddForwardAllowIn(driver->iptables, - network->def->network, - network->def->bridge, - network->def->forwardDev))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to allow routing to '%s' : %s\n"), - network->def->bridge, strerror(err)); - goto routeerr2; - } - - return 1; - - - routeerr2: - iptablesRemoveForwardAllowOut(driver->iptables, - network->def->network, - network->def->bridge, - network->def->forwardDev); - routeerr1: - return 0; -} - -static int -qemudAddIptablesRules(virConnectPtr conn, - struct qemud_driver *driver, - virNetworkObjPtr network) { - int err; - - if (!driver->iptables && !(driver->iptables = iptablesContextNew())) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, - "%s", _("failed to allocate space for IP tables support")); - return 0; - } - - - /* allow DHCP requests through to dnsmasq */ - if ((err = iptablesAddTcpInput(driver->iptables, network->def->bridge, 67))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to allow DHCP requests from '%s' : %s"), - network->def->bridge, strerror(err)); - goto err1; - } - - if ((err = iptablesAddUdpInput(driver->iptables, network->def->bridge, 67))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to allow DHCP requests from '%s' : %s"), - network->def->bridge, strerror(err)); - goto err2; - } - - /* allow DNS requests through to dnsmasq */ - if ((err = iptablesAddTcpInput(driver->iptables, network->def->bridge, 53))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to allow DNS requests from '%s' : %s"), - network->def->bridge, strerror(err)); - goto err3; - } - - if ((err = iptablesAddUdpInput(driver->iptables, network->def->bridge, 53))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to allow DNS requests from '%s' : %s"), - network->def->bridge, strerror(err)); - goto err4; - } - - - /* Catch all rules to block forwarding to/from bridges */ - - if ((err = iptablesAddForwardRejectOut(driver->iptables, network->def->bridge))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to block outbound traffic from '%s' : %s"), - network->def->bridge, strerror(err)); - goto err5; - } - - if ((err = iptablesAddForwardRejectIn(driver->iptables, network->def->bridge))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to block inbound traffic to '%s' : %s"), - network->def->bridge, strerror(err)); - goto err6; - } - - /* Allow traffic between guests on the same bridge */ - if ((err = iptablesAddForwardAllowCross(driver->iptables, network->def->bridge))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to add iptables rule to allow cross bridge traffic on '%s' : %s"), - network->def->bridge, strerror(err)); - goto err7; - } - - - /* If masquerading is enabled, set up the rules*/ - if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT && - !qemudAddMasqueradingIptablesRules(conn, driver, network)) - goto err8; - /* else if routing is enabled, set up the rules*/ - else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE && - !qemudAddRoutingIptablesRules(conn, driver, network)) - goto err8; - - iptablesSaveRules(driver->iptables); - - return 1; - - err8: - iptablesRemoveForwardAllowCross(driver->iptables, - network->def->bridge); - err7: - iptablesRemoveForwardRejectIn(driver->iptables, - network->def->bridge); - err6: - iptablesRemoveForwardRejectOut(driver->iptables, - network->def->bridge); - err5: - iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53); - err4: - iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53); - err3: - iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 67); - err2: - iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67); - err1: - return 0; -} - -static void -qemudRemoveIptablesRules(struct qemud_driver *driver, - virNetworkObjPtr network) { - if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) { - iptablesRemoveForwardMasquerade(driver->iptables, - network->def->network, - network->def->forwardDev); - - if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) - iptablesRemoveForwardAllowRelatedIn(driver->iptables, - network->def->network, - network->def->bridge, - network->def->forwardDev); - else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) - iptablesRemoveForwardAllowIn(driver->iptables, - network->def->network, - network->def->bridge, - network->def->forwardDev); - - iptablesRemoveForwardAllowOut(driver->iptables, - network->def->network, - network->def->bridge, - network->def->forwardDev); - } - iptablesRemoveForwardAllowCross(driver->iptables, network->def->bridge); - iptablesRemoveForwardRejectIn(driver->iptables, network->def->bridge); - iptablesRemoveForwardRejectOut(driver->iptables, network->def->bridge); - iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 53); - iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 53); - iptablesRemoveUdpInput(driver->iptables, network->def->bridge, 67); - iptablesRemoveTcpInput(driver->iptables, network->def->bridge, 67); - iptablesSaveRules(driver->iptables); -} - -static int -qemudEnableIpForwarding(void) -{ -#define PROC_IP_FORWARD "/proc/sys/net/ipv4/ip_forward" - - int fd, ret; - - if ((fd = open(PROC_IP_FORWARD, O_WRONLY|O_TRUNC)) == -1) - return 0; - - if (safewrite(fd, "1\n", 2) < 0) - ret = 0; - - close (fd); - - return 1; - -#undef PROC_IP_FORWARD -} - -static int qemudStartNetworkDaemon(virConnectPtr conn, - struct qemud_driver *driver, - virNetworkObjPtr network) { - int err; - - if (virNetworkIsActive(network)) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("network is already active")); - return -1; - } - - if (!driver->brctl && (err = brInit(&driver->brctl))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("cannot initialize bridge support: %s"), strerror(err)); - return -1; - } - - if ((err = brAddBridge(driver->brctl, &network->def->bridge))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("cannot create bridge '%s' : %s"), - network->def->bridge, strerror(err)); - return -1; - } - - - if (brSetForwardDelay(driver->brctl, network->def->bridge, network->def->delay) < 0) - goto err_delbr; - - if (brSetEnableSTP(driver->brctl, network->def->bridge, network->def->stp ? 1 : 0) < 0) - goto err_delbr; - - if (network->def->ipAddress && - (err = brSetInetAddress(driver->brctl, network->def->bridge, network->def->ipAddress))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("cannot set IP address on bridge '%s' to '%s' : %s"), - network->def->bridge, network->def->ipAddress, strerror(err)); - goto err_delbr; - } - - if (network->def->netmask && - (err = brSetInetNetmask(driver->brctl, network->def->bridge, network->def->netmask))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("cannot set netmask on bridge '%s' to '%s' : %s"), - network->def->bridge, network->def->netmask, strerror(err)); - goto err_delbr; - } - - if (network->def->ipAddress && - (err = brSetInterfaceUp(driver->brctl, network->def->bridge, 1))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to bring the bridge '%s' up : %s"), - network->def->bridge, strerror(err)); - goto err_delbr; - } - - if (!qemudAddIptablesRules(conn, driver, network)) - goto err_delbr1; - - if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE && - !qemudEnableIpForwarding()) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to enable IP forwarding : %s"), strerror(err)); - goto err_delbr2; - } - - if (network->def->nranges && - dhcpStartDhcpDaemon(conn, network) < 0) - goto err_delbr2; - - network->active = 1; - - return 0; - - err_delbr2: - qemudRemoveIptablesRules(driver, network); - - err_delbr1: - if (network->def->ipAddress && - (err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) { - qemudLog(QEMUD_WARN, _("Failed to bring down bridge '%s' : %s\n"), - network->def->bridge, strerror(err)); - } - - err_delbr: - if ((err = brDeleteBridge(driver->brctl, network->def->bridge))) { - qemudLog(QEMUD_WARN, _("Failed to delete bridge '%s' : %s\n"), - network->def->bridge, strerror(err)); - } - - return -1; -} - - -static int qemudShutdownNetworkDaemon(virConnectPtr conn ATTRIBUTE_UNUSED, - struct qemud_driver *driver, - virNetworkObjPtr network) { - int err; - - qemudLog(QEMUD_INFO, _("Shutting down network '%s'\n"), network->def->name); - - if (!virNetworkIsActive(network)) - return 0; - - if (network->dnsmasqPid > 0) - kill(network->dnsmasqPid, SIGTERM); - - qemudRemoveIptablesRules(driver, network); - - if (network->def->ipAddress && - (err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) { - qemudLog(QEMUD_WARN, _("Failed to bring down bridge '%s' : %s\n"), - network->def->bridge, strerror(err)); - } - - if ((err = brDeleteBridge(driver->brctl, network->def->bridge))) { - qemudLog(QEMUD_WARN, _("Failed to delete bridge '%s' : %s\n"), - network->def->bridge, strerror(err)); - } - - if (network->dnsmasqPid > 0 && - waitpid(network->dnsmasqPid, NULL, WNOHANG) != network->dnsmasqPid) { - kill(network->dnsmasqPid, SIGKILL); - if (waitpid(network->dnsmasqPid, NULL, 0) != network->dnsmasqPid) - qemudLog(QEMUD_WARN, - "%s", _("Got unexpected pid for dnsmasq\n")); - } - - network->dnsmasqPid = -1; - network->active = 0; - - if (network->newDef) { - virNetworkDefFree(network->def); - network->def = network->newDef; - network->newDef = NULL; - } - - if (!network->configFile) - virNetworkRemoveInactive(&driver->networks, - network); - return 0; } @@ -3706,323 +3083,6 @@ return ret; } -static virNetworkPtr qemudNetworkLookupByUUID(virConnectPtr conn ATTRIBUTE_UNUSED, - const unsigned char *uuid) { - struct qemud_driver *driver = (struct qemud_driver *)conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, uuid); - virNetworkPtr net; - - if (!network) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_NETWORK, - "%s", _("no network with matching uuid")); - return NULL; - } - - net = virGetNetwork(conn, network->def->name, network->def->uuid); - return net; -} -static virNetworkPtr qemudNetworkLookupByName(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *name) { - struct qemud_driver *driver = (struct qemud_driver *)conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByName(driver->networks, name); - virNetworkPtr net; - - if (!network) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_NETWORK, - "%s", _("no network with matching name")); - return NULL; - } - - net = virGetNetwork(conn, network->def->name, network->def->uuid); - return net; -} - -static virDrvOpenStatus qemudOpenNetwork(virConnectPtr conn, - xmlURIPtr uri ATTRIBUTE_UNUSED, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, - int flags ATTRIBUTE_UNUSED) { - if (!qemu_driver) - return VIR_DRV_OPEN_DECLINED; - - conn->networkPrivateData = qemu_driver; - return VIR_DRV_OPEN_SUCCESS; -} - -static int qemudCloseNetwork(virConnectPtr conn) { - conn->networkPrivateData = NULL; - return 0; -} - -static int qemudNumNetworks(virConnectPtr conn) { - int nactive = 0; - struct qemud_driver *driver = (struct qemud_driver *)conn->networkPrivateData; - virNetworkObjPtr net = driver->networks; - while (net) { - if (virNetworkIsActive(net)) - nactive++; - net = net->next; - } - return nactive; -} - -static int qemudListNetworks(virConnectPtr conn, char **const names, int nnames) { - struct qemud_driver *driver = (struct qemud_driver *)conn->networkPrivateData; - virNetworkObjPtr network = driver->networks; - int got = 0, i; - while (network && got < nnames) { - if (virNetworkIsActive(network)) { - if (!(names[got] = strdup(network->def->name))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, - "%s", _("failed to allocate space for VM name string")); - goto cleanup; - } - got++; - } - network = network->next; - } - return got; - - cleanup: - for (i = 0 ; i < got ; i++) - VIR_FREE(names[i]); - return -1; -} - -static int qemudNumDefinedNetworks(virConnectPtr conn) { - int ninactive = 0; - struct qemud_driver *driver = (struct qemud_driver *)conn->networkPrivateData; - virNetworkObjPtr net = driver->networks; - while (net) { - if (!virNetworkIsActive(net)) - ninactive++; - net = net->next; - } - return ninactive; -} - -static int qemudListDefinedNetworks(virConnectPtr conn, char **const names, int nnames) { - struct qemud_driver *driver = (struct qemud_driver *)conn->networkPrivateData; - virNetworkObjPtr network = driver->networks; - int got = 0, i; - while (network && got < nnames) { - if (!virNetworkIsActive(network)) { - if (!(names[got] = strdup(network->def->name))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, - "%s", _("failed to allocate space for VM name string")); - goto cleanup; - } - got++; - } - network = network->next; - } - return got; - - cleanup: - for (i = 0 ; i < got ; i++) - VIR_FREE(names[i]); - return -1; -} - -static virNetworkPtr qemudNetworkCreate(virConnectPtr conn, const char *xml) { - struct qemud_driver *driver = (struct qemud_driver *)conn->networkPrivateData; - virNetworkDefPtr def; - virNetworkObjPtr network; - virNetworkPtr net; - - if (!(def = virNetworkDefParseString(conn, xml))) - return NULL; - - if (!(network = virNetworkAssignDef(conn, - &driver->networks, - def))) { - virNetworkDefFree(def); - return NULL; - } - - if (qemudStartNetworkDaemon(conn, driver, network) < 0) { - virNetworkRemoveInactive(&driver->networks, - network); - return NULL; - } - - net = virGetNetwork(conn, network->def->name, network->def->uuid); - return net; -} - -static virNetworkPtr qemudNetworkDefine(virConnectPtr conn, const char *xml) { - struct qemud_driver *driver = (struct qemud_driver *)conn->networkPrivateData; - virNetworkDefPtr def; - virNetworkObjPtr network; - - if (!(def = virNetworkDefParseString(conn, xml))) - return NULL; - - if (!(network = virNetworkAssignDef(conn, - &driver->networks, - def))) { - virNetworkDefFree(def); - return NULL; - } - - if (virNetworkSaveConfig(conn, - driver->networkConfigDir, - driver->networkAutostartDir, - network) < 0) { - virNetworkRemoveInactive(&driver->networks, - network); - return NULL; - } - - return virGetNetwork(conn, network->def->name, network->def->uuid); -} - -static int qemudNetworkUndefine(virNetworkPtr net) { - struct qemud_driver *driver = (struct qemud_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); - - if (!network) { - qemudReportError(net->conn, NULL, net, VIR_ERR_INVALID_DOMAIN, - "%s", _("no network with matching uuid")); - return -1; - } - - if (virNetworkIsActive(network)) { - qemudReportError(net->conn, NULL, net, VIR_ERR_INTERNAL_ERROR, - "%s", _("network is still active")); - return -1; - } - - if (virNetworkDeleteConfig(net->conn, network) < 0) - return -1; - - virNetworkRemoveInactive(&driver->networks, - network); - - return 0; -} - -static int qemudNetworkStart(virNetworkPtr net) { - struct qemud_driver *driver = (struct qemud_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); - - if (!network) { - qemudReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, - "%s", _("no network with matching uuid")); - return -1; - } - - return qemudStartNetworkDaemon(net->conn, driver, network); -} - -static int qemudNetworkDestroy(virNetworkPtr net) { - struct qemud_driver *driver = (struct qemud_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); - int ret; - - if (!network) { - qemudReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, - "%s", _("no network with matching uuid")); - return -1; - } - - ret = qemudShutdownNetworkDaemon(net->conn, driver, network); - - return ret; -} - -static char *qemudNetworkDumpXML(virNetworkPtr net, int flags ATTRIBUTE_UNUSED) { - struct qemud_driver *driver = (struct qemud_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); - - if (!network) { - qemudReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, - "%s", _("no network with matching uuid")); - return NULL; - } - - return virNetworkDefFormat(net->conn, network->def); -} - -static char *qemudNetworkGetBridgeName(virNetworkPtr net) { - struct qemud_driver *driver = (struct qemud_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); - char *bridge; - if (!network) { - qemudReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, - "%s", _("no network with matching id")); - return NULL; - } - - bridge = strdup(network->def->bridge); - if (!bridge) { - qemudReportError(net->conn, NULL, net, VIR_ERR_NO_MEMORY, - "%s", _("failed to allocate space for network bridge string")); - return NULL; - } - return bridge; -} - -static int qemudNetworkGetAutostart(virNetworkPtr net, - int *autostart) { - struct qemud_driver *driver = (struct qemud_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); - - if (!network) { - qemudReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, - "%s", _("no network with matching uuid")); - return -1; - } - - *autostart = network->autostart; - - return 0; -} - -static int qemudNetworkSetAutostart(virNetworkPtr net, - int autostart) { - struct qemud_driver *driver = (struct qemud_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); - - if (!network) { - qemudReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, - "%s", _("no network with matching uuid")); - return -1; - } - - autostart = (autostart != 0); - - if (network->autostart == autostart) - return 0; - - if (autostart) { - int err; - - if ((err = virFileMakePath(driver->networkAutostartDir))) { - qemudReportError(net->conn, NULL, net, VIR_ERR_INTERNAL_ERROR, - _("cannot create autostart directory %s: %s"), - driver->networkAutostartDir, strerror(err)); - return -1; - } - - if (symlink(network->configFile, network->autostartLink) < 0) { - qemudReportError(net->conn, NULL, net, VIR_ERR_INTERNAL_ERROR, - _("Failed to create symlink '%s' to '%s': %s"), - network->autostartLink, network->configFile, strerror(errno)); - return -1; - } - } else { - if (unlink(network->autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { - qemudReportError(net->conn, NULL, net, VIR_ERR_INTERNAL_ERROR, - _("Failed to delete symlink '%s': %s"), - network->autostartLink, strerror(errno)); - return -1; - } - } - - network->autostart = autostart; - - return 0; -} static virDriver qemuDriver = { VIR_DRV_QEMU, @@ -4096,28 +3156,7 @@ #endif }; -static virNetworkDriver qemuNetworkDriver = { - "QEMU", - qemudOpenNetwork, /* open */ - qemudCloseNetwork, /* close */ - qemudNumNetworks, /* numOfNetworks */ - qemudListNetworks, /* listNetworks */ - qemudNumDefinedNetworks, /* numOfDefinedNetworks */ - qemudListDefinedNetworks, /* listDefinedNetworks */ - qemudNetworkLookupByUUID, /* networkLookupByUUID */ - qemudNetworkLookupByName, /* networkLookupByName */ - qemudNetworkCreate, /* networkCreateXML */ - qemudNetworkDefine, /* networkDefineXML */ - qemudNetworkUndefine, /* networkUndefine */ - qemudNetworkStart, /* networkCreate */ - qemudNetworkDestroy, /* networkDestroy */ - qemudNetworkDumpXML, /* networkDumpXML */ - qemudNetworkGetBridgeName, /* networkGetBridgeName */ - qemudNetworkGetAutostart, /* networkGetAutostart */ - qemudNetworkSetAutostart, /* networkSetAutostart */ -}; -#ifdef WITH_LIBVIRTD static virStateDriver qemuStateDriver = { qemudStartup, qemudShutdown, @@ -4125,14 +3164,10 @@ qemudActive, NULL }; -#endif int qemudRegister(void) { virRegisterDriver(&qemuDriver); - virRegisterNetworkDriver(&qemuNetworkDriver); -#ifdef WITH_LIBVIRTD virRegisterStateDriver(&qemuStateDriver); -#endif return 0; } -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Fri, Oct 03, 2008 at 01:23:42PM +0100, Daniel P. Berrange wrote:
Due to historical architectural restrictions the virtual network driver is actually part of the QEMU driver. The restrictions have long ago been removed, so its overdue to move the network driver out of the QEMU driver. This patch does this. It is a straight re-factoring with no functional change.
I just noticed a bit of renaming in the functions and of course the initialization changes. The configurability is also a nice bonus. Looks fine to me, +1
The only minor pain point is that the network driver stores it config files under /etc/libvirt/qemu/networks, when ideally it would be in /etc/libvirt/networks. This patch leaves the config files where they already are, but I'm not too happy about that. I'd like to move them to the correct location and just leave a back-compatability symlink or something like that.
can we just consider this a distribution issue, left to the packaging system to provide the backward compat link ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

This patch removes the linked list in the virDomainObjPtr object, and adds a new virDomainObjList struct to track domains as an array. The QEMU, LXC, OpenVZ and Test drivers get updated to account for this API change. domain_conf.c | 99 ++++++++++++++++------------ domain_conf.h | 23 +++--- lxc_conf.h | 2 lxc_driver.c | 112 +++++++++++++------------------ openvz_conf.c | 24 ++---- openvz_conf.h | 2 openvz_driver.c | 54 +++++++-------- qemu_conf.h | 2 qemu_driver.c | 198 +++++++++++++++++++++++++------------------------------- test.c | 77 +++++++-------------- 10 files changed, 274 insertions(+), 319 deletions(-) Daniel diff -r 2bb12d98c26b src/domain_conf.c --- a/src/domain_conf.c Fri Oct 03 12:45:58 2008 +0100 +++ b/src/domain_conf.c Fri Oct 03 12:58:03 2008 +0100 @@ -166,44 +166,40 @@ } -virDomainObjPtr virDomainFindByID(const virDomainObjPtr doms, +virDomainObjPtr virDomainFindByID(const virDomainObjListPtr doms, int id) { - virDomainObjPtr dom = doms; - while (dom) { - if (virDomainIsActive(dom) && dom->def->id == id) - return dom; - dom = dom->next; - } + unsigned int i; + + for (i = 0 ; i < doms->count ; i++) + if (virDomainIsActive(doms->objs[i]) && + doms->objs[i]->def->id == id) + return doms->objs[i]; return NULL; } -virDomainObjPtr virDomainFindByUUID(const virDomainObjPtr doms, +virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms, const unsigned char *uuid) { - virDomainObjPtr dom = doms; + unsigned int i; - while (dom) { - if (!memcmp(dom->def->uuid, uuid, VIR_UUID_BUFLEN)) - return dom; - dom = dom->next; - } + for (i = 0 ; i < doms->count ; i++) + if (!memcmp(doms->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) + return doms->objs[i]; return NULL; } -virDomainObjPtr virDomainFindByName(const virDomainObjPtr doms, +virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms, const char *name) { - virDomainObjPtr dom = doms; + unsigned int i; - while (dom) { - if (STREQ(dom->def->name, name)) - return dom; - dom = dom->next; - } + for (i = 0 ; i < doms->count ; i++) + if (STREQ(doms->objs[i]->def->name, name)) + return doms->objs[i]; return NULL; } @@ -425,13 +421,24 @@ VIR_FREE(dom); } +void virDomainObjListFree(virDomainObjListPtr vms) +{ + unsigned int i; + + for (i = 0 ; i < vms->count ; i++) + virDomainObjFree(vms->objs[i]); + + VIR_FREE(vms->objs); + vms->count = 0; +} + virDomainObjPtr virDomainAssignDef(virConnectPtr conn, - virDomainObjPtr *doms, + virDomainObjListPtr doms, const virDomainDefPtr def) { virDomainObjPtr domain; - if ((domain = virDomainFindByName(*doms, def->name))) { + if ((domain = virDomainFindByName(doms, def->name))) { if (!virDomainIsActive(domain)) { virDomainDefFree(domain->def); domain->def = def; @@ -451,33 +458,41 @@ domain->state = VIR_DOMAIN_SHUTOFF; domain->def = def; - domain->next = *doms; - *doms = domain; + if (VIR_REALLOC_N(doms->objs, doms->count + 1) < 0) { + virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL); + VIR_FREE(domain); + return NULL; + } + + doms->objs[doms->count] = domain; + doms->count++; return domain; } -void virDomainRemoveInactive(virDomainObjPtr *doms, +void virDomainRemoveInactive(virDomainObjListPtr doms, virDomainObjPtr dom) { - virDomainObjPtr prev = NULL; - virDomainObjPtr curr = *doms; + unsigned int i; - while (curr && - curr != dom) { - prev = curr; - curr = curr->next; + for (i = 0 ; i < doms->count ; i++) { + if (doms->objs[i] == dom) { + virDomainObjFree(doms->objs[i]); + + if (i < (doms->count - 1)) + memmove(doms->objs + i, doms->objs + i + 1, + sizeof(*(doms->objs)) * (doms->count - (i + 1))); + + if (VIR_REALLOC_N(doms->objs, doms->count - 1) < 0) { + ; /* Failure to reduce memory allocation isn't fatal */ + } + doms->count--; + + break; + } } - if (curr) { - if (prev) - prev->next = curr->next; - else - *doms = curr->next; - } - - virDomainObjFree(dom); } #ifndef PROXY @@ -3266,7 +3281,7 @@ virDomainObjPtr virDomainLoadConfig(virConnectPtr conn, virCapsPtr caps, - virDomainObjPtr *doms, + virDomainObjListPtr doms, const char *configDir, const char *autostartDir, const char *name) @@ -3305,7 +3320,7 @@ int virDomainLoadAllConfigs(virConnectPtr conn, virCapsPtr caps, - virDomainObjPtr *doms, + virDomainObjListPtr doms, const char *configDir, const char *autostartDir) { diff -r 2bb12d98c26b src/domain_conf.h --- a/src/domain_conf.h Fri Oct 03 12:45:58 2008 +0100 +++ b/src/domain_conf.h Fri Oct 03 12:58:03 2008 +0100 @@ -460,10 +460,14 @@ virDomainDefPtr def; /* The current definition */ virDomainDefPtr newDef; /* New definition to activate at shutdown */ - - virDomainObjPtr next; }; +typedef struct _virDomainObjList virDomainObjList; +typedef virDomainObjList *virDomainObjListPtr; +struct _virDomainObjList { + unsigned int count; + virDomainObjPtr *objs; +}; static inline int virDomainIsActive(virDomainObjPtr dom) @@ -472,11 +476,11 @@ } -virDomainObjPtr virDomainFindByID(const virDomainObjPtr doms, +virDomainObjPtr virDomainFindByID(const virDomainObjListPtr doms, int id); -virDomainObjPtr virDomainFindByUUID(const virDomainObjPtr doms, +virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms, const unsigned char *uuid); -virDomainObjPtr virDomainFindByName(const virDomainObjPtr doms, +virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms, const char *name); @@ -491,11 +495,12 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def); void virDomainDefFree(virDomainDefPtr vm); void virDomainObjFree(virDomainObjPtr vm); +void virDomainObjListFree(virDomainObjListPtr vms); virDomainObjPtr virDomainAssignDef(virConnectPtr conn, - virDomainObjPtr *doms, + virDomainObjListPtr doms, const virDomainDefPtr def); -void virDomainRemoveInactive(virDomainObjPtr *doms, +void virDomainRemoveInactive(virDomainObjListPtr doms, virDomainObjPtr dom); #ifndef PROXY @@ -535,14 +540,14 @@ virDomainObjPtr virDomainLoadConfig(virConnectPtr conn, virCapsPtr caps, - virDomainObjPtr *doms, + virDomainObjListPtr doms, const char *configDir, const char *autostartDir, const char *name); int virDomainLoadAllConfigs(virConnectPtr conn, virCapsPtr caps, - virDomainObjPtr *doms, + virDomainObjListPtr doms, const char *configDir, const char *autostartDir); diff -r 2bb12d98c26b src/lxc_conf.h --- a/src/lxc_conf.h Fri Oct 03 12:45:58 2008 +0100 +++ b/src/lxc_conf.h Fri Oct 03 12:58:03 2008 +0100 @@ -38,7 +38,7 @@ struct __lxc_driver { virCapsPtr caps; - virDomainObjPtr domains; + virDomainObjList domains; char *configDir; char *autostartDir; char *stateDir; diff -r 2bb12d98c26b src/lxc_driver.c --- a/src/lxc_driver.c Fri Oct 03 12:45:58 2008 +0100 +++ b/src/lxc_driver.c Fri Oct 03 12:58:03 2008 +0100 @@ -107,7 +107,7 @@ int id) { lxc_driver_t *driver = (lxc_driver_t *)conn->privateData; - virDomainObjPtr vm = virDomainFindByID(driver->domains, id); + virDomainObjPtr vm = virDomainFindByID(&driver->domains, id); virDomainPtr dom; if (!vm) { @@ -127,7 +127,7 @@ const unsigned char *uuid) { lxc_driver_t *driver = (lxc_driver_t *)conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, uuid); virDomainPtr dom; if (!vm) { @@ -147,7 +147,7 @@ const char *name) { lxc_driver_t *driver = (lxc_driver_t *)conn->privateData; - virDomainObjPtr vm = virDomainFindByName(driver->domains, name); + virDomainObjPtr vm = virDomainFindByName(&driver->domains, name); virDomainPtr dom; if (!vm) { @@ -165,44 +165,40 @@ static int lxcListDomains(virConnectPtr conn, int *ids, int nids) { lxc_driver_t *driver = (lxc_driver_t *)conn->privateData; - virDomainObjPtr vm = driver->domains; - int got = 0; - while (vm && got < nids) { - if (virDomainIsActive(vm)) { - ids[got] = vm->def->id; - got++; - } - vm = vm->next; - } + int got = 0, i; + + for (i = 0 ; i < driver->domains.count && got < nids ; i++) + if (virDomainIsActive(driver->domains.objs[i])) + ids[got++] = driver->domains.objs[i]->def->id; + return got; } static int lxcNumDomains(virConnectPtr conn) { lxc_driver_t *driver = (lxc_driver_t *)conn->privateData; - int n = 0; - virDomainObjPtr dom = driver->domains; - while (dom) { - if (virDomainIsActive(dom)) + int n = 0, i; + + for (i = 0 ; i < driver->domains.count ; i++) + if (virDomainIsActive(driver->domains.objs[i])) n++; - dom = dom->next; - } + return n; } static int lxcListDefinedDomains(virConnectPtr conn, char **const names, int nnames) { lxc_driver_t *driver = (lxc_driver_t *)conn->privateData; - virDomainObjPtr vm = driver->domains; int got = 0, i; - while (vm && got < nnames) { - if (!virDomainIsActive(vm)) { - if (!(names[got] = strdup(vm->def->name))) { - lxcError(conn, NULL, VIR_ERR_NO_MEMORY, NULL); + + for (i = 0 ; i < driver->domains.count && got < nnames ; i++) { + if (!virDomainIsActive(driver->domains.objs[i])) { + if (!(names[got++] = strdup(driver->domains.objs[i]->def->name))) { + lxcError(conn, NULL, VIR_ERR_NO_MEMORY, + "%s", _("failed to allocate space for VM name string")); goto cleanup; } - got++; } - vm = vm->next; } + return got; cleanup: @@ -214,13 +210,12 @@ static int lxcNumDefinedDomains(virConnectPtr conn) { lxc_driver_t *driver = (lxc_driver_t *)conn->privateData; - int n = 0; - virDomainObjPtr dom = driver->domains; - while (dom) { - if (!virDomainIsActive(dom)) + int n = 0, i; + + for (i = 0 ; i < driver->domains.count ; i++) + if (!virDomainIsActive(driver->domains.objs[i])) n++; - dom = dom->next; - } + return n; } @@ -267,7 +262,7 @@ static int lxcDomainUndefine(virDomainPtr dom) { lxc_driver_t *driver = (lxc_driver_t *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN, @@ -302,7 +297,7 @@ virDomainInfoPtr info) { lxc_driver_t *driver = (lxc_driver_t *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN, @@ -328,7 +323,7 @@ static char *lxcGetOSType(virDomainPtr dom) { lxc_driver_t *driver = (lxc_driver_t *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN, @@ -343,7 +338,7 @@ int flags) { lxc_driver_t *driver = (lxc_driver_t *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN, @@ -590,12 +585,14 @@ void *data) { lxc_driver_t *driver = data; - virDomainObjPtr vm = driver->domains; + virDomainObjPtr vm = NULL; + unsigned int i; - while (vm) { - if (vm->monitor == fd) + for (i = 0 ; i < driver->domains.count ; i++) { + if (driver->domains.objs[i]->monitor == fd) { + vm = driver->domains.objs[i]; break; - vm = vm->next; + } } if (!vm) { virEventRemoveHandle(fd); @@ -852,7 +849,7 @@ int rc = -1; virConnectPtr conn = dom->conn; lxc_driver_t *driver = (lxc_driver_t *)(conn->privateData); - virDomainObjPtr vm = virDomainFindByName(driver->domains, dom->name); + virDomainObjPtr vm = virDomainFindByName(&driver->domains, dom->name); if (!vm) { lxcError(conn, dom, VIR_ERR_INVALID_DOMAIN, @@ -932,7 +929,7 @@ static int lxcDomainShutdown(virDomainPtr dom) { lxc_driver_t *driver = (lxc_driver_t*)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id); + virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); if (!vm) { lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN, @@ -955,7 +952,7 @@ static int lxcDomainDestroy(virDomainPtr dom) { lxc_driver_t *driver = (lxc_driver_t*)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id); + virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); if (!vm) { lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN, @@ -984,7 +981,7 @@ static int lxcStartup(void) { uid_t uid = getuid(); - virDomainObjPtr vm; + unsigned int i; /* Check that the user is root */ if (0 != uid) { @@ -1021,21 +1018,18 @@ return -1; } - vm = lxc_driver->domains; - while (vm) { + for (i = 0 ; i < lxc_driver->domains.count ; i++) { + virDomainObjPtr vm = lxc_driver->domains.objs[i]; char *config = NULL; virDomainDefPtr tmp; int rc; - if ((vm->monitor = lxcMonitorClient(NULL, lxc_driver, vm)) < 0) { - vm = vm->next; + if ((vm->monitor = lxcMonitorClient(NULL, lxc_driver, vm)) < 0) continue; - } /* Read pid from controller */ if ((rc = virFileReadPid(lxc_driver->stateDir, vm->def->name, &vm->pid)) != 0) { close(vm->monitor); vm->monitor = -1; - vm = vm->next; continue; } @@ -1060,8 +1054,6 @@ close(vm->monitor); vm->monitor = -1; } - - vm = vm->next; } return 0; @@ -1078,15 +1070,10 @@ static int lxcShutdown(void) { - virDomainObjPtr vm; if (lxc_driver == NULL) return(-1); - vm = lxc_driver->domains; - while (vm) { - virDomainObjPtr next = vm->next; - virDomainObjFree(vm); - vm = next; - } + + virDomainObjListFree(&lxc_driver->domains); lxcFreeDriver(lxc_driver); lxc_driver = NULL; @@ -1102,17 +1089,14 @@ */ static int lxcActive(void) { - virDomainObjPtr dom; + unsigned int i; if (lxc_driver == NULL) return(0); - dom = lxc_driver->domains; - while (dom) { - if (virDomainIsActive(dom)) + for (i = 0 ; i < lxc_driver->domains.count ; i++) + if (virDomainIsActive(lxc_driver->domains.objs[i])) return 1; - dom = dom->next; - } /* Otherwise we're happy to deal with a shutdown */ return 0; diff -r 2bb12d98c26b src/openvz_conf.c --- a/src/openvz_conf.c Fri Oct 03 12:45:58 2008 +0100 +++ b/src/openvz_conf.c Fri Oct 03 12:58:03 2008 +0100 @@ -295,18 +295,10 @@ void openvzFreeDriver(struct openvz_driver *driver) { - virDomainObjPtr dom; - if (!driver) return; - dom = driver->domains; - while (dom) { - virDomainObjPtr tmp = dom->next; - virDomainObjFree(dom); - dom = tmp; - } - + virDomainObjListFree(&driver->domains); virCapabilitiesFree(driver->caps); } @@ -317,7 +309,7 @@ int veid, ret; char status[16]; char uuidstr[VIR_UUID_STRING_BUFLEN]; - virDomainObjPtr dom = NULL, prev = NULL; + virDomainObjPtr dom = NULL; char temp[50]; if (openvzAssignUUIDs() < 0) @@ -385,12 +377,12 @@ dom->def->nets = openvzReadNetworkConf(NULL, veid); - if (prev) { - prev->next = dom; - } else { - driver->domains = dom; - } - prev = dom; + if (VIR_REALLOC_N(driver->domains.objs, + driver->domains.count + 1) < 0) + goto no_memory; + + driver->domains.objs[driver->domains.count++] = dom; + dom = NULL; } fclose(fp); diff -r 2bb12d98c26b src/openvz_conf.h --- a/src/openvz_conf.h Fri Oct 03 12:45:58 2008 +0100 +++ b/src/openvz_conf.h Fri Oct 03 12:58:03 2008 +0100 @@ -47,7 +47,7 @@ struct openvz_driver { virCapsPtr caps; - virDomainObjPtr domains; + virDomainObjList domains; }; void openvzError (virConnectPtr conn, virErrorNumber code, const char *fmt, ...) diff -r 2bb12d98c26b src/openvz_driver.c --- a/src/openvz_driver.c Fri Oct 03 12:45:58 2008 +0100 +++ b/src/openvz_driver.c Fri Oct 03 12:58:03 2008 +0100 @@ -157,7 +157,7 @@ virDomainObjPtr vm; virDomainPtr dom; - vm = virDomainFindByID(driver->domains, id); + vm = virDomainFindByID(&driver->domains, id); if (!vm) { openvzError(conn, VIR_ERR_NO_DOMAIN, NULL); @@ -175,7 +175,7 @@ static char *openvzGetOSType(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); char *ret; if (!vm) { @@ -193,7 +193,7 @@ static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, uuid); virDomainPtr dom; if (!vm) { @@ -212,7 +212,7 @@ static virDomainPtr openvzDomainLookupByName(virConnectPtr conn, const char *name) { struct openvz_driver *driver = (struct openvz_driver *)conn->privateData; - virDomainObjPtr vm = virDomainFindByName(driver->domains, name); + virDomainObjPtr vm = virDomainFindByName(&driver->domains, name); virDomainPtr dom; if (!vm) { @@ -231,7 +231,7 @@ static int openvzDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, @@ -260,7 +260,7 @@ static char *openvzDomainDumpXML(virDomainPtr dom, int flags) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, @@ -275,7 +275,7 @@ static int openvzDomainShutdown(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); const char *prog[] = {VZCTL, "--quiet", "stop", vm ? vm->def->name : NULL, NULL}; if (!vm) { @@ -302,7 +302,7 @@ static int openvzDomainReboot(virDomainPtr dom, unsigned int flags ATTRIBUTE_UNUSED) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); const char *prog[] = {VZCTL, "--quiet", "restart", vm ? vm->def->name : NULL, NULL}; if (!vm) { @@ -434,7 +434,7 @@ return NULL; } - vm = virDomainFindByName(driver->domains, vmdef->name); + vm = virDomainFindByName(&driver->domains, vmdef->name); if (vm) { openvzError(conn, VIR_ERR_OPERATION_FAILED, _("Already an OPENVZ VM active with the id '%s'"), @@ -511,7 +511,7 @@ return NULL; } - vm = virDomainFindByName(driver->domains, vmdef->name); + vm = virDomainFindByName(&driver->domains, vmdef->name); if (vm) { openvzError(conn, VIR_ERR_OPERATION_FAILED, _("Already an OPENVZ VM defined with the id '%s'"), @@ -581,7 +581,7 @@ openvzDomainCreate(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByName(driver->domains, dom->name); + virDomainObjPtr vm = virDomainFindByName(&driver->domains, dom->name); const char *prog[] = {VZCTL, "--quiet", "start", vm ? vm->def->name : NULL, NULL }; if (!vm) { @@ -614,7 +614,7 @@ { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); const char *prog[] = { VZCTL, "--quiet", "destroy", vm ? vm->def->name : NULL, NULL }; if (!vm) { @@ -643,7 +643,7 @@ { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); const char *prog[] = { VZCTL, "--quiet", "set", vm ? vm->def->name : NULL, "--onboot", autostart ? "yes" : "no", "--save", NULL }; @@ -666,7 +666,7 @@ { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); char value[1024]; if (!vm) { @@ -703,7 +703,7 @@ static int openvzDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); char str_vcpus[32]; const char *prog[] = { VZCTL, "--quiet", "set", vm ? vm->def->name : NULL, "--cpus", str_vcpus, "--save", NULL }; @@ -844,15 +844,14 @@ return got; } -static int openvzNumDomains(virConnectPtr conn ATTRIBUTE_UNUSED) { +static int openvzNumDomains(virConnectPtr conn) { struct openvz_driver *driver = conn->privateData; - int nactive = 0; - virDomainObjPtr vm = driver->domains; - while (vm) { - if (virDomainIsActive(vm)) + int nactive = 0, i; + + for (i = 0 ; i < driver->domains.count ; i++) + if (virDomainIsActive(driver->domains.objs[i])) nactive++; - vm = vm->next; - } + return nactive; } @@ -944,13 +943,12 @@ static int openvzNumDefinedDomains(virConnectPtr conn) { struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; - int ninactive = 0; - virDomainObjPtr vm = driver->domains; - while (vm) { - if (!virDomainIsActive(vm)) + int ninactive = 0, i; + + for (i = 0 ; i < driver->domains.count ; i++) + if (!virDomainIsActive(driver->domains.objs[i])) ninactive++; - vm = vm->next; - } + return ninactive; } diff -r 2bb12d98c26b src/qemu_conf.h --- a/src/qemu_conf.h Fri Oct 03 12:45:58 2008 +0100 +++ b/src/qemu_conf.h Fri Oct 03 12:58:03 2008 +0100 @@ -51,7 +51,7 @@ unsigned int qemuVersion; int nextvmid; - virDomainObjPtr domains; + virDomainObjList domains; brControl *brctl; char *configDir; diff -r 2bb12d98c26b src/qemu_driver.c --- a/src/qemu_driver.c Fri Oct 03 12:45:58 2008 +0100 +++ b/src/qemu_driver.c Fri Oct 03 12:58:03 2008 +0100 @@ -126,23 +126,18 @@ static struct qemud_driver *qemu_driver = NULL; -static -void qemudAutostartConfigs(struct qemud_driver *driver) { - virDomainObjPtr vm; +static void +qemudAutostartConfigs(struct qemud_driver *driver) { + unsigned int i; - vm = driver->domains; - while (vm != NULL) { - virDomainObjPtr next = vm->next; - - if (vm->autostart && - !virDomainIsActive(vm) && - qemudStartVMDaemon(NULL, driver, vm, NULL) < 0) { + for (i = 0 ; i < driver->domains.count ; i++) { + if (driver->domains.objs[i]->autostart && + !virDomainIsActive(driver->domains.objs[i]) && + qemudStartVMDaemon(NULL, driver, driver->domains.objs[i], NULL) < 0) { virErrorPtr err = virGetLastError(); qemudLog(QEMUD_ERR, _("Failed to autostart VM '%s': %s\n"), - vm->def->name, err->message); + driver->domains.objs[i]->def->name, err->message); } - - vm = next; } } @@ -240,6 +235,9 @@ */ static int qemudReload(void) { + if (!qemu_driver) + return 0; + virDomainLoadAllConfigs(NULL, qemu_driver->caps, &qemu_driver->domains, @@ -261,13 +259,14 @@ */ static int qemudActive(void) { - virDomainObjPtr dom = qemu_driver->domains; + unsigned int i; - while (dom) { - if (virDomainIsActive(dom)) + if (!qemu_driver) + return 0; + + for (i = 0 ; i < qemu_driver->domains.count ; i++) + if (virDomainIsActive(qemu_driver->domains.objs[i])) return 1; - dom = dom->next; - } /* Otherwise we're happy to deal with a shutdown */ return 0; @@ -280,7 +279,7 @@ */ static int qemudShutdown(void) { - virDomainObjPtr vm; + unsigned int i; if (!qemu_driver) return -1; @@ -288,25 +287,16 @@ virCapabilitiesFree(qemu_driver->caps); /* shutdown active VMs */ - vm = qemu_driver->domains; - while (vm) { - virDomainObjPtr next = vm->next; - if (virDomainIsActive(vm)) - qemudShutdownVMDaemon(NULL, qemu_driver, vm); - if (!vm->persistent) + for (i = 0 ; i < qemu_driver->domains.count ; i++) { + virDomainObjPtr dom = qemu_driver->domains.objs[i]; + if (virDomainIsActive(dom)) + qemudShutdownVMDaemon(NULL, qemu_driver, dom); + if (!dom->persistent) virDomainRemoveInactive(&qemu_driver->domains, - vm); - vm = next; + dom); } - /* free inactive VMs */ - vm = qemu_driver->domains; - while (vm) { - virDomainObjPtr next = vm->next; - virDomainObjFree(vm); - vm = next; - } - qemu_driver->domains = NULL; + virDomainObjListFree(&qemu_driver->domains); VIR_FREE(qemu_driver->logDir); VIR_FREE(qemu_driver->configDir); @@ -1036,15 +1026,16 @@ static void qemudDispatchVMEvent(int fd, int events, void *opaque) { struct qemud_driver *driver = (struct qemud_driver *)opaque; - virDomainObjPtr vm = driver->domains; + virDomainObjPtr vm = NULL; + unsigned int i; - while (vm) { - if (virDomainIsActive(vm) && - (vm->stdout_fd == fd || - vm->stderr_fd == fd)) + for (i = 0 ; i < driver->domains.count ; i++) { + if (virDomainIsActive(driver->domains.objs[i]) && + (driver->domains.objs[i]->stdout_fd == fd || + driver->domains.objs[i]->stderr_fd == fd)) { + vm = driver->domains.objs[i]; break; - - vm = vm->next; + } } if (!vm) @@ -1351,7 +1342,7 @@ static virDomainPtr qemudDomainLookupByID(virConnectPtr conn, int id) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; - virDomainObjPtr vm = virDomainFindByID(driver->domains, id); + virDomainObjPtr vm = virDomainFindByID(&driver->domains, id); virDomainPtr dom; if (!vm) { @@ -1366,7 +1357,7 @@ static virDomainPtr qemudDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, uuid); virDomainPtr dom; if (!vm) { @@ -1381,7 +1372,7 @@ static virDomainPtr qemudDomainLookupByName(virConnectPtr conn, const char *name) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; - virDomainObjPtr vm = virDomainFindByName(driver->domains, name); + virDomainObjPtr vm = virDomainFindByName(&driver->domains, name); virDomainPtr dom; if (!vm) { @@ -1427,26 +1418,22 @@ static int qemudListDomains(virConnectPtr conn, int *ids, int nids) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; - virDomainObjPtr vm = driver->domains; - int got = 0; - while (vm && got < nids) { - if (virDomainIsActive(vm)) { - ids[got] = vm->def->id; - got++; - } - vm = vm->next; - } + int got = 0, i; + + for (i = 0 ; i < driver->domains.count && got < nids ; i++) + if (virDomainIsActive(driver->domains.objs[i])) + ids[got++] = driver->domains.objs[i]->def->id; + return got; } static int qemudNumDomains(virConnectPtr conn) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; - int n = 0; - virDomainObjPtr dom = driver->domains; - while (dom) { - if (virDomainIsActive(dom)) + int n = 0, i; + + for (i = 0 ; i < driver->domains.count ; i++) + if (virDomainIsActive(driver->domains.objs[i])) n++; - dom = dom->next; - } + return n; } static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, @@ -1459,7 +1446,7 @@ if (!(def = virDomainDefParseString(conn, driver->caps, xml))) return NULL; - vm = virDomainFindByName(driver->domains, def->name); + vm = virDomainFindByName(&driver->domains, def->name); if (vm) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("domain '%s' is already defined"), @@ -1467,7 +1454,7 @@ virDomainDefFree(def); return NULL; } - vm = virDomainFindByUUID(driver->domains, def->uuid); + vm = virDomainFindByUUID(&driver->domains, def->uuid); if (vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -1501,7 +1488,7 @@ static int qemudDomainSuspend(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; char *info; - virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id); + virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching id %d"), dom->id); return -1; @@ -1529,7 +1516,7 @@ static int qemudDomainResume(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; char *info; - virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id); + virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching id %d"), dom->id); @@ -1556,7 +1543,7 @@ static int qemudDomainShutdown(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id); + virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); char* info; if (!vm) { @@ -1577,7 +1564,7 @@ static int qemudDomainDestroy(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id); + virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -1596,7 +1583,7 @@ static char *qemudDomainGetOSType(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); char *type; if (!vm) { @@ -1616,7 +1603,7 @@ /* Returns max memory in kb, 0 if error */ static unsigned long qemudDomainGetMaxMemory(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -1632,7 +1619,7 @@ static int qemudDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -1655,7 +1642,7 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -1685,7 +1672,7 @@ static int qemudDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); @@ -1806,7 +1793,7 @@ static int qemudDomainSave(virDomainPtr dom, const char *path) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id); + virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); char *command, *info; int fd; char *safe_path; @@ -1926,8 +1913,8 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { - const struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); int max; if (!vm) { @@ -1970,7 +1957,7 @@ unsigned char *cpumap, int maplen) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); cpu_set_t mask; int i, maxcpu; virNodeInfo nodeinfo; @@ -2023,7 +2010,7 @@ unsigned char *cpumaps, int maplen) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); virNodeInfo nodeinfo; int i, v, maxcpu; @@ -2088,7 +2075,7 @@ static int qemudDomainGetMaxVcpus(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); const char *type; int ret; @@ -2181,9 +2168,9 @@ VIR_FREE(xml); /* Ensure the name and UUID don't already exist in an active VM */ - vm = virDomainFindByUUID(driver->domains, def->uuid); + vm = virDomainFindByUUID(&driver->domains, def->uuid); if (!vm) - vm = virDomainFindByName(driver->domains, def->name); + vm = virDomainFindByName(&driver->domains, def->name); if (vm && virDomainIsActive(vm)) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("domain is already active as '%s'"), vm->def->name); @@ -2234,7 +2221,7 @@ static char *qemudDomainDumpXML(virDomainPtr dom, int flags ATTRIBUTE_UNUSED) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); @@ -2251,19 +2238,18 @@ static int qemudListDefinedDomains(virConnectPtr conn, char **const names, int nnames) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; - virDomainObjPtr vm = driver->domains; int got = 0, i; - while (vm && got < nnames) { - if (!virDomainIsActive(vm)) { - if (!(names[got] = strdup(vm->def->name))) { + + for (i = 0 ; i < driver->domains.count && got < nnames ; i++) { + if (!virDomainIsActive(driver->domains.objs[i])) { + if (!(names[got++] = strdup(driver->domains.objs[i]->def->name))) { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, - "%s", _("failed to allocate space for VM name string")); + "%s", _("failed to allocate space for VM name string")); goto cleanup; } - got++; } - vm = vm->next; } + return got; cleanup: @@ -2272,23 +2258,21 @@ return -1; } - static int qemudNumDefinedDomains(virConnectPtr conn) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; - int n = 0; - virDomainObjPtr dom = driver->domains; - while (dom) { - if (!virDomainIsActive(dom)) + int n = 0, i; + + for (i = 0 ; i < driver->domains.count ; i++) + if (!virDomainIsActive(driver->domains.objs[i])) n++; - dom = dom->next; - } + return n; } static int qemudDomainStart(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -2332,7 +2316,7 @@ static int qemudDomainUndefine(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -2408,7 +2392,7 @@ virDomainDeviceDefPtr dev) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); virDomainDiskDefPtr origdisk, newdisk; char *cmd, *reply, *safe_path; char *devname = NULL; @@ -2525,7 +2509,7 @@ static int qemudDomainAttachUsbMassstorageDevice(virDomainPtr dom, virDomainDeviceDefPtr dev) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); int ret; char *cmd, *reply; virDomainDiskDefPtr *dest, *prev, ptr; @@ -2595,7 +2579,7 @@ static int qemudDomainAttachHostDevice(virDomainPtr dom, virDomainDeviceDefPtr dev) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); int ret; char *cmd, *reply; @@ -2650,7 +2634,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom, const char *xml) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); virDomainDeviceDefPtr dev; int ret = 0; @@ -2696,7 +2680,7 @@ static int qemudDomainGetAutostart(virDomainPtr dom, int *autostart) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -2712,7 +2696,7 @@ static int qemudDomainSetAutostart(virDomainPtr dom, int autostart) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); char *configFile = NULL, *autostartLink = NULL; int ret = -1; @@ -2782,13 +2766,13 @@ const char *path, struct _virDomainBlockStats *stats) { - const struct qemud_driver *driver = + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; char *dummy, *info; const char *p, *eol; char qemu_dev_name[32]; size_t len; - const virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id); + const virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); if (!vm) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -2922,7 +2906,7 @@ { #ifdef __linux__ struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id); + virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); virDomainNetDefPtr net; if (!vm) { @@ -2970,7 +2954,7 @@ unsigned int flags ATTRIBUTE_UNUSED) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); virDomainDiskDefPtr disk; int fd, ret = -1; @@ -3028,7 +3012,7 @@ unsigned int flags) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(driver->domains, dom->id); + virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); char cmd[256], *info; char tmp[] = TEMPDIR "/qemu.mem.XXXXXX"; int fd = -1, ret = -1; diff -r 2bb12d98c26b src/test.c --- a/src/test.c Fri Oct 03 12:45:58 2008 +0100 +++ b/src/test.c Fri Oct 03 12:58:03 2008 +0100 @@ -57,7 +57,7 @@ int nextDomID; virCapsPtr caps; virNodeInfo nodeInfo; - virDomainObjPtr domains; + virDomainObjList domains; virNetworkObjPtr networks; int numCells; testCell cells[MAX_CELLS]; @@ -86,8 +86,8 @@ \ privconn = (testConnPtr)dom->conn->privateData; \ do { \ - if ((privdom = virDomainFindByName(privconn->domains, \ - (dom)->name)) == NULL) { \ + if ((privdom = virDomainFindByName(&privconn->domains, \ + (dom)->name)) == NULL) { \ testError((dom)->conn, (dom), NULL, VIR_ERR_INVALID_ARG, \ __FUNCTION__); \ return (ret); \ @@ -272,7 +272,7 @@ return VIR_DRV_OPEN_SUCCESS; error: - virDomainObjFree(privconn->domains); + virDomainObjListFree(&privconn->domains); virNetworkObjFree(privconn->networks); virCapabilitiesFree(privconn->caps); VIR_FREE(privconn); @@ -507,12 +507,7 @@ VIR_FREE(networks); if (fd != -1) close(fd); - dom = privconn->domains; - while (dom) { - virDomainObjPtr tmp = dom->next; - virDomainObjFree(dom); - dom = tmp; - } + virDomainObjListFree(&privconn->domains); net = privconn->networks; while (net) { virNetworkObjPtr tmp = net->next; @@ -565,16 +560,10 @@ static int testClose(virConnectPtr conn) { - virDomainObjPtr dom; virNetworkObjPtr net; GET_CONNECTION(conn); virCapabilitiesFree(privconn->caps); - dom = privconn->domains; - while (dom) { - virDomainObjPtr tmp = dom->next; - virDomainObjFree(dom); - dom = tmp; - } + virDomainObjListFree(&privconn->domains); net = privconn->networks; while (net) { virNetworkObjPtr tmp = net->next; @@ -652,16 +641,13 @@ static int testNumOfDomains(virConnectPtr conn) { - int numActive = 0; - virDomainObjPtr dom; + unsigned int numActive = 0, i; GET_CONNECTION(conn); - dom = privconn->domains; - while (dom) { - if (virDomainIsActive(dom)) + for (i = 0 ; i < privconn->domains.count ; i++) + if (virDomainIsActive(privconn->domains.objs[i])) numActive++; - dom = dom->next; - } + return numActive; } @@ -700,7 +686,7 @@ virDomainPtr ret; GET_CONNECTION(conn); - if ((dom = virDomainFindByID(privconn->domains, id)) == NULL) { + if ((dom = virDomainFindByID(&privconn->domains, id)) == NULL) { testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); return NULL; } @@ -719,7 +705,7 @@ virDomainObjPtr dom = NULL; GET_CONNECTION(conn); - if ((dom = virDomainFindByUUID(privconn->domains, uuid)) == NULL) { + if ((dom = virDomainFindByUUID(&privconn->domains, uuid)) == NULL) { testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); return NULL; } @@ -738,7 +724,7 @@ virDomainObjPtr dom = NULL; GET_CONNECTION(conn); - if ((dom = virDomainFindByName(privconn->domains, name)) == NULL) { + if ((dom = virDomainFindByName(&privconn->domains, name)) == NULL) { testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); return NULL; } @@ -754,16 +740,13 @@ int *ids, int maxids) { - int n = 0; - virDomainObjPtr dom; + unsigned int n = 0, i; GET_CONNECTION(conn); - dom = privconn->domains; - while (dom && n < maxids) { - if (virDomainIsActive(dom)) - ids[n++] = dom->def->id; - dom = dom->next; - } + for (i = 0 ; i < privconn->domains.count && n < maxids ; i++) + if (virDomainIsActive(privconn->domains.objs[i])) + ids[n++] = privconn->domains.objs[i]->def->id; + return n; } @@ -1107,34 +1090,28 @@ } static int testNumOfDefinedDomains(virConnectPtr conn) { - int numInactive = 0; - virDomainObjPtr dom; + unsigned int numInactive = 0, i; GET_CONNECTION(conn); - dom = privconn->domains; - while (dom) { - if (!virDomainIsActive(dom)) + for (i = 0 ; i < privconn->domains.count ; i++) + if (!virDomainIsActive(privconn->domains.objs[i])) numInactive++; - dom = dom->next; - } + return numInactive; } static int testListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) { - int n = 0; - virDomainObjPtr dom; + unsigned int n = 0, i; GET_CONNECTION(conn); - dom = privconn->domains; memset(names, 0, sizeof(*names)*maxnames); - while (dom && n < maxnames) { - if (!virDomainIsActive(dom) && - !(names[n++] = strdup(dom->def->name))) + for (i = 0 ; i < privconn->domains.count && n < maxnames ; i++) + if (!virDomainIsActive(privconn->domains.objs[i]) && + !(names[n++] = strdup(privconn->domains.objs[i]->def->name))) goto no_memory; - dom = dom->next; - } + return n; no_memory: -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Fri, Oct 03, 2008 at 01:25:17PM +0100, Daniel P. Berrange wrote:
This patch removes the linked list in the virDomainObjPtr object, and adds a new virDomainObjList struct to track domains as an array. The QEMU, LXC, OpenVZ and Test drivers get updated to account for this API change.
Hum, the only danger I see is that then you can't garantee the virDomainObjPtr value for parallel access as the (semi-frequent) realloc is likely to change them. You then need to take the global lock before trying to access this structure and never cache its value. I guess that's fine though considering the expected usage. But each driver still need to access those structures directly, with the future goal of having driver loaded as shared lib modules maybe that interface should be defined as a few entry points to add, remove, lookup and iterate (with a callback). The refactoring here might be a good opportunity to hide the implementation from driver code. Opinion ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Mon, Oct 06, 2008 at 04:45:13PM +0200, Daniel Veillard wrote:
On Fri, Oct 03, 2008 at 01:25:17PM +0100, Daniel P. Berrange wrote:
This patch removes the linked list in the virDomainObjPtr object, and adds a new virDomainObjList struct to track domains as an array. The QEMU, LXC, OpenVZ and Test drivers get updated to account for this API change.
Hum, the only danger I see is that then you can't garantee the virDomainObjPtr value for parallel access as the (semi-frequent) realloc is likely to change them. You then need to take the global lock before trying to access this structure and never cache its value. I guess that's fine though considering the expected usage.
Yes, you can. The list doesn't store the objects themselves, only the pointers to the objects. So, once you have a locked object instance, you can re-alloc the list at will and it won't be change the object. I'll have a proof of concept for the QEMU driver demoing this soon, but the general rule will be thus: To use an existing domain object - lock the driver - find the virDomainObjPtr you need and lock it - unlock the driver - do work with the virDomainObjPtr - ulock the virDomainObjPtr To add a new domain object - lock the driver - create the virDomainObjPtr and lock it - unlock the driver - start the VM / save the config - unlock the virDomainObjPtr To delete a domain object - lock the driver - find the virDomainObjPtr you need and lock it - ulock the virDomainObjPtr - delete the virDomainObjPtr - unlock the driver NB, the lock + immediate unlock of the virDomainObjPtr ensures that no other thread still holds a refernce to the object we're about to delete, and they can't re-acquire one until we unlock the driver. That gives reasonably fine grained locking that works for all driver APIs. In some API calls which can take a long time to complete though, we need to unlock & relock the virDomainObjPtr several times around long running APIs calls.
But each driver still need to access those structures directly, with the future goal of having driver loaded as shared lib modules maybe that interface should be defined as a few entry points to add, remove, lookup and iterate (with a callback). The refactoring here might be a good opportunity to hide the implementation from driver code. Opinion ?
This won't really gain us anything from point of view of thread safety, and I don't think it magically solves any compatability issues wrt to loadable drivers. This is another topic entirely, but when we do have loadable modules, my feeling is to require that they all be built in-tree, and not allow (potentially closed-source) out of tree modules. If we mandate that they're in tree then there's no ABI compatability to worry about. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Mon, Oct 06, 2008 at 04:04:43PM +0100, Daniel P. Berrange wrote:
But each driver still need to access those structures directly, with the future goal of having driver loaded as shared lib modules maybe that interface should be defined as a few entry points to add, remove, lookup and iterate (with a callback). The refactoring here might be a good opportunity to hide the implementation from driver code. Opinion ?
This won't really gain us anything from point of view of thread safety, and I don't think it magically solves any compatability issues wrt to loadable drivers.
This is another topic entirely, but when we do have loadable modules, my feeling is to require that they all be built in-tree, and not allow (potentially closed-source) out of tree modules. If we mandate that they're in tree then there's no ABI compatability to worry about.
Well if you're afraid of closed source modules that won't be possible because in the end it's the daemon which loads them and as we have noticed it will end up being GPLed since we will link some GPL bits like the Hal library in the daemon. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

This patch removes the linked list in the virNetworkObjPtr object, and adds a new virNetworkObjList struct to track domains as an array. The virtual network & test drivers get updated to account for this API change. network_conf.c | 88 +++++++++++++++++++++++----------------- network_conf.h | 20 +++++---- network_driver.c | 120 ++++++++++++++++++++++++------------------------------- test.c | 75 ++++++++++++---------------------- 4 files changed, 145 insertions(+), 158 deletions(-) Daniel diff -r 25b5be9d400e src/network_conf.c --- a/src/network_conf.c Fri Oct 03 12:58:03 2008 +0100 +++ b/src/network_conf.c Fri Oct 03 12:58:27 2008 +0100 @@ -69,28 +69,26 @@ } -virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjPtr nets, +virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjListPtr nets, const unsigned char *uuid) { - virNetworkObjPtr net = nets; - while (net) { - if (!memcmp(net->def->uuid, uuid, VIR_UUID_BUFLEN)) - return net; - net = net->next; - } + unsigned int i; + + for (i = 0 ; i < nets->count ; i++) + if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) + return nets->objs[i]; return NULL; } -virNetworkObjPtr virNetworkFindByName(const virNetworkObjPtr nets, +virNetworkObjPtr virNetworkFindByName(const virNetworkObjListPtr nets, const char *name) { - virNetworkObjPtr net = nets; - while (net) { - if (STREQ(net->def->name, name)) - return net; - net = net->next; - } + unsigned int i; + + for (i = 0 ; i < nets->count ; i++) + if (STREQ(nets->objs[i]->def->name, name)) + return nets->objs[i]; return NULL; } @@ -141,13 +139,24 @@ VIR_FREE(net); } +void virNetworkObjListFree(virNetworkObjListPtr nets) +{ + unsigned int i; + + for (i = 0 ; i < nets->count ; i++) + virNetworkObjFree(nets->objs[i]); + + VIR_FREE(nets->objs); + nets->count = 0; +} + virNetworkObjPtr virNetworkAssignDef(virConnectPtr conn, - virNetworkObjPtr *nets, + virNetworkObjListPtr nets, const virNetworkDefPtr def) { virNetworkObjPtr network; - if ((network = virNetworkFindByName(*nets, def->name))) { + if ((network = virNetworkFindByName(nets, def->name))) { if (!virNetworkIsActive(network)) { virNetworkDefFree(network->def); network->def = def; @@ -166,34 +175,41 @@ } network->def = def; - network->next = *nets; - *nets = network; + if (VIR_REALLOC_N(nets->objs, nets->count + 1) < 0) { + virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL); + VIR_FREE(network); + return NULL; + } + + nets->objs[nets->count] = network; + nets->count++; return network; } -void virNetworkRemoveInactive(virNetworkObjPtr *nets, +void virNetworkRemoveInactive(virNetworkObjListPtr nets, const virNetworkObjPtr net) { - virNetworkObjPtr prev = NULL; - virNetworkObjPtr curr = *nets; + unsigned int i; - while (curr && - curr != net) { - prev = curr; - curr = curr->next; + for (i = 0 ; i < nets->count ; i++) { + if (nets->objs[i] == net) { + virNetworkObjFree(nets->objs[i]); + + if (i < (nets->count - 1)) + memmove(nets->objs + i, nets->objs + i + 1, + sizeof(*(nets->objs)) * (nets->count - (i + 1))); + + if (VIR_REALLOC_N(nets->objs, nets->count - 1) < 0) { + ; /* Failure to reduce memory allocation isn't fatal */ + } + nets->count--; + + break; + } } - - if (curr) { - if (prev) - prev->next = curr->next; - else - *nets = curr->next; - } - - virNetworkObjFree(net); } @@ -699,7 +715,7 @@ } virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn, - virNetworkObjPtr *nets, + virNetworkObjListPtr nets, const char *configDir, const char *autostartDir, const char *file) @@ -753,7 +769,7 @@ } int virNetworkLoadAllConfigs(virConnectPtr conn, - virNetworkObjPtr *nets, + virNetworkObjListPtr nets, const char *configDir, const char *autostartDir) { diff -r 25b5be9d400e src/network_conf.h --- a/src/network_conf.h Fri Oct 03 12:58:03 2008 +0100 +++ b/src/network_conf.h Fri Oct 03 12:58:27 2008 +0100 @@ -88,8 +88,13 @@ virNetworkDefPtr def; /* The current definition */ virNetworkDefPtr newDef; /* New definition to activate at shutdown */ +}; - virNetworkObjPtr next; +typedef struct _virNetworkObjList virNetworkObjList; +typedef virNetworkObjList *virNetworkObjListPtr; +struct _virNetworkObjList { + unsigned int count; + virNetworkObjPtr *objs; }; static inline int @@ -99,19 +104,20 @@ } -virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjPtr nets, +virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjListPtr nets, const unsigned char *uuid); -virNetworkObjPtr virNetworkFindByName(const virNetworkObjPtr nets, +virNetworkObjPtr virNetworkFindByName(const virNetworkObjListPtr nets, const char *name); void virNetworkDefFree(virNetworkDefPtr def); void virNetworkObjFree(virNetworkObjPtr net); +void virNetworkObjListFree(virNetworkObjListPtr vms); virNetworkObjPtr virNetworkAssignDef(virConnectPtr conn, - virNetworkObjPtr *nets, + virNetworkObjListPtr nets, const virNetworkDefPtr def); -void virNetworkRemoveInactive(virNetworkObjPtr *nets, +void virNetworkRemoveInactive(virNetworkObjListPtr nets, const virNetworkObjPtr net); virNetworkDefPtr virNetworkDefParseString(virConnectPtr conn, @@ -132,13 +138,13 @@ virNetworkObjPtr net); virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn, - virNetworkObjPtr *nets, + virNetworkObjListPtr nets, const char *configDir, const char *autostartDir, const char *file); int virNetworkLoadAllConfigs(virConnectPtr conn, - virNetworkObjPtr *nets, + virNetworkObjListPtr nets, const char *configDir, const char *autostartDir); diff -r 25b5be9d400e src/network_driver.c --- a/src/network_driver.c Fri Oct 03 12:58:03 2008 +0100 +++ b/src/network_driver.c Fri Oct 03 12:58:27 2008 +0100 @@ -58,7 +58,7 @@ /* Main driver state */ struct network_driver { - virNetworkObjPtr networks; + virNetworkObjList networks; iptablesContext *iptables; brControl *brctl; @@ -114,23 +114,18 @@ static struct network_driver *driverState = NULL; -static -void networkAutostartConfigs(struct network_driver *driver) { - virNetworkObjPtr network; +static void +networkAutostartConfigs(struct network_driver *driver) { + unsigned int i; - network = driver->networks; - while (network != NULL) { - virNetworkObjPtr next = network->next; - - if (network->autostart && - !virNetworkIsActive(network) && - networkStartNetworkDaemon(NULL, driver, network) < 0) { + for (i = 0 ; i < driver->networks.count ; i++) { + if (driver->networks.objs[i]->autostart && + !virNetworkIsActive(driver->networks.objs[i]) && + networkStartNetworkDaemon(NULL, driver, driver->networks.objs[i]) < 0) { virErrorPtr err = virGetLastError(); networkLog(NETWORK_ERR, _("Failed to autostart network '%s': %s\n"), - network->def->name, err->message); + driver->networks.objs[i]->def->name, err->message); } - - network = next; } } @@ -212,6 +207,9 @@ */ static int networkReload(void) { + if (!driverState) + return 0; + virNetworkLoadAllConfigs(NULL, &driverState->networks, driverState->networkConfigDir, @@ -238,13 +236,14 @@ */ static int networkActive(void) { - virNetworkObjPtr net = driverState->networks; + unsigned int i; - while (net) { - if (virNetworkIsActive(net)) + if (!driverState) + return 0; + + for (i = 0 ; i < driverState->networks.count ; i++) + if (virNetworkIsActive(driverState->networks.objs[i])) return 1; - net = net->next; - } /* Otherwise we're happy to deal with a shutdown */ return 0; @@ -257,28 +256,19 @@ */ static int networkShutdown(void) { - virNetworkObjPtr network; + unsigned int i; if (!driverState) return -1; /* shutdown active networks */ - network = driverState->networks; - while (network) { - virNetworkObjPtr next = network->next; - if (virNetworkIsActive(network)) - networkShutdownNetworkDaemon(NULL, driverState, network); - network = next; - } + for (i = 0 ; i < driverState->networks.count ; i++) + if (virNetworkIsActive(driverState->networks.objs[i])) + networkShutdownNetworkDaemon(NULL, driverState, + driverState->networks.objs[i]); /* free inactive networks */ - network = driverState->networks; - while (network) { - virNetworkObjPtr next = network->next; - virNetworkObjFree(network); - network = next; - } - driverState->networks = NULL; + virNetworkObjListFree(&driverState->networks); VIR_FREE(driverState->logDir); VIR_FREE(driverState->networkConfigDir); @@ -840,7 +830,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr conn ATTRIBUTE_UNUSED, const unsigned char *uuid) { struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, uuid); + virNetworkObjPtr network = virNetworkFindByUUID(&driver->networks, uuid); virNetworkPtr net; if (!network) { @@ -855,7 +845,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr conn ATTRIBUTE_UNUSED, const char *name) { struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByName(driver->networks, name); + virNetworkObjPtr network = virNetworkFindByName(&driver->networks, name); virNetworkPtr net; if (!network) { @@ -885,31 +875,29 @@ } static int networkNumNetworks(virConnectPtr conn) { - int nactive = 0; + int nactive = 0, i; struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; - virNetworkObjPtr net = driver->networks; - while (net) { - if (virNetworkIsActive(net)) + + for (i = 0 ; i < driver->networks.count ; i++) + if (virNetworkIsActive(driver->networks.objs[i])) nactive++; - net = net->next; - } + return nactive; } static int networkListNetworks(virConnectPtr conn, char **const names, int nnames) { struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; - virNetworkObjPtr network = driver->networks; int got = 0, i; - while (network && got < nnames) { - if (virNetworkIsActive(network)) { - if (!(names[got] = strdup(network->def->name))) { + + for (i = 0 ; i < driver->networks.count && got < nnames ; i++) { + if (virNetworkIsActive(driver->networks.objs[i])) { + if (!(names[got] = strdup(driver->networks.objs[i]->def->name))) { networkReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, - "%s", _("failed to allocate space for VM name string")); + "%s", _("failed to allocate space for VM name string")); goto cleanup; } got++; } - network = network->next; } return got; @@ -920,31 +908,29 @@ } static int networkNumDefinedNetworks(virConnectPtr conn) { - int ninactive = 0; + int ninactive = 0, i; struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; - virNetworkObjPtr net = driver->networks; - while (net) { - if (!virNetworkIsActive(net)) + + for (i = 0 ; i < driver->networks.count ; i++) + if (!virNetworkIsActive(driver->networks.objs[i])) ninactive++; - net = net->next; - } + return ninactive; } static int networkListDefinedNetworks(virConnectPtr conn, char **const names, int nnames) { struct network_driver *driver = (struct network_driver *)conn->networkPrivateData; - virNetworkObjPtr network = driver->networks; int got = 0, i; - while (network && got < nnames) { - if (!virNetworkIsActive(network)) { - if (!(names[got] = strdup(network->def->name))) { + + for (i = 0 ; i < driver->networks.count && got < nnames ; i++) { + if (!virNetworkIsActive(driver->networks.objs[i])) { + if (!(names[got] = strdup(driver->networks.objs[i]->def->name))) { networkReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, - "%s", _("failed to allocate space for VM name string")); + "%s", _("failed to allocate space for VM name string")); goto cleanup; } got++; } - network = network->next; } return got; @@ -1009,7 +995,7 @@ static int networkUndefine(virNetworkPtr net) { struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + virNetworkObjPtr network = virNetworkFindByUUID(&driver->networks, net->uuid); if (!network) { networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_DOMAIN, @@ -1034,7 +1020,7 @@ static int networkStart(virNetworkPtr net) { struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + virNetworkObjPtr network = virNetworkFindByUUID(&driver->networks, net->uuid); if (!network) { networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, @@ -1047,7 +1033,7 @@ static int networkDestroy(virNetworkPtr net) { struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + virNetworkObjPtr network = virNetworkFindByUUID(&driver->networks, net->uuid); int ret; if (!network) { @@ -1063,7 +1049,7 @@ static char *networkDumpXML(virNetworkPtr net, int flags ATTRIBUTE_UNUSED) { struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + virNetworkObjPtr network = virNetworkFindByUUID(&driver->networks, net->uuid); if (!network) { networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, @@ -1076,7 +1062,7 @@ static char *networkGetBridgeName(virNetworkPtr net) { struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + virNetworkObjPtr network = virNetworkFindByUUID(&driver->networks, net->uuid); char *bridge; if (!network) { networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, @@ -1096,7 +1082,7 @@ static int networkGetAutostart(virNetworkPtr net, int *autostart) { struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + virNetworkObjPtr network = virNetworkFindByUUID(&driver->networks, net->uuid); if (!network) { networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, @@ -1112,7 +1098,7 @@ static int networkSetAutostart(virNetworkPtr net, int autostart) { struct network_driver *driver = (struct network_driver *)net->conn->networkPrivateData; - virNetworkObjPtr network = virNetworkFindByUUID(driver->networks, net->uuid); + virNetworkObjPtr network = virNetworkFindByUUID(&driver->networks, net->uuid); if (!network) { networkReportError(net->conn, NULL, net, VIR_ERR_INVALID_NETWORK, diff -r 25b5be9d400e src/test.c --- a/src/test.c Fri Oct 03 12:58:03 2008 +0100 +++ b/src/test.c Fri Oct 03 12:58:27 2008 +0100 @@ -58,7 +58,7 @@ virCapsPtr caps; virNodeInfo nodeInfo; virDomainObjList domains; - virNetworkObjPtr networks; + virNetworkObjList networks; int numCells; testCell cells[MAX_CELLS]; }; @@ -100,7 +100,7 @@ \ privconn = (testConnPtr)net->conn->privateData; \ do { \ - if ((privnet = virNetworkFindByName(privconn->networks, \ + if ((privnet = virNetworkFindByName(&privconn->networks, \ (net)->name)) == NULL) { \ testError((net)->conn, NULL, (net), VIR_ERR_INVALID_ARG, \ __FUNCTION__); \ @@ -273,7 +273,7 @@ error: virDomainObjListFree(&privconn->domains); - virNetworkObjFree(privconn->networks); + virNetworkObjListFree(&privconn->networks); virCapabilitiesFree(privconn->caps); VIR_FREE(privconn); return VIR_DRV_OPEN_ERROR; @@ -508,12 +508,7 @@ if (fd != -1) close(fd); virDomainObjListFree(&privconn->domains); - net = privconn->networks; - while (net) { - virNetworkObjPtr tmp = net->next; - virNetworkObjFree(net); - net = tmp; - } + virNetworkObjListFree(&privconn->networks); VIR_FREE(privconn); conn->privateData = NULL; return VIR_DRV_OPEN_ERROR; @@ -560,16 +555,12 @@ static int testClose(virConnectPtr conn) { - virNetworkObjPtr net; GET_CONNECTION(conn); + virCapabilitiesFree(privconn->caps); virDomainObjListFree(&privconn->domains); - net = privconn->networks; - while (net) { - virNetworkObjPtr tmp = net->next; - virNetworkObjFree(net); - net = tmp; - } + virNetworkObjListFree(&privconn->networks); + VIR_FREE (privconn); conn->privateData = conn; return 0; @@ -1293,7 +1284,7 @@ virNetworkObjPtr net = NULL; GET_CONNECTION(conn); - if ((net = virNetworkFindByUUID(privconn->networks, uuid)) == NULL) { + if ((net = virNetworkFindByUUID(&privconn->networks, uuid)) == NULL) { testError (conn, NULL, NULL, VIR_ERR_NO_NETWORK, NULL); return NULL; } @@ -1307,7 +1298,7 @@ virNetworkObjPtr net = NULL; GET_CONNECTION(conn); - if ((net = virNetworkFindByName(privconn->networks, name)) == NULL) { + if ((net = virNetworkFindByName(&privconn->networks, name)) == NULL) { testError (conn, NULL, NULL, VIR_ERR_NO_NETWORK, NULL); return NULL; } @@ -1317,32 +1308,26 @@ static int testNumNetworks(virConnectPtr conn) { - int numActive = 0; - virNetworkObjPtr net; + int numActive = 0, i; GET_CONNECTION(conn); - net = privconn->networks; - while (net) { - if (virNetworkIsActive(net)) + for (i = 0 ; i < privconn->networks.count ; i++) + if (virNetworkIsActive(privconn->networks.objs[i])) numActive++; - net = net->next; - } + return numActive; } static int testListNetworks(virConnectPtr conn, char **const names, int nnames) { - int n = 0; - virNetworkObjPtr net; + int n = 0, i; GET_CONNECTION(conn); - net = privconn->networks; memset(names, 0, sizeof(*names)*nnames); - while (net && n < nnames) { - if (virNetworkIsActive(net) && - !(names[n++] = strdup(net->def->name))) + for (i = 0 ; i < privconn->networks.count && n < nnames ; i++) + if (virNetworkIsActive(privconn->networks.objs[i]) && + !(names[n++] = strdup(privconn->networks.objs[i]->def->name))) goto no_memory; - net = net->next; - } + return n; no_memory: @@ -1353,32 +1338,26 @@ } static int testNumDefinedNetworks(virConnectPtr conn) { - int numInactive = 0; - virNetworkObjPtr net; + int numInactive = 0, i; GET_CONNECTION(conn); - net = privconn->networks; - while (net) { - if (!virNetworkIsActive(net)) + for (i = 0 ; i < privconn->networks.count ; i++) + if (!virNetworkIsActive(privconn->networks.objs[i])) numInactive++; - net = net->next; - } + return numInactive; } static int testListDefinedNetworks(virConnectPtr conn, char **const names, int nnames) { - int n = 0; - virNetworkObjPtr net; + int n = 0, i; GET_CONNECTION(conn); - net = privconn->networks; memset(names, 0, sizeof(*names)*nnames); - while (net && n < nnames) { - if (!virNetworkIsActive(net) && - !(names[n++] = strdup(net->def->name))) + for (i = 0 ; i < privconn->networks.count && n < nnames ; i++) + if (!virNetworkIsActive(privconn->networks.objs[i]) && + !(names[n++] = strdup(privconn->networks.objs[i]->def->name))) goto no_memory; - net = net->next; - } + return n; no_memory: -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Fri, Oct 03, 2008 at 01:26:37PM +0100, Daniel P. Berrange wrote:
This patch removes the linked list in the virNetworkObjPtr object, and adds a new virNetworkObjList struct to track domains as an array. The virtual network & test drivers get updated to account for this API change.
Looks fine to me, +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

This patch removes the linked list in the virStoragePoolObjPtr and and virStorageVolDefPtr objects, and adds new virStoragePoolObjList and virStorageVolDefList structs to track pools & vols as arrays. The storage driver driver gets updated to account for this API change. storage_backend_disk.c | 10 + storage_backend_fs.c | 75 +++++-------- storage_backend_iscsi.c | 9 + storage_backend_logical.c | 11 +- storage_conf.c | 165 ++++++++++++++++-------------- storage_conf.h | 37 ++++-- storage_driver.c | 253 ++++++++++++++++++++++------------------------ 7 files changed, 290 insertions(+), 270 deletions(-) Daniel diff -r 03f1715be403 src/storage_backend_disk.c --- a/src/storage_backend_disk.c Fri Oct 03 12:58:28 2008 +0100 +++ b/src/storage_backend_disk.c Fri Oct 03 13:07:54 2008 +0100 @@ -185,9 +185,13 @@ return -1; } - vol->next = pool->volumes; - pool->volumes = vol; - pool->nvolumes++; + if (VIR_REALLOC_N(pool->volumes.objs, + pool->volumes.count+1) < 0) { + virStorageReportError(conn, VIR_ERR_NO_MEMORY, _("volume")); + virStorageVolDefFree(vol); + return -1; + } + pool->volumes.objs[pool->volumes.count++] = vol; /* Prepended path will be same for all partitions, so we can * strip the path to form a reasonable pool-unique name diff -r 03f1715be403 src/storage_backend_fs.c --- a/src/storage_backend_fs.c Fri Oct 03 12:58:28 2008 +0100 +++ b/src/storage_backend_fs.c Fri Oct 03 13:07:54 2008 +0100 @@ -822,6 +822,7 @@ DIR *dir; struct dirent *ent; struct statvfs sb; + virStorageVolDefPtr vol = NULL; if (!(dir = opendir(pool->def->target.path))) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, @@ -831,61 +832,42 @@ } while ((ent = readdir(dir)) != NULL) { - virStorageVolDefPtr vol; int ret; - if (VIR_ALLOC(vol) < 0) { - virStorageReportError(conn, VIR_ERR_NO_MEMORY, - "%s", _("volume")); - goto cleanup; - } + if (VIR_ALLOC(vol) < 0) + goto no_memory; - vol->name = strdup(ent->d_name); - if (vol->name == NULL) { - VIR_FREE(vol); - virStorageReportError(conn, VIR_ERR_NO_MEMORY, - "%s", _("volume name")); - goto cleanup; - } + if ((vol->name = strdup(ent->d_name)) == NULL) + goto no_memory; vol->target.format = VIR_STORAGE_VOL_RAW; /* Real value is filled in during probe */ if (VIR_ALLOC_N(vol->target.path, strlen(pool->def->target.path) + - 1 + strlen(vol->name) + 1) < 0) { - VIR_FREE(vol->target.path); - VIR_FREE(vol); - virStorageReportError(conn, VIR_ERR_NO_MEMORY, - "%s", _("volume name")); - goto cleanup; - } + 1 + strlen(vol->name) + 1) < 0) + goto no_memory; + strcpy(vol->target.path, pool->def->target.path); strcat(vol->target.path, "/"); strcat(vol->target.path, vol->name); - if ((vol->key = strdup(vol->target.path)) == NULL) { - VIR_FREE(vol->name); - VIR_FREE(vol->target.path); - VIR_FREE(vol); - virStorageReportError(conn, VIR_ERR_NO_MEMORY, - "%s", _("volume key")); - goto cleanup; + if ((vol->key = strdup(vol->target.path)) == NULL) + goto no_memory; + + if ((ret = virStorageBackendProbeFile(conn, vol) < 0)) { + if (ret == -1) + goto no_memory; + else { + /* Silently ignore non-regular files, + * eg '.' '..', 'lost+found' */ + virStorageVolDefFree(vol); + vol = NULL; + continue; + } } - if ((ret = virStorageBackendProbeFile(conn, vol) < 0)) { - VIR_FREE(vol->key); - VIR_FREE(vol->name); - VIR_FREE(vol->target.path); - VIR_FREE(vol); - if (ret == -1) - goto cleanup; - else - /* Silently ignore non-regular files, - * eg '.' '..', 'lost+found' */ - continue; - } - - vol->next = pool->volumes; - pool->volumes = vol; - pool->nvolumes++; - continue; + if (VIR_REALLOC_N(pool->volumes.objs, + pool->volumes.count+1) < 0) + goto no_memory; + pool->volumes.objs[pool->volumes.count++] = vol; + vol = NULL; } closedir(dir); @@ -904,8 +886,13 @@ return 0; +no_memory: + virStorageReportError(conn, VIR_ERR_NO_MEMORY, NULL); + /* fallthrough */ + cleanup: closedir(dir); + virStorageVolDefFree(vol); virStoragePoolObjClearVols(pool); return -1; } diff -r 03f1715be403 src/storage_backend_iscsi.c --- a/src/storage_backend_iscsi.c Fri Oct 03 12:58:28 2008 +0100 +++ b/src/storage_backend_iscsi.c Fri Oct 03 13:07:54 2008 +0100 @@ -236,9 +236,12 @@ pool->def->capacity += vol->capacity; pool->def->allocation += vol->allocation; - vol->next = pool->volumes; - pool->volumes = vol; - pool->nvolumes++; + if (VIR_REALLOC_N(pool->volumes.objs, + pool->volumes.count+1) < 0) { + virStorageReportError(conn, VIR_ERR_NO_MEMORY, NULL); + goto cleanup; + } + pool->volumes.objs[pool->volumes.count++] = vol; close(fd); diff -r 03f1715be403 src/storage_backend_logical.c --- a/src/storage_backend_logical.c Fri Oct 03 12:58:28 2008 +0100 +++ b/src/storage_backend_logical.c Fri Oct 03 13:07:54 2008 +0100 @@ -119,12 +119,17 @@ if ((vol->name = strdup(groups[0])) == NULL) { virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("volume")); + virStorageVolDefFree(vol); return -1; } - vol->next = pool->volumes; - pool->volumes = vol; - pool->nvolumes++; + if (VIR_REALLOC_N(pool->volumes.objs, + pool->volumes.count + 1)) { + virStorageReportError(conn, VIR_ERR_NO_MEMORY, NULL); + virStorageVolDefFree(vol); + return -1; + } + pool->volumes.objs[pool->volumes.count++] = vol; } if (vol->target.path == NULL) { diff -r 03f1715be403 src/storage_conf.c --- a/src/storage_conf.c Fri Oct 03 12:58:28 2008 +0100 +++ b/src/storage_conf.c Fri Oct 03 13:07:54 2008 +0100 @@ -71,6 +71,10 @@ void virStorageVolDefFree(virStorageVolDefPtr def) { int i; + + if (!def) + return; + VIR_FREE(def->name); VIR_FREE(def->key); @@ -87,6 +91,9 @@ void virStoragePoolDefFree(virStoragePoolDefPtr def) { int i; + + if (!def) + return; VIR_FREE(def->name); VIR_FREE(def->source.host.name); @@ -111,38 +118,48 @@ void virStoragePoolObjFree(virStoragePoolObjPtr obj) { - if (obj->def) - virStoragePoolDefFree(obj->def); - if (obj->newDef) - virStoragePoolDefFree(obj->newDef); + if (!obj) + return; + + virStoragePoolDefFree(obj->def); + virStoragePoolDefFree(obj->newDef); VIR_FREE(obj->configFile); VIR_FREE(obj->autostartLink); VIR_FREE(obj); } +void virStoragePoolObjListFree(virStoragePoolObjListPtr pools) +{ + unsigned int i; + for (i = 0 ; i < pools->count ; i++) + virStoragePoolObjFree(pools->objs[i]); + VIR_FREE(pools->objs); + pools->count = 0; +} + void -virStoragePoolObjRemove(virStorageDriverStatePtr driver, +virStoragePoolObjRemove(virStoragePoolObjListPtr pools, virStoragePoolObjPtr pool) { - virStoragePoolObjPtr prev = NULL, curr; + unsigned int i; - curr = driver->pools; - while (curr != pool) { - prev = curr; - curr = curr->next; + for (i = 0 ; i < pools->count ; i++) { + if (pools->objs[i] == pool) { + virStoragePoolObjFree(pools->objs[i]); + + if (i < (pools->count - 1)) + memmove(pools->objs + i, pools->objs + i + 1, + sizeof(*(pools->objs)) * (pools->count - (i + 1))); + + if (VIR_REALLOC_N(pools->objs, pools->count - 1) < 0) { + ; /* Failure to reduce memory allocation isn't fatal */ + } + pools->count--; + + break; + } } - - if (curr) { - if (prev) - prev->next = curr->next; - else - driver->pools = curr->next; - - driver->ninactivePools--; - } - - virStoragePoolObjFree(pool); } @@ -925,29 +942,25 @@ virStoragePoolObjPtr -virStoragePoolObjFindByUUID(virStorageDriverStatePtr driver, +virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools, const unsigned char *uuid) { - virStoragePoolObjPtr pool = driver->pools; + unsigned int i; - while (pool) { - if (!memcmp(pool->def->uuid, uuid, VIR_UUID_BUFLEN)) - return pool; - pool = pool->next; - } + for (i = 0 ; i < pools->count ; i++) + if (!memcmp(pools->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) + return pools->objs[i]; return NULL; } virStoragePoolObjPtr -virStoragePoolObjFindByName(virStorageDriverStatePtr driver, +virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, const char *name) { - virStoragePoolObjPtr pool = driver->pools; + unsigned int i; - while (pool) { - if (STREQ(pool->def->name, name)) - return pool; - pool = pool->next; - } + for (i = 0 ; i < pools->count ; i++) + if (STREQ(pools->objs[i]->def->name, name)) + return pools->objs[i]; return NULL; } @@ -955,26 +968,22 @@ void virStoragePoolObjClearVols(virStoragePoolObjPtr pool) { - virStorageVolDefPtr vol = pool->volumes; - while (vol) { - virStorageVolDefPtr next = vol->next; - virStorageVolDefFree(vol); - vol = next; - } - pool->volumes = NULL; - pool->nvolumes = 0; + unsigned int i; + for (i = 0 ; i < pool->volumes.count ; i++) + virStorageVolDefFree(pool->volumes.objs[i]); + + VIR_FREE(pool->volumes.objs); + pool->volumes.count = 0; } virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr pool, const char *key) { - virStorageVolDefPtr vol = pool->volumes; + unsigned int i; - while (vol) { - if (STREQ(vol->key, key)) - return vol; - vol = vol->next; - } + for (i = 0 ; i < pool->volumes.count ; i++) + if (STREQ(pool->volumes.objs[i]->key, key)) + return pool->volumes.objs[i]; return NULL; } @@ -982,13 +991,11 @@ virStorageVolDefPtr virStorageVolDefFindByPath(virStoragePoolObjPtr pool, const char *path) { - virStorageVolDefPtr vol = pool->volumes; + unsigned int i; - while (vol) { - if (STREQ(vol->target.path, path)) - return vol; - vol = vol->next; - } + for (i = 0 ; i < pool->volumes.count ; i++) + if (STREQ(pool->volumes.objs[i]->target.path, path)) + return pool->volumes.objs[i]; return NULL; } @@ -996,24 +1003,22 @@ virStorageVolDefPtr virStorageVolDefFindByName(virStoragePoolObjPtr pool, const char *name) { - virStorageVolDefPtr vol = pool->volumes; + unsigned int i; - while (vol) { - if (STREQ(vol->name, name)) - return vol; - vol = vol->next; - } + for (i = 0 ; i < pool->volumes.count ; i++) + if (STREQ(pool->volumes.objs[i]->name, name)) + return pool->volumes.objs[i]; return NULL; } virStoragePoolObjPtr virStoragePoolObjAssignDef(virConnectPtr conn, - virStorageDriverStatePtr driver, + virStoragePoolObjListPtr pools, virStoragePoolDefPtr def) { virStoragePoolObjPtr pool; - if ((pool = virStoragePoolObjFindByName(driver, def->name))) { + if ((pool = virStoragePoolObjFindByName(pools, def->name))) { if (!virStoragePoolObjIsActive(pool)) { virStoragePoolDefFree(pool->def); pool->def = def; @@ -1033,16 +1038,21 @@ pool->active = 0; pool->def = def; - pool->next = driver->pools; - driver->pools = pool; - driver->ninactivePools++; + if (VIR_REALLOC_N(pools->objs, pools->count+1) < 0) { + pool->def = NULL; + virStoragePoolObjFree(pool); + virStorageReportError(conn, VIR_ERR_NO_MEMORY, NULL); + return NULL; + } + pools->objs[pools->count++] = pool; return pool; } static virStoragePoolObjPtr -virStoragePoolObjLoad(virStorageDriverStatePtr driver, +virStoragePoolObjLoad(virConnectPtr conn, + virStoragePoolObjListPtr pools, const char *file, const char *path, const char *xml, @@ -1064,7 +1074,7 @@ return NULL; } - if (!(pool = virStoragePoolObjAssignDef(NULL, driver, def))) { + if (!(pool = virStoragePoolObjAssignDef(conn, pools, def))) { virStorageLog("Failed to load storage pool config '%s': out of memory", path); virStoragePoolDefFree(def); return NULL; @@ -1091,15 +1101,18 @@ int -virStoragePoolObjScanConfigs(virStorageDriverStatePtr driver) { +virStoragePoolLoadAllConfigs(virConnectPtr conn, + virStoragePoolObjListPtr pools, + const char *configDir, + const char *autostartDir) { DIR *dir; struct dirent *entry; - if (!(dir = opendir(driver->configDir))) { + if (!(dir = opendir(configDir))) { if (errno == ENOENT) return 0; virStorageLog("Failed to open dir '%s': %s", - driver->configDir, strerror(errno)); + configDir, strerror(errno)); return -1; } @@ -1114,24 +1127,24 @@ if (!virFileHasSuffix(entry->d_name, ".xml")) continue; - if (virFileBuildPath(driver->configDir, entry->d_name, + if (virFileBuildPath(configDir, entry->d_name, NULL, path, PATH_MAX) < 0) { virStorageLog("Config filename '%s/%s' is too long", - driver->configDir, entry->d_name); + configDir, entry->d_name); continue; } - if (virFileBuildPath(driver->autostartDir, entry->d_name, + if (virFileBuildPath(autostartDir, entry->d_name, NULL, autostartLink, PATH_MAX) < 0) { virStorageLog("Autostart link path '%s/%s' is too long", - driver->autostartDir, entry->d_name); + autostartDir, entry->d_name); continue; } if (virFileReadAll(path, 8192, &xml) < 0) continue; - virStoragePoolObjLoad(driver, entry->d_name, path, xml, autostartLink); + virStoragePoolObjLoad(conn, pools, entry->d_name, path, xml, autostartLink); VIR_FREE(xml); } diff -r 03f1715be403 src/storage_conf.h --- a/src/storage_conf.h Fri Oct 03 12:58:28 2008 +0100 +++ b/src/storage_conf.h Fri Oct 03 13:07:54 2008 +0100 @@ -87,10 +87,14 @@ virStorageVolSource source; virStorageVolTarget target; - - virStorageVolDefPtr next; }; +typedef struct _virStorageVolDefList virStorageVolDefList; +typedef virStorageVolDefList *virStorageVolDefListPtr; +struct _virStorageVolDefList { + unsigned int count; + virStorageVolDefPtr *objs; +}; @@ -222,10 +226,14 @@ virStoragePoolDefPtr def; virStoragePoolDefPtr newDef; - int nvolumes; - virStorageVolDefPtr volumes; + virStorageVolDefList volumes; +}; - virStoragePoolObjPtr next; +typedef struct _virStoragePoolObjList virStoragePoolObjList; +typedef virStoragePoolObjList *virStoragePoolObjListPtr; +struct _virStoragePoolObjList { + unsigned int count; + virStoragePoolObjPtr *objs; }; @@ -235,9 +243,8 @@ typedef virStorageDriverState *virStorageDriverStatePtr; struct _virStorageDriverState { - int nactivePools; - int ninactivePools; - virStoragePoolObjPtr pools; + virStoragePoolObjList pools; + char *configDir; char *autostartDir; }; @@ -252,11 +259,14 @@ const char *fmt, ...) ATTRIBUTE_FORMAT(printf, 3, 4); -int virStoragePoolObjScanConfigs(virStorageDriverStatePtr driver); +int virStoragePoolLoadAllConfigs(virConnectPtr conn, + virStoragePoolObjListPtr pools, + const char *configDir, + const char *autostartDir); -virStoragePoolObjPtr virStoragePoolObjFindByUUID(virStorageDriverStatePtr driver, +virStoragePoolObjPtr virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools, const unsigned char *uuid); -virStoragePoolObjPtr virStoragePoolObjFindByName(virStorageDriverStatePtr driver, +virStoragePoolObjPtr virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, const char *name); virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr pool, @@ -283,7 +293,7 @@ virStorageVolDefPtr def); virStoragePoolObjPtr virStoragePoolObjAssignDef(virConnectPtr conn, - virStorageDriverStatePtr driver, + virStoragePoolObjListPtr pools, virStoragePoolDefPtr def); int virStoragePoolObjSaveDef(virConnectPtr conn, @@ -296,7 +306,8 @@ void virStorageVolDefFree(virStorageVolDefPtr def); void virStoragePoolDefFree(virStoragePoolDefPtr def); void virStoragePoolObjFree(virStoragePoolObjPtr pool); -void virStoragePoolObjRemove(virStorageDriverStatePtr driver, +void virStoragePoolObjListFree(virStoragePoolObjListPtr pools); +void virStoragePoolObjRemove(virStoragePoolObjListPtr pools, virStoragePoolObjPtr pool); #endif /* __VIR_STORAGE_DRIVER_H__ */ diff -r 03f1715be403 src/storage_driver.c --- a/src/storage_driver.c Fri Oct 03 12:58:28 2008 +0100 +++ b/src/storage_driver.c Fri Oct 03 13:07:54 2008 +0100 @@ -49,11 +49,10 @@ static void storageDriverAutostart(virStorageDriverStatePtr driver) { - virStoragePoolObjPtr pool; + unsigned int i; - pool = driver->pools; - while (pool != NULL) { - virStoragePoolObjPtr next = pool->next; + for (i = 0 ; i < driver->pools.count ; i++) { + virStoragePoolObjPtr pool = driver->pools.objs[i]; if (pool->autostart && !virStoragePoolObjIsActive(pool)) { @@ -61,7 +60,6 @@ if ((backend = virStorageBackendForType(pool->def->type)) == NULL) { storageLog("Missing backend %d", pool->def->type); - pool = next; continue; } @@ -70,7 +68,6 @@ virErrorPtr err = virGetLastError(); storageLog("Failed to autostart storage pool '%s': %s", pool->def->name, err ? err->message : NULL); - pool = next; continue; } @@ -80,15 +77,10 @@ backend->stopPool(NULL, pool); storageLog("Failed to autostart storage pool '%s': %s", pool->def->name, err ? err->message : NULL); - pool = next; continue; } pool->active = 1; - driver->nactivePools++; - driver->ninactivePools--; } - - pool = next; } } @@ -149,7 +141,10 @@ } */ - if (virStoragePoolObjScanConfigs(driverState) < 0) { + if (virStoragePoolLoadAllConfigs(NULL, + &driverState->pools, + driverState->configDir, + driverState->autostartDir) < 0) { storageDriverShutdown(); return -1; } @@ -173,7 +168,13 @@ */ static int storageDriverReload(void) { - virStoragePoolObjScanConfigs(driverState); + if (!driverState) + return -1; + + virStoragePoolLoadAllConfigs(NULL, + &driverState->pools, + driverState->configDir, + driverState->autostartDir); storageDriverAutostart(driverState); return 0; @@ -188,11 +189,17 @@ */ static int storageDriverActive(void) { + unsigned int i; + + if (!driverState) + return 0; + /* If we've any active networks or guests, then we * mark this driver as active */ - if (driverState->nactivePools) - return 1; + for (i = 0 ; i < driverState->pools.count ; i++) + if (virStoragePoolObjIsActive(driverState->pools.objs[i])) + return 1; /* Otherwise we're happy to deal with a shutdown */ return 0; @@ -205,15 +212,15 @@ */ static int storageDriverShutdown(void) { - virStoragePoolObjPtr pool; + unsigned int i; if (!driverState) return -1; /* shutdown active pools */ - pool = driverState->pools; - while (pool) { - virStoragePoolObjPtr next = pool->next; + for (i = 0 ; i < driverState->pools.count ; i++) { + virStoragePoolObjPtr pool = driverState->pools.objs[i]; + if (virStoragePoolObjIsActive(pool)) { virStorageBackendPtr backend; if ((backend = virStorageBackendForType(pool->def->type)) == NULL) { @@ -229,24 +236,14 @@ } virStoragePoolObjClearVols(pool); } - pool = next; } /* free inactive pools */ - pool = driverState->pools; - while (pool) { - virStoragePoolObjPtr next = pool->next; - virStoragePoolObjFree(pool); - pool = next; - } - driverState->pools = NULL; - driverState->nactivePools = 0; - driverState->ninactivePools = 0; + virStoragePoolObjListFree(&driverState->pools); - free(driverState->configDir); - free(driverState->autostartDir); - free(driverState); - driverState = NULL; + VIR_FREE(driverState->configDir); + VIR_FREE(driverState->autostartDir); + VIR_FREE(driverState); return 0; } @@ -258,7 +255,7 @@ const unsigned char *uuid) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, uuid); virStoragePoolPtr ret; if (!pool) { @@ -276,7 +273,7 @@ const char *name) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByName(driver, name); + virStoragePoolObjPtr pool = virStoragePoolObjFindByName(&driver->pools, name); virStoragePoolPtr ret; if (!pool) { @@ -316,7 +313,13 @@ storageNumPools(virConnectPtr conn) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)conn->storagePrivateData; - return driver->nactivePools; + unsigned int i, nactive = 0; + + for (i = 0 ; i < driver->pools.count ; i++) + if (virStoragePoolObjIsActive(driver->pools.objs[i])) + nactive++; + + return nactive; } static int @@ -325,18 +328,17 @@ int nnames) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)conn->storagePrivateData; - virStoragePoolObjPtr pool = driver->pools; int got = 0, i; - while (pool && got < nnames) { - if (virStoragePoolObjIsActive(pool)) { - if (!(names[got] = strdup(pool->def->name))) { + + for (i = 0 ; i < driver->pools.count && got < nnames ; i++) { + if (virStoragePoolObjIsActive(driver->pools.objs[i])) { + if (!(names[got] = strdup(driver->pools.objs[i]->def->name))) { virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("names")); goto cleanup; } got++; } - pool = pool->next; } return got; @@ -353,7 +355,13 @@ storageNumDefinedPools(virConnectPtr conn) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)conn->storagePrivateData; - return driver->ninactivePools; + unsigned int i, nactive = 0; + + for (i = 0 ; i < driver->pools.count ; i++) + if (!virStoragePoolObjIsActive(driver->pools.objs[i])) + nactive++; + + return nactive; } static int @@ -362,18 +370,17 @@ int nnames) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)conn->storagePrivateData; - virStoragePoolObjPtr pool = driver->pools; int got = 0, i; - while (pool && got < nnames) { - if (!virStoragePoolObjIsActive(pool)) { - if (!(names[got] = strdup(pool->def->name))) { + + for (i = 0 ; i < driver->pools.count && got < nnames ; i++) { + if (!virStoragePoolObjIsActive(driver->pools.objs[i])) { + if (!(names[got] = strdup(driver->pools.objs[i]->def->name))) { virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("names")); goto cleanup; } got++; } - pool = pool->next; } return got; @@ -424,8 +431,8 @@ if (!(def = virStoragePoolDefParse(conn, xml, NULL))) return NULL; - if (virStoragePoolObjFindByUUID(driver, def->uuid) || - virStoragePoolObjFindByName(driver, def->name)) { + if (virStoragePoolObjFindByUUID(&driver->pools, def->uuid) || + virStoragePoolObjFindByName(&driver->pools, def->name)) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("storage pool already exists")); virStoragePoolDefFree(def); @@ -437,7 +444,7 @@ return NULL; } - if (!(pool = virStoragePoolObjAssignDef(conn, driver, def))) { + if (!(pool = virStoragePoolObjAssignDef(conn, &driver->pools, def))) { virStoragePoolDefFree(def); return NULL; } @@ -451,8 +458,6 @@ return NULL; } pool->active = 1; - driver->nactivePools++; - driver->ninactivePools--; ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid); @@ -478,13 +483,13 @@ return NULL; } - if (!(pool = virStoragePoolObjAssignDef(conn, driver, def))) { + if (!(pool = virStoragePoolObjAssignDef(conn, &driver->pools, def))) { virStoragePoolDefFree(def); return NULL; } if (virStoragePoolObjSaveDef(conn, driver, pool, def) < 0) { - virStoragePoolObjRemove(driver, pool); + virStoragePoolObjRemove(&driver->pools, pool); return NULL; } @@ -496,7 +501,7 @@ storagePoolUndefine(virStoragePoolPtr obj) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); if (!pool) { virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL, @@ -517,12 +522,10 @@ storageLog("Failed to delete autostart link '%s': %s", pool->autostartLink, strerror(errno)); - free(pool->configFile); - pool->configFile = NULL; - free(pool->autostartLink); - pool->autostartLink = NULL; + VIR_FREE(pool->configFile); + VIR_FREE(pool->autostartLink); - virStoragePoolObjRemove(driver, pool); + virStoragePoolObjRemove(&driver->pools, pool); return 0; } @@ -532,7 +535,7 @@ unsigned int flags ATTRIBUTE_UNUSED) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); virStorageBackendPtr backend; if (!pool) { @@ -560,8 +563,6 @@ } pool->active = 1; - driver->nactivePools++; - driver->ninactivePools--; return 0; } @@ -571,7 +572,7 @@ unsigned int flags) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); virStorageBackendPtr backend; if (!pool) { @@ -602,7 +603,7 @@ storagePoolDestroy(virStoragePoolPtr obj) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); virStorageBackendPtr backend; if (!pool) { @@ -628,11 +629,9 @@ virStoragePoolObjClearVols(pool); pool->active = 0; - driver->nactivePools--; - driver->ninactivePools++; if (pool->configFile == NULL) - virStoragePoolObjRemove(driver, pool); + virStoragePoolObjRemove(&driver->pools, pool); return 0; } @@ -643,7 +642,7 @@ unsigned int flags) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); virStorageBackendPtr backend; if (!pool) { @@ -679,7 +678,7 @@ unsigned int flags ATTRIBUTE_UNUSED) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); virStorageBackendPtr backend; int ret = 0; @@ -705,11 +704,9 @@ backend->stopPool(obj->conn, pool); pool->active = 0; - driver->nactivePools--; - driver->ninactivePools++; if (pool->configFile == NULL) - virStoragePoolObjRemove(driver, pool); + virStoragePoolObjRemove(&driver->pools, pool); } return ret; @@ -721,7 +718,7 @@ virStoragePoolInfoPtr info) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); virStorageBackendPtr backend; if (!pool) { @@ -751,7 +748,7 @@ unsigned int flags ATTRIBUTE_UNUSED) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); if (!pool) { virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL, @@ -767,7 +764,7 @@ int *autostart) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); if (!pool) { virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL, @@ -789,7 +786,7 @@ int autostart) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); if (!pool) { virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL, @@ -845,7 +842,7 @@ storagePoolNumVolumes(virStoragePoolPtr obj) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); if (!pool) { virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL, @@ -859,7 +856,7 @@ return -1; } - return pool->nvolumes; + return pool->volumes.count; } static int @@ -868,9 +865,8 @@ int maxnames) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); - int i = 0; - virStorageVolDefPtr vol; + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); + int i, n = 0; if (!pool) { virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL, @@ -885,25 +881,20 @@ } memset(names, 0, maxnames); - vol = pool->volumes; - while (vol && i < maxnames) { - names[i] = strdup(vol->name); - if (names[i] == NULL) { + for (i = 0 ; i < pool->volumes.count && n < maxnames ; i++) { + if ((names[n++] = strdup(pool->volumes.objs[i]->name)) == NULL) { virStorageReportError(obj->conn, VIR_ERR_NO_MEMORY, "%s", _("name")); goto cleanup; } - vol = vol->next; - i++; } - return i; + return n; cleanup: - for (i = 0 ; i < maxnames ; i++) { - free(names[i]); - names[i] = NULL; - } + for (n = 0 ; n < maxnames ; n++) + VIR_FREE(names[i]); + memset(names, 0, maxnames); return -1; } @@ -914,7 +905,7 @@ const char *name) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); virStorageVolDefPtr vol; if (!pool) { @@ -946,19 +937,19 @@ const char *key) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)conn->storagePrivateData; - virStoragePoolObjPtr pool = driver->pools; + unsigned int i; - while (pool) { - if (virStoragePoolObjIsActive(pool)) { - virStorageVolDefPtr vol = virStorageVolDefFindByKey(pool, key); + for (i = 0 ; i < driver->pools.count ; i++) { + if (virStoragePoolObjIsActive(driver->pools.objs[i])) { + virStorageVolDefPtr vol = + virStorageVolDefFindByKey(driver->pools.objs[i], key); if (vol) return virGetStorageVol(conn, - pool->def->name, + driver->pools.objs[i]->def->name, vol->name, vol->key); } - pool = pool->next; } virStorageReportError(conn, VIR_ERR_INVALID_STORAGE_VOL, @@ -971,19 +962,19 @@ const char *path) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)conn->storagePrivateData; - virStoragePoolObjPtr pool = driver->pools; + unsigned int i; - while (pool) { - if (virStoragePoolObjIsActive(pool)) { - virStorageVolDefPtr vol = virStorageVolDefFindByPath(pool, path); + for (i = 0 ; i < driver->pools.count ; i++) { + if (virStoragePoolObjIsActive(driver->pools.objs[i])) { + virStorageVolDefPtr vol = + virStorageVolDefFindByPath(driver->pools.objs[i], path); if (vol) return virGetStorageVol(conn, - pool->def->name, + driver->pools.objs[i]->def->name, vol->name, vol->key); } - pool = pool->next; } virStorageReportError(conn, VIR_ERR_INVALID_STORAGE_VOL, @@ -997,7 +988,7 @@ unsigned int flags ATTRIBUTE_UNUSED) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(driver, obj->uuid); + virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); virStorageBackendPtr backend; virStorageVolDefPtr vol; @@ -1027,6 +1018,13 @@ return NULL; } + if (VIR_REALLOC_N(pool->volumes.objs, + pool->volumes.count+1) < 0) { + virStorageReportError(obj->conn, VIR_ERR_NO_MEMORY, NULL); + virStorageVolDefFree(vol); + return NULL; + } + if (!backend->createVol) { virStorageReportError(obj->conn, VIR_ERR_NO_SUPPORT, "%s", _("storage pool does not support volume creation")); @@ -1039,9 +1037,7 @@ return NULL; } - vol->next = pool->volumes; - pool->volumes = vol; - pool->nvolumes++; + pool->volumes.objs[pool->volumes.count++] = vol; return virGetStorageVol(obj->conn, pool->def->name, vol->name, vol->key); } @@ -1051,9 +1047,10 @@ unsigned int flags) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByName(driver, obj->pool); + virStoragePoolObjPtr pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); virStorageBackendPtr backend; - virStorageVolDefPtr vol, tmp, prev; + virStorageVolDefPtr vol; + unsigned int i; if (!pool) { virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL, @@ -1089,22 +1086,22 @@ return -1; } - prev = NULL; - tmp = pool->volumes; - while (tmp) { - if (tmp == vol) { + for (i = 0 ; i < pool->volumes.count ; i++) { + if (pool->volumes.objs[i] == vol) { + virStorageVolDefFree(vol); + + if (i < (pool->volumes.count - 1)) + memmove(pool->volumes.objs + i, pool->volumes.objs + i + 1, + sizeof(*(pool->volumes.objs)) * (pool->volumes.count - (i + 1))); + + if (VIR_REALLOC_N(pool->volumes.objs, pool->volumes.count - 1) < 0) { + ; /* Failure to reduce memory allocation isn't fatal */ + } + pool->volumes.count--; + break; } - prev = tmp; - tmp = tmp->next; } - if (prev) { - prev->next = vol->next; - } else { - pool->volumes = vol->next; - } - pool->nvolumes--; - virStorageVolDefFree(vol); return 0; } @@ -1114,7 +1111,7 @@ virStorageVolInfoPtr info) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByName(driver, obj->pool); + virStoragePoolObjPtr pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); virStorageBackendPtr backend; virStorageVolDefPtr vol; @@ -1158,7 +1155,7 @@ unsigned int flags ATTRIBUTE_UNUSED) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByName(driver, obj->pool); + virStoragePoolObjPtr pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); virStorageBackendPtr backend; virStorageVolDefPtr vol; @@ -1192,7 +1189,7 @@ storageVolumeGetPath(virStorageVolPtr obj) { virStorageDriverStatePtr driver = (virStorageDriverStatePtr)obj->conn->storagePrivateData; - virStoragePoolObjPtr pool = virStoragePoolObjFindByName(driver, obj->pool); + virStoragePoolObjPtr pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); virStorageVolDefPtr vol; char *ret; -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Fri, Oct 03, 2008 at 01:28:31PM +0100, Daniel P. Berrange wrote:
This patch removes the linked list in the virStoragePoolObjPtr and and virStorageVolDefPtr objects, and adds new virStoragePoolObjList and virStorageVolDefList structs to track pools & vols as arrays. The storage driver driver gets updated to account for this API change.
Okay, there is also quite a bit of cleanup in that patch too. What about the active and inactive pool counts, is that really not work to keep counting those ? just wondering ... Looks fine, +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Mon, Oct 06, 2008 at 04:54:42PM +0200, Daniel Veillard wrote:
On Fri, Oct 03, 2008 at 01:28:31PM +0100, Daniel P. Berrange wrote:
This patch removes the linked list in the virStoragePoolObjPtr and and virStorageVolDefPtr objects, and adds new virStoragePoolObjList and virStorageVolDefList structs to track pools & vols as arrays. The storage driver driver gets updated to account for this API change.
Okay, there is also quite a bit of cleanup in that patch too. What about the active and inactive pool counts, is that really not work to keep counting those ? just wondering ...
We used to have the explicit active vs inactive count in the domain/network drivers too, but killed them off. It will complicate thread locking, because active vs inactive is a property of the individual object and not the driver as a whole. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

This one isn't strictly needed yet, but while we're removing linked lists we might as well kill off all of them. So this removes the linked lists for devices associated with a domain. src/domain_conf.c | 244 +++++++++++++++----------------- src/domain_conf.h | 47 +++--- src/lxc_container.c | 38 ++-- src/lxc_controller.c | 11 - src/lxc_driver.c | 29 +-- src/openvz_conf.c | 35 ++-- src/openvz_driver.c | 36 ++-- src/qemu_conf.c | 57 ++----- src/qemu_driver.c | 79 ++++------ src/xend_internal.c | 214 +++++++++++++--------------- src/xend_internal.h | 2 src/xm_internal.c | 228 ++++++++++++----------------- tests/sexpr2xmldata/sexpr2xml-fv-v2.xml | 10 - 13 files changed, 480 insertions(+), 550 deletions(-) Daniel diff -r 44cb26ad18bc src/domain_conf.c --- a/src/domain_conf.c Fri Oct 03 12:21:48 2008 +0100 +++ b/src/domain_conf.c Fri Oct 03 12:44:51 2008 +0100 @@ -230,7 +230,6 @@ if (!def) return; - virDomainInputDefFree(def->next); VIR_FREE(def); } @@ -244,7 +243,6 @@ VIR_FREE(def->driverName); VIR_FREE(def->driverType); - virDomainDiskDefFree(def->next); VIR_FREE(def); } @@ -256,7 +254,6 @@ VIR_FREE(def->src); VIR_FREE(def->dst); - virDomainFSDefFree(def->next); VIR_FREE(def); } @@ -290,7 +287,6 @@ } VIR_FREE(def->ifname); - virDomainNetDefFree(def->next); VIR_FREE(def); } @@ -324,7 +320,6 @@ break; } - virDomainChrDefFree(def->next); VIR_FREE(def); } @@ -333,7 +328,6 @@ if (!def) return; - virDomainSoundDefFree(def->next); VIR_FREE(def); } @@ -343,7 +337,6 @@ return; VIR_FREE(def->target); - virDomainHostdevDefFree(def->next); VIR_FREE(def); } @@ -375,19 +368,45 @@ void virDomainDefFree(virDomainDefPtr def) { + unsigned int i; + if (!def) return; virDomainGraphicsDefFree(def->graphics); - virDomainInputDefFree(def->inputs); - virDomainDiskDefFree(def->disks); - virDomainFSDefFree(def->fss); - virDomainNetDefFree(def->nets); - virDomainChrDefFree(def->serials); - virDomainChrDefFree(def->parallels); + + for (i = 0 ; i < def->ninputs ; i++) + virDomainInputDefFree(def->inputs[i]); + VIR_FREE(def->inputs); + + for (i = 0 ; i < def->ndisks ; i++) + virDomainDiskDefFree(def->disks[i]); + VIR_FREE(def->disks); + + for (i = 0 ; i < def->nfss ; i++) + virDomainFSDefFree(def->fss[i]); + VIR_FREE(def->fss); + + for (i = 0 ; i < def->nnets ; i++) + virDomainNetDefFree(def->nets[i]); + VIR_FREE(def->nets); + for (i = 0 ; i < def->nserials ; i++) + virDomainChrDefFree(def->serials[i]); + VIR_FREE(def->serials); + + for (i = 0 ; i < def->nparallels ; i++) + virDomainChrDefFree(def->parallels[i]); + VIR_FREE(def->parallels); + virDomainChrDefFree(def->console); - virDomainSoundDefFree(def->sounds); - virDomainHostdevDefFree(def->hostdevs); + + for (i = 0 ; i < def->nsounds ; i++) + virDomainSoundDefFree(def->sounds[i]); + VIR_FREE(def->sounds); + + for (i = 0 ; i < def->nhostdevs ; i++) + virDomainHostdevDefFree(def->hostdevs[i]); + VIR_FREE(def->hostdevs); VIR_FREE(def->os.type); VIR_FREE(def->os.arch); @@ -1695,6 +1714,14 @@ return NULL; } +int virDomainDiskQSort(const void *a, const void *b) +{ + const virDomainDiskDefPtr *da = a; + const virDomainDiskDefPtr *db = b; + + return virDomainDiskCompare(*da, *db); +} + static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, virCapsPtr caps, @@ -1936,36 +1963,18 @@ "%s", _("cannot extract disk devices")); goto error; } + if (n && VIR_ALLOC_N(def->disks, n) < 0) + goto no_memory; for (i = 0 ; i < n ; i++) { virDomainDiskDefPtr disk = virDomainDiskDefParseXML(conn, nodes[i]); if (!disk) goto error; - /* Maintain list in sorted order according to target device name */ - virDomainDiskDefPtr ptr = def->disks; - virDomainDiskDefPtr *prev = &(def->disks); - while (ptr) { - if (STREQ(disk->dst, ptr->dst)) { - virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("duplicate disk target '%s'"), - disk->dst); - goto error; - } - if (virDomainDiskCompare(disk, ptr) < 0) { - disk->next = ptr; - *prev = disk; - break; - } - prev = &(ptr->next); - ptr = ptr->next; - } - - if (!ptr) { - disk->next = ptr; - *prev = disk; - } + def->disks[def->ndisks++] = disk; } + qsort(def->disks, def->ndisks, sizeof(*def->disks), + virDomainDiskQSort); VIR_FREE(nodes); /* analysis of the filesystems */ @@ -1974,14 +1983,15 @@ "%s", _("cannot extract filesystem devices")); goto error; } - for (i = n - 1 ; i >= 0 ; i--) { + if (n && VIR_ALLOC_N(def->fss, n) < 0) + goto no_memory; + for (i = 0 ; i < n ; i++) { virDomainFSDefPtr fs = virDomainFSDefParseXML(conn, nodes[i]); if (!fs) goto error; - fs->next = def->fss; - def->fss = fs; + def->fss[def->nfss++] = fs; } VIR_FREE(nodes); @@ -1991,14 +2001,15 @@ "%s", _("cannot extract network devices")); goto error; } - for (i = n - 1 ; i >= 0 ; i--) { + if (n && VIR_ALLOC_N(def->nets, n) < 0) + goto no_memory; + for (i = 0 ; i < n ; i++) { virDomainNetDefPtr net = virDomainNetDefParseXML(conn, nodes[i]); if (!net) goto error; - net->next = def->nets; - def->nets = net; + def->nets[def->nnets++] = net; } VIR_FREE(nodes); @@ -2009,15 +2020,17 @@ "%s", _("cannot extract parallel devices")); goto error; } - for (i = n - 1 ; i >= 0 ; i--) { + if (n && VIR_ALLOC_N(def->parallels, n) < 0) + goto no_memory; + + for (i = 0 ; i < n ; i++) { virDomainChrDefPtr chr = virDomainChrDefParseXML(conn, nodes[i]); if (!chr) goto error; chr->dstPort = i; - chr->next = def->parallels; - def->parallels = chr; + def->parallels[def->nparallels++] = chr; } VIR_FREE(nodes); @@ -2026,15 +2039,17 @@ "%s", _("cannot extract serial devices")); goto error; } - for (i = n - 1 ; i >= 0 ; i--) { + if (n && VIR_ALLOC_N(def->serials, n) < 0) + goto no_memory; + + for (i = 0 ; i < n ; i++) { virDomainChrDefPtr chr = virDomainChrDefParseXML(conn, nodes[i]); if (!chr) goto error; chr->dstPort = i; - chr->next = def->serials; - def->serials = chr; + def->serials[def->nserials++] = chr; } VIR_FREE(nodes); @@ -2042,7 +2057,7 @@ * If no serial devices were listed, then look for console * devices which is the legacy syntax for the same thing */ - if (def->serials == NULL) { + if (def->nserials == 0) { if ((node = virXPathNode(conn, "./devices/console[1]", ctxt)) != NULL) { virDomainChrDefPtr chr = virDomainChrDefParseXML(conn, node); @@ -2055,8 +2070,12 @@ * while for non-HVM it was a parvirt console */ if (STREQ(def->os.type, "hvm")) { - chr->next = def->serials; - def->serials = chr; + if (VIR_ALLOC_N(def->serials, 1) < 0) { + virDomainChrDefFree(chr); + goto no_memory; + } + def->nserials = 1; + def->serials[0] = chr; } else { def->console = chr; } @@ -2070,7 +2089,10 @@ "%s", _("cannot extract input devices")); goto error; } - for (i = n - 1 ; i >= 0 ; i--) { + if (n && VIR_ALLOC_N(def->inputs, n) < 0) + goto no_memory; + + for (i = 0 ; i < n ; i++) { virDomainInputDefPtr input = virDomainInputDefParseXML(conn, def->os.type, nodes[i]); @@ -2091,8 +2113,7 @@ continue; } - input->next = def->inputs; - def->inputs = input; + def->inputs[def->ninputs++] = input; } VIR_FREE(nodes); @@ -2127,8 +2148,13 @@ input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE; input->bus = VIR_DOMAIN_INPUT_BUS_XEN; } - input->next = def->inputs; - def->inputs = input; + + if (VIR_REALLOC_N(def->inputs, def->ninputs + 1) < 0) { + virDomainInputDefFree(input); + goto no_memory; + } + def->inputs[def->ninputs] = input; + def->ninputs++; } @@ -2138,28 +2164,26 @@ "%s", _("cannot extract sound devices")); goto error; } - for (i = n - 1 ; i >= 0 ; i--) { - int collision = 0; - virDomainSoundDefPtr check; + if (n && VIR_ALLOC_N(def->sounds, n) < 0) + goto no_memory; + for (i = 0 ; i < n ; i++) { + int collision = 0, j; virDomainSoundDefPtr sound = virDomainSoundDefParseXML(conn, nodes[i]); if (!sound) goto error; /* Verify there's no duplicated sound card */ - check = def->sounds; - while (check) { - if (check->model == sound->model) + for (j = 0 ; j < def->nsounds ; j++) { + if (def->sounds[j]->model == sound->model) collision = 1; - check = check->next; } if (collision) { virDomainSoundDefFree(sound); continue; } - sound->next = def->sounds; - def->sounds = sound; + def->sounds[def->nsounds++] = sound; } VIR_FREE(nodes); @@ -2169,17 +2193,22 @@ "%s", _("cannot extract host devices")); goto error; } + if (n && VIR_ALLOC_N(def->hostdevs, n) < 0) + goto no_memory; for (i = 0 ; i < n ; i++) { virDomainHostdevDefPtr hostdev = virDomainHostdevDefParseXML(conn, nodes[i]); if (!hostdev) goto error; - hostdev->next = def->hostdevs; - def->hostdevs = hostdev; + def->hostdevs[def->nhostdevs++] = hostdev; } VIR_FREE(nodes); return def; + +no_memory: + virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL); + /* fallthrough */ error: VIR_FREE(tmp); @@ -2971,13 +3000,6 @@ virBuffer buf = VIR_BUFFER_INITIALIZER; unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; - virDomainDiskDefPtr disk; - virDomainFSDefPtr fs; - virDomainNetDefPtr net; - virDomainSoundDefPtr sound; - virDomainInputDefPtr input; - virDomainChrDefPtr chr; - virDomainHostdevDefPtr hostdev; const char *type = NULL, *tmp; int n, allones = 1; @@ -3114,67 +3136,49 @@ virBufferEscapeString(&buf, " <emulator>%s</emulator>\n", def->emulator); - disk = def->disks; - while (disk) { - if (virDomainDiskDefFormat(conn, &buf, disk) < 0) + for (n = 0 ; n < def->ndisks ; n++) + if (virDomainDiskDefFormat(conn, &buf, def->disks[n]) < 0) goto cleanup; - disk = disk->next; - } - fs = def->fss; - while (fs) { - if (virDomainFSDefFormat(conn, &buf, fs) < 0) + for (n = 0 ; n < def->nfss ; n++) + if (virDomainFSDefFormat(conn, &buf, def->fss[n]) < 0) goto cleanup; - fs = fs->next; - } - net = def->nets; - while (net) { - if (virDomainNetDefFormat(conn, &buf, net) < 0) + + for (n = 0 ; n < def->nnets ; n++) + if (virDomainNetDefFormat(conn, &buf, def->nets[n]) < 0) goto cleanup; - net = net->next; - } + for (n = 0 ; n < def->nserials ; n++) + if (virDomainChrDefFormat(conn, &buf, def->serials[n], "serial") < 0) + goto cleanup; - chr = def->serials; - while (chr) { - if (virDomainChrDefFormat(conn, &buf, chr, "serial") < 0) + for (n = 0 ; n < def->nparallels ; n++) + if (virDomainChrDefFormat(conn, &buf, def->parallels[n], "parallel") < 0) goto cleanup; - chr = chr->next; - } - - chr = def->parallels; - while (chr) { - if (virDomainChrDefFormat(conn, &buf, chr, "parallel") < 0) - goto cleanup; - chr = chr->next; - } /* If there's a PV console that's preferred.. */ if (def->console) { if (virDomainChrDefFormat(conn, &buf, def->console, "console") < 0) goto cleanup; - } else if (def->serials != NULL) { + } else if (def->nserials != 0) { /* ..else for legacy compat duplicate the serial device as a console */ - if (virDomainChrDefFormat(conn, &buf, def->serials, "console") < 0) + if (virDomainChrDefFormat(conn, &buf, def->serials[n], "console") < 0) goto cleanup; } - input = def->inputs; - while (input) { - if (input->bus == VIR_DOMAIN_INPUT_BUS_USB && - virDomainInputDefFormat(conn, &buf, input) < 0) + for (n = 0 ; n < def->ninputs ; n++) + if (def->inputs[n]->bus == VIR_DOMAIN_INPUT_BUS_USB && + virDomainInputDefFormat(conn, &buf, def->inputs[n]) < 0) goto cleanup; - input = input->next; - } if (def->graphics) { /* If graphics is enabled, add the implicit mouse */ virDomainInputDef autoInput = { VIR_DOMAIN_INPUT_TYPE_MOUSE, STREQ(def->os.type, "hvm") ? - VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN, - NULL }; + VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN + }; if (virDomainInputDefFormat(conn, &buf, &autoInput) < 0) goto cleanup; @@ -3183,19 +3187,13 @@ goto cleanup; } - sound = def->sounds; - while(sound) { - if (virDomainSoundDefFormat(conn, &buf, sound) < 0) + for (n = 0 ; n < def->nsounds ; n++) + if (virDomainSoundDefFormat(conn, &buf, def->sounds[n]) < 0) goto cleanup; - sound = sound->next; - } - hostdev = def->hostdevs; - while (hostdev) { - if (virDomainHostdevDefFormat(conn, &buf, hostdev) < 0) + for (n = 0 ; n < def->nhostdevs ; n++) + if (virDomainHostdevDefFormat(conn, &buf, def->hostdevs[n]) < 0) goto cleanup; - hostdev = hostdev->next; - } virBufferAddLit(&buf, " </devices>\n"); virBufferAddLit(&buf, "</domain>\n"); diff -r 44cb26ad18bc src/domain_conf.h --- a/src/domain_conf.h Fri Oct 03 12:21:48 2008 +0100 +++ b/src/domain_conf.h Fri Oct 03 12:44:51 2008 +0100 @@ -90,8 +90,6 @@ char *driverType; unsigned int readonly : 1; unsigned int shared : 1; - - virDomainDiskDefPtr next; }; @@ -112,8 +110,6 @@ char *src; char *dst; unsigned int readonly : 1; - - virDomainFSDefPtr next; }; @@ -158,8 +154,6 @@ } bridge; } data; char *ifname; - - virDomainNetDefPtr next; }; enum virDomainChrSrcType { @@ -211,8 +205,6 @@ int listen; } nix; } data; - - virDomainChrDefPtr next; }; enum virDomainInputType { @@ -235,7 +227,6 @@ struct _virDomainInputDef { int type; int bus; - virDomainInputDefPtr next; }; enum virDomainSoundModel { @@ -250,7 +241,6 @@ typedef virDomainSoundDef *virDomainSoundDefPtr; struct _virDomainSoundDef { int model; - virDomainSoundDefPtr next; }; /* 3 possible graphics console modes */ @@ -324,7 +314,6 @@ } caps; } source; char* target; - virDomainHostdevDefPtr next; }; /* Flags for the 'type' field in next struct */ @@ -428,15 +417,34 @@ int localtime; + /* Only 1 */ virDomainGraphicsDefPtr graphics; - virDomainDiskDefPtr disks; - virDomainFSDefPtr fss; - virDomainNetDefPtr nets; - virDomainInputDefPtr inputs; - virDomainSoundDefPtr sounds; - virDomainHostdevDefPtr hostdevs; - virDomainChrDefPtr serials; - virDomainChrDefPtr parallels; + + int ndisks; + virDomainDiskDefPtr *disks; + + int nfss; + virDomainFSDefPtr *fss; + + int nnets; + virDomainNetDefPtr *nets; + + int ninputs; + virDomainInputDefPtr *inputs; + + int nsounds; + virDomainSoundDefPtr *sounds; + + int nhostdevs; + virDomainHostdevDefPtr *hostdevs; + + int nserials; + virDomainChrDefPtr *serials; + + int nparallels; + virDomainChrDefPtr *parallels; + + /* Only 1 */ virDomainChrDefPtr console; }; @@ -531,6 +539,7 @@ char *cpuset, int maxcpu); +int virDomainDiskQSort(const void *a, const void *b); int virDomainDiskCompare(virDomainDiskDefPtr a, virDomainDiskDefPtr b); diff -r 44cb26ad18bc src/lxc_container.c --- a/src/lxc_container.c Fri Oct 03 12:21:48 2008 +0100 +++ b/src/lxc_container.c Fri Oct 03 12:44:51 2008 +0100 @@ -368,28 +368,28 @@ static int lxcContainerMountNewFS(virDomainDefPtr vmDef) { - virDomainFSDefPtr tmp; + int i; /* Pull in rest of container's mounts */ - for (tmp = vmDef->fss; tmp; tmp = tmp->next) { + for (i = 0 ; i < vmDef->nfss ; i++) { char *src; - if (STREQ(tmp->dst, "/")) + if (STREQ(vmDef->fss[i]->dst, "/")) continue; // XXX fix - if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) + if (vmDef->fss[i]->type != VIR_DOMAIN_FS_TYPE_MOUNT) continue; - if (asprintf(&src, "/.oldroot/%s", tmp->src) < 0) { + if (asprintf(&src, "/.oldroot/%s", vmDef->fss[i]->src) < 0) { lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); return -1; } - if (virFileMakePath(tmp->dst) < 0 || - mount(src, tmp->dst, NULL, MS_BIND, NULL) < 0) { + if (virFileMakePath(vmDef->fss[i]->dst) < 0 || + mount(src, vmDef->fss[i]->dst, NULL, MS_BIND, NULL) < 0) { VIR_FREE(src); lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("failed to mount %s at %s for container: %s"), - tmp->src, tmp->dst, strerror(errno)); + vmDef->fss[i]->src, vmDef->fss[i]->dst, strerror(errno)); return -1; } VIR_FREE(src); @@ -482,21 +482,21 @@ but with extra stuff mapped in */ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef) { - virDomainFSDefPtr tmp; + int i; - for (tmp = vmDef->fss; tmp; tmp = tmp->next) { + for (i = 0 ; i < vmDef->nfss ; i++) { // XXX fix to support other mount types - if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) + if (vmDef->fss[i]->type != VIR_DOMAIN_FS_TYPE_MOUNT) continue; - if (mount(tmp->src, - tmp->dst, + if (mount(vmDef->fss[i]->src, + vmDef->fss[i]->dst, NULL, MS_BIND, NULL) < 0) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("failed to mount %s at %s for container: %s"), - tmp->src, tmp->dst, strerror(errno)); + vmDef->fss[i]->src, vmDef->fss[i]->dst, strerror(errno)); return -1; } } @@ -514,14 +514,14 @@ static int lxcContainerSetupMounts(virDomainDefPtr vmDef) { - virDomainFSDefPtr tmp; + int i; virDomainFSDefPtr root = NULL; - for (tmp = vmDef->fss; tmp && !root; tmp = tmp->next) { - if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) + for (i = 0 ; i < vmDef->nfss ; i++) { + if (vmDef->fss[i]->type != VIR_DOMAIN_FS_TYPE_MOUNT) continue; - if (STREQ(tmp->dst, "/")) - root = tmp; + if (STREQ(vmDef->fss[i]->dst, "/")) + root = vmDef->fss[i]; } if (root) diff -r 44cb26ad18bc src/lxc_controller.c --- a/src/lxc_controller.c Fri Oct 03 12:21:48 2008 +0100 +++ b/src/lxc_controller.c Fri Oct 03 12:44:51 2008 +0100 @@ -429,8 +429,6 @@ int bg = 0; virCapsPtr caps = NULL; virDomainDefPtr def = NULL; - int nnets = 0; - virDomainNetDefPtr nets = NULL; char *configFile = NULL; char *sockpath = NULL; const struct option const options[] = { @@ -525,14 +523,9 @@ if ((def = virDomainDefParseFile(NULL, caps, configFile)) == NULL) goto cleanup; - nets = def->nets; - while (nets) { - nnets++; - nets = nets->next; - } - if (nnets != nveths) { + if (def->nnets != nveths) { fprintf(stderr, "%s: expecting %d veths, but got %d\n", - argv[0], nnets, nveths); + argv[0], def->nnets, nveths); goto cleanup; } diff -r 44cb26ad18bc src/lxc_driver.c --- a/src/lxc_driver.c Fri Oct 03 12:21:48 2008 +0100 +++ b/src/lxc_driver.c Fri Oct 03 12:44:51 2008 +0100 @@ -370,7 +370,7 @@ int rc = -1; int waitRc; int childStatus = -1; - virDomainNetDefPtr net; + int i; while (((waitRc = waitpid(vm->pid, &childStatus, 0)) == -1) && errno == EINTR) @@ -400,9 +400,9 @@ vm->def->id = -1; vm->monitor = -1; - for (net = vm->def->nets; net; net = net->next) { - vethInterfaceUpOrDown(net->ifname, 0); - vethDelete(net->ifname); + for (i = 0 ; i < vm->def->nnets ; i++) { + vethInterfaceUpOrDown(vm->def->nets[i]->ifname, 0); + vethDelete(vm->def->nets[i]->ifname); } return rc; @@ -423,8 +423,7 @@ unsigned int *nveths, char ***veths) { - int rc = -1; - virDomainNetDefPtr net; + int rc = -1, i; char *bridge = NULL; char parentVeth[PATH_MAX] = ""; char containerVeth[PATH_MAX] = ""; @@ -433,12 +432,12 @@ if (brInit(&brctl) != 0) return -1; - for (net = def->nets; net; net = net->next) { - switch (net->type) { + for (i = 0 ; i < def->nnets ; i++) { + switch (def->nets[i]->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: { virNetworkPtr network = virNetworkLookupByName(conn, - net->data.network.name); + def->nets[i]->data.network.name); if (!network) { goto error_exit; } @@ -449,7 +448,7 @@ break; } case VIR_DOMAIN_NET_TYPE_BRIDGE: - bridge = net->data.bridge.brname; + bridge = def->nets[i]->data.bridge.brname; break; } @@ -461,8 +460,8 @@ } DEBUG0("calling vethCreate()"); - if (NULL != net->ifname) { - strcpy(parentVeth, net->ifname); + if (NULL != def->nets[i]->ifname) { + strcpy(parentVeth, def->nets[i]->ifname); } DEBUG("parentVeth: %s, containerVeth: %s", parentVeth, containerVeth); if (0 != (rc = vethCreate(parentVeth, PATH_MAX, containerVeth, PATH_MAX))) { @@ -470,15 +469,15 @@ _("failed to create veth device pair: %d"), rc); goto error_exit; } - if (NULL == net->ifname) { - net->ifname = strdup(parentVeth); + if (NULL == def->nets[i]->ifname) { + def->nets[i]->ifname = strdup(parentVeth); } if (VIR_REALLOC_N(*veths, (*nveths)+1) < 0) goto error_exit; if (((*veths)[(*nveths)++] = strdup(containerVeth)) == NULL) goto error_exit; - if (NULL == net->ifname) { + if (NULL == def->nets[i]->ifname) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("failed to allocate veth names")); goto error_exit; diff -r 44cb26ad18bc src/openvz_conf.c --- a/src/openvz_conf.c Fri Oct 03 12:21:48 2008 +0100 +++ b/src/openvz_conf.c Fri Oct 03 12:44:51 2008 +0100 @@ -171,11 +171,12 @@ return -1; } -static virDomainNetDefPtr -openvzReadNetworkConf(virConnectPtr conn, int veid) { +static int +openvzReadNetworkConf(virConnectPtr conn, + virDomainDefPtr def, + int veid) { int ret; - virDomainNetDefPtr net = NULL; - virDomainNetDefPtr new_net; + virDomainNetDefPtr net; char temp[4096]; char *token, *saveptr = NULL; @@ -193,17 +194,19 @@ } else if (ret > 0) { token = strtok_r(temp, " ", &saveptr); while (token != NULL) { - new_net = NULL; - if (VIR_ALLOC(new_net) < 0) + if (VIR_ALLOC(net) < 0) goto no_memory; - new_net->next = net; - net = new_net; net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; net->data.ethernet.ipaddr = strdup(token); if (net->data.ethernet.ipaddr == NULL) goto no_memory; + + if (VIR_REALLOC_N(def->nets, def->nnets + 1) < 0) + goto no_memory; + def->nets[def->nnets++] = net; + net = NULL; token = strtok_r(NULL, " ", &saveptr); } @@ -224,11 +227,8 @@ token = strtok_r(temp, ";", &saveptr); while (token != NULL) { /*add new device to list*/ - new_net = NULL; - if (VIR_ALLOC(new_net) < 0) + if (VIR_ALLOC(net) < 0) goto no_memory; - new_net->next = net; - net = new_net; net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; @@ -278,16 +278,21 @@ p = ++next; } while (p < token + strlen(token)); + if (VIR_REALLOC_N(def->nets, def->nnets + 1) < 0) + goto no_memory; + def->nets[def->nnets++] = net; + net = NULL; + token = strtok_r(NULL, ";", &saveptr); } } - return net; + return 0; no_memory: openvzError(conn, VIR_ERR_NO_MEMORY, NULL); error: virDomainNetDefFree(net); - return NULL; + return -1; } @@ -375,7 +380,7 @@ /* XXX load rest of VM config data .... */ - dom->def->nets = openvzReadNetworkConf(NULL, veid); + openvzReadNetworkConf(NULL, dom->def, veid); if (VIR_REALLOC_N(driver->domains.objs, driver->domains.count + 1) < 0) diff -r 44cb26ad18bc src/openvz_driver.c --- a/src/openvz_driver.c Fri Oct 03 12:21:48 2008 +0100 +++ b/src/openvz_driver.c Fri Oct 03 12:44:51 2008 +0100 @@ -117,21 +117,21 @@ ADD_ARG_LIT("create"); ADD_ARG_LIT(vmdef->name); - if (vmdef->fss) { - if (vmdef->fss->type != VIR_DOMAIN_FS_TYPE_TEMPLATE) { + if (vmdef->nfss) { + if (vmdef->fss[0]->type != VIR_DOMAIN_FS_TYPE_TEMPLATE) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("only filesystem templates are supported")); return -1; } - if (vmdef->fss->next) { + if (vmdef->nfss > 1) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("only one filesystem supported")); return -1; } ADD_ARG_LIT("--ostemplate"); - ADD_ARG_LIT(vmdef->fss->src); + ADD_ARG_LIT(vmdef->fss[0]->src); } #if 0 if ((vmdef->profile && *(vmdef->profile))) { @@ -394,12 +394,6 @@ } } - if (net->next != NULL) - if (openvzDomainSetNetwork(conn, vpsid, net->next) < 0) { - rc = -1; - goto exit; - } - exit: cmdExecFree(prog); VIR_FREE(mac); @@ -422,6 +416,7 @@ virDomainDefPtr vmdef = NULL; virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; + int i; const char *prog[OPENVZ_MAX_ARG]; prog[0] = NULL; @@ -471,10 +466,12 @@ if (dom) dom->id = -1; - if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->nets) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Could not configure network")); - goto exit; + for (i = 0 ; i < vmdef->nnets ; i++) { + if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->nets[i]) < 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Could not configure network")); + goto exit; + } } if (vmdef->vcpus > 0) { @@ -497,6 +494,7 @@ virDomainDefPtr vmdef = NULL; virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; + int i; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; const char *progstart[] = {VZCTL, "--quiet", "start", NULL, NULL}; const char *progcreate[OPENVZ_MAX_ARG]; @@ -542,10 +540,12 @@ goto exit; } - if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->nets) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Could not configure network")); - goto exit; + for (i = 0 ; i < vmdef->nnets ; i++) { + if (openvzDomainSetNetwork(conn, vmdef->name, vmdef->nets[i]) < 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Could not configure network")); + goto exit; + } } progstart[3] = vmdef->name; diff -r 44cb26ad18bc src/qemu_conf.c --- a/src/qemu_conf.c Fri Oct 03 12:21:48 2008 +0100 +++ b/src/qemu_conf.c Fri Oct 03 12:44:51 2008 +0100 @@ -723,13 +723,6 @@ char memory[50]; char vcpus[50]; char boot[VIR_DOMAIN_BOOT_LAST]; - virDomainDiskDefPtr disk = vm->def->disks; - virDomainNetDefPtr net = vm->def->nets; - virDomainInputDefPtr input = vm->def->inputs; - virDomainSoundDefPtr sound = vm->def->sounds; - virDomainHostdevDefPtr hostdev = vm->def->hostdevs; - virDomainChrDefPtr serial = vm->def->serials; - virDomainChrDefPtr parallel = vm->def->parallels; struct utsname ut; int disableKQEMU = 0; int qargc = 0, qarga = 0; @@ -923,10 +916,11 @@ } } - while (disk) { + for (i = 0 ; i < vm->def->ndisks ; i++) { char opt[PATH_MAX]; const char *media = NULL; int bootable = 0; + virDomainDiskDefPtr disk = vm->def->disks[i]; int idx = virDiskNameToIndex(disk->dst); const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); @@ -938,7 +932,6 @@ _("unsupported usb disk type for '%s'"), disk->src); goto error; } - disk = disk->next; continue; } @@ -974,12 +967,12 @@ ADD_ARG_LIT("-drive"); ADD_ARG_LIT(opt); - disk = disk->next; } } else { - while (disk) { + for (i = 0 ; i < vm->def->ndisks ; i++) { char dev[NAME_MAX]; char file[PATH_MAX]; + virDomainDiskDefPtr disk = vm->def->disks[i]; if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { @@ -989,7 +982,6 @@ _("unsupported usb disk type for '%s'"), disk->src); goto error; } - disk = disk->next; continue; } @@ -998,7 +990,6 @@ if (disk->src) { snprintf(dev, NAME_MAX, "-%s", "cdrom"); } else { - disk = disk->next; continue; } } else { @@ -1016,18 +1007,17 @@ ADD_ARG_LIT(dev); ADD_ARG_LIT(file); - - disk = disk->next; } } - if (!net) { + if (!vm->def->nnets) { ADD_ARG_LIT("-net"); ADD_ARG_LIT("none"); } else { int vlan = 0; - while (net) { + for (i = 0 ; i < vm->def->nnets ; i++) { char nic[100]; + virDomainNetDefPtr net = vm->def->nets[i]; if (snprintf(nic, sizeof(nic), "nic,macaddr=%02x:%02x:%02x:%02x:%02x:%02x,vlan=%d%s%s", @@ -1108,53 +1098,50 @@ } } - net = net->next; vlan++; } } - if (!serial) { + if (!vm->def->nserials) { ADD_ARG_LIT("-serial"); ADD_ARG_LIT("none"); } else { - while (serial) { + for (i = 0 ; i < vm->def->nserials ; i++) { char buf[4096]; + virDomainChrDefPtr serial = vm->def->serials[i]; if (qemudBuildCommandLineChrDevStr(serial, buf, sizeof(buf)) < 0) goto error; ADD_ARG_LIT("-serial"); ADD_ARG_LIT(buf); - - serial = serial->next; } } - if (!parallel) { + if (!vm->def->nparallels) { ADD_ARG_LIT("-parallel"); ADD_ARG_LIT("none"); } else { - while (parallel) { + for (i = 0 ; i < vm->def->nparallels ; i++) { char buf[4096]; + virDomainChrDefPtr parallel = vm->def->parallels[i]; if (qemudBuildCommandLineChrDevStr(parallel, buf, sizeof(buf)) < 0) goto error; ADD_ARG_LIT("-parallel"); ADD_ARG_LIT(buf); - - parallel = parallel->next; } } ADD_ARG_LIT("-usb"); - while (input) { + for (i = 0 ; i < vm->def->ninputs ; i++) { + virDomainInputDefPtr input = vm->def->inputs[i]; + if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { ADD_ARG_LIT("-usbdevice"); ADD_ARG_LIT(input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "mouse" : "tablet"); } - - input = input->next; } if (vm->def->graphics && @@ -1217,13 +1204,14 @@ } /* Add sound hardware */ - if (sound) { + if (vm->def->nsounds) { int size = 100; char *modstr; if (VIR_ALLOC_N(modstr, size+1) < 0) goto no_memory; - while(sound && size > 0) { + for (i = 0 ; i < vm->def->nsounds && size > 0 ; i++) { + virDomainSoundDefPtr sound = vm->def->sounds[i]; const char *model = virDomainSoundModelTypeToString(sound->model); if (!model) { VIR_FREE(modstr); @@ -1233,8 +1221,7 @@ } strncat(modstr, model, size); size -= strlen(model); - sound = sound->next; - if (sound) + if (i < (vm->def->nsounds - 1)) strncat(modstr, ",", size--); } ADD_ARG_LIT("-soundhw"); @@ -1242,9 +1229,10 @@ } /* Add host passthrough hardware */ - while (hostdev) { + for (i = 0 ; i < vm->def->nhostdevs ; i++) { int ret; char* usbdev; + virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i]; if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { @@ -1266,7 +1254,6 @@ ADD_ARG_LIT(usbdev); VIR_FREE(usbdev); } - hostdev = hostdev->next; } if (migrateFrom) { diff -r 44cb26ad18bc src/qemu_driver.c --- a/src/qemu_driver.c Fri Oct 03 12:21:48 2008 +0100 +++ b/src/qemu_driver.c Fri Oct 03 12:44:51 2008 +0100 @@ -498,8 +498,7 @@ { char *monitor = NULL; size_t offset = 0; - virDomainChrDefPtr chr; - int ret; + int ret, i; /* The order in which QEMU prints out the PTY paths is the order in which it procsses its monitor, serial @@ -511,25 +510,23 @@ goto cleanup; /* then the serial devices */ - chr = vm->def->serials; - while (chr) { + for (i = 0 ; i < vm->def->nserials ; i++) { + virDomainChrDefPtr chr = vm->def->serials[i]; if (chr->type == VIR_DOMAIN_CHR_TYPE_PTY) { if ((ret = qemudExtractMonitorPath(conn, output, &offset, &chr->data.file.path)) != 0) goto cleanup; } - chr = chr->next; } /* and finally the parallel devices */ - chr = vm->def->parallels; - while (chr) { + for (i = 0 ; i < vm->def->nparallels ; i++) { + virDomainChrDefPtr chr = vm->def->parallels[i]; if (chr->type == VIR_DOMAIN_CHR_TYPE_PTY) { if ((ret = qemudExtractMonitorPath(conn, output, &offset, &chr->data.file.path)) != 0) goto cleanup; } - chr = chr->next; } /* Got them all, so now open the monitor console */ @@ -2397,6 +2394,7 @@ char *cmd, *reply, *safe_path; char *devname = NULL; unsigned int qemuCmdFlags; + int i; if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -2405,12 +2403,12 @@ } newdisk = dev->data.disk; - origdisk = vm->def->disks; - while (origdisk) { - if (origdisk->bus == newdisk->bus && - STREQ(origdisk->dst, newdisk->dst)) + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (vm->def->disks[i]->bus == newdisk->bus && + STREQ(vm->def->disks[i]->dst, newdisk->dst)) { + origdisk = vm->def->disks[i]; break; - origdisk = origdisk->next; + } } if (!origdisk) { @@ -2512,7 +2510,6 @@ virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); int ret; char *cmd, *reply; - virDomainDiskDefPtr *dest, *prev, ptr; if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -2520,26 +2517,9 @@ return -1; } - /* Find spot in domain definition where we will put the disk */ - ptr = vm->def->disks; - prev = &(vm->def->disks); - while (ptr) { - if (STREQ(dev->data.disk->dst, ptr->dst)) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, - _("duplicate disk target '%s'"), - dev->data.disk->dst); - return -1; - } - if (virDomainDiskCompare(dev->data.disk, ptr) < 0) { - dest = &(ptr); - break; - } - prev = &(ptr->next); - ptr = ptr->next; - } - - if (!ptr) { - dest = prev; + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) { + qemudReportError(dom->conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL); + return -1; } ret = asprintf(&cmd, "usb_add disk:%s", dev->data.disk->src); @@ -2567,9 +2547,9 @@ return -1; } - /* Actually update the xml */ - dev->data.disk->next = *dest; - *prev = dev->data.disk; + vm->def->disks[vm->def->ndisks++] = dev->data.disk; + qsort(vm->def->disks, vm->def->ndisks, sizeof(*vm->def->disks), + virDomainDiskQSort); VIR_FREE(reply); VIR_FREE(cmd); @@ -2586,6 +2566,10 @@ if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); + return -1; + } + if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) { + qemudReportError(dom->conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL); return -1; } @@ -2622,9 +2606,7 @@ return -1; } - /* Update xml */ - dev->data.hostdev->next = vm->def->hostdevs; - vm->def->hostdevs = dev->data.hostdev; + vm->def->hostdevs[vm->def->nhostdevs++] = dev->data.hostdev; VIR_FREE(reply); VIR_FREE(cmd); @@ -2907,7 +2889,7 @@ #ifdef __linux__ struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); - virDomainNetDefPtr net; + int i; if (!vm) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -2928,8 +2910,9 @@ } /* Check the path is one of the domain's network interfaces. */ - for (net = vm->def->nets; net; net = net->next) { - if (net->ifname && STREQ (net->ifname, path)) + for (i = 0 ; i < vm->def->nnets ; i++) { + if (vm->def->nets[i]->ifname && + STREQ (vm->def->nets[i]->ifname, path)) goto ok; } @@ -2955,8 +2938,7 @@ { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); - virDomainDiskDefPtr disk; - int fd, ret = -1; + int fd, ret = -1, i; if (!vm) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -2971,9 +2953,10 @@ } /* Check the path belongs to this domain. */ - for (disk = vm->def->disks ; disk != NULL ; disk = disk->next) { - if (disk->src != NULL && - STREQ (disk->src, path)) goto found; + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (vm->def->disks[i]->src != NULL && + STREQ (vm->def->disks[i]->src, path)) + goto found; } qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, _("invalid path")); diff -r 44cb26ad18bc src/xend_internal.c --- a/src/xend_internal.c Fri Oct 03 12:21:48 2008 +0100 +++ b/src/xend_internal.c Fri Oct 03 12:44:51 2008 +0100 @@ -1647,7 +1647,7 @@ int xendConfigVersion) { const struct sexpr *cur, *node; - virDomainDiskDefPtr disk = NULL, prev = def->disks; + virDomainDiskDefPtr disk = NULL; for (cur = root; cur->kind == SEXPR_CONS; cur = cur->u.s.cdr) { node = cur->u.s.car; @@ -1781,12 +1781,10 @@ strchr(mode, '!')) disk->shared = 1; - if (prev) - prev->next = disk; - else - def->disks = disk; - - prev = disk; + if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) + goto no_memory; + + def->disks[def->ndisks++] = disk; disk = NULL; } } @@ -1807,7 +1805,7 @@ virDomainDefPtr def, const struct sexpr *root) { - virDomainNetDefPtr net = NULL, prev = def->nets; + virDomainNetDefPtr net = NULL; const struct sexpr *cur, *node; const char *tmp; int vif_index = 0; @@ -1880,10 +1878,10 @@ !(net->model = strdup(model))) goto no_memory; - if (prev) - prev->next = net; - else - def->nets = net; + if (VIR_REALLOC_N(def->nets, def->nnets + 1) < 0) + goto no_memory; + + def->nets[def->nnets++] = net; vif_index++; } } @@ -1905,22 +1903,22 @@ { if (STREQ(str, "all")) { int i; - virDomainSoundDefPtr prev = NULL; + + if (VIR_ALLOC_N(def->sounds, + VIR_DOMAIN_SOUND_MODEL_LAST) < 0) + goto no_memory; + for (i = 0 ; i < VIR_DOMAIN_SOUND_MODEL_LAST ; i++) { virDomainSoundDefPtr sound; if (VIR_ALLOC(sound) < 0) goto no_memory; sound->model = i; - if (prev) - prev->next = sound; - else - def->sounds = sound; - prev = sound; + def->sounds[def->nsounds++] = sound; } } else { char model[10]; const char *offset = str, *offset2; - virDomainSoundDefPtr prev = NULL; + do { int len; virDomainSoundDefPtr sound; @@ -1945,11 +1943,12 @@ goto error; } - if (prev) - prev->next = sound; - else - def->sounds = sound; - prev = sound; + if (VIR_REALLOC_N(def->sounds, def->nsounds+1) < 0) { + virDomainSoundDefFree(sound); + goto no_memory; + } + + def->sounds[def->nsounds++] = sound; offset = offset2 ? offset2 + 1 : NULL; } while (offset); } @@ -1968,7 +1967,6 @@ virDomainDefPtr def, const struct sexpr *root) { - virDomainInputDefPtr prev = def->inputs; struct sexpr *cur, *node; const char *tmp; @@ -1988,11 +1986,11 @@ else input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE; - if (prev) - prev->next = input; - else - def->inputs = input; - prev = input; + if (VIR_REALLOC_N(def->inputs, def->ninputs+1) < 0) { + VIR_FREE(input); + goto no_memory; + } + def->inputs[def->ninputs++] = input; } else { /* XXX Handle other non-input USB devices later */ } @@ -2333,34 +2331,31 @@ xendConfigVersion == 1) { tmp = sexpr_node(root, "domain/image/hvm/cdrom"); if ((tmp != NULL) && (tmp[0] != 0)) { - virDomainDiskDefPtr disk, prev; + virDomainDiskDefPtr disk; if (VIR_ALLOC(disk) < 0) goto no_memory; if (!(disk->src = strdup(tmp))) { - VIR_FREE(disk); + virDomainDiskDefFree(disk); goto no_memory; } disk->type = VIR_DOMAIN_DISK_TYPE_FILE; disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; if (!(disk->dst = strdup("hdc"))) { - VIR_FREE(disk); + virDomainDiskDefFree(disk); goto no_memory; } if (!(disk->driverName = strdup("file"))) { - VIR_FREE(disk); + virDomainDiskDefFree(disk); goto no_memory; } disk->bus = VIR_DOMAIN_DISK_BUS_IDE; disk->readonly = 1; - prev = def->disks; - while (prev && prev->next) { - prev = prev->next; - } - if (prev) - prev->next = disk; - else - def->disks = disk; + if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) { + virDomainDiskDefFree(disk); + goto no_memory; + } + def->disks[def->ndisks++] = disk; } } @@ -2372,7 +2367,7 @@ for (i = 0 ; i < sizeof(fds)/sizeof(fds[0]) ; i++) { tmp = sexpr_fmt_node(root, "domain/image/hvm/%s", fds[i]); if ((tmp != NULL) && (tmp[0] != 0)) { - virDomainDiskDefPtr disk, prev; + virDomainDiskDefPtr disk; if (VIR_ALLOC(disk) < 0) goto no_memory; if (!(disk->src = strdup(tmp))) { @@ -2382,26 +2377,25 @@ disk->type = VIR_DOMAIN_DISK_TYPE_FILE; disk->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY; if (!(disk->dst = strdup(fds[i]))) { - VIR_FREE(disk); + virDomainDiskDefFree(disk); goto no_memory; } if (!(disk->driverName = strdup("file"))) { - VIR_FREE(disk); + virDomainDiskDefFree(disk); goto no_memory; } disk->bus = VIR_DOMAIN_DISK_BUS_FDC; - prev = def->disks; - while (prev && prev->next) { - prev = prev->next; - } - if (prev) - prev->next = disk; - else - def->disks = disk; - } - } - } + if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) { + virDomainDiskDefFree(disk); + goto no_memory; + } + def->disks[def->ndisks++] = disk; + } + } + } + qsort(def->disks, def->ndisks, sizeof(*def->disks), + virDomainDiskQSort); /* in case of HVM we have USB device emulation */ if (hvm && @@ -2413,14 +2407,26 @@ if (hvm) { tmp = sexpr_node(root, "domain/image/hvm/serial"); if (tmp && STRNEQ(tmp, "none")) { - if ((def->serials = xenDaemonParseSxprChar(conn, tmp, tty)) == NULL) - goto error; + virDomainChrDefPtr chr; + if ((chr = xenDaemonParseSxprChar(conn, tmp, tty)) == NULL) + goto error; + if (VIR_REALLOC_N(def->serials, def->nserials+1) < 0) { + virDomainChrDefFree(chr); + goto no_memory; + } + def->serials[def->nserials++] = chr; } tmp = sexpr_node(root, "domain/image/hvm/parallel"); if (tmp && STRNEQ(tmp, "none")) { + virDomainChrDefPtr chr; /* XXX does XenD stuff parallel port tty info into xenstore somewhere ? */ - if ((def->parallels = xenDaemonParseSxprChar(conn, tmp, NULL)) == NULL) - goto error; + if ((chr = xenDaemonParseSxprChar(conn, tmp, NULL)) == NULL) + goto error; + if (VIR_REALLOC_N(def->parallels, def->nparallels+1) < 0) { + virDomainChrDefFree(chr); + goto no_memory; + } + def->parallels[def->nparallels++] = chr; } } else { /* Fake a paravirt console, since that's not in the sexpr */ @@ -4726,9 +4732,8 @@ xenUnifiedPrivatePtr priv; struct sexpr *root = NULL; int fd = -1, ret = -1; - int found = 0; - virDomainDefPtr def; - virDomainDiskDefPtr disk; + int found = 0, i; + virDomainDefPtr def; priv = (xenUnifiedPrivatePtr) domain->conn->privateData; @@ -4757,14 +4762,12 @@ if (!(def = xenDaemonParseSxpr(domain->conn, root, priv->xendConfigVersion, NULL))) goto cleanup; - disk = def->disks; - while (disk) { - if (disk->src && - STREQ(disk->src, path)) { + for (i = 0 ; i < def->ndisks ; i++) { + if (def->disks[i]->src && + STREQ(def->disks[i]->src, path)) { found = 1; break; } - disk = disk->next; } if (!found) { virXendError (domain->conn, VIR_ERR_INVALID_ARG, @@ -5211,21 +5214,20 @@ int xenDaemonFormatSxprSound(virConnectPtr conn, - virDomainSoundDefPtr sound, + virDomainDefPtr def, virBufferPtr buf) { const char *str; - virDomainSoundDefPtr prev = NULL; - - while (sound) { - if (!(str = virDomainSoundModelTypeToString(sound->model))) { - virXendError(conn, VIR_ERR_INTERNAL_ERROR, - _("unexpected sound model %d"), sound->model); - return -1; - } - virBufferVSprintf(buf, "%s%s", prev ? "," : "", str); - prev = sound; - sound = sound->next; + int i; + + for (i = 0 ; i < def->nsounds ; i++) { + if (!(str = virDomainSoundModelTypeToString(def->sounds[i]->model))) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("unexpected sound model %d"), + def->sounds[i]->model); + return -1; + } + virBufferVSprintf(buf, "%s%s", i ? "," : "", str); } return 0; @@ -5275,9 +5277,6 @@ char uuidstr[VIR_UUID_STRING_BUFLEN]; const char *tmp; int hvm = 0, i; - virDomainNetDefPtr net; - virDomainDiskDefPtr disk; - virDomainInputDefPtr input; virBufferAddLit(&buf, "(vm "); virBufferVSprintf(&buf, "(name '%s')", def->name); @@ -5389,16 +5388,14 @@ /* get the cdrom device file */ /* Only XenD <= 3.0.2 wants cdrom config here */ if (xendConfigVersion == 1) { - disk = def->disks; - while (disk) { - if (disk->type == VIR_DOMAIN_DISK_DEVICE_CDROM && - STREQ(disk->dst, "hdc") && - disk->src) { + for (i = 0 ; i < def->ndisks ; i++) { + if (def->disks[i]->type == VIR_DOMAIN_DISK_DEVICE_CDROM && + STREQ(def->disks[i]->dst, "hdc") && + def->disks[i]->src) { virBufferVSprintf(&buf, "(cdrom '%s')", - disk->src); + def->disks[i]->src); break; } - disk = disk->next; } } @@ -5411,16 +5408,13 @@ virBufferAddLit(&buf, "(usb 1)"); - input = def->inputs; - while (input) { - if (xenDaemonFormatSxprInput(conn, input, &buf) < 0) - goto error; - input = input->next; - } + for (i = 0 ; i < def->ninputs ; i++) + if (xenDaemonFormatSxprInput(conn, def->inputs[i], &buf) < 0) + goto error; if (def->parallels) { virBufferAddLit(&buf, "(parallel "); - if (xenDaemonFormatSxprChr(conn, def->parallels, &buf) < 0) + if (xenDaemonFormatSxprChr(conn, def->parallels[0], &buf) < 0) goto error; virBufferAddLit(&buf, ")"); } else { @@ -5428,7 +5422,7 @@ } if (def->serials) { virBufferAddLit(&buf, "(serial "); - if (xenDaemonFormatSxprChr(conn, def->serials, &buf) < 0) + if (xenDaemonFormatSxprChr(conn, def->serials[0], &buf) < 0) goto error; virBufferAddLit(&buf, ")"); } else { @@ -5440,7 +5434,7 @@ if (def->sounds) { virBufferAddLit(&buf, "(soundhw '"); - if (xenDaemonFormatSxprSound(conn, def->sounds, &buf) < 0) + if (xenDaemonFormatSxprSound(conn, def, &buf) < 0) goto error; virBufferAddLit(&buf, "')"); } @@ -5462,19 +5456,15 @@ virBufferAddLit(&buf, "))"); } - disk = def->disks; - while (disk) { - if (xenDaemonFormatSxprDisk(conn, disk, &buf, hvm, xendConfigVersion, 0) < 0) - goto error; - disk = disk->next; - } - - net = def->nets; - while (net) { - if (xenDaemonFormatSxprNet(conn, net, &buf, hvm, xendConfigVersion, 0) < 0) - goto error; - net = net->next; - } + for (i = 0 ; i < def->ndisks ; i++) + if (xenDaemonFormatSxprDisk(conn, def->disks[i], + &buf, hvm, xendConfigVersion, 0) < 0) + goto error; + + for (i = 0 ; i < def->nnets ; i++) + if (xenDaemonFormatSxprNet(conn, def->nets[i], + &buf, hvm, xendConfigVersion, 0) < 0) + goto error; /* New style PV graphics config xen >= 3.0.4, * or HVM graphics config xen >= 3.0.5 */ diff -r 44cb26ad18bc src/xend_internal.h --- a/src/xend_internal.h Fri Oct 03 12:21:48 2008 +0100 +++ b/src/xend_internal.h Fri Oct 03 12:44:51 2008 +0100 @@ -115,7 +115,7 @@ virBufferPtr buf); int xenDaemonFormatSxprSound(virConnectPtr conn, - virDomainSoundDefPtr sound, + virDomainDefPtr def, virBufferPtr buf); char * diff -r 44cb26ad18bc src/xm_internal.c --- a/src/xm_internal.c Fri Oct 03 12:21:48 2008 +0100 +++ b/src/xm_internal.c Fri Oct 03 12:44:51 2008 +0100 @@ -51,8 +51,6 @@ static int xenXMConfigSetString(virConfPtr conf, const char *setting, const char *str); -static int xenXMDiskCompare(virDomainDiskDefPtr a, - virDomainDiskDefPtr b); typedef struct xenXMConfCache *xenXMConfCachePtr; typedef struct xenXMConfCache { @@ -888,20 +886,9 @@ disk->shared = 1; /* Maintain list in sorted order according to target device name */ - if (def->disks == NULL) { - disk->next = def->disks; - def->disks = disk; - } else { - virDomainDiskDefPtr ptr = def->disks; - while (ptr) { - if (!ptr->next || xenXMDiskCompare(disk, ptr->next) < 0) { - disk->next = ptr->next; - ptr->next = disk; - break; - } - ptr = ptr->next; - } - } + if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) + goto no_memory; + def->disks[def->ndisks++] = disk; disk = NULL; skipdisk: @@ -928,25 +915,14 @@ disk->bus = VIR_DOMAIN_DISK_BUS_IDE; disk->readonly = 1; - - /* Maintain list in sorted order according to target device name */ - if (def->disks == NULL) { - disk->next = def->disks; - def->disks = disk; - } else { - virDomainDiskDefPtr ptr = def->disks; - while (ptr) { - if (!ptr->next || xenXMDiskCompare(disk, ptr->next) < 0) { - disk->next = ptr->next; - ptr->next = disk; - break; - } - ptr = ptr->next; - } - } + if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) + goto no_memory; + def->disks[def->ndisks++] = disk; disk = NULL; } } + qsort(def->disks, def->ndisks, sizeof(*def->disks), + virDomainDiskQSort); list = virConfGetValue(conf, "vif"); if (list && list->type == VIR_CONF_LIST) { @@ -1064,15 +1040,9 @@ !(net->model = strdup(model))) goto no_memory; - if (!def->nets) { - net->next = NULL; - def->nets = net; - } else { - virDomainNetDefPtr ptr = def->nets; - while (ptr->next) - ptr = ptr->next; - ptr->next = net; - } + if (VIR_REALLOC_N(def->nets, def->nnets+1) < 0) + goto no_memory; + def->nets[def->nnets++] = net; net = NULL; skipnic: @@ -1094,7 +1064,12 @@ input->type = STREQ(str, "tablet") ? VIR_DOMAIN_INPUT_TYPE_TABLET : VIR_DOMAIN_INPUT_TYPE_MOUSE; - def->inputs = input; + if (VIR_ALLOC_N(def->inputs, 1) < 0) { + virDomainInputDefFree(input); + goto no_memory; + } + def->inputs[0] = input; + def->ninputs = 1; } } @@ -1212,17 +1187,38 @@ } if (hvm) { + virDomainChrDefPtr chr = NULL; + if (xenXMConfigGetString(conn, conf, "parallel", &str, NULL) < 0) goto cleanup; if (str && STRNEQ(str, "none") && - !(def->parallels = xenDaemonParseSxprChar(conn, str, NULL))) + !(chr = xenDaemonParseSxprChar(conn, str, NULL))) goto cleanup; + + if (chr) { + if (VIR_ALLOC_N(def->parallels, 1) < 0) { + virDomainChrDefFree(chr); + goto no_memory; + } + def->parallels[0] = chr; + def->nparallels++; + chr = NULL; + } if (xenXMConfigGetString(conn, conf, "serial", &str, NULL) < 0) goto cleanup; if (str && STRNEQ(str, "none") && - !(def->serials = xenDaemonParseSxprChar(conn, str, NULL))) + !(chr = xenDaemonParseSxprChar(conn, str, NULL))) goto cleanup; + + if (chr) { + if (VIR_ALLOC_N(def->serials, 1) < 0) { + virDomainChrDefFree(chr); + goto no_memory; + } + def->serials[0] = chr; + def->nserials++; + } } else { if (!(def->console = xenDaemonParseSxprChar(conn, "pty", NULL))) goto cleanup; @@ -1821,8 +1817,6 @@ char *cpus = NULL; const char *lifecycle; char uuid[VIR_UUID_STRING_BUFLEN]; - virDomainDiskDefPtr disk; - virDomainNetDefPtr net; virConfValuePtr diskVal = NULL; virConfValuePtr netVal = NULL; @@ -1915,15 +1909,16 @@ goto no_memory; if (priv->xendConfigVersion == 1) { - disk = def->disks; - while (disk) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM && - disk->dst && STREQ(disk->dst, "hdc") && disk->src) { - if (xenXMConfigSetString(conf, "cdrom", disk->src) < 0) + for (i = 0 ; i < def->ndisks ; i++) { + if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM && + def->disks[i]->dst && + STREQ(def->disks[i]->dst, "hdc") && + def->disks[i]->src) { + if (xenXMConfigSetString(conf, "cdrom", + def->disks[i]->src) < 0) goto no_memory; break; } - disk = disk->next; } } @@ -1976,23 +1971,20 @@ if (hvm) { - virDomainInputDefPtr input; if (def->emulator && xenXMConfigSetString(conf, "device_model", def->emulator) < 0) goto no_memory; - input = def->inputs; - while (input) { - if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { + for (i = 0 ; i < def->ninputs ; i++) { + if (def->inputs[i]->bus == VIR_DOMAIN_INPUT_BUS_USB) { if (xenXMConfigSetInt(conf, "usb", 1) < 0) goto no_memory; if (xenXMConfigSetString(conf, "usbdevice", - input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? + def->inputs[i]->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "mouse" : "tablet") < 0) goto no_memory; break; } - input = input->next; } } @@ -2092,27 +2084,24 @@ } /* analyze of the devices */ - disk = def->disks; if (VIR_ALLOC(diskVal) < 0) goto no_memory; diskVal->type = VIR_CONF_LIST; diskVal->list = NULL; - while (disk) { + for (i = 0 ; i < def->ndisks ; i++) { if (priv->xendConfigVersion == 1 && - disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM && - disk->dst && STREQ(disk->dst, "hdc")) { - disk = disk->next; + def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM && + def->disks[i]->dst && + STREQ(def->disks[i]->dst, "hdc")) { continue; } - if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) continue; - if (xenXMDomainConfigFormatDisk(conn, diskVal, disk, + if (xenXMDomainConfigFormatDisk(conn, diskVal, def->disks[i], hvm, priv->xendConfigVersion) < 0) goto cleanup; - - disk = disk->next; } if (diskVal->list == NULL) VIR_FREE(diskVal); @@ -2123,18 +2112,16 @@ diskVal = NULL; - net = def->nets; if (VIR_ALLOC(netVal) < 0) goto no_memory; netVal->type = VIR_CONF_LIST; netVal->list = NULL; - while (net) { - if (xenXMDomainConfigFormatNet(conn, netVal, net, + for (i = 0 ; i < def->nnets ; i++) { + if (xenXMDomainConfigFormatNet(conn, netVal, + def->nets[i], hvm) < 0) goto cleanup; - - net = net->next; } if (netVal->list == NULL) VIR_FREE(netVal); @@ -2145,12 +2132,12 @@ netVal = NULL; if (hvm) { - if (def->parallels) { + if (def->nparallels) { virBuffer buf = VIR_BUFFER_INITIALIZER; char *str; int ret; - ret = xenDaemonFormatSxprChr(conn, def->parallels, &buf); + ret = xenDaemonFormatSxprChr(conn, def->parallels[0], &buf); str = virBufferContentAndReset(&buf); if (ret == 0) ret = xenXMConfigSetString(conf, "parallel", str); @@ -2162,12 +2149,12 @@ goto no_memory; } - if (def->serials) { + if (def->nserials) { virBuffer buf = VIR_BUFFER_INITIALIZER; char *str; int ret; - ret = xenDaemonFormatSxprChr(conn, def->serials, &buf); + ret = xenDaemonFormatSxprChr(conn, def->serials[0], &buf); str = virBufferContentAndReset(&buf); if (ret == 0) ret = xenXMConfigSetString(conf, "serial", str); @@ -2184,7 +2171,7 @@ virBuffer buf = VIR_BUFFER_INITIALIZER; char *str = NULL; int ret = xenDaemonFormatSxprSound(conn, - def->sounds, + def, &buf); str = virBufferContentAndReset(&buf); if (ret == 0) @@ -2433,14 +2420,6 @@ return virHashSize(nameConfigMap); } -static int xenXMDiskCompare(virDomainDiskDefPtr a, - virDomainDiskDefPtr b) { - if (a->bus == b->bus) - return virDiskNameToIndex(a->dst) - virDiskNameToIndex(b->dst); - else - return a->bus - b->bus; -} - /** * xenXMDomainAttachDevice: @@ -2458,6 +2437,7 @@ xenXMConfCachePtr entry = NULL; int ret = -1; virDomainDeviceDefPtr dev = NULL; + virDomainDefPtr def; if ((!domain) || (!domain->conn) || (!domain->name) || (!xml)) { xenXMError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG, @@ -2473,6 +2453,7 @@ return -1; if (!(entry = virHashLookup(configCache, filename))) return -1; + def = entry->def; if (!(dev = virDomainDeviceDefParse(domain->conn, entry->def, @@ -2482,34 +2463,24 @@ switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: { - /* Maintain list in sorted order according to target device name */ - if (entry->def->disks == NULL) { - dev->data.disk->next = entry->def->disks; - entry->def->disks = dev->data.disk; - } else { - virDomainDiskDefPtr ptr = entry->def->disks; - while (ptr) { - if (!ptr->next || xenXMDiskCompare(dev->data.disk, ptr->next) < 0) { - dev->data.disk->next = ptr->next; - ptr->next = dev->data.disk; - break; - } - ptr = ptr->next; - } + if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) { + xenXMError(domain->conn, VIR_ERR_NO_MEMORY, NULL); + goto cleanup; } + def->disks[def->ndisks++] = dev->data.disk; dev->data.disk = NULL; + qsort(def->disks, def->ndisks, sizeof(*def->disks), + virDomainDiskQSort); } break; case VIR_DOMAIN_DEVICE_NET: { - virDomainNetDefPtr net = entry->def->nets; - while (net && net->next) - net = net->next; - if (net) - net->next = dev->data.net; - else - entry->def->nets = dev->data.net; + if (VIR_REALLOC_N(def->nets, def->nnets+1) < 0) { + xenXMError(domain->conn, VIR_ERR_NO_MEMORY, NULL); + goto cleanup; + } + def->nets[def->nnets++] = dev->data.net; dev->data.net = NULL; break; } @@ -2571,7 +2542,9 @@ const char *filename = NULL; xenXMConfCachePtr entry = NULL; virDomainDeviceDefPtr dev = NULL; + virDomainDefPtr def; int ret = -1; + int i; if ((!domain) || (!domain->conn) || (!domain->name) || (!xml)) { xenXMError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG, @@ -2586,6 +2559,7 @@ return -1; if (!(entry = virHashLookup(configCache, filename))) return -1; + def = entry->def; if (!(dev = virDomainDeviceDefParse(domain->conn, entry->def, @@ -2595,42 +2569,34 @@ switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: { - virDomainDiskDefPtr disk = entry->def->disks; - virDomainDiskDefPtr prev = NULL; - while (disk) { - if (disk->dst && + for (i = 0 ; i < def->ndisks ; i++) { + if (def->disks[i]->dst && dev->data.disk->dst && - STREQ(disk->dst, dev->data.disk->dst)) { - if (prev) { - prev->next = disk->next; - } else { - entry->def->disks = disk->next; - } - virDomainDiskDefFree(disk); + STREQ(def->disks[i]->dst, dev->data.disk->dst)) { + virDomainDiskDefFree(def->disks[i]); + if (i < (def->ndisks - 1)) + memmove(def->disks + i, + def->disks + i + 1, + def->ndisks - (i + 1)); break; } - prev = disk; - disk = disk->next; } break; } case VIR_DOMAIN_DEVICE_NET: { - virDomainNetDefPtr net = entry->def->nets; - virDomainNetDefPtr prev = NULL; - while (net) { - if (!memcmp(net->mac, dev->data.net->mac, VIR_DOMAIN_NET_MAC_SIZE)) { - if (prev) { - prev->next = net->next; - } else { - entry->def->nets = net->next; - } - virDomainNetDefFree(net); + for (i = 0 ; i < def->nnets ; i++) { + if (!memcmp(def->nets[i]->mac, + dev->data.net->mac, + VIR_DOMAIN_NET_MAC_SIZE)) { + virDomainNetDefFree(def->nets[i]); + if (i < (def->nnets - 1)) + memmove(def->nets + i, + def->nets + i + 1, + def->nnets - (i + 1)); break; } - prev = net; - net = net->next; } break; } diff -r 44cb26ad18bc tests/sexpr2xmldata/sexpr2xml-fv-v2.xml --- a/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml Fri Oct 03 12:21:48 2008 +0100 +++ b/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml Fri Oct 03 12:44:51 2008 +0100 @@ -18,16 +18,16 @@ <on_crash>restart</on_crash> <devices> <emulator>/usr/lib64/xen/bin/qemu-dm</emulator> + <disk type='file' device='disk'> + <driver name='file'/> + <source file='/root/foo.img'/> + <target dev='hda' bus='ide'/> + </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso'/> <target dev='hdc' bus='ide'/> <readonly/> - </disk> - <disk type='file' device='disk'> - <driver name='file'/> - <source file='/root/foo.img'/> - <target dev='hda' bus='ide'/> </disk> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Fri, Oct 03, 2008 at 01:29:44PM +0100, Daniel P. Berrange wrote:
This one isn't strictly needed yet, but while we're removing linked lists we might as well kill off all of them. So this removes the linked lists for devices associated with a domain.
In general this makes sense, this makes for smaller structure (no next link) iterations are done on a compact memory structure, and this makes sorting way simpler thank to qsort, I guess it's beneficial. I remember though that realloc() can be really really slow on some Windows [1] and as for the domain tables, it would be even nicer if we could use some kind of generic API for the table iterations. Sounds fine, +1 Daniel [1] http://mail.gnome.org/archives/xml/2004-July/msg00093.html -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel P. Berrange
-
Daniel Veillard