- Devel - libvirt List Archives

[libvirt] PATCH: Centralize some duplicated NUMA code
by Daniel P. Berrange 03 Jun '09

03 Jun '09

The QEMU and UML drivers currently duplicate a non-trivial chunk of NUMA code. This patch moves it all into src/nodeinfo.c which already has some other common NUMA code for capabilities initialization. In doing so it changes the functions to have a common 'nodeXXXX' naming scheme, to avoid clash with the public APIs using 'virXXXX'. Finally, these central functions can be directly used in the driver function tables, so we kill off some pointless wrappers and just add these funtions directly to drivers. libvirt_private.syms | 6 +- lxc_conf.c | 2 nodeinfo.c | 127 +++++++++++++++++++++++++++++++++++++++++++-------- nodeinfo.h | 11 +++- openvz_conf.c | 7 -- openvz_driver.c | 7 -- qemu_conf.c | 2 qemu_driver.c | 97 ++------------------------------------ uml_conf.c | 2 uml_driver.c | 84 +-------------------------------- vbox/vbox_tmpl.c | 7 -- 11 files changed, 138 insertions(+), 214 deletions(-) Daniel diff -r 91d1b6ebc112 src/libvirt_private.syms --- a/src/libvirt_private.syms Wed May 27 22:57:21 2009 +0100 +++ b/src/libvirt_private.syms Thu May 28 11:33:16 2009 +0100 @@ -220,8 +220,10 @@ virNetworkObjUnlock; # nodeinfo.h -virNodeInfoPopulate; -virCapsInitNUMA; +nodeGetInfo; +nodeCapsInitNUMA; +nodeGetCellsFreeMemory; +nodeGetFreeMemory; # node_device_conf.h diff -r 91d1b6ebc112 src/lxc_conf.c --- a/src/lxc_conf.c Wed May 27 22:57:21 2009 +0100 +++ b/src/lxc_conf.c Thu May 28 11:33:16 2009 +0100 @@ -46,7 +46,7 @@ virCapsPtr lxcCapsInit(void) 0, 0)) == NULL) goto no_memory; - if (virCapsInitNUMA(caps) < 0) + if (nodeCapsInitNUMA(caps) < 0) goto no_memory; /* XXX shouldn't 'borrow' KVM's prefix */ diff -r 91d1b6ebc112 src/nodeinfo.c --- a/src/nodeinfo.c Wed May 27 22:57:21 2009 +0100 +++ b/src/nodeinfo.c Thu May 28 11:33:16 2009 +0100 @@ -48,6 +48,10 @@ #define VIR_FROM_THIS VIR_FROM_NONE +#define nodeReportError(conn, code, fmt...) \ + virReportErrorHelper(conn, VIR_FROM_NONE, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) + #ifdef __linux__ #define CPUINFO_PATH "/proc/cpuinfo" @@ -73,9 +77,8 @@ int linuxNodeInfoCPUPopulate(virConnectP while (*buf && c_isspace(*buf)) buf++; if (*buf != ':') { - virRaiseError(conn, NULL, NULL, 0, VIR_ERR_INTERNAL_ERROR, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "%s", _("parsing cpuinfo processor")); + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("parsing cpuinfo processor")); return -1; } nodeinfo->cpus++; @@ -86,9 +89,8 @@ int linuxNodeInfoCPUPopulate(virConnectP while (*buf && c_isspace(*buf)) buf++; if (*buf != ':' || !buf[1]) { - virRaiseError(conn, NULL, NULL, 0, VIR_ERR_INTERNAL_ERROR, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "%s", _("parsing cpuinfo cpu MHz")); + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("parsing cpuinfo cpu MHz")); return -1; } if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0 @@ -102,9 +104,8 @@ int linuxNodeInfoCPUPopulate(virConnectP while (*buf && c_isspace(*buf)) buf++; if (*buf != ':' || !buf[1]) { - virRaiseError(conn, NULL, NULL, 0, VIR_ERR_INTERNAL_ERROR, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "parsing cpuinfo cpu cores %c", *buf); + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "parsing cpuinfo cpu cores %c", *buf); return -1; } if (virStrToLong_ui(buf+1, &p, 10, &id) == 0 @@ -115,9 +116,8 @@ int linuxNodeInfoCPUPopulate(virConnectP } if (!nodeinfo->cpus) { - virRaiseError(conn, NULL, NULL, 0, VIR_ERR_INTERNAL_ERROR, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "%s", _("no cpus found")); + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("no cpus found")); return -1; } @@ -133,8 +133,8 @@ int linuxNodeInfoCPUPopulate(virConnectP #endif -int virNodeInfoPopulate(virConnectPtr conn, - virNodeInfoPtr nodeinfo) { +int nodeGetInfo(virConnectPtr conn, + virNodeInfoPtr nodeinfo) { #ifdef HAVE_UNAME struct utsname info; @@ -170,9 +170,8 @@ int virNodeInfoPopulate(virConnectPtr co } #else /* XXX Solaris will need an impl later if they port QEMU driver */ - virRaiseError(conn, NULL, NULL, 0, VIR_ERR_INTERNAL_ERROR, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "%s:%s not implemented on this platform\n", __FILE__, __FUNCTION__); + nodeError(conn, VIR_ERR_NO_SUPPORT, "%s" + _("node info not implemented on this platform")); return -1; #endif } @@ -189,7 +188,7 @@ int virNodeInfoPopulate(virConnectPtr co (((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1) int -virCapsInitNUMA(virCapsPtr caps) +nodeCapsInitNUMA(virCapsPtr caps) { int n; unsigned long *mask = NULL; @@ -237,6 +236,96 @@ cleanup: VIR_FREE(mask); return ret; } + + +int +nodeGetCellsFreeMemory(virConnectPtr conn, + unsigned long long *freeMems, + int startCell, + int maxCells) +{ + int n, lastCell, numCells; + int ret = -1; + int maxCell; + + if (numa_available() < 0) { + nodeReportError(conn, VIR_ERR_NO_SUPPORT, + "%s", _("NUMA not supported on this host")); + goto cleanup; + } + maxCell = numa_max_node(); + if (startCell > maxCell) { + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("start cell %d out of range (0-%d)"), + startCell, maxCell); + goto cleanup; + } + lastCell = startCell + maxCells - 1; + if (lastCell > maxCell) + lastCell = maxCell; + + for (numCells = 0, n = startCell ; n <= lastCell ; n++) { + long long mem; + if (numa_node_size64(n, &mem) < 0) { + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to query NUMA free memory")); + goto cleanup; + } + freeMems[numCells++] = mem; + } + ret = numCells; + +cleanup: + return ret; +} + +unsigned long long +nodeGetFreeMemory(virConnectPtr conn) +{ + unsigned long long freeMem = 0; + int n; + + if (numa_available() < 0) { + nodeReportError(conn, VIR_ERR_NO_SUPPORT, + "%s", _("NUMA not supported on this host")); + goto cleanup; + } + + for (n = 0 ; n <= numa_max_node() ; n++) { + long long mem; + if (numa_node_size64(n, &mem) < 0) { + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to query NUMA free memory")); + goto cleanup; + } + freeMem += mem; + } + +cleanup: + return freeMem; +} + #else -int virCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { return 0; } +int nodeCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { + return 0; +} + +int nodeGetCellsFreeMemory(virConnectPtr conn, + unsigned long long *freeMems ATTRIBUTE_UNUSED, + int startCell ATTRIBUTE_UNUSED, + int maxCells ATTRIBUTE_UNUSED) +{ + nodeReportError(conn, VIR_ERR_NO_SUPPORT, "%s", + _("NUMA memory information not available on this platform")); + return -1; +} + +unsigned long long nodeGetFreeMemory(virConnectPtr conn) +{ + nodeReportError(conn, VIR_ERR_NO_SUPPORT, "%s", + _("NUMA memory information not available on this platform")); + return 0; +} #endif + + diff -r 91d1b6ebc112 src/nodeinfo.h --- a/src/nodeinfo.h Wed May 27 22:57:21 2009 +0100 +++ b/src/nodeinfo.h Thu May 28 11:33:16 2009 +0100 @@ -27,7 +27,14 @@ #include "libvirt/libvirt.h" #include "capabilities.h" -int virNodeInfoPopulate(virConnectPtr conn, virNodeInfoPtr nodeinfo); -int virCapsInitNUMA(virCapsPtr caps); +int nodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo); +int nodeCapsInitNUMA(virCapsPtr caps); + + +int nodeGetCellsFreeMemory(virConnectPtr conn, + unsigned long long *freeMems, + int startCell, + int maxCells); +unsigned long long nodeGetFreeMemory(virConnectPtr conn); #endif /* __VIR_NODEINFO_H__*/ diff -r 91d1b6ebc112 src/openvz_conf.c --- a/src/openvz_conf.c Wed May 27 22:57:21 2009 +0100 +++ b/src/openvz_conf.c Thu May 28 11:33:16 2009 +0100 @@ -148,7 +148,7 @@ virCapsPtr openvzCapsInit(void) 0, 0)) == NULL) goto no_memory; - if (virCapsInitNUMA(caps) < 0) + if (nodeCapsInitNUMA(caps) < 0) goto no_memory; virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 }); @@ -527,11 +527,8 @@ openvzGetNodeCPUs(void) { virNodeInfo nodeinfo; - if (virNodeInfoPopulate(NULL, &nodeinfo) < 0) { - openvzError(NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("Cound not read nodeinfo")); + if (nodeGetInfo(NULL, &nodeinfo) < 0) return 0; - } return nodeinfo.cpus; } diff -r 91d1b6ebc112 src/openvz_driver.c --- a/src/openvz_driver.c Wed May 27 22:57:21 2009 +0100 +++ b/src/openvz_driver.c Thu May 28 11:33:16 2009 +0100 @@ -1136,11 +1136,6 @@ static const char *openvzGetType(virConn return strdup("OpenVZ"); } -static int openvzGetNodeInfo(virConnectPtr conn, - virNodeInfoPtr nodeinfo) { - return virNodeInfoPopulate(conn, nodeinfo); -} - static char *openvzGetCapabilities(virConnectPtr conn) { struct openvz_driver *driver = conn->privateData; char *ret; @@ -1316,7 +1311,7 @@ static virDriver openvzDriver = { openvzGetVersion, /* version */ NULL, /* getHostname */ openvzGetMaxVCPUs, /* getMaxVcpus */ - openvzGetNodeInfo, /* nodeGetInfo */ + nodeGetInfo, /* nodeGetInfo */ openvzGetCapabilities, /* getCapabilities */ openvzListDomains, /* listDomains */ openvzNumDomains, /* numOfDomains */ diff -r 91d1b6ebc112 src/qemu_conf.c --- a/src/qemu_conf.c Wed May 27 22:57:21 2009 +0100 +++ b/src/qemu_conf.c Thu May 28 11:33:16 2009 +0100 @@ -377,7 +377,7 @@ virCapsPtr qemudCapsInit(void) { /* Using KVM's mac prefix for QEMU too */ virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 }); - if (virCapsInitNUMA(caps) < 0) + if (nodeCapsInitNUMA(caps) < 0) goto no_memory; virCapabilitiesAddHostMigrateTransport(caps, diff -r 91d1b6ebc112 src/qemu_driver.c --- a/src/qemu_driver.c Wed May 27 22:57:21 2009 +0100 +++ b/src/qemu_driver.c Thu May 28 11:33:16 2009 +0100 @@ -44,11 +44,6 @@ #include <sys/wait.h> #include <sys/ioctl.h> -#if HAVE_NUMACTL -#define NUMA_VERSION1_COMPATIBILITY 1 -#include <numa.h> -#endif - #if HAVE_SCHED_H #include <sched.h> #endif @@ -1099,7 +1094,7 @@ qemudInitCpus(virConnectPtr conn, int i, maxcpu = QEMUD_CPUMASK_LEN; virNodeInfo nodeinfo; - if (virNodeInfoPopulate(conn, &nodeinfo) < 0) + if (nodeGetInfo(conn, &nodeinfo) < 0) return -1; /* setaffinity fails if you set bits for CPUs which @@ -1869,11 +1864,6 @@ static int qemudGetMaxVCPUs(virConnectPt return -1; } -static int qemudGetNodeInfo(virConnectPtr conn, - virNodeInfoPtr nodeinfo) { - return virNodeInfoPopulate(conn, nodeinfo); -} - static char *qemudGetCapabilities(virConnectPtr conn) { struct qemud_driver *driver = conn->privateData; @@ -1890,76 +1880,6 @@ static char *qemudGetCapabilities(virCon } -#if HAVE_NUMACTL -static int -qemudNodeGetCellsFreeMemory(virConnectPtr conn, - unsigned long long *freeMems, - int startCell, - int maxCells) -{ - int n, lastCell, numCells; - int ret = -1; - int maxCell; - - if (numa_available() < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, - "%s", _("NUMA not supported on this host")); - goto cleanup; - } - maxCell = numa_max_node(); - if (startCell > maxCell) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("start cell %d out of range (0-%d)"), - startCell, maxCell); - goto cleanup; - } - lastCell = startCell + maxCells - 1; - if (lastCell > maxCell) - lastCell = maxCell; - - for (numCells = 0, n = startCell ; n <= lastCell ; n++) { - long long mem; - if (numa_node_size64(n, &mem) < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("Failed to query NUMA free memory")); - goto cleanup; - } - freeMems[numCells++] = mem; - } - ret = numCells; - -cleanup: - return ret; -} - -static unsigned long long -qemudNodeGetFreeMemory (virConnectPtr conn) -{ - unsigned long long freeMem = 0; - int n; - - if (numa_available() < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, - "%s", _("NUMA not supported on this host")); - goto cleanup; - } - - for (n = 0 ; n <= numa_max_node() ; n++) { - long long mem; - if (numa_node_size64(n, &mem) < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("Failed to query NUMA free memory")); - goto cleanup; - } - freeMem += mem; - } - -cleanup: - return freeMem; -} - -#endif - static int qemudGetProcessInfo(unsigned long long *cpuTime, int pid) { char proc[PATH_MAX]; FILE *pidinfo; @@ -2984,7 +2904,7 @@ qemudDomainPinVcpu(virDomainPtr dom, goto cleanup; } - if (virNodeInfoPopulate(dom->conn, &nodeinfo) < 0) + if (nodeGetInfo(dom->conn, &nodeinfo) < 0) goto cleanup; maxcpu = maplen * 8; @@ -3046,7 +2966,7 @@ qemudDomainGetVcpus(virDomainPtr dom, goto cleanup; } - if (virNodeInfoPopulate(dom->conn, &nodeinfo) < 0) + if (nodeGetInfo(dom->conn, &nodeinfo) < 0) goto cleanup; maxcpu = maplen * 8; @@ -5338,7 +5258,7 @@ static virDriver qemuDriver = { qemudGetVersion, /* version */ qemudGetHostname, /* getHostname */ qemudGetMaxVCPUs, /* getMaxVcpus */ - qemudGetNodeInfo, /* nodeGetInfo */ + nodeGetInfo, /* nodeGetInfo */ qemudGetCapabilities, /* getCapabilities */ qemudListDomains, /* listDomains */ qemudNumDomains, /* numOfDomains */ @@ -5392,13 +5312,8 @@ static virDriver qemuDriver = { qemudDomainInterfaceStats, /* domainInterfaceStats */ qemudDomainBlockPeek, /* domainBlockPeek */ qemudDomainMemoryPeek, /* domainMemoryPeek */ -#if HAVE_NUMACTL - qemudNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ - qemudNodeGetFreeMemory, /* getFreeMemory */ -#else - NULL, /* nodeGetCellsFreeMemory */ - NULL, /* getFreeMemory */ -#endif + nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ + nodeGetFreeMemory, /* getFreeMemory */ qemudDomainEventRegister, /* domainEventRegister */ qemudDomainEventDeregister, /* domainEventDeregister */ qemudDomainMigratePrepare2, /* domainMigratePrepare2 */ diff -r 91d1b6ebc112 src/uml_conf.c --- a/src/uml_conf.c Wed May 27 22:57:21 2009 +0100 +++ b/src/uml_conf.c Thu May 28 11:33:16 2009 +0100 @@ -62,7 +62,7 @@ virCapsPtr umlCapsInit(void) { 0, 0)) == NULL) goto no_memory; - if (virCapsInitNUMA(caps) < 0) + if (nodeCapsInitNUMA(caps) < 0) goto no_memory; if ((guest = virCapabilitiesAddGuest(caps, diff -r 91d1b6ebc112 src/uml_driver.c --- a/src/uml_driver.c Wed May 27 22:57:21 2009 +0100 +++ b/src/uml_driver.c Thu May 28 11:33:16 2009 +0100 @@ -45,11 +45,6 @@ #include <sys/ioctl.h> #include <sys/inotify.h> -#if HAVE_NUMACTL -#define NUMA_VERSION1_COMPATIBILITY 1 -#include <numa.h> -#endif - #include "uml_driver.h" #include "uml_conf.h" #include "event.h" @@ -935,11 +930,6 @@ static const char *umlGetType(virConnect return "UML"; } -static int umlGetNodeInfo(virConnectPtr conn, - virNodeInfoPtr nodeinfo) { - return virNodeInfoPopulate(conn, nodeinfo); -} - static char *umlGetCapabilities(virConnectPtr conn) { struct uml_driver *driver = (struct uml_driver *)conn->privateData; @@ -954,69 +944,6 @@ static char *umlGetCapabilities(virConne } -#if HAVE_NUMACTL -static int -umlNodeGetCellsFreeMemory(virConnectPtr conn, - unsigned long long *freeMems, - int startCell, - int maxCells) -{ - int n, lastCell, numCells; - int ret = -1; - - if (numa_available() < 0) { - umlReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, - "%s", _("NUMA not supported on this host")); - goto cleanup; - } - lastCell = startCell + maxCells - 1; - if (lastCell > numa_max_node()) - lastCell = numa_max_node(); - - for (numCells = 0, n = startCell ; n <= lastCell ; n++) { - long long mem; - if (numa_node_size64(n, &mem) < 0) { - umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("Failed to query NUMA free memory")); - goto cleanup; - } - freeMems[numCells++] = mem; - } - ret = numCells; - -cleanup: - return ret; -} - -static unsigned long long -umlNodeGetFreeMemory (virConnectPtr conn) -{ - unsigned long long freeMem = 0; - unsigned long long ret = -1; - int n; - - if (numa_available() < 0) { - umlReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, - "%s", _("NUMA not supported on this host")); - goto cleanup; - } - - for (n = 0 ; n <= numa_max_node() ; n++) { - long long mem; - if (numa_node_size64(n, &mem) < 0) { - umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("Failed to query NUMA free memory")); - goto cleanup; - } - freeMem += mem; - } - ret = freeMem; - -cleanup: - return ret; -} - -#endif static int umlGetProcessInfo(unsigned long long *cpuTime, int pid) { char proc[PATH_MAX]; @@ -1826,7 +1753,7 @@ static virDriver umlDriver = { umlGetVersion, /* version */ umlGetHostname, /* getHostname */ NULL, /* getMaxVcpus */ - umlGetNodeInfo, /* nodeGetInfo */ + nodeGetInfo, /* nodeGetInfo */ umlGetCapabilities, /* getCapabilities */ umlListDomains, /* listDomains */ umlNumDomains, /* numOfDomains */ @@ -1875,13 +1802,8 @@ static virDriver umlDriver = { NULL, /* domainInterfaceStats */ umlDomainBlockPeek, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ -#if HAVE_NUMACTL - umlNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ - umlNodeGetFreeMemory, /* getFreeMemory */ -#else - NULL, /* nodeGetCellsFreeMemory */ - NULL, /* getFreeMemory */ -#endif + virNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ + virNodeGetFreeMemory, /* getFreeMemory */ NULL, /* domainEventRegister */ NULL, /* domainEventDeregister */ NULL, /* domainMigratePrepare2 */ diff -r 91d1b6ebc112 src/vbox/vbox_tmpl.c --- a/src/vbox/vbox_tmpl.c Wed May 27 22:57:21 2009 +0100 +++ b/src/vbox/vbox_tmpl.c Thu May 28 11:33:16 2009 +0100 @@ -186,7 +186,7 @@ static virCapsPtr vboxCapsInit(void) { 0, 0)) == NULL) goto no_memory; - if (virCapsInitNUMA(caps) < 0) + if (nodeCapsInitNUMA(caps) < 0) goto no_memory; virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x08, 0x00, 0x27 }); @@ -413,9 +413,6 @@ static int vboxGetMaxVcpus(virConnectPtr return ret; } -static int vboxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) { - return virNodeInfoPopulate(conn, nodeinfo); -} static char *vboxGetCapabilities(virConnectPtr conn) { vboxGlobalData *data = conn->privateData; @@ -4909,7 +4906,7 @@ virDriver NAME(Driver) = { .version = vboxGetVersion, .getHostname = vboxGetHostname, .getMaxVcpus = vboxGetMaxVcpus, - .nodeGetInfo = vboxNodeGetInfo, + .nodeGetInfo = nodeGetInfo, .getCapabilities = vboxGetCapabilities, .listDomains = vboxListDomains, .numOfDomains = vboxNumOfDomains, -- |: 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 :|

3 3

[libvirt] PATCH: Include OS driver name in device information
by Daniel P. Berrange 03 Jun '09

03 Jun '09

This patch extends the node device XML from $ virsh nodedev-dumpxml pci_8086_27d6 <device> <name>pci_8086_27d6</name> <parent>computer</parent> <capability type='pci'> <domain>0</domain> <bus>0</bus> <slot>28</slot> <function>3</function> <product id='0x27d6'>82801G (ICH7 Family) PCI Express Port 4</product> <vendor id='0x8086'>Intel Corporation</vendor> </capability> </device> To, also include the operating system driver name. $ virsh nodedev-dumpxml pci_8086_27d6 <device> <name>pci_8086_27d6</name> <parent>computer</parent> <driver> <name>pcieport-driver</name> </driver> <capability type='pci'> <domain>0</domain> <bus>0</bus> <slot>28</slot> <function>3</function> <product id='0x27d6'>82801G (ICH7 Family) PCI Express Port 4</product> <vendor id='0x8086'>Intel Corporation</vendor> </capability> </device> We're not defining any particular semantics for the driver name, it is just a opaque string following rules of the OS in question. Daniel diff -r ec78a5d6c00c src/node_device.c --- a/src/node_device.c Thu May 28 14:42:24 2009 +0100 +++ b/src/node_device.c Thu May 28 16:00:12 2009 +0100 @@ -46,6 +46,60 @@ static int dev_has_cap(const virNodeDevi return 0; } +#ifdef __linux__ +static int update_driver_name(virConnectPtr conn, + virNodeDeviceObjPtr dev) +{ + char *driver_link = NULL; + char devpath[PATH_MAX]; + char *p; + int ret = -1; + int n; + + VIR_FREE(dev->def->driver); + + if (virAsprintf(&driver_link, "%s/driver", dev->devicePath) < 0) { + virReportOOMError(conn); + goto cleanup; + } + + /* Some devices don't have an explicit driver, so just return + without a name */ + if (access(driver_link, R_OK) < 0) { + ret = 0; + goto cleanup; + } + + if ((n = readlink(driver_link, devpath, sizeof devpath)) < 0) { + virReportSystemError(conn, errno, + _("cannot resolve driver link %s"), driver_link); + goto cleanup; + } + devpath[n] = '\0'; + + p = strrchr(devpath, '/'); + if (p) { + dev->def->driver = strdup(p+1); + if (!dev->def->driver) { + virReportOOMError(conn); + goto cleanup; + } + } + ret = 0; + +cleanup: + VIR_FREE(driver_link); + return ret; +} +#else +/* XXX: Implement me for non-linux */ +static int update_driver_name(virConnectPtr conn ATTRIBUTE_UNUSED, + virNodeDeviceObjPtr dev ATTRIBUTE_UNUSED) +{ + return 0; +} +#endif + void nodeDeviceLock(virDeviceMonitorStatePtr driver) { @@ -150,6 +204,7 @@ static char *nodeDeviceDumpXML(virNodeDe goto cleanup; } + update_driver_name(dev->conn, obj); ret = virNodeDeviceDefFormat(dev->conn, obj->def); cleanup: diff -r ec78a5d6c00c src/node_device_conf.c --- a/src/node_device_conf.c Thu May 28 14:42:24 2009 +0100 +++ b/src/node_device_conf.c Thu May 28 16:00:12 2009 +0100 @@ -78,6 +78,7 @@ void virNodeDeviceDefFree(virNodeDeviceD VIR_FREE(def->name); VIR_FREE(def->parent); + VIR_FREE(def->driver); caps = def->caps; while (caps) { @@ -94,6 +95,7 @@ void virNodeDeviceObjFree(virNodeDeviceO if (!dev) return; + VIR_FREE(dev->devicePath); virNodeDeviceDefFree(dev->def); if (dev->privateFree) (*dev->privateFree)(dev->privateData); @@ -191,6 +193,11 @@ char *virNodeDeviceDefFormat(virConnectP if (def->parent) virBufferEscapeString(&buf, " <parent>%s</parent>\n", def->parent); + if (def->driver) { + virBufferAddLit(&buf, " <driver>\n"); + virBufferEscapeString(&buf, " <name>%s</name>\n", def->driver); + virBufferAddLit(&buf, " </driver>\n"); + } for (caps = def->caps; caps; caps = caps->next) { char uuidstr[VIR_UUID_STRING_BUFLEN]; diff -r ec78a5d6c00c src/node_device_conf.h --- a/src/node_device_conf.h Thu May 28 14:42:24 2009 +0100 +++ b/src/node_device_conf.h Thu May 28 16:00:12 2009 +0100 @@ -136,6 +136,7 @@ typedef virNodeDeviceDef *virNodeDeviceD struct _virNodeDeviceDef { char *name; /* device name (unique on node) */ char *parent; /* optional parent device name */ + char *driver; /* optional driver name */ virNodeDevCapsDefPtr caps; /* optional device capabilities */ }; @@ -145,6 +146,7 @@ typedef virNodeDeviceObj *virNodeDeviceO struct _virNodeDeviceObj { virMutex lock; + char *devicePath; /* OS specific path to device metadat, eg sysfs */ virNodeDeviceDefPtr def; /* device definition */ void *privateData; /* driver-specific private data */ void (*privateFree)(void *data); /* destructor for private data */ diff -r ec78a5d6c00c src/node_device_hal.c --- a/src/node_device_hal.c Thu May 28 14:42:24 2009 +0100 +++ b/src/node_device_hal.c Thu May 28 16:00:12 2009 +0100 @@ -413,6 +413,7 @@ static void dev_create(const char *udi) const char *name = hal_name(udi); int rv; char *privData = strdup(udi); + char *devicePath = NULL; if (!privData) return; @@ -439,15 +440,22 @@ static void dev_create(const char *udi) if (def->caps == NULL) goto cleanup; + /* Some devices don't have a path in sysfs, so ignore failure */ + get_str_prop(ctx, udi, "linux.sysfs_path", &devicePath); + dev = virNodeDeviceAssignDef(NULL, &driverState->devs, def); - if (!dev) + if (!dev) { + VIR_FREE(devicePath); goto failure; + } dev->privateData = privData; dev->privateFree = free_udi; + dev->devicePath = devicePath; + virNodeDeviceObjUnlock(dev); nodeDeviceUnlock(driverState); -- |: 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 :|

3 3

[libvirt] Anyway to configure multiple boot devices?
by Matty 03 Jun '09

03 Jun '09

I am running a number of KVM guests on a physical machine, and want to configure the hosts to boot from the network if the internal disk isn't bootable. After reading through a number of documents, I came across the following bug report: https://bugzilla.redhat.com/show_bug.cgi?id=472236 According to that document (and the libvirt Domain XML format document), I should be able to configure two boot device statements by adding two boot tags to the domain configuration: <os> <type arch='x86_64' machine='pc'>hvm</type> <boot dev='hd'/> <boot dev='network'/> </os> When I attempt to start a guest with this configuration (I added the new line using virsh's edit command), I receive the following error: virsh # start kvmnode2 libvir: QEMU error : internal error Failure while reading monitor startup output: Input/output error error: Failed to start domain kvmnode2 Has anyone bumped into this? I checked the KVM, libvirt and fedora ML archives, but wasn't able to locate a solution to this. I am using Fedora 10 with the latest updates. Thanks for any insight, - Ryan

2 1

[libvirt] Upgrade and PCI Passthrough
by Aaron Clausen 03 Jun '09

03 Jun '09

I upgraded my Debian host to KVM 72 and libvirt 0.6.3-4 in the hopes of gaining use of PCI passthrough. I suspect that the upgrade may have gone a bit awry, because when I attempt to enable passthrough, I get the following error: /usr/bin/kvm: invalid option -- '-pcidevice' Is the version of KVM that I installed too old or is something else going on? -- Aaron Clausen mightymartianca(a)gmail.com

2 1

[libvirt] PATCH: Wire up NUMA APIs in VirtualBox driver
by Daniel P. Berrange 02 Jun '09

02 Jun '09

This patch simply wires up the shared NUMA API implementations in the virtualbox driver Daniel diff -r 8b34df5f9ddf src/vbox/vbox_tmpl.c --- a/src/vbox/vbox_tmpl.c Thu May 28 11:55:49 2009 +0100 +++ b/src/vbox/vbox_tmpl.c Thu May 28 11:56:10 2009 +0100 @@ -4955,8 +4955,8 @@ virDriver NAME(Driver) = { NULL, /* domainInterfaceStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ - NULL, /* nodeGetCellsFreeMemory */ - NULL, /* getFreeMemory */ + nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ + nodeGetFreeMemory, /* getFreeMemory */ NULL, /* domainEventRegister */ NULL, /* domainEventDeregister */ NULL, /* domainMigratePrepare2 */ -- |: 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 :|

3 2

[libvirt] PATCH: Convert VBox driver to preferred style
by Daniel P. Berrange 02 Jun '09

02 Jun '09

The driver function table in the VBox driver is currently using named struct member initializers. We previously removed all these from other drivers, so we have a clearer indication of missing APIs in each driver. This patch updates VBox to follow the preferred style Daniel diff -r f639344b66bc src/vbox/vbox_tmpl.c --- a/src/vbox/vbox_tmpl.c Thu May 28 11:36:37 2009 +0100 +++ b/src/vbox/vbox_tmpl.c Thu May 28 11:55:49 2009 +0100 @@ -4899,64 +4899,72 @@ cleanup: virDriver NAME(Driver) = { VIR_DRV_VBOX, "VBOX", - .open = vboxOpen, - .close = vboxClose, - .supports_feature = NULL, - .type = NULL, - .version = vboxGetVersion, - .getHostname = vboxGetHostname, - .getMaxVcpus = vboxGetMaxVcpus, - .nodeGetInfo = nodeGetInfo, - .getCapabilities = vboxGetCapabilities, - .listDomains = vboxListDomains, - .numOfDomains = vboxNumOfDomains, - .domainCreateXML = vboxDomainCreateXML, - .domainLookupByID = vboxDomainLookupByID, - .domainLookupByUUID = vboxDomainLookupByUUID, - .domainLookupByName = vboxDomainLookupByName, - .domainSuspend = vboxDomainSuspend, - .domainResume = vboxDomainResume, - .domainShutdown = vboxDomainShutdown, - .domainReboot = vboxDomainReboot, - .domainDestroy = vboxDomainDestroy, - .domainGetOSType = vboxDomainGetOSType, - .domainGetMaxMemory = NULL, - .domainSetMaxMemory = NULL, - .domainSetMemory = vboxDomainSetMemory, - .domainGetInfo = vboxDomainGetInfo, - .domainSave = vboxDomainSave, - .domainRestore = NULL, - .domainCoreDump = NULL, - .domainSetVcpus = NULL, - .domainPinVcpu = NULL, - .domainGetVcpus = NULL, - .domainGetMaxVcpus = NULL, - .domainDumpXML = vboxDomainDumpXML, - .listDefinedDomains = vboxListDefinedDomains, - .numOfDefinedDomains = vboxNumOfDefinedDomains, - .domainCreate = vboxDomainCreate, - .domainDefineXML = vboxDomainDefineXML, - .domainUndefine = vboxDomainUndefine, - .domainAttachDevice = vboxDomainAttachDevice, - .domainDetachDevice = vboxDomainDetachDevice, - .domainGetAutostart = NULL, - .domainSetAutostart = NULL, - .domainGetSchedulerType = NULL, - .domainGetSchedulerParameters = NULL, - .domainSetSchedulerParameters = NULL, - .domainMigratePrepare = NULL, - .domainMigratePerform = NULL, - .domainMigrateFinish = NULL, - .domainBlockStats = NULL, - .domainInterfaceStats = NULL, - .domainBlockPeek = NULL, - .domainMemoryPeek = NULL, - .nodeGetCellsFreeMemory = NULL, - .getFreeMemory = NULL, - .domainEventRegister = NULL, - .domainEventDeregister = NULL, - .domainMigratePrepare2 = NULL, - .domainMigrateFinish2 = NULL, + vboxOpen, /* open */ + vboxClose, /* close */ + NULL, /* supports_feature */ + NULL, /* type */ + vboxGetVersion, /* version */ + vboxGetHostname, /* getHostname */ + vboxGetMaxVcpus, /* getMaxVcpus */ + nodeGetInfo, /* nodeGetInfo */ + vboxGetCapabilities, /* getCapabilities */ + vboxListDomains, /* listDomains */ + vboxNumOfDomains, /* numOfDomains */ + vboxDomainCreateXML, /* domainCreateXML */ + vboxDomainLookupByID, /* domainLookupByID */ + vboxDomainLookupByUUID, /* domainLookupByUUID */ + vboxDomainLookupByName, /* domainLookupByName */ + vboxDomainSuspend, /* domainSuspend */ + vboxDomainResume, /* domainResume */ + vboxDomainShutdown, /* domainShutdown */ + vboxDomainReboot, /* domainReboot */ + vboxDomainDestroy, /* domainDestroy */ + vboxDomainGetOSType, /* domainGetOSType */ + NULL, /* domainGetMaxMemory */ + NULL, /* domainSetMaxMemory */ + vboxDomainSetMemory, /* domainSetMemory */ + vboxDomainGetInfo, /* domainGetInfo */ + vboxDomainSave, /* domainSave */ + NULL, /* domainRestore */ + NULL, /* domainCoreDump */ + NULL, /* domainSetVcpus */ + NULL, /* domainPinVcpu */ + NULL, /* domainGetVcpus */ + NULL, /* domainGetMaxVcpus */ + NULL, /* domainGetSecurityLabel */ + NULL, /* nodeGetSecurityModel */ + vboxDomainDumpXML, /* domainDumpXML */ + NULL, /* domainXmlFromNative */ + NULL, /* domainXmlToNative */ + vboxListDefinedDomains, /* listDefinedDomains */ + vboxNumOfDefinedDomains, /* numOfDefinedDomains */ + vboxDomainCreate, /* domainCreate */ + vboxDomainDefineXML, /* domainDefineXML */ + vboxDomainUndefine, /* domainUndefine */ + vboxDomainAttachDevice, /* domainAttachDevice */ + vboxDomainDetachDevice, /* domainDetachDevice */ + NULL, /* domainGetAutostart */ + NULL, /* domainSetAutostart */ + NULL, /* domainGetSchedulerType */ + NULL, /* domainGetSchedulerParameters */ + NULL, /* domainSetSchedulerParameters */ + NULL, /* domainMigratePrepare */ + NULL, /* domainMigratePerform */ + NULL, /* domainMigrateFinish */ + NULL, /* domainBlockStats */ + NULL, /* domainInterfaceStats */ + NULL, /* domainBlockPeek */ + NULL, /* domainMemoryPeek */ + NULL, /* nodeGetCellsFreeMemory */ + NULL, /* getFreeMemory */ + NULL, /* domainEventRegister */ + NULL, /* domainEventDeregister */ + NULL, /* domainMigratePrepare2 */ + NULL, /* domainMigrateFinish2 */ + NULL, /* nodeDeviceDettach */ + NULL, /* nodeDeviceReAttach */ + NULL, /* nodeDeviceReset */ + }; virNetworkDriver NAME(NetworkDriver) = { -- |: 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 :|

3 2

[libvirt] PATCH: Implement capabilities & NUMA APIs in LXC driver
by Daniel P. Berrange 02 Jun '09

02 Jun '09

The LXC driver is missing a implementation of the capabilities API, even though it has its capabilities object available in its internally struct. This patch exposes the capabilities, and also hooks up the shared NUMA APIs. Daniel diff -r 0d3150636e3e src/lxc_driver.c --- a/src/lxc_driver.c Thu May 28 11:33:16 2009 +0100 +++ b/src/lxc_driver.c Thu May 28 11:36:37 2009 +0100 @@ -47,6 +47,7 @@ #include "veth.h" #include "event.h" #include "cgroup.h" +#include "nodeinfo.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -115,6 +116,19 @@ static int lxcClose(virConnectPtr conn) return 0; } +static char *lxcGetCapabilities(virConnectPtr conn) { + lxc_driver_t *driver = conn->privateData; + char *xml; + + lxcDriverLock(driver); + if ((xml = virCapabilitiesFormatXML(driver->caps)) == NULL) + virReportOOMError(conn); + lxcDriverUnlock(driver); + + return xml; +} + + static virDomainPtr lxcDomainLookupByID(virConnectPtr conn, int id) { @@ -1429,8 +1443,8 @@ static virDriver lxcDriver = { lxcVersion, /* version */ lxcGetHostname, /* getHostname */ NULL, /* getMaxVcpus */ - NULL, /* nodeGetInfo */ - NULL, /* getCapabilities */ + nodeGetInfo, /* nodeGetInfo */ + lxcGetCapabilities, /* getCapabilities */ lxcListDomains, /* listDomains */ lxcNumDomains, /* numOfDomains */ lxcDomainCreateAndStart, /* domainCreateXML */ @@ -1478,8 +1492,8 @@ static virDriver lxcDriver = { NULL, /* domainInterfaceStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ - NULL, /* nodeGetCellsFreeMemory */ - NULL, /* getFreeMemory */ + nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ + nodeGetFreeMemory, /* getFreeMemory */ NULL, /* domainEventRegister */ NULL, /* domainEventDeregister */ NULL, /* domainMigratePrepare2 */ -- |: 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 :|

2 1

[libvirt] PATCH: Support networking in User Mode Linux driver
by Daniel P. Berrange 02 Jun '09

02 Jun '09

This patch adds support for bridge, virtual network, multicast and user-mode networking in the User Mode Linux driver. You can't pass an open TAP device file descriptor to UML, so we also extend the bridge.c file to supporting creation & deletion of persistent TAP devices. Daniel diff --git a/src/bridge.c b/src/bridge.c --- a/src/bridge.c +++ b/src/bridge.c @@ -451,8 +451,11 @@ brProbeVnetHdr(int tapfd) * * This function creates a new tap device on a bridge. @ifname can be either * a fixed name or a name template with '%d' for dynamic name allocation. - * in either case the final name for the bridge will be stored in @ifname - * and the associated file descriptor in @tapfd. + * in either case the final name for the bridge will be stored in @ifname. + * If the @tapfd parameter is supplied, the open tap device file + * descriptor will be returned, otherwise the TAP device will be made + * persistent and closed. The caller must use brDeleteTap to remove + * a persistent TAP devices when it is no longer needed. * * Returns 0 in case of success or an errno code in case of failure. */ @@ -465,7 +468,7 @@ brAddTap(brControl *ctl, { int id, subst, fd; - if (!ctl || !ctl->fd || !bridge || !ifname || !tapfd) + if (!ctl || !ctl->fd || !bridge || !ifname) return EINVAL; subst = id = 0; @@ -520,10 +523,14 @@ brAddTap(brControl *ctl, goto error; if ((errno = brSetInterfaceUp(ctl, try.ifr_name, 1))) goto error; + if (!tapfd && + (errno = ioctl(fd, TUNSETPERSIST, 1))) + goto error; VIR_FREE(*ifname); if (!(*ifname = strdup(try.ifr_name))) goto error; - *tapfd = fd; + if (tapfd) + *tapfd = fd; return 0; } @@ -535,6 +542,43 @@ brAddTap(brControl *ctl, return errno; } + +int brDeleteTap(brControl *ctl, + const char *ifname) +{ + struct ifreq try; + int len; + int fd; + + if (!ctl || !ctl->fd || !ifname) + return EINVAL; + + if ((fd = open("/dev/net/tun", O_RDWR)) < 0) + return errno; + + memset(&try, 0, sizeof(struct ifreq)); + try.ifr_flags = IFF_TAP|IFF_NO_PI; + + len = strlen(ifname); + if (len >= BR_IFNAME_MAXLEN - 1) { + errno = EINVAL; + goto error; + } + + strncpy(try.ifr_name, ifname, len); + try.ifr_name[len] = '\0'; + + if (ioctl(fd, TUNSETIFF, &try) == 0) { + if ((errno = ioctl(fd, TUNSETPERSIST, 0))) + goto error; + } + + error: + close(fd); + + return errno; +} + /** * brSetInterfaceUp: diff --git a/src/bridge.h b/src/bridge.h --- a/src/bridge.h +++ b/src/bridge.h @@ -60,11 +60,19 @@ int brDeleteInterface (brContr const char *bridge, const char *iface); +enum { + BR_TAP_VNET_HDR = (1 << 0), + BR_TAP_PERSIST = (1 << 1), +}; + int brAddTap (brControl *ctl, const char *bridge, char **ifname, - int vnet_hdr, + int features, int *tapfd); + +int brDeleteTap (brControl *ctl, + const char *ifname); int brSetInterfaceUp (brControl *ctl, const char *ifname, diff --git a/src/domain_conf.c b/src/domain_conf.c --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -3146,6 +3146,7 @@ virDomainNetDefFormat(virConnectPtr conn else virBufferVSprintf(buf, " <source port='%d'/>\n", def->data.socket.port); + break; case VIR_DOMAIN_NET_TYPE_INTERNAL: virBufferEscapeString(buf, " <source name='%s'/>\n", diff --git a/src/libvirt_bridge.syms b/src/libvirt_bridge.syms --- a/src/libvirt_bridge.syms +++ b/src/libvirt_bridge.syms @@ -8,6 +8,7 @@ brAddBridge; brAddBridge; brAddInterface; brAddTap; +brDeleteTap; brDeleteBridge; brHasBridge; brInit; diff --git a/src/uml_conf.c b/src/uml_conf.c --- a/src/uml_conf.c +++ b/src/uml_conf.c @@ -44,6 +44,7 @@ #include "memory.h" #include "nodeinfo.h" #include "verify.h" +#include "bridge.h" #define VIR_FROM_THIS VIR_FROM_UML @@ -90,6 +91,172 @@ virCapsPtr umlCapsInit(void) { return NULL; } + +static int +umlConnectTapDevice(virConnectPtr conn, + virDomainNetDefPtr net, + const char *bridge) +{ + int tapfd = -1; + int err; + brControl *brctl = NULL; + + if (!net->ifname || + STRPREFIX(net->ifname, "vnet") || + strchr(net->ifname, '%')) { + VIR_FREE(net->ifname); + if (!(net->ifname = strdup("vnet%d"))) + goto no_memory; + } + + if ((err = brInit(&brctl))) { + char ebuf[1024]; + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot initialize bridge support: %s"), + virStrerror(err, ebuf, sizeof ebuf)); + goto error; + } + + if ((err = brAddTap(brctl, bridge, + &net->ifname, BR_TAP_PERSIST, &tapfd))) { + if (errno == ENOTSUP) { + /* In this particular case, give a better diagnostic. */ + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to add tap interface to bridge. " + "%s is not a bridge device"), bridge); + } else { + char ebuf[1024]; + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to add tap interface '%s' " + "to bridge '%s' : %s"), + net->ifname, bridge, virStrerror(err, ebuf, sizeof ebuf)); + } + goto error; + } + close(tapfd); + + brShutdown(brctl); + + return 0; + +no_memory: + virReportOOMError(conn); +error: + brShutdown(brctl); + return -1; +} + +static char * +umlBuildCommandLineNet(virConnectPtr conn, + virDomainNetDefPtr def, + int idx) +{ + char *ret; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + /* General format: ethNN=type,options */ + + virBufferVSprintf(&buf, "eth%d=", idx); + + switch (def->type) { + case VIR_DOMAIN_NET_TYPE_USER: + /* ethNNN=slirp,macaddr */ + virBufferAddLit(&buf, "slirp"); + break; + + case VIR_DOMAIN_NET_TYPE_ETHERNET: + /* ethNNN=tuntap,tapname,macaddr,gateway */ + virBufferAddLit(&buf, "tuntap"); + if (def->data.ethernet.ipaddr) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("IP address not supported for ethernet inteface")); + goto error; + } + if (def->data.ethernet.script) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("script execution not supported for ethernet inteface")); + goto error; + } + break; + + case VIR_DOMAIN_NET_TYPE_SERVER: + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("TCP server networking type not supported")); + goto error; + + case VIR_DOMAIN_NET_TYPE_CLIENT: + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("TCP client networking type not supported")); + goto error; + + case VIR_DOMAIN_NET_TYPE_MCAST: + /* ethNNN=tuntap,macaddr,ipaddr,port */ + virBufferAddLit(&buf, "mcast"); + break; + + case VIR_DOMAIN_NET_TYPE_NETWORK: + { + char *bridge; + virNetworkPtr network = virNetworkLookupByName(conn, + def->data.network.name); + if (!network) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Network '%s' not found"), + def->data.network.name); + goto error; + } + bridge = virNetworkGetBridgeName(network); + virNetworkFree(network); + if (bridge == NULL) { + goto error; + } + + if (umlConnectTapDevice(conn, def, bridge) < 0) { + VIR_FREE(bridge); + goto error; + } + + /* ethNNN=tuntap,tapname,macaddr,gateway */ + virBufferVSprintf(&buf, "tuntap,%s", def->ifname); + break; + } + + case VIR_DOMAIN_NET_TYPE_BRIDGE: + if (umlConnectTapDevice(conn, def, def->data.bridge.brname) < 0) + goto error; + + /* ethNNN=tuntap,tapname,macaddr,gateway */ + virBufferVSprintf(&buf, "tuntap,%s", def->ifname); + break; + + case VIR_DOMAIN_NET_TYPE_INTERNAL: + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("internal networking type not supported")); + goto error; + } + + virBufferVSprintf(&buf, ",%02x:%02x:%02x:%02x:%02x:%02x", + def->mac[0], def->mac[1], def->mac[2], + def->mac[3], def->mac[4], def->mac[5]); + + if (def->type == VIR_DOMAIN_NET_TYPE_MCAST) { + virBufferVSprintf(&buf, ",%s,%d", + def->data.socket.address, + def->data.socket.port); + } + + if (virBufferError(&buf)) { + virReportOOMError(conn); + return NULL; + } + + return virBufferContentAndReset(&buf); + +error: + ret = virBufferContentAndReset(&buf); + VIR_FREE(ret); + return NULL; +} static char * umlBuildCommandLineChr(virConnectPtr conn, @@ -166,9 +333,8 @@ int umlBuildCommandLine(virConnectPtr co struct uml_driver *driver ATTRIBUTE_UNUSED, virDomainObjPtr vm, const char ***retargv, - const char ***retenv, - int **tapfds, - int *ntapfds) { + const char ***retenv) +{ int i, j; char memory[50]; struct utsname ut; @@ -277,6 +443,13 @@ int umlBuildCommandLine(virConnectPtr co ADD_ARG_PAIR(disk->dst, disk->src); } + for (i = 0 ; i < vm->def->nnets ; i++) { + char *ret = umlBuildCommandLineNet(conn, vm->def->nets[i], i); + if (!ret) + goto error; + ADD_ARG(ret); + } + for (i = 0 ; i < UML_MAX_CHAR_DEVICE ; i++) { char *ret; if (i == 0 && vm->def->console) @@ -311,13 +484,7 @@ int umlBuildCommandLine(virConnectPtr co no_memory: virReportOOMError(conn); error: - if (tapfds && - *tapfds) { - for (i = 0; i < *ntapfds; i++) - close((*tapfds)[i]); - VIR_FREE(*tapfds); - *ntapfds = 0; - } + if (qargv) { for (i = 0 ; i < qargc ; i++) VIR_FREE((qargv)[i]); diff --git a/src/uml_conf.h b/src/uml_conf.h --- a/src/uml_conf.h +++ b/src/uml_conf.h @@ -70,8 +70,6 @@ int umlBuildCommandLine (v struct uml_driver *driver, virDomainObjPtr dom, const char ***retargv, - const char ***retenv, - int **tapfds, - int *ntapfds); + const char ***retenv); #endif /* __UML_CONF_H */ diff --git a/src/uml_driver.c b/src/uml_driver.c --- a/src/uml_driver.c +++ b/src/uml_driver.c @@ -715,6 +721,35 @@ error: return -1; } + +static int umlCleanupTapDevices(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainObjPtr vm) { + int i; + int err; + int ret = 0; + brControl *brctl = NULL; + VIR_ERROR0("Cleanup tap"); + if (brInit(&brctl) < 0) + return -1; + + for (i = 0 ; i < vm->def->nnets ; i++) { + virDomainNetDefPtr def = vm->def->nets[i]; + + if (def->type != VIR_DOMAIN_NET_TYPE_BRIDGE && + def->type != VIR_DOMAIN_NET_TYPE_NETWORK) + continue; + + VIR_ERROR("Cleanup '%s'", def->ifname); + err = brDeleteTap(brctl, def->ifname); + if (err) { + VIR_ERROR("Cleanup failed %d", err); + ret = -1; + } + } + VIR_ERROR0("Cleanup tap done"); + brShutdown(brctl); + return ret; +} static int umlStartVMDaemon(virConnectPtr conn, struct uml_driver *driver, @@ -726,8 +761,6 @@ static int umlStartVMDaemon(virConnectPt char *logfile; int logfd = -1; struct stat sb; - int *tapfds = NULL; - int ntapfds = 0; fd_set keepfd; char ebuf[1024]; @@ -786,9 +819,9 @@ static int umlStartVMDaemon(virConnectPt } if (umlBuildCommandLine(conn, driver, vm, - &argv, &progenv, - &tapfds, &ntapfds) < 0) { + &argv, &progenv) < 0) { close(logfd); + umlCleanupTapDevices(conn, vm); return -1; } @@ -818,9 +851,6 @@ static int umlStartVMDaemon(virConnectPt vm->monitor = -1; - for (i = 0 ; i < ntapfds ; i++) - FD_SET(tapfds[i], &keepfd); - ret = virExecDaemonize(conn, argv, progenv, &keepfd, &pid, -1, &logfd, &logfd, 0, NULL, NULL, NULL); @@ -834,15 +864,14 @@ static int umlStartVMDaemon(virConnectPt VIR_FREE(progenv[i]); VIR_FREE(progenv); - if (tapfds) { - for (i = 0 ; i < ntapfds ; i++) { - close(tapfds[i]); - } - VIR_FREE(tapfds); - } + if (ret < 0) + umlCleanupTapDevices(conn, vm); /* NB we don't mark it running here - we do that async with inotify */ + /* XXX what if someone else tries to start it again + before we get the inotification ? Sounds like + trouble.... */ return ret; } @@ -874,6 +901,8 @@ static void umlShutdownVMDaemon(virConne vm->state = VIR_DOMAIN_SHUTOFF; VIR_FREE(vm->vcpupids); vm->nvcpupids = 0; + + umlCleanupTapDevices(conn, vm); if (vm->newDef) { virDomainDefFree(vm->def); -- |: 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 :|

2 1

[libvirt] PATCH: Misc user mode linux startup bugs
by Daniel P. Berrange 02 Jun '09

02 Jun '09

There's a number of annoying bugs in the user mode linux driver startup code. This patch fixes lots of them. We failed to check umlMonitorCommand for error, leading to a NULL de-reference due to another bug in that function. This patch also checks if 'res' is NULL before de-referenceing it, just in case. In the code for handling inotify delete events, we duplicated alot of the umlShutdownVMDaemon code, but forgot to close the monitor. This patches makes it just call umlShutdownVMDaemon directly. In one place where we spin in a usleep loop, we forgot to increment our retries counter, making it spin forever. Finally, switch to virKillProcess and make the latter also avoids pid==1, since I can't imagine a time where we'd ever want to kill 'init' :-) Daniel diff --git a/src/uml_driver.c b/src/uml_driver.c --- a/src/uml_driver.c +++ b/src/uml_driver.c @@ -166,9 +166,10 @@ umlIdentifyOneChrPTY(virConnectPtr conn, return -1; } requery: - umlMonitorCommand(NULL, driver, dom, cmd, &res); + if (umlMonitorCommand(NULL, driver, dom, cmd, &res) < 0) + return -1; - if (STRPREFIX(res, "pts:")) { + if (res && STRPREFIX(res, "pts:")) { VIR_FREE(def->data.file.path); if ((def->data.file.path = strdup(res + 4)) == NULL) { virReportOOMError(conn); @@ -176,7 +177,7 @@ requery: VIR_FREE(cmd); return -1; } - } else if (STRPREFIX(res, "pts")) { + } else if (!res || STRPREFIX(res, "pts")) { /* It can take a while to startup, so retry for upto 5 seconds */ /* XXX should do this in a better non-blocking @@ -263,19 +264,15 @@ reread: } if (e->mask & IN_DELETE) { + VIR_DEBUG("Got inotify domain shutdown '%s'", name); if (!virDomainIsActive(dom)) { virDomainObjUnlock(dom); continue; } - dom->def->id = -1; - dom->pid = -1; - if (dom->newDef) { - virDomainDefFree(dom->def); - dom->def = dom->newDef; - } - dom->state = VIR_DOMAIN_SHUTOFF; + umlShutdownVMDaemon(NULL, driver, dom); } else if (e->mask & (IN_CREATE | IN_MODIFY)) { + VIR_DEBUG("Got inotify domain startup '%s'", name); if (virDomainIsActive(dom)) { virDomainObjUnlock(dom); continue; @@ -289,11 +286,13 @@ reread: dom->def->id = driver->nextvmid++; dom->state = VIR_DOMAIN_RUNNING; - if (umlOpenMonitor(NULL, driver, dom) < 0) + if (umlOpenMonitor(NULL, driver, dom) < 0) { + VIR_WARN0("Could not open monitor for new domain"); umlShutdownVMDaemon(NULL, driver, dom); - - if (umlIdentifyChrPTY(NULL, driver, dom) < 0) + } else if (umlIdentifyChrPTY(NULL, driver, dom) < 0) { + VIR_WARN0("Could not identify charater devices for new domain"); umlShutdownVMDaemon(NULL, driver, dom); + } } virDomainObjUnlock(dom); } @@ -383,6 +382,7 @@ umlStartup(void) { goto error; } + VIR_INFO("Adding inotify watch on %s", uml_driver->monitorDir); if (inotify_add_watch(uml_driver->inotifyFD, uml_driver->monitorDir, IN_CREATE | IN_MODIFY | IN_DELETE) < 0) { @@ -595,10 +595,11 @@ static int umlOpenMonitor(virConnectPtr if (umlMonitorAddress(conn, driver, vm, &addr) < 0) return -1; + VIR_DEBUG("Dest address for monitor is '%s'", addr.sun_path); restat: if (stat(addr.sun_path, &sb) < 0) { if (errno == ENOENT && - retries < 50) { + retries++ < 50) { usleep(1000 * 100); goto restat; } @@ -612,7 +613,8 @@ restat: } memset(addr.sun_path, 0, sizeof addr.sun_path); - sprintf(addr.sun_path + 1, "%u", getpid()); + sprintf(addr.sun_path + 1, "libvirt-uml-%u", vm->pid); + VIR_DEBUG("Reply address for monitor is '%s'", addr.sun_path+1); if (bind(vm->monitor, (struct sockaddr *)&addr, sizeof addr) < 0) { virReportSystemError(conn, errno, "%s", _("cannot bind socket")); @@ -656,6 +658,8 @@ static int umlMonitorCommand(virConnectP int retlen = 0, ret = 0; struct sockaddr_un addr; unsigned int addrlen; + + VIR_DEBUG("Run command '%s'", cmd); *reply = NULL; @@ -706,6 +710,8 @@ static int umlMonitorCommand(virConnectP } while (res.extra); + VIR_DEBUG("Command reply is '%s'", NULLSTR(retdata)); + *reply = retdata; return ret; @@ -852,12 +881,10 @@ static void umlShutdownVMDaemon(virConne virDomainObjPtr vm) { int ret; - if (!virDomainIsActive(vm) || - vm->pid <= 1) + if (!virDomainIsActive(vm)) return; - - kill(vm->pid, SIGTERM); + virKillProcess(vm->pid, SIGTERM); if (vm->monitor != -1) close(vm->monitor); diff --git a/src/util.c b/src/util.c --- a/src/util.c +++ b/src/util.c @@ -1680,7 +1680,7 @@ char *virGetHostname(void) /* send signal to a single process */ int virKillProcess(pid_t pid, int sig) { - if (pid < 1) { + if (pid <= 1) { errno = ESRCH; return -1; } -- |: 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 :|

2 1

[libvirt] PATCH: Fix libvirt log initialization to not override environment
by Daniel P. Berrange 02 Jun '09

02 Jun '09

Witht he libvirtd daemon, if you start it up with LIBVIRT_LOG_OUTPUTS or LIBVIRT_LOG_FILTERS set, then it'll initially use these settings for logging, but then when it reads the config file it'll reset all logging and ignore your environment variables. This is rather confusing ! This quick fix makes it honour this env variables, if the config file has not got explicit overrides. Daniel diff --git a/qemud/qemud.c b/qemud/qemud.c --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -2482,6 +2482,11 @@ qemudSetLogging(virConfPtr conf, const c /* there is no default filters */ GET_CONF_STR (conf, filename, log_filters); + if (!log_filters) { + debugEnv = getenv("LIBVIRT_LOG_FILTERS"); + if (debugEnv) + log_filters = strdup(debugEnv); + } virLogParseFilters(log_filters); /* @@ -2489,7 +2494,12 @@ qemudSetLogging(virConfPtr conf, const c * all logs to stderr if not running as daemon */ GET_CONF_STR (conf, filename, log_outputs); - if (log_outputs == NULL) { + if (!log_outputs) { + debugEnv = getenv("LIBVIRT_LOG_OUTPUTS"); + if (debugEnv) + log_outputs = strdup(debugEnv); + } + if (!log_outputs) { if (godaemon) { char *tmp = NULL; if (virAsprintf (&tmp, "%d:syslog:libvirtd", log_level) < 0) @@ -2499,8 +2509,9 @@ qemudSetLogging(virConfPtr conf, const c } else { virLogParseOutputs("0:stderr:libvirtd"); } - } else + } else { virLogParseOutputs(log_outputs); + } ret = 0; free_and_fail: -- |: 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 :|

2 1