From patchwork Tue Jan 26 14:38:46 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Campbell X-Patchwork-Id: 8123691 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 09E92BEEE5 for ; Tue, 26 Jan 2016 14:41:44 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0B97A2026D for ; Tue, 26 Jan 2016 14:41:43 +0000 (UTC) Received: from lists.xen.org (lists.xenproject.org [50.57.142.19]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E708C201F2 for ; Tue, 26 Jan 2016 14:41:41 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xen.org) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aO4lm-0004oi-Jq; Tue, 26 Jan 2016 14:38:58 +0000 Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aO4lk-0004oY-K9 for xen-devel@lists.xen.org; Tue, 26 Jan 2016 14:38:56 +0000 Received: from [85.158.143.35] by server-3.bemta-4.messagelabs.com id DF/E2-31122-FF487A65; Tue, 26 Jan 2016 14:38:55 +0000 X-Env-Sender: prvs=8260ee218=Ian.Campbell@citrix.com X-Msg-Ref: server-4.tower-21.messagelabs.com!1453819131!12166980!1 X-Originating-IP: [66.165.176.89] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni44OSA9PiAyMDMwMDc=\n, received_headers: No Received headers X-StarScan-Received: X-StarScan-Version: 7.35.1; banners=-,-,- X-VirusChecked: Checked Received: (qmail 12660 invoked from network); 26 Jan 2016 14:38:54 -0000 Received: from smtp.citrix.com (HELO SMTP.CITRIX.COM) (66.165.176.89) by server-4.tower-21.messagelabs.com with RC4-SHA encrypted SMTP; 26 Jan 2016 14:38:54 -0000 X-IronPort-AV: E=Sophos;i="5.22,350,1449532800"; d="scan'208";a="327701318" From: Ian Campbell To: , , Date: Tue, 26 Jan 2016 14:38:46 +0000 Message-ID: <1453819126-10218-1-git-send-email-ian.campbell@citrix.com> X-Mailer: git-send-email 2.1.4 MIME-Version: 1.0 X-DLP: MIA2 Cc: Ian Campbell , suse.dev@fea.st Subject: [Xen-devel] [PATCH] tools/libxl: improve logging on domain create failure. X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP A user reported[0] that xl create failed with just: libxl: error: libxl_create.c:892:initiate_domain_create: Unable to set domain build info defaults and some resulting fallout, but without indicating why it was unable to set the defaults, even in verbose mode[1]. Go through libxl__domain_{create,build}_info_setdefault and ensure that each error path logs something. In most cases this involved simply adding a call to LOG. In two cases this involved switching from strdup to libxl__strdup(NOGC) and removing the existing error handling. When switching from qemu-xen to qemu-xen-traditional (because the former is not available) log at level INFO rather than VERBOSE, so the message would normally be printed. Also tweak the language here. I'm not sure all these messages are reachable (some might be shadowed by previous error paths) but it seems better to err on the side of caution. [0] http://lists.xen.org/archives/html/xen-users/2016-01/msg00125.html [1] http://lists.xen.org/archives/html/xen-users/2016-01/msg00129.html Signed-off-by: Ian Campbell Cc: suse.dev@fea.st Acked-by: Wei Liu --- suse.dev, this might help diagnose the issue you are seeing. Given the usability issue, I think this ought to be backported. --- tools/libxl/libxl_create.c | 48 +++++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index e491d83..de5d27f 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -30,8 +30,10 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc, libxl_domain_create_info *c_info) { - if (!c_info->type) + if (!c_info->type) { + LOG(ERROR, "domain type unspecified"); return ERROR_INVAL; + } if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { libxl_defbool_setdefault(&c_info->hap, true); @@ -66,8 +68,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, int i; if (b_info->type != LIBXL_DOMAIN_TYPE_HVM && - b_info->type != LIBXL_DOMAIN_TYPE_PV) + b_info->type != LIBXL_DOMAIN_TYPE_PV) { + LOG(ERROR, "invalid domain type"); return ERROR_INVAL; + } libxl_defbool_setdefault(&b_info->device_model_stubdomain, false); @@ -97,8 +101,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, if (rc < 0) { /* qemu-xen unavailable, use qemu-xen-traditional */ if (errno == ENOENT) { - LOGE(VERBOSE, "qemu-xen is unavailable" - ", use qemu-xen-traditional instead"); + LOGE(INFO, "qemu-xen is unavailable" + ", using qemu-xen-traditional instead"); b_info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL; } else { @@ -121,18 +125,24 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, b_info->u.hvm.bios = LIBXL_BIOS_TYPE_SEABIOS; break; case LIBXL_DEVICE_MODEL_VERSION_NONE: break; - default:return ERROR_INVAL; + default: + LOG(ERROR, "unknown device model version"); + return ERROR_INVAL; } /* Enforce BIOS<->Device Model version relationship */ switch (b_info->device_model_version) { case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: - if (b_info->u.hvm.bios != LIBXL_BIOS_TYPE_ROMBIOS) + if (b_info->u.hvm.bios != LIBXL_BIOS_TYPE_ROMBIOS) { + LOG(ERROR, "qemu-xen-traditional requires bios=rombios."); return ERROR_INVAL; + } break; case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN: - if (b_info->u.hvm.bios == LIBXL_BIOS_TYPE_ROMBIOS) + if (b_info->u.hvm.bios == LIBXL_BIOS_TYPE_ROMBIOS) { + LOG(ERROR, "qemu-xen does not support bios=rombios."); return ERROR_INVAL; + } break; case LIBXL_DEVICE_MODEL_VERSION_NONE: break; @@ -160,19 +170,25 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, if (!b_info->max_vcpus) b_info->max_vcpus = 1; if (!b_info->avail_vcpus.size) { - if (libxl_cpu_bitmap_alloc(CTX, &b_info->avail_vcpus, 1)) + if (libxl_cpu_bitmap_alloc(CTX, &b_info->avail_vcpus, 1)) { + LOG(ERROR, "unable to allocate avail_vcpus bitmap"); return ERROR_FAIL; + } libxl_bitmap_set(&b_info->avail_vcpus, 0); - } else if (b_info->avail_vcpus.size > HVM_MAX_VCPUS) + } else if (b_info->avail_vcpus.size > HVM_MAX_VCPUS) { + LOG(ERROR, "avail_vcpus bitmap contains too many VCPUS"); return ERROR_FAIL; + } /* In libxl internals, we want to deal with vcpu_hard_affinity only! */ if (b_info->cpumap.size && !b_info->num_vcpu_hard_affinity) { b_info->vcpu_hard_affinity = libxl__calloc(gc, b_info->max_vcpus, sizeof(libxl_bitmap)); for (i = 0; i < b_info->max_vcpus; i++) { - if (libxl_cpu_bitmap_alloc(CTX, &b_info->vcpu_hard_affinity[i], 0)) + if (libxl_cpu_bitmap_alloc(CTX, &b_info->vcpu_hard_affinity[i], 0)) { + LOG(ERROR, "failed to allocate vcpu hard affinity bitmap"); return ERROR_FAIL; + } libxl_bitmap_copy(CTX, &b_info->vcpu_hard_affinity[i], &b_info->cpumap); } @@ -318,18 +334,14 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, return ERROR_INVAL; } - if (!b_info->u.hvm.boot) { - b_info->u.hvm.boot = strdup("cda"); - if (!b_info->u.hvm.boot) return ERROR_NOMEM; - } + if (!b_info->u.hvm.boot) + b_info->u.hvm.boot = libxl__strdup(NOGC, "cda"); libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true); if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) { libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true); - if (!b_info->u.hvm.vnc.listen) { - b_info->u.hvm.vnc.listen = strdup("127.0.0.1"); - if (!b_info->u.hvm.vnc.listen) return ERROR_NOMEM; - } + if (!b_info->u.hvm.vnc.listen) + b_info->u.hvm.vnc.listen = libxl__strdup(NOGC, "127.0.0.1"); } libxl_defbool_setdefault(&b_info->u.hvm.sdl.enable, false);