[libvirt] libvirt: [PATCH] libxl: Check for control_d string to decide about dom0

[libvirt] libvirt: [PATCH] libxl: Check for control_d string to decide about dom0

[Stefan Bader]

I have been looking into a bug report (see BugLink) which reported
libvirt to fail starting inside a Xen guest. Upon further investigation
I found that some tools that help monitoring Xen guests will mount
xenfs to /proc/xen. This will create a capabilities files there even
if the guest is not dom0. However it will return nothing when reading
from it.

Ian, just to sanity check myself. I looked at the xenfs code and to
me there only seem to be those two outcomes (either "control_d" for
running in dom0 or notrhing if not).

With the following patch applied, libvirt starts up correctly in
the normal guests (with xenfs mounted) without initializing libxl.
And also in dom0 where it still enables the libxl driver (if the
xl toolstack is selected).

-Stefan

>From f11949caca6dfe1a802472a2a6d4fe760115ccc6 Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader@canonical.com>
Date: Wed, 12 Mar 2014 11:37:16 +0100
Subject: [PATCH] libxl: Check for control_d string to decide about dom0

As soon as any guest mounts xenfs to /proc/xen, there is a capabilities
file in that directory. However it returns nothing when reading from it.
Change the test to actually check the contents of the file.

BugLink: http://bugs.launchpad.net/bugs/1248025

Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 src/libxl/libxl_driver.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 65d80a2..844e828 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -944,6 +944,7 @@ libxlDriverShouldLoad(bool privileged)
     bool ret = false;
     virCommandPtr cmd;
     int status;
+    char *output = NULL;
 
     /* Don't load if non-root */
     if (!privileged) {
@@ -951,8 +952,17 @@ libxlDriverShouldLoad(bool privileged)
         return ret;
     }
 
-    /* Don't load if not running on a Xen control domain (dom0) */
-    if (!virFileExists("/proc/xen/capabilities")) {
+    /*
+     * Don't load if not running on a Xen control domain (dom0). It is not
+     * sufficient to check for the file to exist as any guest can mount
+     * xenfs to /proc/xen.
+     */
+    status = virFileReadAll("/proc/xen/capabilities", 10. &output);
+    if (status >= 0) {
+        status = strncmp(output, "control_d", 9);
+    }
+    VIR_FREE(output);
+    if (status) {
         VIR_INFO("No Xen capabilities detected, probably not running "
                  "in a Xen Dom0.  Disabling libxenlight driver");
 
-- 
1.7.9.5

Re: [libvirt] libvirt: [PATCH] libxl: Check for control_d string to decide about dom0

[Daniel P. Berrange]

On Wed, Mar 12, 2014 at 01:03:26PM +0100, Stefan Bader wrote:
> I have been looking into a bug report (see BugLink) which reported
> libvirt to fail starting inside a Xen guest. Upon further investigation
> I found that some tools that help monitoring Xen guests will mount
> xenfs to /proc/xen. This will create a capabilities files there even
> if the guest is not dom0. However it will return nothing when reading
> from it.
> 
> Ian, just to sanity check myself. I looked at the xenfs code and to
> me there only seem to be those two outcomes (either "control_d" for
> running in dom0 or notrhing if not).
> 
> With the following patch applied, libvirt starts up correctly in
> the normal guests (with xenfs mounted) without initializing libxl.
> And also in dom0 where it still enables the libxl driver (if the
> xl toolstack is selected).
> 
> -Stefan
> 
> >From f11949caca6dfe1a802472a2a6d4fe760115ccc6 Mon Sep 17 00:00:00 2001
> From: Stefan Bader <stefan.bader@canonical.com>
> Date: Wed, 12 Mar 2014 11:37:16 +0100
> Subject: [PATCH] libxl: Check for control_d string to decide about dom0
> 
> As soon as any guest mounts xenfs to /proc/xen, there is a capabilities
> file in that directory. However it returns nothing when reading from it.
> Change the test to actually check the contents of the file.
> 
> BugLink: http://bugs.launchpad.net/bugs/1248025
> 
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  src/libxl/libxl_driver.c |   14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 65d80a2..844e828 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -944,6 +944,7 @@ libxlDriverShouldLoad(bool privileged)
>      bool ret = false;
>      virCommandPtr cmd;
>      int status;
> +    char *output = NULL;
>  
>      /* Don't load if non-root */
>      if (!privileged) {
> @@ -951,8 +952,17 @@ libxlDriverShouldLoad(bool privileged)
>          return ret;
>      }
>  
> -    /* Don't load if not running on a Xen control domain (dom0) */
> -    if (!virFileExists("/proc/xen/capabilities")) {
> +    /*
> +     * Don't load if not running on a Xen control domain (dom0). It is not
> +     * sufficient to check for the file to exist as any guest can mount
> +     * xenfs to /proc/xen.
> +     */
> +    status = virFileReadAll("/proc/xen/capabilities", 10. &output);
> +    if (status >= 0) {
> +        status = strncmp(output, "control_d", 9);
> +    }
> +    VIR_FREE(output);
> +    if (status) {
>          VIR_INFO("No Xen capabilities detected, probably not running "
>                   "in a Xen Dom0.  Disabling libxenlight driver");

This looks reasonable to me. I recall that the initscripts for the old
XenD daemon would also check for this file to decide whether to start
or not.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [libvirt] libvirt: [PATCH] libxl: Check for control_d string to decide about dom0

[Ian Campbell]

On Wed, 2014-03-12 at 13:03 +0100, Stefan Bader wrote:
> I have been looking into a bug report (see BugLink) which reported
> libvirt to fail starting inside a Xen guest. Upon further investigation
> I found that some tools that help monitoring Xen guests will mount
> xenfs to /proc/xen. This will create a capabilities files there even
> if the guest is not dom0. However it will return nothing when reading
> from it.

This seems consistent with the xencommons initscript which does:
        # run this script only in dom0:
        # no capabilities file in xenlinux domU kernel
        # empty capabilities file in pv_ops domU kernel
        if test -f /proc/xen/capabilities && \
           ! grep -q "control_d" /proc/xen/capabilities ; then
                exit 0
        fi

> 
> Ian, just to sanity check myself. I looked at the xenfs code and to
> me there only seem to be those two outcomes (either "control_d" for
> running in dom0 or notrhing if not).
> 
> With the following patch applied, libvirt starts up correctly in
> the normal guests (with xenfs mounted) without initializing libxl.
> And also in dom0 where it still enables the libxl driver (if the
> xl toolstack is selected).
> 
> -Stefan
> 
> From f11949caca6dfe1a802472a2a6d4fe760115ccc6 Mon Sep 17 00:00:00 2001
> From: Stefan Bader <stefan.bader@canonical.com>
> Date: Wed, 12 Mar 2014 11:37:16 +0100
> Subject: [PATCH] libxl: Check for control_d string to decide about dom0
> 
> As soon as any guest mounts xenfs to /proc/xen, there is a capabilities
> file in that directory. However it returns nothing when reading from it.
> Change the test to actually check the contents of the file.
> 
> BugLink: http://bugs.launchpad.net/bugs/1248025
> 
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  src/libxl/libxl_driver.c |   14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 65d80a2..844e828 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -944,6 +944,7 @@ libxlDriverShouldLoad(bool privileged)
>      bool ret = false;
>      virCommandPtr cmd;
>      int status;
> +    char *output = NULL;
>  
>      /* Don't load if non-root */
>      if (!privileged) {
> @@ -951,8 +952,17 @@ libxlDriverShouldLoad(bool privileged)
>          return ret;
>      }
>  
> -    /* Don't load if not running on a Xen control domain (dom0) */
> -    if (!virFileExists("/proc/xen/capabilities")) {
> +    /*
> +     * Don't load if not running on a Xen control domain (dom0). It is not
> +     * sufficient to check for the file to exist as any guest can mount
> +     * xenfs to /proc/xen.
> +     */
> +    status = virFileReadAll("/proc/xen/capabilities", 10. &output);

Is this "." supposed to be a ","?

> +    if (status >= 0) {
> +        status = strncmp(output, "control_d", 9);
> +    }
> +    VIR_FREE(output);
> +    if (status) {
>          VIR_INFO("No Xen capabilities detected, probably not running "
>                   "in a Xen Dom0.  Disabling libxenlight driver");
>  

Re: [libvirt] libvirt: [PATCH] libxl: Check for control_d string to decide about dom0

[Stefan Bader]

[-- Attachment #1.1: Type: text/plain, Size: 3434 bytes --]

On 12.03.2014 13:08, Ian Campbell wrote:
> On Wed, 2014-03-12 at 13:03 +0100, Stefan Bader wrote:
>> I have been looking into a bug report (see BugLink) which reported
>> libvirt to fail starting inside a Xen guest. Upon further investigation
>> I found that some tools that help monitoring Xen guests will mount
>> xenfs to /proc/xen. This will create a capabilities files there even
>> if the guest is not dom0. However it will return nothing when reading
>> from it.
> 
> This seems consistent with the xencommons initscript which does:
>         # run this script only in dom0:
>         # no capabilities file in xenlinux domU kernel
>         # empty capabilities file in pv_ops domU kernel
>         if test -f /proc/xen/capabilities && \
>            ! grep -q "control_d" /proc/xen/capabilities ; then
>                 exit 0
>         fi
> 
>>
>> Ian, just to sanity check myself. I looked at the xenfs code and to
>> me there only seem to be those two outcomes (either "control_d" for
>> running in dom0 or notrhing if not).
>>
>> With the following patch applied, libvirt starts up correctly in
>> the normal guests (with xenfs mounted) without initializing libxl.
>> And also in dom0 where it still enables the libxl driver (if the
>> xl toolstack is selected).
>>
>> -Stefan
>>
>> From f11949caca6dfe1a802472a2a6d4fe760115ccc6 Mon Sep 17 00:00:00 2001
>> From: Stefan Bader <stefan.bader@canonical.com>
>> Date: Wed, 12 Mar 2014 11:37:16 +0100
>> Subject: [PATCH] libxl: Check for control_d string to decide about dom0
>>
>> As soon as any guest mounts xenfs to /proc/xen, there is a capabilities
>> file in that directory. However it returns nothing when reading from it.
>> Change the test to actually check the contents of the file.
>>
>> BugLink: http://bugs.launchpad.net/bugs/1248025
>>
>> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
>> ---
>>  src/libxl/libxl_driver.c |   14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 65d80a2..844e828 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -944,6 +944,7 @@ libxlDriverShouldLoad(bool privileged)
>>      bool ret = false;
>>      virCommandPtr cmd;
>>      int status;
>> +    char *output = NULL;
>>  
>>      /* Don't load if non-root */
>>      if (!privileged) {
>> @@ -951,8 +952,17 @@ libxlDriverShouldLoad(bool privileged)
>>          return ret;
>>      }
>>  
>> -    /* Don't load if not running on a Xen control domain (dom0) */
>> -    if (!virFileExists("/proc/xen/capabilities")) {
>> +    /*
>> +     * Don't load if not running on a Xen control domain (dom0). It is not
>> +     * sufficient to check for the file to exist as any guest can mount
>> +     * xenfs to /proc/xen.
>> +     */
>> +    status = virFileReadAll("/proc/xen/capabilities", 10. &output);
> 
> Is this "." supposed to be a ","?

Darn, I thought I had fixed that. Unfortunately in the wrong place. Yes, that
should be a ",". Sorry

-Stefan
> 
>> +    if (status >= 0) {
>> +        status = strncmp(output, "control_d", 9);
>> +    }
>> +    VIR_FREE(output);
>> +    if (status) {
>>          VIR_INFO("No Xen capabilities detected, probably not running "
>>                   "in a Xen Dom0.  Disabling libxenlight driver");
>>  
> 
> 



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [libvirt] [Xen-devel] libvirt: [PATCH] libxl: Check for control_d string to decide about dom0

[Jim Fehlig]

Stefan Bader wrote:
> On 12.03.2014 13:08, Ian Campbell wrote:
>   
>> On Wed, 2014-03-12 at 13:03 +0100, Stefan Bader wrote:
>>     
>>> I have been looking into a bug report (see BugLink) which reported
>>> libvirt to fail starting inside a Xen guest. Upon further investigation
>>> I found that some tools that help monitoring Xen guests will mount
>>> xenfs to /proc/xen. This will create a capabilities files there even
>>> if the guest is not dom0. However it will return nothing when reading
>>> from it.
>>>       
>> This seems consistent with the xencommons initscript which does:
>>         # run this script only in dom0:
>>         # no capabilities file in xenlinux domU kernel
>>         # empty capabilities file in pv_ops domU kernel
>>         if test -f /proc/xen/capabilities && \
>>            ! grep -q "control_d" /proc/xen/capabilities ; then
>>                 exit 0
>>         fi
>>
>>     
>>> Ian, just to sanity check myself. I looked at the xenfs code and to
>>> me there only seem to be those two outcomes (either "control_d" for
>>> running in dom0 or notrhing if not).
>>>
>>> With the following patch applied, libvirt starts up correctly in
>>> the normal guests (with xenfs mounted) without initializing libxl.
>>> And also in dom0 where it still enables the libxl driver (if the
>>> xl toolstack is selected).
>>>
>>> -Stefan
>>>
>>> From f11949caca6dfe1a802472a2a6d4fe760115ccc6 Mon Sep 17 00:00:00 2001
>>> From: Stefan Bader <stefan.bader@canonical.com>
>>> Date: Wed, 12 Mar 2014 11:37:16 +0100
>>> Subject: [PATCH] libxl: Check for control_d string to decide about dom0
>>>
>>> As soon as any guest mounts xenfs to /proc/xen, there is a capabilities
>>> file in that directory. However it returns nothing when reading from it.
>>> Change the test to actually check the contents of the file.
>>>
>>> BugLink: http://bugs.launchpad.net/bugs/1248025
>>>
>>> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
>>> ---
>>>  src/libxl/libxl_driver.c |   14 ++++++++++++--
>>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>>> index 65d80a2..844e828 100644
>>> --- a/src/libxl/libxl_driver.c
>>> +++ b/src/libxl/libxl_driver.c
>>> @@ -944,6 +944,7 @@ libxlDriverShouldLoad(bool privileged)
>>>      bool ret = false;
>>>      virCommandPtr cmd;
>>>      int status;
>>> +    char *output = NULL;
>>>  
>>>      /* Don't load if non-root */
>>>      if (!privileged) {
>>> @@ -951,8 +952,17 @@ libxlDriverShouldLoad(bool privileged)
>>>          return ret;
>>>      }
>>>  
>>> -    /* Don't load if not running on a Xen control domain (dom0) */
>>> -    if (!virFileExists("/proc/xen/capabilities")) {
>>> +    /*
>>> +     * Don't load if not running on a Xen control domain (dom0). It is not
>>> +     * sufficient to check for the file to exist as any guest can mount
>>> +     * xenfs to /proc/xen.
>>> +     */
>>> +    status = virFileReadAll("/proc/xen/capabilities", 10. &output);
>>>       
>> Is this "." supposed to be a ","?
>>     
>
> Darn, I thought I had fixed that. Unfortunately in the wrong place. Yes, that
> should be a ",". Sorry
>   

I fixed your typo and pushed the patch.  Thanks!

Regards,
Jim