Re: [libvirt] [PATCH 4/5] domain conf: Support multiple <console> devices

On Wed, Jul 14, 2010 at 03:44:55PM -0400, Cole Robinson wrote: > Change console handling to a proper device list, as done for other > character devices. Even though certain drivers can actually handle multiple > consoles, for now just maintain existing behavior where possible. > > Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; [snip] > - chr->target.port = 0; > - /* > - * For HVM console actually created a serial device > - * while for non-HVM it was a parvirt console > - */ > - if (STREQ(def->os.type, "hvm")) { > - if (def->nserials != 0) { > - virDomainChrDefFree(chr); > - } else { > - if (VIR_ALLOC_N(def->serials, 1) < 0) { > + chr->target.port = i; > + > + /* Back compat handling for the first console device */ > + if (i == 0) { > + /* > + * For HVM console actually created a serial device > + * while for non-HVM it was a parvirt console > + */ > + if (STREQ(def->os.type, "hvm")) { > + if (def->nserials != 0) { > virDomainChrDefFree(chr); > - goto no_memory; > + } else { > + if (VIR_ALLOC_N(def->serials, 1) < 0) { > + virDomainChrDefFree(chr); > + goto no_memory; > + } > + def->nserials = 1; > + def->serials[0] = chr; > + chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; > } > - def->nserials = 1; > - def->serials[0] = chr; > - chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; > + } else { > + def->consoles[def->nconsoles++] = chr; > } > } else { > - def->console = chr; > + def->consoles[def->nconsoles++] = chr; > } > } This is where we get into a bit of a mess with back compatability, when combined with the later DefFormat code. The original requirement is that a <console> element generates a <serial> element for HVM guests. In the original code we throw away the virDomainChrDef for the console, since we re-generate it at format time if nconsoles==0. This hack can't work anymore with multiple consoles. Since if we have 2 consoles in the XML and throw away console 0, we have nconsoles==1 during DefFormat and thus won't re-generate the original console 0. To further complicate life, we don't want todo any of this <serial> compat code this at all if console 0 is a virtio console. > @@ -5496,9 +5512,10 @@ virDomainChrDefFormat(virBufferPtr buf, > return -1; > } > > - /* Compat with legacy <console tty='/dev/pts/5'/> syntax */ > virBufferVSprintf(buf, " <%s type='%s'", > elementName, type); > + > + /* Compat with legacy <console tty='/dev/pts/5'/> syntax */ > if (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && > def->type == VIR_DOMAIN_CHR_TYPE_PTY && > !(flags & VIR_DOMAIN_XML_INACTIVE) && Since we're allowing multiple <console> now, we should restrict this hack to just the first one. > @@ -6213,9 +6230,10 @@ char *virDomainDefFormat(virDomainDefPtr def, > goto cleanup; > > /* If there's a PV console that's preferred.. */ > - if (def->console) { > - if (virDomainChrDefFormat(&buf, def->console, flags) < 0) > - goto cleanup; > + if (def->nconsoles) { > + for (n = 0 ; n < def->nconsoles ; n++) > + if (virDomainChrDefFormat(&buf, def->consoles[n], flags) < 0) > + goto cleanup; > } else if (def->nserials != 0) { > /* ..else for legacy compat duplicate the first serial device as a > * console */ This logic can't work anymore. What I think we need todo is to remove the hacks in both the Parse and Format methods. Then add one single hack for the parse method which simply adds a matching <serial> device if nserial==0 and the console device type is serial.