Daniel P. Berrange wrote:
> On Fri, Aug 30, 2013 at 03:46:49PM -0600, 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
>> 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 typo
>
>
>> 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"));
>>
>
>
> For the errors here, it is preferrable to use virReportError() which will
> in turn call VIR_ERROR anywway.
>
I'll change these to virReportError(). Is it generally preferable to
use virReportError() over VIR_ERROR? If so, I'll send a followup patch
to replace any remaining uses of VIR_ERROR.
Regards,
Jim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list