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

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?