[Xen-devel] libxl: Support PV consoles

Message ID 1398435745-10894-1-git-send-email-ian.campbell@citrix.com
State New
Headers show

Commit Message

Ian Campbell April 25, 2014, 2:22 p.m.
Currently the driver only exposes the ability to connect to the serial console
of a Xen guest, which doesn't work for a PV guest. Instead look for a PV
console first and a serial console second, if an HVM guest has a PV console
then it is a good bet that it is preferred, I think.

Tested with the following bit of config XML:

<domain type='xen'>
  ...
  <devices>
    <console type='pty'>
      <target type='xen'/>
    </console>
  </devices>
</domain>

I have observed and tested this on ARM but I believe it also applies to x86 PV
guests.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Clark Laughlin <clark.laughlin@linaro.org>
---
 src/libxl/libxl_driver.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Daniel P. Berrangé April 25, 2014, 3:15 p.m. | #1
On Fri, Apr 25, 2014 at 03:22:25PM +0100, Ian Campbell wrote:
> Currently the driver only exposes the ability to connect to the serial console
> of a Xen guest, which doesn't work for a PV guest. Instead look for a PV
> console first and a serial console second, if an HVM guest has a PV console
> then it is a good bet that it is preferred, I think.
> 
> Tested with the following bit of config XML:
> 
> <domain type='xen'>
>   ...
>   <devices>
>     <console type='pty'>
>       <target type='xen'/>
>     </console>
>   </devices>
> </domain>
> 
> I have observed and tested this on ARM but I believe it also applies to x86 PV
> guests.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jim Fehlig <jfehlig@suse.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
> Cc: Clark Laughlin <clark.laughlin@linaro.org>
> ---
>  src/libxl/libxl_driver.c |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index a6ae8a1..bcf3595 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -3780,6 +3780,7 @@ libxlDomainOpenConsole(virDomainPtr dom,
>  {
>      virDomainObjPtr vm = NULL;
>      int ret = -1;
> +    libxl_console_type console_type = LIBXL_CONSOLE_TYPE_UNKNOWN;
>      virDomainChrDefPtr chr = NULL;
>      libxlDomainObjPrivatePtr priv;
>      char *console = NULL;
> @@ -3807,8 +3808,15 @@ libxlDomainOpenConsole(virDomainPtr dom,
>  
>      priv = vm->privateData;
>  
> -    if (vm->def->nserials)
> +    if (!chr && vm->def->nconsoles) {
> +        chr = vm->def->consoles[0];
> +        console_type = LIBXL_CONSOLE_TYPE_PV;
> +    }
> +
> +    if (!chr && vm->def->nserials) {
>          chr = vm->def->serials[0];
> +        console_type = LIBXL_CONSOLE_TYPE_SERIAL;
> +    }

I think we can in fact just replace 'nserials' / 'serials' with
'nconsoles' / 'consoles' unconditionally.  If a guest has a
traditional serial port, then this wil be duplicated into the
'def->consoles' array too. So soley looking at def->consoles[0]
should be sufficient.

We can set console_type based on targetType
eg I think this should work for both pv & hvm

    if (vm->def->nconsoles) {
       chr = vm->def->consoles[0];
       console_type = (chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
                       LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
    }
      


Regards,
Daniel
Ian Campbell April 25, 2014, 3:43 p.m. | #2
On Fri, 2014-04-25 at 16:15 +0100, Daniel P. Berrange wrote:
> I think we can in fact just replace 'nserials' / 'serials' with
> 'nconsoles' / 'consoles' unconditionally.  If a guest has a
> traditional serial port, then this wil be duplicated into the
> 'def->consoles' array too. So soley looking at def->consoles[0]
> should be sufficient.

I did vaguely remember something about a thing being duplicated into a
another but not enough of the details to reach this conclusion. Thanks
for the tip.

> We can set console_type based on targetType
> eg I think this should work for both pv & hvm
> 
>     if (vm->def->nconsoles) {
>        chr = vm->def->consoles[0];
>        console_type = (chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
>                        LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
>     }

Yes I expect that will work, I'll check and then send an update patch.
This presumably gets us closer to being able to resolve this too:

    if (dev_name) {
        /* XXX support device aliases in future */
        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                       _("Named device aliases are not supported"));
        goto cleanup;
    }

Ian.

Patch

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a6ae8a1..bcf3595 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3780,6 +3780,7 @@  libxlDomainOpenConsole(virDomainPtr dom,
 {
     virDomainObjPtr vm = NULL;
     int ret = -1;
+    libxl_console_type console_type = LIBXL_CONSOLE_TYPE_UNKNOWN;
     virDomainChrDefPtr chr = NULL;
     libxlDomainObjPrivatePtr priv;
     char *console = NULL;
@@ -3807,8 +3808,15 @@  libxlDomainOpenConsole(virDomainPtr dom,
 
     priv = vm->privateData;
 
-    if (vm->def->nserials)
+    if (!chr && vm->def->nconsoles) {
+        chr = vm->def->consoles[0];
+        console_type = LIBXL_CONSOLE_TYPE_PV;
+    }
+
+    if (!chr && vm->def->nserials) {
         chr = vm->def->serials[0];
+        console_type = LIBXL_CONSOLE_TYPE_SERIAL;
+    }
 
     if (!chr) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -3824,7 +3832,8 @@  libxlDomainOpenConsole(virDomainPtr dom,
         goto cleanup;
     }
 
-    ret = libxl_primary_console_get_tty(priv->ctx, vm->def->id, &console);
+    ret = libxl_console_get_tty(priv->ctx, vm->def->id, chr->target.port,
+                                console_type, &console);
     if (ret)
         goto cleanup;