Re: [libvirt] [PATCH 04/40] Simplify opening of Xen drivers

Daniel P. Berrange wrote:

> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c > index 2ecb86f..b7f1ad4 100644 > --- a/src/xen/xen_driver.c > +++ b/src/xen/xen_driver.c > @@ -86,14 +86,9 @@ static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS] = { > [XEN_UNIFIED_XEND_OFFSET] = &xenDaemonDriver, > [XEN_UNIFIED_XS_OFFSET] = &xenStoreDriver, > [XEN_UNIFIED_XM_OFFSET] = &xenXMDriver, > -#if WITH_XEN_INOTIFY > - [XEN_UNIFIED_INOTIFY_OFFSET] = &xenInotifyDriver, > -#endif > Looks like this was never used, so just removing it right? But there are a lot of loops in this file with 'drivers[i]->', where it might be possible that i == XEN_UNIFIED_INOTIFY_OFFSET?

> static int > -xenUnifiedStateInitialize(bool privileged ATTRIBUTE_UNUSED, > +xenUnifiedStateInitialize(bool privileged, > virStateInhibitCallback callback ATTRIBUTE_UNUSED, > void *opaque ATTRIBUTE_UNUSED) > { > - inside_daemon = true; > + /* Don't allow driver to work in non-root libvirtd */ > + if (privileged) > + inside_daemon = true; > Seems the name 'inside_daemon' is no longer appropriate. Should it be something like 'is_privileged'?

> + VIR_DEBUG("Trying XS sub-driver"); > + if (xenStoreOpen(conn, auth, flags) < 0) > + goto error; > + VIR_DEBUG("Activated XS sub-driver"); > + priv->opened[XEN_UNIFIED_XS_OFFSET] = 1; > > xenNumaInit(conn); > > if (!(priv->caps = xenHypervisorMakeCapabilities(conn))) { > - VIR_DEBUG("Failed to make capabilities"); > - goto fail; > + VIR_DEBUG("Errored to make capabilities"); > Maybe one too many instances of 'fail' replaced with 'error'? I think "Failed to make capabilities" is better than "Errored to make capabilities" :).

> + goto error; > } > > if (!(priv->xmlopt = xenDomainXMLConfInit())) > - goto fail; > + goto error; > > #if WITH_XEN_INOTIFY > - if (xenHavePrivilege()) { > - VIR_DEBUG("Trying Xen inotify sub-driver"); > - if (xenInotifyOpen(conn, auth, flags) == VIR_DRV_OPEN_SUCCESS) { > - VIR_DEBUG("Activated Xen inotify sub-driver"); > - priv->opened[XEN_UNIFIED_INOTIFY_OFFSET] = 1; > - } > - } > + VIR_DEBUG("Trying Xen inotify sub-driver"); > + if (xenInotifyOpen(conn, auth, flags) < 0) > + goto error; > The old code silently continued on if xenInotifyOpen() didn't return success.