[virt-tools-list] [PATCH] virtinst: Pass Xen machine type to libvirt getDomainCapabilities

[Pavel Hrdina]

On Wed, Jun 15, 2016 at 08:31:42AM -0600, Charles Arnold wrote:
> Tell libvirt what machine type the user chose for Xen (PV or HVM).
> Without a type specified, the default is to return the capabilities of a pv
> machine. Passing "xenfv" will allow the "Firmware" option to show up
> under "Hypervisor Details" when a Xen HVM guest install is being customized.
> Also specify the name of the SUSE aavmf firmware for aarch64.
> 
> diff --git a/virtinst/domcapabilities.py b/virtinst/domcapabilities.py
> index 874fa1e..605d77a 100644
> --- a/virtinst/domcapabilities.py
> +++ b/virtinst/domcapabilities.py
> @@ -78,13 +78,20 @@ class _Features(_CapsBlock):
>  
>  class DomainCapabilities(XMLBuilder):
>      @staticmethod
> -    def build_from_params(conn, emulator, arch, machine, hvtype):
> +    def build_from_params(conn, emulator, arch, machine, hvtype, os_type):
>          xml = None
>          if conn.check_support(
>              conn.SUPPORT_CONN_DOMAIN_CAPABILITIES):
> +            machine_type = machine
> +            # For Xen capabilities pass either xenpv or xenfv
> +            if hvtype == "xen":
> +                if os_type == "hvm":
> +                    machine_type = "xenfv"
> +                else:
> +                    machine_type = "xenpv"

Hi Charles

I'm confused about this change, there is no need to do something like this.

virt-install creates a correct XML if you ask for it.  Please check man page for
virt-install, there are two options, --hvm and --paravirt.  If you don't specify
any of them, virt-install creates a PV guest as default.

>              try:
>                  xml = conn.getDomainCapabilities(emulator, arch,
> -                    machine, hvtype)
> +                    machine_type, hvtype)
>              except:
>                  logging.debug("Error fetching domcapabilities XML",
>                      exc_info=True)
> @@ -97,7 +104,7 @@ class DomainCapabilities(XMLBuilder):
>      @staticmethod
>      def build_from_guest(guest):
>          return DomainCapabilities.build_from_params(guest.conn,
> -            guest.emulator, guest.os.arch, guest.os.machine, guest.type)
> +            guest.emulator, guest.os.arch, guest.os.machine, guest.type, guest.os.os_type)
>  
>      # Mapping of UEFI binary names to their associated architectures. We
>      # only use this info to do things automagically for the user, it shouldn't
> @@ -112,6 +119,7 @@ class DomainCapabilities(XMLBuilder):
>          "aarch64": [
>              ".*AAVMF_CODE\.fd",  # RHEL
>              ".*aarch64/QEMU_EFI.*",  # gerd's firmware repo
> +            ".*aavmf-aarch64-.*"  # SUSE
>              ".*aarch64.*",  # generic attempt at a catchall
>          ],
>      }

This hunk should be a separate patch because it's unrelated to the rest of the
patch.  Please send this as a separate patch and also if possible provide some
source where we can validate the naming.

Thanks,

Pavel