Re: [libvirt] [PATCH] Add support for multiple serial ports to Xen driver

On Thu, Jan 20, 2011 at 01:20:50PM +0100, Michal Novotny wrote: > Hi, > this is the patch for to support multiple serial ports > for Xen driver. The definition for Xen has been > introduced in BZ #614004 and this is adding > support to libvirt-based tools. > > The patch was originally written for RHEL-5 and libvirt > 0.8.2 (RHEL-5.6) where it has been tested using > the virsh command and correct device creation has > been confirmed in the xend.log to have the same data > for serial ports using both `xm create` and `virsh > start` commands. Also, domains with both single and > multiple serial ports has been tested to confirm > it won't introduce any regression and everything > was working fine according to my testing. This patch > is the forward-port of RHEL-5 version of the patch. > > Michal > > Signed-off-by: Michal Novotny&lt;minovotn(a)redhat.com&gt; > --- > src/xen/xend_internal.c | 19 ++++++++++++- > src/xen/xm_internal.c | 66 +++++++++++++++++++++++++++++++++++++++-------- > 3 files changed, 73 insertions(+), 14 deletions(-) > > diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c > index 44d5a22..35cdd3c 100644 > --- a/src/xen/xend_internal.c > +++ b/src/xen/xend_internal.c > @@ -5959,8 +5959,23 @@ xenDaemonFormatSxpr(virConnectPtr conn, > } > if (def->serials) { > virBufferAddLit(&buf, "(serial "); > - if (xenDaemonFormatSxprChr(def->serials[0],&buf)< 0) > - goto error; > + /* > + * If domain doesn't have multiple serial ports defined we > + * keep the old-style formatting not to fail the sexpr tests > + */ > + if (def->nserials> 1) { > + for (i = 0; i< def->nserials; i++) { > + virBufferAddLit(&buf, "("); > + if (xenDaemonFormatSxprChr(def->serials[i],&buf)< 0) > + goto error; > + virBufferAddLit(&buf, ")"); > + } > + } > + else { > + if (xenDaemonFormatSxprChr(def->serials[0],&buf)< 0) > + goto error; > + } > + > virBufferAddLit(&buf, ")"); > } else { > virBufferAddLit(&buf, "(serial none)"); > diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c > index bfb6698..1bb62d7 100644 > --- a/src/xen/xm_internal.c > +++ b/src/xen/xm_internal.c > @@ -1432,20 +1432,64 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { > chr = NULL; > } > > - if (xenXMConfigGetString(conf, "serial",&str, NULL)< 0) > - goto cleanup; > - if (str&& STRNEQ(str, "none")&& > - !(chr = xenDaemonParseSxprChar(str, NULL))) > - goto cleanup; > + /* Try to get the list of values to support multiple serial ports */ > + list = virConfGetValue(conf, "serial"); > + if (list&& list->type == VIR_CONF_LIST) { > + list = list->list; > + while (list) { > + char *port; > + > + if ((list->type != VIR_CONF_STRING) || (list->str == NULL)) > + goto skipport; > + > + port = list->str; > + if (VIR_ALLOC(chr)< 0) > + goto no_memory; > > - if (chr) { > - if (VIR_ALLOC_N(def->serials, 1)< 0) { > + if (!(chr = xenDaemonParseSxprChar(port, NULL))) > + goto skipport; > + > + if (VIR_REALLOC_N(def->serials, def->nserials+1)< 0) > + goto no_memory; > + > + chr->targetType = VIR_DOMAIN_CHR_TARGET_TYPE_SERIAL; > + chr->type = VIR_DOMAIN_CHR_TYPE_PTY; > + > + /* Implement device type of serial port when appropriate */ > + if (STRPREFIX(port, "/dev")) { > + chr->type = VIR_DOMAIN_CHR_TYPE_DEV; > + chr->target.port = def->nserials; > + chr->data.file.path = strdup(port); > + > + if (!chr->data.file.path) > + goto no_memory; > + } > + > + def->serials[def->nserials++] = chr; > + chr = NULL; > + > + skipport: > + list = list->next; > virDomainChrDefFree(chr); > - goto no_memory; > } > - chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; > - def->serials[0] = chr; > - def->nserials++; > + } > + /* If domain is not using multiple serial ports we parse data old way */ > + else { > + if (xenXMConfigGetString(conf, "serial",&str, NULL)< 0) > + goto cleanup; > + if (str&& STRNEQ(str, "none")&& > + !(chr = xenDaemonParseSxprChar(str, NULL))) > + goto cleanup; > + > + if (chr) { > + if (VIR_ALLOC_N(def->serials, 1)< 0) { > + virDomainChrDefFree(chr); > + goto no_memory; > + } > + chr->targetType = VIR_DOMAIN_CHR_TARGET_TYPE_SERIAL; > + def->serials[0] = chr; > + def->nserials++; > + } > } > } else { > if (!(def->console = xenDaemonParseSxprChar("pty", NULL))) Hmm, unless I'm missing something, this patch only seems todo half the job. It lets you parse XM configs, or generate SEXPRS, needed to start/create guests. It doesn't let you parse SEXPRS to query XML, or write XM configs to save an updated guest config. Daniel