This patch allows passing of a "-mem-path <arg>" flag to qemu for support of huge page backed guests. A guest may request this option via specifying: <hugepage>on</hugepage> in its domain definition xml file. The request for huge page backing will be attempted within libvirt if the host system has indicated a hugetlbfs mount point in qemu.conf, for example: hugepage_mount = "/hugetlbfs" _and_ the target qemu executable is aware of the "-mem-path" flag. Otherwise this request by a guest will result in an error. This patch does not address setup of the required host hugetlbfs mount point, verifying the mount point is correct/usable, nor assure sufficient free huge pages are available; which are assumed to be addressed by other means. Signed-off-by: john cooper <john.cooper(a)redhat.com&gt; --- diff --git a/src/domain_conf.c b/src/domain_conf.c index f3e4c6c..04d6911 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -2369,6 +2369,17 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, if (virXPathULong(conn, "string(./currentMemory[1])", ctxt, &def->memory) < 0) def->memory = def->maxmem; + tmp = virXPathString(conn, "string(./hugepage[1])", ctxt); + if (!tmp || STREQ(tmp, "off")) + def->hugepage_backed = 0; + else if (STREQ(tmp, "on")) + def->hugepage_backed = 1; + else { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("invalid hugepage mode %s"), tmp); + goto error; + } + if (virXPathULong(conn, "string(./vcpu[1])", ctxt, &def->vcpus) < 0) def->vcpus = 1; @@ -3933,6 +3944,8 @@ char *virDomainDefFormat(virConnectPtr conn, virBufferVSprintf(&buf, " <memory>%lu</memory>\n", def->maxmem); virBufferVSprintf(&buf, " <currentMemory>%lu</currentMemory>\n", def->memory); + if (def->hugepage_backed) + virBufferVSprintf(&buf, " <hugepage>%s</hugepage>\n", "on"); for (n = 0 ; n < def->cpumasklen ; n++) if (def->cpumask[n] != 1) diff --git a/src/domain_conf.h b/src/domain_conf.h index 6e111fa..d6bdcdb 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -481,6 +481,7 @@ struct _virDomainDef { unsigned long memory; unsigned long maxmem; + unsigned char hugepage_backed; unsigned long vcpus; int cpumasklen; char *cpumask; diff --git a/src/qemu.conf b/src/qemu.conf index 3009725..a3387f1 100644 --- a/src/qemu.conf +++ b/src/qemu.conf @@ -95,3 +95,10 @@ # The group ID for QEMU processes run by the system instance #group = "root" + +# If provided by the host and this hugetlbfs mount point is +# configured, a guest may request huge page backing. When this +# mount point is undefined, huge page backing is disabled. + +hugepage_mount = "/hugetlbfs" + diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 4043d70..632b784 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -218,6 +218,17 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } VIR_FREE(group); + p = virConfGetValue (conf, "hugepage_mount"); + CHECK_TYPE ("hugepage_mount", VIR_CONF_STRING); + if (p && p->str) { + VIR_FREE(driver->hugepage_mount); + if (!(driver->hugepage_mount = strdup(p->str))) { + virReportOOMError(NULL); + virConfFree(conf); + return -1; + } + } + virConfFree (conf); return 0; } @@ -500,6 +511,8 @@ static unsigned int qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_VGA; if (strstr(help, "boot=on")) flags |= QEMUD_CMD_FLAG_DRIVE_BOOT; + if (strstr(help, "-mem-path")) + flags |= QEMUD_CMD_FLAG_MEM_PATH; if (version >= 9000) flags |= QEMUD_CMD_FLAG_VNC_COLON; @@ -1125,6 +1138,15 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT("-no-kvm"); ADD_ARG_LIT("-m"); ADD_ARG_LIT(memory); + if (def->hugepage_backed) { + if (!driver->hugepage_mount || !(qemuCmdFlags & QEMUD_CMD_FLAG_MEM_PATH)) { + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, + "%s", _("hugepage backing not supported")); + goto error; + } + ADD_ARG_LIT("-mem-path"); + ADD_ARG_LIT(driver->hugepage_mount); + } ADD_ARG_LIT("-smp"); ADD_ARG_LIT(vcpus); diff --git a/src/qemu_conf.h b/src/qemu_conf.h index fbf2ab9..847597f 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -58,6 +58,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_KVM = (1 << 13), /* Whether KVM is compiled in */ QEMUD_CMD_FLAG_DRIVE_FORMAT = (1 << 14), /* Is -drive format= avail */ QEMUD_CMD_FLAG_VGA = (1 << 15), /* Is -vga avail */ + QEMUD_CMD_FLAG_MEM_PATH = (1 << 16), /* mmap'ped guest backing supported */ }; /* Main driver state */ @@ -86,6 +87,7 @@ struct qemud_driver { char *vncListen; char *vncPassword; char *vncSASLdir; + char *hugepage_mount; virCapsPtr caps; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 00dc6e5..bdecf5a 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -638,6 +638,7 @@ qemudShutdown(void) { VIR_FREE(qemu_driver->vncListen); VIR_FREE(qemu_driver->vncPassword); VIR_FREE(qemu_driver->vncSASLdir); + VIR_FREE(qemu_driver->hugepage_mount); /* Free domain callback list */ virDomainEventCallbackListFree(qemu_driver->domainEventCallbacks); -- john.cooper(a)redhat.com

I've incorporated feedback received on the prior version into the patch below. The host mount point for hugetlbfs is queried by default from /proc/mounts unless overridden in qemu.conf via: hugepage_mount = "<path_to_mount_point>" This should make the concern of establishing a mount point path convention a non-issue for the general case while still allowing the same to be deterministically set if needed. Complete disable of hugepage backing can be accomplished by setting the mount point to "". A guest now requests hugepage backing via specifying: <hugepage enable='on'/> in its domain definition xml file. A 'hugepage' element along with an enable attribute is added to the definition, but the element content is open for future use. As before if a guest requests hugepage backing and either the host mount point can't be determined, or the target qemu doesn't support -mem-path, an error is flagged. Signed-off-by: john cooper <john.cooper(a)redhat.com&gt; --- diff --git a/src/domain_conf.c b/src/domain_conf.c index f3e4c6c..ac328ea 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -2369,6 +2369,18 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, if (virXPathULong(conn, "string(./currentMemory[1])", ctxt, &def->memory) < 0) def->memory = def->maxmem; + tmp = virXPathString(conn, "string(./hugepage/@enable)", ctxt); + if (!tmp || STREQ(tmp, "off")) + def->hugepage_backed = 0; + else if (STREQ(tmp, "on")) + def->hugepage_backed = 1; + else { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("invalid hugepage mode %s"), tmp); + goto error; + } + VIR_FREE(tmp); + if (virXPathULong(conn, "string(./vcpu[1])", ctxt, &def->vcpus) < 0) def->vcpus = 1; @@ -3933,6 +3945,8 @@ char *virDomainDefFormat(virConnectPtr conn, virBufferVSprintf(&buf, " <memory>%lu</memory>\n", def->maxmem); virBufferVSprintf(&buf, " <currentMemory>%lu</currentMemory>\n", def->memory); + virBufferVSprintf(&buf, " <hugepage enable='%s'/>\n", + def->hugepage_backed ? "on" : "off"); for (n = 0 ; n < def->cpumasklen ; n++) if (def->cpumask[n] != 1) diff --git a/src/domain_conf.h b/src/domain_conf.h index 6e111fa..d6bdcdb 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -481,6 +481,7 @@ struct _virDomainDef { unsigned long memory; unsigned long maxmem; + unsigned char hugepage_backed; unsigned long vcpus; int cpumasklen; char *cpumask; diff --git a/src/qemu.conf b/src/qemu.conf index 3009725..9ecac23 100644 --- a/src/qemu.conf +++ b/src/qemu.conf @@ -95,3 +95,13 @@ # The group ID for QEMU processes run by the system instance #group = "root" + +# If provided by the host and a hugetlbfs mount point is configured, +# a guest may request huge page backing. When this mount point is +# unspecified here, determination of a host mount point in /proc/mounts +# will be attempted. Specifying an explicit mount overrides detection +# of the same in /proc/mounts. Setting the mount point to "" will +# disable guest hugepage backing. + +# hugepage_mount = "<path_to_mount_point>" + diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 4043d70..5a3d70c 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -35,6 +35,7 @@ #include <sys/wait.h> #include <arpa/inet.h> #include <sys/utsname.h> +#include <mntent.h> #include "c-ctype.h" #include "virterror_internal.h" @@ -50,6 +51,8 @@ #include "nodeinfo.h" #include "logging.h" +unsigned char *find_mnt(unsigned char *type); + #define VIR_FROM_THIS VIR_FROM_QEMU VIR_ENUM_DECL(virDomainDiskQEMUBus) @@ -87,6 +90,7 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, NULL, /* no arg needed for xen */ NULL /* don't support vbox */); +#define PROC_MOUNT_BUF_LEN 255 int qemudLoadDriverConfig(struct qemud_driver *driver, const char *filename) { @@ -218,10 +222,41 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } VIR_FREE(group); + p = virConfGetValue (conf, "hugepage_mount"); + CHECK_TYPE ("hugepage_mount", VIR_CONF_STRING); + if (!p) + driver->hugepage_mount = find_mnt("hugetlbfs"); + else if (!p->str) + driver->hugepage_mount = NULL; + else { + VIR_FREE(driver->hugepage_mount); + if (!(driver->hugepage_mount = strdup(p->str))) { + virReportOOMError(NULL); + virConfFree(conf); + return -1; + } + } + virConfFree (conf); return 0; } +/* search /proc/mounts for mount point of *type; return pointer to + * malloc'ed string of the path if found, otherwise return NULL + */ +unsigned char *find_mnt(unsigned char *type) +{ + FILE *f = setmntent("/proc/mounts", "r"); + unsigned char buf[PROC_MOUNT_BUF_LEN]; + struct mntent mb; + + if (f) + while (getmntent_r(f, &mb, buf, PROC_MOUNT_BUF_LEN)) + if (STREQ(mb.mnt_type, type)) + return (strdup(mb.mnt_dir)); + return (NULL); +} + /* The list of possible machine types for various architectures, as supported by QEMU - taken from 'qemu -M ?' for each arch */ static const char *const arch_info_hvm_x86_machines[] = { @@ -500,6 +535,8 @@ static unsigned int qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_VGA; if (strstr(help, "boot=on")) flags |= QEMUD_CMD_FLAG_DRIVE_BOOT; + if (strstr(help, "-mem-path")) + flags |= QEMUD_CMD_FLAG_MEM_PATH; if (version >= 9000) flags |= QEMUD_CMD_FLAG_VNC_COLON; @@ -1125,6 +1162,15 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT("-no-kvm"); ADD_ARG_LIT("-m"); ADD_ARG_LIT(memory); + if (def->hugepage_backed) { + if (!driver->hugepage_mount || !(qemuCmdFlags & QEMUD_CMD_FLAG_MEM_PATH)) { + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, + "%s", _("hugepage backing not supported")); + goto error; + } + ADD_ARG_LIT("-mem-path"); + ADD_ARG_LIT(driver->hugepage_mount); + } ADD_ARG_LIT("-smp"); ADD_ARG_LIT(vcpus); diff --git a/src/qemu_conf.h b/src/qemu_conf.h index fbf2ab9..847597f 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -58,6 +58,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_KVM = (1 << 13), /* Whether KVM is compiled in */ QEMUD_CMD_FLAG_DRIVE_FORMAT = (1 << 14), /* Is -drive format= avail */ QEMUD_CMD_FLAG_VGA = (1 << 15), /* Is -vga avail */ + QEMUD_CMD_FLAG_MEM_PATH = (1 << 16), /* mmap'ped guest backing supported */ }; /* Main driver state */ @@ -86,6 +87,7 @@ struct qemud_driver { char *vncListen; char *vncPassword; char *vncSASLdir; + char *hugepage_mount; virCapsPtr caps; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 00dc6e5..bdecf5a 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -638,6 +638,7 @@ qemudShutdown(void) { VIR_FREE(qemu_driver->vncListen); VIR_FREE(qemu_driver->vncPassword); VIR_FREE(qemu_driver->vncSASLdir); + VIR_FREE(qemu_driver->hugepage_mount); /* Free domain callback list */ virDomainEventCallbackListFree(qemu_driver->domainEventCallbacks); -- john.cooper(a)redhat.com

With hotplug, we're going to want to pass a tapfd name rather than an actual file descriptor, so prepare the way by passing a string tapfd to qemuBuildHostNetStr(). * src/qemu_conf.h: qemuBuildHostNetStr() takes a string tapfd now * src/qemu_conf.c: pass qemuBuildHostNetStr() a string rather than an actual file descriptor * src/qemu_driver.c: update qemudDomainAttachNetDevice() for change --- src/qemu_conf.c | 17 ++++++++++++----- src/qemu_conf.h | 2 +- src/qemu_driver.c | 2 +- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index ed77153..e875435 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -910,13 +910,13 @@ qemuBuildHostNetStr(virConnectPtr conn, const char *prefix, char type_sep, int vlan, - int tapfd, + const char *tapfd, char **str) { switch (net->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: - if (virAsprintf(str, "%stap%cfd=%d,vlan=%d%s%s", + if (virAsprintf(str, "%stap%cfd=%s,vlan=%d%s%s", prefix ? prefix : "", type_sep, tapfd, vlan, (net->hostnet_name ? ",name=" : ""), @@ -1534,7 +1534,7 @@ int qemudBuildCommandLine(virConnectPtr conn, for (i = 0 ; i < def->nnets ; i++) { virDomainNetDefPtr net = def->nets[i]; char *nic, *host; - int tapfd = -1; + char *tapfd_name = NULL; net->vlan = i; @@ -1550,7 +1550,7 @@ int qemudBuildCommandLine(virConnectPtr conn, if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK || net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { - tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags); + int tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags); if (tapfd < 0) goto error; @@ -1560,14 +1560,21 @@ int qemudBuildCommandLine(virConnectPtr conn, } (*tapfds)[(*ntapfds)++] = tapfd; + + if (virAsprintf(&tapfd_name, "%d", tapfd) < 0) + goto no_memory; } if (qemuBuildHostNetStr(conn, net, NULL, ',', - net->vlan, tapfd, &host) < 0) + net->vlan, tapfd_name, &host) < 0) { + VIR_FREE(tapfd_name); goto error; + } ADD_ARG_LIT("-net"); ADD_ARG(host); + + VIR_FREE(tapfd_name); } } diff --git a/src/qemu_conf.h b/src/qemu_conf.h index 8ad5f94..8150a54 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -150,7 +150,7 @@ int qemuBuildHostNetStr (virConnectPtr conn, const char *prefix, char type_sep, int vlan, - int tapfd, + const char *tapfd, char **str); int qemuBuildNicStr (virConnectPtr conn, diff --git a/src/qemu_driver.c b/src/qemu_driver.c index c4683ae..7f4e2a1 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -4587,7 +4587,7 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, net->vlan = vm->def->nets[i]->vlan; if (qemuBuildHostNetStr(conn, net, - "host_net_add ", ' ', net->vlan, -1, &cmd) < 0) + "host_net_add ", ' ', net->vlan, NULL, &cmd) < 0) return -1; remove_cmd = NULL; -- 1.6.2.5

In subsequent patches we're going to have a file descriptor to close too, so centralize the error handling cleanups to make things easier. * src/qemu_conf.c: in qemudDomainAttachNetDevice() consolidate the error handling cleanups together --- src/qemu_driver.c | 37 +++++++++++++++---------------------- 1 files changed, 15 insertions(+), 22 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 7f4e2a1..4f0c60e 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -4549,7 +4549,7 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, unsigned int qemuCmdFlags) { virDomainNetDefPtr net = dev->data.net; - char *cmd, *reply, *remove_cmd; + char *cmd = NULL, *reply = NULL, *remove_cmd = NULL; int i; unsigned domain, bus, slot; @@ -4567,16 +4567,12 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, return -1; } - if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0) { - virReportOOMError(conn); - return -1; - } + if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0) + goto no_memory; if ((qemuCmdFlags & QEMUD_CMD_FLAG_NET_NAME) && - qemuAssignNetNames(vm->def, net) < 0) { - virReportOOMError(conn); - return -1; - } + qemuAssignNetNames(vm->def, net) < 0) + goto no_memory; /* Choose a vlan value greater than all other values since * older versions did not store the value in the state file. @@ -4588,23 +4584,19 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, if (qemuBuildHostNetStr(conn, net, "host_net_add ", ' ', net->vlan, NULL, &cmd) < 0) - return -1; + goto cleanup; remove_cmd = NULL; if (net->vlan >= 0 && net->hostnet_name && virAsprintf(&remove_cmd, "host_net_remove %d %s", net->vlan, net->hostnet_name) < 0) { - VIR_FREE(cmd); - virReportOOMError(conn); - return -1; + goto no_memory; } if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED, _("failed to add network backend with '%s'"), cmd); - VIR_FREE(remove_cmd); - VIR_FREE(cmd); - return -1; + goto cleanup; } DEBUG("%s: host_net_add reply: %s", vm->def->name, reply); @@ -4619,19 +4611,16 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED, _("failed to add NIC with '%s'"), cmd); - VIR_FREE(cmd); goto try_remove; } - VIR_FREE(cmd); - if (qemudParsePciAddReply(vm, reply, &domain, &bus, &slot) < 0) { qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED, _("parsing pci_add reply failed: %s"), reply); - VIR_FREE(reply); goto try_remove; } + VIR_FREE(cmd); VIR_FREE(reply); VIR_FREE(remove_cmd); @@ -4644,7 +4633,7 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, return 0; try_remove: - reply = NULL; + VIR_FREE(reply); if (!remove_cmd) VIR_WARN0(_("Unable to remove network backend\n")); @@ -4652,10 +4641,14 @@ try_remove: VIR_WARN(_("Failed to remove network backend with '%s'\n"), remove_cmd); else VIR_DEBUG("%s: host_net_remove reply: %s\n", vm->def->name, reply); + goto cleanup; +no_memory: + virReportOOMError(conn); +cleanup: + VIR_FREE(cmd); VIR_FREE(reply); VIR_FREE(remove_cmd); - return -1; } -- 1.6.2.5

Add a little helper function to write the monitor command followed by carriage return in a single write. This doesn't make any real difference, but allows us to more easily switch to using sendmsg() when using the monitor over a unix socket. * src/qemu_conf.c: split qemudMonitorSend() out --- src/qemu_driver.c | 31 +++++++++++++++++++++++-------- 1 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 4f0c60e..7d4de9e 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2012,6 +2012,27 @@ qemuMonitorDiscardPendingData(virDomainObjPtr vm) { } while (ret > 0); } +static int +qemudMonitorSend(const virDomainObjPtr vm, + const char *cmd) +{ + char *full; + size_t len; + int ret = -1; + + if (virAsprintf(&full, "%s\r", cmd) < 0) + return -1; + + len = strlen(full); + + if (safewrite(vm->monitor, full, len) != len) + goto out; + + ret = 0; +out: + VIR_FREE(full); + return ret; +} static int qemudMonitorCommandExtra(const virDomainObjPtr vm, @@ -2021,14 +2042,10 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm, char **reply) { int size = 0; char *buf = NULL; - size_t cmdlen = strlen(cmd); - size_t extralen = extra ? strlen(extra) : 0; qemuMonitorDiscardPendingData(vm); - if (safewrite(vm->monitor, cmd, cmdlen) != cmdlen) - return -1; - if (safewrite(vm->monitor, "\r", 1) != 1) + if (qemudMonitorSend(vm, cmd) < 0) return -1; *reply = NULL; @@ -2063,9 +2080,7 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm, if (buf) { if (extra) { if (strstr(buf, extraPrompt) != NULL) { - if (safewrite(vm->monitor, extra, extralen) != extralen) - return -1; - if (safewrite(vm->monitor, "\r", 1) != 1) + if (qemudMonitorSend(vm, extra) < 0) return -1; extra = NULL; } -- 1.6.2.5

Switch from using write() to using sendmsg() on QEMU's monitor socket so that we can add support for SCM_RIGHTS. * src/qemu_driver.c: add sendmsg() based qemudMonitorSendUnix() and use it when the monitor fd is a unix socket --- src/qemu_driver.c | 37 +++++++++++++++++++++++++++++++++++-- 1 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 7d4de9e..e6e6786 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2013,6 +2013,30 @@ qemuMonitorDiscardPendingData(virDomainObjPtr vm) { } static int +qemudMonitorSendUnix(const virDomainObjPtr vm, + const char *cmd, + size_t cmdlen) +{ + struct msghdr msg; + struct iovec iov[1]; + ssize_t ret; + + memset(&msg, 0, sizeof(msg)); + + iov[0].iov_base = (void *)cmd; + iov[0].iov_len = cmdlen; + + msg.msg_iov = iov; + msg.msg_iovlen = 1; + + do { + ret = sendmsg(vm->monitor, &msg, 0); + } while (ret < 0 && errno == EINTR); + + return ret == cmdlen ? 0 : -1; +} + +static int qemudMonitorSend(const virDomainObjPtr vm, const char *cmd) { @@ -2025,8 +2049,17 @@ qemudMonitorSend(const virDomainObjPtr vm, len = strlen(full); - if (safewrite(vm->monitor, full, len) != len) - goto out; + switch (vm->monitor_chr->type) { + case VIR_DOMAIN_CHR_TYPE_UNIX: + if (qemudMonitorSendUnix(vm, full, len) < 0) + goto out; + break; + default: + case VIR_DOMAIN_CHR_TYPE_PTY: + if (safewrite(vm->monitor, full, len) != len) + goto out; + break; + } ret = 0; out: -- 1.6.2.5

On Wed, Jul 22, 2009 at 10:57:34PM +0100, Mark McLoughlin wrote: Oooooh, of course this is why you added the allocation instead of doing two safewrite() calls. In which case, ACK to this & previous patch 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 :|

In order to hotplug a network/bridge backed NIC, we need to first create the tap file descriptor, add the tap interface to the bridge and then pass the file descriptor to the qemu process using the 'getfd' monitor command. Once the tapfd has been accepted, we create the network backend using host_net_add, supplying the name assigned to the tapfd. If this fails, we need to close the tapfd in qemu using the 'closefd' monitor command. * src/qemu_driver.c: add support for tapfd based hotplug in qemudDomainAttachNetDevice() --- src/qemu_driver.c | 73 +++++++++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index a7861be..23cbd8f 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -4620,13 +4620,15 @@ static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn, } static int qemudDomainAttachNetDevice(virConnectPtr conn, + struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev, unsigned int qemuCmdFlags) { virDomainNetDefPtr net = dev->data.net; char *cmd = NULL, *reply = NULL, *remove_cmd = NULL; - int i; + char *tapfd_name = NULL, *tapfd_close = NULL; + int i, tapfd = -1; unsigned domain, bus, slot; if (!(qemuCmdFlags & QEMUD_CMD_FLAG_HOST_NET_ADD)) { @@ -4637,10 +4639,16 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE || net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { - qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT, - _("network device type '%s' cannot be attached"), - virDomainNetTypeToString(net->type)); - return -1; + if (vm->monitor_chr->type != VIR_DOMAIN_CHR_TYPE_UNIX) { + qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT, + _("network device type '%s' cannot be attached: " + "qemu is not using a unix socket monitor"), + virDomainNetTypeToString(net->type)); + return -1; + } + + if ((tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags)) < 0) + return -1; } if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0) @@ -4658,27 +4666,55 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, if (vm->def->nets[i]->vlan >= net->vlan) net->vlan = vm->def->nets[i]->vlan; - if (qemuBuildHostNetStr(conn, net, - "host_net_add ", ' ', net->vlan, NULL, &cmd) < 0) - goto cleanup; + if (tapfd != -1) { + if (virAsprintf(&tapfd_name, "fd-%s", net->hostnet_name) < 0) + goto no_memory; + + if (virAsprintf(&tapfd_close, "closefd %s", tapfd_name) < 0) + goto no_memory; + + if (virAsprintf(&cmd, "getfd %s", tapfd_name) < 0) + goto no_memory; + + if (qemudMonitorCommandWithFd(vm, cmd, tapfd, &reply) < 0) { + qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + _("failed to pass fd to qemu with '%s'"), cmd); + goto cleanup; + } + + DEBUG("%s: getfd reply: %s", vm->def->name, reply); + + VIR_FREE(reply); + VIR_FREE(cmd); + } + + if (qemuBuildHostNetStr(conn, net, "host_net_add ", ' ', + net->vlan, tapfd_name, &cmd) < 0) + goto try_tapfd_close; remove_cmd = NULL; if (net->vlan >= 0 && net->hostnet_name && virAsprintf(&remove_cmd, "host_net_remove %d %s", net->vlan, net->hostnet_name) < 0) { - goto no_memory; + virReportOOMError(conn); + goto try_tapfd_close; } if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED, _("failed to add network backend with '%s'"), cmd); - goto cleanup; + goto try_tapfd_close; } DEBUG("%s: host_net_add reply: %s", vm->def->name, reply); VIR_FREE(reply); VIR_FREE(cmd); + VIR_FREE(tapfd_name); + VIR_FREE(tapfd_close); + if (tapfd != -1) + close(tapfd); + tapfd = -1; if (qemuBuildNicStr(conn, net, "pci_add pci_addr=auto ", ' ', net->vlan, &cmd) < 0) @@ -4719,12 +4755,27 @@ try_remove: VIR_DEBUG("%s: host_net_remove reply: %s\n", vm->def->name, reply); goto cleanup; +try_tapfd_close: + VIR_FREE(reply); + + if (tapfd_close) { + if (qemudMonitorCommand(vm, tapfd_close, &reply) < 0) + VIR_WARN(_("Failed to close tapfd with '%s'\n"), tapfd_close); + else + VIR_DEBUG("%s: closefd: %s\n", vm->def->name, reply); + } + goto cleanup; + no_memory: virReportOOMError(conn); cleanup: VIR_FREE(cmd); VIR_FREE(reply); VIR_FREE(remove_cmd); + VIR_FREE(tapfd_close); + VIR_FREE(tapfd_name); + if (tapfd != -1) + close(tapfd); return -1; } @@ -4854,7 +4905,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto cleanup; } } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { - ret = qemudDomainAttachNetDevice(dom->conn, vm, dev, qemuCmdFlags); + ret = qemudDomainAttachNetDevice(dom->conn, driver, vm, dev, qemuCmdFlags); } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { -- 1.6.2.5

On Mon, 2009-07-27 at 11:06 +0200, Daniel Veillard wrote: Yep, the below hunk is all we need - I'll squash it into 7/7 Thanks, Mark. diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 23cbd8f..d65a2a3 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -4684,6 +4684,16 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, DEBUG("%s: getfd reply: %s", vm->def->name, reply); + /* If the command isn't supported then qemu prints: + * unknown command: getfd" */ + if (strstr(reply, "unknown command:")) { + qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT, + "%s", + _("bridge/network interface attach not supported: " + "qemu 'getfd' monitor command available")); + goto cleanup; + } + VIR_FREE(reply); VIR_FREE(cmd); }