From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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: See my earlier question about documenting this in docs/formatdomain.html.in. 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). 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. 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(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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 Wed, Nov 02, 2011 at 02:46:50PM -0600, Eric Blake wrote: 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 :|

On 10/20/2011 08:47 AM, Daniel P. Berrange wrote: ACK. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org