[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2] libxl: load on FreeBSD



On Thu, May 28, 2015 at 11:11:06AM +0400, Roman Bogorodskiy wrote:
 Jim Fehlig wrote:

On 05/27/2015 09:06 AM, Martin Kletzander wrote:
> On Sun, May 24, 2015 at 06:45:02PM +0300, Roman Bogorodskiy wrote:
>> The libxl tries to check if it's running in dom0 by parsing
>> /proc/xen/capabilities and if that fails it doesn't load.
>>
>> There's no procfs interface in Xen on FreeBSD, so this check always
>> fails.
>>
>> In addition to checking procfs, check if /dev/xen/xenstored, that's enough to
>> check if we're running in dom0 in FreeBSD case.
>> ---
>> src/libxl/libxl_driver.c | 42 ++++++++++++++++++++++--------------------
>> 1 file changed, 22 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 12be816..fddafa1 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -74,6 +74,7 @@ VIR_LOG_INIT("libxl.libxl_driver");
>> #define LIBXL_CONFIG_FORMAT_SEXPR "xen-sxpr"
>>
>> #define HYPERVISOR_CAPABILITIES "/proc/xen/capabilities"
>> +#define HYPERVISOR_XENSTORED "/dev/xen/xenstored"
>>
>> /* Number of Xen scheduler parameters */
>> #define XEN_SCHED_CREDIT_NPARAM   2
>> @@ -427,8 +428,6 @@ static bool
>> libxlDriverShouldLoad(bool privileged)
>> {
>>     bool ret = false;
>> -    int status;
>> -    char *output = NULL;
>>
>>     /* Don't load if non-root */
>>     if (!privileged) {
>> @@ -436,24 +435,27 @@ libxlDriverShouldLoad(bool privileged)
>>         return ret;
>>     }
>>
>> -    if (!virFileExists(HYPERVISOR_CAPABILITIES)) {
>> -        VIR_INFO("Disabling driver as " HYPERVISOR_CAPABILITIES
>> -                 " does not exist");
>> -        return ret;
>> -    }
>> -    /*
>> -     * 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(HYPERVISOR_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");
>> -
>> +    if (virFileExists(HYPERVISOR_CAPABILITIES)) {
>> +        int status;
>> +        char *output = NULL;
>> +        /*
>> +         * 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(HYPERVISOR_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");
>> +
>> +            return ret;
>> +        }
>> +    } else if (!virFileExists(HYPERVISOR_XENSTORED)) {
>> +        VIR_INFO("Disabling driver as neither " HYPERVISOR_CAPABILITIES
>> +                 " nor " HYPERVISOR_CAPABILITIES " exist");
>
> s/HYPERVISOR_CAPABILITIES/HYPERVISOR_XENSTORED/
>
> ACK with that changed.

For the record, I tested this on Linux.  Looks good with Martin's comment addressed.

Regards,
Jim

Thanks for giving it a test!

Is this safe to push during the freeze?


I also tried it before ACKing on XEN-enabled linux (made sure there is
no /dev/xen/xenstored so it doesn't collide), so I'd say it's fine
with me.

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]