Roman Bogorodskiy wrote:
> Martin Kletzander wrote:
>
> > 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.
>
> Last few days were quite busy for me, so I didn't push this and
> decided that it's too late now. I'll wait for the freeze to finish and
> push that, there's no rush. Meanwhile, probably I'll be able to find
> FreeBSD testers for libvirt/libxl to get it more tested for the next
> release.
>
> Thanks,
>
> Roman Bogorodskiy
Pushed.
Roman Bogorodskiy
Attachment:
pgpuYzTMGWVjE.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list