Re: [Spice-devel] [RFC PATCH] qemu: Use heads parameter for QXL driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey,

On Thu, Jun 11, 2015 at 12:39:50PM +0100, Frediano Ziglio wrote:
> Allow to specify maximum number of head to QXL driver.

I've tested this with an older qemu without qxl-vga.max_outputs, and
with a newer one with support for it, and in both cases this is doing
the right thing.

> 
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
>  src/qemu/qemu_capabilities.c                     |  2 ++
>  src/qemu/qemu_capabilities.h                     |  1 +
>  src/qemu/qemu_command.c                          | 11 +++++++++++
>  tests/qemucapabilitiesdata/caps_1.2.2-1.caps     |  1 +
>  tests/qemucapabilitiesdata/caps_1.2.2-1.replies  |  8 ++++++++
>  tests/qemucapabilitiesdata/caps_1.3.1-1.caps     |  1 +
>  tests/qemucapabilitiesdata/caps_1.3.1-1.replies  |  8 ++++++++
>  tests/qemucapabilitiesdata/caps_1.4.2-1.caps     |  1 +
>  tests/qemucapabilitiesdata/caps_1.4.2-1.replies  |  8 ++++++++
>  tests/qemucapabilitiesdata/caps_1.5.3-1.caps     |  1 +
>  tests/qemucapabilitiesdata/caps_1.5.3-1.replies  |  8 ++++++++
>  tests/qemucapabilitiesdata/caps_1.6.0-1.caps     |  1 +
>  tests/qemucapabilitiesdata/caps_1.6.0-1.replies  |  8 ++++++++
>  tests/qemucapabilitiesdata/caps_1.6.50-1.caps    |  1 +
>  tests/qemucapabilitiesdata/caps_1.6.50-1.replies |  8 ++++++++
>  tests/qemucapabilitiesdata/caps_2.1.1-1.caps     |  1 +
>  tests/qemucapabilitiesdata/caps_2.1.1-1.replies  |  8 ++++++++
>  17 files changed, 77 insertions(+)
> 
> The patch to support the "max_outputs" in Qemu is still not merged but
> I got agreement on the name of the argument.
> 
> Actually can be a compatiblity problem as heads in the XML configuration
> was set by default to '1'.
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index ca7a7c2..cdc2575 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -285,6 +285,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>                "dea-key-wrap",
>                "pci-serial",
>                "aarch64-off",
> +              "qxl-vga.max_outputs",
>      );


In order to be consistent with the rest of the file, this should be

+
+              "qxl-vga.max_outputs", /* 190 */
 

>  
>  
> @@ -1643,6 +1644,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxl[] = {
>  
>  static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxlVga[] = {
>      { "vgamem_mb", QEMU_CAPS_QXL_VGA_VGAMEM },
> +    { "max_outputs", QEMU_CAPS_QXL_VGA_MAX_OUTPUTS },
>  };
>  
>  struct virQEMUCapsObjectTypeProps {
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index b5a7770..a2ea84b 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -229,6 +229,7 @@ typedef enum {
>      QEMU_CAPS_DEA_KEY_WRAP       = 187, /* -machine dea_key_wrap */
>      QEMU_CAPS_DEVICE_PCI_SERIAL  = 188, /* -device pci-serial */
>      QEMU_CAPS_CPU_AARCH64_OFF    = 189, /* -cpu ...,aarch64=off */
> +    QEMU_CAPS_QXL_VGA_MAX_OUTPUTS = 190, /* qxl-vga.max_outputs */
>  
>      QEMU_CAPS_LAST,                   /* this must always be the last item */
>  } virQEMUCapsFlags;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 0a6d92f..2bd63e1 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5610,6 +5610,11 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def,
>              /* QEMU accepts mebibytes for vgamem_mb. */
>              virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vgamem / 1024);
>          }
> +
> +        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS) &&
> +            video->heads > 0) {
> +            virBufferAsprintf(&buf, ",max_outputs=%u", video->heads);
> +        }
>      } else if (video->vram &&
>          ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA &&
>            virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) ||
> @@ -10234,6 +10239,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>                      unsigned int ram = def->videos[0]->ram;
>                      unsigned int vram = def->videos[0]->vram;
>                      unsigned int vgamem = def->videos[0]->vgamem;
> +                    unsigned int heads = def->videos[0]->heads;
>  
>                      if (vram > (UINT_MAX / 1024)) {
>                          virReportError(VIR_ERR_OVERFLOW,
> @@ -10264,6 +10270,11 @@ qemuBuildCommandLine(virConnectPtr conn,
>                          virCommandAddArgFormat(cmd, "%s.vgamem_mb=%u",
>                                                 dev, vgamem / 1024);
>                      }
> +                    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS) && heads > 0) {
> +                        virCommandAddArg(cmd, "-global");
> +                        virCommandAddArgFormat(cmd, "%s.max_outputs=%u",
> +                                               dev, heads);
> +                    }

This part of the code is a fallback for QEMU not supporting -device. As
the max_outputs option is new, I'm not sure this will ever be triggered.

>                  }
>  
>                  if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
> diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
> index 30239df..7791e42 100644
> --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
> +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
> @@ -120,4 +120,5 @@
>      <flag name='vmware-svga.vgamem_mb'/>
>      <flag name='qxl.vgamem_mb'/>
>      <flag name='qxl-vga.vgamem_mb'/>
> +    <flag name='qxl-vga.max_outputs'/>
>    </qemuCaps>
> diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.replies b/tests/qemucapabilitiesdata/caps_1.2.2-1.replies
> index f501218..aa1d3f9 100644
> --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.replies
> +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.replies
> @@ -1488,6 +1488,10 @@
>              "type": "pci-devfn"
>          },
>          {
> +            "name": "max_outputs",
> +            "type": "uint16"
> +        },
> +        {
>              "name": "vgamem_mb",
>              "type": "uint32"
>          },
> @@ -1554,6 +1558,10 @@
>              "type": "pci-devfn"
>          },
>          {
> +            "name": "max_outputs",
> +            "type": "uint16"
> +        },
> +        {
>              "name": "vgamem_mb",
>              "type": "uint32"
>          },

I have no idea how all these tests work, so I have only took a quick
look, and checked that make check still passes. I find it odd to have
qxl-vga.max_outputs be listed for older qemu releases, but maybe this is
how it's meant to be done.

All in all, looks good to me.

Christophe

Attachment: pgpBpkIuL3Ofg.pgp
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]