Re: [Qemu-devel] [PATCH] hid: Extend the event queue size to 1024

2016-04-15 Thread Dinar Valeev 
On 04/14/2016 06:19 PM, Gerd Hoffmann wrote:
> On Do, 2016-04-14 at 17:29 +0200, Alexander Graf wrote:
>> On 04/14/2016 05:17 PM, Gerd Hoffmann wrote:
>>> On Do, 2016-04-14 at 16:25 +0200, Alexander Graf wrote:
 We can currently buffer up to 16 events in our event queue for input
 devices. While it sounds like 16 events backlog for everyone should
 be enough, our automated testing tools (OpenQA) manage to easily
 type faster than our guests can handle.
>>> IMO you should fix OpenQA to type slower then.
>>>
>>> Real hardware is made for human typing speeds.  qemu emulates real
>>> hardware.  Typing insane fast is going to cause problems because the
>>> hardware interfaces are not designed for that.
>>>
>>> Note the ps/2 keyboard has only a 16 byte queue too.  Used to be 256
>>> bytes, but that caused problems because this diverges from real hardware
>>> (see commit 2858ab09e6f708e381fc1a1cc87e747a690c4884 for all the
>>> details).
>>>
>>> Note that the generic input code has support for slow typing already
>>> (see qemu_input_event_send_key_delay in ui/input.c).  That queue will
>>> (a) work for all keyboard devices and (b) isn't visible to the guest, so
>>> using that might be a reasonable option if you can't put the delay logic
>>> into OpenQA.  How to you feed the guest with keystrokes?  monitor?  vnc?
>>
>> It used to type via the monitor, but these days everything goes via vnc.
>>
>> Vnc already uses qemu_input_event_send_key_delay today, so I'm not sure 
>> where things fall apart.
> 
> Try a larger delay for starters.
> 
> Possibly OpenQA starts typing before the guest detected the device, in
> that case the delay wouldn't avoid the queue filling up because the
> guest doesn't read it (yet).
This happens randomly.

Yes we use vnc to type into the screen. If we start 10 guests in parallel,
one or two might have keyboard issues.

like we send "zypper lr" the guest receives "zyppe lr"

or "ps -ef | grep bin/X ; echo __wHP > /dev/hvc0" results into "ps -ef | GREP 
BIN?X ; echo __wHP > /dev/hvc0"

openQA types with the same speed on all architectures. however we see problems 
only on aarch64 and POWER due to USB keyboard usage.
> 
> cheers,
>   Gerd
>

Re: [Qemu-devel] [PATCH 2/2] bootdevice: update boot_order in MachineState

2015-01-28 Thread Dinar Valeev 
On 01/28/2015 02:48 AM, Gonglei wrote:
 On 2015/1/27 18:49, Dinar Valeev wrote:
 
 On 01/27/2015 10:18 AM, Gonglei wrote:
 On 2015/1/27 16:57, Dinar Valeev wrote:

 On 01/27/2015 03:51 AM, Gonglei wrote:
 On 2015/1/27 7:52, dval...@suse.de wrote:

 From: Dinar Valeev dval...@suse.com

 on sPAPR we need to update boot_order in MachineState in case it
 got changed on reset.

 Signed-off-by: Dinar Valeev dval...@suse.com
 ---
  bootdevice.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/bootdevice.c b/bootdevice.c
 index 5914417..4f11a06 100644
 --- a/bootdevice.c
 +++ b/bootdevice.c
 @@ -26,6 +26,7 @@
  #include qapi/visitor.h
  #include qemu/error-report.h
  #include hw/hw.h
 +#include hw/boards.h
  
  typedef struct FWBootEntry FWBootEntry;
  
 @@ -50,6 +51,8 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, 
 void *opaque)
  void qemu_boot_set(const char *boot_order, Error **errp)
  {
  Error *local_err = NULL;
 +MachineState *machine = MACHINE(qdev_get_machine());
 +machine-boot_order = boot_order;
  
  if (!boot_set_handler) {
  error_setg(errp, no function defined to set boot device list 
 for

 Have you registered boot set handler on ppc/sPAPR platform by calling
 qemu_register_boot_set()? Otherwise qemu_boot_set function
  will return error.
 No, I set boot_order on each machine reset. My tests are showing it works 
 without an error.

 That's interesting. Does this function be called?
 Yes, then simply returns.
 Would you debug it by setting a breakpoint ?
 I added a trace event.
  if (!boot_set_handler) {
 +trace_qemu_boot_set(boot_order);
  error_setg(errp, no function defined to set boot device list for
this architecture);
  return;

 And I see this now in qemu's monitor. Still I don't see error message.
 
 That's because NULL is passed to this function in restore_boot_order()
 the error is ignored (commit f183993). I have seen the previous conversation
 about your patch serials. And I think this is the reason which
 you moved machine-boot_order = boot_order before
 checking boot_set_handler variable based on Alexander's
 suggestion, right? But I think this is not a good idea.
Yes

Any proposal how this can be done differently?

It seems I'm almost alone who wants -boot once=X option to get fixed for sPAPR. 
We use it in test automation, and we need to be sure that we boot from hard 
disk once installation is done.

Thanks,
Dinar
 
 Regards,
 -Gonglei

Re: [Qemu-devel] [PATCH 2/2] bootdevice: update boot_order in MachineState

2015-01-27 Thread Dinar Valeev 
On 01/27/2015 03:51 AM, Gonglei wrote:
 On 2015/1/27 7:52, dval...@suse.de wrote:
 
 From: Dinar Valeev dval...@suse.com

 on sPAPR we need to update boot_order in MachineState in case it
 got changed on reset.

 Signed-off-by: Dinar Valeev dval...@suse.com
 ---
  bootdevice.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/bootdevice.c b/bootdevice.c
 index 5914417..4f11a06 100644
 --- a/bootdevice.c
 +++ b/bootdevice.c
 @@ -26,6 +26,7 @@
  #include qapi/visitor.h
  #include qemu/error-report.h
  #include hw/hw.h
 +#include hw/boards.h
  
  typedef struct FWBootEntry FWBootEntry;
  
 @@ -50,6 +51,8 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, void 
 *opaque)
  void qemu_boot_set(const char *boot_order, Error **errp)
  {
  Error *local_err = NULL;
 +MachineState *machine = MACHINE(qdev_get_machine());
 +machine-boot_order = boot_order;
  
  if (!boot_set_handler) {
  error_setg(errp, no function defined to set boot device list for
 
 Have you registered boot set handler on ppc/sPAPR platform by calling
 qemu_register_boot_set()? Otherwise qemu_boot_set function
  will return error.
No, I set boot_order on each machine reset. My tests are showing it works 
without an error.
Previous patch was using qemu_register_boot_set, but Alexender Graf, suggested 
me to use MachineState and simply update on each guest reset.

 
 Regards,
 -Gonglei

Re: [Qemu-devel] [PATCH 2/2] bootdevice: update boot_order in MachineState

2015-01-27 Thread Dinar Valeev 
On 01/27/2015 10:18 AM, Gonglei wrote:
 On 2015/1/27 16:57, Dinar Valeev wrote:
 
 On 01/27/2015 03:51 AM, Gonglei wrote:
 On 2015/1/27 7:52, dval...@suse.de wrote:

 From: Dinar Valeev dval...@suse.com

 on sPAPR we need to update boot_order in MachineState in case it
 got changed on reset.

 Signed-off-by: Dinar Valeev dval...@suse.com
 ---
  bootdevice.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/bootdevice.c b/bootdevice.c
 index 5914417..4f11a06 100644
 --- a/bootdevice.c
 +++ b/bootdevice.c
 @@ -26,6 +26,7 @@
  #include qapi/visitor.h
  #include qemu/error-report.h
  #include hw/hw.h
 +#include hw/boards.h
  
  typedef struct FWBootEntry FWBootEntry;
  
 @@ -50,6 +51,8 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, 
 void *opaque)
  void qemu_boot_set(const char *boot_order, Error **errp)
  {
  Error *local_err = NULL;
 +MachineState *machine = MACHINE(qdev_get_machine());
 +machine-boot_order = boot_order;
  
  if (!boot_set_handler) {
  error_setg(errp, no function defined to set boot device list for

 Have you registered boot set handler on ppc/sPAPR platform by calling
 qemu_register_boot_set()? Otherwise qemu_boot_set function
  will return error.
 No, I set boot_order on each machine reset. My tests are showing it works 
 without an error.
 
 That's interesting. Does this function be called?
I'll see what I can do here. I recall once I debugged once option, I haven't 
seen
error message either.

 Would you debug it by setting a breakpoint ?
 
 Regards,
 -Gonglei
 
 Previous patch was using qemu_register_boot_set, but Alexender Graf, 
 suggested me to use MachineState and simply update on each guest reset.


 Regards,
 -Gonglei

Re: [Qemu-devel] [PATCH 2/2] bootdevice: update boot_order in MachineState

2015-01-27 Thread Dinar Valeev 
On 01/27/2015 10:18 AM, Gonglei wrote:
 On 2015/1/27 16:57, Dinar Valeev wrote:
 
 On 01/27/2015 03:51 AM, Gonglei wrote:
 On 2015/1/27 7:52, dval...@suse.de wrote:

 From: Dinar Valeev dval...@suse.com

 on sPAPR we need to update boot_order in MachineState in case it
 got changed on reset.

 Signed-off-by: Dinar Valeev dval...@suse.com
 ---
  bootdevice.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/bootdevice.c b/bootdevice.c
 index 5914417..4f11a06 100644
 --- a/bootdevice.c
 +++ b/bootdevice.c
 @@ -26,6 +26,7 @@
  #include qapi/visitor.h
  #include qemu/error-report.h
  #include hw/hw.h
 +#include hw/boards.h
  
  typedef struct FWBootEntry FWBootEntry;
  
 @@ -50,6 +51,8 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, 
 void *opaque)
  void qemu_boot_set(const char *boot_order, Error **errp)
  {
  Error *local_err = NULL;
 +MachineState *machine = MACHINE(qdev_get_machine());
 +machine-boot_order = boot_order;
  
  if (!boot_set_handler) {
  error_setg(errp, no function defined to set boot device list for

 Have you registered boot set handler on ppc/sPAPR platform by calling
 qemu_register_boot_set()? Otherwise qemu_boot_set function
  will return error.
 No, I set boot_order on each machine reset. My tests are showing it works 
 without an error.
 
 That's interesting. Does this function be called?
Yes, then simply returns.
 Would you debug it by setting a breakpoint ?
I added a trace event.
 if (!boot_set_handler) {
+trace_qemu_boot_set(boot_order);
 error_setg(errp, no function defined to set boot device list for
   this architecture);
 return;

And I see this now in qemu's monitor. Still I don't see error message.
 
 Regards,
 -Gonglei
 
 Previous patch was using qemu_register_boot_set, but Alexender Graf, 
 suggested me to use MachineState and simply update on each guest reset.


 Regards,
 -Gonglei

Re: [Qemu-devel] [PATCH 1/2] hw/ppc/spapr.c Set default boot order

2015-01-26 Thread Dinar Valeev 

On 01/26/2015 01:37 PM, Markus Armbruster wrote:

Dinar Valeev dval...@suse.de writes:


On 01/26/2015 10:11 AM, Markus Armbruster wrote:

dval...@suse.de writes:


From: Dinar Valeev dval...@suse.com

In order to use -boot once=X option we need to have default list
   where restore to on reset.


Really?  What happens without this patch?


qemu segfaults on reset.
0  reset-all  Segmentation fault


Next time, include a backtrace, please.

Ok, sorry for that.


Here's what I think happens.

Boot order comes from --boot parameter once, order, or else the machine
type's .default_boot_order.  The latter is null for you.

It gets passed via ppc_spapr_init() to spapr_create_fdt_skel(), which
sets qemu,boot-device in the FDT to it, but only when it isn't null.

If it comes from parameter once, we additionally register a reset
handler to switch it to parameter order or else .default_boot_order on
reset.  If you specify once, but not order, this is null for you.

On reset, reset handler restore_boot_order() runs.  Unlike
spapr_create_fdt_skel(), it doesn't check for null, and crashes in
validate_bootdevices().

Correct?

Yes

qemu_register_boot_set is implemented in PATCH 2/2. on reset boot_device 
is restored to NULL


For me, a null .default_boot_order means machine type does not support
boot order (this is how commit c165473 treats it).  Arguably, --boot
order and once should be rejected then.
AFICS SLOF handles qemu,boot-device as boot device, if nothing passed 
then it goes disk, cdrom, network. Which is the same as cdn list.



If I understand you correctly, your machine type does support boot
order.  Giving it a non-null .default_boot_order makes sense then.  The
appropriate value depends on firmware.  It could even be .

The null check in spapr_create_fdt_skel() looks superfluous then.
Consider dropping it.

Makes sense?

Re: [Qemu-devel] [PATCH 1/2] hw/ppc/spapr.c Set default boot order

2015-01-26 Thread Dinar Valeev 

On 01/26/2015 10:11 AM, Markus Armbruster wrote:

dval...@suse.de writes:


From: Dinar Valeev dval...@suse.com

In order to use -boot once=X option we need to have default list
  where restore to on reset.


Really?  What happens without this patch?


qemu segfaults on reset.
0  reset-all  Segmentation fault

Re: [Qemu-devel] [PATCH 2/2] hw/ppc/spapr Add qemu_register_boot_set for SPAPR

2015-01-26 Thread Dinar Valeev 

On 01/24/2015 12:04 AM, Alexander Graf wrote:



On 23.01.15 23:51, dval...@suse.de wrote:

From: Dinar Valeev dval...@suse.com

In order to have -boot once=d functioning, it is required to have
qemu_register_boot_set

qemu-system-ppc64 -enable-kvm -boot once=d

Ready!
0  dev /chosen   ok
0  .properties
...
qemu,boot-device d
...
0  reset-all

Ready!
0  dev /chosen   ok
0  .properties
...
qemu,boot-device cdn
...

Signed-off-by: Dinar Valeev dval...@suse.com
---
  hw/ppc/spapr.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3d2cfa3..38b03fc 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -314,6 +314,16 @@ static void add_str(GString *s, const gchar *s1)
  g_string_append_len(s, s1, strlen(s1) + 1);
  }

+static void spapr_boot_set(void *opaque, const char *boot_device,
+   Error **errp)
+{
+int offset;
+offset = fdt_path_offset(opaque, /chosen);
+fdt_setprop_string(opaque, offset, qemu,boot-device, boot_device);
+
+}
+
+
  static void *spapr_create_fdt_skel(hwaddr initrd_base,
 hwaddr initrd_size,
 hwaddr kernel_size,
@@ -414,6 +424,8 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
  if (boot_device) {
  _FDT((fdt_property_string(fdt, qemu,boot-device, boot_device)));
  }
+qemu_register_boot_set(spapr_boot_set, fdt);


If you simply move the code above (the _FDT() one) from create_fdt_skel
to spapr_finalize_fdt() you should have the same net effect and much
cleaner code :).

I've tried your proposal, on reset boot-device property stays d

qemu,boot-device d
0  reset-all
qemu,boot-device d


Here is what I tried:
@@ -411,9 +410,6 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
 _FDT((fdt_property(fdt, qemu,boot-kernel-le, NULL, 0)));
 }
 }
-if (boot_device) {
-_FDT((fdt_property_string(fdt, qemu,boot-device, boot_device)));
-}
 if (boot_menu) {
 _FDT((fdt_property_cell(fdt, qemu,boot-menu, boot_menu)));
 }
@@ -730,6 +726,8 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
 char *bootlist;
 void *fdt;
 sPAPRPHBState *phb;
+MachineState *machine = MACHINE(qdev_get_machine());
+const char *boot_device = machine-boot_order;

 fdt = g_malloc(FDT_MAX_SIZE);

@@ -769,6 +767,14 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
 fprintf(stderr, Couldn't finalize CPU device tree properties\n);
 }

+if (boot_device) {
+int offset = fdt_path_offset(fdt, /chosen);
+ret = fdt_setprop_string(fdt, offset, qemu,boot-device, 
boot_device);

+if (ret  0) {
+fprintf(stderr, Couldn't set up boot-device dt property\n);
+}
+}
+
 bootlist = get_boot_devices_list(cb, true);
 if (cb  bootlist) {
 int offset = fdt_path_offset(fdt, /chosen);

Dinar,


Alex


+
  if (boot_menu) {
  _FDT((fdt_property_cell(fdt, qemu,boot-menu, boot_menu)));
  }

Re: [Qemu-devel] [PATCH 2/2] hw/ppc/spapr Add qemu_register_boot_set for SPAPR

2015-01-24 Thread Dinar Valeev 

On 01/24/2015 12:04 AM, Alexander Graf wrote:



On 23.01.15 23:51, dval...@suse.de wrote:

From: Dinar Valeev dval...@suse.com

In order to have -boot once=d functioning, it is required to have
qemu_register_boot_set

qemu-system-ppc64 -enable-kvm -boot once=d

Ready!
0  dev /chosen   ok
0  .properties
...
qemu,boot-device d
...
0  reset-all

Ready!
0  dev /chosen   ok
0  .properties
...
qemu,boot-device cdn
...

Signed-off-by: Dinar Valeev dval...@suse.com
---
  hw/ppc/spapr.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3d2cfa3..38b03fc 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -314,6 +314,16 @@ static void add_str(GString *s, const gchar *s1)
  g_string_append_len(s, s1, strlen(s1) + 1);
  }

+static void spapr_boot_set(void *opaque, const char *boot_device,
+   Error **errp)
+{
+int offset;
+offset = fdt_path_offset(opaque, /chosen);
+fdt_setprop_string(opaque, offset, qemu,boot-device, boot_device);
+
+}
+
+
  static void *spapr_create_fdt_skel(hwaddr initrd_base,
 hwaddr initrd_size,
 hwaddr kernel_size,
@@ -414,6 +424,8 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
  if (boot_device) {
  _FDT((fdt_property_string(fdt, qemu,boot-device, boot_device)));
  }
+qemu_register_boot_set(spapr_boot_set, fdt);


If you simply move the code above (the _FDT() one) from create_fdt_skel
to spapr_finalize_fdt() you should have the same net effect and much
cleaner code :).
Haven't tried it, but I suspect -boot once=d on reset will be still 
equal to -boot d behaviour.


And does it make sense to split boot_device from the rest?


Alex


+
  if (boot_menu) {
  _FDT((fdt_property_cell(fdt, qemu,boot-menu, boot_menu)));
  }

Re: [Qemu-devel] [PATCH] hw/input/hid.c Fix capslock hid code

2015-01-22 Thread Dinar Valeev 



On 01/22/2015 08:20 AM, Gerd Hoffmann wrote:

On Mi, 2015-01-21 at 23:48 +0100, dval...@suse.de wrote:

From: Dinar Valeev dval...@suse.com

When ever USB keyboard is used, e.g. '-usbdevice keyboard' pressing
caps lock key send 0x32 hid code, which is treated as backslash.
Instead it should be 0x39 code. This affects sending uppercase keys,
as they typed whith caps lock active.

While on x86 this can be workarounded by using ps/2 protocol. On
Power it is crusial as we don't have anything else than USB.

This is fixes guest automation tasts over vnc.


That probably happens because your test automation vnc client sends
uppercase letters without also sending shift-down/up events.  The qemu
vnc server logic which is supposed to sync up capslock state between
guest and host thinks capslock is pressed on the host then and injects
some synthetic capslock key events.

Just say'ing FYI, that doesn't render the fix invalid, but you may have
wondered why you are not seeing this with a normal vnc client ;)
Yeah, I thought about sending shift+letter. It become better, but not 
really. Sending shift-b would print |B.


I've seen this behaviour with a home grown perl vnc client, ruby-vnc as 
well as with manual vncviewer client, vnc and SDL ui.


The reproducer is simple: qemu-system-x86_64 -enable-kvm -cdrom 
SLE-12-Server-MINI-ISO-x86_64-GM-DVD.iso -usbdevice keyboard -m 1024


Pressing caps lock produces \. showkey says backslash is pressed.

Dinar,


Added to input queue.

Thanks


thanks,
   Gerd

[Qemu-devel] [PATCH] configure: Enable pie for powerpc and arm Linux

2013-11-04 Thread Dinar Valeev 
From: Dinar Valeev dval...@suse.de

This patch enables pie for PowerPC and ARM architectures

Signed-off-by: Dinar Valeev dval...@suse.com
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 91372f9..0130e7e 100755
--- a/configure
+++ b/configure
@@ -1297,7 +1297,7 @@ fi
 
 if test $pie = ; then
   case $cpu-$targetos in
-i386-Linux|x86_64-Linux|x32-Linux|i386-OpenBSD|x86_64-OpenBSD)
+
i386-Linux|x86_64-Linux|x32-Linux|ppc*-Linux|arm*-Linux|aarch64*-Linux|i386-OpenBSD|x86_64-OpenBSD)
   ;;
 *)
   pie=no
-- 
1.7.12.4