[libvirt] [PATCH 0/7] Allow multiple console devices for each guest
The current XML schema only allows for a single <console> element per guest. Many hypervisors support multiple paravirt consoles, and it'd be desirable to support that. This series does just that, enabling multiple consoles for UML, QEMU and LXC
From: "Daniel P. Berrange" <berrange@redhat.com> While Xen only has a single paravirt console, UML, and QEMU both support multiple paravirt consoles. The LXC driver can also be trivially made to support multiple consoles. This patch extends the XML to allow multiple <console> elements in the XML. It also makes the UML and QEMU drivers support this config. * src/conf/domain_conf.c, src/conf/domain_conf.h: Allow multiple <console> devices * src/lxc/lxc_driver.c, src/xen/xen_driver.c, src/xenxs/xen_sxpr.c, src/xenxs/xen_xm.c: Update for internal API changes * src/security/security_selinux.c, src/security/virt-aa-helper.c: Only label consoles that aren't a copy of the serial device * src/qemu/qemu_command.c, src/qemu/qemu_driver.c, src/qemu/qemu_process.c, src/uml/uml_conf.c, src/uml/uml_driver.c: Support multiple console devices * tests/qemuxml2xmltest.c, tests/qemuxml2argvtest.c: Extra tests for multiple virtio consoles. Set QEMU_CAPS_CHARDEV for all console /channel tests * tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-auto.args, tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args tests/qemuxml2argvdata/qemuxml2argv-console-virtio.args: Update for correct chardev syntax * tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many.args, tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many.xml: New test file --- src/conf/domain_conf.c | 110 ++++++++++++++------ src/conf/domain_conf.h | 4 +- src/lxc/lxc_driver.c | 20 +++- src/qemu/qemu_command.c | 12 +- src/qemu/qemu_driver.c | 11 +- src/qemu/qemu_process.c | 8 +- src/security/security_selinux.c | 10 ++ src/security/virt-aa-helper.c | 16 ++- src/uml/uml_conf.c | 8 +- src/uml/uml_driver.c | 10 +- src/xen/xen_driver.c | 4 +- src/xenxs/xen_sxpr.c | 17 ++- src/xenxs/xen_xm.c | 11 ++- .../qemuxml2argv-channel-virtio-auto.args | 5 +- .../qemuxml2argv-channel-virtio.args | 5 +- .../qemuxml2argv-console-virtio-many.args | 12 ++ .../qemuxml2argv-console-virtio-many.xml | 41 +++++++ .../qemuxml2argv-console-virtio.args | 5 +- tests/qemuxml2argvtest.c | 8 +- tests/qemuxml2xmltest.c | 1 + 20 files changed, 233 insertions(+), 85 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5959593..f90bece 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1208,7 +1208,9 @@ void virDomainDefFree(virDomainDefPtr def) virDomainChrDefFree(def->channels[i]); VIR_FREE(def->channels); - virDomainChrDefFree(def->console); + for (i = 0 ; i < def->nconsoles ; i++) + virDomainChrDefFree(def->consoles[i]); + VIR_FREE(def->consoles); for (i = 0 ; i < def->nsounds ; i++) virDomainSoundDefFree(def->sounds[i]); @@ -1599,6 +1601,9 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, for (i = 0; i < def->nchannels ; i++) if (cb(def, &def->channels[i]->info, opaque) < 0) return -1; + for (i = 0; i < def->nconsoles ; i++) + if (cb(def, &def->consoles[i]->info, opaque) < 0) + return -1; for (i = 0; i < def->ninputs ; i++) if (cb(def, &def->inputs[i]->info, opaque) < 0) return -1; @@ -1611,9 +1616,6 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, if (def->memballoon) if (cb(def, &def->memballoon->info, opaque) < 0) return -1; - if (def->console) - if (cb(def, &def->console->info, opaque) < 0) - return -1; for (i = 0; i < def->nhubs ; i++) if (cb(def, &def->hubs[i]->info, opaque) < 0) return -1; @@ -7059,20 +7061,46 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } VIR_FREE(nodes); - if ((node = virXPathNode("./devices/console[1]", ctxt)) != NULL) { + if ((n = virXPathNodeSet("./devices/console", ctxt, &nodes)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot extract console devices")); + goto error; + } + if (n && VIR_ALLOC_N(def->consoles, n) < 0) + goto no_memory; + + for (i = 0 ; i < n ; i++) { virDomainChrDefPtr chr = virDomainChrDefParseXML(caps, - node, + nodes[i], flags); if (!chr) goto error; - chr->target.port = 0; /* - * For HVM console actually created a serial device - * while for non-HVM it was a parvirt console + * Some really crazy backcompat stuff for consoles + * + * Historically the first (and only) '<console>' + * element in an HVM guest was treated as being + * an alias for a <serial> device. + * + * So if we see that this console device should + * be a serial device, then we move the config + * over to def->serials[0] (or discard it if + * that already exists + * + * We then fill def->consoles[0] with a stub + * just so we get sequencing correct for consoles + * > 0 */ if (STREQ(def->os.type, "hvm") && - chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) { + (chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)) { + if (i != 0) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only the first console can be a serial port")); + goto error; + } + + /* Either discard or move this chr to the serial config */ if (def->nserials != 0) { virDomainChrDefFree(chr); } else { @@ -7080,14 +7108,24 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virDomainChrDefFree(chr); goto no_memory; } + chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; def->nserials = 1; def->serials[0] = chr; - chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; + chr->target.port = 0; } - } else { - def->console = chr; + + /* And create a stub placeholder */ + if (VIR_ALLOC(chr) < 0) + goto no_memory; + chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; + chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } + + chr->target.port = i; + + def->consoles[def->nconsoles++] = chr; } + VIR_FREE(nodes); if ((n = virXPathNodeSet("./devices/channel", ctxt, &nodes)) < 0) { goto error; @@ -8533,14 +8571,17 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, if (!virDomainChannelDefCheckABIStability(src->channels[i], dst->channels[i])) goto cleanup; - if ((!src->console && dst->console) || - (src->console && !dst->console)) { + if (src->nconsoles != dst->nconsoles) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain console count %d does not match source %d"), - dst->console ? 1 : 0, src->console ? 1 : 0); + dst->nconsoles, src->nconsoles); goto cleanup; } + for (i = 0 ; i < src->nconsoles ; i++) + if (!virDomainConsoleDefCheckABIStability(src->consoles[i], dst->consoles[i])) + goto cleanup; + if (src->nhubs != dst->nhubs) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain hub device count %d does not match source %d"), @@ -8552,9 +8593,6 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, if (!virDomainHubDefCheckABIStability(src->hubs[i], dst->hubs[i])) goto cleanup; - if (src->console && - !virDomainConsoleDefCheckABIStability(src->console, dst->console)) - goto cleanup; if ((!src->watchdog && dst->watchdog) || (src->watchdog && !dst->watchdog)) { @@ -8676,8 +8714,8 @@ static int virDomainDefMaybeAddVirtioSerialController(virDomainDefPtr def) } } - if (def->console) { - virDomainChrDefPtr console = def->console; + for (i = 0 ; i < def->nconsoles ; i++) { + virDomainChrDefPtr console = def->consoles[i]; if (console->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { int idx = 0; @@ -10904,15 +10942,27 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (virDomainChrDefFormat(buf, def->parallels[n], flags) < 0) goto cleanup; - /* If there's a PV console that's preferred.. */ - if (def->console) { - if (virDomainChrDefFormat(buf, def->console, flags) < 0) + for (n = 0 ; n < def->nconsoles ; n++) { + virDomainChrDef console; + /* Back compat, ignore the console element for hvm guests + * if it is type == serial + */ + if (STREQ(def->os.type, "hvm") && + (def->consoles[n]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) && + (n < def->nserials)) { + memcpy(&console, def->serials[n], sizeof(console)); + console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; + } else { + memcpy(&console, def->consoles[n], sizeof(console)); + } + if (virDomainChrDefFormat(buf, &console, flags) < 0) goto cleanup; - } else if (def->nserials != 0) { - /* ..else for legacy compat duplicate the first serial device as a - * console */ + } + if (STREQ(def->os.type, "hvm") && + def->nconsoles == 0 && + def->nserials > 0) { virDomainChrDef console; - memcpy(&console, def->serials[0], sizeof(console)); + memcpy(&console, def->serials[n], sizeof(console)); console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; if (virDomainChrDefFormat(buf, &console, flags) < 0) goto cleanup; @@ -12533,9 +12583,9 @@ int virDomainChrDefForeach(virDomainDefPtr def, if (abortOnError && rc != 0) goto done; } - if (def->console) { + for (i = 0 ; i < def->nconsoles ; i++) { if ((iter)(def, - def->console, + def->consoles[i], opaque) < 0) rc = -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2119b5a..a78b3ab 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1403,6 +1403,9 @@ struct _virDomainDef { int nchannels; virDomainChrDefPtr *channels; + int nconsoles; + virDomainChrDefPtr *consoles; + size_t nleases; virDomainLeaseDefPtr *leases; @@ -1410,7 +1413,6 @@ struct _virDomainDef { virDomainHubDefPtr *hubs; /* Only 1 */ - virDomainChrDefPtr console; virSecurityLabelDef seclabel; virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f08e8d1..9b5c9db 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1683,10 +1683,18 @@ static int lxcVmStart(virConnectPtr conn, _("Failed to allocate tty")); goto cleanup; } - if (vm->def->console && - vm->def->console->source.type == VIR_DOMAIN_CHR_TYPE_PTY) { - VIR_FREE(vm->def->console->source.data.file.path); - vm->def->console->source.data.file.path = parentTtyPath; + if (vm->def->nconsoles) { + if (vm->def->nconsoles > 1) { + lxcError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only one console supported")); + goto cleanup; + } + if (vm->def->consoles[0]->source.type == VIR_DOMAIN_CHR_TYPE_PTY) { + VIR_FREE(vm->def->consoles[0]->source.data.file.path); + vm->def->consoles[0]->source.data.file.path = parentTtyPath; + } else { + VIR_FREE(parentTtyPath); + } } else { VIR_FREE(parentTtyPath); } @@ -3022,8 +3030,8 @@ lxcDomainOpenConsole(virDomainPtr dom, _("Named device aliases are not supported")); goto cleanup; } else { - if (vm->def->console) - chr = vm->def->console; + if (vm->def->nconsoles) + chr = vm->def->consoles[0]; else if (vm->def->nserials) chr = vm->def->serials[0]; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 30c0be6..bfa0b63 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -716,16 +716,16 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virBitmapPtr qemuCaps) if (virAsprintf(&def->channels[i]->info.alias, "channel%d", i) < 0) goto no_memory; } - for (i = 0; i < def->nsmartcards ; i++) { - if (virAsprintf(&def->smartcards[i]->info.alias, "smartcard%d", i) < 0) + for (i = 0; i < def->nconsoles ; i++) { + if (virAsprintf(&def->consoles[i]->info.alias, "console%d", i) < 0) goto no_memory; } for (i = 0; i < def->nhubs ; i++) { if (virAsprintf(&def->hubs[i]->info.alias, "hub%d", i) < 0) goto no_memory; } - if (def->console) { - if (virAsprintf(&def->console->info.alias, "console%d", i) < 0) + for (i = 0; i < def->nsmartcards ; i++) { + if (virAsprintf(&def->smartcards[i]->info.alias, "smartcard%d", i) < 0) goto no_memory; } if (def->watchdog) { @@ -4493,8 +4493,8 @@ qemuBuildCommandLine(virConnectPtr conn, } /* Explicit console devices */ - if (def->console) { - virDomainChrDefPtr console = def->console; + for (i = 0 ; i < def->nconsoles ; i++) { + virDomainChrDefPtr console = def->consoles[i]; char *devstr; switch(console->targetType) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 84ef4dd..3d84bac 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10481,9 +10481,10 @@ qemuDomainOpenConsole(virDomainPtr dom, } if (dev_name) { - if (vm->def->console && - STREQ(dev_name, vm->def->console->info.alias)) - chr = vm->def->console; + for (i = 0 ; !chr && i < vm->def->nconsoles ; i++) { + if (STREQ(dev_name, vm->def->consoles[i]->info.alias)) + chr = vm->def->consoles[i]; + } for (i = 0 ; !chr && i < vm->def->nserials ; i++) { if (STREQ(dev_name, vm->def->serials[i]->info.alias)) chr = vm->def->serials[i]; @@ -10493,8 +10494,8 @@ qemuDomainOpenConsole(virDomainPtr dom, chr = vm->def->parallels[i]; } } else { - if (vm->def->console) - chr = vm->def->console; + if (vm->def->nconsoles) + chr = vm->def->consoles[0]; else if (vm->def->nserials) chr = vm->def->serials[0]; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a7fe86c..5cdd954 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1112,8 +1112,8 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm, paths, chardevfmt) < 0) return -1; - if (vm->def->console && - qemuProcessLookupPTYs(&vm->def->console, 1, paths, chardevfmt) < 0) + if (qemuProcessLookupPTYs(vm->def->consoles, vm->def->nconsoles, + paths, chardevfmt) < 0) return -1; return 0; @@ -1161,8 +1161,8 @@ qemuProcessFindCharDevicePTYs(virDomainObjPtr vm, } } - if (vm->def->console) { - virDomainChrDefPtr chr = vm->def->console; + for (i = 0 ; i < vm->def->nconsoles ; i++) { + virDomainChrDefPtr chr = vm->def->consoles[i]; if (chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY && chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { if ((ret = qemuProcessExtractTTYPath(output, &offset, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index e1a257d..78c0d45 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -888,6 +888,11 @@ SELinuxRestoreSecurityChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, { virDomainObjPtr vm = opaque; + /* This is taken care of by processing of def->serials */ + if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) + return 0; + return SELinuxRestoreSecurityChardevLabel(vm, &dev->source); } @@ -1228,6 +1233,11 @@ SELinuxSetSecurityChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, { virDomainObjPtr vm = opaque; + /* This is taken care of by processing of def->serials */ + if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) + return 0; + return SELinuxSetSecurityChardevLabel(vm, &dev->source); } diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index ad4b8a7..71a4586 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -925,12 +925,16 @@ get_files(vahControl * ctl) ctl->def->serials[i]->source.type) != 0) goto clean; - if (ctl->def->console && ctl->def->console->source.data.file.path) - if (vah_add_file_chardev(&buf, - ctl->def->console->source.data.file.path, - "rw", - ctl->def->console->source.type) != 0) - goto clean; + for (i = 0; i < ctl->def->nconsoles; i++) + if (ctl->def->consoles[i] && + (ctl->def->consoles[i]->source.type == VIR_DOMAIN_CHR_TYPE_PTY || + ctl->def->consoles[i]->source.type == VIR_DOMAIN_CHR_TYPE_DEV || + ctl->def->consoles[i]->source.type == VIR_DOMAIN_CHR_TYPE_FILE || + ctl->def->consoles[i]->source.type == VIR_DOMAIN_CHR_TYPE_PIPE) && + ctl->def->consoles[i]->source.data.file.path) + if (vah_add_file(&buf, + ctl->def->consoles[i]->source.data.file.path, "rw") != 0) + goto clean; for (i = 0 ; i < ctl->def->nparallels; i++) if (ctl->def->parallels[i] && diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 7b5e094..4459a2a 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -466,9 +466,13 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn, } for (i = 0 ; i < UML_MAX_CHAR_DEVICE ; i++) { + virDomainChrDefPtr chr = NULL; char *ret = NULL; - if (i == 0 && vm->def->console) - ret = umlBuildCommandLineChr(vm->def->console, "con", cmd); + for (j = 0 ; j < vm->def->nconsoles ; j++) + if (vm->def->consoles[j]->target.port == i) + chr = vm->def->consoles[j]; + if (chr) + ret = umlBuildCommandLineChr(chr, "con", cmd); if (!ret) if (virAsprintf(&ret, "con%d=none", i) < 0) goto no_memory; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 16ab73a..44529f0 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -248,10 +248,10 @@ umlIdentifyChrPTY(struct uml_driver *driver, { int i; - if (dom->def->console && - dom->def->console->source.type == VIR_DOMAIN_CHR_TYPE_PTY) + for (i = 0 ; i < dom->def->nserials; i++) + if (dom->def->consoles[i]->source.type == VIR_DOMAIN_CHR_TYPE_PTY) if (umlIdentifyOneChrPTY(driver, dom, - dom->def->console, "con") < 0) + dom->def->consoles[i], "con") < 0) return -1; for (i = 0 ; i < dom->def->nserials; i++) @@ -2390,8 +2390,8 @@ umlDomainOpenConsole(virDomainPtr dom, _("Named device aliases are not supported")); goto cleanup; } else { - if (vm->def->console) - chr = vm->def->console; + if (vm->def->nconsoles) + chr = vm->def->consoles[0]; else if (vm->def->nserials) chr = vm->def->serials[0]; } diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index b3e7782..beb2ba3 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2136,8 +2136,8 @@ xenUnifiedDomainOpenConsole(virDomainPtr dom, if (!def) goto cleanup; - if (def->console) - chr = def->console; + if (def->nconsoles) + chr = def->consoles[0]; else if (def->nserials) chr = def->serials[0]; diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index d44b0dc..e0bc043 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -202,7 +202,6 @@ xenParseSxprChar(const char *value, } } - /* Compat with legacy <console tty='/dev/pts/5'/> syntax */ switch (def->source.type) { case VIR_DOMAIN_CHR_TYPE_PTY: if (tty != NULL && @@ -1395,12 +1394,15 @@ xenParseSxpr(const struct sexpr *root, def->parallels[def->nparallels++] = chr; } } else { + def->nconsoles = 1; + if (VIR_ALLOC_N(def->consoles, 1) < 0) + goto no_memory; /* Fake a paravirt console, since that's not in the sexpr */ - if (!(def->console = xenParseSxprChar("pty", tty))) + if (!(def->consoles[0] = xenParseSxprChar("pty", tty))) goto error; - def->console->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; - def->console->target.port = 0; - def->console->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; + def->consoles[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; + def->consoles[0]->target.port = 0; + def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; } VIR_FREE(tty); @@ -1613,6 +1615,11 @@ xenFormatSxprChr(virDomainChrDefPtr def, if (def->source.data.nix.listen) virBufferAddLit(buf, ",server,nowait"); break; + + default: + XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported chr device type '%s'"), type); + return -1; } if (virBufferError(buf)) { diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index ff173d8..a2ef8c8 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1066,11 +1066,14 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, } } } else { - if (!(def->console = xenParseSxprChar("pty", NULL))) + def->nconsoles = 1; + if (VIR_ALLOC_N(def->consoles, 1) < 0) + goto no_memory; + if (!(def->consoles[0] = xenParseSxprChar("pty", NULL))) goto cleanup; - def->console->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; - def->console->target.port = 0; - def->console->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; + def->consoles[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; + def->consoles[0]->target.port = 0; + def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; } if (hvm) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-auto.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-auto.args index e3fbc24..715bda6 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-auto.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-auto.args @@ -1,6 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ -pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \ virtio-serial-pci,id=virtio-serial0,max_ports=16,vectors=4,bus=pci.0,addr=0x3 \ -device virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa -device \ virtio-serial-pci,id=virtio-serial2,bus=pci.0,addr=0x4 -hda \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args index 5356bcc..d46ed67 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args @@ -1,6 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ -pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \ virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa -hda \ /dev/HostVG/QEMUGuest1 -chardev pty,id=charchannel0 -device virtserialport,\ bus=virtio-serial1.0,nr=3,chardev=charchannel0,id=channel0,\ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many.args b/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many.args new file mode 100644 index 0000000..8af2549 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many.args @@ -0,0 +1,12 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \ +virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -hda \ +/dev/HostVG/QEMUGuest1 -chardev pty,id=charserial0 \ +-device isa-serial,chardev=charserial0,id=serial0 -chardev pty,id=charconsole1 \ +-device virtconsole,chardev=charconsole1,id=console1 -chardev \ +pty,id=charconsole2 -device virtconsole,chardev=charconsole2,id=console2 \ +-chardev pty,id=charconsole3 -device virtconsole,chardev=charconsole3,\ +id=console3 -usb -device virtio-balloon-pci,id=balloon0,\ +bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many.xml b/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many.xml new file mode 100644 index 0000000..e65fb74 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-console-virtio-many.xml @@ -0,0 +1,41 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <controller type='virtio-serial' index='0'/> + <serial type='pty'> + <target port='0'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <console type='pty'> + <target type='virtio' port='1'/> + </console> + <console type='pty'> + <target type='virtio' port='2'/> + </console> + <console type='pty'> + <target type='virtio' port='3'/> + </console> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-console-virtio.args b/tests/qemuxml2argvdata/qemuxml2argv-console-virtio.args index 7e852ff..9f23c27 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-console-virtio.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-console-virtio.args @@ -1,6 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ -pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \ virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -hda \ /dev/HostVG/QEMUGuest1 -chardev pty,id=charconsole0 -device virtconsole,\ chardev=charconsole0,id=console0 -usb -device virtio-balloon-pci,id=balloon0,\ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a7ae925..2d158cf 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -471,11 +471,13 @@ mymain(void) DO_TEST("channel-guestfwd", false, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("channel-virtio", false, - QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST("channel-virtio-auto", false, - QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST("console-virtio", false, - QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); + DO_TEST("console-virtio-many", false, + QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST("channel-spicevmc", false, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SPICE, QEMU_CAPS_CHARDEV_SPICEVMC); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index af635d9..3f37520 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -174,6 +174,7 @@ mymain(void) DO_TEST("serial-many"); DO_TEST("parallel-tcp"); DO_TEST("console-compat"); + DO_TEST("console-virtio-many"); DO_TEST("channel-guestfwd"); DO_TEST("channel-virtio"); -- 1.7.6.4
On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
While Xen only has a single paravirt console, UML, and QEMU both support multiple paravirt consoles. The LXC driver can also be trivially made to support multiple consoles. This patch extends the XML to allow multiple <console> elements in the XML. It also makes the UML and QEMU drivers support this config.
src/conf/domain_conf.c | 110 ++++++++++++++------ src/conf/domain_conf.h | 4 +-
I still need to review this series, but a quick question - shouldn't this patch also update docs/formatdomain.html.in, under name="elementCharConsole", to mention that multiple consoles are now supported in particular hypervisors? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
While Xen only has a single paravirt console, UML, and QEMU both support multiple paravirt consoles. The LXC driver can also be trivially made to support multiple consoles. This patch extends the XML to allow multiple <console> elements in the XML. It also makes the UML and QEMU drivers support this config.
See my earlier question about documenting this in docs/formatdomain.html.in.
+++ b/src/qemu/qemu_process.c @@ -1112,8 +1112,8 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm, paths, chardevfmt)< 0) return -1;
- if (vm->def->console&& - qemuProcessLookupPTYs(&vm->def->console, 1, paths, chardevfmt)< 0) + if (qemuProcessLookupPTYs(vm->def->consoles, vm->def->nconsoles, + paths, chardevfmt)< 0)
Does qemuProcessLookupPTYs behave if vm->def->nconsoles is 0 (we previously skipped calling it if there were no consoles, so you may need to skip calling it here in the same situation).
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-auto.args @@ -1,6 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ -pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
Is this conversion from '-monitor' to '-device -mon' intentional? I think its okay, but wonder if it should have been split into a separate patch, since it has nothing to do with consoles.
+++ b/tests/qemuxml2argvdata/qemuxml2argv-console-virtio.args @@ -1,6 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ -pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \ virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -hda \ /dev/HostVG/QEMUGuest1 -chardev pty,id=charconsole0 -device virtconsole,\ chardev=charconsole0,id=console0 -usb -device virtio-balloon-pci,id=balloon0,\ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a7ae925..2d158cf 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -471,11 +471,13 @@ mymain(void) DO_TEST("channel-guestfwd", false, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("channel-virtio", false, - QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG);
Ah, I see - you altered the flags being passed, which is why the .args files had to change. It makes sense that you need QEMU_CAPS_CHARDEV for console-virtio-many to work, and you just made the other tests consistent; still, my observation above about splitting just this part of the tests change into a separate commit might help. ACK once you resolve the qemuProcessLookupPTYs question. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, Nov 01, 2011 at 04:03:27PM -0600, Eric Blake wrote:
On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
While Xen only has a single paravirt console, UML, and QEMU both support multiple paravirt consoles. The LXC driver can also be trivially made to support multiple consoles. This patch extends the XML to allow multiple <console> elements in the XML. It also makes the UML and QEMU drivers support this config.
See my earlier question about documenting this in docs/formatdomain.html.in.
+++ b/src/qemu/qemu_process.c @@ -1112,8 +1112,8 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm, paths, chardevfmt)< 0) return -1;
- if (vm->def->console&& - qemuProcessLookupPTYs(&vm->def->console, 1, paths, chardevfmt)< 0) + if (qemuProcessLookupPTYs(vm->def->consoles, vm->def->nconsoles, + paths, chardevfmt)< 0)
Does qemuProcessLookupPTYs behave if vm->def->nconsoles is 0 (we previously skipped calling it if there were no consoles, so you may need to skip calling it here in the same situation).
We already call qemuProcessLookupPTYs unconditionally for nserials, nparallels and nchannels & I've double checked its safe :-)
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-auto.args @@ -1,6 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ -pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \
Is this conversion from '-monitor' to '-device -mon' intentional? I think its okay, but wonder if it should have been split into a separate patch, since it has nothing to do with consoles.
+++ b/tests/qemuxml2argvdata/qemuxml2argv-console-virtio.args @@ -1,6 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ -pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \ virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -hda \ /dev/HostVG/QEMUGuest1 -chardev pty,id=charconsole0 -device virtconsole,\ chardev=charconsole0,id=console0 -usb -device virtio-balloon-pci,id=balloon0,\ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a7ae925..2d158cf 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -471,11 +471,13 @@ mymain(void) DO_TEST("channel-guestfwd", false, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("channel-virtio", false, - QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + QEMU_CAPS_DEVICE, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG);
Ah, I see - you altered the flags being passed, which is why the .args files had to change. It makes sense that you need QEMU_CAPS_CHARDEV for console-virtio-many to work, and you just made the other tests consistent; still, my observation above about splitting just this part of the tests change into a separate commit might help.
Yes, this test case was using a bogus combination of capabilities flags that would never exist in a real QEMU binary, so giving test output that was bogus and caused problems with many console support Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: "Daniel P. Berrange" <berrange@redhat.com> qemuBuildVirtioSerialPortDevStr was mistakenly accessing the target.name field in the virDomainChrDef object for chardevs belonging to a console. Those chardevs only have port set, and if there's > 1 console, the > 1port number results in trying to access a target.name with address 0x1 * src/qemu/qemu_command.c: Fix target.name handling and make code more robust wrt error reporting * src/qemu/qemu_command.c: Conditionally access target.name --- src/qemu/qemu_command.c | 39 +++++++++++++++++++++++++++------------ 1 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bfa0b63..24d3dd1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2845,13 +2845,24 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev, virBitmapPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; - if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE) + switch (dev->deviceType) { + case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: virBufferAddLit(&buf, "virtconsole"); - else if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) && - dev->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) - virBufferAddLit(&buf, "spicevmc"); - else - virBufferAddLit(&buf, "virtserialport"); + break; + case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL: + /* Legacy syntax '-device spicevmc' */ + if (dev->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC && + qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC)) { + virBufferAddLit(&buf, "spicevmc"); + } else { + virBufferAddLit(&buf, "virtserialport"); + } + break; + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Cannot use virtio serial for parallel/serial devices")); + return NULL; + } if (dev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { /* Check it's a virtio-serial address */ @@ -2872,7 +2883,8 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev, dev->info.addr.vioserial.port); } - if (dev->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC && + if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && + dev->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC && dev->target.name && STRNEQ(dev->target.name, "com.redhat.spice.0")) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -2880,15 +2892,18 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev, dev->target.name); goto error; } - if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) && - dev->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { - virBufferAsprintf(&buf, ",id=%s", dev->info.alias); - } else { + + if (!(dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && + dev->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC && + qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC))) { virBufferAsprintf(&buf, ",chardev=char%s,id=%s", dev->info.alias, dev->info.alias); - if (dev->target.name) { + if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && + dev->target.name) { virBufferAsprintf(&buf, ",name=%s", dev->target.name); } + } else { + virBufferAsprintf(&buf, ",id=%s", dev->info.alias); } if (virBufferError(&buf)) { virReportOOMError(); -- 1.7.6.4
On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
qemuBuildVirtioSerialPortDevStr was mistakenly accessing the target.name field in the virDomainChrDef object for chardevs belonging to a console. Those chardevs only have port set, and if there's> 1 console, the> 1port number results in trying to access a target.name with address 0x1
* src/qemu/qemu_command.c: Fix target.name handling and make code more robust wrt error reporting
* src/qemu/qemu_command.c: Conditionally access target.name --- src/qemu/qemu_command.c | 39 +++++++++++++++++++++++++++------------ 1 files changed, 27 insertions(+), 12 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> The current I/O code for LXC uses a hand crafted event loop to forward I/O between the container & host app, based on epoll to handle EOF on PTYs. This event loop is not easily extendable to add more consoles, or monitor other types of file descriptors. Remove the custom event loop and replace it with a normal libvirt event loop. When detecting EOF on a PTY, disable the event watch on that FD, and fork off a background thread that does a edge-triggered epoll() on the FD. When the FD finally shows new incoming data, the thread re-enables the watch on the FD and exits. When getting EOF from a read() on the PTY, the existing code would do waitpid(WNOHANG) to see if the container had exited. Unfortunately there is a race condition, because even though the process has closed its stdio handles, it might still exist. To deal with this the new event loop uses a SIG_CHILD handler to perform the waitpid only when the container is known to have actually exited. * src/lxc/lxc_controller.c: Rewrite the event loop to use the standard APIs. --- cfg.mk | 2 +- src/lxc/lxc_controller.c | 607 ++++++++++++++++++++++++++++++---------------- 2 files changed, 404 insertions(+), 205 deletions(-) diff --git a/cfg.mk b/cfg.mk index 5a13158..463ce0c 100644 --- a/cfg.mk +++ b/cfg.mk @@ -676,7 +676,7 @@ $(srcdir)/src/remote/remote_client_bodies.h: $(srcdir)/src/remote/remote_protoco # List all syntax-check exemptions: exclude_file_name_regexp--sc_avoid_strcase = ^tools/virsh\.c$$ -_src1=libvirt|fdstream|qemu/qemu_monitor|util/(command|util)|xen/xend_internal|rpc/virnetsocket +_src1=libvirt|fdstream|qemu/qemu_monitor|util/(command|util)|xen/xend_internal|rpc/virnetsocket|lxc/lxc_controller exclude_file_name_regexp--sc_avoid_write = \ ^(src/($(_src1))|daemon/libvirtd|tools/console|tests/(shunload|virnettlscontext)test)\.c$$ diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index c4e7832..fcd57d9 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -438,45 +438,6 @@ error: return -1; } -/** - * lxcFdForward: - * @readFd: file descriptor to read - * @writeFd: file desriptor to write - * - * Reads 1 byte of data from readFd and writes to writeFd. - * - * Returns 0 on success, EAGAIN if returned on read, or -1 in case of error - */ -static int lxcFdForward(int readFd, int writeFd) -{ - int rc = -1; - char buf[2]; - - if (1 != (saferead(readFd, buf, 1))) { - if (EAGAIN == errno) { - rc = EAGAIN; - goto cleanup; - } - - virReportSystemError(errno, - _("read of fd %d failed"), - readFd); - goto cleanup; - } - - if (1 != (safewrite(writeFd, buf, 1))) { - virReportSystemError(errno, - _("write to fd %d failed"), - writeFd); - goto cleanup; - } - - rc = 0; - -cleanup: - return rc; -} - static int lxcControllerClearCapabilities(void) { @@ -496,15 +457,10 @@ static int lxcControllerClearCapabilities(void) return 0; } -typedef struct _lxcTtyForwardFd_t { - int fd; - int active; -} lxcTtyForwardFd_t; - /* Return true if it is ok to ignore an accept-after-epoll syscall that fails with the specified errno value. Else false. */ static bool -ignorable_epoll_accept_errno(int errnum) +ignorable_accept_errno(int errnum) { return (errnum == EINVAL || errnum == ECONNABORTED @@ -512,202 +468,433 @@ ignorable_epoll_accept_errno(int errnum) || errnum == EWOULDBLOCK); } -static bool -lxcPidGone(pid_t container) +static bool quit = false; +static virMutex lock; +static int sigpipe[2]; + +static void lxcSignalChildHandler(int signum ATTRIBUTE_UNUSED) +{ + ignore_value(write(sigpipe[1], "1", 1)); +} + +static void lxcSignalChildIO(int watch ATTRIBUTE_UNUSED, + int fd ATTRIBUTE_UNUSED, + int events ATTRIBUTE_UNUSED, void *opaque) { - waitpid(container, NULL, WNOHANG); + char buf[1]; + int ret; + int *container = opaque; + + ignore_value(read(sigpipe[0], buf, 1)); + ret = waitpid(-1, NULL, WNOHANG); + if (ret == *container) { + virMutexLock(&lock); + quit = true; + virMutexUnlock(&lock); + } +} + + +struct lxcConsole { + + int hostWatch; + int hostFd; /* PTY FD in the host OS */ + bool hostClosed; + int contWatch; + int contFd; /* PTY FD in the container */ + bool contClosed; + + size_t fromHostLen; + char fromHostBuf[1024]; + size_t fromContLen; + char fromContBuf[1024]; +}; + +struct lxcMonitor { + int serverWatch; + int serverFd; /* Server listen socket */ + int clientWatch; + int clientFd; /* Current client FD (if any) */ +}; - if (kill(container, 0) < 0 && - errno == ESRCH) - return true; - return false; +static void lxcClientIO(int watch ATTRIBUTE_UNUSED, int fd, int events, void *opaque) +{ + struct lxcMonitor *monitor = opaque; + char buf[1024]; + ssize_t ret; + + if (events & (VIR_EVENT_HANDLE_HANGUP | + VIR_EVENT_HANDLE_ERROR)) { + virEventRemoveHandle(monitor->clientWatch); + monitor->clientWatch = -1; + return; + } + +reread: + ret = read(fd, buf, sizeof(buf)); + if (ret == -1 && errno == EINTR) + goto reread; + if (ret == -1 && errno == EAGAIN) + return; + if (ret == -1) { + lxcError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to read from monitor client")); + quit = true; + return; + } + if (ret == 0) { + VIR_DEBUG("Client %d gone", fd); + VIR_FORCE_CLOSE(monitor->clientFd); + virEventRemoveHandle(monitor->clientWatch); + monitor->clientWatch = -1; + } +} + + +static void lxcServerAccept(int watch ATTRIBUTE_UNUSED, int fd, int events ATTRIBUTE_UNUSED, void *opaque) +{ + struct lxcMonitor *monitor = opaque; + int client; + + if ((client = accept(fd, NULL, NULL)) < 0) { + /* First reflex may be simply to declare accept failure + to be a fatal error. However, accept may fail when + a client quits between the above poll and here. + That case is not fatal, but rather to be expected, + if not common, so ignore it. */ + if (ignorable_accept_errno(errno)) + return; + virReportSystemError(errno, "%s", + _("Unable to accept monitor client")); + quit = true; + return; + } + VIR_DEBUG("New client %d (old %d)\n", client, monitor->clientFd); + VIR_FORCE_CLOSE(monitor->clientFd); + virEventRemoveHandle(monitor->clientWatch); + + monitor->clientFd = client; + if ((monitor->clientWatch = virEventAddHandle(monitor->clientFd, + VIR_EVENT_HANDLE_READABLE, + lxcClientIO, + monitor, + NULL)) < 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to watch client socket")); + quit = true; + return; + } +} + +static void lxcConsoleUpdateWatch(struct lxcConsole *console) +{ + int hostEvents = 0; + int contEvents = 0; + + if (!console->hostClosed) { + if (console->fromHostLen < sizeof(console->fromHostBuf)) + hostEvents |= VIR_EVENT_HANDLE_READABLE; + if (console->fromContLen) + hostEvents |= VIR_EVENT_HANDLE_WRITABLE; + } + if (!console->contClosed) { + if (console->fromContLen < sizeof(console->fromContBuf)) + contEvents |= VIR_EVENT_HANDLE_READABLE; + if (console->fromHostLen) + contEvents |= VIR_EVENT_HANDLE_WRITABLE; + } + + virEventUpdateHandle(console->contWatch, contEvents); + virEventUpdateHandle(console->hostWatch, hostEvents); } + +struct lxcConsoleEOFData { + struct lxcConsole *console; + int fd; +}; + + +static void lxcConsoleEOFThread(void *opaque) +{ + struct lxcConsoleEOFData *data = opaque; + int ret; + int epollfd = -1; + struct epoll_event event; + + VIR_WARN("MOnitor %d", data->fd); + if ((epollfd = epoll_create(2)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to create epoll fd")); + goto cleanup; + } + + event.events = EPOLLIN | EPOLLET; + event.data.fd = data->fd; + if (epoll_ctl(epollfd, EPOLL_CTL_ADD, data->fd, &event) < 0) { + virReportSystemError(errno, "%s", + _("Unable to add epoll fd")); + goto cleanup; + } + + for (;;) { + ret = epoll_wait(epollfd, &event, 1, -1); + VIR_WARN("Got %d 0x%x", ret, event.events); + if (ret < 0) { + if (ret == EINTR) + continue; + virReportSystemError(errno, "%s", + _("Unable to wait on epoll")); + quit = true; + goto cleanup; + } + + /* If we get HUP+dead PID, we just re-enable the main loop + * which will see the PID has died and exit */ + if ((event.events & EPOLLIN)) { + VIR_WARN("Reenable %d", data->fd); + virMutexLock(&lock); + if (event.data.fd == data->console->hostFd) { + data->console->hostClosed = false; + } else { + data->console->contClosed = false; + } + lxcConsoleUpdateWatch(data->console); + virMutexUnlock(&lock); + break; + } + } + +cleanup: + VIR_WARN("Done %d", data->fd); + VIR_FORCE_CLOSE(epollfd); + VIR_FREE(data); +} + +static int lxcCheckEOF(struct lxcConsole *console, int fd) +{ + struct lxcConsoleEOFData *data; + virThread thread; + + if (VIR_ALLOC(data) < 0) { + virReportOOMError(); + return -1; + } + + data->console = console; + data->fd = fd; + + if (virThreadCreate(&thread, false, lxcConsoleEOFThread, data) < 0) { + VIR_FREE(data); + return -1; + } + return 0; +} + +static void lxcConsoleIO(int watch, int fd, int events, void *opaque) +{ + struct lxcConsole *console = opaque; + + virMutexLock(&lock); + if (events & VIR_EVENT_HANDLE_READABLE) { + char *buf; + size_t *len; + size_t avail; + ssize_t done; + if (watch == console->hostWatch) { + buf = console->fromHostBuf; + len = &console->fromHostLen; + avail = sizeof(console->fromHostBuf) - *len; + } else { + buf = console->fromContBuf; + len = &console->fromContLen; + avail = sizeof(console->fromContBuf) - *len; + } + reread: + done = read(fd, buf + *len, avail); + if (done == -1 && errno == EINTR) + goto reread; + if (done == -1 && errno != EAGAIN) { + virReportSystemError(errno, "%s", + _("Unable to read container pty")); + goto error; + } + if (done > 0) + *len += done; + else { + VIR_WARN("Read fd %d done %d errno %d", fd, (int)done, errno); + } + } + + if (events & VIR_EVENT_HANDLE_WRITABLE) { + char *buf; + size_t *len; + ssize_t done; + if (watch == console->hostWatch) { + buf = console->fromContBuf; + len = &console->fromContLen; + } else { + buf = console->fromHostBuf; + len = &console->fromHostLen; + } + + rewrite: + done = write(fd, buf, *len); + if (done == -1 && errno == EINTR) + goto rewrite; + if (done == -1 && errno != EAGAIN) { + virReportSystemError(errno, "%s", + _("Unable to write to container pty")); + goto error; + } + if (done > 0) { + memmove(buf, buf + done, (*len - done)); + *len -= done; + } else { + VIR_WARN("Write fd %d done %d errno %d", fd, (int)done, errno); + } + } + + if (events & VIR_EVENT_HANDLE_HANGUP) { + if (watch == console->hostWatch) { + console->hostClosed = true; + } else { + console->contClosed = true; + } + VIR_WARN("Got EOF on %d %d", watch, fd); + if (lxcCheckEOF(console, fd) < 0) + goto error; + } + + lxcConsoleUpdateWatch(console); + virMutexUnlock(&lock); + return; + +error: + virEventRemoveHandle(console->contWatch); + virEventRemoveHandle(console->hostWatch); + console->contWatch = console->hostWatch = -1; + quit = true; + virMutexUnlock(&lock); +} + + /** * lxcControllerMain - * @monitor: server socket fd to accept client requests - * @client: initial client which is the libvirtd daemon - * @appPty: open fd for application facing Pty - * @contPty: open fd for container facing Pty + * @serverFd: server socket fd to accept client requests + * @clientFd: initial client which is the libvirtd daemon + * @hostFd: open fd for application facing Pty + * @contFd: open fd for container facing Pty * - * Forwards traffic between fds. Data read from appPty will be written to contPty - * This process loops forever. - * This uses epoll in edge triggered mode to avoid a hard loop on POLLHUP - * events when the user disconnects the virsh console via ctrl-] + * Processes I/O on consoles and the monitor * * Returns 0 on success or -1 in case of error */ -static int lxcControllerMain(int monitor, - int client, - int appPty, - int contPty, +static int lxcControllerMain(int serverFd, + int clientFd, + int hostFd, + int contFd, pid_t container) { + struct lxcConsole console = { + .hostFd = hostFd, + .contFd = contFd, + }; + struct lxcMonitor monitor = { + .serverFd = serverFd, + .clientFd = clientFd, + }; + virErrorPtr err; int rc = -1; - int epollFd; - struct epoll_event epollEvent; - int numEvents; - int numActive = 0; - lxcTtyForwardFd_t fdArray[2]; - int timeout = -1; - int curFdOff = 0; - int writeFdOff = 0; - - fdArray[0].fd = appPty; - fdArray[0].active = 0; - fdArray[1].fd = contPty; - fdArray[1].active = 0; - - VIR_DEBUG("monitor=%d client=%d appPty=%d contPty=%d", - monitor, client, appPty, contPty); - - /* create the epoll fild descriptor */ - epollFd = epoll_create(2); - if (0 > epollFd) { + + if (virMutexInit(&lock) < 0) + goto cleanup2; + + if (pipe(sigpipe) < 0) { virReportSystemError(errno, "%s", - _("epoll_create(2) failed")); + _("Cannot create signal pipe")); goto cleanup; } - /* add the file descriptors the epoll fd */ - memset(&epollEvent, 0x00, sizeof(epollEvent)); - epollEvent.events = EPOLLIN|EPOLLET; /* edge triggered */ - epollEvent.data.fd = appPty; - if (0 > epoll_ctl(epollFd, EPOLL_CTL_ADD, appPty, &epollEvent)) { - virReportSystemError(errno, "%s", - _("epoll_ctl(appPty) failed")); + if (virEventAddHandle(sigpipe[0], + VIR_EVENT_HANDLE_READABLE, + lxcSignalChildIO, + &container, + NULL) < 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to watch signal socket")); goto cleanup; } - epollEvent.data.fd = contPty; - if (0 > epoll_ctl(epollFd, EPOLL_CTL_ADD, contPty, &epollEvent)) { + + if (signal(SIGCHLD, lxcSignalChildHandler) == SIG_ERR) { virReportSystemError(errno, "%s", - _("epoll_ctl(contPty) failed")); + _("Cannot install signal handler")); goto cleanup; } - epollEvent.events = EPOLLIN; - epollEvent.data.fd = monitor; - if (0 > epoll_ctl(epollFd, EPOLL_CTL_ADD, monitor, &epollEvent)) { - virReportSystemError(errno, "%s", - _("epoll_ctl(monitor) failed")); + VIR_DEBUG("serverFd=%d clientFd=%d hostFd=%d contFd=%d", + serverFd, clientFd, hostFd, contFd); + virResetLastError(); + + if ((monitor.serverWatch = virEventAddHandle(monitor.serverFd, + VIR_EVENT_HANDLE_READABLE, + lxcServerAccept, + &monitor, + NULL)) < 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to watch monitor socket")); goto cleanup; } - epollEvent.events = EPOLLHUP; - epollEvent.data.fd = client; - if (0 > epoll_ctl(epollFd, EPOLL_CTL_ADD, client, &epollEvent)) { - virReportSystemError(errno, "%s", - _("epoll_ctl(client) failed")); + if (monitor.clientFd != -1 && + (monitor.clientWatch = virEventAddHandle(monitor.clientFd, + VIR_EVENT_HANDLE_READABLE, + lxcClientIO, + &monitor, + NULL)) < 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to watch client socket")); goto cleanup; } - while (1) { - /* if active fd's, return if no events, else wait forever */ - timeout = (numActive > 0) ? 0 : -1; - numEvents = epoll_wait(epollFd, &epollEvent, 1, timeout); - if (numEvents > 0) { - if (epollEvent.data.fd == monitor) { - int fd = accept(monitor, NULL, 0); - if (fd < 0) { - /* First reflex may be simply to declare accept failure - to be a fatal error. However, accept may fail when - a client quits between the above epoll_wait and here. - That case is not fatal, but rather to be expected, - if not common, so ignore it. */ - if (ignorable_epoll_accept_errno(errno)) - continue; - virReportSystemError(errno, "%s", - _("accept(monitor,...) failed")); - goto cleanup; - } - if (client != -1) { /* Already connected, so kick new one out */ - VIR_FORCE_CLOSE(fd); - continue; - } - client = fd; - epollEvent.events = EPOLLHUP; - epollEvent.data.fd = client; - if (0 > epoll_ctl(epollFd, EPOLL_CTL_ADD, client, &epollEvent)) { - virReportSystemError(errno, "%s", - _("epoll_ctl(client) failed")); - goto cleanup; - } - } else if (client != -1 && epollEvent.data.fd == client) { - if (0 > epoll_ctl(epollFd, EPOLL_CTL_DEL, client, &epollEvent)) { - virReportSystemError(errno, "%s", - _("epoll_ctl(client) failed")); - goto cleanup; - } - VIR_FORCE_CLOSE(client); - } else { - if (epollEvent.events & EPOLLIN) { - curFdOff = epollEvent.data.fd == appPty ? 0 : 1; - if (!fdArray[curFdOff].active) { - fdArray[curFdOff].active = 1; - ++numActive; - } - } else if (epollEvent.events & EPOLLHUP) { - if (lxcPidGone(container)) - goto cleanup; - curFdOff = epollEvent.data.fd == appPty ? 0 : 1; - if (fdArray[curFdOff].active) { - fdArray[curFdOff].active = 0; - --numActive; - } - continue; - } else { - lxcError(VIR_ERR_INTERNAL_ERROR, - _("error event %d"), epollEvent.events); - goto cleanup; - } - } - } else if (0 == numEvents) { - if (2 == numActive) { - /* both fds active, toggle between the two */ - curFdOff ^= 1; - } else { - /* only one active, if current is active, use it, else it */ - /* must be the other one (ie. curFd just went inactive) */ - curFdOff = fdArray[curFdOff].active ? curFdOff : curFdOff ^ 1; - } + if ((console.hostWatch = virEventAddHandle(console.hostFd, + VIR_EVENT_HANDLE_READABLE, + lxcConsoleIO, + &console, + NULL)) < 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to watch host console PTY")); + goto cleanup; + } - } else { - if (EINTR == errno) { - continue; - } + if ((console.contWatch = virEventAddHandle(console.contFd, + VIR_EVENT_HANDLE_READABLE, + lxcConsoleIO, + &console, + NULL)) < 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to watch host console PTY")); + goto cleanup; + } - /* error */ - virReportSystemError(errno, "%s", - _("epoll_wait() failed")); + while (!quit) { + if (virEventRunDefaultImpl() < 0) goto cleanup; - - } - - if (0 < numActive) { - writeFdOff = curFdOff ^ 1; - rc = lxcFdForward(fdArray[curFdOff].fd, fdArray[writeFdOff].fd); - - if (EAGAIN == rc) { - /* this fd no longer has data, set it as inactive */ - --numActive; - fdArray[curFdOff].active = 0; - } else if (-1 == rc) { - if (lxcPidGone(container)) - goto cleanup; - continue; - } - - } - } - rc = 0; + err = virGetLastError(); + if (!err || err->code == VIR_ERR_OK) + rc = 0; cleanup: - VIR_FORCE_CLOSE(appPty); - VIR_FORCE_CLOSE(contPty); - VIR_FORCE_CLOSE(epollFd); + virMutexDestroy(&lock); + signal(SIGCHLD, SIG_DFL); +cleanup2: + VIR_FORCE_CLOSE(console.hostFd); + VIR_FORCE_CLOSE(console.contFd); + VIR_FORCE_CLOSE(monitor.serverFd); + VIR_FORCE_CLOSE(monitor.clientFd); return rc; } @@ -1004,7 +1191,17 @@ lxcControllerRun(virDomainDefPtr def, } VIR_FORCE_CLOSE(handshakefd); + if (virSetBlocking(monitor, false) < 0 || + virSetBlocking(client, false) < 0 || + virSetBlocking(appPty, false) < 0 || + virSetBlocking(containerPty, false) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set file descriptor non blocking")); + goto cleanup; + } + rc = lxcControllerMain(monitor, client, appPty, containerPty, container); + monitor = client = appPty = containerPty = -1; cleanup: VIR_FREE(devptmx); @@ -1153,6 +1350,8 @@ int main(int argc, char *argv[]) goto cleanup; } + virEventRegisterDefaultImpl(); + if ((caps = lxcCapsInit()) == NULL) goto cleanup; -- 1.7.6.4
On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
The current I/O code for LXC uses a hand crafted event loop to forward I/O between the container& host app, based on epoll to handle EOF on PTYs. This event loop is not easily extendable to add more consoles, or monitor other types of
s/extendable/extensible/
file descriptors.
Remove the custom event loop and replace it with a normal libvirt event loop. When detecting EOF on a PTY, disable the event watch on that FD, and fork off a background thread that does a edge-triggered epoll() on the FD. When the FD finally shows new incoming data, the thread re-enables the watch on the FD and exits.
When getting EOF from a read() on the PTY, the existing code would do waitpid(WNOHANG) to see if the container had exited. Unfortunately there is a race condition, because even though the process has closed its stdio handles, it might still exist.
To deal with this the new event loop uses a SIG_CHILD handler to perform the waitpid only when the container is known to have actually exited.
* src/lxc/lxc_controller.c: Rewrite the event loop to use the standard APIs. --- cfg.mk | 2 +- src/lxc/lxc_controller.c | 607 ++++++++++++++++++++++++++++++---------------- 2 files changed, 404 insertions(+), 205 deletions(-)
Big; hopefully I get through it today.
+ +static void lxcConsoleEOFThread(void *opaque) +{ + struct lxcConsoleEOFData *data = opaque; + int ret; + int epollfd = -1; + struct epoll_event event; + + VIR_WARN("MOnitor %d", data->fd);
s/MOnitor/Monitor/ should this be VIR_DEBUG instead of VIR_WARN?
+ if ((epollfd = epoll_create(2))< 0) { + virReportSystemError(errno, "%s", + _("Unable to create epoll fd")); + goto cleanup; + }
Should we be using epoll_create1(EPOLL_CLOEXEC) instead?
+static void lxcConsoleIO(int watch, int fd, int events, void *opaque) +{ ... + if (done> 0) + *len += done; + else { + VIR_WARN("Read fd %d done %d errno %d", fd, (int)done, errno); + }
Use {} on both branches or neither, but not just one branch.
+error: + virEventRemoveHandle(console->contWatch); + virEventRemoveHandle(console->hostWatch); + console->contWatch = console->hostWatch = -1; + quit = true; + virMutexUnlock(&lock);
Some of your code set 'quit = true' outside the 'lock' mutex; was that intentional?
+ if (virMutexInit(&lock)< 0) + goto cleanup2; + + if (pipe(sigpipe)< 0) {
Shouldn't the signal pipe be non-blocking? I'm not sure if cloexec matters, but for consistency, we might as well set it. In other words, should this be: pipe2(sigpipe, O_CLOEXEC|O_NONBLOCK)
+ if (virEventAddHandle(sigpipe[0], + VIR_EVENT_HANDLE_READABLE, + lxcSignalChildIO, +&container, + NULL)< 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to watch signal socket"));
s/socket/pipe/ ACK with nits fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Currently the LXC controller only supports setup of a single text console. This is wired up to the container init's stdio, as well as /dev/console and /dev/tty1. Extending support for multiple consoles, means wiring up additional PTYs to /dev/tty2, /dev/tty3, etc, etc. The LXC controller is passed multiple open file handles, one for each console requested. * src/lxc/lxc_container.c, src/lxc/lxc_container.h: Wire up all the /dev/ttyN links required to symlink to /dev/pts/NN * src/lxc/lxc_container.h: Open more container side /dev/pts/NN devices, and adapt event loop to handle I/O from all consoles * src/lxc/lxc_driver.c: Setup multiple host side PTYs --- src/lxc/lxc_container.c | 81 +++++++++++++++--------- src/lxc/lxc_container.h | 3 +- src/lxc/lxc_controller.c | 160 +++++++++++++++++++++++++++++----------------- src/lxc/lxc_driver.c | 62 +++++++++++------- 4 files changed, 191 insertions(+), 115 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index e9891f7..eb0ee6e 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -90,7 +90,8 @@ struct __lxc_child_argv { unsigned int nveths; char **veths; int monitor; - char *ttyPath; + char **ttyPaths; + size_t nttyPaths; int handshakefd; }; @@ -517,9 +518,9 @@ static int lxcContainerMountDevFS(virDomainFSDefPtr root) return rc; } -static int lxcContainerPopulateDevices(void) +static int lxcContainerPopulateDevices(char **ttyPaths, size_t nttyPaths) { - int i; + size_t i; const struct { int maj; int min; @@ -561,21 +562,28 @@ static int lxcContainerPopulateDevices(void) } } - /* XXX we should allow multiple consoles per container - * for tty2, tty3, etc, but the domain XML does not - * handle this yet - */ - if (symlink("/dev/pts/0", "/dev/tty1") < 0) { - virReportSystemError(errno, "%s", - _("Failed to symlink /dev/pts/0 to /dev/tty1")); - return -1; - } - if (symlink("/dev/pts/0", "/dev/console") < 0) { - virReportSystemError(errno, "%s", - _("Failed to symlink /dev/pts/0 to /dev/console")); - return -1; + for (i = 0 ; i < nttyPaths ; i++) { + char *tty; + if (virAsprintf(&tty, "/dev/tty%zu", i+1) < 0) { + virReportOOMError(); + return -1; + } + if (symlink(ttyPaths[i], tty) < 0) { + VIR_FREE(tty); + virReportSystemError(errno, + _("Failed to symlink %s to %s"), + ttyPaths[i], tty); + return -1; + } + VIR_FREE(tty); + if (i == 0 && + symlink(ttyPaths[i], "/dev/console") < 0) { + virReportSystemError(errno, + _("Failed to symlink %s to /dev/console"), + ttyPaths[i]); + return -1; + } } - return 0; } @@ -904,7 +912,9 @@ static int lxcContainerUnmountOldFS(void) * this is based on this thread http://lkml.org/lkml/2008/3/5/29 */ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, - virDomainFSDefPtr root) + virDomainFSDefPtr root, + char **ttyPaths, + size_t nttyPaths) { /* Gives us a private root, leaving all parent OS mounts on /.oldroot */ if (lxcContainerPivotRoot(root) < 0) @@ -919,7 +929,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, return -1; /* Populates device nodes in /dev/ */ - if (lxcContainerPopulateDevices() < 0) + if (lxcContainerPopulateDevices(ttyPaths, nttyPaths) < 0) return -1; /* Sets up any non-root mounts from guest config */ @@ -963,10 +973,12 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef) } static int lxcContainerSetupMounts(virDomainDefPtr vmDef, - virDomainFSDefPtr root) + virDomainFSDefPtr root, + char **ttyPaths, + size_t nttyPaths) { if (root) - return lxcContainerSetupPivotRoot(vmDef, root); + return lxcContainerSetupPivotRoot(vmDef, root, ttyPaths, nttyPaths); else return lxcContainerSetupExtraMounts(vmDef); } @@ -1050,17 +1062,25 @@ static int lxcContainerChild( void *data ) root = virDomainGetRootFilesystem(vmDef); - if (root) { - if (virAsprintf(&ttyPath, "%s%s", root->src, argv->ttyPath) < 0) { - virReportOOMError(); - goto cleanup; + if (argv->nttyPaths) { + if (root) { + if (virAsprintf(&ttyPath, "%s%s", root->src, argv->ttyPaths[0]) < 0) { + virReportOOMError(); + goto cleanup; + } + } else { + if (!(ttyPath = strdup(argv->ttyPaths[0]))) { + virReportOOMError(); + goto cleanup; + } } } else { - if (!(ttyPath = strdup(argv->ttyPath))) { + if (!(ttyPath = strdup("/dev/null"))) { virReportOOMError(); goto cleanup; } } + VIR_DEBUG("Container TTY path: %s", ttyPath); ttyfd = open(ttyPath, O_RDWR|O_NOCTTY); @@ -1071,7 +1091,7 @@ static int lxcContainerChild( void *data ) goto cleanup; } - if (lxcContainerSetupMounts(vmDef, root) < 0) + if (lxcContainerSetupMounts(vmDef, root, argv->ttyPaths, argv->nttyPaths) < 0) goto cleanup; if (!virFileExists(vmDef->os.init)) { @@ -1175,14 +1195,15 @@ int lxcContainerStart(virDomainDefPtr def, char **veths, int control, int handshakefd, - char *ttyPath) + char **ttyPaths, + size_t nttyPaths) { pid_t pid; int cflags; int stacksize = getpagesize() * 4; char *stack, *stacktop; - lxc_child_argv_t args = { def, nveths, veths, control, ttyPath, - handshakefd}; + lxc_child_argv_t args = { def, nveths, veths, control, + ttyPaths, nttyPaths, handshakefd}; /* allocate a stack for the container */ if (VIR_ALLOC_N(stack, stacksize) < 0) { diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h index d6d9b6d..ffeda5e 100644 --- a/src/lxc/lxc_container.h +++ b/src/lxc/lxc_container.h @@ -53,7 +53,8 @@ int lxcContainerStart(virDomainDefPtr def, char **veths, int control, int handshakefd, - char *ttyPath); + char **ttyPaths, + size_t nttyPaths); int lxcContainerAvailable(int features); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index fcd57d9..2596387 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -793,20 +793,19 @@ error: */ static int lxcControllerMain(int serverFd, int clientFd, - int hostFd, - int contFd, + int *hostFds, + int *contFds, + size_t nFds, pid_t container) { - struct lxcConsole console = { - .hostFd = hostFd, - .contFd = contFd, - }; + struct lxcConsole *consoles; struct lxcMonitor monitor = { .serverFd = serverFd, .clientFd = clientFd, }; virErrorPtr err; int rc = -1; + size_t i; if (virMutexInit(&lock) < 0) goto cleanup2; @@ -833,8 +832,8 @@ static int lxcControllerMain(int serverFd, goto cleanup; } - VIR_DEBUG("serverFd=%d clientFd=%d hostFd=%d contFd=%d", - serverFd, clientFd, hostFd, contFd); + VIR_DEBUG("serverFd=%d clientFd=%d", + serverFd, clientFd); virResetLastError(); if ((monitor.serverWatch = virEventAddHandle(monitor.serverFd, @@ -858,24 +857,34 @@ static int lxcControllerMain(int serverFd, goto cleanup; } - if ((console.hostWatch = virEventAddHandle(console.hostFd, - VIR_EVENT_HANDLE_READABLE, - lxcConsoleIO, - &console, - NULL)) < 0) { - lxcError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to watch host console PTY")); + if (VIR_ALLOC_N(consoles, nFds) < 0) { + virReportOOMError(); goto cleanup; } - if ((console.contWatch = virEventAddHandle(console.contFd, - VIR_EVENT_HANDLE_READABLE, - lxcConsoleIO, - &console, - NULL)) < 0) { - lxcError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to watch host console PTY")); - goto cleanup; + for (i = 0 ; i < nFds ; i++) { + consoles[i].hostFd = hostFds[i]; + consoles[i].contFd = contFds[i]; + + if ((consoles[i].hostWatch = virEventAddHandle(consoles[i].hostFd, + VIR_EVENT_HANDLE_READABLE, + lxcConsoleIO, + &consoles[i], + NULL)) < 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to watch host console PTY")); + goto cleanup; + } + + if ((consoles[i].contWatch = virEventAddHandle(consoles[i].contFd, + VIR_EVENT_HANDLE_READABLE, + lxcConsoleIO, + &consoles[i], + NULL)) < 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to watch host console PTY")); + goto cleanup; + } } while (!quit) { @@ -891,10 +900,9 @@ cleanup: virMutexDestroy(&lock); signal(SIGCHLD, SIG_DFL); cleanup2: - VIR_FORCE_CLOSE(console.hostFd); - VIR_FORCE_CLOSE(console.contFd); VIR_FORCE_CLOSE(monitor.serverFd); VIR_FORCE_CLOSE(monitor.clientFd); + VIR_FREE(consoles); return rc; } @@ -1019,14 +1027,15 @@ lxcControllerRun(virDomainDefPtr def, char **veths, int monitor, int client, - int appPty, + int *ttyFDs, + size_t nttyFDs, int handshakefd) { int rc = -1; int control[2] = { -1, -1}; int containerhandshake[2] = { -1, -1 }; - int containerPty = -1; - char *containerPtyPath = NULL; + int *containerTtyFDs = NULL; + char **containerTtyPaths = NULL; pid_t container = -1; virDomainFSDefPtr root; char *devpts = NULL; @@ -1035,6 +1044,15 @@ lxcControllerRun(virDomainDefPtr def, int *loopDevs = NULL; size_t i; + if (VIR_ALLOC_N(containerTtyFDs, nttyFDs) < 0) { + virReportOOMError(); + goto cleanup; + } + if (VIR_ALLOC_N(containerTtyPaths, nttyFDs) < 0) { + virReportOOMError(); + goto cleanup; + } + if (socketpair(PF_UNIX, SOCK_STREAM, 0, control) < 0) { virReportSystemError(errno, "%s", _("sockpair failed")); @@ -1125,26 +1143,36 @@ lxcControllerRun(virDomainDefPtr def, VIR_WARN("Kernel does not support private devpts, using shared devpts"); VIR_FREE(devptmx); } - } - - if (devptmx) { - VIR_DEBUG("Opening tty on private %s", devptmx); - if (lxcCreateTty(devptmx, &containerPty, &containerPtyPath) < 0) { - virReportSystemError(errno, "%s", - _("Failed to allocate tty")); - goto cleanup; - } } else { - VIR_DEBUG("Opening tty on shared /dev/ptmx"); - if (virFileOpenTty(&containerPty, - &containerPtyPath, - 0) < 0) { - virReportSystemError(errno, "%s", - _("Failed to allocate tty")); + if (nttyFDs != -1) { + lxcError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Expected exactly one TTY fd")); goto cleanup; } } + for (i = 0 ; i < nttyFDs ; i++) { + if (devptmx) { + VIR_DEBUG("Opening tty on private %s", devptmx); + if (lxcCreateTty(devptmx, + &containerTtyFDs[i], + &containerTtyPaths[i]) < 0) { + virReportSystemError(errno, "%s", + _("Failed to allocate tty")); + goto cleanup; + } + } else { + VIR_DEBUG("Opening tty on shared /dev/ptmx"); + if (virFileOpenTty(&containerTtyFDs[i], + &containerTtyPaths[i], + 0) < 0) { + virReportSystemError(errno, "%s", + _("Failed to allocate tty")); + goto cleanup; + } + } + } + if (lxcSetPersonality(def) < 0) goto cleanup; @@ -1153,7 +1181,8 @@ lxcControllerRun(virDomainDefPtr def, veths, control[1], containerhandshake[1], - containerPtyPath)) < 0) + containerTtyPaths, + nttyFDs)) < 0) goto cleanup; VIR_FORCE_CLOSE(control[1]); VIR_FORCE_CLOSE(containerhandshake[1]); @@ -1192,28 +1221,39 @@ lxcControllerRun(virDomainDefPtr def, VIR_FORCE_CLOSE(handshakefd); if (virSetBlocking(monitor, false) < 0 || - virSetBlocking(client, false) < 0 || - virSetBlocking(appPty, false) < 0 || - virSetBlocking(containerPty, false) < 0) { + virSetBlocking(client, false) < 0) { virReportSystemError(errno, "%s", _("Unable to set file descriptor non blocking")); goto cleanup; } + for (i = 0 ; i < nttyFDs ; i++) { + if (virSetBlocking(ttyFDs[i], false) < 0 || + virSetBlocking(containerTtyFDs[i], false) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set file descriptor non blocking")); + goto cleanup; + } + } - rc = lxcControllerMain(monitor, client, appPty, containerPty, container); - monitor = client = appPty = containerPty = -1; + rc = lxcControllerMain(monitor, client, ttyFDs, containerTtyFDs, nttyFDs, container); + monitor = client = -1; cleanup: VIR_FREE(devptmx); VIR_FREE(devpts); VIR_FORCE_CLOSE(control[0]); VIR_FORCE_CLOSE(control[1]); - VIR_FREE(containerPtyPath); - VIR_FORCE_CLOSE(containerPty); VIR_FORCE_CLOSE(handshakefd); VIR_FORCE_CLOSE(containerhandshake[0]); VIR_FORCE_CLOSE(containerhandshake[1]); + for (i = 0 ; i < nttyFDs ; i++) + VIR_FREE(containerTtyPaths[i]); + VIR_FREE(containerTtyPaths); + for (i = 0 ; i < nttyFDs ; i++) + VIR_FORCE_CLOSE(containerTtyFDs[i]); + VIR_FREE(containerTtyFDs); + for (i = 0 ; i < nloopDevs ; i++) VIR_FORCE_CLOSE(loopDevs[i]); VIR_FREE(loopDevs); @@ -1239,7 +1279,6 @@ int main(int argc, char *argv[]) int nveths = 0; char **veths = NULL; int monitor = -1; - int appPty = -1; int handshakefd = -1; int bg = 0; virCapsPtr caps = NULL; @@ -1255,6 +1294,8 @@ int main(int argc, char *argv[]) { "help", 0, NULL, 'h' }, { 0, 0, 0, 0 }, }; + int *ttyFDs = NULL; + size_t nttyFDs = 0; if (setlocale(LC_ALL, "") == NULL || bindtextdomain(PACKAGE, LOCALEDIR) == NULL || @@ -1296,7 +1337,11 @@ int main(int argc, char *argv[]) break; case 'c': - if (virStrToLong_i(optarg, NULL, 10, &appPty) < 0) { + if (VIR_REALLOC_N(ttyFDs, nttyFDs + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + if (virStrToLong_i(optarg, NULL, 10, &ttyFDs[nttyFDs++]) < 0) { fprintf(stderr, "malformed --console argument '%s'", optarg); goto cleanup; } @@ -1334,11 +1379,6 @@ int main(int argc, char *argv[]) goto cleanup; } - if (appPty < 0) { - fprintf(stderr, "%s: missing --console argument for container PTY\n", argv[0]); - goto cleanup; - } - if (handshakefd < 0) { fprintf(stderr, "%s: missing --handshake argument for container PTY\n", argv[0]); @@ -1418,8 +1458,8 @@ int main(int argc, char *argv[]) goto cleanup; } - rc = lxcControllerRun(def, nveths, veths, monitor, client, appPty, - handshakefd); + rc = lxcControllerRun(def, nveths, veths, monitor, client, + ttyFDs, nttyFDs, handshakefd); cleanup: diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9b5c9db..8e02676 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1451,11 +1451,12 @@ lxcBuildControllerCmd(lxc_driver_t *driver, virDomainObjPtr vm, int nveths, char **veths, - int appPty, + int *ttyFDs, + size_t nttyFDs, int logfile, int handshakefd) { - int i; + size_t i; char *filterstr; char *outputstr; virCommandPtr cmd; @@ -1496,8 +1497,12 @@ lxcBuildControllerCmd(lxc_driver_t *driver, virLogGetDefaultPriority()); } - virCommandAddArgList(cmd, "--name", vm->def->name, "--console", NULL); - virCommandAddArgFormat(cmd, "%d", appPty); + virCommandAddArgList(cmd, "--name", vm->def->name, NULL); + for (i = 0 ; i < nttyFDs ; i++) { + virCommandAddArg(cmd, "--console"); + virCommandAddArgFormat(cmd, "%d", ttyFDs[i]); + virCommandPreserveFD(cmd, ttyFDs[i]); + } virCommandAddArg(cmd, "--handshake"); virCommandAddArgFormat(cmd, "%d", handshakefd); virCommandAddArg(cmd, "--background"); @@ -1522,7 +1527,6 @@ lxcBuildControllerCmd(lxc_driver_t *driver, goto cleanup; } - virCommandPreserveFD(cmd, appPty); virCommandPreserveFD(cmd, handshakefd); virCommandSetOutputFD(cmd, &logfile); virCommandSetErrorFD(cmd, &logfile); @@ -1625,9 +1629,9 @@ static int lxcVmStart(virConnectPtr conn, virDomainRunningReason reason) { int rc = -1, r; - unsigned int i; - int parentTty; - char *parentTtyPath = NULL; + size_t nttyFDs = 0; + int *ttyFDs = NULL; + size_t i; char *logfile = NULL; int logfd = -1; unsigned int nveths = 0; @@ -1677,26 +1681,34 @@ static int lxcVmStart(virConnectPtr conn, return -1; } - /* open parent tty */ - if (virFileOpenTty(&parentTty, &parentTtyPath, 1) < 0) { - virReportSystemError(errno, "%s", - _("Failed to allocate tty")); + /* Here we open all the PTYs we need on the host OS side. + * The LXC controller will open the guest OS side PTYs + * forward I/O between them. + */ + nttyFDs = vm->def->nconsoles; + if (VIR_ALLOC_N(ttyFDs, nttyFDs) < 0) { + virReportOOMError(); goto cleanup; } - if (vm->def->nconsoles) { - if (vm->def->nconsoles > 1) { + for (i = 0 ; i < vm->def->nconsoles ; i++) + ttyFDs[i] = -1; + + for (i = 0 ; i < vm->def->nconsoles ; i++) { + char *ttyPath; + if (vm->def->consoles[i]->source.type != VIR_DOMAIN_CHR_TYPE_PTY) { lxcError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only one console supported")); + _("Only PTY console types are supported")); goto cleanup; } - if (vm->def->consoles[0]->source.type == VIR_DOMAIN_CHR_TYPE_PTY) { - VIR_FREE(vm->def->consoles[0]->source.data.file.path); - vm->def->consoles[0]->source.data.file.path = parentTtyPath; - } else { - VIR_FREE(parentTtyPath); + + if (virFileOpenTty(&ttyFDs[i], &ttyPath, 1) < 0) { + virReportSystemError(errno, "%s", + _("Failed to allocate tty")); + goto cleanup; } - } else { - VIR_FREE(parentTtyPath); + + VIR_FREE(vm->def->consoles[i]->source.data.file.path); + vm->def->consoles[i]->source.data.file.path = ttyPath; } if (lxcSetupInterfaces(conn, vm->def, &nveths, &veths) != 0) @@ -1723,7 +1735,8 @@ static int lxcVmStart(virConnectPtr conn, if (!(cmd = lxcBuildControllerCmd(driver, vm, nveths, veths, - parentTty, logfd, handshakefds[1]))) + ttyFDs, nttyFDs, + logfd, handshakefds[1]))) goto cleanup; /* Log timestamp */ @@ -1825,7 +1838,8 @@ cleanup: VIR_FORCE_CLOSE(priv->monitor); virDomainConfVMNWFilterTeardown(vm); } - VIR_FORCE_CLOSE(parentTty); + for (i = 0 ; i < nttyFDs ; i++) + VIR_FORCE_CLOSE(ttyFDs[i]); VIR_FORCE_CLOSE(handshakefds[0]); VIR_FORCE_CLOSE(handshakefds[1]); VIR_FREE(logfile); -- 1.7.6.4
On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
Currently the LXC controller only supports setup of a single text console. This is wired up to the container init's stdio, as well as /dev/console and /dev/tty1. Extending support for multiple consoles, means wiring up additional PTYs to /dev/tty2, /dev/tty3, etc, etc. The LXC controller is passed multiple open file handles, one for each console requested.
* src/lxc/lxc_container.c, src/lxc/lxc_container.h: Wire up all the /dev/ttyN links required to symlink to /dev/pts/NN * src/lxc/lxc_container.h: Open more container side /dev/pts/NN devices, and adapt event loop to handle I/O from all consoles * src/lxc/lxc_driver.c: Setup multiple host side PTYs --- src/lxc/lxc_container.c | 81 +++++++++++++++--------- src/lxc/lxc_container.h | 3 +- src/lxc/lxc_controller.c | 160 +++++++++++++++++++++++++++++----------------- src/lxc/lxc_driver.c | 62 +++++++++++------- 4 files changed, 191 insertions(+), 115 deletions(-)
@@ -891,10 +900,9 @@ cleanup: virMutexDestroy(&lock); signal(SIGCHLD, SIG_DFL); cleanup2: - VIR_FORCE_CLOSE(console.hostFd); - VIR_FORCE_CLOSE(console.contFd); VIR_FORCE_CLOSE(monitor.serverFd); VIR_FORCE_CLOSE(monitor.clientFd); + VIR_FREE(consoles);
The old code was closing all fds; should the new code be iterating over consoles and freeing both fd members per consoles[i]?
@@ -1677,26 +1681,34 @@ static int lxcVmStart(virConnectPtr conn, return -1; }
- /* open parent tty */ - if (virFileOpenTty(&parentTty,&parentTtyPath, 1)< 0) { - virReportSystemError(errno, "%s", - _("Failed to allocate tty")); + /* Here we open all the PTYs we need on the host OS side. + * The LXC controller will open the guest OS side PTYs + * forward I/O between them.
s/forward/and forward/ ACK with those points fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Wed, Nov 02, 2011 at 02:46:50PM -0600, Eric Blake wrote:
On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
Currently the LXC controller only supports setup of a single text console. This is wired up to the container init's stdio, as well as /dev/console and /dev/tty1. Extending support for multiple consoles, means wiring up additional PTYs to /dev/tty2, /dev/tty3, etc, etc. The LXC controller is passed multiple open file handles, one for each console requested.
* src/lxc/lxc_container.c, src/lxc/lxc_container.h: Wire up all the /dev/ttyN links required to symlink to /dev/pts/NN * src/lxc/lxc_container.h: Open more container side /dev/pts/NN devices, and adapt event loop to handle I/O from all consoles * src/lxc/lxc_driver.c: Setup multiple host side PTYs --- src/lxc/lxc_container.c | 81 +++++++++++++++--------- src/lxc/lxc_container.h | 3 +- src/lxc/lxc_controller.c | 160 +++++++++++++++++++++++++++++----------------- src/lxc/lxc_driver.c | 62 +++++++++++------- 4 files changed, 191 insertions(+), 115 deletions(-)
@@ -891,10 +900,9 @@ cleanup: virMutexDestroy(&lock); signal(SIGCHLD, SIG_DFL); cleanup2: - VIR_FORCE_CLOSE(console.hostFd); - VIR_FORCE_CLOSE(console.contFd); VIR_FORCE_CLOSE(monitor.serverFd); VIR_FORCE_CLOSE(monitor.clientFd); + VIR_FREE(consoles);
The old code was closing all fds; should the new code be iterating over consoles and freeing both fd members per consoles[i]?
These FDs were actually passed into this method, so I decided it was better practice for the caller to close them instead, since it opened them. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: "Daniel P. Berrange" <berrange@redhat.com> When no <target> element was set at all, the default console target type was not being honoured * src/conf/domain_conf.c: Set default target type for consoles with no <target> --- src/conf/domain_conf.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f90bece..a4d91ed 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3913,6 +3913,9 @@ virDomainChrDefParseXML(virCapsPtr caps, nodeName); } + /* Initialize this now, in case there is no actual 'target' element at all */ + def->targetType = virDomainChrDefaultTargetType(caps, def->deviceType); + cur = node->children; remaining = virDomainChrSourceDefParseXML(&def->source, cur, flags); if (remaining < 0) -- 1.7.6.4
On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
When no<target> element was set at all, the default console target type was not being honoured
* src/conf/domain_conf.c: Set default target type for consoles with no<target> --- src/conf/domain_conf.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f90bece..a4d91ed 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3913,6 +3913,9 @@ virDomainChrDefParseXML(virCapsPtr caps, nodeName); }
+ /* Initialize this now, in case there is no actual 'target' element at all */ + def->targetType = virDomainChrDefaultTargetType(caps, def->deviceType); +
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> To allow virDomainOpenConsole to access non-primary consoles, device aliases are required to be set. Until now only the QEMU driver has done this. Update LXC & UML to set aliases for any console devices * src/lxc/lxc_driver.c, src/uml/uml_driver.c: Set aliases for console devices --- src/lxc/lxc_driver.c | 30 ++++++++++++++++++++++++------ src/uml/uml_driver.c | 41 +++++++++++++++++++++++++++++++++++------ 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 8e02676..037d781 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1681,6 +1681,14 @@ static int lxcVmStart(virConnectPtr conn, return -1; } + /* Do this upfront, so any part of the startup process can add + * runtime state to vm->def that won't be persisted. This let's us + * report implicit runtime defaults in the XML, like vnc listen/socket + */ + VIR_DEBUG("Setting current domain def as transient"); + if (virDomainObjSetDefTransient(driver->caps, vm, true) < 0) + goto cleanup; + /* Here we open all the PTYs we need on the host OS side. * The LXC controller will open the guest OS side PTYs * forward I/O between them. @@ -1709,6 +1717,12 @@ static int lxcVmStart(virConnectPtr conn, VIR_FREE(vm->def->consoles[i]->source.data.file.path); vm->def->consoles[i]->source.data.file.path = ttyPath; + + VIR_FREE(vm->def->consoles[i]->info.alias); + if (virAsprintf(&vm->def->consoles[i]->info.alias, "console%zu", i) < 0) { + virReportOOMError(); + goto cleanup; + } } if (lxcSetupInterfaces(conn, vm->def, &nveths, &veths) != 0) @@ -3020,6 +3034,7 @@ lxcDomainOpenConsole(virDomainPtr dom, char uuidstr[VIR_UUID_STRING_BUFLEN]; int ret = -1; virDomainChrDefPtr chr = NULL; + size_t i; virCheckFlags(0, -1); @@ -3039,10 +3054,12 @@ lxcDomainOpenConsole(virDomainPtr dom, } if (dev_name) { - /* XXX support device aliases in future */ - lxcError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Named device aliases are not supported")); - goto cleanup; + for (i = 0 ; i < vm->def->nconsoles ; i++) { + if (STREQ(vm->def->consoles[i]->info.alias, dev_name)) { + chr = vm->def->consoles[i]; + break; + } + } } else { if (vm->def->nconsoles) chr = vm->def->consoles[0]; @@ -3051,8 +3068,9 @@ lxcDomainOpenConsole(virDomainPtr dom, } if (!chr) { - lxcError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot find default console device")); + lxcError(VIR_ERR_INTERNAL_ERROR, + _("cannot find console device '%s'"), + dev_name ? dev_name : _("default")); goto cleanup; } diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 44529f0..c9fe0ac 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -987,11 +987,12 @@ static int umlStartVMDaemon(virConnectPtr conn, struct uml_driver *driver, virDomainObjPtr vm, bool autoDestroy) { - int ret; + int ret = -1; char *logfile; int logfd = -1; umlDomainObjPrivatePtr priv = vm->privateData; virCommandPtr cmd = NULL; + size_t i; if (virDomainObjIsActive(vm)) { umlReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -1045,6 +1046,16 @@ static int umlStartVMDaemon(virConnectPtr conn, return -1; } + /* Do this upfront, so any part of the startup process can add + * runtime state to vm->def that won't be persisted. This let's us + * report implicit runtime defaults in the XML, like vnc listen/socket + */ + VIR_DEBUG("Setting current domain def as transient"); + if (virDomainObjSetDefTransient(driver->caps, vm, true) < 0) { + VIR_FORCE_CLOSE(logfd); + return -1; + } + if (!(cmd = umlBuildCommandLine(conn, driver, vm))) { VIR_FORCE_CLOSE(logfd); virDomainConfVMNWFilterTeardown(vm); @@ -1052,6 +1063,14 @@ static int umlStartVMDaemon(virConnectPtr conn, return -1; } + for (i = 0 ; i < vm->def->nconsoles ; i++) { + VIR_FREE(vm->def->consoles[i]->info.alias); + if (virAsprintf(&vm->def->consoles[i]->info.alias, "console%zu", i) < 0) { + virReportOOMError(); + goto cleanup; + } + } + virCommandWriteArgLog(cmd, logfd); priv->monitor = -1; @@ -1077,6 +1096,12 @@ cleanup: if (ret < 0) { virDomainConfVMNWFilterTeardown(vm); umlCleanupTapDevices(vm); + if (vm->newDef) { + virDomainDefFree(vm->def); + vm->def = vm->newDef; + vm->def->id = -1; + vm->newDef = NULL; + } } /* NB we don't mark it running here - we do that async @@ -2366,6 +2391,7 @@ umlDomainOpenConsole(virDomainPtr dom, char uuidstr[VIR_UUID_STRING_BUFLEN]; int ret = -1; virDomainChrDefPtr chr = NULL; + size_t i; virCheckFlags(0, -1); @@ -2385,10 +2411,12 @@ umlDomainOpenConsole(virDomainPtr dom, } if (dev_name) { - /* XXX support device aliases in future */ - umlReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Named device aliases are not supported")); - goto cleanup; + for (i = 0 ; i < vm->def->nconsoles ; i++) { + if (STREQ(vm->def->consoles[i]->info.alias, dev_name)) { + chr = vm->def->consoles[i]; + break; + } + } } else { if (vm->def->nconsoles) chr = vm->def->consoles[0]; @@ -2398,7 +2426,8 @@ umlDomainOpenConsole(virDomainPtr dom, if (!chr) { umlReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find character device %s"), dev_name); + _("cannot find console device '%s'"), + dev_name ? dev_name : _("default")); goto cleanup; } -- 1.7.6.4
On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
To allow virDomainOpenConsole to access non-primary consoles, device aliases are required to be set. Until now only the QEMU driver has done this. Update LXC& UML to set aliases for any console devices
* src/lxc/lxc_driver.c, src/uml/uml_driver.c: Set aliases for console devices --- src/lxc/lxc_driver.c | 30 ++++++++++++++++++++++++------ src/uml/uml_driver.c | 41 +++++++++++++++++++++++++++++++++++------ 2 files changed, 59 insertions(+), 12 deletions(-)
+ /* Do this upfront, so any part of the startup process can add
s/upfront/up front/g ACK with that spelling fix. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> The default console type may vary based on the OS type. ie a Xen paravirt guests wants a 'xen' console, while a fullvirt guests wants a 'serial' console. A plain integer default console type in the capabilities does not suffice. Instead introduce a callback that is passed the OS type. * src/conf/capabilities.h: Use a callback for default console type * src/conf/domain_conf.c, src/conf/domain_conf.h: Use callback for default console type. Add missing LXC/OpenVZ console types. * src/esx/esx_driver.c, src/libxl/libxl_conf.c, src/lxc/lxc_conf.c, src/openvz/openvz_conf.c, src/phyp/phyp_driver.c, src/qemu/qemu_capabilities.c, src/uml/uml_conf.c, src/vbox/vbox_tmpl.c, src/vmware/vmware_conf.c, src/xen/xen_hypervisor.c, src/xenapi/xenapi_driver.c: Set default console type callback --- src/conf/capabilities.h | 2 +- src/conf/domain_conf.c | 32 ++++++++++++++++++++++++-------- src/conf/domain_conf.h | 2 ++ src/esx/esx_driver.c | 6 ++++++ src/libxl/libxl_conf.c | 11 ++++++++++- src/lxc/lxc_conf.c | 8 ++++++++ src/openvz/openvz_conf.c | 6 ++++++ src/phyp/phyp_driver.c | 9 +++++++++ src/qemu/qemu_capabilities.c | 8 +++++++- src/uml/uml_conf.c | 9 ++++++++- src/vbox/vbox_tmpl.c | 10 ++++++++++ src/vmware/vmware_conf.c | 9 +++++++++ src/xen/xen_hypervisor.c | 10 +++++++++- src/xenapi/xenapi_driver.c | 12 ++++++++++++ 14 files changed, 121 insertions(+), 13 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index e2fa1d6..dd4a827 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -144,7 +144,7 @@ struct _virCaps { unsigned int emulatorRequired : 1; const char *defaultDiskDriverName; const char *defaultDiskDriverType; - int defaultConsoleTargetType; + int (*defaultConsoleTargetType)(const char *ostype); void *(*privateDataAllocFunc)(void); void (*privateDataFreeFunc)(void *); int (*privateDataXMLFormat)(virBufferPtr, void *); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a4d91ed..c48a224 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -285,7 +285,9 @@ VIR_ENUM_IMPL(virDomainChrConsoleTarget, "serial", "xen", "uml", - "virtio") + "virtio", + "lxc", + "openvz") VIR_ENUM_IMPL(virDomainChrDevice, VIR_DOMAIN_CHR_DEVICE_TYPE_LAST, "parallel", @@ -3473,7 +3475,9 @@ error: } static int -virDomainChrDefaultTargetType(virCapsPtr caps, int devtype) { +virDomainChrDefaultTargetType(virCapsPtr caps, + virDomainDefPtr def, + int devtype) { int target = -1; @@ -3485,7 +3489,12 @@ virDomainChrDefaultTargetType(virCapsPtr caps, int devtype) { break; case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: - target = caps->defaultConsoleTargetType; + if (!caps->defaultConsoleTargetType) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Driver does not have a default console type set")); + return -1; + } + target = caps->defaultConsoleTargetType(def->os.type); break; case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: @@ -3501,6 +3510,7 @@ virDomainChrDefaultTargetType(virCapsPtr caps, int devtype) { static int virDomainChrTargetTypeFromString(virCapsPtr caps, + virDomainDefPtr def, int devtype, const char *targetType) { @@ -3508,7 +3518,7 @@ virDomainChrTargetTypeFromString(virCapsPtr caps, int target = 0; if (!targetType) { - target = virDomainChrDefaultTargetType(caps, devtype); + target = virDomainChrDefaultTargetType(caps, def, devtype); goto out; } @@ -3535,6 +3545,7 @@ out: static int virDomainChrDefParseTargetXML(virCapsPtr caps, + virDomainDefPtr vmdef, virDomainChrDefPtr def, xmlNodePtr cur) { @@ -3545,8 +3556,8 @@ virDomainChrDefParseTargetXML(virCapsPtr caps, const char *portStr = NULL; if ((def->targetType = - virDomainChrTargetTypeFromString(caps, - def->deviceType, targetType)) < 0) { + virDomainChrTargetTypeFromString(caps, vmdef, + def->deviceType, targetType)) < 0) { goto error; } @@ -3884,6 +3895,7 @@ virDomainChrDefNew(void) { */ static virDomainChrDefPtr virDomainChrDefParseXML(virCapsPtr caps, + virDomainDefPtr vmdef, xmlNodePtr node, unsigned int flags) { @@ -3914,7 +3926,7 @@ virDomainChrDefParseXML(virCapsPtr caps, } /* Initialize this now, in case there is no actual 'target' element at all */ - def->targetType = virDomainChrDefaultTargetType(caps, def->deviceType); + def->targetType = virDomainChrDefaultTargetType(caps, vmdef, def->deviceType); cur = node->children; remaining = virDomainChrSourceDefParseXML(&def->source, cur, flags); @@ -3924,7 +3936,7 @@ virDomainChrDefParseXML(virCapsPtr caps, while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { if (xmlStrEqual(cur->name, BAD_CAST "target")) { - if (virDomainChrDefParseTargetXML(caps, def, cur) < 0) { + if (virDomainChrDefParseTargetXML(caps, vmdef, def, cur) < 0) { goto error; } } @@ -7020,6 +7032,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, for (i = 0 ; i < n ; i++) { virDomainChrDefPtr chr = virDomainChrDefParseXML(caps, + def, nodes[i], flags); if (!chr) @@ -7046,6 +7059,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, for (i = 0 ; i < n ; i++) { virDomainChrDefPtr chr = virDomainChrDefParseXML(caps, + def, nodes[i], flags); if (!chr) @@ -7074,6 +7088,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, for (i = 0 ; i < n ; i++) { virDomainChrDefPtr chr = virDomainChrDefParseXML(caps, + def, nodes[i], flags); if (!chr) @@ -7138,6 +7153,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, for (i = 0 ; i < n ; i++) { virDomainChrDefPtr chr = virDomainChrDefParseXML(caps, + def, nodes[i], flags); if (!chr) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a78b3ab..4ad860c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -554,6 +554,8 @@ enum virDomainChrConsoleTargetType { VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO, + VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC, + VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST, }; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 41086ef..33e8e7d 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -588,6 +588,11 @@ esxLookupHostSystemBiosUuid(esxPrivate *priv, unsigned char *uuid) } +static int esxDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) +{ + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; +} + static virCapsPtr esxCapsInit(esxPrivate *priv) @@ -615,6 +620,7 @@ esxCapsInit(esxPrivate *priv) virCapabilitiesAddHostMigrateTransport(caps, "vpxmigr"); caps->hasWideScsiBus = true; + caps->defaultConsoleTargetType = esxDefaultConsoleType; if (esxLookupHostSystemBiosUuid(priv, caps->host.host_uuid) < 0) { goto failure; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index b9bce14..e94e06b 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -114,6 +114,15 @@ libxlNextFreeVncPort(libxlDriverPrivatePtr driver, int startPort) return -1; } + +static int libxlDefaultConsoleType(const char *ostype) +{ + if (STREQ(ostype, "hvm")) + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; + else + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; +} + static virCapsPtr libxlBuildCapabilities(const char *hostmachine, int host_pae, @@ -206,7 +215,7 @@ libxlBuildCapabilities(const char *hostmachine, } } - caps->defaultConsoleTargetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; + caps->defaultConsoleTargetType = libxlDefaultConsoleType; return caps; diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index b2586eb..52e99ca 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -40,6 +40,12 @@ #define VIR_FROM_THIS VIR_FROM_LXC +static int lxcDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) +{ + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC; +} + + /* Functions */ virCapsPtr lxcCapsInit(void) { @@ -54,6 +60,8 @@ virCapsPtr lxcCapsInit(void) 0, 0)) == NULL) goto error; + caps->defaultConsoleTargetType = lxcDefaultConsoleType; + /* Some machines have problematic NUMA toplogy causing * unexpected failures. We don't want to break the QEMU * driver in this scenario, so log errors & carry on diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index c60a97f..c0517db 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -129,6 +129,11 @@ int openvzExtractVersion(struct openvz_driver *driver) } +static int openvzDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) +{ + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ; +} + virCapsPtr openvzCapsInit(void) { struct utsname utsname; @@ -165,6 +170,7 @@ virCapsPtr openvzCapsInit(void) goto no_memory; caps->defaultInitPath = "/sbin/init"; + caps->defaultConsoleTargetType = openvzDefaultConsoleType; return caps; no_memory: diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index ff16aae..5873624 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -291,6 +291,13 @@ phypGetVIOSPartitionID(virConnectPtr conn) return id; } + +static int phypDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) +{ + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; +} + + static virCapsPtr phypCapsInit(void) { @@ -328,6 +335,8 @@ phypCapsInit(void) "phyp", NULL, NULL, 0, NULL) == NULL) goto no_memory; + caps->defaultConsoleTargetType = phypDefaultConsoleType; + return caps; no_memory: diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5f0356c..37990ec 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -807,6 +807,12 @@ error: } +static int qemuDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) +{ + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; +} + + virCapsPtr qemuCapsInit(virCapsPtr old_caps) { struct utsname utsname; @@ -874,7 +880,7 @@ virCapsPtr qemuCapsInit(virCapsPtr old_caps) /* QEMU Requires an emulator in the XML */ virCapabilitiesSetEmulatorRequired(caps); - caps->defaultConsoleTargetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; + caps->defaultConsoleTargetType = qemuDefaultConsoleType; return caps; diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 4459a2a..f2bdd74 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -54,6 +54,13 @@ #define umlLog(level, msg, ...) \ virLogMessage(__FILE__, level, 0, msg, __VA_ARGS__) + +static int umlDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) +{ + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML; +} + + virCapsPtr umlCapsInit(void) { struct utsname utsname; virCapsPtr caps; @@ -99,7 +106,7 @@ virCapsPtr umlCapsInit(void) { NULL) == NULL) goto error; - caps->defaultConsoleTargetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML; + caps->defaultConsoleTargetType = umlDefaultConsoleType; return caps; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index bc19b63..73ef2de 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -823,6 +823,13 @@ cleanup: return result; } + +static int vboxDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) +{ + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; +} + + static virCapsPtr vboxCapsInit(void) { struct utsname utsname; virCapsPtr caps; @@ -856,6 +863,9 @@ static virCapsPtr vboxCapsInit(void) { 0, NULL) == NULL) goto no_memory; + + caps->defaultConsoleTargetType = vboxDefaultConsoleType; + return caps; no_memory: diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index efefab4..3680ca0 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -49,6 +49,13 @@ vmwareFreeDriver(struct vmware_driver *driver) VIR_FREE(driver); } + +static int vmwareDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) +{ + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; +} + + virCapsPtr vmwareCapsInit(void) { @@ -117,6 +124,8 @@ vmwareCapsInit(void) goto error; } + caps->defaultConsoleTargetType = vmwareDefaultConsoleType; + cleanup: virCPUDefFree(cpu); cpuDataFree(utsname.machine, data); diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 58ae6a3..3054267 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -2277,6 +2277,14 @@ struct guest_arch { }; +static int xenDefaultConsoleType(const char *ostype) +{ + if (STREQ(ostype, "hvm")) + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; + else + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; +} + static virCapsPtr xenHypervisorBuildCapabilities(virConnectPtr conn, const char *hostmachine, @@ -2406,7 +2414,7 @@ xenHypervisorBuildCapabilities(virConnectPtr conn, } - caps->defaultConsoleTargetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; + caps->defaultConsoleTargetType = xenDefaultConsoleType; return caps; diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 3946455..8648aee 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -48,6 +48,16 @@ virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) + +static int xenapiDefaultConsoleType(const char *ostype) +{ + if (STREQ(ostype, "hvm")) + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; + else + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; +} + + /* * getCapsObject * @@ -78,6 +88,8 @@ getCapsObject (void) if (!domain2) goto error_cleanup; + caps->defaultConsoleTargetType = xenapiDefaultConsoleType; + return caps; error_cleanup: -- 1.7.6.4
On 10/20/2011 08:47 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
The default console type may vary based on the OS type. ie a Xen paravirt guests wants a 'xen' console, while a fullvirt guests wants a 'serial' console.
A plain integer default console type in the capabilities does not suffice. Instead introduce a callback that is passed the OS type.
* src/conf/capabilities.h: Use a callback for default console type * src/conf/domain_conf.c, src/conf/domain_conf.h: Use callback for default console type. Add missing LXC/OpenVZ console types. * src/esx/esx_driver.c, src/libxl/libxl_conf.c, src/lxc/lxc_conf.c, src/openvz/openvz_conf.c, src/phyp/phyp_driver.c, src/qemu/qemu_capabilities.c, src/uml/uml_conf.c, src/vbox/vbox_tmpl.c, src/vmware/vmware_conf.c, src/xen/xen_hypervisor.c, src/xenapi/xenapi_driver.c: Set default console type callback
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/10/20 Daniel P. Berrange <berrange@redhat.com>:
From: "Daniel P. Berrange" <berrange@redhat.com>
The default console type may vary based on the OS type. ie a Xen paravirt guests wants a 'xen' console, while a fullvirt guests wants a 'serial' console.
A plain integer default console type in the capabilities does not suffice. Instead introduce a callback that is passed the OS type.
* src/conf/capabilities.h: Use a callback for default console type * src/conf/domain_conf.c, src/conf/domain_conf.h: Use callback for default console type. Add missing LXC/OpenVZ console types. * src/esx/esx_driver.c, src/libxl/libxl_conf.c, src/lxc/lxc_conf.c, src/openvz/openvz_conf.c, src/phyp/phyp_driver.c, src/qemu/qemu_capabilities.c, src/uml/uml_conf.c, src/vbox/vbox_tmpl.c, src/vmware/vmware_conf.c, src/xen/xen_hypervisor.c, src/xenapi/xenapi_driver.c: Set default console type callback ---
With this patch virt-aa-helper-test fails on Ubuntu: FAIL: exited with '1' console: '--dryrun -r -u libvirt-00000000-0000-0000-0000-0123456789ab': FAIL: exited with '1' console (pty): '--dryrun -r -u libvirt-00000000-0000-0000-0000-0123456789ab': FAIL: exited with '1' console (pipe): '--dryrun -r -u libvirt-00000000-0000-0000-0000-0123456789ab': FAIL: virt-aa-helper-test -- Matthias Bolte http://photron.blogspot.com
On Sat, Nov 05, 2011 at 08:58:59PM +0100, Matthias Bolte wrote:
2011/10/20 Daniel P. Berrange <berrange@redhat.com>:
From: "Daniel P. Berrange" <berrange@redhat.com>
The default console type may vary based on the OS type. ie a Xen paravirt guests wants a 'xen' console, while a fullvirt guests wants a 'serial' console.
A plain integer default console type in the capabilities does not suffice. Instead introduce a callback that is passed the OS type.
* src/conf/capabilities.h: Use a callback for default console type * src/conf/domain_conf.c, src/conf/domain_conf.h: Use callback for default console type. Add missing LXC/OpenVZ console types. * src/esx/esx_driver.c, src/libxl/libxl_conf.c, src/lxc/lxc_conf.c, src/openvz/openvz_conf.c, src/phyp/phyp_driver.c, src/qemu/qemu_capabilities.c, src/uml/uml_conf.c, src/vbox/vbox_tmpl.c, src/vmware/vmware_conf.c, src/xen/xen_hypervisor.c, src/xenapi/xenapi_driver.c: Set default console type callback ---
With this patch virt-aa-helper-test fails on Ubuntu:
FAIL: exited with '1' console: '--dryrun -r -u libvirt-00000000-0000-0000-0000-0123456789ab': FAIL: exited with '1' console (pty): '--dryrun -r -u libvirt-00000000-0000-0000-0000-0123456789ab': FAIL: exited with '1' console (pipe): '--dryrun -r -u libvirt-00000000-0000-0000-0000-0123456789ab': FAIL: virt-aa-helper-test
I've just sent a patch which ought to solve this Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Matthias Bolte