ping
On Fri, Aug 03, 2018 at 03:28:16PM +0100, Daniel P. Berrangé wrote:
> The virCapabilitiesDomainDataLookupInternal() is given a list of
> parameters representing the desired domain characteristics. It then has
> to look throught the capabilities to identify an acceptable match.
>
> The virCapsDomainDataCompare() method is used for filtering out
> candidates which don't match the desired criteria. It is called
> primarily from the innermost loops and as such is doing many repeated
> checks. For example if architcture and os type were checked at the top
> level loop the two inner loops could be avoided entirely. If emulator
> and domain type were checked in the 2nd level loop the 3rd level loop
> can be avoided too.
>
> This change thus removes the virCapsDomainDataCompare() method and puts
> suitable checks at the start of each loop to ensure it executes the
> minimal number of loop iterations possible. The code becomes clearer to
> understand as a nice side-effect.
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
> src/conf/capabilities.c | 100 ++++++++++++++++++----------------------
> 1 file changed, 45 insertions(+), 55 deletions(-)
>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 0f96500294..cfd5132329 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -604,46 +604,6 @@ virCapabilitiesHostSecModelAddBaseLabel(virCapsHostSecModelPtr secmodel,
> return -1;
> }
>
> -static bool
> -virCapsDomainDataCompare(virCapsGuestPtr guest,
> - virCapsGuestDomainPtr domain,
> - virCapsGuestMachinePtr machine,
> - int ostype,
> - virArch arch,
> - virDomainVirtType domaintype,
> - const char *emulator,
> - const char *machinetype)
> -{
> - const char *check_emulator = NULL;
> -
> - if (ostype != -1 && guest->ostype != ostype)
> - return false;
> - if ((arch != VIR_ARCH_NONE) && (guest->arch.id != arch))
> - return false;
> -
> - if (domaintype != VIR_DOMAIN_VIRT_NONE &&
> - (!domain || domain->type != domaintype))
> - return false;
> -
> - if (emulator) {
> - if (domain)
> - check_emulator = domain->info.emulator;
> - if (!check_emulator)
> - check_emulator = guest->arch.defaultInfo.emulator;
> - if (STRNEQ_NULLABLE(check_emulator, emulator))
> - return false;
> - }
> -
> - if (machinetype) {
> - if (!machine)
> - return false;
> - if (STRNEQ(machine->name, machinetype) &&
> - (STRNEQ_NULLABLE(machine->canonical, machinetype)))
> - return false;
> - }
> -
> - return true;
> -}
>
> static virCapsDomainDataPtr
> virCapabilitiesDomainDataLookupInternal(virCapsPtr caps,
> @@ -659,13 +619,45 @@ virCapabilitiesDomainDataLookupInternal(virCapsPtr caps,
> virCapsDomainDataPtr ret = NULL;
> size_t i, j, k;
>
> + VIR_DEBUG("Lookup ostype=%d arch=%d domaintype=%d emulator=%s machine=%s",
> + ostype, arch, domaintype, NULLSTR(emulator), NULLSTR(machinetype));
> for (i = 0; i < caps->nguests; i++) {
> virCapsGuestPtr guest = caps->guests[i];
>
> + if (ostype != -1 && guest->ostype != ostype) {
> + VIR_DEBUG("Skip os type want=%d vs got=%d", ostype, guest->ostype);
> + continue;
> + }
> + VIR_DEBUG("Match os type %d", ostype);
> +
> + if ((arch != VIR_ARCH_NONE) && (guest->arch.id != arch)) {
> + VIR_DEBUG("Skip arch want=%d vs got=%d", arch, guest->arch.id);
> + continue;
> + }
> + VIR_DEBUG("Match arch %d", arch);
> +
> for (j = 0; j < guest->arch.ndomains; j++) {
> virCapsGuestDomainPtr domain = guest->arch.domains[j];
> virCapsGuestMachinePtr *machinelist;
> int nmachines;
> + const char *check_emulator = NULL;
> +
> + if (domaintype != VIR_DOMAIN_VIRT_NONE &&
> + (domain->type != domaintype)) {
> + VIR_DEBUG("Skip domain type want=%d vs got=%d", domaintype, domain->type);
> + continue;
> + }
> + VIR_DEBUG("Match domain type %d", domaintype);
> +
> + check_emulator = domain->info.emulator;
> + if (!check_emulator)
> + check_emulator = guest->arch.defaultInfo.emulator;
> + if (emulator && STRNEQ_NULLABLE(check_emulator, emulator)) {
> + VIR_DEBUG("Skip emulator got=%s vs want=%s",
> + emulator, NULLSTR(check_emulator));
> + continue;
> + }
> + VIR_DEBUG("Match emulator %s", NULLSTR(emulator));
>
> if (domain->info.nmachines) {
> nmachines = domain->info.nmachines;
> @@ -677,32 +669,29 @@ virCapabilitiesDomainDataLookupInternal(virCapsPtr caps,
>
> for (k = 0; k < nmachines; k++) {
> virCapsGuestMachinePtr machine = machinelist[k];
> - if (!virCapsDomainDataCompare(guest, domain, machine,
> - ostype, arch, domaintype,
> - emulator, machinetype))
> +
> + if (machinetype &&
> + STRNEQ(machine->name, machinetype) &&
> + STRNEQ_NULLABLE(machine->canonical, machinetype)) {
> + VIR_DEBUG("Skip machine type want=%s vs got=%s got=%s",
> + machinetype, machine->name, NULLSTR(machine->canonical));
> continue;
> + }
> + VIR_DEBUG("Match machine type machine %s\n", NULLSTR(machinetype));
>
> foundmachine = machine;
> break;
> }
>
> - if (!foundmachine) {
> - if (!virCapsDomainDataCompare(guest, domain, NULL,
> - ostype, arch, domaintype,
> - emulator, machinetype))
> - continue;
> - }
> + if (!foundmachine && nmachines)
> + continue;
>
> founddomain = domain;
> break;
> }
>
> - if (!founddomain) {
> - if (!virCapsDomainDataCompare(guest, NULL, NULL,
> - ostype, arch, domaintype,
> - emulator, machinetype))
> - continue;
> - }
> + if (!founddomain)
> + continue;
>
> foundguest = guest;
> break;
> @@ -731,6 +720,7 @@ virCapabilitiesDomainDataLookupInternal(virCapsPtr caps,
> goto error;
> }
>
> + VIR_DEBUG("No match %s", virBufferCurrentContent(&buf));
> virReportError(VIR_ERR_INVALID_ARG,
> _("could not find capabilities for %s"),
> virBufferCurrentContent(&buf));
> --
> 2.17.1
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
