[libvirt] [PATCH v3 1/4] libxl: support serial list

[Jim Fehlig]

On 08/17/2016 08:20 PM, Bob Liu wrote:
> Add support for multi serial devices, after this patch virsh can be used to
> connect different serial devices of running domains. E.g.
> vish # console <xxx> --devname serial<xxx>
>
> Note:
> This depends on a xen/libxl bug fix to have libxl_console_get_tty(...) correctly
> returning the tty path (as opposed to always returning the first one).
> [0] https://lists.xen.org/archives/html/xen-devel/2016-08/msg00438.html
>
> Signed-off-by: Bob Liu <bob.liu at oracle.com>
> ---
> v3: Comments from Jim.
> v2: Add #ifdef LIBXL_HAVE_BUILDINFO_SERIAL_LIST.

Not that it matters much, but I guess this is actually V4 since Joao had a
review comment on V3 :-).

With the exception of a small nit in patch 2, which I've already fixed in my
branch, V4 looks good to me and works fine in my testing - ACK. But we'll have
to wait until 2.2.0 is released before pushing. Thanks for your patience!

Regards,
Jim

> ---
>  src/libxl/libxl_conf.c   | 23 ++++++++++++++++++++---
>  src/libxl/libxl_domain.c | 29 ++++++++++++++++++++++++++---
>  src/libxl/libxl_driver.c | 17 +++++++++--------
>  3 files changed, 55 insertions(+), 14 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 146e08a..32db975 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -431,14 +431,31 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>          }
>  
>          if (def->nserials) {
> -            if (def->nserials > 1) {
> +            if (def->nserials == 1) {
> +                if (libxlMakeChrdevStr(def->serials[0], &b_info->u.hvm.serial) <
> +                    0)
> +                    return -1;
> +            } else {
> +#ifdef LIBXL_HAVE_BUILDINFO_SERIAL_LIST
> +                if (VIR_ALLOC_N(b_info->u.hvm.serial_list, def->nserials + 1) <
> +                    0)
> +                    return -1;
> +                for (i = 0; i < def->nserials; i++) {
> +                    if (libxlMakeChrdevStr(def->serials[i],
> +                                           &b_info->u.hvm.serial_list[i]) < 0)
> +                    {
> +                        libxl_string_list_dispose(&b_info->u.hvm.serial_list);
> +                        return -1;
> +                    }
> +                }
> +                b_info->u.hvm.serial_list[i] = NULL;
> +#else
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                                 "%s",
>                                 _("Only one serial device is supported by libxl"));
>                  return -1;
> +#endif
>              }
> -            if (libxlMakeChrdevStr(def->serials[0], &b_info->u.hvm.serial) < 0)
> -                return -1;
>          }
>  
>          if (def->nparallels) {
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 0e26b91..f529a2e 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -960,18 +960,20 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback)
>  {
>      virDomainObjPtr vm = for_callback;
>      size_t i;
> +    virDomainChrDefPtr chr;
> +    char *console = NULL;
> +    int ret;
>  
>      virObjectLock(vm);
>      for (i = 0; i < vm->def->nconsoles; i++) {
> -        virDomainChrDefPtr chr = vm->def->consoles[i];
> +        chr = vm->def->consoles[i];
> +
>          if (i == 0 &&
>              chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)
>              chr = vm->def->serials[0];
>  
>          if (chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
>              libxl_console_type console_type;
> -            char *console = NULL;
> -            int ret;
>  
>              console_type =
>                  (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL ?
> @@ -989,6 +991,27 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback)
>              VIR_FREE(console);
>          }
>      }
> +    for (i = 0; i < vm->def->nserials; i++) {
> +        chr = vm->def->serials[i];
> +
> +        ignore_value(virAsprintf(&chr->info.alias, "serial%zd", i));
> +        if (chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
> +            if (chr->source.data.file.path)
> +                continue;
> +            ret = libxl_console_get_tty(ctx, ev->domid,
> +                                        chr->target.port,
> +                                        LIBXL_CONSOLE_TYPE_SERIAL,
> +                                        &console);
> +            if (!ret) {
> +                VIR_FREE(chr->source.data.file.path);
> +                if (console && console[0] != '\0') {
> +                    ignore_value(VIR_STRDUP(chr->source.data.file.path,
> +                                            console));
> +                }
> +            }
> +            VIR_FREE(console);
> +        }
> +    }
>      virObjectUnlock(vm);
>      libxl_event_free(ctx, ev);
>  }
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index f153f69..a34eb02 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -4450,13 +4450,6 @@ libxlDomainOpenConsole(virDomainPtr dom,
>  
>      virCheckFlags(VIR_DOMAIN_CONSOLE_FORCE, -1);
>  
> -    if (dev_name) {
> -        /* XXX support device aliases in future */
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("Named device aliases are not supported"));
> -        goto cleanup;
> -    }
> -
>      if (!(vm = libxlDomObjFromDomain(dom)))
>          goto cleanup;
>  
> @@ -4472,8 +4465,16 @@ libxlDomainOpenConsole(virDomainPtr dom,
>      }
>  
>      priv = vm->privateData;
> +    if (dev_name) {
> +        size_t i;
>  
> -    if (vm->def->nconsoles) {
> +        for (i = 0; !chr && i < vm->def->nserials; i++) {
> +            if (STREQ(dev_name, vm->def->serials[i]->info.alias)) {
> +                chr = vm->def->serials[i];
> +                break;
> +            }
> +        }
> +    } else if (vm->def->nconsoles) {
>          chr = vm->def->consoles[0];
>          if (chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)
>              chr = vm->def->serials[0];