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

[Jim Fehlig <jfehlig@suse.com>]

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