From patchwork Fri May 13 18:53:54 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cole Robinson X-Patchwork-Id: 67797 Delivered-To: patch@linaro.org Received: by 10.140.92.199 with SMTP id b65csp415276qge; Fri, 13 May 2016 11:58:37 -0700 (PDT) X-Received: by 10.28.127.81 with SMTP id a78mr5299809wmd.34.1463165917208; Fri, 13 May 2016 11:58:37 -0700 (PDT) Return-Path: Received: from mx5-phx2.redhat.com (mx5-phx2.redhat.com. [209.132.183.37]) by mx.google.com with ESMTPS id k125si314998wmk.45.2016.05.13.11.58.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 13 May 2016 11:58:37 -0700 (PDT) Received-SPF: pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.37 as permitted sender) client-ip=209.132.183.37; Authentication-Results: mx.google.com; spf=pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.37 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx5-phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4DIrvNB023537; Fri, 13 May 2016 14:53:58 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u4DIrtv6020213 for ; Fri, 13 May 2016 14:53:55 -0400 Received: from [10.3.113.169] (ovpn-113-169.phx2.redhat.com [10.3.113.169]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4DIrtQa014551; Fri, 13 May 2016 14:53:55 -0400 To: Andrea Bolognani , libvir-list@redhat.com References: <1462884387-12563-1-git-send-email-abologna@redhat.com> <1462884387-12563-2-git-send-email-abologna@redhat.com> <1463068394.3613.67.camel@redhat.com> From: Cole Robinson Message-ID: <939cae58-9b78-09b3-cadd-38ba297c60c0@redhat.com> Date: Fri, 13 May 2016 14:53:54 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <1463068394.3613.67.camel@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-loop: libvir-list@redhat.com Subject: Re: [libvirt] [PATCH 1/4] qemu: Automatically choose usable GIC version X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com On 05/12/2016 11:53 AM, Andrea Bolognani wrote: > On Tue, 2016-05-10 at 18:42 -0400, Cole Robinson wrote: >>> >>> + if (virQEMUCapsFillDomainCaps(caps, qemuCaps, NULL, 0) < 0) >>> + goto cleanup; >>> + >>> + gic = &(caps->gic); >>> + >>> + /* Pick the best GIC version from those available */ >>> + if (gic->supported) { >>> + virGICVersion version; >>> + >>> + VIR_DEBUG("Looking for usable GIC version in domain capabilities"); >>> + for (version = VIR_GIC_VERSION_LAST - 1; >>> + version > VIR_GIC_VERSION_NONE; >>> + version--) { >>> + if (VIR_DOMAIN_CAPS_ENUM_IS_SET(gic->version, version)) { >>> + >>> + VIR_DEBUG("Using GIC version %s", >>> + virGICVersionTypeToString(version)); >>> + def->gic_version = version; >>> + break; >>> + } >>> + } >>> } >> >> Hmm that's a bit of a new pattern... it seems the only thing you really need >> from domcaps is the bit of logic we encode via >> virQEMUCapsFillDomainFeatureGICCaps. Maybe break that logic out into a public >> function and call it here, rather than spinning up domcaps for a small bit of >> info? Or is there more to it? > > Nothing more to it :) > > Do you mean I should make virQEMUCapsFillDomainFeatureGICCaps() > public and use it here to fill only the part of the domain > capabilities I'm actually going to use, or create a new function > altogether? > > Because right now I'm not seeing a way to do the latter without > introducing some code duplication or making things quite a bit > uglier... Maybe I'm just tired :) > Can you break apart the logic like the attached patch, then call the new function from the above code? I didn't try plugging it into your patches but it looks to me like it should work - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 1bddf43..f05efd2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4256,6 +4256,32 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, } +static bool +virQEMUCapsGICSupported(virArch arch, + virDomainVirtType virttype, + const char *machine, + virGICCapabilityPtr cap) +{ + if (arch != VIR_ARCH_ARMV7L && + arch != VIR_ARCH_AARCH64) + return false; + + if (STRNEQ(machine, "virt") && + !STRPREFIX(machine, "virt-")) + return false; + + if (virttype == VIR_DOMAIN_VIRT_KVM && + !(cap->implementation & VIR_GIC_IMPLEMENTATION_KERNEL)) + return false; + + if (virttype == VIR_DOMAIN_VIRT_QEMU && + !(cap->implementation & VIR_GIC_IMPLEMENTATION_EMULATED)) + return false; + + return true; +} + + /** * virQEMUCapsFillDomainFeatureGICCaps: * @qemuCaps: QEMU capabilities @@ -4282,23 +4308,12 @@ virQEMUCapsFillDomainFeatureGICCaps(virQEMUCapsPtr qemuCaps, virDomainCapsFeatureGICPtr gic = &domCaps->gic; size_t i; - if (domCaps->arch != VIR_ARCH_ARMV7L && - domCaps->arch != VIR_ARCH_AARCH64) - return 0; - - if (STRNEQ(domCaps->machine, "virt") && - !STRPREFIX(domCaps->machine, "virt-")) - return 0; - for (i = 0; i < qemuCaps->ngicCapabilities; i++) { virGICCapabilityPtr cap = &qemuCaps->gicCapabilities[i]; - - if (domCaps->virttype == VIR_DOMAIN_VIRT_KVM && - !(cap->implementation & VIR_GIC_IMPLEMENTATION_KERNEL)) - continue; - - if (domCaps->virttype == VIR_DOMAIN_VIRT_QEMU && - !(cap->implementation & VIR_GIC_IMPLEMENTATION_EMULATED)) + if (!virQEMUCapsGICSupported(domCaps->arch, + domCaps->virttype, + domCaps->machine, + cap)) continue; gic->supported = true;