Re: [libvirt] [PATCH v3 3/6] Remove PS2 mouse device for non-X86 platforms

2013-12-10 Thread Ján Tomko 
On 12/10/2013 07:02 AM, Li Zhang wrote:
 From: Li Zhang zhlci...@linux.vnet.ibm.com
 
 PS2 device only works for X86 platform, other platforms may need
 USB mouse. Athough it doesn't influence the QEMU command line, but
 It's not right to add one PS2 mouse for non-X86 platform.
 
 This patch is to remove PS2 device definition from other platforms.
 Add one default USB mouse for PPC64. It can be also added for other
 platforms if necessary.
 
 Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
 ---
  src/conf/domain_conf.c | 59 
 --
  src/qemu/qemu_domain.c | 20 +++-
  src/util/virarch.h |  2 +
  .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |  2 +-
  4 files changed, 42 insertions(+), 41 deletions(-)

 @@ -12260,30 +12263,6 @@ virDomainDefParseXML(xmlDocPtr xml,
  }
  VIR_FREE(nodes);
  
 -/* If graphics are enabled, there's an implicit PS2 mouse */
 -if (def-ngraphics  0) {
 -virDomainInputDefPtr input;
 -
 -if (VIR_ALLOC(input)  0) {
 -goto error;
 -}
 -if (STREQ(def-os.type, hvm)) {
 -input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
 -input-bus = VIR_DOMAIN_INPUT_BUS_PS2;
 -} else {
 -input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
 -input-bus = VIR_DOMAIN_INPUT_BUS_XEN;
 -}
 -
 -if (VIR_REALLOC_N(def-inputs, def-ninputs + 1)  0) {
 -virDomainInputDefFree(input);
 -goto error;
 -}
 -def-inputs[def-ninputs] = input;
 -def-ninputs++;
 -}
 -
 -

Here you remove auto-adding of the mouse for all drivers, but only re-add it
to the QEMU driver, leading to failures in sexpr2xmltest.


  /* analysis of the sound devices */
  if ((n = virXPathNodeSet(./devices/sound, ctxt, nodes))  0) {
  goto error;
 @@ -17201,15 +17180,17 @@ virDomainDefFormatInternal(virDomainDefPtr def,
  
  if (def-ngraphics  0) {
  /* If graphics is enabled, add the implicit mouse */
 -virDomainInputDef autoInput = {
 -VIR_DOMAIN_INPUT_TYPE_MOUSE,
 -STREQ(def-os.type, hvm) ?
 -VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN,
 -{ .alias = NULL },
 -};
 -
 -if (virDomainInputDefFormat(buf, autoInput, flags)  0)
 -goto error;
 +if (ARCH_IS_X86(def-os.arch)) {
 +virDomainInputDef autoInput = {
 +VIR_DOMAIN_INPUT_TYPE_MOUSE,
 +STREQ(def-os.type, hvm) ?
 +VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN,
 +{ .alias = NULL },
 +};
 +if (virDomainInputDefFormat(buf, autoInput, flags)  0)
 +goto error;
 +}
 +
  
  for (n = 0; n  def-ngraphics; n++)
  if (virDomainGraphicsDefFormat(buf, def-graphics[n], flags)  0)

Hmm, if non-USB input devices get skipped when generating the command line and
when formatting the XML, I wonder why are we adding it there in the first place.

 diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
 index 346fec3..75e615a 100644
 --- a/src/qemu/qemu_domain.c
 +++ b/src/qemu/qemu_domain.c
 @@ -691,6 +691,8 @@ qemuDomainDefPostParse(virDomainDefPtr def,
  bool addPCIRoot = false;
  bool addPCIeRoot = false;
  bool addDefaultMemballoon = true;
 +bool addDefaultMouse = false;
 +int  mouse_bus = VIR_DOMAIN_INPUT_BUS_XEN;
  
  /* check for emulator and create a default one if needed */
  if (!def-emulator 
 @@ -721,6 +723,9 @@ qemuDomainDefPostParse(virDomainDefPtr def,
  !STRPREFIX(def-os.machine, rhel))
  break;
  addPCIRoot = true;
 +addDefaultMouse = true;
 +if (STREQ(def-os.type, hvm))
 +mouse_bus = VIR_DOMAIN_INPUT_BUS_PS2;

os.type has to be hvm in the qemu driver.

  break;
  
  case VIR_ARCH_ARMV7L:

Jan




signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 3/6] Remove PS2 mouse device for non-X86 platforms

2013-12-10 Thread Li Zhang 

On 2013年12月11日 00:17, Ján Tomko wrote:

On 12/10/2013 07:02 AM, Li Zhang wrote:

From: Li Zhang zhlci...@linux.vnet.ibm.com

PS2 device only works for X86 platform, other platforms may need
USB mouse. Athough it doesn't influence the QEMU command line, but
It's not right to add one PS2 mouse for non-X86 platform.

This patch is to remove PS2 device definition from other platforms.
Add one default USB mouse for PPC64. It can be also added for other
platforms if necessary.

Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
  src/conf/domain_conf.c | 59 --
  src/qemu/qemu_domain.c | 20 +++-
  src/util/virarch.h |  2 +
  .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |  2 +-
  4 files changed, 42 insertions(+), 41 deletions(-)
@@ -12260,30 +12263,6 @@ virDomainDefParseXML(xmlDocPtr xml,
  }
  VIR_FREE(nodes);
  
-/* If graphics are enabled, there's an implicit PS2 mouse */

-if (def-ngraphics  0) {
-virDomainInputDefPtr input;
-
-if (VIR_ALLOC(input)  0) {
-goto error;
-}
-if (STREQ(def-os.type, hvm)) {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_PS2;
-} else {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_XEN;
-}
-
-if (VIR_REALLOC_N(def-inputs, def-ninputs + 1)  0) {
-virDomainInputDefFree(input);
-goto error;
-}
-def-inputs[def-ninputs] = input;
-def-ninputs++;
-}
-
-

Here you remove auto-adding of the mouse for all drivers, but only re-add it
to the QEMU driver, leading to failures in sexpr2xmltest.

Sorry, I made some mistakes.
Let me correct it.





  /* analysis of the sound devices */
  if ((n = virXPathNodeSet(./devices/sound, ctxt, nodes))  0) {
  goto error;
@@ -17201,15 +17180,17 @@ virDomainDefFormatInternal(virDomainDefPtr def,
  
  if (def-ngraphics  0) {

  /* If graphics is enabled, add the implicit mouse */
-virDomainInputDef autoInput = {
-VIR_DOMAIN_INPUT_TYPE_MOUSE,
-STREQ(def-os.type, hvm) ?
-VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN,
-{ .alias = NULL },
-};
-
-if (virDomainInputDefFormat(buf, autoInput, flags)  0)
-goto error;
+if (ARCH_IS_X86(def-os.arch)) {
+virDomainInputDef autoInput = {
+VIR_DOMAIN_INPUT_TYPE_MOUSE,
+STREQ(def-os.type, hvm) ?
+VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN,
+{ .alias = NULL },
+};
+if (virDomainInputDefFormat(buf, autoInput, flags)  0)
+goto error;
+}
+
  
  for (n = 0; n  def-ngraphics; n++)

  if (virDomainGraphicsDefFormat(buf, def-graphics[n], flags)  0)

Hmm, if non-USB input devices get skipped when generating the command line and
when formatting the XML, I wonder why are we adding it there in the first place.


I also have some confusion about it.
For PS2 mouse, what I have seen is that it doesn't create QEMU command line.
But it is configured in XML file. This device is added by QEMU if XEN is 
not enabled (hw/pc_piix.c).
I am not sure whether how this device is added on other kind of 
virtualization.
This may be not necessary to add this device in libvirt, it is not 
configurable.


Thanks.
-Li




diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 346fec3..75e615a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -691,6 +691,8 @@ qemuDomainDefPostParse(virDomainDefPtr def,
  bool addPCIRoot = false;
  bool addPCIeRoot = false;
  bool addDefaultMemballoon = true;
+bool addDefaultMouse = false;
+int  mouse_bus = VIR_DOMAIN_INPUT_BUS_XEN;
  
  /* check for emulator and create a default one if needed */

  if (!def-emulator 
@@ -721,6 +723,9 @@ qemuDomainDefPostParse(virDomainDefPtr def,
  !STRPREFIX(def-os.machine, rhel))
  break;
  addPCIRoot = true;
+addDefaultMouse = true;
+if (STREQ(def-os.type, hvm))
+mouse_bus = VIR_DOMAIN_INPUT_BUS_PS2;

os.type has to be hvm in the qemu driver.


OK, I will remove this check.




  break;
  
  case VIR_ARCH_ARMV7L:

Jan




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

[libvirt] [PATCH v3 3/6] Remove PS2 mouse device for non-X86 platforms

2013-12-09 Thread Li Zhang 
From: Li Zhang zhlci...@linux.vnet.ibm.com

PS2 device only works for X86 platform, other platforms may need
USB mouse. Athough it doesn't influence the QEMU command line, but
It's not right to add one PS2 mouse for non-X86 platform.

This patch is to remove PS2 device definition from other platforms.
Add one default USB mouse for PPC64. It can be also added for other
platforms if necessary.

Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
 src/conf/domain_conf.c | 59 --
 src/qemu/qemu_domain.c | 20 +++-
 src/util/virarch.h |  2 +
 .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |  2 +-
 4 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 82339ea..e53a786 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7567,7 +7567,7 @@ error:
 
 /* Parse the XML definition for an input device */
 static virDomainInputDefPtr
-virDomainInputDefParseXML(const char *ostype,
+virDomainInputDefParseXML(const virDomainDef *dom,
   xmlNodePtr node,
   unsigned int flags)
 {
@@ -7600,7 +7600,7 @@ virDomainInputDefParseXML(const char *ostype,
 goto error;
 }
 
-if (STREQ(ostype, hvm)) {
+if (STREQ(dom-os.type, hvm)) {
 if (def-bus == VIR_DOMAIN_INPUT_BUS_PS2  /* Only allow mouse 
for ps2 */
 def-type != VIR_DOMAIN_INPUT_TYPE_MOUSE) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -7628,8 +7628,9 @@ virDomainInputDefParseXML(const char *ostype,
 }
 }
 } else {
-if (STREQ(ostype, hvm)) {
-if (def-type == VIR_DOMAIN_INPUT_TYPE_MOUSE)
+if (STREQ(dom-os.type, hvm)) {
+if (def-type == VIR_DOMAIN_INPUT_TYPE_MOUSE 
+ARCH_IS_X86(dom-os.arch))
 def-bus = VIR_DOMAIN_INPUT_BUS_PS2;
 else
 def-bus = VIR_DOMAIN_INPUT_BUS_USB;
@@ -9631,7 +9632,7 @@ virDomainDeviceDefParse(const char *xmlStr,
 goto error;
 break;
 case VIR_DOMAIN_DEVICE_INPUT:
-if (!(dev-data.input = virDomainInputDefParseXML(def-os.type,
+if (!(dev-data.input = virDomainInputDefParseXML(def,
   node, flags)))
 goto error;
 break;
@@ -12211,7 +12212,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 
 for (i = 0; i  n; i++) {
-virDomainInputDefPtr input = virDomainInputDefParseXML(def-os.type,
+virDomainInputDefPtr input = virDomainInputDefParseXML(def,
nodes[i],
flags);
 if (!input)
@@ -12230,9 +12231,11 @@ virDomainDefParseXML(xmlDocPtr xml,
  * with graphics, so don't store it.
  * XXX will this be true for other virt types ? */
 if ((STREQ(def-os.type, hvm) 
+ ARCH_IS_X86(def-os.arch) 
  input-bus == VIR_DOMAIN_INPUT_BUS_PS2 
  input-type == VIR_DOMAIN_INPUT_TYPE_MOUSE) ||
 (STRNEQ(def-os.type, hvm) 
+ ARCH_IS_X86(def-os.arch) 
  input-bus == VIR_DOMAIN_INPUT_BUS_XEN 
  input-type == VIR_DOMAIN_INPUT_TYPE_MOUSE)) {
 virDomainInputDefFree(input);
@@ -12260,30 +12263,6 @@ virDomainDefParseXML(xmlDocPtr xml,
 }
 VIR_FREE(nodes);
 
-/* If graphics are enabled, there's an implicit PS2 mouse */
-if (def-ngraphics  0) {
-virDomainInputDefPtr input;
-
-if (VIR_ALLOC(input)  0) {
-goto error;
-}
-if (STREQ(def-os.type, hvm)) {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_PS2;
-} else {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_XEN;
-}
-
-if (VIR_REALLOC_N(def-inputs, def-ninputs + 1)  0) {
-virDomainInputDefFree(input);
-goto error;
-}
-def-inputs[def-ninputs] = input;
-def-ninputs++;
-}
-
-
 /* analysis of the sound devices */
 if ((n = virXPathNodeSet(./devices/sound, ctxt, nodes))  0) {
 goto error;
@@ -17201,15 +17180,17 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 
 if (def-ngraphics  0) {
 /* If graphics is enabled, add the implicit mouse */
-virDomainInputDef autoInput = {
-VIR_DOMAIN_INPUT_TYPE_MOUSE,
-STREQ(def-os.type, hvm) ?
-VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN,
-{ .alias = NULL },
-};
-
-if (virDomainInputDefFormat(buf, autoInput, flags)  0)
-goto error;
+if (ARCH_IS_X86(def-os.arch)) {
+virDomainInputDef autoInput = {
+