[libvirt] [PATCH RFC] libxl: set serial source path for console type=serial

[Jim Fehlig]

Joao Martins wrote:
> Guests use a <console /> (and sometimes <serial /> pair) to represent
> the console. On the callback that is called when console is brought up
> (NB: before domain starts), we fill the path of the console element with
> the appropriate "/dev/pts/X". For PV guests it all works fine, although
> for HVM guests it doesn't. Consequently we end up seeing erronous
> behaviour on HVM guests where console path is not displayed on the XML
> but still can be accessed with virDomainOpenConsole after booting guest.
> Consumers of this XML (e.g. Openstack) also won't be able to use the
> console path.

Can you provide example input domXML config that causes this problem?

Regards,
Jim

> Finally, the path set in consoles array won't persist
> across daemon reboots, thus rendering admins/users with no access to
> console with a message such as:
> 
> "error: operation failed: PTY device is not yet assigned"
> 
> This is because: for HVM guests, DefFormatInternal will ignore the
> console element and instead write it with the content of the serial
> element for target type = serial which isn't touched in our
> libxlConsoleCallback. To address that we introduce a new helper routine
> libxlConsoleSetSourcePath() that sets the source path and thus also
> handling this case. That is by setting the source path on with serial
> element akin to the one indexed by console "port". This way we keep
> similar behaviour for PV and HVM guests.
> 
> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
> ---
>  src/libxl/libxl_domain.c | 35 +++++++++++++++++++++++++++++++----
>  1 file changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 221af87..4a46143 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -955,6 +955,35 @@ libxlNetworkPrepareDevices(virDomainDefPtr def)
>      return 0;
>  }
>  
> +static int
> +libxlConsoleSetSourcePath(virDomainDefPtr def, virDomainChrDefPtr console,
> +                          char *path)
> +{
> +    int ret = -1;
> +    size_t i;
> +
> +    if (!path || path[0] == '\0')
> +        return ret;
> +
> +    if (VIR_STRDUP(console->source.data.file.path, path) < 0)
> +        return ret;
> +
> +    if (console->targetType != VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)
> +        return 0;
> +
> +    for (i = 0; i < def->nserials; i++) {
> +        virDomainChrDefPtr serial = def->serials[i];
> +
> +        if (serial->target.port == console->target.port &&
> +            VIR_STRDUP(serial->source.data.file.path, path) >= 0) {
> +            ret = 0;
> +            break;
> +        }
> +    }
> +
> +    return ret;
> +}
> +
>  static void
>  libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback)
>  {
> @@ -977,10 +1006,8 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback)
>                                          &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));
> -                }
> +                if (libxlConsoleSetSourcePath(vm->def, chr, console) < 0)
> +                    VIR_WARN("Failed to set console source path");
>              }
>              VIR_FREE(console);
>          }