[libvirt] [jenkins-ci PATCH v5 4/5] mappings: extend mapping to allow per-arch entries

[Andrea Bolognani]

On Tue, 2019-02-26 at 11:00 +0000, Daniel P. Berrangé wrote:
> For example to prevent Xen being installed on any s390x
> 
>   xen:
>     default-s390x:
>     deb: libxen-dev
>     Fedora: xen-devel
> 
> Or the inverse to only install Xen on x86_64 on Debian, but allow
> it on all archs on Fedora
> 
>   xen:
>     deb-x86_64: libxen-dev
>     Fedora: xen-devel
> 
> Note that the architecture specific matches are considered after
> all the non-architcture matches.

I feel like the order entries are processed with this implementation
can be quite counter-intuitive. The root problem of course is that up
until this point we have had a single axis to work on, and we're now
introducing a second one which is independent of the existing one but
which we still have to fit along with it into the same flat hierarchy
somehow...

Consider an example such as

  foo:
    Fedora: oldfoo
    Fedora-s390x:
    FedoraRawhide: foo

The general intuition up until now has been that appending something
to a label would make it more specific, and only one item on each
level would possibly match, but now both Fedora-s390x and
FedoraRawhide appear to be on the same level and *both* could apply
to an s390x Fedora Rawhide machine, making it less clear which one
would take precedence.

I suggest that the example above would be rewritten as

  foo:
    Fedora: oldfoo
    FedoraRawhide: foo
    s390x-Fedora:

where two changes happened: first of all, the lines have been
shuffled around to match the order they would actually be processed,
which is something that we could have done with the above as well,
but more importantly the architecture name is now *prepended* to the
existing labels to clearly signal that it's part of a completely
separate hierarchy. The ambiguity is now gone entirely, and which
entry has precedence is easier to see at a glance.

Implementing this alternative proposal requires only very minor
tweaks to the code you posted.

[...]
> @@ -424,6 +427,15 @@ class Application:
>              raise Error(
>                  "Failed to run {} on '{}': {}".format(playbook, hosts, ex))
>  
> +    def _libvirt_host_arch(self):
> +        # Same canonicalization as libvirt virArchFromHost
> +        arch = platform.machine()
> +        if arch in ["i386", "i486", "i586"]:
> +            arch = "i686"
> +        if arch == "amd64":
> +            arch = "x86_64"
> +        return arch

This should be a @staticmethod in the Utils class. I'd also rename
it to get_native_arch().

[...]
> @@ -278,6 +308,8 @@ mappings:
>  
>    libnuma:
>      deb: libnuma-dev
> +    deb-armv6l:
> +    deb-armv7l:
>      rpm: numactl-devel
>  
>    libparted:
> @@ -821,7 +853,9 @@ mappings:
>      Debian8:
>  
>    xen:
> -    deb: libxen-dev
> +    deb-x86_64: libxen-dev
> +    deb-armv7l: libxen-dev
> +    deb-aarch64: libxen-dev
>      Fedora: xen-devel

Updating the documentation as you implement new features is good,
but these last two hunks should go in a separate commit.

-- 
Andrea Bolognani / Red Hat / Virtualization