[Qemu-devel] [PATCHv5 00/03] Replace the COLO comparing thread with IOThread

[Wang yong]

From: Wang Yong 

It's a good idea to use IOThread instead of COLO comparing thread.
comparing thread can be completely replaced by IOThread, so this idea came.

This series of updates mainly include IOThread supports the GMainContext
event loop, then the old packet regularly check and primary/secondary network
packets compare all into the IOThread processing.

Please review,thanks.

wangyong(3):
qemu-iothread: IOThread supports the GMainContext event loop
colo-compare: Use IOThread to Check old packet regularly and
 Process pactkets of the primary
colo-compare: Update the COLO document to add the IOThread
 configuration

 include/sysemu/iothread.h | 10 +
 iothread.c| 54 +++
 net/colo-compare.c | 75 --
 docs/colo-proxy.txt | 3 ++- 
 4 file changed, 97 insertions(+), 39 deletions(-)

--
1.8.3.1

[Qemu-devel] [PATCHv5 03/03] colo-compare: Update the COLO document to add the IOThread configuration

[Wang yong]

From: Wang Yong 

Update colo-proxy.txt,add IOThread configuration.
Later we have to configure IOThread,if not COLO can not work.

Signed-off-by: Wang Yong 
Signed-off-by: Wang Guang 
---
 docs/colo-proxy.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/colo-proxy.txt b/docs/colo-proxy.txt
index c4941de..ce3f783 100644
--- a/docs/colo-proxy.txt
+++ b/docs/colo-proxy.txt
@@ -170,10 +170,11 @@ Primary(ip:3.3.3.3):
 -chardev socket,id=compare0-0,host=3.3.3.3,port=9001
 -chardev socket,id=compare_out,host=3.3.3.3,port=9005,server,nowait
 -chardev socket,id=compare_out0,host=3.3.3.3,port=9005
+-object iothread,id=iothread1
 -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0
 -object filter-redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out
 -object filter-redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0
--object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0
+-object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,iothread=iothread1
 
 Secondary(ip:3.3.3.8):
 -netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down script=/etc/qemu-ifdown
-- 
1.8.3.1

[Qemu-devel] [PATCHv5 01/03] qemu-iothread: IOThread supports the GMainContext event loop

[Wang yong]

From: Wang Yong 

IOThread uses AioContext event loop and does not run a GMainContext.
Therefore,chardev cannot work in IOThread,such as the chardev is
used for colo-compare packets reception.

This patch makes the IOThread run the GMainContext event loop,
chardev and IOThread can work together.

Signed-off-by: Wang Yong 
Signed-off-by: Wang Guang 
---
 include/sysemu/iothread.h |  4 
 iothread.c| 45 +
 2 files changed, 49 insertions(+)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index e6da1a4..d2985b3 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -24,6 +24,9 @@ typedef struct {
 
 QemuThread thread;
 AioContext *ctx;
+GMainContext *worker_context;
+GMainLoop *main_loop;
+GOnce once;
 QemuMutex init_done_lock;
 QemuCond init_done_cond;/* is thread initialization done? */
 bool stopping;
@@ -41,5 +44,6 @@ typedef struct {
 char *iothread_get_id(IOThread *iothread);
 AioContext *iothread_get_aio_context(IOThread *iothread);
 void iothread_stop_all(void);
+GMainContext *iothread_get_g_main_context(IOThread *iothread);
 
 #endif /* IOTHREAD_H */
diff --git a/iothread.c b/iothread.c
index beeb870..44c8944 100644
--- a/iothread.c
+++ b/iothread.c
@@ -57,6 +57,23 @@ static void *iothread_run(void *opaque)
 
 while (!atomic_read(&iothread->stopping)) {
 aio_poll(iothread->ctx, true);
+
+if (atomic_read(&iothread->worker_context)) {
+GMainLoop *loop;
+
+g_main_context_push_thread_default(iothread->worker_context);
+iothread->main_loop =
+g_main_loop_new(iothread->worker_context, TRUE);
+loop = iothread->main_loop;
+
+g_main_loop_run(iothread->main_loop);
+iothread->main_loop = NULL;
+g_main_loop_unref(loop);
+
+g_main_context_pop_thread_default(iothread->worker_context);
+g_main_context_unref(iothread->worker_context);
+iothread->worker_context = NULL;
+}
 }
 
 rcu_unregister_thread();
@@ -73,6 +90,9 @@ static int iothread_stop(Object *object, void *opaque)
 }
 iothread->stopping = true;
 aio_notify(iothread->ctx);
+if (atomic_read(&iothread->main_loop)) {
+g_main_loop_quit(iothread->main_loop);
+}
 qemu_thread_join(&iothread->thread);
 return 0;
 }
@@ -125,6 +145,7 @@ static void iothread_complete(UserCreatable *obj, Error 
**errp)
 
 qemu_mutex_init(&iothread->init_done_lock);
 qemu_cond_init(&iothread->init_done_cond);
+iothread->once = (GOnce) G_ONCE_INIT;
 
 /* This assumes we are called from a thread with useful CPU affinity for us
  * to inherit.
@@ -309,3 +330,27 @@ void iothread_stop_all(void)
 
 object_child_foreach(container, iothread_stop, NULL);
 }
+
+static gpointer iothread_g_main_context_init(gpointer opaque)
+{
+AioContext *ctx;
+IOThread *iothread = opaque;
+GSource *source;
+
+iothread->worker_context = g_main_context_new();
+
+ctx = iothread_get_aio_context(iothread);
+source = aio_get_g_source(ctx);
+g_source_attach(source, iothread->worker_context);
+g_source_unref(source);
+
+aio_notify(iothread->ctx);
+return NULL;
+}
+
+GMainContext *iothread_get_g_main_context(IOThread *iothread)
+{
+g_once(&iothread->once, iothread_g_main_context_init, iothread);
+
+return iothread->worker_context;
+}
-- 
1.8.3.1

[Qemu-devel] [PATCHv5 02/03] colo-compare: Use IOThread to Check old packet regularly and Process pactkets of the primary

[Wang yong]

From: Wang Yong 

Remove the task which check old packet in the comparing thread,
then use IOthread context timer to handle it.

Process pactkets in the IOThread which arrived over the socket.
we use iothread_get_g_main_context to create a new g_main_loop in
the IOThread.then the packets from the primary and the secondary
are processed in the IOThread.

Finally remove the colo-compare thread using the IOThread instead.

Signed-off-by: Wang Yong 
Signed-off-by: Wang Guang 
---
 net/colo-compare.c | 83 +-
 1 file changed, 45 insertions(+), 38 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 5fe8e3f..b2a2a13 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -29,6 +29,7 @@
 #include "qemu/sockets.h"
 #include "qapi-visit.h"
 #include "net/colo.h"
+#include "sysemu/iothread.h"
 
 #define TYPE_COLO_COMPARE "colo-compare"
 #define COLO_COMPARE(obj) \
@@ -82,11 +83,10 @@ typedef struct CompareState {
 GQueue conn_list;
 /* hashtable to save connection */
 GHashTable *connection_track_table;
-/* compare thread, a thread for each NIC */
-QemuThread thread;
 
+IOThread *iothread;
 GMainContext *worker_context;
-GMainLoop *compare_loop;
+QEMUTimer *packet_check_timer;
 } CompareState;
 
 typedef struct CompareClass {
@@ -597,22 +597,40 @@ static void compare_sec_chr_in(void *opaque, const 
uint8_t *buf, int size)
  * Check old packet regularly so it can watch for any packets
  * that the secondary hasn't produced equivalents of.
  */
-static gboolean check_old_packet_regular(void *opaque)
+static void check_old_packet_regular(void *opaque)
 {
 CompareState *s = opaque;
 
 /* if have old packet we will notify checkpoint */
 colo_old_packet_check(s);
+timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+REGULAR_PACKET_CHECK_MS);
+}
+
+static void colo_compare_timer_init(CompareState *s)
+{
+AioContext *ctx = iothread_get_aio_context(s->iothread);
 
-return TRUE;
+s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_VIRTUAL,
+SCALE_MS, check_old_packet_regular,
+s);
+timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+REGULAR_PACKET_CHECK_MS);
 }
 
-static void *colo_compare_thread(void *opaque)
+static void colo_compare_timer_del(CompareState *s)
 {
-CompareState *s = opaque;
-GSource *timeout_source;
+if (s->packet_check_timer) {
+timer_del(s->packet_check_timer);
+timer_free(s->packet_check_timer);
+s->packet_check_timer = NULL;
+}
+ }
 
-s->worker_context = g_main_context_new();
+static void colo_compare_iothread(CompareState *s)
+{
+object_ref(OBJECT(s->iothread));
+s->worker_context = iothread_get_g_main_context(s->iothread);
 
 qemu_chr_fe_set_handlers(&s->chr_pri_in, compare_chr_can_read,
  compare_pri_chr_in, NULL, NULL,
@@ -621,20 +639,7 @@ static void *colo_compare_thread(void *opaque)
  compare_sec_chr_in, NULL, NULL,
  s, s->worker_context, true);
 
-s->compare_loop = g_main_loop_new(s->worker_context, FALSE);
-
-/* To kick any packets that the secondary doesn't match */
-timeout_source = g_timeout_source_new(REGULAR_PACKET_CHECK_MS);
-g_source_set_callback(timeout_source,
-  (GSourceFunc)check_old_packet_regular, s, NULL);
-g_source_attach(timeout_source, s->worker_context);
-
-g_main_loop_run(s->compare_loop);
-
-g_source_unref(timeout_source);
-g_main_loop_unref(s->compare_loop);
-g_main_context_unref(s->worker_context);
-return NULL;
+colo_compare_timer_init(s);
 }
 
 static char *compare_get_pri_indev(Object *obj, Error **errp)
@@ -759,12 +764,10 @@ static void colo_compare_complete(UserCreatable *uc, 
Error **errp)
 {
 CompareState *s = COLO_COMPARE(uc);
 Chardev *chr;
-char thread_name[64];
-static int compare_id;
 
-if (!s->pri_indev || !s->sec_indev || !s->outdev) {
+if (!s->pri_indev || !s->sec_indev || !s->outdev || !s->iothread) {
 error_setg(errp, "colo compare needs 'primary_in' ,"
-   "'secondary_in','outdev' property set");
+   "'secondary_in','outdev','iothread' property set");
 return;
 } else if (!strcmp(s->pri_indev, s->outdev) ||
!strcmp(s->sec_indev, s->outdev) ||
@@ -799,12 +802,7 @@ static void colo_compare_complete(UserCreatable *uc, Error 
**errp)
   g_free,
   connection_destroy);
 
-sprintf(thread_name, "colo-compare %d", compare_id);
-qemu_thread_create(&s->thread, thread_name,
-   colo_compare_thread, s,
-   QEMU_THREAD_JOINABLE);
-compare

Re: [Qemu-devel] [PATCH 00/79] Patch Round-up for stable 2.9.1, freeze on 2017-09-04

[Cornelia Huck]

On Tue, 29 Aug 2017 04:18:20 +0200
Thomas Huth  wrote:

> On 29.08.2017 02:13, Michael Roth wrote:
> > Hi everyone,
> > 
> > The following new patches are queued for QEMU stable v2.9.1:
> > 
> >   https://github.com/mdroth/qemu/commits/stable-2.9-staging
> > 
> > The release is planned for 2017-09-07:
> > 
> >   http://wiki.qemu.org/Planning/2.9
> > 
> > Please respond here or CC qemu-sta...@nongnu.org on any patches you
> > think should be included in the release.  
> 
> I'd like to suggest the following patches:
> 
> 601b9a9008c5a612d76073bb - target-s390x: Mask the SIGP order_code ...

> 99efaa2696caaf6182958e27 - hw/s390x/ipl: Fix crash with ...

Agreed on the s390x patches.

Re: [Qemu-devel] [PATCH] virtio-input: send rel-wheel events for wheel buttons

[Ladi Prosek]

On Wed, Aug 23, 2017 at 3:51 PM, Gerd Hoffmann  wrote:
> qemu uses wheel-up/down button events for mouse wheel input, however
> linux applications typically want REL_WHEEL events.
>
> This fixes wheel with linux guests. Tested with X11/wayland, and
> windows virtio-input driver.
>
> Based on a patch from Marc.
> Added property to enable/disable wheel axis.
> TODO: add compat properties for old machine types.
>
> Cc: Marc-André Lureau 
> Signed-off-by: Gerd Hoffmann 

Verified that the Windows driver correctly handles axis-based wheel. Thanks!

Tested-by: Ladi Prosek 

Re: [Qemu-devel] [PATCH 1/4] s390x/pci: fixup trap_msix()

[Cornelia Huck]

On Tue, 29 Aug 2017 12:32:17 +0800
Yi Min Zhao  wrote:

> 在 2017/8/28 下午10:51, Cornelia Huck 写道:
> > On Mon, 28 Aug 2017 10:04:44 +0200
> > Yi Min Zhao  wrote:
> >  
> >> The function trap_msix() is to check if pcistg instruction would access
> >> msix table entries. The correct boundary condition should be
> >> [table_offset, table_offset+entries*entry_size). But the current
> >> condition calculated misses the last entry. So let's fixup it.
> >>
> >> Acked-by: Dong Jia Shi 
> >> Reviewed-by: Pierre Morel 
> >> Signed-off-by: Yi Min Zhao 
> >> ---
> >>   hw/s390x/s390-pci-inst.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> >> index b7beb8c36a..eba9ffb5f2 100644
> >> --- a/hw/s390x/s390-pci-inst.c
> >> +++ b/hw/s390x/s390-pci-inst.c
> >> @@ -440,8 +440,8 @@ static int trap_msix(S390PCIBusDevice *pbdev, uint64_t 
> >> offset, uint8_t pcias)
> >>   {
> >>   if (pbdev->msix.available && pbdev->msix.table_bar == pcias &&
> >>   offset >= pbdev->msix.table_offset &&
> >> -offset <= pbdev->msix.table_offset +
> >> -  (pbdev->msix.entries - 1) * PCI_MSIX_ENTRY_SIZE) {
> >> +offset < (pbdev->msix.table_offset +
> >> +  pbdev->msix.entries * PCI_MSIX_ENTRY_SIZE)) {
> >>   return 1;
> >>   } else {
> >>   return 0;  
> > What happened before due to the miscalculation? Write to wrong memory
> > region?
> >
> >  
> We tried to plug virtio-net pci device but failed. After inspected, we
> found that the device uses two msix entries but the last one was
> missed. Then we cannot register interrupt successfully because we
> should call trap_msixi() in order to save some useful and arch
> information into msix message. But what about wrong memory region
> didn't happen.

So, the guest just was not able to use the second msix entry, but did
not get any exception?

Re: [Qemu-devel] [PATCH 1/4] s390x/pci: fixup trap_msix()

[Yi Min Zhao]

在 2017/8/29 下午4:00, Cornelia Huck 写道:

On Tue, 29 Aug 2017 12:32:17 +0800
Yi Min Zhao  wrote:


在 2017/8/28 下午10:51, Cornelia Huck 写道:

On Mon, 28 Aug 2017 10:04:44 +0200
Yi Min Zhao  wrote:
  

The function trap_msix() is to check if pcistg instruction would access
msix table entries. The correct boundary condition should be
[table_offset, table_offset+entries*entry_size). But the current
condition calculated misses the last entry. So let's fixup it.

Acked-by: Dong Jia Shi 
Reviewed-by: Pierre Morel 
Signed-off-by: Yi Min Zhao 
---
   hw/s390x/s390-pci-inst.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index b7beb8c36a..eba9ffb5f2 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -440,8 +440,8 @@ static int trap_msix(S390PCIBusDevice *pbdev, uint64_t 
offset, uint8_t pcias)
   {
   if (pbdev->msix.available && pbdev->msix.table_bar == pcias &&
   offset >= pbdev->msix.table_offset &&
-offset <= pbdev->msix.table_offset +
-  (pbdev->msix.entries - 1) * PCI_MSIX_ENTRY_SIZE) {
+offset < (pbdev->msix.table_offset +
+  pbdev->msix.entries * PCI_MSIX_ENTRY_SIZE)) {
   return 1;
   } else {
   return 0;

What happened before due to the miscalculation? Write to wrong memory
region?

  

We tried to plug virtio-net pci device but failed. After inspected, we
found that the device uses two msix entries but the last one was
missed. Then we cannot register interrupt successfully because we
should call trap_msixi() in order to save some useful and arch
information into msix message. But what about wrong memory region
didn't happen.

So, the guest just was not able to use the second msix entry, but did
not get any exception?



Yes, didn't get any exception. The guest just kept waiting for something
(I guess that might be the response for interrupt register) and then the
system had no response. What I can do is only destroy the guest.

Re: [Qemu-devel] [PATCH 4/4] s390x/pci: add iommu replay callback

[Cornelia Huck]

[Restored cc:s. Please remember to do reply-all.]

On Tue, 29 Aug 2017 12:46:51 +0800
Yi Min Zhao  wrote:

> 在 2017/8/28 下午11:57, Cornelia Huck 写道:
> > On Mon, 28 Aug 2017 10:04:47 +0200
> > Yi Min Zhao  wrote:
> >
> >> Let's introduce iommu replay callback for s390 pci iommu memory region.
> >> Currently we don't need any dma mapping replay. So let it return
> >> directly. This implementation will avoid meaningless loops calling
> >> translation callback.
> >>
> >> Reviewed-by: Pierre Morel 
> >> Reviewed-by: Halil Pasic 
> >> Signed-off-by: Yi Min Zhao 
> >> ---
> >>   hw/s390x/s390-pci-bus.c | 8 
> >>   1 file changed, 8 insertions(+)
> >>
> >> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> >> index 9e1f7ff5c5..359509ccea 100644
> >> --- a/hw/s390x/s390-pci-bus.c
> >> +++ b/hw/s390x/s390-pci-bus.c
> >> @@ -407,6 +407,13 @@ static IOMMUTLBEntry 
> >> s390_translate_iommu(IOMMUMemoryRegion *mr, hwaddr addr,
> >>   return ret;
> >>   }
> >>   
> >> +static void s390_pci_iommu_replay(IOMMUMemoryRegion *iommu,
> >> +  IOMMUNotifier *notifier)
> >> +{
> >> +/* we don't need iommu replay currently */
> > This really needs to be heavier on the _why_. My guess is that anything
> > which would require replay goes through the rpcit instruction?
> My understanding is:
> Our arch is different from others. Each pci device has its own iommu, not
> like other archs' implementation. So currently there must be no existing
> mappings belonging to any newly plugged pci device whose iommu doesn't
> have any mapping at the time.

So please put that explanation into the function. (Also, "currently"?
Are we expecting it to change?)

> In addition, it's also due to vfio blocking migration. If vfio-pci supports
> migration in future, we could implement iommu replay by that time.

That's not an argument: This is the base zpci support, it should not
depend on the supported devices and what they do. (What's the status of
virtio-pci, btw? Does it work with your patches applied, or is there
still more to be done?)

Re: [Qemu-devel] [PATCH 1/4] s390x/pci: fixup trap_msix()

[Yi Min Zhao]

在 2017/8/29 下午4:00, Cornelia Huck 写道:

On Tue, 29 Aug 2017 12:32:17 +0800
Yi Min Zhao  wrote:


在 2017/8/28 下午10:51, Cornelia Huck 写道:

On Mon, 28 Aug 2017 10:04:44 +0200
Yi Min Zhao  wrote:
  

The function trap_msix() is to check if pcistg instruction would access
msix table entries. The correct boundary condition should be
[table_offset, table_offset+entries*entry_size). But the current
condition calculated misses the last entry. So let's fixup it.

Acked-by: Dong Jia Shi 
Reviewed-by: Pierre Morel 
Signed-off-by: Yi Min Zhao 
---
   hw/s390x/s390-pci-inst.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index b7beb8c36a..eba9ffb5f2 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -440,8 +440,8 @@ static int trap_msix(S390PCIBusDevice *pbdev, uint64_t 
offset, uint8_t pcias)
   {
   if (pbdev->msix.available && pbdev->msix.table_bar == pcias &&
   offset >= pbdev->msix.table_offset &&
-offset <= pbdev->msix.table_offset +
-  (pbdev->msix.entries - 1) * PCI_MSIX_ENTRY_SIZE) {
+offset < (pbdev->msix.table_offset +
+  pbdev->msix.entries * PCI_MSIX_ENTRY_SIZE)) {
   return 1;
   } else {
   return 0;

What happened before due to the miscalculation? Write to wrong memory
region?

  

We tried to plug virtio-net pci device but failed. After inspected, we
found that the device uses two msix entries but the last one was
missed. Then we cannot register interrupt successfully because we
should call trap_msixi() in order to save some useful and arch
information into msix message. But what about wrong memory region
didn't happen.

So, the guest just was not able to use the second msix entry, but did
not get any exception?


Forget one thing. The zpci idx is saved in msix message. The second msix 
entry has not been

trapped so that no idx has been saved, on the other hand idx 0 is saved. So
kvm_arch_fixup_msi_route() will update irq route according to the zpci 
device whose idx is 0.
So the wrong logic in trap_msix() will result that flic mixes different 
pci devices' adapter interrupts.

Re: [Qemu-devel] [PATCH 1/4] s390x/pci: fixup trap_msix()

[Cornelia Huck]

On Tue, 29 Aug 2017 16:12:26 +0800
Yi Min Zhao  wrote:

> 在 2017/8/29 下午4:00, Cornelia Huck 写道:
> > On Tue, 29 Aug 2017 12:32:17 +0800
> > Yi Min Zhao  wrote:
> >  
> >> 在 2017/8/28 下午10:51, Cornelia Huck 写道:  
> >>> On Mon, 28 Aug 2017 10:04:44 +0200
> >>> Yi Min Zhao  wrote:
> >>> 
>  The function trap_msix() is to check if pcistg instruction would access
>  msix table entries. The correct boundary condition should be
>  [table_offset, table_offset+entries*entry_size). But the current
>  condition calculated misses the last entry. So let's fixup it.
> 
>  Acked-by: Dong Jia Shi 
>  Reviewed-by: Pierre Morel 
>  Signed-off-by: Yi Min Zhao 
>  ---
> hw/s390x/s390-pci-inst.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
>  diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
>  index b7beb8c36a..eba9ffb5f2 100644
>  --- a/hw/s390x/s390-pci-inst.c
>  +++ b/hw/s390x/s390-pci-inst.c
>  @@ -440,8 +440,8 @@ static int trap_msix(S390PCIBusDevice *pbdev, 
>  uint64_t offset, uint8_t pcias)
> {
> if (pbdev->msix.available && pbdev->msix.table_bar == pcias &&
> offset >= pbdev->msix.table_offset &&
>  -offset <= pbdev->msix.table_offset +
>  -  (pbdev->msix.entries - 1) * PCI_MSIX_ENTRY_SIZE) {
>  +offset < (pbdev->msix.table_offset +
>  +  pbdev->msix.entries * PCI_MSIX_ENTRY_SIZE)) {
> return 1;
> } else {
> return 0;  
> >>> What happened before due to the miscalculation? Write to wrong memory
> >>> region?
> >>>
> >>> 
> >> We tried to plug virtio-net pci device but failed. After inspected, we
> >> found that the device uses two msix entries but the last one was
> >> missed. Then we cannot register interrupt successfully because we
> >> should call trap_msixi() in order to save some useful and arch
> >> information into msix message. But what about wrong memory region
> >> didn't happen.  
> > So, the guest just was not able to use the second msix entry, but did
> > not get any exception?
> >
> >  
> Forget one thing. The zpci idx is saved in msix message. The second msix 
> entry has not been
> trapped so that no idx has been saved, on the other hand idx 0 is saved. So
> kvm_arch_fixup_msi_route() will update irq route according to the zpci 
> device whose idx is 0.
> So the wrong logic in trap_msix() will result that flic mixes different 
> pci devices' adapter interrupts.

Ouch. So this only ever worked for the small subset of pci devices we
can passthrough (assuming none of them use more than one msix entry)?

I'm tempted to have this cc:ed to stable so we can fixup 2.10 (which is
the first version with usable zpci support).

Re: [Qemu-devel] [PATCH 4/4] s390x/pci: add iommu replay callback

[Yi Min Zhao]

在 2017/8/29 下午4:07, Cornelia Huck 写道:

[Restored cc:s. Please remember to do reply-all.]

On Tue, 29 Aug 2017 12:46:51 +0800
Yi Min Zhao  wrote:


在 2017/8/28 下午11:57, Cornelia Huck 写道:

On Mon, 28 Aug 2017 10:04:47 +0200
Yi Min Zhao  wrote:


Let's introduce iommu replay callback for s390 pci iommu memory region.
Currently we don't need any dma mapping replay. So let it return
directly. This implementation will avoid meaningless loops calling
translation callback.

Reviewed-by: Pierre Morel 
Reviewed-by: Halil Pasic 
Signed-off-by: Yi Min Zhao 
---
   hw/s390x/s390-pci-bus.c | 8 
   1 file changed, 8 insertions(+)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 9e1f7ff5c5..359509ccea 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -407,6 +407,13 @@ static IOMMUTLBEntry 
s390_translate_iommu(IOMMUMemoryRegion *mr, hwaddr addr,
   return ret;
   }
   
+static void s390_pci_iommu_replay(IOMMUMemoryRegion *iommu,

+  IOMMUNotifier *notifier)
+{
+/* we don't need iommu replay currently */

This really needs to be heavier on the _why_. My guess is that anything
which would require replay goes through the rpcit instruction?

My understanding is:
Our arch is different from others. Each pci device has its own iommu, not
like other archs' implementation. So currently there must be no existing
mappings belonging to any newly plugged pci device whose iommu doesn't
have any mapping at the time.

So please put that explanation into the function. (Also, "currently"?
Are we expecting it to change?)
The iommu replay function is originally introduced for vfio. I think 
they want to re-build
the existing mappings because vfio has a copy of mappings in kernel. For 
our case,

the mappings would be cleanup when a pci device unplugged, and new mappings
would be created when a pci device plugged. I think replay only can 
happen during

vfio-pci device migration.



In addition, it's also due to vfio blocking migration. If vfio-pci supports
migration in future, we could implement iommu replay by that time.

That's not an argument: This is the base zpci support, it should not
depend on the supported devices and what they do. (What's the status of
virtio-pci, btw? Does it work with your patches applied, or is there
still more to be done?)


My understanding is virtio-pci doesn't need replay. All mappings of any 
pci device are existing in
guest memory. Once the mappings is built, all of them could be migrated 
along with the guest

system. But I might misunderstand it. Please correct me.

Re: [Qemu-devel] understanding qemu devices

[Thomas Huth]

On 18.07.2017 23:44, Eric Blake wrote:
> Based on an IRC conversation today, here's some notes that may help
> newcomers understand what is actually happening with qemu devices.  The
> initial question was how does a guest mount a qcow2 file (the poster was
> wondering if it was a loop or FUSE filesystem) - does it mean the guest
> has to understand qcow2?
> 
> (Hopefully, someone with more experience with good documentation layouts
> and time might be willing to take this and further enhance it into a
> nice part of the qemu.org website.)

I just found some spare minutes to read through your mail. Thanks for
the text, I think this is a good summary for newcomers, indeed!

And yes, I agree, this should appear somewhere on the qemu.org website!
I think the best place is either a page in the Wiki, or a post in the
QEMU blog. I assume you already have a wiki account? If not, let me
know. Or for doing a blog post, you basically have to
 git clone git://git.qemu-project.org/qemu-web.git
and add a new file in the "_posts" directory. Have a look at the other
files there to know how to specify the meta information. Then send a
patch to qemu-devel, with Paolo on CC (and myself if you want to get a
review or test with Jekyll).

 Thomas



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC v2 16/32] vhost+postcopy: Send address back to qemu

[Peter Xu]

On Thu, Aug 24, 2017 at 08:27:14PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> We need a better way, but at the moment we need the address of the
> mappings sent back to qemu so it can interpret the messages on the
> userfaultfd it reads.
> 
> Note: We don't ask for the default 'ack' reply since we've got our own.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  contrib/libvhost-user/libvhost-user.c | 15 -
>  docs/interop/vhost-user.txt   |  6 
>  hw/virtio/trace-events|  1 +
>  hw/virtio/vhost-user.c| 57 
> ++-
>  4 files changed, 77 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/libvhost-user/libvhost-user.c 
> b/contrib/libvhost-user/libvhost-user.c
> index e6ab059a03..5ec54f7d60 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -477,13 +477,26 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
>  DPRINT("%s: region %d: Registered userfault for %llx + %llx\n",
>  __func__, i, reg_struct.range.start, 
> reg_struct.range.len);
>  /* TODO: Stash 'zero' support flags somewhere */
> -/* TODO: Get address back to QEMU */
>  
> +/* TODO: We need to find a way for the qemu not to see the 
> virtual
> + * addresses of the clients, so as to keep better separation.
> + */
> +/* Return the address to QEMU so that it can translate the ufd
> + * fault addresses back.
> + */
> +msg_region->userspace_addr = (uintptr_t)(mmap_addr +
> + 
> dev_region->mmap_offset);
>  }
>  
>  close(vmsg->fds[i]);
>  }
>  
> +if (dev->postcopy_listening) {
> +/* Need to return the addresses - send the updated message back */
> +vmsg->fd_num = 0;
> +return true;
> +}
> +
>  return false;
>  }
>  
> diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> index 73c3dd74db..b2a548c94d 100644
> --- a/docs/interop/vhost-user.txt
> +++ b/docs/interop/vhost-user.txt
> @@ -413,12 +413,18 @@ Master message types
>Id: 5
>Equivalent ioctl: VHOST_SET_MEM_TABLE
>Master payload: memory regions description
> +  Slave payload: (postcopy only) memory regions description
>  
>Sets the memory map regions on the slave so it can translate the vring
>addresses. In the ancillary data there is an array of file descriptors
>for each memory mapped region. The size and ordering of the fds matches
>the number and ordering of memory regions.
>  
> +  When postcopy-listening has been received, SET_MEM_TABLE replies with
> +  the bases of the memory mapped regions to the master.  It must have 
> mmap'd
> +  the regions and enabled userfaultfd on them.  Note NEED_REPLY_MASK
> +  is not set in this case.
> +
>   * VHOST_USER_SET_LOG_BASE
>  
>Id: 6
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index f736c7c84f..63fd4a79cf 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -2,6 +2,7 @@
>  
>  # hw/virtio/vhost-user.c
>  vhost_user_postcopy_listen(void) ""
> +vhost_user_set_mem_table_postcopy(uint64_t client_addr, uint64_t qhva, int 
> reply_i, int region_i) "client:0x%"PRIx64" for hva: 0x%"PRIx64" reply %d 
> region %d"
>  
>  # hw/virtio/virtio.c
>  virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned 
> out_num) "elem %p size %zd in_num %u out_num %u"
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 9178271ab2..2e4eb0864a 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -19,6 +19,7 @@
>  #include "qemu/sockets.h"
>  #include "migration/migration.h"
>  #include "migration/postcopy-ram.h"
> +#include "trace.h"
>  
>  #include 
>  #include 
> @@ -133,6 +134,7 @@ struct vhost_user {
>  int slave_fd;
>  NotifierWithReturn postcopy_notifier;
>  struct PostCopyFD  postcopy_fd;
> +uint64_t   postcopy_client_bases[VHOST_MEMORY_MAX_NREGIONS];
>  };
>  
>  static bool ioeventfd_enabled(void)
> @@ -300,11 +302,13 @@ static int vhost_user_set_log_base(struct vhost_dev 
> *dev, uint64_t base,
>  static int vhost_user_set_mem_table(struct vhost_dev *dev,
>  struct vhost_memory *mem)
>  {
> +struct vhost_user *u = dev->opaque;
>  int fds[VHOST_MEMORY_MAX_NREGIONS];
>  int i, fd;
>  size_t fd_num = 0;
>  bool reply_supported = virtio_has_feature(dev->protocol_features,
> -  
> VHOST_USER_PROTOCOL_F_REPLY_ACK);
> +  VHOST_USER_PROTOCOL_F_REPLY_ACK) &&
> +   !u->postcopy_fd.handler;

(indent)

>  
>  VhostUserMsg msg = {
>  .request = VHOST_USER_SET_ME

Re: [Qemu-devel] [PATCH 1/4] s390x/pci: fixup trap_msix()

[Yi Min Zhao]

在 2017/8/29 下午4:22, Cornelia Huck 写道:

On Tue, 29 Aug 2017 16:12:26 +0800
Yi Min Zhao  wrote:


在 2017/8/29 下午4:00, Cornelia Huck 写道:

On Tue, 29 Aug 2017 12:32:17 +0800
Yi Min Zhao  wrote:
  

在 2017/8/28 下午10:51, Cornelia Huck 写道:

On Mon, 28 Aug 2017 10:04:44 +0200
Yi Min Zhao  wrote:
 

The function trap_msix() is to check if pcistg instruction would access
msix table entries. The correct boundary condition should be
[table_offset, table_offset+entries*entry_size). But the current
condition calculated misses the last entry. So let's fixup it.

Acked-by: Dong Jia Shi 
Reviewed-by: Pierre Morel 
Signed-off-by: Yi Min Zhao 
---
hw/s390x/s390-pci-inst.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index b7beb8c36a..eba9ffb5f2 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -440,8 +440,8 @@ static int trap_msix(S390PCIBusDevice *pbdev, uint64_t 
offset, uint8_t pcias)
{
if (pbdev->msix.available && pbdev->msix.table_bar == pcias &&
offset >= pbdev->msix.table_offset &&
-offset <= pbdev->msix.table_offset +
-  (pbdev->msix.entries - 1) * PCI_MSIX_ENTRY_SIZE) {
+offset < (pbdev->msix.table_offset +
+  pbdev->msix.entries * PCI_MSIX_ENTRY_SIZE)) {
return 1;
} else {
return 0;

What happened before due to the miscalculation? Write to wrong memory
region?

 

We tried to plug virtio-net pci device but failed. After inspected, we
found that the device uses two msix entries but the last one was
missed. Then we cannot register interrupt successfully because we
should call trap_msixi() in order to save some useful and arch
information into msix message. But what about wrong memory region
didn't happen.

So, the guest just was not able to use the second msix entry, but did
not get any exception?

  

Forget one thing. The zpci idx is saved in msix message. The second msix
entry has not been
trapped so that no idx has been saved, on the other hand idx 0 is saved. So
kvm_arch_fixup_msi_route() will update irq route according to the zpci
device whose idx is 0.
So the wrong logic in trap_msix() will result that flic mixes different
pci devices' adapter interrupts.

Ouch. So this only ever worked for the small subset of pci devices we
can passthrough (assuming none of them use more than one msix entry)?
Because any passthroughed pci devices which I tested has more than 2 
msix entries. And not all
entries will be used. I find that the last entry is never touched. But 
virtio pci is much fancy and only
uses two entries. So I encountered the issue when I tested virtio-pci 
device.


I'm tempted to have this cc:ed to stable so we can fixup 2.10 (which is
the first version with usable zpci support).



Thanks!

Re: [Qemu-devel] GSOC Report: Moving I/O throttling and write notifiers into block filter drivers

[Stefan Hajnoczi]

On Mon, Aug 28, 2017 at 05:57:28PM +0300, Manos Pitsidianakis wrote:
> Branches / Patches
> ==
> 
> The 'throttle' and 'throttle-remove-legacy' patches should be merged soon
> after master unfreezes from the 2.10 release. The rest of the patch series
> are in final stages of review on qemu-devel except for block-insert-node
> which is an RFC [5].
> 
> Already merged patches in 2.10
>  https://github.com/qemu/qemu/commits/v2.10.0-rc4?author=epilys
> Already merged patches for 2.11
>  https://www.mail-archive.com/qemu-devel@nongnu.org/msg470461.html
> 
> [0] [insert-node] block-insert-node and block-remove-node commands
>  https://github.com/epilys/qemu/commits/insert-node?author=epilys
> 
> [1] [throttle] add throttle filter driver
>  https://github.com/epilys/qemu/commits/throttle?author=epilys
>  Message-ID: <20170825132028.6184-1-el13...@mail.ntua.gr>
>  https://www.mail-archive.com/qemu-devel@nongnu.org/msg476047.html
> 
> [2] [throttle-remove-legacy] remove legacy throttling interface
>  https://github.com/epilys/qemu/commits/throttle-remove-legacy?author=epilys
> 
> [3] [4] [notify] https://github.com/epilys/qemu/commits/notify?author=epilys
> 
> [5] block-insert-node RFC
>  Message-ID: <20170815074513.9055-1-el13...@mail.ntua.gr>
>  https://www.mail-archive.com/qemu-devel@nongnu.org/msg473619.html

Thanks for your hard work, Manos!

Stefan

Re: [Qemu-devel] [RFC PATCH qemu] exec: Destroy dispatch immediately

[Alexey Kardashevskiy]

On 25/08/17 18:53, Paolo Bonzini wrote:
> On 25/08/2017 10:31, Alexey Kardashevskiy wrote:
>> Otherwise old dispatch holds way too much memory before RCU gets
>> a chance to free old dispatches.
>>
>> Signed-off-by: Alexey Kardashevskiy 
>> ---
>>
>> This is a follow-up to the "Memory use with >100 virtio devices"
>> thread.
>>
>> I assume this is a dirty hack (which fixes the problem though)
> 
> This doesn't work.  AddressSpaceDispatch can be accessed outside the big
> QEMU lock, which is why it is destroyed via call_rcu.
> 
> The solution is to: 1) share the FlatView structures if they refer to
> the same root memory region; 2) have one AddressSpaceDispatch per
> FlatView instead of one per AddressSpace, so that FlatView reference
> counting takes care of clearing the AddressSpaceDispatch too.  Neither
> is particularly hard.  The second requires getting rid of the
> as->dispatch_listener and "hard coding" the creation of the
> AddressSpaceDispatch from the FlatView in memory.c.


While I am trying this approach, here is a cheap workaround -

diff --git a/vl.c b/vl.c
index 8e247cc2a2..4d95bc2a6a 100644
--- a/vl.c
+++ b/vl.c
@@ -4655,12 +4655,16 @@ int main(int argc, char **argv, char **envp)
 igd_gfx_passthru();

 /* init generic devices */
+memory_region_transaction_begin();
+
 rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
 if (qemu_opts_foreach(qemu_find_opts("device"),
   device_init_func, NULL, NULL)) {
 exit(1);
 }

+memory_region_transaction_commit();
+
 cpu_synchronize_all_post_init();

 rom_reset_order_override();



Just for self-education - how dirty is this hack?

This effectively postpones all dispatch trees allocation/disposal till MR
transation depth is 0 (which happens in this
memory_region_transaction_commit()), for 500 virtio devices it reduces the
amount of resident RAM from 45GB to 11GB with 2GB guest, good for speed too
- time to build a machine is reduced from 4:00 to 1:20.




> 
> Paolo
> 
>> and I wonder what the proper solution would be. Thanks.
>>
>>
>> What happens here is that every virtio block device creates 2 address
>> spaces - for modern config space (called "virtio-pci-cfg-as") and
>> for busmaster (common pci thing, called after the device name,
>> in my case "virtio-blk-pci").
>>
>> Each address_space_init() updates topology for _every_ address space.
>> Every topology update (address_space_update_topology()) creates a new
>> dispatch tree - AddressSpaceDispatch with nodes (1KB) and
>> sections (48KB) and destroys the old one.
>>
>> However the dispatch destructor is postponed via RCU which does not
>> get a chance to execute until the machine is initialized but before
>> we get there, memory is not returned to the pool, and this is a lot
>> of memory which grows n^2.
>>
>>
>> Interestingly, mem_add() from exec.c is called twice:
>> as as->dispatch_listener.region_add() and
>> as as->dispatch_listener.region_nop() - I did not understand
>> the trick but it does not work if I remove the .region_nop() hook.
>> How does it work? :)
>>
>> ---
>>  exec.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 01ac21e3cd..ea5f3eb209 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -2707,7 +2707,7 @@ static void mem_commit(MemoryListener *listener)
>>  
>>  atomic_rcu_set(&as->dispatch, next);
>>  if (cur) {
>> -call_rcu(cur, address_space_dispatch_free, rcu);
>> +address_space_dispatch_free(cur);
>>  }
>>  }
>>  
>>
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH 1/4] s390x/pci: fixup trap_msix()

[Cornelia Huck]

On Tue, 29 Aug 2017 16:33:52 +0800
Yi Min Zhao  wrote:

> 在 2017/8/29 下午4:22, Cornelia Huck 写道:
> > On Tue, 29 Aug 2017 16:12:26 +0800
> > Yi Min Zhao  wrote:
> >  
> >> 在 2017/8/29 下午4:00, Cornelia Huck 写道:  
> >>> On Tue, 29 Aug 2017 12:32:17 +0800
> >>> Yi Min Zhao  wrote:
> >>> 
>  在 2017/8/28 下午10:51, Cornelia Huck 写道:  
> > On Mon, 28 Aug 2017 10:04:44 +0200
> > Yi Min Zhao  wrote:
> >
> >> The function trap_msix() is to check if pcistg instruction would access
> >> msix table entries. The correct boundary condition should be
> >> [table_offset, table_offset+entries*entry_size). But the current
> >> condition calculated misses the last entry. So let's fixup it.
> >>
> >> Acked-by: Dong Jia Shi 
> >> Reviewed-by: Pierre Morel 
> >> Signed-off-by: Yi Min Zhao 
> >> ---
> >> hw/s390x/s390-pci-inst.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> >> index b7beb8c36a..eba9ffb5f2 100644
> >> --- a/hw/s390x/s390-pci-inst.c
> >> +++ b/hw/s390x/s390-pci-inst.c
> >> @@ -440,8 +440,8 @@ static int trap_msix(S390PCIBusDevice *pbdev, 
> >> uint64_t offset, uint8_t pcias)
> >> {
> >> if (pbdev->msix.available && pbdev->msix.table_bar == pcias &&
> >> offset >= pbdev->msix.table_offset &&
> >> -offset <= pbdev->msix.table_offset +
> >> -  (pbdev->msix.entries - 1) * PCI_MSIX_ENTRY_SIZE) {
> >> +offset < (pbdev->msix.table_offset +
> >> +  pbdev->msix.entries * PCI_MSIX_ENTRY_SIZE)) {
> >> return 1;
> >> } else {
> >> return 0;  
> > What happened before due to the miscalculation? Write to wrong memory
> > region?
> >
> >
>  We tried to plug virtio-net pci device but failed. After inspected, we
>  found that the device uses two msix entries but the last one was
>  missed. Then we cannot register interrupt successfully because we
>  should call trap_msixi() in order to save some useful and arch
>  information into msix message. But what about wrong memory region
>  didn't happen.  
> >>> So, the guest just was not able to use the second msix entry, but did
> >>> not get any exception?
> >>>
> >>> 
> >> Forget one thing. The zpci idx is saved in msix message. The second msix
> >> entry has not been
> >> trapped so that no idx has been saved, on the other hand idx 0 is saved. So
> >> kvm_arch_fixup_msi_route() will update irq route according to the zpci
> >> device whose idx is 0.
> >> So the wrong logic in trap_msix() will result that flic mixes different
> >> pci devices' adapter interrupts.  
> > Ouch. So this only ever worked for the small subset of pci devices we
> > can passthrough (assuming none of them use more than one msix entry)?  
> Because any passthroughed pci devices which I tested has more than 2 
> msix entries. And not all
> entries will be used. I find that the last entry is never touched. But 
> virtio pci is much fancy and only
> uses two entries. So I encountered the issue when I tested virtio-pci 
> device.

So that really sounds to me like "we've been lucky"...

> >
> > I'm tempted to have this cc:ed to stable so we can fixup 2.10 (which is
> > the first version with usable zpci support).

...and I'll add cc:stable, as we don't really have any control from
qemu which kind of devices are handled by vfio.

Re: [Qemu-devel] [PATCH 01/14] hvf: add support for Hypervisor.framework in the configure script

[Stefan Hajnoczi]

On Sun, Aug 27, 2017 at 08:56:41PM -0500, Sergio Andres Gomez Del Real wrote:
> @@ -3619,6 +3619,16 @@ applicable to MAC and Windows platform, and thus does 
> not conflict with
>  KVM.
>  ETEXI
>  
> +DEF("enable-hvf", 0, QEMU_OPTION_enable_hvf, \
> +"-enable-hvf enable Hypervisor.framework virtualization support\n", 
> QEMU_ARCH_I386)
> +STEXI
> +@item -enable-hvf
> +@findex -enable-hvf
> +Enable Hypervisor.framework support. This option is only available if
> +HVF support is enabled when compiling. HVF is only applicable to MAC
> +platform, and thus does not conflict with KVM.
> +ETEXI

-enable-$ACCEL is a legacy option.  The preferred syntax is -M accel=$ACCL.

There's no need to add -enable-hvf, please remove it in the next
revision of this patch.  Users that want hvf should use the modern -M
accel=hvf syntax.

Besides this:
Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance

[Jason Wang]

On 2017年08月22日 15:16, no-re...@patchew.org wrote:

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1503305719-2512-1-git-send-email-zhangchen.f...@cn.fujitsu.com
Subject: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
 echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
 if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; 
then
 failed=1
 echo
 fi
 n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
37052358b5 net/colo-compare.c: Fix comments and scheme
46824a6565 net/colo-compare.c: Adjust net queue pop order for performance
acea048383 net/colo-compare.c: Optimize unpredictable tcp options comparison

=== OUTPUT BEGIN ===
Checking PATCH 1/3: net/colo-compare.c: Optimize unpredictable tcp options 
comparison...
Checking PATCH 2/3: net/colo-compare.c: Adjust net queue pop order for 
performance...
Checking PATCH 3/3: net/colo-compare.c: Fix comments and scheme...
ERROR: space prohibited after that '-' (ctx:WxW)
#18: FILE: net/colo-compare.c:47:
+  |   conn list   + - >  conn + --- >  conn + -- > ...
  ^

ERROR: space prohibited after that '-' (ctx:OxW)
#18: FILE: net/colo-compare.c:47:
+  |   conn list   + - >  conn + --- >  conn + -- > ...
^

ERROR: space required one side of that '--' (ctx:WxW)
#18: FILE: net/colo-compare.c:47:
+  |   conn list   + - >  conn + --- >  conn + -- > ...
^

total: 3 errors, 0 warnings, 40 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org


Fam, this looks like a false positive since it was actually an ascii 
graph inside a comment?


Thanks

Re: [Qemu-devel] [PATCH] MAINTAINERS: Update mail address for COLO Proxy

[Jason Wang]

On 2017年08月29日 13:31, Zhang Chen wrote:

Hi~

No news for long time.

Ping...


Thanks

Zhang Chen


On 08/23/2017 04:51 PM, Zhang Chen wrote:

My Fujitsu mail account will be disabled soon, update the mail info
to my private mail.

Signed-off-by: Zhang Chen 
---
  MAINTAINERS | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index ccee28b..3b530a7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1561,7 +1561,7 @@ F: include/migration/failover.h
  F: docs/COLO-FT.txt
COLO Proxy
-M: Zhang Chen 
+M: Zhang Chen 
  M: Li Zhijian 
  S: Supported
  F: docs/colo-proxy.txt




Applied.

Thanks

Re: [Qemu-devel] [QGA] Bug of qga?

[Sameeh Jubran]

Hi Sam,

Thanks for reporting this, in order to debug this efficiently I need you to
provide me with more info, if you can provide it that would be great.
Can you reproduce the issue with ncat? You can use ncat to connect to the
socket by running the following command:

nc -U /tmp/qga.sock

Which OS are you running? Which qemu-ga version are you running? can you
reproduce with upstream qemu-ga?

On Tue, Aug 29, 2017 at 9:19 AM, Sam  wrote:

> So how to fix this bug? And where should I to report bug?
>
> 2017-08-29 13:52 GMT+08:00 Sam :
>
> > I could repeat this several times, I think it's mis-order of qmp in qga
> > socket.
> >
> > 2017-08-25 11:09 GMT+08:00 Sam :
> >
> >> Also I found:
> >>
> >> when I use `socat` to take a qga socket, then I use `socat` to
> >> communicate it will got error.
> >> But also SOMETIMES, I will not got error and will communicate OK.
> >>
> >> If one user take qga socket, another user should got error, is it? But
> >> why sometimes, the communicate is OK?
> >>
> >> 2017-08-25 10:11 GMT+08:00 Sam :
> >>
> >>> Hi all,
> >>>
> >>> I'm using qga to send `route -n` and `ping` command to guest. But I
> >>> found SOMETIMES, the second `ping` command's result is the same as
> `route
> >>> -n` command.
> >>>
> >>> So I guess is there some cache mechanism of qga command result? So when
> >>> I send the second command, and receive from qga socket, I receive the
> >>> result of first command.
> >>>
> >>> Or is this bug happened because of I use async mechanism of python code
> >>> to operate qga socket?
> >>>
> >>> This is the python code I use to operate on this qga socket:
> >>>
> >>> try:
>  sock=socket(AF_UNIX, SOCK_STREAM)
>  sock.settimeout(20)
>  sock.connect(vm_qga_sockpath)
>  sock.send(cmd)
>  while True:
>  res = sock.recv(1024)
>  if len(res):
>  break
>  except Exception as e:
>  res = -1
>  finally:
>  sock.settimeout(None)
>  sock.close()
> >>>
> >>>
> >>
> >
>



-- 
Respectfully,
*Sameeh Jubran*
*Linkedin *
*Software Engineer @ Daynix .*

[Qemu-devel] [PATCH v2] qemu-iotests: Extend non-shared storage migration test (194)

[Kashyap Chamarthy]

This is the follow-up patch that was discussed[*] as part of feedback to
qemu-iotest 194.

Changes in this patch:

  - Supply 'job-id' parameter to `drive-mirror` invocation.

  - Issue QMP `block-job-cancel` command on the source QEMU to
gracefully complete `drive-mirror` operation.

  - Stop the NBD server on the destination QEMU.

  - Finally, check for both the events: MIGRATION and
BLOCK_JOB_COMPLETED

With the above, the test will also be (almost) in sync with the
procedure outlined in the document 'live-block-operations.rst'[+]
(section: "QMP invocation for live storage migration with
``drive-mirror`` + NBD").

[*] https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg04820.html
-- qemu-iotests: add 194 non-shared storage migration test
[+] 
https://git.qemu.org/gitweb.cgi?p=qemu.git;a=blob;f=docs/interop/live-block-operations.rst

Signed-off-by: Kashyap Chamarthy 
---
Changes in v2:
 - Check for both the events: MIGRATION and BLOCK_JOB_COMPLETED (Eric)
---
 tests/qemu-iotests/194 | 25 +++--
 tests/qemu-iotests/194.out | 11 ---
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
index 
8028111e21bed5cf4a2e8e32dc04aa5a9ea9caca..9d81189fe3e790d060da5a0fd41836f4b57f95c7
 100755
--- a/tests/qemu-iotests/194
+++ b/tests/qemu-iotests/194
@@ -46,16 +46,17 @@ iotests.log('Launching NBD server on destination...')
 iotests.log(dest_vm.qmp('nbd-server-start', addr={'type': 'unix', 'data': 
{'path': nbd_sock_path}}))
 iotests.log(dest_vm.qmp('nbd-server-add', device='drive0', writable=True))
 
-iotests.log('Starting drive-mirror on source...')
+iotests.log('Starting `drive-mirror` on source...')
 iotests.log(source_vm.qmp(
   'drive-mirror',
   device='drive0',
   target='nbd+unix:///drive0?socket={0}'.format(nbd_sock_path),
   sync='full',
   format='raw', # always raw, the server handles the format
-  mode='existing'))
+  mode='existing',
+  job_id='mirror-job0'))
 
-iotests.log('Waiting for drive-mirror to complete...')
+iotests.log('Waiting for `drive-mirror` to complete...')
 iotests.log(source_vm.event_wait('BLOCK_JOB_READY'),
 filters=[iotests.filter_qmp_event])
 
@@ -66,8 +67,20 @@ dest_vm.qmp('migrate-set-capabilities',
 capabilities=[{'capability': 'events', 'state': True}])
 iotests.log(source_vm.qmp('migrate', 
uri='unix:{0}'.format(migration_sock_path)))
 
+iotests.log('Gracefully ending the `drive-mirror` job on source...')
+iotests.log(source_vm.qmp('block-job-cancel', device='mirror-job0'))
+
+iotests.log('Stopping the NBD server on destination...')
+iotests.log(dest_vm.qmp('nbd-server-stop'))
+
 while True:
-event = source_vm.event_wait('MIGRATION')
-iotests.log(event, filters=[iotests.filter_qmp_event])
-if event['data']['status'] in ('completed', 'failed'):
+event1 = source_vm.event_wait('MIGRATION')
+iotests.log(event1, filters=[iotests.filter_qmp_event])
+if event1['data']['status'] in ('completed', 'failed'):
+break
+
+while True:
+event2 = source_vm.event_wait('BLOCK_JOB_COMPLETED')
+iotests.log(event2, filters=[iotests.filter_qmp_event])
+if event2['event'] == 'BLOCK_JOB_COMPLETED':
 break
diff --git a/tests/qemu-iotests/194.out b/tests/qemu-iotests/194.out
index 
ae501fecacb706b1851cb9063ce9c9d5a28bb7ea..a461d7e21abb2dee54d72d93d644100f9fbcb17d
 100644
--- a/tests/qemu-iotests/194.out
+++ b/tests/qemu-iotests/194.out
@@ -2,12 +2,17 @@ Launching VMs...
 Launching NBD server on destination...
 {u'return': {}}
 {u'return': {}}
-Starting drive-mirror on source...
+Starting `drive-mirror` on source...
 {u'return': {}}
-Waiting for drive-mirror to complete...
-{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': 
{u'device': u'drive0', u'type': u'mirror', u'speed': 0, u'len': 1073741824, 
u'offset': 1073741824}, u'event': u'BLOCK_JOB_READY'}
+Waiting for `drive-mirror` to complete...
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': 
{u'device': u'mirror-job0', u'type': u'mirror', u'speed': 0, u'len': 
1073741824, u'offset': 1073741824}, u'event': u'BLOCK_JOB_READY'}
 Starting migration...
 {u'return': {}}
+Gracefully ending the `drive-mirror` job on source...
+{u'return': {}}
+Stopping the NBD server on destination...
+{u'return': {}}
 {u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': 
{u'status': u'setup'}, u'event': u'MIGRATION'}
 {u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': 
{u'status': u'active'}, u'event': u'MIGRATION'}
 {u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': 
{u'status': u'completed'}, u'event': u'MIGRATION'}
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': 
{u'device': u'mirror-job0', u'type': u'mirror', u'speed': 0, u'len': 
1073741824, u'offset': 1073741824}, 

Re: [Qemu-devel] GSOC Report: Moving I/O throttling and write notifiers into block filter drivers

[Alberto Garcia]

On Tue 29 Aug 2017 10:51:28 AM CEST, Stefan Hajnoczi wrote:

>> Already merged patches in 2.10
>>  https://github.com/qemu/qemu/commits/v2.10.0-rc4?author=epilys
>> Already merged patches for 2.11
>>  https://www.mail-archive.com/qemu-devel@nongnu.org/msg470461.html
>> 
>> [0] [insert-node] block-insert-node and block-remove-node commands
>>  https://github.com/epilys/qemu/commits/insert-node?author=epilys
>> 
>> [1] [throttle] add throttle filter driver
>>  https://github.com/epilys/qemu/commits/throttle?author=epilys
>>  Message-ID: <20170825132028.6184-1-el13...@mail.ntua.gr>
>>  https://www.mail-archive.com/qemu-devel@nongnu.org/msg476047.html
>> 
>> [2] [throttle-remove-legacy] remove legacy throttling interface
>>  https://github.com/epilys/qemu/commits/throttle-remove-legacy?author=epilys
>> 
>> [3] [4] [notify] https://github.com/epilys/qemu/commits/notify?author=epilys
>> 
>> [5] block-insert-node RFC
>>  Message-ID: <20170815074513.9055-1-el13...@mail.ntua.gr>
>>  https://www.mail-archive.com/qemu-devel@nongnu.org/msg473619.html
>
> Thanks for your hard work, Manos!

Indeed, thanks!!

Berto

Re: [Qemu-devel] [PATCH 03/14] hvf: add conditional macros around hvf code in cpus.c

[Stefan Hajnoczi]

On Sun, Aug 27, 2017 at 08:56:43PM -0500, Sergio Andres Gomez Del Real wrote:
> @@ -900,6 +904,11 @@ void cpu_synchronize_all_states(void)
>  
>  CPU_FOREACH(cpu) {
>  cpu_synchronize_state(cpu);
> +#ifdef CONFIG_HVF
> +if (hvf_enabled()) {
> +hvf_cpu_synchronize_state(cpu);
> +}
> +#endif

Searching for CONFIG_KVM and CONFIG_HAX in cpus.c produces no results.
How do they solve the same problem without using #ifdef?  They define
nop stubs in "system/$ACCEL.h" so that cpus.c does not need #ifdefs.

Please follow the same approach for hvf.  Using stubs avoids "bitrot"
because the cpus.c kvm/hax code is always compiled, even when the
feature is disabled.  This way no compiler errors/warnings can hide in
the #ifdef regions that aren't compiled by most people.

Please avoid #ifdef whereever possible.

Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation

[Peter Maydell]

On 26 August 2017 at 01:02, Emilio G. Cota  wrote:
> An additional "nice to have" would be:
>
> * Allow inlining of TCG code by the instrumenter. Example use case:
>   the instrumenter wants to increment a counter every time a
>   basic block is executed. Instead of calling a callback function on every 
> block's
>   execution, we could just have a translation-time callback to emit at the 
> beginning
>   of the translated block the counter increment. This would be much faster, 
> and
>   is something that all other tools (e.g. DynamoRIO/Pin) implement.

This is a feature I would strongly prefer us not to implement.
It exposes too much of QEMU's internals (ie TCG) to the
instrumentation, and it would be pretty complicated to use.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/1] slirp updates

[Peter Maydell]

On 28 August 2017 at 00:05, Samuel Thibault
 wrote:
> warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
> The following changes since commit 04d74e07b4542aad5aa4ad03951b38b767f5314a:
>
>   slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 +0200)
>
> are available in the git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 04d74e07b4542aad5aa4ad03951b38b767f5314a:
>
>   slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 +0200)
>
> 
> slirp updates
>
> 

Is this pull request intended to be for 2.10 (in which
case it needs justification) or for 2.11 (in which case
it's a bit early) ?

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/1] slirp updates

[Samuel Thibault]

Peter Maydell, on mar. 29 août 2017 10:20:31 +0100, wrote:
> On 28 August 2017 at 00:05, Samuel Thibault
>  wrote:
> > warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
> > The following changes since commit 04d74e07b4542aad5aa4ad03951b38b767f5314a:
> >
> >   slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 
> > +0200)
> >
> > are available in the git repository at:
> >
> >   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
> >
> > for you to fetch changes up to 04d74e07b4542aad5aa4ad03951b38b767f5314a:
> >
> >   slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 
> > +0200)
> >
> > 
> > slirp updates
> >
> > 
> 
> Is this pull request intended to be for 2.10 (in which
> case it needs justification) or for 2.11 (in which case
> it's a bit early) ?

It is for 2.10. It fixes at least a DOS: userland can at least crash
qemu with specially-crafted packets.

Samuel

Re: [Qemu-devel] [PATCH 04/14] hvf: add fields to CPUState and CPUX86State; add definitions

[Stefan Hajnoczi]

On Sun, Aug 27, 2017 at 08:56:44PM -0500, Sergio Andres Gomez Del Real wrote:
> This commit adds some fields specific to hvf in CPUState and
> CPUX86State. It also adds some handy #defines.
> 
> Signed-off-by: Sergio Andres Gomez Del Real 
> ---
>  include/qom/cpu.h |  8 
>  target/i386/cpu.h | 23 +++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 25eefea7ab..c46eb61240 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -407,6 +407,14 @@ struct CPUState {
>   * unnecessary flushes.
>   */
>  uint16_t pending_tlb_flush;
> +
> +// HVF

Please see ./CODING_STYLE "7. Comment style":

  We use traditional C-style /* */ comments and avoid // comments.

> +bool hvf_vcpu_dirty;
> +uint64_t hvf_fd; // fd of vcpu created by HVF

File descriptors are ints, why is this uint64_t?

This also raises an issue: none of these fields are used in the patch.
This makes it hard to review the code.  I don't know whether the
definitions are correct without seeing the code that uses them.

Please structure the patch series so that patches are self-contained,
logical changes that can be reviewed in isolation.  These field
definitions should be made in the same patch uses the fields.

> +// Supporting data structures for VMCS capabilities
> +// and x86 emulation state
> +struct hvf_vcpu_caps* hvf_caps;
> +struct hvf_x86_state* hvf_x86;

Coding style:
1. Please use typedef struct {...} TypeName.
2. Please use TypeName *ptr.

>  };
>  
>  QTAILQ_HEAD(CPUTailQ, CPUState);
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 051867399b..7d90f08b98 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -82,15 +82,19 @@
>  #define R_GS 5
>  
>  /* segment descriptor fields */
> +#define DESC_G_SHIFT23
>  #define DESC_G_MASK (1 << 23)
>  #define DESC_B_SHIFT22
>  #define DESC_B_MASK (1 << DESC_B_SHIFT)
>  #define DESC_L_SHIFT21 /* x86_64 only : 64 bit code segment */
>  #define DESC_L_MASK (1 << DESC_L_SHIFT)
> +#define DESC_AVL_SHIFT  20
>  #define DESC_AVL_MASK   (1 << 20)
> +#define DESC_P_SHIFT15
>  #define DESC_P_MASK (1 << 15)
>  #define DESC_DPL_SHIFT  13
>  #define DESC_DPL_MASK   (3 << DESC_DPL_SHIFT)
> +#define DESC_S_SHIFT12
>  #define DESC_S_MASK (1 << 12)

Please reuse the new constants to avoid duplication (just like existing
code for DESC_B_MASK does):

  #define DESC_S_SHIFT12
  #define DESC_S_MASK (1 << DESC_S_SHIFT)

Re: [Qemu-devel] [PATCH 4/4] s390x/pci: add iommu replay callback

[Cornelia Huck]

On Tue, 29 Aug 2017 16:26:10 +0800
Yi Min Zhao  wrote:

> 在 2017/8/29 下午4:07, Cornelia Huck 写道:
> > [Restored cc:s. Please remember to do reply-all.]
> >
> > On Tue, 29 Aug 2017 12:46:51 +0800
> > Yi Min Zhao  wrote:
> >  
> >> 在 2017/8/28 下午11:57, Cornelia Huck 写道:  
> >>> On Mon, 28 Aug 2017 10:04:47 +0200
> >>> Yi Min Zhao  wrote:
> >>>  
>  Let's introduce iommu replay callback for s390 pci iommu memory region.
>  Currently we don't need any dma mapping replay. So let it return
>  directly. This implementation will avoid meaningless loops calling
>  translation callback.
> 
>  Reviewed-by: Pierre Morel 
>  Reviewed-by: Halil Pasic 
>  Signed-off-by: Yi Min Zhao 
>  ---
> hw/s390x/s390-pci-bus.c | 8 
> 1 file changed, 8 insertions(+)
> 
>  diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>  index 9e1f7ff5c5..359509ccea 100644
>  --- a/hw/s390x/s390-pci-bus.c
>  +++ b/hw/s390x/s390-pci-bus.c
>  @@ -407,6 +407,13 @@ static IOMMUTLBEntry 
>  s390_translate_iommu(IOMMUMemoryRegion *mr, hwaddr addr,
> return ret;
> }
> 
>  +static void s390_pci_iommu_replay(IOMMUMemoryRegion *iommu,
>  +  IOMMUNotifier *notifier)
>  +{
>  +/* we don't need iommu replay currently */  
> >>> This really needs to be heavier on the _why_. My guess is that anything
> >>> which would require replay goes through the rpcit instruction?  
> >> My understanding is:
> >> Our arch is different from others. Each pci device has its own iommu, not
> >> like other archs' implementation. So currently there must be no existing
> >> mappings belonging to any newly plugged pci device whose iommu doesn't
> >> have any mapping at the time.  
> > So please put that explanation into the function. (Also, "currently"?
> > Are we expecting it to change?)  
> The iommu replay function is originally introduced for vfio. I think 
> they want to re-build
> the existing mappings because vfio has a copy of mappings in kernel. For 
> our case,
> the mappings would be cleanup when a pci device unplugged, and new mappings
> would be created when a pci device plugged. I think replay only can 
> happen during
> vfio-pci device migration.

So, the base reason is that it is impossible to plug a pci device on
s390x that already has iommu mappings which need to be replayed, which
is due to the "one iommu per zpci device" construct (and independent of
which devices need replay on other architectures)?

Then this should go into the explanation above. (And I'd still like to
know what "currently" refers to. I don't like to rely on some kind of
implicit assumptions - are we expecting this to break?)

> >  
> >> In addition, it's also due to vfio blocking migration. If vfio-pci supports
> >> migration in future, we could implement iommu replay by that time.  
> > That's not an argument: This is the base zpci support, it should not
> > depend on the supported devices and what they do. (What's the status of
> > virtio-pci, btw? Does it work with your patches applied, or is there
> > still more to be done?)
> >
> >  
> My understanding is virtio-pci doesn't need replay. All mappings of any 
> pci device are existing in
> guest memory. Once the mappings is built, all of them could be migrated 
> along with the guest
> system. But I might misunderstand it. Please correct me.

My question was whether virtio-pci works with your patches on top at
all - last time I checked on master, virtio-pci devices failed to
realize with a "msi-x is mandatory" message.

Re: [Qemu-devel] [PATCH 06/14] hvf: add compilation rules to Makefile.objs

[Stefan Hajnoczi]

On Sun, Aug 27, 2017 at 08:56:46PM -0500, Sergio Andres Gomez Del Real wrote:
> This commit adds to target/i386/Makefile.objs the necessary rules so
> that the new files for hvf are compiled by the build system.
> It also adds handling of the -enable-hvf argument in the main function
> in vl.c.
> 
> Signed-off-by: Sergio Andres Gomez Del Real 
> ---
>  target/i386/Makefile.objs | 1 +
>  vl.c  | 7 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/target/i386/Makefile.objs b/target/i386/Makefile.objs
> index 6a26e9d9f0..0bef89c099 100644
> --- a/target/i386/Makefile.objs
> +++ b/target/i386/Makefile.objs
> @@ -12,4 +12,5 @@ obj-$(CONFIG_HAX) += hax-all.o hax-mem.o hax-windows.o
>  endif
>  ifdef CONFIG_DARWIN
>  obj-$(CONFIG_HAX) += hax-all.o hax-mem.o hax-darwin.o
> +obj-$(CONFIG_HVF) += hvf-utils/ hvf-all.o
>  endif

This should be in the commit that imported the .c files.  They should be
compiled right away.

> diff --git a/vl.c b/vl.c
> index 8e247cc2a2..de7d2a3858 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3751,6 +3751,13 @@ int main(int argc, char **argv, char **envp)
>  olist = qemu_find_opts("machine");
>  qemu_opts_parse_noisily(olist, "accel=hax", false);
>  break;
> +#if defined(__APPLE__)
> +case QEMU_OPTION_enable_hvf:
> +olist = qemu_find_opts("machine");
> +qemu_opts_parse_noisily(olist, "accel=hvf", false);
> +hvf_disable(0);
> +break;
> +#endif

This code change is independent of the Makefile.objs change.  It should
be a spearate commit.

As mentioned in another email, -enable-hvf should be dropped.

Re: [Qemu-devel] [PATCH 07/14] hvf: run hvf code through checkpatch.pl and fix style issues

[Stefan Hajnoczi]

On Sun, Aug 27, 2017 at 08:56:47PM -0500, Sergio Andres Gomez Del Real wrote:
> Signed-off-by: Sergio Andres Gomez Del Real 

Please make this the 2nd patch so there are no other patches in between
that modify code that violates coding style.

Re: [Qemu-devel] S390 bios breaks in qemu 2.10.rc3

[Thomas Huth]

On 28.08.2017 09:18, Christian Borntraeger wrote:
> 
> 
> On 08/25/2017 10:29 AM, Cornelia Huck wrote:
>> On Fri, 25 Aug 2017 10:21:58 +0200
>> Christian Borntraeger  wrote:
>>
>>> On 08/25/2017 09:20 AM, Cornelia Huck wrote:
>>
 OK, to recap:

 - the current pre-built bios seems fine
 - rebuilding the bios may yield a version that fails on some systems
   (different compiler?)
 - adding aligned(8) looks like the right thing to do
 - it seems to fix the problem, but on at least one system something
   still seems off (under investigation)  
>>>
>>> Yes. I am out of office today, so for any aligned(8) patch
>>> Acked-by: Christian Borntraeger 
>>> even for 2.10.
>>
>> I fear the 2.10 train has already left the station, but any aligned(8)
>> patch should be cc:stable.
> 
> I think this could be a topic for QEMU summit. Our process of not allowing
> fixes in rcx without requiring an rc(x+1) seems a bit odd. The Linux kernel
> style (there are fixes between the last rc and release) seems more balanced
> as long as we establish some safety nets.

This sounds like a good idea to me, yes. And maybe we could also ease
the situation a little bit by providing the first stable .1 release
already two or three weeks after the .0 release [*] ? Then these "we are
not 100% sure whether this is a severe blocker or not" patches could
simply be provided to the public with that .1 release instead of
blocking the QEMU master branch in freeze state...

 Thomas


[*] I know that means more additional work for Michael - sorry for that
... but at least we should talk about this, I think. Maybe someone else
could also help with the releases if it's too much work for one person?

Re: [Qemu-devel] [PATCH 08/14] apic: add function to apic that will be used by hvf

[Stefan Hajnoczi]

On Sun, Aug 27, 2017 at 08:56:48PM -0500, Sergio Andres Gomez Del Real wrote:
> This commit moves (hides) the function apic_get_highest_priority_irr to
> apic.c and exports it through the interface in apic.h for use by hvf.

I don't see a move.  This patch only adds a new function.  Did you
forget to delete the old function?

> Signed-off-by: Sergio Andres Gomez Del Real 
> ---
>  hw/intc/apic.c | 11 +++
>  include/hw/i386/apic.h |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index fe15fb6024..3de59d07fd 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -305,6 +305,17 @@ static void apic_set_tpr(APICCommonState *s, uint8_t val)
>  }
>  }
>  
> +int apic_get_highest_priority_irr(DeviceState *dev)
> +{
> +APICCommonState *s;
> +
> +if (!dev) {
> +return -1;
> +}
> +s = APIC_COMMON(dev);
> +return get_highest_priority_int(s->irr);
> +}
> +
>  static uint8_t apic_get_tpr(APICCommonState *s)
>  {
>  apic_sync_vapic(s, SYNC_FROM_VAPIC);
> diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h
> index ea48ea9389..a9f6c0aa33 100644
> --- a/include/hw/i386/apic.h
> +++ b/include/hw/i386/apic.h
> @@ -20,6 +20,7 @@ void apic_init_reset(DeviceState *s);
>  void apic_sipi(DeviceState *s);
>  void apic_poll_irq(DeviceState *d);
>  void apic_designate_bsp(DeviceState *d, bool bsp);
> +int apic_get_highest_priority_irr(DeviceState *dev);
>  
>  /* pc.c */
>  DeviceState *cpu_get_current_apic(void);
> -- 
> 2.14.1
> 
> 

Re: [Qemu-devel] [PATCH 10/14] hvf: refactor cpuid code

[Stefan Hajnoczi]

On Sun, Aug 27, 2017 at 08:56:50PM -0500, Sergio Andres Gomez Del Real wrote:
> This commit adds code to request the cpuid features supported by the
> host and hvf; it calls hvf_get_supported_cpuid if hvf is compiled with
> QEMU and enabled.
> 
> Signed-off-by: Sergio Andres Gomez Del Real 
> ---
>  cpus.c|   2 +
>  include/qom/cpu.h |   6 +--
>  include/sysemu/hvf.h  |  18 ++---
>  target/i386/cpu-qom.h |   4 +-
>  target/i386/cpu.c | 108 
> --
>  target/i386/hvf-all.c |  20 +-
>  6 files changed, 113 insertions(+), 45 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 6754ce17cc..2411dfcd3f 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -37,7 +37,9 @@
>  #include "sysemu/hw_accel.h"
>  #include "sysemu/kvm.h"
>  #include "sysemu/hax.h"
> +#ifdef CONFIG_HVF
>  #include "sysemu/hvf.h"
> +#endif

Please avoid #ifdefs.  They are not necessary for kvm or hax.

>  #include "qmp-commands.h"
>  #include "exec/exec-all.h"
>  
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index c46eb61240..ef74c2ce3c 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -408,13 +408,9 @@ struct CPUState {
>   */
>  uint16_t pending_tlb_flush;
>  
> -// HVF
>  bool hvf_vcpu_dirty;
>  uint64_t hvf_fd; // fd of vcpu created by HVF
> -// Supporting data structures for VMCS capabilities
> -// and x86 emulation state
> -struct hvf_vcpu_caps* hvf_caps;
> -struct hvf_x86_state* hvf_x86;
> +struct hvf_x86_state *hvf_x86;

Please squash this with the change that adds these fields.  Patch series
should avoid "code churn" where code is added and then changed again in
the same patch series.  Arrange the patch series in a logical order and
squash down commits using git-rebase -i so new code is introduced in its
final state.  This makes review easier and the git commit history
cleaner (i.e. easier to bisect, backport, and use git-blame(1)).

There are more instances of code churn below.

Re: [Qemu-devel] [Qemu-discuss] changing from net to netdev with vde switches and double nics

[Thomas Huth]

 Hi,

On 28.08.2017 15:58, Paolo wrote:
> Hello everyone,
> 
> I'm at loss as to how to translate from net to netdev syntax the following 
> lines:
> 
> NET0="-net vde,vlan=0,sock=/var/run/vde.ctl00 -net 
> nic,vlan=0,model=rtl8139,macaddr=$NIC0MAC -net 
> tap,vlan=0,ifname=pub2,script=no,downscript=no"
> 
> NET1="-net vde,vlan=1,sock=/var/run/vde.ctl01 -net 
> nic,vlan=1,model=rtl8139,macaddr=$NIC1MAC -net 
> tap,vlan=1,ifname=prv2,script=no,downscript=no"
> 
> Usage for lines above is: qemu blah blah blah $NET0 $NET1 blah blah
> (obviously NIC0MAC and NIC1MAC are declared before in the script)
> 
> these syntax, which has been working flawlessly for ages on my pc, creates 
> two network cards (pubX, prvX) for each one of my virtual machines. Each of 
> the two network cards is attached to a vde switch, so all of pubX cards (X is 
> the machine) are attached to vsw0 (virtaul switch 0) and all of prvX cards 
> are attached to vsw1. The rationale behind this is to attach each qemu 
> machine to a private vlan (172.16.y.z) and to a public vlan (192.168.w.t) at 
> the same time.
> This way the public lan is used when connected to the internet, the private 
> lan works always (i.e. with the router inaccessible) for accessing samba 
> directories on the host (of course, firewalled, ip restricted and so on).
> 
> Translation from net to netdev is simple for the nic (-device) part and for 
> the tap part alike, but how can I tell qemu that the vdeswitch with socket 
> /var/run/vde.ctl00 belongs to the same virtual lan of tap pub2 with device 
> rtl8139 and mac address NIC0MAC?
> 
> Translating from net to netdev we lose the vlan=X info.

That's interesting, I think you're the first person I know in months or
even years who really seems to be using the vlan=x parameter (with x >=
1)...

You're right, you can not translate your setup 1:1 to -netdev anymore,
since there are no 'vlan's (or rather hubs, since 'vlan' is rather a
misnomer here) available with the -netdev parameter.

Question is: Why do you need it at all? I see your point that you want
to have two network cards, but why does each NIC has to be wired to two
host networks (vde *and* tap)? Isn't it sufficient if you connect each
NIC to one VDE network?

 Thomas

Re: [Qemu-devel] [PATCH 4/4] s390x/pci: add iommu replay callback

[Cornelia Huck]

On Tue, 29 Aug 2017 11:33:53 +0200
Cornelia Huck  wrote:

> My question was whether virtio-pci works with your patches on top at
> all - last time I checked on master, virtio-pci devices failed to
> realize with a "msi-x is mandatory" message.

Just checked again, I still get

qemu-system-s390x: -device virtio-rng-pci: MSI-X support is mandatory in the 
S390 architecture

Any clue to what is missing?

Re: [Qemu-devel] [PATCH 00/14] add support for Hypervisor.framework in QEMU

[Stefan Hajnoczi]

On Sun, Aug 27, 2017 at 08:56:40PM -0500, Sergio Andres Gomez Del Real wrote:
> The following patchset adds to QEMU the supporting for macOS's native
> hypervisor, Hypervisor.framework (hvf). The code base is taken from
> Google's Android emulator at
> https://android.googlesource.com/platform/external/qemu/+/emu-master-dev.

What is the status of this feature?  Is it expected to run everything
that TCG can run or are there limitations?  Is live migration supported,
etc?

How do you test hvf?  Are there automated test cases?

Stefan

Re: [Qemu-devel] [PATCH 4/4] s390x/pci: add iommu replay callback

[Yi Min Zhao]

在 2017/8/29 下午5:33, Cornelia Huck 写道:

On Tue, 29 Aug 2017 16:26:10 +0800
Yi Min Zhao  wrote:


在 2017/8/29 下午4:07, Cornelia Huck 写道:

[Restored cc:s. Please remember to do reply-all.]

On Tue, 29 Aug 2017 12:46:51 +0800
Yi Min Zhao  wrote:
  

在 2017/8/28 下午11:57, Cornelia Huck 写道:

On Mon, 28 Aug 2017 10:04:47 +0200
Yi Min Zhao  wrote:
  

Let's introduce iommu replay callback for s390 pci iommu memory region.
Currently we don't need any dma mapping replay. So let it return
directly. This implementation will avoid meaningless loops calling
translation callback.

Reviewed-by: Pierre Morel 
Reviewed-by: Halil Pasic 
Signed-off-by: Yi Min Zhao 
---
hw/s390x/s390-pci-bus.c | 8 
1 file changed, 8 insertions(+)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 9e1f7ff5c5..359509ccea 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -407,6 +407,13 @@ static IOMMUTLBEntry 
s390_translate_iommu(IOMMUMemoryRegion *mr, hwaddr addr,
return ret;
}

+static void s390_pci_iommu_replay(IOMMUMemoryRegion *iommu,

+  IOMMUNotifier *notifier)
+{
+/* we don't need iommu replay currently */

This really needs to be heavier on the _why_. My guess is that anything
which would require replay goes through the rpcit instruction?

My understanding is:
Our arch is different from others. Each pci device has its own iommu, not
like other archs' implementation. So currently there must be no existing
mappings belonging to any newly plugged pci device whose iommu doesn't
have any mapping at the time.

So please put that explanation into the function. (Also, "currently"?
Are we expecting it to change?)

The iommu replay function is originally introduced for vfio. I think
they want to re-build
the existing mappings because vfio has a copy of mappings in kernel. For
our case,
the mappings would be cleanup when a pci device unplugged, and new mappings
would be created when a pci device plugged. I think replay only can
happen during
vfio-pci device migration.

So, the base reason is that it is impossible to plug a pci device on
s390x that already has iommu mappings which need to be replayed, which
is due to the "one iommu per zpci device" construct (and independent of
which devices need replay on other architectures)?

Yes.


Then this should go into the explanation above. (And I'd still like to
know what "currently" refers to. I don't like to rely on some kind of
implicit assumptions - are we expecting this to break?)
As our discussion during internal review, we don't need to replay 
currently because vfio-pci device
doesn't support migration. 'currently' refers to the time of vfio-pci 
device migration block.
Only when vfio-pci supports migration in future, we should fill the 
replaying code.


  

In addition, it's also due to vfio blocking migration. If vfio-pci supports
migration in future, we could implement iommu replay by that time.

That's not an argument: This is the base zpci support, it should not
depend on the supported devices and what they do. (What's the status of
virtio-pci, btw? Does it work with your patches applied, or is there
still more to be done?)

  

My understanding is virtio-pci doesn't need replay. All mappings of any
pci device are existing in
guest memory. Once the mappings is built, all of them could be migrated
along with the guest
system. But I might misunderstand it. Please correct me.

My question was whether virtio-pci works with your patches on top at
all - last time I checked on master, virtio-pci devices failed to
realize with a "msi-x is mandatory" message.


I tested. virtio-pci works fine. The message "msix is mandatory" means 
we only support
msix interrupt. If virtio-pci device (like virtio-rng) doesn't support 
msix, we don't allow it

to plug. I thinik this is not related to iommu replay.

Re: [Qemu-devel] [PATCH 00/14] add support for Hypervisor.framework in QEMU

[Stefan Hajnoczi]

On Sun, Aug 27, 2017 at 08:56:40PM -0500, Sergio Andres Gomez Del Real wrote:
> The following patchset adds to QEMU the supporting for macOS's native
> hypervisor, Hypervisor.framework (hvf). The code base is taken from
> Google's Android emulator at
> https://android.googlesource.com/platform/external/qemu/+/emu-master-dev.

There is no ./MAINTAINERS entry for these new files.

Who will maintain this code?

Stefan

Re: [Qemu-devel] [PATCH 4/4] s390x/pci: add iommu replay callback

[Yi Min Zhao]

在 2017/8/29 下午5:49, Cornelia Huck 写道:

On Tue, 29 Aug 2017 11:33:53 +0200
Cornelia Huck  wrote:


My question was whether virtio-pci works with your patches on top at
all - last time I checked on master, virtio-pci devices failed to
realize with a "msi-x is mandatory" message.

Just checked again, I still get

qemu-system-s390x: -device virtio-rng-pci: MSI-X support is mandatory in the 
S390 architecture

Any clue to what is missing?



virtio-rng-pci doesn't support msix. But msix is required on s390 arch.
So we avoid plugging any pci device without msix support.

Re: [Qemu-devel] [PATCH 4/4] s390x/pci: add iommu replay callback

[Cornelia Huck]

On Tue, 29 Aug 2017 17:51:43 +0800
Yi Min Zhao  wrote:

> 在 2017/8/29 下午5:33, Cornelia Huck 写道:

> > My question was whether virtio-pci works with your patches on top at
> > all - last time I checked on master, virtio-pci devices failed to
> > realize with a "msi-x is mandatory" message.
> >
> >  
> I tested. virtio-pci works fine. The message "msix is mandatory" means 
> we only support
> msix interrupt. If virtio-pci device (like virtio-rng) doesn't support 
> msix, we don't allow it
> to plug.

Ah, that's it (it's a bit strange that not all virtio-pci devices
support msi-x). I can add a virtio-net-pci device just fine.

(Maybe we can enhance the message so that it is clear that it refers to
that particular device?)

> I thinik this is not related to iommu replay.

This question was unrelated to this particular patch, more to the whole
series :)

Re: [Qemu-devel] [RFC] Buffers/caches in VirtIO Balloon driver stats

[Stefan Hajnoczi]

On Sun, Aug 27, 2017 at 11:30:33PM +0200, Tomáš Golembiovský wrote:
> Hi,

I have CCed the relevant mailing lists and people most recently involved
in virtio-balloon discussions.  Hopefully this will help get the right
people to see your questions.

> We'd like to include information about reclaimable memory into the
> statistics in VirtiO Balloon driver. Namely, we'd like to include
> counters for bufferes and caches of Linux kernel. The patch itself is
> pretty trivial -- no problem there. But before we do that I'd like to
> get some input from the QEMU community.
> 
> 1) Is there any reason not to have the stats there?
> 
> 2) Considering the balloon device is multiplatform (Linux, BSD,
> Windows), is there a problem with including buffers/caches? These seem
> to be specific to the Linux virtual memory subsystem. Of course, other
> OSes could just report zeros. Are there some internal stats on those
> OSes that could be filled in? I don't now if such or similar statistic
> are available on BSD. On Windows only SystemCache stat looks like
> something relevant. Anyone familiar with those OSes has any suggestions?
> 
> Tomas
> 
> -- 
> Tomáš Golembiovský 
> 

Re: [Qemu-devel] [PATCH 0/3] scripts: add argparse module for Python 2.6 compatibility

[Stefan Hajnoczi]

On Fri, Aug 25, 2017 at 09:24:36AM -0700, no-re...@patchew.org wrote:
> === OUTPUT BEGIN ===
> Checking PATCH 1/3: scripts: add argparse module for Python 2.6 
> compatibility...
> ERROR: trailing whitespace
> #115: FILE: COPYING.PYTHON:93:
> +Reserved" are retained in Python alone or in any derivative version $
> 
> total: 1 errors, 0 warnings, 2676 lines checked

I'm not going to modify the Python Software Foundation License text for
the sake of checkpatch.pl.  Let's keep it verbatim.

Re: [Qemu-devel] [PATCH 4/4] s390x/pci: add iommu replay callback

[Yi Min Zhao]

在 2017/8/29 下午5:57, Cornelia Huck 写道:

On Tue, 29 Aug 2017 17:51:43 +0800
Yi Min Zhao  wrote:


在 2017/8/29 下午5:33, Cornelia Huck 写道:

My question was whether virtio-pci works with your patches on top at
all - last time I checked on master, virtio-pci devices failed to
realize with a "msi-x is mandatory" message.

  

I tested. virtio-pci works fine. The message "msix is mandatory" means
we only support
msix interrupt. If virtio-pci device (like virtio-rng) doesn't support
msix, we don't allow it
to plug.

Ah, that's it (it's a bit strange that not all virtio-pci devices
support msi-x). I can add a virtio-net-pci device just fine.

(Maybe we can enhance the message so that it is clear that it refers to
that particular device?)
Hum, I think so. But it's not urgent. I could post another patch for 
message enhancement

after this series review. Do you agree?



I thinik this is not related to iommu replay.

This question was unrelated to this particular patch, more to the whole
series :)



Yup.

Re: [Qemu-devel] [PATCH 0/3] scripts: add argparse module for Python 2.6 compatibility

[Stefan Hajnoczi]

On Fri, Aug 25, 2017 at 12:42:44PM -0500, Eric Blake wrote:
> On 08/25/2017 11:40 AM, Peter Maydell wrote:
> > Our choices about our dependencies are generally driven by "what
> > are the versions available on the oldest distros which we wish
> > to support building QEMU on", which typically is whatever the
> > long-term-support versions of Ubuntu, SUSE, Redhat, etc are.
> > 
> > Has somebody checked what that means for our Python version
> > requirements?
> 
> At least this one:
> 
> RHEL/CentOS 6: Python-2.6.6

Thanks for checking this.

Stefan

Re: [Qemu-devel] wrong ioctl error handling on dirty pages sync?

[Stefan Hajnoczi]

On Fri, Aug 25, 2017 at 01:42:55PM +0200, Ján Poctavek wrote:
> Hi guys,
> 
> Maybe it is just my lack of understanding, this seems like a bug to me:
> 
> To get list of dirty pages, qemu calls kvm_vm_ioctl() with
> KVM_GET_DIRTY_LOG:
> https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L494
> 
> and considers the ioctl call failed when -1 is returned.
> 
> But the kvm_vm_ioctl() itself returns -errno, not the -1 on error:
> https://github.com/qemu/qemu/blob/v2.10.0-rc4/accel/kvm/kvm-all.c#L2142
> 
> Thanks in advance for sheding some light into this for me.

Looks like a bug to me.  Do you want to send a patch?

Guidelines on how to submit a patch are here:
http://wiki.qemu.org/Contribute/SubmitAPatch

Stefan

Re: [Qemu-devel] Make NVME device "migratable" (savevm)

[Stefan Hajnoczi]

On Fri, Aug 25, 2017 at 11:39:30AM +0300, Sergei Dyshel wrote:
> Hi all,
> From what I understand, I can't "savevm" a VM that uses NVME device because
> it has ".unmigratable = 1" in the code. What support must be implemented in
> order to make it "migratable"?

CCing Keith Busch, the NVMe maintainer.

$ scripts/get_maintainer.pl -f hw/block/nvme.c
Keith Busch  (supporter:nvme)

Re: [Qemu-devel] [PATCH v11 2/6] ACPI: Add APEI GHES Table Generation support

[Igor Mammedov]

On Fri, 18 Aug 2017 22:23:43 +0800
Dongjiu Geng  wrote:

> This implements APEI GHES Table by passing the error CPER info
> to the guest via a fw_cfg_blob. After a CPER info is recorded, an
> SEA(Synchronous External Abort)/SEI(SError Interrupt) exception
> will be injected into the guest OS.

it's a bit complex patch/functionality so I've just mosty skimmed and
commented only on structure of the patch and changes I'd like to see
so it would be more structured and review-able.

I'd suggest to add doc patch first which will describe how it's
supposed to work between QEMU/firmware/guest OS with expected
flows.
 
> Below is the table layout, the max number of error soure is 11,
> which is classified by notification type.
> 
>  etc/acpi/tables   etc/hardware_errors
> 
> ==
> + +--++--+
> | | HEST ||address   |  
> +--+
> | +--+|registers |  | 
> Error Status |
> | | GHES0|| ++  | 
> Data Block 0 |
> | +--+ +->| |status_address0 |->| 
> ++
> | | .| |  | ++  | 
> |  CPER  |
> | | error_status_address-+-+ +--->| |status_address1 |--+   | 
> |  CPER  |
> | | .|   || ++  |   | 
> |    |
> | | read_ack_register+-+ ||  .   |  |   | 
> |  CPER  |
> | | read_ack_preserve| | |+--+  |   | 
> +-++
> | | read_ack_write   | | | +->| |status_address10|+ |   | 
> Error Status |
> + +--+ | | |  | ++| |   | 
> Data Block 1 |
> | | GHES1| +-+-+->| | ack_value0 || +-->| 
> ++
> + +--+   | |  | ++| | 
> |  CPER  |
> | | .|   | | +--->| | ack_value1 || | 
> |  CPER  |
> | | error_status_address-+---+ | || ++| | 
> |    |
> | | .| | || |  . || | 
> |  CPER  |
> | | read_ack_register+-+-+| ++| 
> +-++
> | | read_ack_preserve| |   +->| | ack_value10|| | 
> |..  |
> | | read_ack_write   | |   |  | ++| | 
> ++
> + +--| |   |  | | 
> Error Status |
> | | ...  | |   |  | | 
> Data Block 10|
> + +--+ |   |  +>| 
> ++
> | | GHES10   | |   || 
> |  CPER  |
> + +--+ |   || 
> |  CPER  |
> | | .| |   || 
> |    |
> | | error_status_address-+-+   || 
> |  CPER  |
> | | .| |
> +-++
> | | read_ack_register+-+
> | | read_ack_preserve|
> | | read_ack_write   |
> + +--+
these diagram shows relations between tables which not necessarily bad
but as layout it's useless.
 * Probably there is not much sense to have HEST table here, it's described
   well enough in spec. You might just put reference here.
 * these diagrams should go into doc/spec patch
 * when you describe layout you need to show what and at what offsets
   in which order in which blob/file is located. See ACPI spec for example
   and/or docs/specs/acpi_nvdimm.txt docs/specs/acpi_mem_hotplug.txt for 
inspiration.

> For GHESv2 error source, the OSPM must acknowledges the error via Read Ack 
> register.
> so user space must check the ack value to avoid read-write race condition.
> 
> Signed-off-by: Dongjiu Geng 
> ---
>  hw/acpi/aml-build.c |   2 +
>  hw/acpi/hest_ghes.c | 345 
> 
>  hw/arm/virt-acpi-build.c|   6 +
>  include/hw/acpi/aml-build.h |   1 +
>  include/hw/acpi/hest_ghes.h |  47 ++
>  5 files changed, 401 insertions(+)
>  create mode 100644 hw/acpi/hest_ghes.c
>  create mode 100644 include/hw/acpi/hest_ghes.h
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 36a6cc4..6849e5f 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> 

Re: [Qemu-devel] S390 bios breaks in qemu 2.10.rc3

[Christian Borntraeger]

On 08/29/2017 11:35 AM, Thomas Huth wrote:
> On 28.08.2017 09:18, Christian Borntraeger wrote:
>>
>>
>> On 08/25/2017 10:29 AM, Cornelia Huck wrote:
>>> On Fri, 25 Aug 2017 10:21:58 +0200
>>> Christian Borntraeger  wrote:
>>>
 On 08/25/2017 09:20 AM, Cornelia Huck wrote:
>>>
> OK, to recap:
>
> - the current pre-built bios seems fine
> - rebuilding the bios may yield a version that fails on some systems
>   (different compiler?)
> - adding aligned(8) looks like the right thing to do
> - it seems to fix the problem, but on at least one system something
>   still seems off (under investigation)  

 Yes. I am out of office today, so for any aligned(8) patch
 Acked-by: Christian Borntraeger 
 even for 2.10.
>>>
>>> I fear the 2.10 train has already left the station, but any aligned(8)
>>> patch should be cc:stable.
>>
>> I think this could be a topic for QEMU summit. Our process of not allowing
>> fixes in rcx without requiring an rc(x+1) seems a bit odd. The Linux kernel
>> style (there are fixes between the last rc and release) seems more balanced
>> as long as we establish some safety nets.
> 
> This sounds like a good idea to me, yes. And maybe we could also ease
> the situation a little bit by providing the first stable .1 release
> already two or three weeks after the .0 release [*] ?

+1

> Then these "we are
> not 100% sure whether this is a severe blocker or not" patches could
> simply be provided to the public with that .1 release instead of
> blocking the QEMU master branch in freeze state...
> 
>  Thomas
> 
> 
> [*] I know that means more additional work for Michael - sorry for that
> ... but at least we should talk about this, I think. Maybe someone else
> could also help with the releases if it's too much work for one person?
> 

Re: [Qemu-devel] [RFC 5/6] migration: store listen task tag

[Daniel P. Berrange]

On Wed, Aug 16, 2017 at 05:47:13PM +0800, Peter Xu wrote:
> On Tue, Aug 15, 2017 at 05:47:08PM +0800, Peter Xu wrote:
> > On Tue, Aug 15, 2017 at 10:27:07AM +0100, Daniel P. Berrange wrote:
> > > On Tue, Aug 15, 2017 at 04:50:06PM +0800, Peter Xu wrote:
> > > > On Tue, Aug 15, 2017 at 09:37:14AM +0100, Daniel P. Berrange wrote:
> > > > > On Tue, Aug 15, 2017 at 02:17:06PM +0800, Peter Xu wrote:
> > > > > > Store the task tag for migration types: tcp/unix/fd/exec in current
> > > > > > MigrationIncomingState struct.
> > > > > > 
> > > > > > For defered migration, no need to store task tag since there is no 
> > > > > > task
> > > > > > running in the main loop at all. For RDMA, let's mark it as todo.
> > > > > > 
> > > > > > Signed-off-by: Peter Xu 
> > > > > > ---
> > > > > >  migration/migration.c | 22 ++
> > > > > >  migration/migration.h |  2 ++
> > > > > >  2 files changed, 20 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > > > index c9b7085..daf356b 100644
> > > > > > --- a/migration/migration.c
> > > > > > +++ b/migration/migration.c
> > > > > > @@ -171,6 +171,7 @@ void migration_incoming_state_destroy(void)
> > > > > >  mis->from_src_file = NULL;
> > > > > >  }
> > > > > >  
> > > > > > +mis->listen_task_tag = 0;
> > > > > >  qemu_event_destroy(&mis->main_thread_load_event);
> > > > > >  }
> > > > > >  
> > > > > > @@ -265,25 +266,31 @@ int 
> > > > > > migrate_send_rp_req_pages(MigrationIncomingState *mis, const char 
> > > > > > *rbname,
> > > > > >  void qemu_start_incoming_migration(const char *uri, Error **errp)
> > > > > >  {
> > > > > >  const char *p;
> > > > > > +guint task_tag = 0;
> > > > > > +MigrationIncomingState *mis = migration_incoming_get_current();
> > > > > >  
> > > > > >  qapi_event_send_migration(MIGRATION_STATUS_SETUP, 
> > > > > > &error_abort);
> > > > > >  if (!strcmp(uri, "defer")) {
> > > > > >  deferred_incoming_migration(errp);
> > > > > >  } else if (strstart(uri, "tcp:", &p)) {
> > > > > > -tcp_start_incoming_migration(p, errp);
> > > > > > +task_tag = tcp_start_incoming_migration(p, errp);
> > > > > >  #ifdef CONFIG_RDMA
> > > > > >  } else if (strstart(uri, "rdma:", &p)) {
> > > > > > +/* TODO: store task tag for RDMA migrations */
> > > > > >  rdma_start_incoming_migration(p, errp);
> > > > > >  #endif
> > > > > >  } else if (strstart(uri, "exec:", &p)) {
> > > > > > -exec_start_incoming_migration(p, errp);
> > > > > > +task_tag = exec_start_incoming_migration(p, errp);
> > > > > >  } else if (strstart(uri, "unix:", &p)) {
> > > > > > -unix_start_incoming_migration(p, errp);
> > > > > > +task_tag = unix_start_incoming_migration(p, errp);
> > > > > >  } else if (strstart(uri, "fd:", &p)) {
> > > > > > -fd_start_incoming_migration(p, errp);
> > > > > > +task_tag = fd_start_incoming_migration(p, errp);
> > > > > >  } else {
> > > > > >  error_setg(errp, "unknown migration protocol: %s", uri);
> > > > > > +return;
> > > > > >  }
> > > > > > +
> > > > > > +mis->listen_task_tag = task_tag;
> > > > > 
> > > > > This is unsafe as currently written. The main loop watches that are
> > > > > associated with these tags are all unregistered when incoming 
> > > > > migration
> > > > > starts. So if you save them, and then unregister later when postcopy
> > > > > is "stuck", then you're likely unregistering a tag associated with
> > > > > some other random part of QEMU, because the saved value is no longer
> > > > > valid.
> > > > 
> > > > IIUC for incoming migration, the watch is not detached until
> > > > migration_fd_process_incoming() completes. So:
> > > > 
> > > > - for precopy, the watch should be detached when migration completes
> > > > 
> > > > - for postcopy, the watch should be detached when postcopy starts,
> > > >   then main loop thread returns, while the ram_load_thread starts to
> > > >   continue the migration.
> > > > 
> > > > So basically the watch is detaching itself after
> > > > migration_fd_process_incoming() completes. To handle that, this patch
> > > > has this change:
> > > > 
> > > > @@ -422,6 +429,13 @@ void migration_fd_process_incoming(QEMUFile *f)
> > > >  co = qemu_coroutine_create(process_incoming_migration_co, f);
> > > >  qemu_coroutine_enter(co);
> > > >  }
> > > > +
> > > > +/*
> > > > + * When reach here, we should not need the listening port any
> > > > + * more. We'll detach the listening task soon, let's reset the
> > > > + * listen task tag.
> > > > + */
> > > > +mis->listen_task_tag = 0;
> > > > 
> > > > Would this make sure the listen_task_tag is always valid if nonzero?
> > > 
> > > Mixing 'return FALSE' from callbacks, with g_source_remove is a recipe
> > > for hard to diagnose bugs, so should be avoided in general.
> > 
> 

Re: [Qemu-devel] About virtio device hotplug in Q35! 【外域邮件.谨慎查阅】

[Bob Chen]

The topology is already having all GPUs directly attached to root bus 0. In
this situation you can't see the LnkSta attribute in any capabilities.

The other way of using emulated switch would somehow show this attribute,
at 8 GT/s, although the real bandwidth is low as usual.

2017-08-23 2:06 GMT+08:00 Michael S. Tsirkin :

> On Tue, Aug 22, 2017 at 10:56:59AM -0600, Alex Williamson wrote:
> > On Tue, 22 Aug 2017 15:04:55 +0800
> > Bob Chen  wrote:
> >
> > > Hi,
> > >
> > > I got a spec from Nvidia which illustrates how to enable GPU p2p in
> > > virtualization environment. (See attached)
> >
> > Neat, looks like we should implement a new QEMU vfio-pci option,
> > something like nvidia-gpudirect-p2p-id=.  I don't think I'd want to
> > code the policy of where to enable it into QEMU or the kernel, so we'd
> > push it up to management layers or users to decide.
> >
> > > The key is to append the legacy pci capabilities list when setting up
> the
> > > hypervisor, with a Nvidia customized capability config.
> > >
> > > I added some hack in hw/vfio/pci.c and managed to implement that.
> > >
> > > Then I found the GPU was able to recognize its peer, and the latency
> has
> > > dropped. ✅
> > >
> > > However the bandwidth didn't improve, but decreased instead. ❌
> > >
> > > Any suggestions?
> >
> > What's the VM topology?  I've found that in a Q35 configuration with
> > GPUs downstream of an emulated root port, the NVIDIA driver in the
> > guest will downshift the physical link rate to 2.5GT/s and never
> > increase it back to 8GT/s.  I believe this is because the virtual
> > downstream port only advertises Gen1 link speeds.
>
>
> Fixing that would be nice, and it's great that you now actually have a
> reproducer that can be used to test it properly.
>
> Exposing higher link speeds is a bit of work since there are now all
> kind of corner cases to cover as guests may play with link speeds and we
> must pretend we change it accordingly.  An especially interesting
> question is what to do with the assigned device when guest tries to play
> with port link speed. It's kind of similar to AER in that respect.
>
> I guess we can just ignore it for starters.
>
> >  If the GPUs are on
> > the root complex (ie. pcie.0) the physical link will run at 2.5GT/s
> > when the GPU is idle and upshift to 8GT/s under load.  This also
> > happens if the GPU is exposed in a conventional PCI topology to the
> > VM.  Another interesting data point is that an older Kepler GRID card
> > does not have this issue, dynamically shifting the link speed under
> > load regardless of the VM PCI/e topology, while a new M60 using the
> > same driver experiences this problem.  I've filed a bug with NVIDIA as
> > this seems to be a regression, but it appears (untested) that the
> > hypervisor should take the approach of exposing full, up-to-date PCIe
> > link capabilities and report a link status matching the downstream
> > devices.
>
>
> > I'd suggest during your testing, watch lspci info for the GPU from the
> > host, noting the behavior of LnkSta (Link Status) to check if the
> > devices gets stuck at 2.5GT/s in your VM configuration and adjust the
> > topology until it works, likely placing the GPUs on pcie.0 for a Q35
> > based machine.  Thanks,
> >
> > Alex
>

Re: [Qemu-devel] [PATCH 16/26] qapi: add conditions to VNC type/commands/events on the schema

[Daniel P. Berrange]

On Thu, Aug 17, 2017 at 09:04:38AM +0200, Markus Armbruster wrote:
> Copying our resident VNC maintainer^Wodd fixer Gerd.
> 
> Also copying Dan for QCryptoCipherAlgorithm.
> 
> Gerd, Dan, this patch is about making VNC support visible in
> query-qmp-schema, by having the QAPI generators generate suitable
> ifdeffery.  Bonus: no need for QMP command stubs for
> !defined(CONFIG_VNC).

[snip]

> * QCryptoCipherAlgorithm
> 
>   This:
> 
> # @des-rfb: RFB specific variant of single DES. Do not use except in VNC.
> 
>   I guess we could compile this out if we wanted to.  I doubt we do, but
>   Dan might have other ideas.

It isn't worth the effort to make that conditionally compiled. We getting
DES from nettle/gcrypt these days, instead of our custom in-tree desrfb.c
file, so its no burden to have it.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance

[Fam Zheng]

On Tue, 08/29 17:01, Jason Wang wrote:
> 
> 
> On 2017年08月22日 15:16, no-re...@patchew.org wrote:
> > Hi,
> > 
> > This series seems to have some coding style problems. See output below for
> > more information:
> > 
> > Type: series
> > Message-id: 1503305719-2512-1-git-send-email-zhangchen.f...@cn.fujitsu.com
> > Subject: [Qemu-devel] [PATCH V4 0/3] Optimize COLO-compare performance
> > 
> > === TEST SCRIPT BEGIN ===
> > #!/bin/bash
> > 
> > BASE=base
> > n=1
> > total=$(git log --oneline $BASE.. | wc -l)
> > failed=0
> > 
> > git config --local diff.renamelimit 0
> > git config --local diff.renames True
> > 
> > commits="$(git log --format=%H --reverse $BASE..)"
> > for c in $commits; do
> >  echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> >  if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback 
> > -; then
> >  failed=1
> >  echo
> >  fi
> >  n=$((n+1))
> > done
> > 
> > exit $failed
> > === TEST SCRIPT END ===
> > 
> > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> > Switched to a new branch 'test'
> > 37052358b5 net/colo-compare.c: Fix comments and scheme
> > 46824a6565 net/colo-compare.c: Adjust net queue pop order for performance
> > acea048383 net/colo-compare.c: Optimize unpredictable tcp options comparison
> > 
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/3: net/colo-compare.c: Optimize unpredictable tcp options 
> > comparison...
> > Checking PATCH 2/3: net/colo-compare.c: Adjust net queue pop order for 
> > performance...
> > Checking PATCH 3/3: net/colo-compare.c: Fix comments and scheme...
> > ERROR: space prohibited after that '-' (ctx:WxW)
> > #18: FILE: net/colo-compare.c:47:
> > +  |   conn list   + - >  conn + --- >  conn + -- > 
> > ...
> >   ^
> > 
> > ERROR: space prohibited after that '-' (ctx:OxW)
> > #18: FILE: net/colo-compare.c:47:
> > +  |   conn list   + - >  conn + --- >  conn + -- > 
> > ...
> > ^
> > 
> > ERROR: space required one side of that '--' (ctx:WxW)
> > #18: FILE: net/colo-compare.c:47:
> > +  |   conn list   + - >  conn + --- >  conn + -- > 
> > ...
> > ^
> > 
> > total: 3 errors, 0 warnings, 40 lines checked
> > 
> > Your patch has style problems, please review.  If any of these errors
> > are false positives report them to the maintainer, see
> > CHECKPATCH in MAINTAINERS.
> > 
> > === OUTPUT END ===
> > 
> > Test command exited with code: 1
> > 
> > 
> > ---
> > Email generated automatically by Patchew [http://patchew.org/].
> > Please send your feedback to patchew-de...@freelists.org
> 
> Fam, this looks like a false positive since it was actually an ascii graph
> inside a comment?
> 

Yes, let's just ignore this report.

Fam

Re: [Qemu-devel] [PATCH 16/26] qapi: add conditions to VNC type/commands/events on the schema

[Marc-André Lureau]

Hi

- Original Message -
> On Thu, Aug 17, 2017 at 09:04:38AM +0200, Markus Armbruster wrote:
> > Copying our resident VNC maintainer^Wodd fixer Gerd.
> > 
> > Also copying Dan for QCryptoCipherAlgorithm.
> > 
> > Gerd, Dan, this patch is about making VNC support visible in
> > query-qmp-schema, by having the QAPI generators generate suitable
> > ifdeffery.  Bonus: no need for QMP command stubs for
> > !defined(CONFIG_VNC).
> 
> [snip]
> 
> > * QCryptoCipherAlgorithm
> > 
> >   This:
> >  
> > # @des-rfb: RFB specific variant of single DES. Do not use except in
> > VNC.
> > 
> >   I guess we could compile this out if we wanted to.  I doubt we do, but
> >   Dan might have other ideas.
> 
> It isn't worth the effort to make that conditionally compiled. We getting
> DES from nettle/gcrypt these days, instead of our custom in-tree desrfb.c
> file, so its no burden to have it.

fyi, the last iteration of the patch made it conditional too.

Re: [Qemu-devel] [PATCH v4] vl: exit if maxcpus is negative

[Fam Zheng]

On Tue, 08/29 11:15, Seeteena Thoufeek wrote:
> ---Steps to Reproduce---
> 
> When passed a negative number to 'maxcpus' parameter, Qemu aborts
> with a core dump.
> 
> Run the following command with maxcpus argument as negative number
> 
> ppc64-softmmu/qemu-system-ppc64 --nographic -vga none -machine
> pseries,accel=kvm,kvm-type=HV -m size=200g -device virtio-blk-pci,
> drive=rootdisk -drive file=/home/images/pegas-1.0-ppc64le.qcow2,
> if=none,cache=none,id=rootdisk,format=qcow2 -monitor telnet
> :127.0.0.1:1234,server,nowait -net nic,model=virtio -net
> user -redir tcp:2000::22 -device nec-usb-xhci -smp 8,cores=1,
> threads=1,maxcpus=-12
> 
> (process:12149): GLib-ERROR **: gmem.c:130: failed to allocate
>  18446744073709550568 bytes
> 
> Trace/breakpoint trap
> 
> Reported-by: R.Nageswara Sastry 
> Signed-off-by: Seeteena Thoufeek 
> ---
> v1 -> v2:
>   - Fix the error check in vl.c to make it generic.
> v2 -> v3:
>   - Fix coding style pointed out by patchew.
>   - Fix check for "<= 0" instead of just "< 0".
> v3 -> v4:
>   - Fix subject line.
>   - Removed space before ":" from vl.c:1248
>   - Removed Reviewed-by: flag.
> ---
>  vl.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/vl.c b/vl.c
> index 8e247cc..2d9e73d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1244,7 +1244,10 @@ static void smp_parse(QemuOpts *opts)
>  }
>  
>  max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
> -
> +if (max_cpus <= 0) {
> +error_report("Invalid max_cpus: %d", max_cpus);
> +exit(1);
> +}
>  if (max_cpus < cpus) {
>  error_report("maxcpus must be equal to or greater than smp");
>  exit(1);
> -- 
> 1.8.3.1
> 
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v4 08/10] Makefile: Add rules to run vm tests

[Fam Zheng]

On Mon, 08/28 17:18, Philippe Mathieu-Daudé wrote:
> On 08/28/2017 02:47 PM, Fam Zheng wrote:
> > Signed-off-by: Fam Zheng 
> > Reviewed-by: Stefan Hajnoczi 
> > ---
> >   Makefile  |  2 ++
> >   configure |  2 +-
> >   tests/vm/Makefile.include | 40 
> >   3 files changed, 43 insertions(+), 1 deletion(-)
> >   create mode 100644 tests/vm/Makefile.include
> > 
> > diff --git a/Makefile b/Makefile
> > index 81447b1f08..2798a5ca69 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -813,6 +813,7 @@ endif
> >   -include $(wildcard *.d tests/*.d)
> >   include $(SRC_PATH)/tests/docker/Makefile.include
> > +include $(SRC_PATH)/tests/vm/Makefile.include
> >   .PHONY: help
> >   help:
> > @@ -836,6 +837,7 @@ help:
> > @echo  'Test targets:'
> > @echo  '  check   - Run all tests (check-help for details)'
> > @echo  '  docker  - Help about targets running tests inside 
> > Docker containers'
> > +   @echo  '  vm-test - Help about targets running tests inside VM'
> > @echo  ''
> > @echo  'Documentation targets:'
> > @echo  '  html info pdf txt'
> > diff --git a/configure b/configure
> > index dd73cce62f..9a3052e9ad 100755
> > --- a/configure
> > +++ b/configure
> > @@ -6544,7 +6544,7 @@ if test "$ccache_cpp2" = "yes"; then
> >   fi
> >   # build tree in object directory in case the source is not in the current 
> > directory
> > -DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos 
> > tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
> > +DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos 
> > tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm"
> >   DIRS="$DIRS docs docs/interop fsdev"
> >   DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
> >   DIRS="$DIRS roms/seabios roms/vgabios"
> > diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
> > new file mode 100644
> > index 00..6e133ae2a6
> > --- /dev/null
> > +++ b/tests/vm/Makefile.include
> > @@ -0,0 +1,40 @@
> > +# Makefile for VM tests
> > +
> > +.PHONY: vm-build-all
> > +
> > +IMAGES := ubuntu.i386 freebsd netbsd openbsd
> > +IMAGE_FILES := $(patsubst %, tests/vm/%.img, $(IMAGES))
> > +
> > +.PRECIOUS: $(IMAGE_FILES)
> > +
> > +vm-test:
> > +   @echo "vm-test: Test QEMU in preconfigured virtual machines"
> > +   @echo
> > +   @echo "  vm-build-ubuntu.i386- Build QEMU in ubuntu i386 VM"
> > +   @echo "  vm-build-freebsd- Build QEMU in FreeBSD VM"
> > +   @echo "  vm-build-netbsd - Build QEMU in NetBSD VM"
> > +   @echo "  vm-build-freebsd- Build QEMU in OpenBSD VM"
> 
> vm-build-openbsd ;)

Yes, thanks!

> 
> is it possible to specify OS version?
> 
> like "vm-build-ubuntu:16.04@i386" and having a default:
> 
> "vm-build-ubuntu@i386" -> "vm-build-ubuntu:16.04@i386"

Maybe, when we do have more than one options. :)

Fam

> 
> > +
> > +vm-build-all: $(addprefix vm-build-, $(IMAGES))
> > +
> > +tests/vm/%.img: $(SRC_PATH)/tests/vm/%
> > +   $(call quiet-command, \
> > +   $(SRC_PATH)/tests/vm/$* \
> > +   $(if $(V)$(DEBUG), --debug) \
> > +   --image "$@" \
> > +   --force \
> > +   --build-image $@, \
> > +   "  VM-IMAGE $*")
> > +
> > +
> > +# Build in VM $(IMAGE)
> > +vm-build-%: tests/vm/%.img
> > +   $(call quiet-command, \
> > +   $(SRC_PATH)/tests/vm/$* \
> > +   $(if $(V)$(DEBUG), --debug) \
> > +   $(if $(DEBUG), --interactive) \
> > +   $(if $(J),--jobs $(J)) \
> > +   --image "$<" \
> > +   --build-qemu $(SRC_PATH), \
> > +   "  VM-BUILD $*")
> > +
> > 
> 

Re: [Qemu-devel] [PATCH for-2.11 6/6] ppc: drop caching ObjectClass from PowerPCCPUAlias

[David Gibson]

On Fri, Aug 25, 2017 at 09:49:48AM +0200, Igor Mammedov wrote:
> On Fri, 25 Aug 2017 14:22:03 +1000
> David Gibson  wrote:
> 
> > On Thu, Aug 24, 2017 at 10:21:51AM +0200, Igor Mammedov wrote:
> > > Caching there practically doesn't give any benefits
> > > and that at slow path druring querying supported CPU list.
> > > But it introduces non conventional path of where from
> > > comes used CPU type name (kvm_ppc_register_host_cpu_type).
> > > 
> > > Taking in account that kvm_ppc_register_host_cpu_type()
> > > fixes up models the aliases point to, it's sufficient to
> > > make ppc_cpu_class_by_name() translate cpu alias to
> > > correct cpu type name.
> > > So drop PowerPCCPUAlias::oc field + ppc_cpu_class_by_alias()
> > > and let ppc_cpu_class_by_name() do conversion to cpu type name,
> > > which simplifies code a little bit saving ~20LOC and trouble
> > > wondering why ppc_cpu_class_by_alias() is necessary.
> > > 
> > > Signed-off-by: Igor Mammedov   
> > 
> > Unfortunately, this will break things.  This isn't purely a cache, in
> > the case handled by kvm_ppc_register_host_cpu_type(), 'oc' is *not*
> > redundant with the name.  The name is based on the specific CPU
> > description, but the oc points to the information for the "host" cpu.
> > 
> > There may well be a better way to do this, but this isn't it.
> > 
> > The problem we're trying to solve here is that KVM HV basically only
> > works with -cpu host.  But for the benefit of libvirt (and others), we
> > want, say, -cpu POWER8 to also work if the host *is* a POWER8.  But we
> > *don't* want to require that the host be exactly the same model of
> > POWER8 as "POWER8" would be aliased to with TCG.
> 
> here is snippet from kvm_ppc_register_host_cpu_type()
> 
> 1:
> ppc_cpu_aliases[i].model = g_strdup(object_class_get_name(oc));   
>
> suffix = strstr(ppc_cpu_aliases[i].model, "-"TYPE_POWERPC_CPU);   
>
> if (suffix) { 
>
> *suffix = 0;  
>
> }
> 2:
> ppc_cpu_aliases[i].oc = oc;
> 
> where model is fixed up (1) to the name that corresponds to
> exactly the same 'oc' that is cached into (2)
> 
> any follow up attempt to translate fixed up alias will end up resolving
> it into model => the same 'oc' that would be cached in ppc_cpu_aliases[i].oc
> 
> isn't it?


You're right, I misread the code, sorry.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.11 v2] hw/ppc: CAS reset on early device hotplug

[David Gibson]

On Fri, Aug 25, 2017 at 06:11:19PM -0300, Daniel Henrique Barboza wrote:
> This patch is a follow up on the discussions made in patch
> "hw/ppc: disable hotplug before CAS is completed" that can be
> found at [1].
> 
> At this moment, we do not support CPU/memory hotplug in early
> boot stages, before CAS. When a hotplug occurs, the event is logged
> in an internal RTAS event log queue and an IRQ pulse is fired. In
> regular conditions, the guest handles the interrupt by executing
> check_exception, fetching the generated hotplug event and enabling
> the device for use.
> 
> In early boot, this IRQ isn't caught (SLOF does not handle hotplug
> events), leaving the event in the rtas event log queue. If the guest
> executes check_exception due to another hotplug event, the re-assertion
> of the IRQ ends up de-queuing the first hotplug event as well. In short,
> a device hotplugged before CAS is considered coldplugged by SLOF.
> This leads to device misbehavior and, in some cases, guest kernel
> Ooops when trying to unplug the device.
> 
> A proper fix would be to turn every device hotplugged before CAS
> as a colplugged device. This is not trivial to do with the current
> code base though - the FDT is written in the guest memory at
> ppc_spapr_reset and can't be retrieved without adding extra state
> (fdt_size for example) that will need to managed and migrated. Adding
> the hotplugged DT in the middle of CAS negotiation via the updated DT
> tree works with CPU devs, but panics the guest kernel at boot. Additional
> analysis would be necessary for LMBs and PCI devices. There are
> questions to be made in QEMU/SLOF/kernel level about how we can make
> this change in a sustainable way.
> 
> Until we go all the way with the proper fix, this patch works around
> the situation by issuing a CAS reset if a hotplugged device is detected
> during CAS:
> 
> - the DRC conditions that warrant a CAS reset is the same as those that
> triggers a DRC migration - the DRC must have a device attached and
> the DRC state is not equal to its ready_state. With that in mind, this
> patch makes use of 'spapr_drc_needed' to determine if a CAS reset
> is needed.
> 
> - In the middle of CAS negotiations, the function
> 'spapr_hotplugged_dev_before_cas' goes through all the DRCs to see
> if there are any DRC that requires a reset, using spapr_drc_needed. If
> that happens, returns '1' in 'spapr_h_cas_compose_response' which will set
> spapr->cas_reboot to true, causing the machine to reboot.
> 
> - a small fix was made in 'spapr_drc_needed' to change how we detect
> a DRC device. Using dr_entity_sense worked for physical DRCs but,
> for logical DRCs, it didn't cover the case where a logical DRC has
> a drc->dev but the state is LOGICAL_UNUSABLE (e.g. a hotplugged CPU before
> CAS). In this case, the dr_entity_sense of this DRC returns UNUSABLE and
> spapr_drc_needed was return 'false' for a scenario what we would like
> to migrate the DRC (or issue a CAS reset). Changing it to check for
> drc->dev instead works for all DRC types.
> 
> - a new function called 'spapr_clear_pending_events' was created
> and is being called inside ppc_spapr_reset. This function clears
> the pending_events QTAILQ that holds the RTAS event logs. This prevents
> old/deprecated events from persisting after a reset.
> 
> No changes are made for coldplug devices.
> 
> [1] http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02855.html
> 
> Signed-off-by: Daniel Henrique Barboza 

Sorry I've taken a while to review.

> ---
>  hw/ppc/spapr.c | 34 ++
>  hw/ppc/spapr_drc.c |  5 ++---
>  include/hw/ppc/spapr_drc.h |  1 +
>  3 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index fb1e5e0..4b23ad3 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -790,6 +790,26 @@ out:
>  return ret;
>  }
>  
> +static bool spapr_hotplugged_dev_before_cas(void)
> +{
> +Object *drc_container, *obj;
> +ObjectProperty *prop;
> +ObjectPropertyIterator iter;
> +
> +drc_container = container_get(object_get_root(), "/dr-connector");
> +object_property_iter_init(&iter, drc_container);
> +while ((prop = object_property_iter_next(&iter))) {
> +if (!strstart(prop->type, "link<", NULL)) {
> +continue;
> +}
> +obj = object_property_get_link(drc_container, prop->name, NULL);
> +if (spapr_drc_needed(obj)) {
> +return true;
> +}
> +}
> +return false;
> +}
> +
>  int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
>   target_ulong addr, target_ulong size,
>   sPAPROptionVector *ov5_updates)
> @@ -797,6 +817,10 @@ int spapr_h_cas_compose_response(sPAPRMachineState 
> *spapr,
>  void *fdt, *fdt_skel;
>  sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
>  
> +if (spapr_hotplugged_dev_before_cas()) {
> +  

Re: [Qemu-devel] [Qemu-ppc] [PATCH 00/15] Sam460ex emulation

[David Gibson]

On Sun, Aug 27, 2017 at 02:34:14PM +0200, BALATON Zoltan wrote:
> Hello,
> 
> Just to confirm where we are with this series, let me summarise what I got
> from the replies and what's my plan for this based on that. Here's the list
> of patches for reference:
> 
> [PATCH 01/15] ppc4xx: Move MAL from ppc405_uc to ppc4xx_devs
> [PATCH 02/15] ppc4xx: Make MAL emulation more generic
> [PATCH 03/15] ohci: Allow sysbus version to be used as a companion
> [PATCH 04/15] ehci: Add ppc4xx-ehci for the USB 2.0 controller in embedded 
> PPC SoCs
> [PATCH 05/15] ppc4xx: Split off 4xx I2C emulation from ppc405_uc to its own 
> file
> [PATCH 06/15] ppc4xx_i2c: QOMify
> [PATCH 07/15] ppc4xx_i2c: Move to hw/i2c
> [PATCH 08/15] ppc4xx_i2c: Implement basic I2C functions
> [PATCH 09/15] hw/ide: Emulate SiI3112 SATA controller
> [PATCH 10/15] ppc440: Add emulation of plb-pcix controller found in some 440 
> SoCs
> [PATCH 11/15] ppc: Add 460EX embedded CPU
> [PATCH 12/15] ppc4xx: Export ECB and PLB emulation
> [PATCH 13/15] ppc4xx: Add more PLB registers
> [PATCH 14/15] ppc4xx: Add device models found in PPC440 core SoCs
> [PATCH 15/15] ppc: Add aCube Sam460ex board
> 
> - I think 1-7 is either already queued for 2.11 or could be applied as is
> having got reviews without need to change so I don't need to resend them. So
> are 11-13 if I got that right. That leaves 8-10 and 14-15 that I'll resend.

The way to handle this is to pull the 'ppc-for-2.11' tree from
git://github.com/dgibson/qemu.git, then rebase your series on top of
it.  That should automatically filter out the patches I've already applied.

> - I'm not sure if 8 was reviewed or do I need to make any modifications to
> it.

I think I didn't get to it.  Just include it in the next spin and
we'll see how we go.

> 
> - I know I should change 9-10 and 14-15 based on review comments and resend.
> 
> Since I'll likely also need to change those patches to fix the bugs we know
> about currently, I thought I should probably resend after those bugs are
> fixed to avoid subsequent fixup patches. Since this will not be before next
> week (because I don't have time for it before that) I hope development
> should be open again and those patches that are already OK could be merged
> by then so I can rebase on that and only resend the missing ones as
> described above. Is that OK or did I miss something?

2.11 dev should open soon, but I don't know how long it will take to
get my already queued patches merged there.

> David, in case you want to stage all of this in your for-2.11 branch before
> all patches are ready and send them together, then where is this branch I
> should rebase to?

As above, use the 'ppc-for-2.11' branch at
git://github.com/dgibson/qemu.git.  Note that this is a rebasing branch.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC v2 0/8] monitor: allow per-monitor thread

[Daniel P. Berrange]

On Wed, Aug 23, 2017 at 02:51:03PM +0800, Peter Xu wrote:
> v2:
> - fixed "make check" error that patchew reported
> - moved the thread_join upper in monitor_data_destroy(), before
>   resources are released
> - added one new patch (current patch 3) that fixes a nasty risk
>   condition with IOWatchPoll.  Please see commit message for more
>   information.
> - added a g_main_context_wakeup() to make sure the separate loop
>   thread can be kicked always when we want to destroy the per-monitor
>   threads.
> - added one new patch (current patch 8) to introduce migration mgmt
>   lock for migrate_incoming.
> 
> This is an extended work for migration postcopy recovery. This series
> is tested with the following series to make sure it solves the monitor
> hang problem that we have encountered for postcopy recovery:
> 
>   [RFC 00/29] Migration: postcopy failure recovery
>   [RFC 0/6] migration: re-use migrate_incoming for postcopy recovery
> 
> The root problem is that, monitor commands are all handled in main
> loop thread now, no matter how many monitors we specify. And, if main
> loop thread hangs due to some reason, all monitors will be stuck.
> This can be done in reversed order as well: if any of the monitor
> hangs, it will hang the main loop, and the rest of the monitors (if
> there is any).
> 
> That affects postcopy recovery, since the recovery requires user input
> on destination side.  If monitors hang, the destination VM dies and
> lose hope for even a final recovery.
> 
> So, sometimes we need to make sure the monitor be alive, at least one
> of them.
> 
> The whole idea of this series is that instead if handling monitor
> commands all in main loop thread, we do it separately in per-monitor
> threads.  Then, even if main loop thread hangs at any point by any
> reason, per-monitor thread can still survive.  Further, we add hint in
> QMP/HMP to show whether a command can be executed without QMP, if so,
> we avoid taking BQL when running that command.  It greatly reduced
> contention of BQL.  Now the only user of that new parameter (currently
> I call it "without-bql") is "migrate-incoming" command, which is the
> only command to rescue a paused postcopy migration.
> 
> However, even with the series, it does not mean that per-monitor
> threads will never hang.  One example is that we can still run "info
> vcpus" in per-monitor threads during a paused postcopy (in that state,
> page faults are never handled, and "info cpus" will never return since
> it tries to sync every vcpus).  So to make sure it does not hang, we
> not only need the per-monitor thread, the user should be careful as
> well on how to use it.
> 
> For postcopy recovery, we may need dedicated monitor channel for
> recovery.  In other words, a destination VM that supports postcopy
> recovery would possibly need:
> 
>   -qmp MAIN_CHANNEL -qmp RECOVERY_CHANNEL

I think this is a really horrible thing to expose to management applications.
They should not need to be aware of fact that QEMU is buggy and thus requires
that certain commands be run on different monitors to work around the bug.

I'd much prefer to see the problem described handled transparently inside
QEMU. One approach is have a dedicated thread in QEMU responsible for all
monitor I/O. This thread should never actually execute monitor commands
though, it would simply parse the command request and put data onto a queue
of pending commands, thus it could never hang. The command queue could be
processed by the main thread, or by another thread that is interested.
eg the migration thread could process any queued commands related to
migration directly.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for-2.11 v2] hw/ppc: CAS reset on early device hotplug

[David Gibson]

On Fri, Aug 25, 2017 at 06:11:18PM -0300, Daniel Henrique Barboza wrote:
> v2:
> - rebased with ppc-for-2.11
> - function 'spapr_cas_completed' dropped
> - function 'spapr_drc_needed' made public and it's now used inside
>   'spapr_hotplugged_dev_before_cas'
> - 'spapr_drc_needed' was changed to support the migration of logical
>   DRCs with devs attached in UNUSED state
> - new function: 'spapr_clear_pending_events'. This function is used
>   inside ppc_spapr_reset to reset the pending_events QTAILQ

Thanks for the followup, unfortunately there is still an important bug
left, see comments on the patch itself.

At a higher level, though, looking at the event reset code made me
think of a possible even simpler solution to this problem.

The queue of events (both hotplug and epow) is already in a simple
internal form that's independent of the two delivery mechanisms.  The
only difference is what event source triggers the interrupt.  This
explains why an extra hotplug event after the CAS "unstuck" the queue.

AFAICT, a spurious interrupts here should be harmless - the kernel
will just check the queue and find nothing there.

So, it should be sufficient to, after CAS, pulse the hotplug queue
interrupt if the hotplug queue is negotiated.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2] spapr: fallback to raw mode if best compat mode cannot be set during CAS

[David Gibson]

On Thu, Aug 17, 2017 at 01:23:50PM +0200, Greg Kurz wrote:
> KVM PR doesn't allow to set a compat mode. This causes ppc_set_compat_all()
> to fail and we return H_HARDWARE to the guest right away.
> 
> This is excessive: even if we favor compat mode since commit 152ef803ceb19,
> we should at least fallback to raw mode if the guest supports it.
> 
> This patch modifies cas_check_pvr() so that it also reports that the real
> PVR was found in the table supplied by the guest. Note that this is only
> makes sense if raw mode isn't explicitely disabled (ie, the user didn't
> set the machine "max-cpu-compat" property). If this is the case, we can
> simply ignore ppc_set_compat_all() failures, and let the guest run in raw
> mode.
> 
> Signed-off-by: Greg Kurz 
> ---
> v2: - initialize raw_mode_supported to silent patchew

I hesitated about this, because making guest visible changes based on
host abilities can get really messy.  However we already have a bunch
of things like that in CAS, and we already migrate the compatibility
state, so I think it should be ok.

Applied to ppc-for-2.11.

> ---
>  hw/ppc/spapr_hcall.c |   18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 07b3da8dc4cd..2f4c4f59e110 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1441,7 +1441,8 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
>  }
>  
>  static uint32_t cas_check_pvr(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> -  target_ulong *addr, Error **errp)
> +  target_ulong *addr, bool *raw_mode_supported,
> +  Error **errp)
>  {
>  bool explicit_match = false; /* Matched the CPU's real PVR */
>  uint32_t max_compat = spapr->max_compat_pvr;
> @@ -1481,6 +1482,8 @@ static uint32_t cas_check_pvr(sPAPRMachineState *spapr, 
> PowerPCCPU *cpu,
>  return 0;
>  }
>  
> +*raw_mode_supported = explicit_match;
> +
>  /* Parsing finished */
>  trace_spapr_cas_pvr(cpu->compat_pvr, explicit_match, best_compat);
>  
> @@ -1499,8 +1502,9 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>  sPAPROptionVector *ov1_guest, *ov5_guest, *ov5_cas_old, *ov5_updates;
>  bool guest_radix;
>  Error *local_err = NULL;
> +bool raw_mode_supported = false;
>  
> -cas_pvr = cas_check_pvr(spapr, cpu, &addr, &local_err);
> +cas_pvr = cas_check_pvr(spapr, cpu, &addr, &raw_mode_supported, 
> &local_err);
>  if (local_err) {
>  error_report_err(local_err);
>  return H_HARDWARE;
> @@ -1510,8 +1514,14 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>  if (cpu->compat_pvr != cas_pvr) {
>  ppc_set_compat_all(cas_pvr, &local_err);
>  if (local_err) {
> -error_report_err(local_err);
> -return H_HARDWARE;
> +/* We fail to set compat mode (likely because running with KVM 
> PR),
> + * but maybe we can fallback to raw mode if the guest supports 
> it.
> + */
> +if (!raw_mode_supported) {
> +error_report_err(local_err);
> +return H_HARDWARE;
> +}
> +local_err = NULL;
>  }
>  }
>  
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.11 2/6] ppc: make cpu_model translation to type consistent

[David Gibson]

On Fri, Aug 25, 2017 at 04:34:26PM +0200, Igor Mammedov wrote:
> On Fri, 25 Aug 2017 23:28:00 +1000
> David Gibson  wrote:
> 
> > On Fri, Aug 25, 2017 at 01:40:07PM +0200, Igor Mammedov wrote:
> > > On Fri, 25 Aug 2017 19:45:38 +1000
> > > David Gibson  wrote:
> > >   
> > > > On Fri, Aug 25, 2017 at 09:27:40AM +0200, Igor Mammedov wrote:  
> > > > > On Fri, 25 Aug 2017 11:38:19 +1000
> > > > > David Gibson  wrote:
> > > > > 
> > > > > > On Thu, Aug 24, 2017 at 10:21:47AM +0200, Igor Mammedov wrote:
> > > > > > > PPC handles -cpu FOO rather incosistently,
> > > > > > > i.e. it does case-insensitive matching of FOO to
> > > > > > > a CPU type (see: ppc_cpu_compare_class_name) but
> > > > > > > handles alias names as case-sensitive, as result:
> > > > > > > 
> > > > > > >  # qemu-system-ppc64 -M mac99 -cpu g3
> > > > > > >  qemu-system-ppc64: unable to find CPU model ' kN�U'
> > > > > > > 
> > > > > > >  # qemu-system-ppc64 -cpu 970MP_V1.1
> > > > > > >  qemu-system-ppc64: Unable to find sPAPR CPU Core definition
> > > > > > > 
> > > > > > > while
> > > > > > > 
> > > > > > >  # qemu-system-ppc64 -M mac99 -cpu G3
> > > > > > >  # qemu-system-ppc64 -cpu 970MP_v1.1
> > > > > > > 
> > > > > > > start up just fine.
> > > > > > > 
> > > > > > > Considering we can't take case-insensitive matching away,
> > > > > > > make it case-insensitive for  all alias/type/core_type
> > > > > > > lookups.
> > > > > > > 
> > > > > > > As side effect it allows to remove duplicate core types
> > > > > > > which are the same but use lower-case letters in name.
> > > > > > > 
> > > > > > > Signed-off-by: Igor Mammedov   
> > > > > > 
> > > > > > Hmm.  So, I'm certainly good with making the matching more 
> > > > > > consistent,
> > > > > > and removing extra entries differing only in case.
> > > > > > 
> > > > > > However, I don't like capitalizing everything in the model table.  
> > > > > > The
> > > > > > "canonical" capitalization - as in how it appears in manuals and
> > > > > > marketing info - is quite frequently mixed case.  So I think making
> > > > > > everything caps in the table will make it harder to read in the
> > > > > > context of looking at the corresponding documentation.
> > > > > only cpu models => typenames are made caps, the rest
> > > > 
> > > > Yes, I know.  A number of the CPU model names themselves have mixed
> > > > case.  Really.
> > > >   
> > > > > incl. description is kept as is in mixed case.
> > > > > It looks like _desc is the thing one would use for
> > > > > 1:1 lookup in spec, while _name isn't direct match consistently
> > > > > (i.e. sometimes it skips spaces and sometimes spaces are replaced by 
> > > > > underscore).
> > > > > So keeping _name in mixed case unnecessarily complicates name->type 
> > > > > lookup.
> > > > > 
> > > > > These mixed case + case-insensitive is what made lookup
> > > > > code over-engineered and overly complex.
> > > > 
> > > > I'm fine with making all the lookups case insensitive.  I just don't
> > > > want to drop the case on the names in the actual code.  
> > > I've tried to find a way but considering that names are statically
> > > 'converted' into typenames and lack of ability to magically
> > > uppercase them with preprocessor, I've gave up on this approach.  
> > 
> > Ok.
> > 
> > > Providing that there is _desc with model name that should be
> > > greppable in spec and complexity of lookup code that mixed-cased
> > > typenames incur, I've opted in favor of simplifying lookup
> > > code at expense uniform typenames (which is inter QEMU thingy).
> > > It is easier to maintain and less chances to make mistake.  
> > 
> > Yeah, I guess.  It just looks weird to have non-standard capsing in
> > the table field.
> > 
> > Any chance we could standardize on lowercase rather than upper - for
> > some reason that would seem less odd to me (I guess because C-ish
> > names and identifiers are usually lowercased).
> The only reason I've picked up uppercase is that diffstat is smaller
> then with lowercase.
> So if you prefer lowercase, I'll switch case in v2.

Please do, thanks.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 09/16] crypto: Use qapi_enum_parse() in qcrypto_block_luks_name_lookup()

[Daniel P. Berrange]

On Thu, Aug 24, 2017 at 10:46:04AM +0200, Markus Armbruster wrote:
> Cc: "Daniel P. Berrange" 
> Signed-off-by: Markus Armbruster 
> ---
>  crypto/block-luks.c | 19 +++
>  1 file changed, 7 insertions(+), 12 deletions(-)

Acked-by: Daniel P. Berrange 

> 
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index afb8543..c3cacdb 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -20,6 +20,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
> +#include "qapi/util.h"
>  #include "qemu/bswap.h"
>  
>  #include "crypto/block-luks.h"
> @@ -265,39 +266,33 @@ 
> qcrypto_block_luks_cipher_alg_lookup(QCryptoCipherAlgorithm alg,
>   * make that function emit a more friendly error message */
>  static int qcrypto_block_luks_name_lookup(const char *name,
>const char *const *map,
> -  size_t maplen,
>const char *type,
>Error **errp)
>  {
> -size_t i;
> -for (i = 0; i < maplen; i++) {
> -if (g_str_equal(map[i], name)) {
> -return i;
> -}
> -}
> +int ret = qapi_enum_parse(map, name, -1, NULL);
>  
> -error_setg(errp, "%s %s not supported", type, name);
> -return 0;
> +if (ret < 0) {
> +error_setg(errp, "%s %s not supported", type, name);
> +return 0;
> +}
> +return ret;
>  }
>  
>  #define qcrypto_block_luks_cipher_mode_lookup(name, errp)   \
>  qcrypto_block_luks_name_lookup(name,\
> QCryptoCipherMode_lookup,\
> -   QCRYPTO_CIPHER_MODE__MAX,\
> "Cipher mode",   \
> errp)
>  
>  #define qcrypto_block_luks_hash_name_lookup(name, errp) \
>  qcrypto_block_luks_name_lookup(name,\
> QCryptoHashAlgorithm_lookup, \
> -   QCRYPTO_HASH_ALG__MAX,   \
> "Hash algorithm",\
> errp)
>  
>  #define qcrypto_block_luks_ivgen_name_lookup(name, errp)\
>  qcrypto_block_luks_name_lookup(name,\
> QCryptoIVGenAlgorithm_lookup,\
> -   QCRYPTO_IVGEN_ALG__MAX,  \
> "IV generator",  \
> errp)
>  
> -- 
> 2.7.5
> 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 3/3] hostmem-file: Add "discard-data" option

[Daniel P. Berrange]

On Thu, Aug 24, 2017 at 04:23:15PM -0300, Eduardo Habkost wrote:
> The new option can be used to indicate that the file contents can
> be destroyed and don't need to be flushed to disk when QEMU exits
> or when the memory backend object is removed.
> 
> Internally, it will trigger a madvise(MADV_REMOVE) call when the
> memory backend is removed.
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Replaced 'persistent=no' with 'discard-data=yes', to make it clear
>   that the flag will destroy data on the backing file.
> * Call madvise() directly from unparent() method instead of
>   relying on low-level memory backend code to call it.
>   v1 relied on getting the memory region reference count back to
>   0, which doesn't happen when QEMU is exiting because there's no
>   machine cleanup code to ensure that.
> ---
>  backends/hostmem-file.c | 29 +
>  qemu-options.hx |  5 -
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index fc4ef46..e44c319 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -32,6 +32,7 @@ struct HostMemoryBackendFile {
>  HostMemoryBackend parent_obj;
>  
>  bool share;
> +bool discard_data;
>  char *mem_path;
>  };
>  
> @@ -103,16 +104,44 @@ static void file_memory_backend_set_share(Object *o, 
> bool value, Error **errp)
>  fb->share = value;
>  }
>  
> +static bool file_memory_backend_get_discard_data(Object *o, Error **errp)
> +{
> +return MEMORY_BACKEND_FILE(o)->discard_data;
> +}
> +
> +static void file_memory_backend_set_discard_data(Object *o, bool value,
> +   Error **errp)
> +{
> +MEMORY_BACKEND_FILE(o)->discard_data = value;
> +}
> +
> +static void file_backend_unparent(Object *obj)
> +{
> +HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> +HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj);
> +
> +if (host_memory_backend_mr_inited(backend) && fb->discard_data) {
> +void *ptr = memory_region_get_ram_ptr(&backend->mr);
> +uint64_t sz = memory_region_size(&backend->mr);
> +
> +qemu_madvise(ptr, sz, QEMU_MADV_REMOVE);
> +}
> +}
> +
>  static void
>  file_backend_class_init(ObjectClass *oc, void *data)
>  {
>  HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
>  
>  bc->alloc = file_backend_memory_alloc;
> +oc->unparent = file_backend_unparent;
>  
>  object_class_property_add_bool(oc, "share",
>  file_memory_backend_get_share, file_memory_backend_set_share,
>  &error_abort);
> +object_class_property_add_bool(oc, "discard-data",
> +file_memory_backend_get_discard_data, 
> file_memory_backend_set_discard_data,
> +&error_abort);
>  object_class_property_add_str(oc, "mem-path",
>  get_mem_path, set_mem_path,
>  &error_abort);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9f6e2ad..ad985e4 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4160,7 +4160,7 @@ property must be set.  These objects are placed in the
>  
>  @table @option
>  
> -@item -object 
> memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off}
> +@item -object 
> memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off},discard-data=@var{on|off}
>  
>  Creates a memory file backend object, which can be used to back
>  the guest RAM with huge pages. The @option{id} parameter is a
> @@ -4172,6 +4172,9 @@ the path to either a shared memory or huge page 
> filesystem mount.
>  The @option{share} boolean option determines whether the memory
>  region is marked as private to QEMU, or shared. The latter allows
>  a co-operating external process to access the QEMU memory region.
> +Setting the @option{discard-data} boolean option to @var{on}
> +indicates that file contents can be destroyed when QEMU exits,
> +to avoid unnecessarily flushing data to the backing file.

We should note that this only works if QEMU shuts down normally. If QEMU
is aggressively killed (SIGKILL) or aborts for some reason, then we'll
never get a chance to invoke madvise(), so presumably the kernel will
still flush the data

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v11 2/6] ACPI: Add APEI GHES Table Generation support

[gengdongjiu]

Igor,
  Thank you very much for your review and comments, I will check your comments 
in detail and reply to you.


On 2017/8/29 18:20, Igor Mammedov wrote:
> On Fri, 18 Aug 2017 22:23:43 +0800
> Dongjiu Geng  wrote:
> 
>> This implements APEI GHES Table by passing the error CPER info
>> to the guest via a fw_cfg_blob. After a CPER info is recorded, an
>> SEA(Synchronous External Abort)/SEI(SError Interrupt) exception
>> will be injected into the guest OS.
> 
> it's a bit complex patch/functionality so I've just mosty skimmed and
> commented only on structure of the patch and changes I'd like to see
> so it would be more structured and review-able.
> 
> I'd suggest to add doc patch first which will describe how it's
> supposed to work between QEMU/firmware/guest OS with expected
> flows.
>  
>> Below is the table layout, the max number of error soure is 11,
>> which is classified by notification type.
>>
>>  etc/acpi/tables   etc/hardware_errors
>> 
>> ==
>> + +--++--+
>> | | HEST ||address   |  
>> +--+
>> | +--+|registers |  
>> | Error Status |
>> | | GHES0|| ++  
>> | Data Block 0 |
>> | +--+ +->| |status_address0 
>> |->| ++
>> | | .| |  | ++  
>> | |  CPER  |
>> | | error_status_address-+-+ +--->| |status_address1 |--+   
>> | |  CPER  |
>> | | .|   || ++  |   
>> | |    |
>> | | read_ack_register+-+ ||  .   |  |   
>> | |  CPER  |
>> | | read_ack_preserve| | |+--+  |   
>> | +-++
>> | | read_ack_write   | | | +->| |status_address10|+ |   
>> | Error Status |
>> + +--+ | | |  | ++| |   
>> | Data Block 1 |
>> | | GHES1| +-+-+->| | ack_value0 || 
>> +-->| ++
>> + +--+   | |  | ++| 
>> | |  CPER  |
>> | | .|   | | +--->| | ack_value1 || 
>> | |  CPER  |
>> | | error_status_address-+---+ | || ++| 
>> | |    |
>> | | .| | || |  . || 
>> | |  CPER  |
>> | | read_ack_register+-+-+| ++| 
>> +-++
>> | | read_ack_preserve| |   +->| | ack_value10|| 
>> | |..  |
>> | | read_ack_write   | |   |  | ++| 
>> | ++
>> + +--| |   |  | 
>> | Error Status |
>> | | ...  | |   |  | 
>> | Data Block 10|
>> + +--+ |   |  
>> +>| ++
>> | | GHES10   | |   |
>> | |  CPER  |
>> + +--+ |   |
>> | |  CPER  |
>> | | .| |   |
>> | |    |
>> | | error_status_address-+-+   |
>> | |  CPER  |
>> | | .| |
>> +-++
>> | | read_ack_register+-+
>> | | read_ack_preserve|
>> | | read_ack_write   |
>> + +--+
> these diagram shows relations between tables which not necessarily bad
> but as layout it's useless.
>  * Probably there is not much sense to have HEST table here, it's described
>well enough in spec. You might just put reference here.
>  * these diagrams should go into doc/spec patch
>  * when you describe layout you need to show what and at what offsets
>in which order in which blob/file is located. See ACPI spec for example
>and/or docs/specs/acpi_nvdimm.txt docs/specs/acpi_mem_hotplug.txt for 
> inspiration.
> 
>> For GHESv2 error source, the OSPM must acknowledges the error via Read Ack 
>> register.
>> so user space must check the ack value to avoid read-write race condition.
>>
>> Signed-off-by: Dongjiu Geng 
>> ---
>>  hw/acpi/aml-build.c |   2 +
>>  hw/acpi/hest_ghes.c | 345 
>> 
>>  hw/arm/virt-acpi-build.c|   6 +
>>  include/hw/acpi/aml-build.h |   1 +
>>  include/hw/acpi/hest_ghes.h |  47 ++

[Qemu-devel] [Bug 1713066] Re: Incorrect handling of aarch64 ldp in some cases

[Andrew]

Yes, D1.13.4 is what I want, I'm not completely familiar with this part
of the document.

Based on my reading of gen_load_exclusive I agree that it looks correct,
and loading to a float/vector won't affect the address generation.

I have worked around this in FreeBSD my switching the order of the
registers in the affected load & store, but still have an image I can
test a fix with.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1713066

Title:
  Incorrect handling of aarch64 ldp in some cases

Status in QEMU:
  New

Bug description:
  In some cases the ldp instruction (and presumably other multi-register
  loads and stores) can behave incorrectly.

  Given the following instruction:
  ldp x0, x1, [x0]

  This will load two 64 bit values from memory, however if each location
  to load is on a different page and the second page is unmapped this
  will raise an exception. When this happens x0 has already been updated
  so after the exception handler has run the operating system will try
  to rerun the instruction. QEMU will now try to perform an invalid load
  and raise a new exception.

  I believe this is incorrect as section D.1.14.5 of the ARMv8 reference
  manual B.a states that, on taking an exception, registers used in the
  generation of addresses are restored to their initial value, so x0
  shouldn't be changed, where x1 can be un an unknown state.

  I found the issue running FreeBSD with the cortex-strings
  implementation of memcpy. This uses a similar instruction when copying
  between 64 and 96 bytes.

  I've observed this on:
  QEMU emulator version 2.5.0 (Debian 1:2.5+dfsg-5ubuntu10.14), Copyright (c) 
2003-2008 Fabrice Bellard

  And checked I still get the same behaviour on:
  QEMU emulator version 2.9.94 (v2.10.0-rc4-dirty)
  Git revision: 248b23735645f7cbb503d9be6f5bf825f2a603ab

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1713066/+subscriptions

Re: [Qemu-devel] [PATCH 02/14] hvf: add code base from Google's QEMU repository

[Daniel P. Berrange]

On Sun, Aug 27, 2017 at 08:56:42PM -0500, Sergio Andres Gomez Del Real wrote:
> This file begins tracking the files that will be the code base for HVF
> support in QEMU.
> 
> Signed-off-by: Sergio Andres Gomez Del Real 
> ---

> diff --git a/target/i386/hvf-all.c b/target/i386/hvf-all.c
> new file mode 100644
> index 00..06cd8429eb
> --- /dev/null
> +++ b/target/i386/hvf-all.c
> @@ -0,0 +1,982 @@

There's no copyright header on this file.

A few others a missing copyright headers too, but they're all simple
.h files, so not critical. This, though, is a major .c file so I think
it really should have a copyright header present. IIUC, this is not
your code, but rather copied from the google repo, so ought to get the
original author to update this.

> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +
> +#include "sysemu/hvf.h"
> +#include "hvf-i386.h"
> +#include "hvf-utils/vmcs.h"
> +#include "hvf-utils/vmx.h"
> +#include "hvf-utils/x86.h"
> +#include "hvf-utils/x86_descr.h"
> +#include "hvf-utils/x86_mmu.h"
> +#include "hvf-utils/x86_decode.h"
> +#include "hvf-utils/x86_emu.h"
> +#include "hvf-utils/x86_cpuid.h"
> +#include "hvf-utils/x86hvf.h"
> +
> +#include 
> +#include 
> +
> +#include "exec/address-spaces.h"
> +#include "exec/exec-all.h"
> +#include "exec/ioport.h"
> +#include "hw/i386/apic_internal.h"
> +#include "hw/boards.h"
> +#include "qemu/main-loop.h"
> +#include "strings.h"
> +#include "trace.h"
> +#include "sysemu/accel.h"
> +#include "sysemu/sysemu.h"
> +#include "target/i386/cpu.h"
> +
> +pthread_rwlock_t mem_lock = PTHREAD_RWLOCK_INITIALIZER;
> +HVFState *hvf_state;
> +static int hvf_disabled = 1;
> +
> +static void assert_hvf_ok(hv_return_t ret)
> +{
> +if (ret == HV_SUCCESS)
> +return;
> +
> +switch (ret) {
> +case HV_ERROR:
> +fprintf(stderr, "Error: HV_ERROR\n");
> +break;
> +case HV_BUSY:
> +fprintf(stderr, "Error: HV_BUSY\n");
> +break;
> +case HV_BAD_ARGUMENT:
> +fprintf(stderr, "Error: HV_BAD_ARGUMENT\n");
> +break;
> +case HV_NO_RESOURCES:
> +fprintf(stderr, "Error: HV_NO_RESOURCES\n");
> +break;
> +case HV_NO_DEVICE:
> +fprintf(stderr, "Error: HV_NO_DEVICE\n");
> +break;
> +case HV_UNSUPPORTED:
> +fprintf(stderr, "Error: HV_UNSUPPORTED\n");
> +break;
> +default:
> +fprintf(stderr, "Unknown Error\n");
> +}
> +
> +abort();
> +}

There's loads of fprintfs() throughout this file - it really needs to
be fixed to use error_report() 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v3] console: add question-mark escape operator

[Alexander Graf]

Some termcaps (found using SLES11SP1) use [? sequences. According to man
console_codes (http://linux.die.net/man/4/console_codes) the question mark
is a nop and should simply be ignored.

This patch does exactly that, rendering screen output readable when
outputting guest serial consoles to the graphical console emulator.

Signed-off-by: Alexander Graf 

---

v1 -> v2:

  - rebase to 2.10

v2 -> v3:

  - fix coding style issues
---
 ui/console.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ui/console.c b/ui/console.c
index d2d3534c49..37fee92001 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -880,8 +880,9 @@ static void console_putchar(QemuConsole *s, int ch)
 } else {
 if (s->nb_esc_params < MAX_ESC_PARAMS)
 s->nb_esc_params++;
-if (ch == ';')
+if (ch == ';' || ch == '?') {
 break;
+}
 trace_console_putchar_csi(s->esc_params[0], s->esc_params[1],
   ch, s->nb_esc_params);
 s->state = TTY_STATE_NORM;
-- 
2.12.3

Re: [Qemu-devel] [RFC] Buffers/caches in VirtIO Balloon driver stats

[Wei Wang]

On 08/29/2017 05:57 PM, Stefan Hajnoczi wrote:

On Sun, Aug 27, 2017 at 11:30:33PM +0200, Tomáš Golembiovský wrote:

Hi,

I have CCed the relevant mailing lists and people most recently involved
in virtio-balloon discussions.  Hopefully this will help get the right
people to see your questions.


We'd like to include information about reclaimable memory into the
statistics in VirtiO Balloon driver. Namely, we'd like to include
counters for bufferes and caches of Linux kernel. The patch itself is
pretty trivial -- no problem there. But before we do that I'd like to
get some input from the QEMU community.

1) Is there any reason not to have the stats there?


Could you please share the usages of reclaimable memory via the stats?




2) Considering the balloon device is multiplatform (Linux, BSD,
Windows), is there a problem with including buffers/caches? These seem
to be specific to the Linux virtual memory subsystem. Of course, other
OSes could just report zeros. Are there some internal stats on those
OSes that could be filled in? I don't now if such or similar statistic
are available on BSD. On Windows only SystemCache stat looks like
something relevant. Anyone familiar with those OSes has any suggestions?




One of the solutions that I'm thinking about is to make virtio 
platform-ware.


That is, the device by default supports
VIRTIO_F_LINUX,
VIRTIO_F_WINDOWS,
VIRTIO_F_BSD.

For the Linux driver, only VIRTIO_F_LINUX is supported, then we can
have Linux specific driver implementations under that feature.


Best,
Wei

[Qemu-devel] [Bug 1713066] Re: Incorrect handling of aarch64 ldp in some cases

[Peter Maydell]

Richard Henderson has posted a patch which should fix this:
http://patchwork.ozlabs.org/patch/806051/

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1713066

Title:
  Incorrect handling of aarch64 ldp in some cases

Status in QEMU:
  New

Bug description:
  In some cases the ldp instruction (and presumably other multi-register
  loads and stores) can behave incorrectly.

  Given the following instruction:
  ldp x0, x1, [x0]

  This will load two 64 bit values from memory, however if each location
  to load is on a different page and the second page is unmapped this
  will raise an exception. When this happens x0 has already been updated
  so after the exception handler has run the operating system will try
  to rerun the instruction. QEMU will now try to perform an invalid load
  and raise a new exception.

  I believe this is incorrect as section D.1.14.5 of the ARMv8 reference
  manual B.a states that, on taking an exception, registers used in the
  generation of addresses are restored to their initial value, so x0
  shouldn't be changed, where x1 can be un an unknown state.

  I found the issue running FreeBSD with the cortex-strings
  implementation of memcpy. This uses a similar instruction when copying
  between 64 and 96 bytes.

  I've observed this on:
  QEMU emulator version 2.5.0 (Debian 1:2.5+dfsg-5ubuntu10.14), Copyright (c) 
2003-2008 Fabrice Bellard

  And checked I still get the same behaviour on:
  QEMU emulator version 2.9.94 (v2.10.0-rc4-dirty)
  Git revision: 248b23735645f7cbb503d9be6f5bf825f2a603ab

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1713066/+subscriptions

[Qemu-devel] [Bug 1713434] Re: prom-env-test test aborted and core dumped

[R.Nageswara Sastry]

Strange,

# gcc --version
gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16)
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

OS is RHEL based.
# uname -a
Linux host 4.11.0-26.el7a.ppc64le #1 SMP Wed Aug 23 17:27:28 EDT 2017 ppc64le 
ppc64le ppc64le GNU/Linux

TEST: tests/prom-env-test... (pid=6726)
  /ppc64/prom-env/mac99:
>> =
>> OpenBIOS 1.1 [Jul 13 2017 18:28]
>> Configuration device id QEMU version 1 machine id 3
>> CPUs: 1
>> Memory: 128M
>> UUID: ----
>> CPU type PowerPC,970FX
milliseconds isn't unique.
OK
  /ppc64/prom-env/g3beige:
>> =
>> OpenBIOS 1.1 [Jul 13 2017 18:28]
>> Configuration device id QEMU version 1 machine id 2
>> CPUs: 1
>> Memory: 128M
>> UUID: ----
>> CPU type PowerPC,750
milliseconds isn't unique.
OK
  /ppc64/prom-env/pseries:

SLOF **
QEMU Starting
 Build Date = Jul 24 2017 15:15:46
 FW Version = git-89f519f09bf85091
 Press "s" to enter Open Firmware.

**500
ERROR:tests/prom-env-test.c:42:check_guest_memory: assertion failed (signature 
== MAGIC): (0x7c7f1b78 == 0xcafec0de)
FAIL
GTester: last random seed: R02Sf6897626ac2ebaf5edd7aa24cc1999df
(pid=6951)
FAIL: tests/prom-env-test

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1713434

Title:
  prom-env-test test aborted and core dumped

Status in QEMU:
  In Progress

Bug description:
  On ppc64le architecture machine the following test case Aborted and
  Core dumped.

  # tests/prom-env-test --quiet --keep-going -m=quick --GTestLogFD=6
  **
  ERROR:tests/libqtest.c:628:qtest_get_arch: assertion failed: (qemu != NULL)
  Aborted (core dumped)

  Steps to re-produce:
  clone from the git
  configure & compile 
  run the unit tests by 'make check'

  (gdb) bt
  #0  0x3fff9d60eff0 in raise () from /lib64/libc.so.6
  #1  0x3fff9d61136c in abort () from /lib64/libc.so.6
  #2  0x3fff9de1aa04 in g_assertion_message () from /lib64/libglib-2.0.so.0
  #3  0x3fff9de1ab0c in g_assertion_message_expr () from 
/lib64/libglib-2.0.so.0
  #4  0x1000cc30 in qtest_get_arch () at tests/libqtest.c:628
  #5  0x100048f0 in main (argc=5, argv=0x32145538) at 
tests/prom-env-test.c:82
  (gdb) i r
  r0 0xfa   250
  r1 0x32144d30 70368510627120
  r2 0x3fff9d7b9900 7036709176
  r3 0x00
  r4 0x12a7 4775
  r5 0x66
  r6 0x88
  r7 0x11
  r8 0x00
  r9 0x00
  r100x00
  r110x00
  r120x00
  r130x3fff9dfa1950 70367099623760
  r140x00
  r150x00
  r160x00
  r170x00
  r180x00
  r190x00
  r200x00
  r210x00
  r220x00
  r230x00
  r240x00
  r250x00
  r260x00
  r270x100287f8 268601336
  r280x16841b40 377756480
  r290x4c   76
  r300x32144de8 70368510627304
  r310x66
  pc 0x3fff9d60eff0 0x3fff9d60eff0 
  msr0x9280f033 10376293541503627315
  cr 0x42000842 1107298370
  lr 0x3fff9d61136c 0x3fff9d61136c 
  ctr0x00
  xer0x00
  orig_r30x12a7 4775
  trap   0xc00  3072

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1713434/+subscriptions

Re: [Qemu-devel] [PATCH] s390-ccw: Fix alignment for CCW1

[Cornelia Huck]

On Mon, 28 Aug 2017 10:28:53 -0400
Farhan Ali  wrote:

> On 08/28/2017 10:19 AM, Halil Pasic wrote:
> >
> >
> > On 08/28/2017 04:15 PM, Farhan Ali wrote:  
> >>
> >>
> >> On 08/28/2017 10:05 AM, Cornelia Huck wrote:  
> >> It's the alignment of the CCW which causes the problem.
> >>
> >> The exact error message when starting the guest was:
> >>
> >> ! No virtio device found !
> >>
> >> Since it worked for SCSI and CDL, and failed for LDL disks on that 
> >> particular system, we are not really sure what caused the failure.
> >> Debugging it further showed the CCW for LDL disks were not aligned at 
> >> double word boundary.  
> >>> This is really, really odd, as the low-level ccw code is the same for
> >>> any disk type...
> >>>  
> >> Exactly!
> >>  
> >> Trying the test on a different system with LDL disks worked fine, with 
> >> the aligned(8) fix.  
> > Do you happen to have an old s390-ccw.img laying around in the test 
> > folder? QEMU might pick up
> > this one (e.g. when calling it without libvirt from the command line).
> >  
>  I explicitly mention the bios to use with '-bios' option and pick up the
>  latest bios. Without the aligned fix I see the error and with the fix it
>  works fine.  
> >>> Wait, so the fix fixes it? Or am I confused now?
> >>>  
> >>
> >> It fixes in my system and one other system we tried on. But fails on a 
> >> system where this issue was first noticed.  
> >
> > This is very confusing. So you have tried -bios on the system
> > where the issue was first noticed and the issue still persists
> > despite of the fixed bios is specified?
> >  
> Yes.
> 
> The system where the issue was first noticed, applying the fix for the 
> bios, fixes for:
> 
> 1) CDL disks
> 2) SCSI disks
> 
> But fails for LDL disk.
> 
> On my system and one other system, the fix works for all the disk types, 
> CDL, SCSI and LDL and fixes the issue.

Are you using different toolchains on the failing and the working
systems? Does it work when you copy the bios from a working system?

(Clutching at straws here...)

Re: [Qemu-devel] [PATCH v4 03/10] tests: Add vm test lib

[Philippe Mathieu-Daudé]

Hi Fam,

On 08/28/2017 02:47 PM, Fam Zheng wrote:

This is the common code to implement a "VM test" to

   1) Download and initialize a pre-defined VM that has necessary
   dependencies to build QEMU and SSH access.

   2) Archive $SRC_PATH to a .tar file.

   3) Boot the VM, and pass the source tar file to the guest.

   4) SSH into the VM, untar the source tarball, build from the source.

Signed-off-by: Fam Zheng 
---
  tests/vm/basevm.py | 287 +
  1 file changed, 287 insertions(+)
  create mode 100755 tests/vm/basevm.py

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
new file mode 100755
index 00..d0095c5332
--- /dev/null
+++ b/tests/vm/basevm.py
@@ -0,0 +1,287 @@
+#!/usr/bin/env python
+#
+# VM testing base class
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import logging
+import time
+import datetime
+sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
+from qemu import QEMUMachine
+import subprocess
+import hashlib
+import optparse
+import atexit
+import tempfile
+import shutil
+import multiprocessing
+import traceback
+
+SSH_KEY = """\
+-BEGIN RSA PRIVATE KEY-
+MIIEowIBAAKCAQEAopAuOlmLV6LVHdFBj8/eeOwI9CqguIJPp7eAQSZvOiB4Ag/R
+coEhl/RBbrV5Yc/SmSD4PTpJO/iM10RwliNjDb4a3I8q3sykRJu9c9PI/YsH8WN9
++NH2NjKPtJIcKTu287IM5JYxyB6nDoOzILbTyJ1TDR/xH6qYEfBAyiblggdjcvhA
+RTf93QIn39F/xLypXvT1K2O9BJEsnJ8lEUvB2UXhKo/JTfSeZF8wPBeowaP9EONk
+7b+nuJOWHGg68Ji6wVi62tjwl2Szch6lxIhZBpnV7QNRKMfYHP6eIyF4pusazzZq
+Telsq6xI2ghecWLzb/MF5A+rklsGx2FNuJSAJwIDAQABAoIBAHHi4o/8VZNivz0x
+cWXn8erzKV6tUoWQvW85Lj/2RiwJvSlsnYZDkx5af1CpEE2HA/pFT8PNRqsd+MWC
+7AEy710cVsM4BYerBFYQaYxwzblaoojo88LSjVPw3h5Z0iLM8+IMVd36nwuc9dpE
+R8TecMZ1+U4Tl6BgqkK+9xToZRdPKdjS8L5MoFhGN+xY0vRbbJbGaV9Q0IHxLBkB
+rEBV7T1mUynneCHRUQlJQEwJmKpT8MH3IjsUXlG5YvnuuvcQJSNTaW2iDLxuOKp8
+cxW8+qL88zpb1D5dppoIu6rlrugN0azSq70ruFJQPc/A8GQrDKoGgRQiagxNY3u+
+vHZzXlECgYEA0dKO3gfkSxsDBb94sQwskMScqLhcKhztEa8kPxTx6Yqh+x8/scx3
+XhJyOt669P8U1v8a/2Al+s81oZzzfQSzO1Q7gEwSrgBcRMSIoRBUw9uYcy02ngb/
+j/ng3DGivfJztjjiSJwb46FHkJ2JR8mF2UisC6UMXk3NgFY/3vWQx78CgYEAxlcG
+T3hfSWSmTgKRczMJuHQOX9ULfTBIqwP5VqkkkiavzigGRirzb5lgnmuTSPTpF0LB
+XVPjR2M4q+7gzP0Dca3pocrvLEoxjwIKnCbYKnyyvnUoE9qHv4Kr+vDbgWpa2LXG
+JbLmE7tgTCIp20jOPPT4xuDvlbzQZBJ5qCQSoZkCgYEAgrotSSihlCnAOFSTXbu4
+CHp3IKe8xIBBNENq0eK61kcJpOxTQvOha3sSsJsU4JAM6+cFaxb8kseHIqonCj1j
+bhOM/uJmwQJ4el/4wGDsbxriYOBKpyq1D38gGhDS1IW6kk3erl6VAb36WJ/OaGum
+eTpN9vNeQWM4Jj2WjdNx4QECgYAwTdd6mU1TmZCrJRL5ZG+0nYc2rbMrnQvFoqUi
+BvWiJovggHzur90zy73tNzPaq9Ls2FQxf5G1vCN8NCRJqEEjeYCR59OSDMu/EXc2
+CnvQ9SevHOdS1oEDEjcCWZCMFzPi3XpRih1gptzQDe31uuiHjf3cqcGPzTlPdfRt
+D8P92QKBgC4UaBvIRwREVJsdZzpIzm224Bpe8LOmA7DeTnjlT0b3lkGiBJ36/Q0p
+VhYh/6cjX4/iuIs7gJbGon7B+YPB8scmOi3fj0+nkJAONue1mMfBNkba6qQTc6Y2
+5mEKw2/O7/JpND7ucU3OK9plcw/qnrWDgHxl0Iz95+OzUIIagxne
+-END RSA PRIVATE KEY-
+"""
+SSH_PUB_KEY = """\
+ssh-rsa 
B3NzaC1yc2EDAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn
 qemu-vm-key
+"""
+
+class BaseVM(object):
+GUEST_USER = "qemu"
+GUEST_PASS = "qemupass"
+ROOT_PASS = "qemupass"
+
+# The script to run in the guest that builds QEMU
+BUILD_SCRIPT = ""
+# The guest name, to be overridden by subclasses
+name = "#base"
+def __init__(self, debug=False, vcpus=None):
+self._guest = None
+self._tmpdir = tempfile.mkdtemp(prefix="qemu-vm-")
+atexit.register(shutil.rmtree, self._tmpdir)
+
+self._ssh_key_file = os.path.join(self._tmpdir, "id_rsa")
+open(self._ssh_key_file, "w").write(SSH_KEY)
+subprocess.check_call(["chmod", "600", self._ssh_key_file])
+
+self._ssh_pub_key_file = os.path.join(self._tmpdir, "id_rsa.pub")
+open(self._ssh_pub_key_file, "w").write(SSH_PUB_KEY)
+
+self.debug = debug
+self._stderr = sys.stderr
+self._devnull = open("/dev/null", "w")
+if self.debug:
+self._stdout = sys.stdout
+else:
+self._stdout = self._devnull
+self._args = [ \
+"-nodefaults", "-m", "2G",
+"-cpu", "host",
+"-netdev", "user,id=vnet,hostfwd=:0.0.0.0:0-:22",
+"-device", "virtio-net-pci,netdev=vnet",
+"-vnc", ":0,to=20",
+"-serial", "file:%s" % os.path.join(self._tmpdir, "serial.out")]
+if vcpus:
+self._args += ["-smp", str(vcpus)]
+if os.access("/dev/kvm", os.R_OK | os.W_OK):
+self._args += ["-enable-kvm"]
+else:
+logging.i

[Qemu-devel] [PATCH] misc: Remove unused Error variables

[Alberto Garcia]

There's a few cases which we're passing an Error pointer to a function
only to discard it immediately afterwards without checking it. In
these cases we can simply remove the variable and pass NULL instead.

Signed-off-by: Alberto Garcia 
---
 block/qcow.c  | 12 +++-
 block/qcow2.c |  8 ++--
 dump.c|  4 +---
 3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index c08cdc4a7b..63904a26ee 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -454,13 +454,11 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
 start_sect = (offset & ~(s->cluster_size - 1)) >> 9;
 for(i = 0; i < s->cluster_sectors; i++) {
 if (i < n_start || i >= n_end) {
-Error *err = NULL;
 memset(s->cluster_data, 0x00, 512);
 if (qcrypto_block_encrypt(s->crypto, start_sect + 
i,
   s->cluster_data,
   BDRV_SECTOR_SIZE,
-  &err) < 0) {
-error_free(err);
+  NULL) < 0) {
 errno = EIO;
 return -1;
 }
@@ -572,7 +570,6 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, 
int64_t sector_num,
 QEMUIOVector hd_qiov;
 uint8_t *buf;
 void *orig_buf;
-Error *err = NULL;
 
 if (qiov->niov > 1) {
 buf = orig_buf = qemu_try_blockalign(bs, qiov->size);
@@ -637,7 +634,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, 
int64_t sector_num,
 if (bs->encrypted) {
 assert(s->crypto);
 if (qcrypto_block_decrypt(s->crypto, sector_num, buf,
-  n * BDRV_SECTOR_SIZE, &err) < 0) {
+  n * BDRV_SECTOR_SIZE, NULL) < 0) {
 goto fail;
 }
 }
@@ -660,7 +657,6 @@ done:
 return ret;
 
 fail:
-error_free(err);
 ret = -EIO;
 goto done;
 }
@@ -709,11 +705,9 @@ static coroutine_fn int qcow_co_writev(BlockDriverState 
*bs, int64_t sector_num,
 break;
 }
 if (bs->encrypted) {
-Error *err = NULL;
 assert(s->crypto);
 if (qcrypto_block_encrypt(s->crypto, sector_num, buf,
-  n * BDRV_SECTOR_SIZE, &err) < 0) {
-error_free(err);
+  n * BDRV_SECTOR_SIZE, NULL) < 0) {
 ret = -EIO;
 break;
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index 40ba26c111..fbfffadc76 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1820,15 +1820,13 @@ static coroutine_fn int 
qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
 assert(s->crypto);
 assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
 assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
-Error *err = NULL;
 if (qcrypto_block_decrypt(s->crypto,
   (s->crypt_physical_offset ?
cluster_offset + offset_in_cluster :
offset) >> BDRV_SECTOR_BITS,
   cluster_data,
   cur_bytes,
-  &err) < 0) {
-error_free(err);
+  NULL) < 0) {
 ret = -EIO;
 goto fail;
 }
@@ -1942,7 +1940,6 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState 
*bs, uint64_t offset,
 qemu_iovec_concat(&hd_qiov, qiov, bytes_done, cur_bytes);
 
 if (bs->encrypted) {
-Error *err = NULL;
 assert(s->crypto);
 if (!cluster_data) {
 cluster_data = qemu_try_blockalign(bs->file->bs,
@@ -1963,8 +1960,7 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState 
*bs, uint64_t offset,
cluster_offset + offset_in_cluster :
offset) >> BDRV_SECTOR_BITS,
   cluster_data,
-  cur_bytes, &err) < 0) {
-error_free(err);
+  cur_bytes, NULL) < 0) {
 ret = -EIO;
 goto fail;
 }
diff --git a/dump.c b/dump.c
index d9090a24cc..a79773d0f7 100644
--- a/dump.c
+++ b/dump.c
@@ -1695,10 +1695,8 @@ static void dump_process(DumpState *s, Error **errp)
 
 static void *dump_thread(void *data)
 {
-Error *err 

Re: [Qemu-devel] [PATCH v4 06/10] tests: Add NetBSD image

[Philippe Mathieu-Daudé]

On 08/28/2017 02:47 PM, Fam Zheng wrote:

The image is prepared following instructions as in:

https://wiki.qemu.org/Hosts/BSD

Signed-off-by: Fam Zheng 
Reviewed-by: Kamil Rytarowski 
---
  tests/vm/netbsd | 42 ++
  1 file changed, 42 insertions(+)
  create mode 100755 tests/vm/netbsd

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
new file mode 100755
index 00..559e89c8a6
--- /dev/null
+++ b/tests/vm/netbsd
@@ -0,0 +1,42 @@
+#!/usr/bin/env python
+#
+# NetBSD VM image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+
+class NetBSDVM(basevm.BaseVM):
+name = "netbsd"
+BUILD_SCRIPT = """
+set -e;
+cd $(mktemp -d /var/tmp/qemu-test.XX);
+tar -xf /dev/rld1a;
+./configure --python=python2.7 {configure_opts};
+gmake -j{jobs};
+gmake check;
+"""
+
+def build_image(self, img):
+cimg = 
self._download_with_cache("http://download.patchew.org/netbsd-7.1-amd64.img.xz";,
+ 
sha256sum='b633d565b0eac3d02015cd0c81440bd8a7a8df8512615ac1ee05d318be015732')
+img_tmp_xz = img + ".tmp.xz"
+img_tmp = img + ".tmp"
+subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
+subprocess.check_call(["xz", "-df", img_tmp_xz])
+if os.path.exists(img):
+os.remove(img)
+os.rename(img_tmp, img)
+
+if __name__ == "__main__":
+sys.exit(basevm.main(NetBSDVM))


This one is failing:

DEBUG:root:ssh_cmd: ssh -q -o StrictHostKeyChecking=no -o 
UserKnownHostsFile=/dev/null -o ConnectTimeout=1 -p 34091 -i 
/tmp/qemu-vm-59XYOj/id_rsa qemu@127.0.0.1

set -e;
cd $(mktemp -d /var/tmp/qemu-test.XX);
tar -xf /dev/rld1a;
./configure --python=python2.7 ;
gmake -j4;
gmake check;

...
  CC  bt-host.o
  CC  bt-vhci.o
  CC  dma-helpers.o
  CC  vl.o
  CC  tpm.o
In file included from vl.c:72:0:
/var/tmp/qemu-test.ht0XHU/include/hw/loader.h:4:29: fatal error: 
hw/nvram/fw_cfg.h: No such file or directory

 #include "hw/nvram/fw_cfg.h"
 ^
compilation terminated.
/var/tmp/qemu-test.ht0XHU/rules.mak:66: recipe for target 'vl.o' failed
gmake: *** [vl.o] Error 1
gmake: *** Waiting for unfinished jobs
tests/vm/Makefile.include:32: recipe for target 'vm-build-netbsd' failed
make: *** [vm-build-netbsd] Error 3

Re: [Qemu-devel] [PATCH v4 03/10] tests: Add vm test lib

[Daniel P. Berrange]

On Tue, Aug 29, 2017 at 09:06:48AM -0300, Philippe Mathieu-Daudé wrote:
> Hi Fam,
> 
> On 08/28/2017 02:47 PM, Fam Zheng wrote:
> > This is the common code to implement a "VM test" to
> > 
> >1) Download and initialize a pre-defined VM that has necessary
> >dependencies to build QEMU and SSH access.
> > 
> >2) Archive $SRC_PATH to a .tar file.
> > 
> >3) Boot the VM, and pass the source tar file to the guest.
> > 
> >4) SSH into the VM, untar the source tarball, build from the source.
> > 
> > Signed-off-by: Fam Zheng 
> > ---
> >   tests/vm/basevm.py | 287 
> > +
> >   1 file changed, 287 insertions(+)
> >   create mode 100755 tests/vm/basevm.py
> > 
> > diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> > new file mode 100755
> > index 00..d0095c5332
> > --- /dev/null
> > +++ b/tests/vm/basevm.py

> > +def add_source_dir(self, data_dir):
> > +name = "data-" + hashlib.sha1(data_dir).hexdigest()[:5]
> > +tarfile = os.path.join(self._tmpdir, name + ".tar")
> > +logging.debug("Creating archive %s for data dir: %s", tarfile, 
> > data_dir)
> > +if subprocess.call("type gtar", stdout=self._devnull,
> > +   stderr=self._devnull, shell=True) == 0:
> > +tar_cmd = "gtar"
> > +else:
> > +tar_cmd = "tar"
> > +subprocess.check_call([tar_cmd,
> > +   "--exclude-vcs",
> > +   "--exclude=tests/vm/*.img",
> > +   "--exclude=tests/vm/*.img.*",
> > +   "--exclude=*.d",
> > +   "--exclude=*.o",
> > +   "--exclude=docker-src.*",
> > +   "-cf", tarfile, '.'], cwd=data_dir,
> 
> I'm not happy with this command :/
> My distrib uses tmpfs for /tmp and suddently the whole X window became
> irresponsive until this script failing after filling 8G of /tmp and swap:
> 
> ...
> DEBUG:root:Creating archive /tmp/qemu-vm-F7CY9O/data-3a52c.tar for data dir:
> .
> tar: /tmp/qemu-vm-F7CY9O/data-3a52c.tar: Wrote only 4096 of 10240 bytes
> tar: Error is not recoverable: exiting now
> Failed to prepare guest environment
> 
> Then I figured out my workdir is full of testing stuff, debug images,
> firmwares, coredumps, etc.
> 
> I'll think of another way.

Yeah, /tmp should never be used for anything which has significant
size. Could go for /var/tmp instead, but IMHO just use the QEMU build
dir, as is done for (almost) all other build & test artifacts and
thus avoid any global dirs.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v4 03/10] tests: Add vm test lib

[Philippe Mathieu-Daudé]

On 08/28/2017 02:47 PM, Fam Zheng wrote:

This is the common code to implement a "VM test" to

   1) Download and initialize a pre-defined VM that has necessary
   dependencies to build QEMU and SSH access.

   2) Archive $SRC_PATH to a .tar file.

   3) Boot the VM, and pass the source tar file to the guest.

   4) SSH into the VM, untar the source tarball, build from the source.

Signed-off-by: Fam Zheng 
---
  tests/vm/basevm.py | 287 +
  1 file changed, 287 insertions(+)
  create mode 100755 tests/vm/basevm.py

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
new file mode 100755
index 00..d0095c5332
--- /dev/null
+++ b/tests/vm/basevm.py
@@ -0,0 +1,287 @@
+#!/usr/bin/env python
+#
+# VM testing base class
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import logging
+import time
+import datetime
+sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
+from qemu import QEMUMachine
+import subprocess
+import hashlib
+import optparse
+import atexit
+import tempfile
+import shutil
+import multiprocessing
+import traceback
+
+SSH_KEY = """\
+-BEGIN RSA PRIVATE KEY-
+MIIEowIBAAKCAQEAopAuOlmLV6LVHdFBj8/eeOwI9CqguIJPp7eAQSZvOiB4Ag/R
+coEhl/RBbrV5Yc/SmSD4PTpJO/iM10RwliNjDb4a3I8q3sykRJu9c9PI/YsH8WN9
++NH2NjKPtJIcKTu287IM5JYxyB6nDoOzILbTyJ1TDR/xH6qYEfBAyiblggdjcvhA
+RTf93QIn39F/xLypXvT1K2O9BJEsnJ8lEUvB2UXhKo/JTfSeZF8wPBeowaP9EONk
+7b+nuJOWHGg68Ji6wVi62tjwl2Szch6lxIhZBpnV7QNRKMfYHP6eIyF4pusazzZq
+Telsq6xI2ghecWLzb/MF5A+rklsGx2FNuJSAJwIDAQABAoIBAHHi4o/8VZNivz0x
+cWXn8erzKV6tUoWQvW85Lj/2RiwJvSlsnYZDkx5af1CpEE2HA/pFT8PNRqsd+MWC
+7AEy710cVsM4BYerBFYQaYxwzblaoojo88LSjVPw3h5Z0iLM8+IMVd36nwuc9dpE
+R8TecMZ1+U4Tl6BgqkK+9xToZRdPKdjS8L5MoFhGN+xY0vRbbJbGaV9Q0IHxLBkB
+rEBV7T1mUynneCHRUQlJQEwJmKpT8MH3IjsUXlG5YvnuuvcQJSNTaW2iDLxuOKp8
+cxW8+qL88zpb1D5dppoIu6rlrugN0azSq70ruFJQPc/A8GQrDKoGgRQiagxNY3u+
+vHZzXlECgYEA0dKO3gfkSxsDBb94sQwskMScqLhcKhztEa8kPxTx6Yqh+x8/scx3
+XhJyOt669P8U1v8a/2Al+s81oZzzfQSzO1Q7gEwSrgBcRMSIoRBUw9uYcy02ngb/
+j/ng3DGivfJztjjiSJwb46FHkJ2JR8mF2UisC6UMXk3NgFY/3vWQx78CgYEAxlcG
+T3hfSWSmTgKRczMJuHQOX9ULfTBIqwP5VqkkkiavzigGRirzb5lgnmuTSPTpF0LB
+XVPjR2M4q+7gzP0Dca3pocrvLEoxjwIKnCbYKnyyvnUoE9qHv4Kr+vDbgWpa2LXG
+JbLmE7tgTCIp20jOPPT4xuDvlbzQZBJ5qCQSoZkCgYEAgrotSSihlCnAOFSTXbu4
+CHp3IKe8xIBBNENq0eK61kcJpOxTQvOha3sSsJsU4JAM6+cFaxb8kseHIqonCj1j
+bhOM/uJmwQJ4el/4wGDsbxriYOBKpyq1D38gGhDS1IW6kk3erl6VAb36WJ/OaGum
+eTpN9vNeQWM4Jj2WjdNx4QECgYAwTdd6mU1TmZCrJRL5ZG+0nYc2rbMrnQvFoqUi
+BvWiJovggHzur90zy73tNzPaq9Ls2FQxf5G1vCN8NCRJqEEjeYCR59OSDMu/EXc2
+CnvQ9SevHOdS1oEDEjcCWZCMFzPi3XpRih1gptzQDe31uuiHjf3cqcGPzTlPdfRt
+D8P92QKBgC4UaBvIRwREVJsdZzpIzm224Bpe8LOmA7DeTnjlT0b3lkGiBJ36/Q0p
+VhYh/6cjX4/iuIs7gJbGon7B+YPB8scmOi3fj0+nkJAONue1mMfBNkba6qQTc6Y2
+5mEKw2/O7/JpND7ucU3OK9plcw/qnrWDgHxl0Iz95+OzUIIagxne
+-END RSA PRIVATE KEY-
+"""
+SSH_PUB_KEY = """\
+ssh-rsa 
B3NzaC1yc2EDAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn
 qemu-vm-key
+"""
+
+class BaseVM(object):
+GUEST_USER = "qemu"
+GUEST_PASS = "qemupass"
+ROOT_PASS = "qemupass"
+
+# The script to run in the guest that builds QEMU
+BUILD_SCRIPT = ""
+# The guest name, to be overridden by subclasses
+name = "#base"
+def __init__(self, debug=False, vcpus=None):
+self._guest = None
+self._tmpdir = tempfile.mkdtemp(prefix="qemu-vm-")
+atexit.register(shutil.rmtree, self._tmpdir)
+
+self._ssh_key_file = os.path.join(self._tmpdir, "id_rsa")
+open(self._ssh_key_file, "w").write(SSH_KEY)
+subprocess.check_call(["chmod", "600", self._ssh_key_file])
+
+self._ssh_pub_key_file = os.path.join(self._tmpdir, "id_rsa.pub")
+open(self._ssh_pub_key_file, "w").write(SSH_PUB_KEY)
+
+self.debug = debug
+self._stderr = sys.stderr
+self._devnull = open("/dev/null", "w")
+if self.debug:
+self._stdout = sys.stdout
+else:
+self._stdout = self._devnull
+self._args = [ \
+"-nodefaults", "-m", "2G",
+"-cpu", "host",
+"-netdev", "user,id=vnet,hostfwd=:0.0.0.0:0-:22",


Testing with debian/unstable:

$ make vm-build-netbsd V=1
./tests/vm/netbsd  --debug   --image "tests/vm/netbsd.img" --build-qemu .
DEBUG:root:Creating archive /tmp/qemu-vm-PxfXNv/data-3a52c.tar for data 
dir: .
DEBUG:root:QEMU args: -nodefaults -m 2G -cpu host -netdev 
user,id=vnet,hostfwd=:0.0.0.0:0-:22 -device virtio-net-pci,netdev=vnet 
-vnc :0,to=20 -serial file:/tm

Re: [Qemu-devel] [RFC for-2.10 1/3] pci/pcie: Make a consistent helper for switching PCI/PCIe "hybrid" devices

[David Gibson]

On Wed, Apr 26, 2017 at 06:23:58PM +0300, Michael S. Tsirkin wrote:
> On Tue, Mar 28, 2017 at 01:16:49PM +1100, David Gibson wrote:
> > virtio-pci and XHCI are "hybrid" devices in the sense that they can present
> > themselves as either PCIe or plain PCI devices depending on the machine
> > and bus they're connected to.
> > 
> > For virtio-pci to present as PCIe it requires that it's connected to a PCIe
> > bus and that it's not a root bus - this is to ensure that the device is
> > connected via a PCIe root port or downstream port rather than being a
> > integrated endpoint.  Some guests (Windows in particular AIUI) don't really
> > cope with PCIe integrated endpoints.
> > 
> > For XHCI it only checks that the bus is PCIe, but that probably means it
> > would cause problems if attached as an integrated devices directly to a
> > PCIe root bus.
> > 
> > This patch makes the test consistent between XHCI and virtio-pci, and
> > clarifies things by having them both use a new 'pci_allow_hybrid_pcie()'
> > helper which performs the same check as virtio-pci.
> > 
> > Signed-off-by: David Gibson 
> > ---
> >  hw/pci/pci.c   | 7 +++
> >  hw/usb/hcd-xhci.c  | 2 +-
> >  hw/virtio/virtio-pci.c | 3 +--
> >  include/hw/pci/pci.h   | 1 +
> >  4 files changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index bd8043c..779787b 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -390,6 +390,13 @@ bool pci_bus_is_root(PCIBus *bus)
> >  return PCI_BUS_GET_CLASS(bus)->is_root(bus);
> >  }
> >  
> > +bool pci_allow_hybrid_pcie(PCIDevice *pci_dev)
> > +{
> > +PCIBus *bus = pci_dev->bus;
> > +
> > +return pci_bus_is_express(bus) && !pci_bus_is_root(bus);
> > +}
> > +
> >  void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> >   const char *name,
> >   MemoryRegion *address_space_mem,
> 
> I'd prefer pci_allow_hybrid_pci_pcie.

Ok, I've made that change for the next spin (aimed at 2.11, obviously).

> 
> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > index f0af852..a7ff4fd 100644
> > --- a/hw/usb/hcd-xhci.c
> > +++ b/hw/usb/hcd-xhci.c
> > @@ -3619,7 +3619,7 @@ static void usb_xhci_realize(struct PCIDevice *dev, 
> > Error **errp)
> >   
> > PCI_BASE_ADDRESS_SPACE_MEMORY|PCI_BASE_ADDRESS_MEM_TYPE_64,
> >   &xhci->mem);
> >  
> > -if (pci_bus_is_express(dev->bus) ||
> > +if (pci_allow_hybrid_pcie(dev) ||
> >  xhci_get_flag(xhci, XHCI_FLAG_FORCE_PCIE_ENDCAP)) {
> >  ret = pcie_endpoint_cap_init(dev, 0xa0);
> >  assert(ret >= 0);
> 
> This seems to change the behaviour for xhci on a root bus - what
> am I missing?

Nothing.  I didn't consider the backwards compat implications; I'll
fix it for the next spin.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH v2 0/3] nbd-client: enter read_reply_co during init to avoid crash

[Stefan Hajnoczi]

v2:
 * Rewrote Patch 1 following Paolo's suggestion [Paolo]

See Patch 1 for the segfault fix.  Patches 2 & 3 add qemu-iotests coverage.

This is a rare crash that we'll probably only see in testing.  It only seems to
happen with UNIX domain sockets.

Stefan Hajnoczi (3):
  nbd-client: avoid read_reply_co entry if send failed
  qemu-iotests: improve nbd-fault-injector.py startup protocol
  qemu-iotests: test NBD over UNIX domain sockets in 083

 block/nbd-client.c   |  25 ++
 tests/qemu-iotests/083   | 138 ++---
 tests/qemu-iotests/083.out   | 145 +++
 tests/qemu-iotests/common.filter |   4 +-
 tests/qemu-iotests/nbd-fault-injector.py |   4 +
 5 files changed, 228 insertions(+), 88 deletions(-)

-- 
2.13.5

[Qemu-devel] [PATCH v2 2/3] qemu-iotests: improve nbd-fault-injector.py startup protocol

[Stefan Hajnoczi]

Currently 083 waits for the nbd-fault-injector.py server to start up by
looping until netstat shows the TCP listen socket.

The startup protocol can be simplified by passing a 0 port number to
nbd-fault-injector.py.  The kernel will allocate a port in bind(2) and
the final port number can be printed by nbd-fault-injector.py.

This should make it slightly nicer and less TCP-specific to wait for
server startup.  This patch changes nbd-fault-injector.py, the next one
will rewrite server startup in 083.

Reviewed-by: Eric Blake 
Signed-off-by: Stefan Hajnoczi 
---
 tests/qemu-iotests/nbd-fault-injector.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/nbd-fault-injector.py 
b/tests/qemu-iotests/nbd-fault-injector.py
index 6c07191a5a..1c10dcb51c 100755
--- a/tests/qemu-iotests/nbd-fault-injector.py
+++ b/tests/qemu-iotests/nbd-fault-injector.py
@@ -235,11 +235,15 @@ def open_socket(path):
 sock = socket.socket()
 sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
 sock.bind((host, int(port)))
+
+# If given port was 0 the final port number is now available
+path = '%s:%d' % sock.getsockname()
 else:
 sock = socket.socket(socket.AF_UNIX)
 sock.bind(path)
 sock.listen(0)
 print 'Listening on %s' % path
+sys.stdout.flush() # another process may be waiting, show message now
 return sock
 
 def usage(args):
-- 
2.13.5

[Qemu-devel] [PATCH v2 3/3] qemu-iotests: test NBD over UNIX domain sockets in 083

[Stefan Hajnoczi]

083 only tests TCP.  Some failures might be specific to UNIX domain
sockets.

A few adjustments are necessary:

1. Generating a port number and waiting for server startup is
   TCP-specific.  Use the new nbd-fault-injector.py startup protocol to
   fetch the address.  This is a little more elegant because we don't
   need netstat anymore.

2. The NBD filter does not work for the UNIX domain sockets URIs we
   generate and must be extended.

3. Run all tests twice: once for TCP and once for UNIX domain sockets.

Reviewed-by: Eric Blake 
Signed-off-by: Stefan Hajnoczi 
---
 tests/qemu-iotests/083   | 138 +++--
 tests/qemu-iotests/083.out   | 145 ++-
 tests/qemu-iotests/common.filter |   4 +-
 3 files changed, 215 insertions(+), 72 deletions(-)

diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083
index bff9360048..0306f112da 100755
--- a/tests/qemu-iotests/083
+++ b/tests/qemu-iotests/083
@@ -27,6 +27,14 @@ echo "QA output created by $seq"
 here=`pwd`
 status=1   # failure is the default!
 
+_cleanup()
+{
+   rm -f nbd.sock
+   rm -f nbd-fault-injector.out
+   rm -f nbd-fault-injector.conf
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
@@ -35,81 +43,105 @@ _supported_fmt generic
 _supported_proto nbd
 _supported_os Linux
 
-# Pick a TCP port based on our pid.  This way multiple instances of this test
-# can run in parallel without conflicting.
-choose_tcp_port() {
-   echo $((($$ % 31744) + 1024)) # 1024 <= port < 32768
-}
-
-wait_for_tcp_port() {
-   while ! (netstat --tcp --listening --numeric | \
-grep "$1.*0\\.0\\.0\\.0:\\*.*LISTEN") >/dev/null 2>&1; do
-   sleep 0.1
-   done
-}
-
 check_disconnect() {
+   local event export_name=foo extra_args nbd_addr nbd_url proto when
+
+   while true; do
+   case $1 in
+   --classic-negotiation)
+   shift
+   extra_args=--classic-negotiation
+   export_name=
+   ;;
+   --tcp)
+   shift
+   proto=tcp
+   ;;
+   --unix)
+   shift
+   proto=unix
+   ;;
+   *)
+   break
+   ;;
+   esac
+   done
+
event=$1
when=$2
-   negotiation=$3
echo "=== Check disconnect $when $event ==="
echo
 
-   port=$(choose_tcp_port)
-
cat > "$TEST_DIR/nbd-fault-injector.conf" <"$TEST_DIR/nbd-fault-injector.out" 2>&1 &
+
+   # Wait for server to be ready
+   while ! grep -q 'Listening on ' "$TEST_DIR/nbd-fault-injector.out"; do
+   sleep 0.1
+   done
+
+   # Extract the final address (port number has now been assigned in tcp 
case)
+   nbd_addr=$(sed 's/Listening on \(.*\)$/\1/' 
"$TEST_DIR/nbd-fault-injector.out")
+
+   if [ "$proto" = "tcp" ]; then
+   nbd_url="nbd+tcp://$nbd_addr/$export_name"
+   else
+   nbd_url="nbd+unix:///$export_name?socket=$nbd_addr"
fi
 
-   $PYTHON nbd-fault-injector.py $extra_args "127.0.0.1:$port" 
"$TEST_DIR/nbd-fault-injector.conf" >/dev/null 2>&1 &
-   wait_for_tcp_port "127\\.0\\.0\\.1:$port"
$QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | _filter_nbd
 
echo
 }
 
-for event in neg1 "export" neg2 request reply data; do
-   for when in before after; do
-   check_disconnect "$event" "$when"
-   done
-
-   # Also inject short replies from the NBD server
-   case "$event" in
-   neg1)
-   for when in 8 16; do
-   check_disconnect "$event" "$when"
-   done
-   ;;
-   "export")
-   for when in 4 12 16; do
-   check_disconnect "$event" "$when"
-   done
-   ;;
-   neg2)
-   for when in 8 10; do
-   check_disconnect "$event" "$when"
+for proto in tcp unix; do
+   for event in neg1 "export" neg2 request reply data; do
+   for when in before after; do
+   check_disconnect "--$proto" "$event" "$when"
done
-   ;;
-   reply)
-   for when in 4 8; do
-   check_disconnect "$event" "$when"
-   done
-   ;;
-   esac
-done
 
-# Also check classic negotiation without export information
-for when in before 8 16 24 28 after; do
-   check_disconnect "neg-classic" "$when" --classic-negotiation
+   # Also inject short replies from the NBD server
+   case "$event" in
+   neg1)
+   for when in 8 16; do
+  

[Qemu-devel] [PATCH v2 1/3] nbd-client: avoid read_reply_co entry if send failed

[Stefan Hajnoczi]

The following segfault is encountered if the NBD server closes the UNIX
domain socket immediately after negotiation:

  Program terminated with signal SIGSEGV, Segmentation fault.
  #0  aio_co_schedule (ctx=0x0, co=0xd3c0ff2ef0) at util/async.c:441
  441   QSLIST_INSERT_HEAD_ATOMIC(&ctx->scheduled_coroutines,
  (gdb) bt
  #0  0x00d3c01a50f8 in aio_co_schedule (ctx=0x0, co=0xd3c0ff2ef0) at 
util/async.c:441
  #1  0x00d3c012fa90 in nbd_coroutine_end (bs=bs@entry=0xd3c0fec650, 
request=) at block/nbd-client.c:207
  #2  0x00d3c012fb58 in nbd_client_co_preadv (bs=0xd3c0fec650, offset=0, 
bytes=, qiov=0x7ffc10a91b20, flags=0) at block/nbd-client.c:237
  #3  0x00d3c0128e63 in bdrv_driver_preadv (bs=bs@entry=0xd3c0fec650, 
offset=offset@entry=0, bytes=bytes@entry=512, qiov=qiov@entry=0x7ffc10a91b20, 
flags=0) at block/io.c:836
  #4  0x00d3c012c3e0 in bdrv_aligned_preadv 
(child=child@entry=0xd3c0ff51d0, req=req@entry=0x7f31885d6e90, 
offset=offset@entry=0, bytes=bytes@entry=512, align=align@entry=1, 
qiov=qiov@entry=0x7ffc10a91b20, f
+lags=0) at block/io.c:1086
  #5  0x00d3c012c6b8 in bdrv_co_preadv (child=0xd3c0ff51d0, 
offset=offset@entry=0, bytes=bytes@entry=512, qiov=qiov@entry=0x7ffc10a91b20, 
flags=flags@entry=0) at block/io.c:1182
  #6  0x00d3c011cc17 in blk_co_preadv (blk=0xd3c0ff4f80, offset=0, 
bytes=512, qiov=0x7ffc10a91b20, flags=0) at block/block-backend.c:1032
  #7  0x00d3c011ccec in blk_read_entry (opaque=0x7ffc10a91b40) at 
block/block-backend.c:1079
  #8  0x00d3c01bbb96 in coroutine_trampoline (i0=, 
i1=) at util/coroutine-ucontext.c:79
  #9  0x7f3196cb8600 in __start_context () at /lib64/libc.so.6

The problem is that nbd_client_init() uses
nbd_client_attach_aio_context() -> aio_co_schedule(new_context,
client->read_reply_co).  Execution of read_reply_co is deferred to a BH
which doesn't run until later.

In the mean time blk_co_preadv() can be called and nbd_coroutine_end()
calls aio_wake() on read_reply_co.  At this point in time
read_reply_co's ctx isn't set because it has never been entered yet.

This patch simplifies the nbd_co_send_request() ->
nbd_co_receive_reply() -> nbd_coroutine_end() lifecycle to just
nbd_co_send_request() -> nbd_co_receive_reply().  The request is "ended"
if an error occurs at any point.  Callers no longer have to invoke
nbd_coroutine_end().

This cleanup also eliminates the segfault because we don't call
aio_co_schedule() to wake up s->read_reply_co if sending the request
failed.  It is only necessary to wake up s->read_reply_co if a reply was
received.

Note this only happens with UNIX domain sockets on Linux.  It doesn't
seem possible to reproduce this with TCP sockets.

Suggested-by: Paolo Bonzini 
Signed-off-by: Stefan Hajnoczi 
---
 block/nbd-client.c | 25 +
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 25bcaa2346..ea728fffc8 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -144,12 +144,12 @@ static int nbd_co_send_request(BlockDriverState *bs,
 request->handle = INDEX_TO_HANDLE(s, i);
 
 if (s->quit) {
-qemu_co_mutex_unlock(&s->send_mutex);
-return -EIO;
+rc = -EIO;
+goto err;
 }
 if (!s->ioc) {
-qemu_co_mutex_unlock(&s->send_mutex);
-return -EPIPE;
+rc = -EPIPE;
+goto err;
 }
 
 if (qiov) {
@@ -166,8 +166,13 @@ static int nbd_co_send_request(BlockDriverState *bs,
 } else {
 rc = nbd_send_request(s->ioc, request);
 }
+
+err:
 if (rc < 0) {
 s->quit = true;
+s->requests[i].coroutine = NULL;
+s->in_flight--;
+qemu_co_queue_next(&s->free_sema);
 }
 qemu_co_mutex_unlock(&s->send_mutex);
 return rc;
@@ -201,13 +206,6 @@ static void nbd_co_receive_reply(NBDClientSession *s,
 /* Tell the read handler to read another header.  */
 s->reply.handle = 0;
 }
-}
-
-static void nbd_coroutine_end(BlockDriverState *bs,
-  NBDRequest *request)
-{
-NBDClientSession *s = nbd_get_client_session(bs);
-int i = HANDLE_TO_INDEX(s, request->handle);
 
 s->requests[i].coroutine = NULL;
 
@@ -243,7 +241,6 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t 
offset,
 } else {
 nbd_co_receive_reply(client, &request, &reply, qiov);
 }
-nbd_coroutine_end(bs, &request);
 return -reply.error;
 }
 
@@ -272,7 +269,6 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t 
offset,
 } else {
 nbd_co_receive_reply(client, &request, &reply, NULL);
 }
-nbd_coroutine_end(bs, &request);
 return -reply.error;
 }
 
@@ -306,7 +302,6 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, 
int64_t offset,
 } else {
 nbd_co_receive_reply(client, &request, &reply, NULL);
 }
-nbd_coroutine_end(bs, &request);
 return -reply.error;
 }
 
@@ -330,7 +325,6 @@ int nbd_client_co_flush(

Re: [Qemu-devel] [PATCH] misc: Remove unused Error variables

[Philippe Mathieu-Daudé]

Hi Alberto,

On 08/29/2017 09:08 AM, Alberto Garcia wrote:

There's a few cases which we're passing an Error pointer to a function
only to discard it immediately afterwards without checking it. In
these cases we can simply remove the variable and pass NULL instead.


How did you notice?



Signed-off-by: Alberto Garcia 


Reviewed-by: Philippe Mathieu-Daudé 


---
  block/qcow.c  | 12 +++-
  block/qcow2.c |  8 ++--
  dump.c|  4 +---
  3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index c08cdc4a7b..63904a26ee 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -454,13 +454,11 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
  start_sect = (offset & ~(s->cluster_size - 1)) >> 9;
  for(i = 0; i < s->cluster_sectors; i++) {
  if (i < n_start || i >= n_end) {
-Error *err = NULL;
  memset(s->cluster_data, 0x00, 512);
  if (qcrypto_block_encrypt(s->crypto, start_sect + 
i,
s->cluster_data,
BDRV_SECTOR_SIZE,
-  &err) < 0) {
-error_free(err);
+  NULL) < 0) {
  errno = EIO;
  return -1;
  }
@@ -572,7 +570,6 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, 
int64_t sector_num,
  QEMUIOVector hd_qiov;
  uint8_t *buf;
  void *orig_buf;
-Error *err = NULL;
  
  if (qiov->niov > 1) {

  buf = orig_buf = qemu_try_blockalign(bs, qiov->size);
@@ -637,7 +634,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, 
int64_t sector_num,
  if (bs->encrypted) {
  assert(s->crypto);
  if (qcrypto_block_decrypt(s->crypto, sector_num, buf,
-  n * BDRV_SECTOR_SIZE, &err) < 0) {
+  n * BDRV_SECTOR_SIZE, NULL) < 0) {
  goto fail;
  }
  }
@@ -660,7 +657,6 @@ done:
  return ret;
  
  fail:

-error_free(err);
  ret = -EIO;
  goto done;
  }
@@ -709,11 +705,9 @@ static coroutine_fn int qcow_co_writev(BlockDriverState 
*bs, int64_t sector_num,
  break;
  }
  if (bs->encrypted) {
-Error *err = NULL;
  assert(s->crypto);
  if (qcrypto_block_encrypt(s->crypto, sector_num, buf,
-  n * BDRV_SECTOR_SIZE, &err) < 0) {
-error_free(err);
+  n * BDRV_SECTOR_SIZE, NULL) < 0) {
  ret = -EIO;
  break;
  }
diff --git a/block/qcow2.c b/block/qcow2.c
index 40ba26c111..fbfffadc76 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1820,15 +1820,13 @@ static coroutine_fn int 
qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
  assert(s->crypto);
  assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
  assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
-Error *err = NULL;
  if (qcrypto_block_decrypt(s->crypto,
(s->crypt_physical_offset ?
 cluster_offset + offset_in_cluster 
:
 offset) >> BDRV_SECTOR_BITS,
cluster_data,
cur_bytes,
-  &err) < 0) {
-error_free(err);
+  NULL) < 0) {
  ret = -EIO;
  goto fail;
  }
@@ -1942,7 +1940,6 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState 
*bs, uint64_t offset,
  qemu_iovec_concat(&hd_qiov, qiov, bytes_done, cur_bytes);
  
  if (bs->encrypted) {

-Error *err = NULL;
  assert(s->crypto);
  if (!cluster_data) {
  cluster_data = qemu_try_blockalign(bs->file->bs,
@@ -1963,8 +1960,7 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState 
*bs, uint64_t offset,
 cluster_offset + offset_in_cluster :
 offset) >> BDRV_SECTOR_BITS,
cluster_data,
-  cur_bytes, &err) < 0) {
-error_free(err);
+  cur_bytes, NULL) < 0) {
  ret = -EIO;
  goto fail;
  }
diff --git a/dump.c b/dump.c
ind

[Qemu-devel] [Bug 1713434] Re: prom-env-test test aborted and core dumped

[Thomas Huth]

Very weird, looks like SLOF crashed at a very early stage here. I've got
no further clue how to debug this ... could you maybe try it on another
POWER9 host if possible? Or check whether slof.bin accidentially got
corrupted (md5sum pc-bios/slof.bin should give you
db83598b28052e9c12972d86c37b0c69)? Or maybe you could also ask Nikunj to
have a look at this?

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1713434

Title:
  prom-env-test test aborted and core dumped

Status in QEMU:
  In Progress

Bug description:
  On ppc64le architecture machine the following test case Aborted and
  Core dumped.

  # tests/prom-env-test --quiet --keep-going -m=quick --GTestLogFD=6
  **
  ERROR:tests/libqtest.c:628:qtest_get_arch: assertion failed: (qemu != NULL)
  Aborted (core dumped)

  Steps to re-produce:
  clone from the git
  configure & compile 
  run the unit tests by 'make check'

  (gdb) bt
  #0  0x3fff9d60eff0 in raise () from /lib64/libc.so.6
  #1  0x3fff9d61136c in abort () from /lib64/libc.so.6
  #2  0x3fff9de1aa04 in g_assertion_message () from /lib64/libglib-2.0.so.0
  #3  0x3fff9de1ab0c in g_assertion_message_expr () from 
/lib64/libglib-2.0.so.0
  #4  0x1000cc30 in qtest_get_arch () at tests/libqtest.c:628
  #5  0x100048f0 in main (argc=5, argv=0x32145538) at 
tests/prom-env-test.c:82
  (gdb) i r
  r0 0xfa   250
  r1 0x32144d30 70368510627120
  r2 0x3fff9d7b9900 7036709176
  r3 0x00
  r4 0x12a7 4775
  r5 0x66
  r6 0x88
  r7 0x11
  r8 0x00
  r9 0x00
  r100x00
  r110x00
  r120x00
  r130x3fff9dfa1950 70367099623760
  r140x00
  r150x00
  r160x00
  r170x00
  r180x00
  r190x00
  r200x00
  r210x00
  r220x00
  r230x00
  r240x00
  r250x00
  r260x00
  r270x100287f8 268601336
  r280x16841b40 377756480
  r290x4c   76
  r300x32144de8 70368510627304
  r310x66
  pc 0x3fff9d60eff0 0x3fff9d60eff0 
  msr0x9280f033 10376293541503627315
  cr 0x42000842 1107298370
  lr 0x3fff9d61136c 0x3fff9d61136c 
  ctr0x00
  xer0x00
  orig_r30x12a7 4775
  trap   0xc00  3072

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1713434/+subscriptions

Re: [Qemu-devel] [PATCH] s390-ccw: Fix alignment for CCW1

[Farhan Ali]

On 08/29/2017 08:04 AM, Cornelia Huck wrote:

On Mon, 28 Aug 2017 10:28:53 -0400
Farhan Ali  wrote:


On 08/28/2017 10:19 AM, Halil Pasic wrote:



On 08/28/2017 04:15 PM, Farhan Ali wrote:



On 08/28/2017 10:05 AM, Cornelia Huck wrote:

It's the alignment of the CCW which causes the problem.

The exact error message when starting the guest was:

! No virtio device found !

Since it worked for SCSI and CDL, and failed for LDL disks on that particular 
system, we are not really sure what caused the failure.
Debugging it further showed the CCW for LDL disks were not aligned at double 
word boundary.

This is really, really odd, as the low-level ccw code is the same for
any disk type...


Exactly!


Trying the test on a different system with LDL disks worked fine, with the 
aligned(8) fix.

Do you happen to have an old s390-ccw.img laying around in the test folder? 
QEMU might pick up
this one (e.g. when calling it without libvirt from the command line).


I explicitly mention the bios to use with '-bios' option and pick up the
latest bios. Without the aligned fix I see the error and with the fix it
works fine.

Wait, so the fix fixes it? Or am I confused now?



It fixes in my system and one other system we tried on. But fails on a system 
where this issue was first noticed.


This is very confusing. So you have tried -bios on the system
where the issue was first noticed and the issue still persists
despite of the fixed bios is specified?


Yes.

The system where the issue was first noticed, applying the fix for the
bios, fixes for:

1) CDL disks
2) SCSI disks

But fails for LDL disk.

On my system and one other system, the fix works for all the disk types,
CDL, SCSI and LDL and fixes the issue.


Are you using different toolchains on the failing and the working
systems? Does it work when you copy the bios from a working system?

(Clutching at straws here...)



So yesterday we realized for the failing system, the bios wasn't being 
built on that system rather it was being built on a different system and 
being copied over to the failing system. :/


Building the bios on the failing system with the fix, resolves the issue 
and we did not see anymore failures.

So I think I can safely say this patch fixes the alignment problem.

Re: [Qemu-devel] [PATCH] s390-ccw: Fix alignment for CCW1

[Cornelia Huck]

On Tue, 29 Aug 2017 08:39:27 -0400
Farhan Ali  wrote:

> On 08/29/2017 08:04 AM, Cornelia Huck wrote:
> > On Mon, 28 Aug 2017 10:28:53 -0400
> > Farhan Ali  wrote:
> >  
> >> On 08/28/2017 10:19 AM, Halil Pasic wrote:  
> >>>
> >>>
> >>> On 08/28/2017 04:15 PM, Farhan Ali wrote:  
> 
> 
>  On 08/28/2017 10:05 AM, Cornelia Huck wrote:  
>  It's the alignment of the CCW which causes the problem.
> 
>  The exact error message when starting the guest was:
> 
>  ! No virtio device found !
> 
>  Since it worked for SCSI and CDL, and failed for LDL disks on that 
>  particular system, we are not really sure what caused the failure.
>  Debugging it further showed the CCW for LDL disks were not aligned 
>  at double word boundary.  
> > This is really, really odd, as the low-level ccw code is the same for
> > any disk type...
> >  
>  Exactly!
>   
>  Trying the test on a different system with LDL disks worked fine, 
>  with the aligned(8) fix.  
> >>> Do you happen to have an old s390-ccw.img laying around in the test 
> >>> folder? QEMU might pick up
> >>> this one (e.g. when calling it without libvirt from the command line).
> >>>  
> >> I explicitly mention the bios to use with '-bios' option and pick up 
> >> the
> >> latest bios. Without the aligned fix I see the error and with the fix 
> >> it
> >> works fine.  
> > Wait, so the fix fixes it? Or am I confused now?
> >  
> 
>  It fixes in my system and one other system we tried on. But fails on a 
>  system where this issue was first noticed.  
> >>>
> >>> This is very confusing. So you have tried -bios on the system
> >>> where the issue was first noticed and the issue still persists
> >>> despite of the fixed bios is specified?
> >>>  
> >> Yes.
> >>
> >> The system where the issue was first noticed, applying the fix for the
> >> bios, fixes for:
> >>
> >> 1) CDL disks
> >> 2) SCSI disks
> >>
> >> But fails for LDL disk.
> >>
> >> On my system and one other system, the fix works for all the disk types,
> >> CDL, SCSI and LDL and fixes the issue.  
> >
> > Are you using different toolchains on the failing and the working
> > systems? Does it work when you copy the bios from a working system?
> >
> > (Clutching at straws here...)
> >  
> 
> So yesterday we realized for the failing system, the bios wasn't being 
> built on that system rather it was being built on a different system and 
> being copied over to the failing system. :/

Oh dear... the system it was built on hopefully was missing the fix,
right? (I'm getting a bit paranoid here.)

> 
> Building the bios on the failing system with the fix, resolves the issue 
> and we did not see anymore failures.
> So I think I can safely say this patch fixes the alignment problem.

Out of interest, which toolchain are you using? My rebuild is on F26.

Re: [Qemu-devel] [Qemu-block] [PATCH v4 03/15] block: introduce BDRV_REQ_ALLOCATE flag

[Alberto Garcia]

On Tue 01 Aug 2017 04:19:00 PM CEST, Anton Nefedov wrote:
> The flag is supposed to indicate that the region of the disk image has
> to be sufficiently allocated so it reads as zeroes. The call with the flag
> set has to return -ENOTSUP if allocation cannot be done efficiently
> (i.e. without falling back to writing actual buffers)
>
> Signed-off-by: Anton Nefedov 

Reviewed-by: Alberto Garcia 

> +/* allocation request with qiov provided doesn't make much sense */
> +assert(!(qiov && flags & BDRV_REQ_ALLOCATE));

> +assert(!(flags & BDRV_REQ_MAY_UNMAP && flags & BDRV_REQ_ALLOCATE));
> +
> +if (flags & BDRV_REQ_ALLOCATE &&
> +!(child->bs->supported_zero_flags & BDRV_REQ_ALLOCATE))

I find it more readable with parentheses like this:

  assert(!(qiov && (flags & BDRV_REQ_ALLOCATE)));
  assert(!((flags & BDRV_REQ_MAY_UNMAP) && (flags & BDRV_REQ_ALLOCATE)));
  if ((flags & BDRV_REQ_ALLOCATE) &&
  !(child->bs->supported_zero_flags & BDRV_REQ_ALLOCATE))

but your code is correct as it is.

Berto

Re: [Qemu-devel] [Qemu-block] [PATCH v4 05/15] file-posix: support BDRV_REQ_ALLOCATE

[Alberto Garcia]

On Tue 01 Aug 2017 04:19:02 PM CEST, Anton Nefedov wrote:
> Current write_zeroes implementation is good enough to satisfy this flag too
>
> Signed-off-by: Anton Nefedov 

Reviewed-by: Alberto Garcia 

Berto

Re: [Qemu-devel] [RFC for-2.10 0/3] Rework handling of PCI/PCIe "hybrid" devices

[Eduardo Habkost]

On Tue, Mar 28, 2017 at 01:16:48PM +1100, David Gibson wrote:
> A couple of devices - virtio-pci and XHCI - can present themselves to
> the guest as either PCI or PCIe devices depending on how they're
> attached.  However, the logic is a little different between the two
> devices.  In addition the logic in virtio makes it difficult to put a
> PCIe virtio device into a "pseries" guest because of the unusual way
> the paravirtualized PCI bus works there.

virtio-pci and xhci are not the only hybrid devices.  What about
vmxnet3, pvscsi, and vfio-pci?

> 
> This series makes the logic more consistent, and allows per-machine
> overrides to address that.
> 
> Currently patch 3/3 shows a non-obvious side effect of this change.  A
> PCIe virtio device is, by default, modern mode only, but the qtest
> logic doesn't handle modern-only virtio devices correctly.  We work
> around this by explicitly adding disable-legacy=off to the testcases.
> It would probably be better to update libqos so that it can handle
> modern virtio devices.
> 
> David Gibson (3):
>   pci/pcie: Make a consistent helper for switching PCI/PCIe "hybrid"
> devices
>   pci: Allow host bridges to override PCI/PCIe hybrid device behaviour
>   pseries: Allow PCIe virtio and XHCI on pseries machine type
> 
>  hw/pci/pci.c  | 14 ++
>  hw/ppc/spapr_pci.c|  9 +
>  hw/usb/hcd-xhci.c |  2 +-
>  hw/virtio/virtio-pci.c|  3 +--
>  include/hw/pci/pci.h  |  1 +
>  include/hw/pci/pci_host.h |  1 +
>  tests/virtio-9p-test.c|  2 +-
>  tests/virtio-blk-test.c   |  4 ++--
>  tests/virtio-net-test.c   |  2 +-
>  tests/virtio-scsi-test.c  |  2 +-
>  10 files changed, 32 insertions(+), 8 deletions(-)
> 
> -- 
> 2.9.3
> 
> 

-- 
Eduardo

[Qemu-devel] [Bug 1713434] Re: prom-env-test test aborted and core dumped

[R.Nageswara Sastry]

# md5sum  ./pc-bios/slof.bin
db83598b28052e9c12972d86c37b0c69  ./pc-bios/slof.bin

Same as what you mentioned.

Will try to get a different machine and try. If the problem still
persists, I will check with Nikunj.

Thanks a lot for your time. I have learned many things while interacting
with you.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1713434

Title:
  prom-env-test test aborted and core dumped

Status in QEMU:
  In Progress

Bug description:
  On ppc64le architecture machine the following test case Aborted and
  Core dumped.

  # tests/prom-env-test --quiet --keep-going -m=quick --GTestLogFD=6
  **
  ERROR:tests/libqtest.c:628:qtest_get_arch: assertion failed: (qemu != NULL)
  Aborted (core dumped)

  Steps to re-produce:
  clone from the git
  configure & compile 
  run the unit tests by 'make check'

  (gdb) bt
  #0  0x3fff9d60eff0 in raise () from /lib64/libc.so.6
  #1  0x3fff9d61136c in abort () from /lib64/libc.so.6
  #2  0x3fff9de1aa04 in g_assertion_message () from /lib64/libglib-2.0.so.0
  #3  0x3fff9de1ab0c in g_assertion_message_expr () from 
/lib64/libglib-2.0.so.0
  #4  0x1000cc30 in qtest_get_arch () at tests/libqtest.c:628
  #5  0x100048f0 in main (argc=5, argv=0x32145538) at 
tests/prom-env-test.c:82
  (gdb) i r
  r0 0xfa   250
  r1 0x32144d30 70368510627120
  r2 0x3fff9d7b9900 7036709176
  r3 0x00
  r4 0x12a7 4775
  r5 0x66
  r6 0x88
  r7 0x11
  r8 0x00
  r9 0x00
  r100x00
  r110x00
  r120x00
  r130x3fff9dfa1950 70367099623760
  r140x00
  r150x00
  r160x00
  r170x00
  r180x00
  r190x00
  r200x00
  r210x00
  r220x00
  r230x00
  r240x00
  r250x00
  r260x00
  r270x100287f8 268601336
  r280x16841b40 377756480
  r290x4c   76
  r300x32144de8 70368510627304
  r310x66
  pc 0x3fff9d60eff0 0x3fff9d60eff0 
  msr0x9280f033 10376293541503627315
  cr 0x42000842 1107298370
  lr 0x3fff9d61136c 0x3fff9d61136c 
  ctr0x00
  xer0x00
  orig_r30x12a7 4775
  trap   0xc00  3072

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1713434/+subscriptions

Re: [Qemu-devel] [Qemu-block] [PATCH v4 06/15] block: support BDRV_REQ_ALLOCATE in passthrough drivers

[Alberto Garcia]

On Tue 01 Aug 2017 04:19:03 PM CEST, Anton Nefedov wrote:
> Support the flag if the underlying BDS supports it
>
> Signed-off-by: Anton Nefedov 

Reviewed-by: Alberto Garcia 

Berto

Re: [Qemu-devel] [PATCH v4 03/10] tests: Add vm test lib

[Philippe Mathieu-Daudé]

Hi Fam, Kamil,


On 08/28/2017 02:47 PM, Fam Zheng wrote:

[...]

+    subprocess.check_call([tar_cmd,
+   "--exclude-vcs",
+   "--exclude=tests/vm/*.img",
+   "--exclude=tests/vm/*.img.*",
+   "--exclude=*.d",
+   "--exclude=*.o",
+   "--exclude=docker-src.*",
+   "-cf", tarfile, '.'], cwd=data_dir,


I'm not happy with this command :/
My distrib uses tmpfs for /tmp and suddently the whole X window became 
irresponsive until this script failing after filling 8G of /tmp and swap:


...
DEBUG:root:Creating archive /tmp/qemu-vm-F7CY9O/data-3a52c.tar for data 
dir: .

tar: /tmp/qemu-vm-F7CY9O/data-3a52c.tar: Wrote only 4096 of 10240 bytes
tar: Error is not recoverable: exiting now
Failed to prepare guest environment

Then I figured out my workdir is full of testing stuff, debug images, 
firmwares, coredumps, etc.


I'll think of another way.


Using:

(git ls-files; git submodule foreach --recursive "git ls-files | sed 
s_^_\$sm_path/_" | sed /^Entering/d) > files.txt


and:

subprocess.check_call([tar_cmd,
   "-cf", tarfile,
   "-v" if self.debug else "",
   "-T", "files.txt"], cwd=data_dir,
  stdin=self._devnull, stdout=self._stdout)

Current /master and submodules generated tarball is ~305MB uncompressed.

Kamil do you mind testing this command on NetBSD?

(obviously 'files.txt' goes in tmpdir).

Regards,

Phil.

Re: [Qemu-devel] Persistent bitmaps for non-qcow2 formats

[Yaniv Lavi (Dary)]

YANIV LAVI (YANIV DARY)

SENIOR TECHNICAL PRODUCT MANAGER

Red Hat Israel Ltd. 

34 Jerusalem Road, Building A, 1st floor

Ra'anana, Israel 4350109

yl...@redhat.comT: +972-9-7692306/8272306 F: +972-9-7692223IM: ylavi
  TRIED. TESTED. TRUSTED. 
@redhatnews    Red Hat
   Red Hat



On Mon, Aug 28, 2017 at 9:11 PM, John Snow  wrote:

>
>
> On 08/27/2017 10:57 PM, Fam Zheng wrote:
> > On Fri, 08/25 15:44, Max Reitz wrote:
> >> Well, OK.  The main argument against supporting anything but qcow2 is
> >> "if you want features, use qcow2; and we are working on making qcow2 as
> >> fast as possible."  I think that's a very good argument still.  At some
> >> point I (and probably others, too) had the idea of making qcow2 files in
> >> raw layout:
> >
> >
> > Yes! I think this idea makes a whole lot of sense, too. Metadata tables
> can be
> > generated so old implementation can still use it.
> >
> > Fam
> >
> >> Have the data as a blob, just like a raw file, padded by
> >> metadata around it.  An autoclear flag would specify that the qcow2 file
> >> is in this format, and if so, you could simply access it like a raw file
> >> and should have exactly the same speed as a raw file.  Maybe that would
> >> solve this whole issue, too?
>
> I wonder if this would be sufficient to alleviate the desire to use raw
> files...
>
> (Eh, well, realistically, someone's still always going to ask if they
> can use various features with non-qcow2 files...)
>
> Nir, Yaniv; any input?
>

We are using raw format for performance reasons.
As we have many customers that currently use this format, not support it
would be a blocker the use of the feature.
At a minimum we would require ability to convert raw to qcow2 raw-layout.

Please also consider that we are planning to go on the OSP route of LUN per
disk and would still want the tracking to work.
I makes sense that for that and raw format you will be able to save the
mapping to another file other than a qcow.


>
> (Context: We're debating how to add persistent bitmaps to raw files as I
> was informed that RHV was 'asking about it.' Max is reminding me there
> is a proposal for a style of QCOW2 that uses a raw layout for data,
> mitigating or eliminating any performance hits related to the L2 cache.
> What I am not aware of is why RHV would use raw files for any purpose.
> Is it performance? Simplicity? Could RHV use a raw-layout qcow2?)
>
> --js
>

Re: [Qemu-devel] [PATCH v2 3/3] hostmem-file: Add "discard-data" option

[Eduardo Habkost]

On Tue, Aug 29, 2017 at 12:13:45PM +0100, Daniel P. Berrange wrote:
> On Thu, Aug 24, 2017 at 04:23:15PM -0300, Eduardo Habkost wrote:
> > The new option can be used to indicate that the file contents can
> > be destroyed and don't need to be flushed to disk when QEMU exits
> > or when the memory backend object is removed.
> > 
> > Internally, it will trigger a madvise(MADV_REMOVE) call when the
> > memory backend is removed.
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> > Changes v1 -> v2:
> > * Replaced 'persistent=no' with 'discard-data=yes', to make it clear
> >   that the flag will destroy data on the backing file.
> > * Call madvise() directly from unparent() method instead of
> >   relying on low-level memory backend code to call it.
> >   v1 relied on getting the memory region reference count back to
> >   0, which doesn't happen when QEMU is exiting because there's no
> >   machine cleanup code to ensure that.
> > ---
> >  backends/hostmem-file.c | 29 +
> >  qemu-options.hx |  5 -
> >  2 files changed, 33 insertions(+), 1 deletion(-)
> > 
> > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> > index fc4ef46..e44c319 100644
> > --- a/backends/hostmem-file.c
> > +++ b/backends/hostmem-file.c
> > @@ -32,6 +32,7 @@ struct HostMemoryBackendFile {
> >  HostMemoryBackend parent_obj;
> >  
> >  bool share;
> > +bool discard_data;
> >  char *mem_path;
> >  };
> >  
> > @@ -103,16 +104,44 @@ static void file_memory_backend_set_share(Object *o, 
> > bool value, Error **errp)
> >  fb->share = value;
> >  }
> >  
> > +static bool file_memory_backend_get_discard_data(Object *o, Error **errp)
> > +{
> > +return MEMORY_BACKEND_FILE(o)->discard_data;
> > +}
> > +
> > +static void file_memory_backend_set_discard_data(Object *o, bool value,
> > +   Error **errp)
> > +{
> > +MEMORY_BACKEND_FILE(o)->discard_data = value;
> > +}
> > +
> > +static void file_backend_unparent(Object *obj)
> > +{
> > +HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> > +HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj);
> > +
> > +if (host_memory_backend_mr_inited(backend) && fb->discard_data) {
> > +void *ptr = memory_region_get_ram_ptr(&backend->mr);
> > +uint64_t sz = memory_region_size(&backend->mr);
> > +
> > +qemu_madvise(ptr, sz, QEMU_MADV_REMOVE);
> > +}
> > +}
> > +
> >  static void
> >  file_backend_class_init(ObjectClass *oc, void *data)
> >  {
> >  HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
> >  
> >  bc->alloc = file_backend_memory_alloc;
> > +oc->unparent = file_backend_unparent;
> >  
> >  object_class_property_add_bool(oc, "share",
> >  file_memory_backend_get_share, file_memory_backend_set_share,
> >  &error_abort);
> > +object_class_property_add_bool(oc, "discard-data",
> > +file_memory_backend_get_discard_data, 
> > file_memory_backend_set_discard_data,
> > +&error_abort);
> >  object_class_property_add_str(oc, "mem-path",
> >  get_mem_path, set_mem_path,
> >  &error_abort);
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 9f6e2ad..ad985e4 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -4160,7 +4160,7 @@ property must be set.  These objects are placed in the
> >  
> >  @table @option
> >  
> > -@item -object 
> > memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off}
> > +@item -object 
> > memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off},discard-data=@var{on|off}
> >  
> >  Creates a memory file backend object, which can be used to back
> >  the guest RAM with huge pages. The @option{id} parameter is a
> > @@ -4172,6 +4172,9 @@ the path to either a shared memory or huge page 
> > filesystem mount.
> >  The @option{share} boolean option determines whether the memory
> >  region is marked as private to QEMU, or shared. The latter allows
> >  a co-operating external process to access the QEMU memory region.
> > +Setting the @option{discard-data} boolean option to @var{on}
> > +indicates that file contents can be destroyed when QEMU exits,
> > +to avoid unnecessarily flushing data to the backing file.
> 
> We should note that this only works if QEMU shuts down normally. If QEMU
> is aggressively killed (SIGKILL) or aborts for some reason, then we'll
> never get a chance to invoke madvise(), so presumably the kernel will
> still flush the data

Good point.  I tried to not give any guarantees by saying
"contents _can_ be destroyed", but users may still have different
expectations.

I will change it to:

  Setting the @option{discard-data} boolean option to @var{on}
  indicates that file contents can be destroyed when QEMU exits,
  to avoid unnecessarily flushing data to the backing file.  Note
  that @option{discard-data} is only an optimization, and QEMU
  might not disca

Re: [Qemu-devel] [PATCH] misc: Remove unused Error variables

[Alberto Garcia]

On Tue 29 Aug 2017 02:31:35 PM CEST, Philippe Mathieu-Daudé wrote:

>> There's a few cases which we're passing an Error pointer to a function
>> only to discard it immediately afterwards without checking it. In
>> these cases we can simply remove the variable and pass NULL instead.
>
> How did you notice?

I noticed it while I was working on something else. It's also not the
first time I see something like this (see e.g. 026ac1586bdbd184e240).

Berto

Re: [Qemu-devel] [PATCH v4 03/10] tests: Add vm test lib

[Kamil Rytarowski]

On 29.08.2017 15:10, Philippe Mathieu-Daudé wrote:
> Hi Fam, Kamil,
> 
>> On 08/28/2017 02:47 PM, Fam Zheng wrote:
> [...]
>>> +subprocess.check_call([tar_cmd,
>>> +   "--exclude-vcs",
>>> +   "--exclude=tests/vm/*.img",
>>> +   "--exclude=tests/vm/*.img.*",
>>> +   "--exclude=*.d",
>>> +   "--exclude=*.o",
>>> +   "--exclude=docker-src.*",
>>> +   "-cf", tarfile, '.'], cwd=data_dir,
>>
>> I'm not happy with this command :/
>> My distrib uses tmpfs for /tmp and suddently the whole X window became
>> irresponsive until this script failing after filling 8G of /tmp and swap:
>>
>> ...
>> DEBUG:root:Creating archive /tmp/qemu-vm-F7CY9O/data-3a52c.tar for
>> data dir: .
>> tar: /tmp/qemu-vm-F7CY9O/data-3a52c.tar: Wrote only 4096 of 10240 bytes
>> tar: Error is not recoverable: exiting now
>> Failed to prepare guest environment
>>
>> Then I figured out my workdir is full of testing stuff, debug images,
>> firmwares, coredumps, etc.
>>
>> I'll think of another way.
> 
> Using:
> 
> (git ls-files; git submodule foreach --recursive "git ls-files | sed
> s_^_\$sm_path/_" | sed /^Entering/d) > files.txt
> 
> and:
> 
> subprocess.check_call([tar_cmd,
>"-cf", tarfile,
>"-v" if self.debug else "",
>"-T", "files.txt"], cwd=data_dir,
>   stdin=self._devnull, stdout=self._stdout)
> 
> Current /master and submodules generated tarball is ~305MB uncompressed.
> 
> Kamil do you mind testing this command on NetBSD?
> 
> (obviously 'files.txt' goes in tmpdir).
> 

Result:

http://www.netbsd.org/~kamil/qemu/files.txt

I've tested this command line on a checkout from Jul 17th rev. 77031ee1ce4c

> Regards,
> 
> Phil.
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 03/10] tests: Add vm test lib

[Philippe Mathieu-Daudé]

On 08/29/2017 10:22 AM, Kamil Rytarowski wrote:

On 29.08.2017 15:10, Philippe Mathieu-Daudé wrote:

Hi Fam, Kamil,


On 08/28/2017 02:47 PM, Fam Zheng wrote:

[...]

+subprocess.check_call([tar_cmd,
+   "--exclude-vcs",
+   "--exclude=tests/vm/*.img",
+   "--exclude=tests/vm/*.img.*",
+   "--exclude=*.d",
+   "--exclude=*.o",
+   "--exclude=docker-src.*",
+   "-cf", tarfile, '.'], cwd=data_dir,


I'm not happy with this command :/
My distrib uses tmpfs for /tmp and suddently the whole X window became
irresponsive until this script failing after filling 8G of /tmp and swap:

...
DEBUG:root:Creating archive /tmp/qemu-vm-F7CY9O/data-3a52c.tar for
data dir: .
tar: /tmp/qemu-vm-F7CY9O/data-3a52c.tar: Wrote only 4096 of 10240 bytes
tar: Error is not recoverable: exiting now
Failed to prepare guest environment

Then I figured out my workdir is full of testing stuff, debug images,
firmwares, coredumps, etc.

I'll think of another way.


Using:

(git ls-files; git submodule foreach --recursive "git ls-files | sed
s_^_\$sm_path/_" | sed /^Entering/d) > files.txt

and:

 subprocess.check_call([tar_cmd,
"-cf", tarfile,
"-v" if self.debug else "",
"-T", "files.txt"], cwd=data_dir,
   stdin=self._devnull, stdout=self._stdout)

Current /master and submodules generated tarball is ~305MB uncompressed.

Kamil do you mind testing this command on NetBSD?

(obviously 'files.txt' goes in tmpdir).



Result:

http://www.netbsd.org/~kamil/qemu/files.txt


\o/


I've tested this command line on a checkout from Jul 17th rev. 77031ee1ce4c


Thanks Kamil :)

Fam are you OK using this command instead?

Regards,

Phil.

[Qemu-devel] [Bug 1713434] Re: prom-env-test test aborted and core dumped

[R.Nageswara Sastry]

On other machine with same OS and gcc level, it's working fine. Not getting 
what went wrong in the machine where I can re-produce this issue.
I guess this bug can be closed. Thank you.

  /ppc64/prom-env/pseries:

SLOF **
QEMU Starting
 Build Date = Jul 24 2017 15:15:46
 FW Version = git-89f519f09bf85091
 Press "s" to enter Open Firmware.

Populating /vdevice methods
Populating /vdevice/vty@7100
Populating /vdevice/nvram@7101
Populating /pci@8002000
(!) Executing code specified in nvramrc
 SLOF Setup = OK
PASS: tests/prom-env-test

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1713434

Title:
  prom-env-test test aborted and core dumped

Status in QEMU:
  In Progress

Bug description:
  On ppc64le architecture machine the following test case Aborted and
  Core dumped.

  # tests/prom-env-test --quiet --keep-going -m=quick --GTestLogFD=6
  **
  ERROR:tests/libqtest.c:628:qtest_get_arch: assertion failed: (qemu != NULL)
  Aborted (core dumped)

  Steps to re-produce:
  clone from the git
  configure & compile 
  run the unit tests by 'make check'

  (gdb) bt
  #0  0x3fff9d60eff0 in raise () from /lib64/libc.so.6
  #1  0x3fff9d61136c in abort () from /lib64/libc.so.6
  #2  0x3fff9de1aa04 in g_assertion_message () from /lib64/libglib-2.0.so.0
  #3  0x3fff9de1ab0c in g_assertion_message_expr () from 
/lib64/libglib-2.0.so.0
  #4  0x1000cc30 in qtest_get_arch () at tests/libqtest.c:628
  #5  0x100048f0 in main (argc=5, argv=0x32145538) at 
tests/prom-env-test.c:82
  (gdb) i r
  r0 0xfa   250
  r1 0x32144d30 70368510627120
  r2 0x3fff9d7b9900 7036709176
  r3 0x00
  r4 0x12a7 4775
  r5 0x66
  r6 0x88
  r7 0x11
  r8 0x00
  r9 0x00
  r100x00
  r110x00
  r120x00
  r130x3fff9dfa1950 70367099623760
  r140x00
  r150x00
  r160x00
  r170x00
  r180x00
  r190x00
  r200x00
  r210x00
  r220x00
  r230x00
  r240x00
  r250x00
  r260x00
  r270x100287f8 268601336
  r280x16841b40 377756480
  r290x4c   76
  r300x32144de8 70368510627304
  r310x66
  pc 0x3fff9d60eff0 0x3fff9d60eff0 
  msr0x9280f033 10376293541503627315
  cr 0x42000842 1107298370
  lr 0x3fff9d61136c 0x3fff9d61136c 
  ctr0x00
  xer0x00
  orig_r30x12a7 4775
  trap   0xc00  3072

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1713434/+subscriptions