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

2015-06-11 Thread Christophe Fergeau
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 fzig...@redhat.com
 ---
  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 

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

2015-06-11 Thread Christophe Fergeau
On Thu, Jun 11, 2015 at 12:39:50PM +0100, Frediano Ziglio wrote:
 
 Actually can be a compatiblity problem as heads in the XML configuration
 was set by default to '1'.

Yes, this bit is worrying, the old behaviour could be considered as
buggy as the XML contained '1' but the number of heads was not enforced.
Suddenly enforcing the heads='1' (which libvirt will add by default to
domain definitions which don't have it) will cause a change of behaviour
for old guests though.

Something like the patch below changes libvirt so that we don't always
append heads='1' to domain XML, but I don't know if this interacts
correctly with parallels and vmx which force it to be 1 (this probably should
not be an issue, but maybe there are latent bugs). Also this is part of the 
things
which are checked as part of virDomainVideoDefCheckABIStability() , so I
suspect we'll need to be extra careful there too :(

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 36de844..43067e9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11421,7 +11421,7 @@ virDomainVideoDefParseXML(xmlNodePtr node,
 goto error;
 }
 } else {
-def-heads = 1;
+def-heads = 0;
 }

 if (virDomainDeviceInfoParseXML(node, NULL, def-info, flags)  0)
@@ -15507,7 +15507,6 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 }
 video-vram = virDomainVideoDefaultRAM(def, video-type);
-video-heads = 1;
 if (VIR_ALLOC_N(def-videos, 1)  0) {
 virDomainVideoDefFree(video);
 goto error;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2bd63e1..03a0458 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -13411,7 +13411,6 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
 vid-ram = 0;
 vid-vgamem = 0;
 }
-vid-heads = 1;

 if (VIR_APPEND_ELEMENT(def-videos, def-nvideos, vid)  0) {
 virDomainVideoDefFree(vid);


pgp8QCXxNtEhF.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list