[Qemu-devel] [PATCH v2 0/2] Delay initialization of memory backends

[Qemu-devel] [PATCH v2 0/2] Delay initialization of memory backends

[Eduardo Habkost]

While trying to fix the original bug in v1, another bug was fixed
by accident: TCG initialization of dirty_log_mask was broken when
using memory backends.

The fix, on the other hand, broke vhost-user-test because it
relied on TCG, even though TCG is incompatible with vhost.

This new version changes vhost-user-test to not rely on TCG, so
we can finally fix the initialization ordering of memory
backends.

Eduardo Habkost (2):
  vhost-user-test: Use libqos instead of pxe-virtio.rom
  vl: Delay initialization of memory backends

 tests/Makefile.include  |  2 +-
 tests/vhost-user-test.c | 37 ++++++++++++++++++++++++++++++++++---
 vl.c                    | 13 +++++++++++++
 3 files changed, 48 insertions(+), 4 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v2 1/2] vhost-user-test: Use libqos instead of pxe-virtio.rom

[Eduardo Habkost]

vhost-user-test relies on iPXE just to initialize the virtio-net
device, and doesn't do any actual packet tx/rx testing.

In addition to that, the test relies on TCG, which is
imcompatible with vhost. The test only worked by accident: a bug
the memory backend initialization made memory regions not have
the DIRTY_MEMORY_CODE bit set in dirty_log_mask.

This changes vhost-user-test to initialize the virtio-net device
using libqos, and not use TCG nor pxe-virtio.rom.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes series v1 -> series v2:
* (new patch)
---
 tests/Makefile.include  |  2 +-
 tests/vhost-user-test.c | 37 ++++++++++++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 14be491..03382b5 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -622,7 +622,7 @@ tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-usb-obj-y)
 tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o $(libqos-usb-obj-y)
 tests/pc-cpu-test$(EXESUF): tests/pc-cpu-test.o
 tests/postcopy-test$(EXESUF): tests/postcopy-test.o
-tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o $(qtest-obj-y) $(test-io-obj-y)
+tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o $(qtest-obj-y) $(test-io-obj-y) $(libqos-virtio-obj-y)
 tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o $(test-util-obj-y)
 tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(test-block-obj-y)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 27b10c1..b89a551 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -16,8 +16,13 @@
 #include "qemu/sockets.h"
 #include "sysemu/char.h"
 #include "sysemu/sysemu.h"
+#include "libqos/libqos.h"
+#include "libqos/pci-pc.h"
+#include "libqos/virtio-pci.h"
 
 #include <linux/vhost.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_net.h>
 #include <sys/vfs.h>
 
 /* GLIB version compatibility flags */
@@ -29,14 +34,13 @@
 #define HAVE_MONOTONIC_TIME
 #endif
 
-#define QEMU_CMD_ACCEL  " -machine accel=tcg"
 #define QEMU_CMD_MEM    " -m %d -object memory-backend-file,id=mem,size=%dM,"\
                         "mem-path=%s,share=on -numa node,memdev=mem"
 #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
 #define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=%s,vhostforce"
-#define QEMU_CMD_NET    " -device virtio-net-pci,netdev=net0,romfile=./pc-bios/pxe-virtio.rom"
+#define QEMU_CMD_NET    " -device virtio-net-pci,netdev=net0"
 
-#define QEMU_CMD        QEMU_CMD_ACCEL QEMU_CMD_MEM QEMU_CMD_CHR \
+#define QEMU_CMD        QEMU_CMD_MEM QEMU_CMD_CHR \
                         QEMU_CMD_NETDEV QEMU_CMD_NET
 
 #define HUGETLBFS_MAGIC       0x958458f6
@@ -136,6 +140,30 @@ typedef struct TestServer {
 static const char *tmpfs;
 static const char *root;
 
+static void init_virtio_dev(TestServer *s)
+{
+    QPCIBus *bus;
+    QVirtioPCIDevice *dev;
+    uint32_t features;
+
+    bus = qpci_init_pc();
+    g_assert_nonnull(bus);
+
+    dev = qvirtio_pci_device_find(bus, VIRTIO_ID_NET);
+    g_assert_nonnull(dev);
+
+    qvirtio_pci_device_enable(dev);
+    qvirtio_reset(&qvirtio_pci, &dev->vdev);
+    qvirtio_set_acknowledge(&qvirtio_pci, &dev->vdev);
+    qvirtio_set_driver(&qvirtio_pci, &dev->vdev);
+
+    features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
+    features = features & VIRTIO_NET_F_MAC;
+    qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
+
+    qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
+}
+
 static void wait_for_fds(TestServer *s)
 {
     gint64 end_time;
@@ -548,6 +576,7 @@ static void test_migrate(void)
     from = qtest_start(cmd);
     g_free(cmd);
 
+    init_virtio_dev(s);
     wait_for_fds(s);
     size = get_log_size(s);
     g_assert_cmpint(size, ==, (2 * 1024 * 1024) / (VHOST_LOG_PAGE * 8));
@@ -662,6 +691,7 @@ static void test_reconnect_subprocess(void)
     qtest_start(cmd);
     g_free(cmd);
 
+    init_virtio_dev(s);
     wait_for_fds(s);
     wait_for_rings_started(s, 2);
 
@@ -728,6 +758,7 @@ int main(int argc, char **argv)
 
     s = qtest_start(qemu_cmd);
     g_free(qemu_cmd);
+    init_virtio_dev(server);
 
     qtest_add_data_func("/vhost-user/read-guest-mem", server, read_guest_mem);
     qtest_add_func("/vhost-user/migrate", test_migrate);
-- 
2.7.4

[Qemu-devel] [PATCH v2 2/2] vl: Delay initialization of memory backends

[Eduardo Habkost]

Initialization of memory backends may take a while when
prealloc=yes is used, depending on their size. Initializing
memory backends before chardevs may delay the creation of monitor
sockets, and trigger timeouts on management software that waits
until the monitor socket is created by QEMU. See, for example,
the bug report at:
https://bugzilla.redhat.com/show_bug.cgi?id=1371211

In addition to that, allocating memory before calling
configure_accelerator() breaks the tcg_enabled() checks at
memory_region_init_*().

This patch fixes those problems by adding "memory-backend-*"
classes to the delayed-initialization list.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes series v1 -> series v2:
* Changed comment and commit message to mention the
  dirty_log_mask initialization bug
---
 vl.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/vl.c b/vl.c
index b3c80d5..ee557a1 100644
--- a/vl.c
+++ b/vl.c
@@ -2810,6 +2810,19 @@ static bool object_create_initial(const char *type)
         return false;
     }
 
+    /* Memory allocation by backends needs to be done
+     * after configure_accelerator() (due to the tcg_enabled()
+     * checks at memory_region_init_*()).
+     *
+     * Also, allocation of large amounts of memory may delay
+     * chardev initialization for too long, and trigger timeouts
+     * on software that waits for a monitor socket to be created
+     * (e.g. libvirt).
+     */
+    if (g_str_has_prefix(type, "memory-backend-")) {
+        return false;
+    }
+
     return true;
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 0/2] Delay initialization of memory backends

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> While trying to fix the original bug in v1, another bug was fixed
> by accident: TCG initialization of dirty_log_mask was broken when
> using memory backends.
>
> The fix, on the other hand, broke vhost-user-test because it
> relied on TCG, even though TCG is incompatible with vhost.
>
> This new version changes vhost-user-test to not rely on TCG, so
> we can finally fix the initialization ordering of memory
> backends.

Series
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v2 0/2] Delay initialization of memory backends

[Paolo Bonzini]

On 02/09/2016 20:59, Eduardo Habkost wrote:
> While trying to fix the original bug in v1, another bug was fixed
> by accident: TCG initialization of dirty_log_mask was broken when
> using memory backends.
> 
> The fix, on the other hand, broke vhost-user-test because it
> relied on TCG, even though TCG is incompatible with vhost.
> 
> This new version changes vhost-user-test to not rely on TCG, so
> we can finally fix the initialization ordering of memory
> backends.
> 
> Eduardo Habkost (2):
>   vhost-user-test: Use libqos instead of pxe-virtio.rom
>   vl: Delay initialization of memory backends
> 
>  tests/Makefile.include  |  2 +-
>  tests/vhost-user-test.c | 37 ++++++++++++++++++++++++++++++++++---
>  vl.c                    | 13 +++++++++++++
>  3 files changed, 48 insertions(+), 4 deletions(-)
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

for whoever wants to pick this up...

Paolo

Re: [Qemu-devel] [PATCH v2 0/2] Delay initialization of memory backends

[Eduardo Habkost]

On Mon, Sep 05, 2016 at 02:26:30PM +0200, Paolo Bonzini wrote:
> On 02/09/2016 20:59, Eduardo Habkost wrote:
> > While trying to fix the original bug in v1, another bug was fixed
> > by accident: TCG initialization of dirty_log_mask was broken when
> > using memory backends.
> > 
> > The fix, on the other hand, broke vhost-user-test because it
> > relied on TCG, even though TCG is incompatible with vhost.
> > 
> > This new version changes vhost-user-test to not rely on TCG, so
> > we can finally fix the initialization ordering of memory
> > backends.
> > 
> > Eduardo Habkost (2):
> >   vhost-user-test: Use libqos instead of pxe-virtio.rom
> >   vl: Delay initialization of memory backends
> > 
> >  tests/Makefile.include  |  2 +-
> >  tests/vhost-user-test.c | 37 ++++++++++++++++++++++++++++++++++---
> >  vl.c                    | 13 +++++++++++++
> >  3 files changed, 48 insertions(+), 4 deletions(-)
> > 
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> for whoever wants to pick this up...

I will commit it. Thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 0/2] Delay initialization of memory backends

[Michael S. Tsirkin]

On Mon, Sep 05, 2016 at 04:01:30PM -0300, Eduardo Habkost wrote:
> On Mon, Sep 05, 2016 at 02:26:30PM +0200, Paolo Bonzini wrote:
> > On 02/09/2016 20:59, Eduardo Habkost wrote:
> > > While trying to fix the original bug in v1, another bug was fixed
> > > by accident: TCG initialization of dirty_log_mask was broken when
> > > using memory backends.
> > > 
> > > The fix, on the other hand, broke vhost-user-test because it
> > > relied on TCG, even though TCG is incompatible with vhost.
> > > 
> > > This new version changes vhost-user-test to not rely on TCG, so
> > > we can finally fix the initialization ordering of memory
> > > backends.
> > > 
> > > Eduardo Habkost (2):
> > >   vhost-user-test: Use libqos instead of pxe-virtio.rom
> > >   vl: Delay initialization of memory backends
> > > 
> > >  tests/Makefile.include  |  2 +-
> > >  tests/vhost-user-test.c | 37 ++++++++++++++++++++++++++++++++++---
> > >  vl.c                    | 13 +++++++++++++
> > >  3 files changed, 48 insertions(+), 4 deletions(-)
> > > 
> > 
> > Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> > 
> > for whoever wants to pick this up...
> 
> I will commit it. Thanks!

Feel free to commit if you didn't already.

Belatedly:

Acked-by: Michael S. Tsirkin <mst@redhat.com>


> -- 
> Eduardo

Re: [Qemu-devel] [PATCH v2 0/2] Delay initialization of memory backends

[Maxime Coquelin]

On 09/06/2016 04:24 AM, Michael S. Tsirkin wrote:
> On Mon, Sep 05, 2016 at 04:01:30PM -0300, Eduardo Habkost wrote:
>> On Mon, Sep 05, 2016 at 02:26:30PM +0200, Paolo Bonzini wrote:
>>> On 02/09/2016 20:59, Eduardo Habkost wrote:
>>>> While trying to fix the original bug in v1, another bug was fixed
>>>> by accident: TCG initialization of dirty_log_mask was broken when
>>>> using memory backends.
>>>>
>>>> The fix, on the other hand, broke vhost-user-test because it
>>>> relied on TCG, even though TCG is incompatible with vhost.
>>>>
>>>> This new version changes vhost-user-test to not rely on TCG, so
>>>> we can finally fix the initialization ordering of memory
>>>> backends.
>>>>
>>>> Eduardo Habkost (2):
>>>>   vhost-user-test: Use libqos instead of pxe-virtio.rom
>>>>   vl: Delay initialization of memory backends
>>>>
>>>>  tests/Makefile.include  |  2 +-
>>>>  tests/vhost-user-test.c | 37 ++++++++++++++++++++++++++++++++++---
>>>>  vl.c                    | 13 +++++++++++++
>>>>  3 files changed, 48 insertions(+), 4 deletions(-)
>>>>
>>>
>>> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>>>
>>> for whoever wants to pick this up...
>>
>> I will commit it. Thanks!
>
> Feel free to commit if you didn't already.
>
> Belatedly:
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

Michael, please note that not using TCG also fixes the hang observed
with patch:
28ed5ef ("vhost-user: Attempt to fix a race with set_mem_table.")
It confirms my analysis that it would only happen with relying on TCG.
Should we revert your revert once this series applied to mainline?

So, FWIW now that the patch may be already committed:
Tested-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime