On 30.08.2013 23:46, Jim Fehlig wrote:
> Detect early on in libxl driver initialization if the driver
> should be loaded at all, avoiding needless initialization steps
> that only have to be undone later. While at it, move th
s/th/the/
> detection to a helper function to improve readability.
>
> After detecting that the driver should be loaded, subsequent
> failures such as initializing the log stream, allocating libxl
> ctx, etc. should be treated as failure to initialize the driver.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
> src/libxl/libxl_driver.c | 62 +++++++++++++++++++++++++++++++-----------------
> 1 file changed, 40 insertions(+), 22 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index ff4f6be..759fed5 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -775,34 +775,54 @@ libxlStateCleanup(void)
> return 0;
> }
>
> -static int
> -libxlStateInitialize(bool privileged,
> - virStateInhibitCallback callback ATTRIBUTE_UNUSED,
> - void *opaque ATTRIBUTE_UNUSED)
> +static bool
> +libxlDriverShouldLoad(bool privileged)
> {
> - const libxl_version_info *ver_info;
> - char *log_file = NULL;
> + bool ret = false;
> virCommandPtr cmd;
> - int status, ret = 0;
> - unsigned int free_mem;
> - char ebuf[1024];
> + int status;
>
> - /* Disable libxl driver if non-root */
> + /* Don't load if non-root */
> if (!privileged) {
> VIR_INFO("Not running privileged, disabling libxenlight driver");
> - return 0;
> + return ret;
> + }
> +
> + /* Don't load if not running on a Xen control domain (dom0) */
> + if (!virFileExists("/proc/xen/capabilities")) {
> + VIR_INFO("No Xen capabilities detected, probably not running "
> + "in a Xen Dom0. Disabling libxenlight driver");
> +
> + return ret;
> }
>
> - /* Disable driver if legacy xen toolstack (xend) is in use */
> + /* Don't load if legacy xen toolstack (xend) is in use */
Indentation off.
> cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
> if (virCommandRun(cmd, &status) == 0 && status == 0) {
> VIR_INFO("Legacy xen tool stack seems to be in use, disabling "
> "libxenlight driver.");
> - virCommandFree(cmd);
> - return 0;
> + } else {
> + ret = true;
> }
> virCommandFree(cmd);
>
> + return ret;
> +}
> +
> +static int
> +libxlStateInitialize(bool privileged,
> + virStateInhibitCallback callback ATTRIBUTE_UNUSED,
> + void *opaque ATTRIBUTE_UNUSED)
> +{
> + const libxl_version_info *ver_info;
> + char *log_file = NULL;
> + int ret = 0;
> + unsigned int free_mem;
> + char ebuf[1024];
> +
> + if (!libxlDriverShouldLoad(privileged))
> + return 0;
> +
> if (VIR_ALLOC(libxl_driver) < 0)
> return -1;
>
> @@ -883,21 +903,20 @@ libxlStateInitialize(bool privileged,
> libxl_driver->logger =
> (xentoollog_logger *)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0);
> if (!libxl_driver->logger) {
> - VIR_INFO("cannot create logger for libxenlight, disabling driver");
> - goto fail;
> + VIR_ERROR(_("Failed to create logger for libxenlight"));
> + goto error;
> }
>
> if (libxl_ctx_alloc(&libxl_driver->ctx,
> LIBXL_VERSION, 0,
> libxl_driver->logger)) {
> - VIR_INFO("cannot initialize libxenlight context, probably not running "
> - "in a Xen Dom0, disabling driver");
> - goto fail;
> + VIR_ERROR(_("Failed to initialize libxenlight context"));
> + goto error;
> }
>
> if ((ver_info = libxl_get_version_info(libxl_driver->ctx)) == NULL) {
> - VIR_INFO("cannot version information from libxenlight, disabling driver");
> - goto fail;
> + VIR_ERROR(_("Failed to get version information from libxenlight"));
> + goto error;
> }
> libxl_driver->version = (ver_info->xen_version_major * 1000000) +
> (ver_info->xen_version_minor * 1000);
> @@ -956,7 +975,6 @@ libxlStateInitialize(bool privileged,
>
> error:
> ret = -1;
> -fail:
> VIR_FREE(log_file);
> if (libxl_driver)
> libxlDriverUnlock(libxl_driver);
>
ACK with those two nits fixed.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list