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

[Daniel P. Berrange]

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 <minovotn at redhat.com>
> ---
>  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