[Qemu-devel] [PULL 00/12] pci, pc, virtio, misc bugfixes

[Qemu-devel] [PULL 00/12] pci, pc, virtio, misc bugfixes

[Michael S. Tsirkin]

The following changes since commit 4c24f4004089a308c5de8ed720cf6bd1746aedd8:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140912' into staging (2014-09-12 15:12:26 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to 36a315ef05921aef7a386ec08d866fbe3f626e08:

  vhost-user: fix VIRTIO_NET_F_MRG_RXBUF negotiation (2014-09-14 21:33:01 +0300)

----------------------------------------------------------------
pci, pc, virtio, misc bugfixes

A bunch of bugfixes - some of these will make sense for 2.1.2
Cc: qemu-stable included where appropriate.

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

----------------------------------------------------------------
Damjan Marion (1):
      vhost-user: fix VIRTIO_NET_F_MRG_RXBUF negotiation

Eduardo Habkost (8):
      hw/machine: Free old values of string properties
      test-qdev-global-props: Trivial comment fix
      test-qdev-global-props: Run tests on subprocess
      test-qdev-global-props: Initialize not_used=true for all props
      test-qdev-global-props: Test handling of hotpluggable and non-device types
      qdev: Rename qdev_prop_check_global() to qdev_prop_check_globals()
      qdev: Move global validation to a single function
      Revert "rng-egd: remove redundant free"

Michael S. Tsirkin (3):
      virtio-net: drop assert on vm stop
      Revert "virtio: don't call device on !vm_running"
      virtio-pci: enable bus master for old guests

 include/hw/qdev-core.h           |  10 +--
 include/hw/qdev-properties.h     |   2 +-
 backends/rng-egd.c               |   1 +
 hw/core/machine.c                |   8 ++
 hw/core/qdev-properties-system.c |  18 +----
 hw/core/qdev-properties.c        |  30 ++++++--
 hw/net/vhost_net.c               |   8 +-
 hw/net/virtio-net.c              |   2 -
 hw/virtio/virtio-pci.c           |  10 +++
 hw/virtio/virtio.c               |   9 +--
 tests/test-qdev-global-props.c   | 159 ++++++++++++++++++++++++++++++++++++---
 vl.c                             |   2 +-
 12 files changed, 204 insertions(+), 55 deletions(-)

[Qemu-devel] [PULL 01/12] hw/machine: Free old values of string properties

[Michael S. Tsirkin]

From: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Amos Kong <akong@redhat.com>
Cc: qemu-stable@nongnu.org
---
 hw/core/machine.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index f0046d6..7f3418c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -24,6 +24,7 @@ static void machine_set_accel(Object *obj, const char *value, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
 
+    g_free(ms->accel);
     ms->accel = g_strdup(value);
 }
 
@@ -79,6 +80,7 @@ static void machine_set_kernel(Object *obj, const char *value, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
 
+    g_free(ms->kernel_filename);
     ms->kernel_filename = g_strdup(value);
 }
 
@@ -93,6 +95,7 @@ static void machine_set_initrd(Object *obj, const char *value, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
 
+    g_free(ms->initrd_filename);
     ms->initrd_filename = g_strdup(value);
 }
 
@@ -107,6 +110,7 @@ static void machine_set_append(Object *obj, const char *value, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
 
+    g_free(ms->kernel_cmdline);
     ms->kernel_cmdline = g_strdup(value);
 }
 
@@ -121,6 +125,7 @@ static void machine_set_dtb(Object *obj, const char *value, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
 
+    g_free(ms->dtb);
     ms->dtb = g_strdup(value);
 }
 
@@ -135,6 +140,7 @@ static void machine_set_dumpdtb(Object *obj, const char *value, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
 
+    g_free(ms->dumpdtb);
     ms->dumpdtb = g_strdup(value);
 }
 
@@ -176,6 +182,7 @@ static void machine_set_dt_compatible(Object *obj, const char *value, Error **er
 {
     MachineState *ms = MACHINE(obj);
 
+    g_free(ms->dt_compatible);
     ms->dt_compatible = g_strdup(value);
 }
 
@@ -232,6 +239,7 @@ static void machine_set_firmware(Object *obj, const char *value, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
 
+    g_free(ms->firmware);
     ms->firmware = g_strdup(value);
 }
 
-- 
MST

[Qemu-devel] [PULL 02/12] test-qdev-global-props: Trivial comment fix

[Michael S. Tsirkin]

From: Eduardo Habkost <ehabkost@redhat.com>

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/test-qdev-global-props.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index 2bef04c..e1a1317 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -143,7 +143,7 @@ static const TypeInfo dynamic_prop_type = {
     .class_init = dynamic_class_init,
 };
 
-/* Test setting of static property using global properties */
+/* Test setting of dynamic properties using global properties */
 static void test_dynamic_globalprop(void)
 {
     MyType *mt;
-- 
MST

[Qemu-devel] [PULL 03/12] test-qdev-global-props: Run tests on subprocess

[Michael S. Tsirkin]

From: Eduardo Habkost <ehabkost@redhat.com>

There are multiple reasons for running the global property tests on a
subprocess:

* We need the global_props lists to be empty for each test case, so
  global properties from the previous test won't affect the next one;
* We don't want the qdev_prop_check_global() warnings to pollute test
  output;
* With a subprocess, we can ensure qdev_prop_check_global() is printing
  the warning messages it should.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/test-qdev-global-props.c | 49 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index e1a1317..34223a7 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -65,7 +65,7 @@ static const TypeInfo static_prop_type = {
 };
 
 /* Test simple static property setting to default value */
-static void test_static_prop(void)
+static void test_static_prop_subprocess(void)
 {
     MyType *mt;
 
@@ -75,8 +75,16 @@ static void test_static_prop(void)
     g_assert_cmpuint(mt->prop1, ==, PROP_DEFAULT);
 }
 
+static void test_static_prop(void)
+{
+    g_test_trap_subprocess("/qdev/properties/static/default/subprocess", 0, 0);
+    g_test_trap_assert_passed();
+    g_test_trap_assert_stderr("");
+    g_test_trap_assert_stdout("");
+}
+
 /* Test setting of static property using global properties */
-static void test_static_globalprop(void)
+static void test_static_globalprop_subprocess(void)
 {
     MyType *mt;
     static GlobalProperty props[] = {
@@ -93,6 +101,14 @@ static void test_static_globalprop(void)
     g_assert_cmpuint(mt->prop2, ==, PROP_DEFAULT);
 }
 
+static void test_static_globalprop(void)
+{
+    g_test_trap_subprocess("/qdev/properties/static/global/subprocess", 0, 0);
+    g_test_trap_assert_passed();
+    g_test_trap_assert_stderr("");
+    g_test_trap_assert_stdout("");
+}
+
 #define TYPE_DYNAMIC_PROPS "dynamic-prop-type"
 #define DYNAMIC_TYPE(obj) \
     OBJECT_CHECK(MyType, (obj), TYPE_DYNAMIC_PROPS)
@@ -144,7 +160,7 @@ static const TypeInfo dynamic_prop_type = {
 };
 
 /* Test setting of dynamic properties using global properties */
-static void test_dynamic_globalprop(void)
+static void test_dynamic_globalprop_subprocess(void)
 {
     MyType *mt;
     static GlobalProperty props[] = {
@@ -166,6 +182,16 @@ static void test_dynamic_globalprop(void)
     g_assert_cmpuint(all_used, ==, 1);
 }
 
+static void test_dynamic_globalprop(void)
+{
+    g_test_trap_subprocess("/qdev/properties/dynamic/global/subprocess", 0, 0);
+    g_test_trap_assert_passed();
+    g_test_trap_assert_stderr_unmatched("*prop1*");
+    g_test_trap_assert_stderr_unmatched("*prop2*");
+    g_test_trap_assert_stderr("*Warning: \"-global dynamic-prop-type-bad.prop3=103\" not used\n*");
+    g_test_trap_assert_stdout("");
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
@@ -174,9 +200,20 @@ int main(int argc, char **argv)
     type_register_static(&static_prop_type);
     type_register_static(&dynamic_prop_type);
 
-    g_test_add_func("/qdev/properties/static/default", test_static_prop);
-    g_test_add_func("/qdev/properties/static/global", test_static_globalprop);
-    g_test_add_func("/qdev/properties/dynamic/global", test_dynamic_globalprop);
+    g_test_add_func("/qdev/properties/static/default/subprocess",
+                    test_static_prop_subprocess);
+    g_test_add_func("/qdev/properties/static/default",
+                    test_static_prop);
+
+    g_test_add_func("/qdev/properties/static/global/subprocess",
+                    test_static_globalprop_subprocess);
+    g_test_add_func("/qdev/properties/static/global",
+                    test_static_globalprop);
+
+    g_test_add_func("/qdev/properties/dynamic/global/subprocess",
+                    test_dynamic_globalprop_subprocess);
+    g_test_add_func("/qdev/properties/dynamic/global",
+                    test_dynamic_globalprop);
 
     g_test_run();
 
-- 
MST

Re: [Qemu-devel] [PULL 03/12] test-qdev-global-props: Run tests on subprocess

[Michael Roth]

Quoting Michael S. Tsirkin (2014-09-14 13:41:23)
> From: Eduardo Habkost <ehabkost@redhat.com>
> 
> There are multiple reasons for running the global property tests on a
> subprocess:
> 
> * We need the global_props lists to be empty for each test case, so
>   global properties from the previous test won't affect the next one;
> * We don't want the qdev_prop_check_global() warnings to pollute test
>   output;
> * With a subprocess, we can ensure qdev_prop_check_global() is printing
>   the warning messages it should.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  tests/test-qdev-global-props.c | 49 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 43 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
> index e1a1317..34223a7 100644
> --- a/tests/test-qdev-global-props.c
> +++ b/tests/test-qdev-global-props.c
> @@ -65,7 +65,7 @@ static const TypeInfo static_prop_type = {
>  };
> 
>  /* Test simple static property setting to default value */
> -static void test_static_prop(void)
> +static void test_static_prop_subprocess(void)
>  {
>      MyType *mt;
> 
> @@ -75,8 +75,16 @@ static void test_static_prop(void)
>      g_assert_cmpuint(mt->prop1, ==, PROP_DEFAULT);
>  }
> 
> +static void test_static_prop(void)
> +{
> +    g_test_trap_subprocess("/qdev/properties/static/default/subprocess", 0, 0);
> +    g_test_trap_assert_passed();
> +    g_test_trap_assert_stderr("");
> +    g_test_trap_assert_stdout("");
> +}

<snip>

>  int main(int argc, char **argv)
>  {
>      g_test_init(&argc, &argv, NULL);
> @@ -174,9 +200,20 @@ int main(int argc, char **argv)
>      type_register_static(&static_prop_type);
>      type_register_static(&dynamic_prop_type);
> 
> -    g_test_add_func("/qdev/properties/static/default", test_static_prop);
> -    g_test_add_func("/qdev/properties/static/global", test_static_globalprop);
> -    g_test_add_func("/qdev/properties/dynamic/global", test_dynamic_globalprop);
> +    g_test_add_func("/qdev/properties/static/default/subprocess",
> +                    test_static_prop_subprocess);
> +    g_test_add_func("/qdev/properties/static/default",
> +                    test_static_prop);

Since in the code above test_static_prop is actually the test that re-runs
/qdev/properties/static/default/subprocess under g_test_trap_subprocess, aren't
the tests (or test function implementations) backwards?

> -- 
> MST

Re: [Qemu-devel] [PULL 03/12] test-qdev-global-props: Run tests on subprocess

[Paolo Bonzini]

Il 18/09/2014 18:29, Michael Roth ha scritto:
> <snip>
> 
>> >  int main(int argc, char **argv)
>> >  {
>> >      g_test_init(&argc, &argv, NULL);
>> > @@ -174,9 +200,20 @@ int main(int argc, char **argv)
>> >      type_register_static(&static_prop_type);
>> >      type_register_static(&dynamic_prop_type);
>> > 
>> > -    g_test_add_func("/qdev/properties/static/default", test_static_prop);
>> > -    g_test_add_func("/qdev/properties/static/global", test_static_globalprop);
>> > -    g_test_add_func("/qdev/properties/dynamic/global", test_dynamic_globalprop);
>> > +    g_test_add_func("/qdev/properties/static/default/subprocess",
>> > +                    test_static_prop_subprocess);
>> > +    g_test_add_func("/qdev/properties/static/default",
>> > +                    test_static_prop);
> Since in the code above test_static_prop is actually the test that re-runs
> /qdev/properties/static/default/subprocess under g_test_trap_subprocess, aren't
> the tests (or test function implementations) backwards?
> 

No, it's correct.  The parent test is the parent function, the child
test is the subprocess function.  The child test is automagically
skipped by GTest, I don't know how that works.

Paolo

Re: [Qemu-devel] [PULL 03/12] test-qdev-global-props: Run tests on subprocess

[Michael S. Tsirkin]

On Thu, Sep 18, 2014 at 07:06:16PM +0200, Paolo Bonzini wrote:
> Il 18/09/2014 18:29, Michael Roth ha scritto:
> > <snip>
> > 
> >> >  int main(int argc, char **argv)
> >> >  {
> >> >      g_test_init(&argc, &argv, NULL);
> >> > @@ -174,9 +200,20 @@ int main(int argc, char **argv)
> >> >      type_register_static(&static_prop_type);
> >> >      type_register_static(&dynamic_prop_type);
> >> > 
> >> > -    g_test_add_func("/qdev/properties/static/default", test_static_prop);
> >> > -    g_test_add_func("/qdev/properties/static/global", test_static_globalprop);
> >> > -    g_test_add_func("/qdev/properties/dynamic/global", test_dynamic_globalprop);
> >> > +    g_test_add_func("/qdev/properties/static/default/subprocess",
> >> > +                    test_static_prop_subprocess);
> >> > +    g_test_add_func("/qdev/properties/static/default",
> >> > +                    test_static_prop);
> > Since in the code above test_static_prop is actually the test that re-runs
> > /qdev/properties/static/default/subprocess under g_test_trap_subprocess, aren't
> > the tests (or test function implementations) backwards?
> > 
> 
> No, it's correct.  The parent test is the parent function, the child
> test is the subprocess function.  The child test is automagically
> skipped by GTest, I don't know how that works.
> 
> Paolo

Based on the "subprocess" string in the path.

Re: [Qemu-devel] [PULL 03/12] test-qdev-global-props: Run tests on subprocess

[Eduardo Habkost]

On Thu, Sep 18, 2014 at 11:29:47AM -0500, Michael Roth wrote:
> Quoting Michael S. Tsirkin (2014-09-14 13:41:23)
> > From: Eduardo Habkost <ehabkost@redhat.com>
> > 
> > There are multiple reasons for running the global property tests on a
> > subprocess:
> > 
> > * We need the global_props lists to be empty for each test case, so
> >   global properties from the previous test won't affect the next one;
> > * We don't want the qdev_prop_check_global() warnings to pollute test
> >   output;
> > * With a subprocess, we can ensure qdev_prop_check_global() is printing
> >   the warning messages it should.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  tests/test-qdev-global-props.c | 49 ++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 43 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
> > index e1a1317..34223a7 100644
> > --- a/tests/test-qdev-global-props.c
> > +++ b/tests/test-qdev-global-props.c
> > @@ -65,7 +65,7 @@ static const TypeInfo static_prop_type = {
> >  };
> > 
> >  /* Test simple static property setting to default value */
> > -static void test_static_prop(void)
> > +static void test_static_prop_subprocess(void)
> >  {
> >      MyType *mt;
> > 
> > @@ -75,8 +75,16 @@ static void test_static_prop(void)
> >      g_assert_cmpuint(mt->prop1, ==, PROP_DEFAULT);
> >  }
> > 
> > +static void test_static_prop(void)
> > +{
> > +    g_test_trap_subprocess("/qdev/properties/static/default/subprocess", 0, 0);
> > +    g_test_trap_assert_passed();
> > +    g_test_trap_assert_stderr("");
> > +    g_test_trap_assert_stdout("");
> > +}
> 
> <snip>
> 
> >  int main(int argc, char **argv)
> >  {
> >      g_test_init(&argc, &argv, NULL);
> > @@ -174,9 +200,20 @@ int main(int argc, char **argv)
> >      type_register_static(&static_prop_type);
> >      type_register_static(&dynamic_prop_type);
> > 
> > -    g_test_add_func("/qdev/properties/static/default", test_static_prop);
> > -    g_test_add_func("/qdev/properties/static/global", test_static_globalprop);
> > -    g_test_add_func("/qdev/properties/dynamic/global", test_dynamic_globalprop);
> > +    g_test_add_func("/qdev/properties/static/default/subprocess",
> > +                    test_static_prop_subprocess);
> > +    g_test_add_func("/qdev/properties/static/default",
> > +                    test_static_prop);
> 
> Since in the code above test_static_prop is actually the test that re-runs
> /qdev/properties/static/default/subprocess under g_test_trap_subprocess, aren't
> the tests (or test function implementations) backwards?

Your description is correct: ./static/default is test_static_prop(),
which runs ./static/default/subprocess (test_static_prop_subprocess())
in a subprocess. But I don't understand what's backwards. Are you
talking about the function names, test path strings, or about the
ordering of the g_test_add_func() calls?

Note that the test case that will be run in a subprocess must contain
the "subprocess" component in its path, otherwise g_test_run() will try
to run it as a normal test case. So for the test path, we don't have a
choice. For the function names, I am just following the same convention
used for the test path.

-- 
Eduardo

Re: [Qemu-devel] [PULL 03/12] test-qdev-global-props: Run tests on subprocess

[Michael Roth]

Quoting Eduardo Habkost (2014-09-18 12:27:12)
> On Thu, Sep 18, 2014 at 11:29:47AM -0500, Michael Roth wrote:
> > Quoting Michael S. Tsirkin (2014-09-14 13:41:23)
> > > From: Eduardo Habkost <ehabkost@redhat.com>
> > > 
> > > There are multiple reasons for running the global property tests on a
> > > subprocess:
> > > 
> > > * We need the global_props lists to be empty for each test case, so
> > >   global properties from the previous test won't affect the next one;
> > > * We don't want the qdev_prop_check_global() warnings to pollute test
> > >   output;
> > > * With a subprocess, we can ensure qdev_prop_check_global() is printing
> > >   the warning messages it should.
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  tests/test-qdev-global-props.c | 49 ++++++++++++++++++++++++++++++++++++------
> > >  1 file changed, 43 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
> > > index e1a1317..34223a7 100644
> > > --- a/tests/test-qdev-global-props.c
> > > +++ b/tests/test-qdev-global-props.c
> > > @@ -65,7 +65,7 @@ static const TypeInfo static_prop_type = {
> > >  };
> > > 
> > >  /* Test simple static property setting to default value */
> > > -static void test_static_prop(void)
> > > +static void test_static_prop_subprocess(void)
> > >  {
> > >      MyType *mt;
> > > 
> > > @@ -75,8 +75,16 @@ static void test_static_prop(void)
> > >      g_assert_cmpuint(mt->prop1, ==, PROP_DEFAULT);
> > >  }
> > > 
> > > +static void test_static_prop(void)
> > > +{
> > > +    g_test_trap_subprocess("/qdev/properties/static/default/subprocess", 0, 0);
> > > +    g_test_trap_assert_passed();
> > > +    g_test_trap_assert_stderr("");
> > > +    g_test_trap_assert_stdout("");
> > > +}
> > 
> > <snip>
> > 
> > >  int main(int argc, char **argv)
> > >  {
> > >      g_test_init(&argc, &argv, NULL);
> > > @@ -174,9 +200,20 @@ int main(int argc, char **argv)
> > >      type_register_static(&static_prop_type);
> > >      type_register_static(&dynamic_prop_type);
> > > 
> > > -    g_test_add_func("/qdev/properties/static/default", test_static_prop);
> > > -    g_test_add_func("/qdev/properties/static/global", test_static_globalprop);
> > > -    g_test_add_func("/qdev/properties/dynamic/global", test_dynamic_globalprop);
> > > +    g_test_add_func("/qdev/properties/static/default/subprocess",
> > > +                    test_static_prop_subprocess);
> > > +    g_test_add_func("/qdev/properties/static/default",
> > > +                    test_static_prop);
> > 
> > Since in the code above test_static_prop is actually the test that re-runs
> > /qdev/properties/static/default/subprocess under g_test_trap_subprocess, aren't
> > the tests (or test function implementations) backwards?
> 
> Your description is correct: ./static/default is test_static_prop(),
> which runs ./static/default/subprocess (test_static_prop_subprocess())
> in a subprocess. But I don't understand what's backwards. Are you
> talking about the function names, test path strings, or about the
> ordering of the g_test_add_func() calls?

I was under the false impression that this was adding a "subprocess" variant of
each existing test, where the subprocess variants were supposed to have "subprocess"
after them in the test paths to differentiate them, and the original cases would
retain the same paths. In that context the tests seemed reverse.

Sorry for the confusion, reading the glib documentation would've made this
fairly clear (though still a bit surprising, imo)

> 
> Note that the test case that will be run in a subprocess must contain
> the "subprocess" component in its path, otherwise g_test_run() will try
> to run it as a normal test case. So for the test path, we don't have a
> choice. For the function names, I am just following the same convention
> used for the test path.
> 
> -- 
> Eduardo

[Qemu-devel] [PULL 04/12] test-qdev-global-props: Initialize not_used=true for all props

[Michael S. Tsirkin]

From: Eduardo Habkost <ehabkost@redhat.com>

This will ensure we are actually testing the code which sets
not_used=false when the property is used.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/test-qdev-global-props.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index 34223a7..5488937 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -164,8 +164,8 @@ static void test_dynamic_globalprop_subprocess(void)
 {
     MyType *mt;
     static GlobalProperty props[] = {
-        { TYPE_DYNAMIC_PROPS, "prop1", "101" },
-        { TYPE_DYNAMIC_PROPS, "prop2", "102" },
+        { TYPE_DYNAMIC_PROPS, "prop1", "101", true },
+        { TYPE_DYNAMIC_PROPS, "prop2", "102", true },
         { TYPE_DYNAMIC_PROPS"-bad", "prop3", "103", true },
         {}
     };
@@ -180,6 +180,9 @@ static void test_dynamic_globalprop_subprocess(void)
     g_assert_cmpuint(mt->prop2, ==, 102);
     all_used = qdev_prop_check_global();
     g_assert_cmpuint(all_used, ==, 1);
+    g_assert(!props[0].not_used);
+    g_assert(!props[1].not_used);
+    g_assert(props[2].not_used);
 }
 
 static void test_dynamic_globalprop(void)
-- 
MST

[Qemu-devel] [PULL 05/12] test-qdev-global-props: Test handling of hotpluggable and non-device types

[Michael S. Tsirkin]

From: Eduardo Habkost <ehabkost@redhat.com>

Ensure no warning will be printed for hotpluggable types, and warnings
will be printed for non-device types.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/test-qdev-global-props.c | 55 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index 5488937..26d8cb2 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -113,6 +113,9 @@ static void test_static_globalprop(void)
 #define DYNAMIC_TYPE(obj) \
     OBJECT_CHECK(MyType, (obj), TYPE_DYNAMIC_PROPS)
 
+#define TYPE_UNUSED_HOTPLUG   "hotplug-type"
+#define TYPE_UNUSED_NOHOTPLUG "nohotplug-type"
+
 static void prop1_accessor(Object *obj,
                            Visitor *v,
                            void *opaque,
@@ -159,6 +162,45 @@ static const TypeInfo dynamic_prop_type = {
     .class_init = dynamic_class_init,
 };
 
+static void hotplug_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = NULL;
+    dc->hotpluggable = true;
+}
+
+static const TypeInfo hotplug_type = {
+    .name = TYPE_UNUSED_HOTPLUG,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(MyType),
+    .instance_init = dynamic_instance_init,
+    .class_init = hotplug_class_init,
+};
+
+static void nohotplug_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = NULL;
+    dc->hotpluggable = false;
+}
+
+static const TypeInfo nohotplug_type = {
+    .name = TYPE_UNUSED_NOHOTPLUG,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(MyType),
+    .instance_init = dynamic_instance_init,
+    .class_init = nohotplug_class_init,
+};
+
+#define TYPE_NONDEVICE "nondevice-type"
+
+static const TypeInfo nondevice_type = {
+    .name = TYPE_NONDEVICE,
+    .parent = TYPE_OBJECT,
+};
+
 /* Test setting of dynamic properties using global properties */
 static void test_dynamic_globalprop_subprocess(void)
 {
@@ -167,6 +209,10 @@ static void test_dynamic_globalprop_subprocess(void)
         { TYPE_DYNAMIC_PROPS, "prop1", "101", true },
         { TYPE_DYNAMIC_PROPS, "prop2", "102", true },
         { TYPE_DYNAMIC_PROPS"-bad", "prop3", "103", true },
+        /* .not_used=false to emulate what qdev_add_one_global() does: */
+        { TYPE_UNUSED_HOTPLUG, "prop4", "104", false },
+        { TYPE_UNUSED_NOHOTPLUG, "prop5", "105", true },
+        { TYPE_NONDEVICE, "prop6", "106", true },
         {}
     };
     int all_used;
@@ -183,6 +229,9 @@ static void test_dynamic_globalprop_subprocess(void)
     g_assert(!props[0].not_used);
     g_assert(!props[1].not_used);
     g_assert(props[2].not_used);
+    g_assert(!props[3].not_used);
+    g_assert(props[4].not_used);
+    g_assert(props[5].not_used);
 }
 
 static void test_dynamic_globalprop(void)
@@ -192,6 +241,9 @@ static void test_dynamic_globalprop(void)
     g_test_trap_assert_stderr_unmatched("*prop1*");
     g_test_trap_assert_stderr_unmatched("*prop2*");
     g_test_trap_assert_stderr("*Warning: \"-global dynamic-prop-type-bad.prop3=103\" not used\n*");
+    g_test_trap_assert_stderr_unmatched("*prop4*");
+    g_test_trap_assert_stderr("*Warning: \"-global nohotplug-type.prop5=105\" not used\n*");
+    g_test_trap_assert_stderr("*Warning: \"-global nondevice-type.prop6=106\" not used\n*");
     g_test_trap_assert_stdout("");
 }
 
@@ -202,6 +254,9 @@ int main(int argc, char **argv)
     module_call_init(MODULE_INIT_QOM);
     type_register_static(&static_prop_type);
     type_register_static(&dynamic_prop_type);
+    type_register_static(&hotplug_type);
+    type_register_static(&nohotplug_type);
+    type_register_static(&nondevice_type);
 
     g_test_add_func("/qdev/properties/static/default/subprocess",
                     test_static_prop_subprocess);
-- 
MST

[Qemu-devel] [PULL 06/12] qdev: Rename qdev_prop_check_global() to qdev_prop_check_globals()

[Michael S. Tsirkin]

From: Eduardo Habkost <ehabkost@redhat.com>

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/qdev-properties.h   | 2 +-
 hw/core/qdev-properties.c      | 2 +-
 tests/test-qdev-global-props.c | 2 +-
 vl.c                           | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 77fe3a1..ae56ee5 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -177,7 +177,7 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
 
 void qdev_prop_register_global(GlobalProperty *prop);
 void qdev_prop_register_global_list(GlobalProperty *props);
-int qdev_prop_check_global(void);
+int qdev_prop_check_globals(void);
 void qdev_prop_set_globals(DeviceState *dev, Error **errp);
 void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename,
                                     Error **errp);
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 3d12560..9075453 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -955,7 +955,7 @@ void qdev_prop_register_global_list(GlobalProperty *props)
     }
 }
 
-int qdev_prop_check_global(void)
+int qdev_prop_check_globals(void)
 {
     GlobalProperty *prop;
     int ret = 0;
diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index 26d8cb2..e1b008a 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -224,7 +224,7 @@ static void test_dynamic_globalprop_subprocess(void)
 
     g_assert_cmpuint(mt->prop1, ==, 101);
     g_assert_cmpuint(mt->prop2, ==, 102);
-    all_used = qdev_prop_check_global();
+    all_used = qdev_prop_check_globals();
     g_assert_cmpuint(all_used, ==, 1);
     g_assert(!props[0].not_used);
     g_assert(!props[1].not_used);
diff --git a/vl.c b/vl.c
index 9c9acf5..2aaa558 100644
--- a/vl.c
+++ b/vl.c
@@ -4541,7 +4541,7 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
-    qdev_prop_check_global();
+    qdev_prop_check_globals();
     if (vmstate_dump_file) {
         /* dump and exit */
         dump_vmstate_json_to_file(vmstate_dump_file);
-- 
MST

[Qemu-devel] [PULL 07/12] qdev: Move global validation to a single function

[Michael S. Tsirkin]

From: Eduardo Habkost <ehabkost@redhat.com>

Currently GlobalProperty.not_used=false has multiple meanings:

* It may be a property for a hotpluggable device, which may or may not
  have been used by a device;
* It may be a machine-type-provided property, which may or may not have
  been used by a device.
* It may be a user-provided property that was actually not used by
  any device.

Simplify the logic by having two separate fields: 'user_provided' and
'used'. This allows the entire global property validation logic to be
contained in a single function, and allows more specific error messages.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/qdev-core.h           | 10 +++---
 hw/core/qdev-properties-system.c | 18 +----------
 hw/core/qdev-properties.c        | 28 +++++++++++++----
 tests/test-qdev-global-props.c   | 66 +++++++++++++++++++++++++++++++++-------
 4 files changed, 83 insertions(+), 39 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 0799ff2..178fee2 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -242,16 +242,16 @@ struct PropertyInfo {
 
 /**
  * GlobalProperty:
- * @not_used: Track use of a global property.  Defaults to false in all C99
- * struct initializations.
- *
- * This prevents reports of .compat_props when they are not used.
+ * @user_provided: Set to true if property comes from user-provided config
+ * (command-line or config file).
+ * @used: Set to true if property was used when initializing a device.
  */
 typedef struct GlobalProperty {
     const char *driver;
     const char *property;
     const char *value;
-    bool not_used;
+    bool user_provided;
+    bool used;
     QTAILQ_ENTRY(GlobalProperty) next;
 } GlobalProperty;
 
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index ae0900f..84caa1d 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -388,28 +388,12 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
 static int qdev_add_one_global(QemuOpts *opts, void *opaque)
 {
     GlobalProperty *g;
-    ObjectClass *oc;
 
     g = g_malloc0(sizeof(*g));
     g->driver   = qemu_opt_get(opts, "driver");
     g->property = qemu_opt_get(opts, "property");
     g->value    = qemu_opt_get(opts, "value");
-    oc = object_class_dynamic_cast(object_class_by_name(g->driver),
-                                   TYPE_DEVICE);
-    if (oc) {
-        DeviceClass *dc = DEVICE_CLASS(oc);
-
-        if (dc->hotpluggable) {
-            /* If hotpluggable then skip not_used checking. */
-            g->not_used = false;
-        } else {
-            /* Maybe a typo. */
-            g->not_used = true;
-        }
-    } else {
-        /* Maybe a typo. */
-        g->not_used = true;
-    }
+    g->user_provided = true;
     qdev_prop_register_global(g);
     return 0;
 }
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 9075453..66556d3 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -961,13 +961,29 @@ int qdev_prop_check_globals(void)
     int ret = 0;
 
     QTAILQ_FOREACH(prop, &global_props, next) {
-        if (!prop->not_used) {
+        ObjectClass *oc;
+        DeviceClass *dc;
+        if (prop->used) {
+            continue;
+        }
+        if (!prop->user_provided) {
+            continue;
+        }
+        oc = object_class_by_name(prop->driver);
+        oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
+        if (!oc) {
+            error_report("Warning: global %s.%s has invalid class name",
+                       prop->driver, prop->property);
+            ret = 1;
+            continue;
+        }
+        dc = DEVICE_CLASS(oc);
+        if (!dc->hotpluggable && !prop->used) {
+            error_report("Warning: global %s.%s=%s not used",
+                       prop->driver, prop->property, prop->value);
+            ret = 1;
             continue;
         }
-        ret = 1;
-        error_report("Warning: \"-global %s.%s=%s\" not used",
-                     prop->driver, prop->property, prop->value);
-
     }
     return ret;
 }
@@ -983,7 +999,7 @@ void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename,
         if (strcmp(typename, prop->driver) != 0) {
             continue;
         }
-        prop->not_used = false;
+        prop->used = true;
         object_property_parse(OBJECT(dev), prop->value, prop->property, &err);
         if (err != NULL) {
             error_propagate(errp, err);
diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index e1b008a..0be9835 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -209,8 +209,7 @@ static void test_dynamic_globalprop_subprocess(void)
         { TYPE_DYNAMIC_PROPS, "prop1", "101", true },
         { TYPE_DYNAMIC_PROPS, "prop2", "102", true },
         { TYPE_DYNAMIC_PROPS"-bad", "prop3", "103", true },
-        /* .not_used=false to emulate what qdev_add_one_global() does: */
-        { TYPE_UNUSED_HOTPLUG, "prop4", "104", false },
+        { TYPE_UNUSED_HOTPLUG, "prop4", "104", true },
         { TYPE_UNUSED_NOHOTPLUG, "prop5", "105", true },
         { TYPE_NONDEVICE, "prop6", "106", true },
         {}
@@ -226,12 +225,12 @@ static void test_dynamic_globalprop_subprocess(void)
     g_assert_cmpuint(mt->prop2, ==, 102);
     all_used = qdev_prop_check_globals();
     g_assert_cmpuint(all_used, ==, 1);
-    g_assert(!props[0].not_used);
-    g_assert(!props[1].not_used);
-    g_assert(props[2].not_used);
-    g_assert(!props[3].not_used);
-    g_assert(props[4].not_used);
-    g_assert(props[5].not_used);
+    g_assert(props[0].used);
+    g_assert(props[1].used);
+    g_assert(!props[2].used);
+    g_assert(!props[3].used);
+    g_assert(!props[4].used);
+    g_assert(!props[5].used);
 }
 
 static void test_dynamic_globalprop(void)
@@ -240,10 +239,50 @@ static void test_dynamic_globalprop(void)
     g_test_trap_assert_passed();
     g_test_trap_assert_stderr_unmatched("*prop1*");
     g_test_trap_assert_stderr_unmatched("*prop2*");
-    g_test_trap_assert_stderr("*Warning: \"-global dynamic-prop-type-bad.prop3=103\" not used\n*");
+    g_test_trap_assert_stderr("*Warning: global dynamic-prop-type-bad.prop3 has invalid class name\n*");
     g_test_trap_assert_stderr_unmatched("*prop4*");
-    g_test_trap_assert_stderr("*Warning: \"-global nohotplug-type.prop5=105\" not used\n*");
-    g_test_trap_assert_stderr("*Warning: \"-global nondevice-type.prop6=106\" not used\n*");
+    g_test_trap_assert_stderr("*Warning: global nohotplug-type.prop5=105 not used\n*");
+    g_test_trap_assert_stderr("*Warning: global nondevice-type.prop6 has invalid class name\n*");
+    g_test_trap_assert_stdout("");
+}
+
+/* Test setting of dynamic properties using user_provided=false properties */
+static void test_dynamic_globalprop_nouser_subprocess(void)
+{
+    MyType *mt;
+    static GlobalProperty props[] = {
+        { TYPE_DYNAMIC_PROPS, "prop1", "101" },
+        { TYPE_DYNAMIC_PROPS, "prop2", "102" },
+        { TYPE_DYNAMIC_PROPS"-bad", "prop3", "103" },
+        { TYPE_UNUSED_HOTPLUG, "prop4", "104" },
+        { TYPE_UNUSED_NOHOTPLUG, "prop5", "105" },
+        { TYPE_NONDEVICE, "prop6", "106" },
+        {}
+    };
+    int all_used;
+
+    qdev_prop_register_global_list(props);
+
+    mt = DYNAMIC_TYPE(object_new(TYPE_DYNAMIC_PROPS));
+    qdev_init_nofail(DEVICE(mt));
+
+    g_assert_cmpuint(mt->prop1, ==, 101);
+    g_assert_cmpuint(mt->prop2, ==, 102);
+    all_used = qdev_prop_check_globals();
+    g_assert_cmpuint(all_used, ==, 0);
+    g_assert(props[0].used);
+    g_assert(props[1].used);
+    g_assert(!props[2].used);
+    g_assert(!props[3].used);
+    g_assert(!props[4].used);
+    g_assert(!props[5].used);
+}
+
+static void test_dynamic_globalprop_nouser(void)
+{
+    g_test_trap_subprocess("/qdev/properties/dynamic/global/nouser/subprocess", 0, 0);
+    g_test_trap_assert_passed();
+    g_test_trap_assert_stderr("");
     g_test_trap_assert_stdout("");
 }
 
@@ -273,6 +312,11 @@ int main(int argc, char **argv)
     g_test_add_func("/qdev/properties/dynamic/global",
                     test_dynamic_globalprop);
 
+    g_test_add_func("/qdev/properties/dynamic/global/nouser/subprocess",
+                    test_dynamic_globalprop_nouser_subprocess);
+    g_test_add_func("/qdev/properties/dynamic/global/nouser",
+                    test_dynamic_globalprop_nouser);
+
     g_test_run();
 
     return 0;
-- 
MST

[Qemu-devel] [PULL 08/12] Revert "rng-egd: remove redundant free"

[Michael S. Tsirkin]

From: Eduardo Habkost <ehabkost@redhat.com>

This reverts commit 5e490b6a504912225dff0e520e1c6af68295d238.

Cc: qemu-stable@nongnu.org
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 backends/rng-egd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/backends/rng-egd.c b/backends/rng-egd.c
index 25bb3b4..2962795 100644
--- a/backends/rng-egd.c
+++ b/backends/rng-egd.c
@@ -169,6 +169,7 @@ static void rng_egd_set_chardev(Object *obj, const char *value, Error **errp)
     if (b->opened) {
         error_set(errp, QERR_PERMISSION_DENIED);
     } else {
+        g_free(s->chr_name);
         s->chr_name = g_strdup(value);
     }
 }
-- 
MST

[Qemu-devel] [PULL 09/12] virtio-net: drop assert on vm stop

[Michael S. Tsirkin]

On vm stop, vm_running state set to stopped
before device is notified, so callbacks can get envoked with
vm_running = false; and this is not an error.

Cc: qemu-stable@nongnu.org
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/virtio-net.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 826a2a5..2040eac 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1125,8 +1125,6 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
         return num_packets;
     }
 
-    assert(vdev->vm_running);
-
     if (q->async_tx.elem.out_num) {
         virtio_queue_set_notification(q->tx_vq, 0);
         return num_packets;
-- 
MST

[Qemu-devel] [PULL 10/12] Revert "virtio: don't call device on !vm_running"

[Michael S. Tsirkin]

This reverts commit a1bc7b827e422e1ff065640d8ec5347c4aadfcd8.
    virtio: don't call device on !vm_running
It turns out that virtio net assumes that vm_running
is updated before device status callback in many places,
so this change leads to asserts.
Previous commit fixes the root issue that motivated
a1bc7b827e422e1ff065640d8ec5347c4aadfcd8 differently,
so there's no longer a need for this change.

In the future, we might be able to drop checking vm_running
completely, and check vm state directly.

Reported-by: Dietmar Maurer <dietmar@proxmox.com>
Cc: qemu-stable@nongnu.org
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index ac22238..5c98180 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1108,10 +1108,7 @@ static void virtio_vmstate_change(void *opaque, int running, RunState state)
     BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     bool backend_run = running && (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK);
-
-    if (running) {
-        vdev->vm_running = running;
-    }
+    vdev->vm_running = running;
 
     if (backend_run) {
         virtio_set_status(vdev, vdev->status);
@@ -1124,10 +1121,6 @@ static void virtio_vmstate_change(void *opaque, int running, RunState state)
     if (!backend_run) {
         virtio_set_status(vdev, vdev->status);
     }
-
-    if (!running) {
-        vdev->vm_running = running;
-    }
 }
 
 void virtio_init(VirtIODevice *vdev, const char *name,
-- 
MST

[Qemu-devel] [PULL 11/12] virtio-pci: enable bus master for old guests

[Michael S. Tsirkin]

commit cc943c36faa192cd4b32af8fe5edb31894017d35
    pci: Use bus master address space for delivering MSI/MSI-X messages
breaks virtio-net for rhel6.[56] x86 guests because they don't
enable bus mastering for virtio PCI devices. For the same reason,
rhel6.[56] ppc64 guests cannot boot on a virtio-blk disk anymore.

Old guests forgot to enable bus mastering, enable it automatically on
DRIVER (guests use some devices before DRIVER_OK).

Reported-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Tested-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-pci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ddb5da1..a827cd4 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -314,6 +314,16 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
             msix_unuse_all_vectors(&proxy->pci_dev);
         }
 
+        /* Linux before 2.6.34 drives the device without enabling
+           the PCI device bus master bit. Enable it automatically
+           for the guest. This is a PCI spec violation but so is
+           initiating DMA with bus master bit clear. */
+        if (val == (VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER)) {
+            pci_default_write_config(&proxy->pci_dev, PCI_COMMAND,
+                                     proxy->pci_dev.config[PCI_COMMAND] |
+                                     PCI_COMMAND_MASTER, 1);
+        }
+
         /* Linux before 2.6.34 sets the device as OK without enabling
            the PCI device bus master bit. In this case we need to disable
            some safety checks. */
-- 
MST

[Qemu-devel] [PULL 12/12] vhost-user: fix VIRTIO_NET_F_MRG_RXBUF negotiation

[Michael S. Tsirkin]

From: Damjan Marion <damarion@cisco.com>

Header length check should happen only if backend is kernel. For user
backend there is no reason to reset this bit.

vhost-user code does not define .has_vnet_hdr_len so
VIRTIO_NET_F_MRG_RXBUF cannot be negotiated even if both sides
support it.

Signed-off-by: Damjan Marion <damarion@cisco.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/vhost_net.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index b21e7a4..77bb93e 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -163,11 +163,11 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
     if (r < 0) {
         goto fail;
     }
-    if (!qemu_has_vnet_hdr_len(options->net_backend,
-                               sizeof(struct virtio_net_hdr_mrg_rxbuf))) {
-        net->dev.features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
-    }
     if (backend_kernel) {
+        if (!qemu_has_vnet_hdr_len(options->net_backend,
+                               sizeof(struct virtio_net_hdr_mrg_rxbuf))) {
+            net->dev.features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
+        }
         if (~net->dev.features & net->dev.backend_features) {
             fprintf(stderr, "vhost lacks feature mask %" PRIu64
                    " for backend\n",
-- 
MST

Re: [Qemu-devel] [PULL 12/12] vhost-user: fix VIRTIO_NET_F_MRG_RXBUF negotiation

[Linhaifeng]

On 2014/9/15 2:41, Michael S. Tsirkin wrote:
> From: Damjan Marion <damarion@cisco.com>
> 
> Header length check should happen only if backend is kernel. For user
> backend there is no reason to reset this bit.
> 
> vhost-user code does not define .has_vnet_hdr_len so
> VIRTIO_NET_F_MRG_RXBUF cannot be negotiated even if both sides
> support it.
> 
> Signed-off-by: Damjan Marion <damarion@cisco.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/net/vhost_net.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index b21e7a4..77bb93e 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -163,11 +163,11 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>      if (r < 0) {
>          goto fail;
>      }
> -    if (!qemu_has_vnet_hdr_len(options->net_backend,
> -                               sizeof(struct virtio_net_hdr_mrg_rxbuf))) {
> -        net->dev.features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
> -    }
>      if (backend_kernel) {
> +        if (!qemu_has_vnet_hdr_len(options->net_backend,
> +                               sizeof(struct virtio_net_hdr_mrg_rxbuf))) {
> +            net->dev.features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
> +        }
>          if (~net->dev.features & net->dev.backend_features) {
>              fprintf(stderr, "vhost lacks feature mask %" PRIu64
>                     " for backend\n",
> 

why vhost-user code not define .has_vnet_hdr_len? I think vhost-user code should define it.

if packet bigger than MTU,this will be a bug?

Re: [Qemu-devel] [PULL 12/12] vhost-user: fix VIRTIO_NET_F_MRG_RXBUF negotiation

[Michael S. Tsirkin]

On Sun, Sep 14, 2014 at 09:41:56PM +0300, Michael S. Tsirkin wrote:
> From: Damjan Marion <damarion@cisco.com>
> 
> Header length check should happen only if backend is kernel. For user
> backend there is no reason to reset this bit.
> 
> vhost-user code does not define .has_vnet_hdr_len so
> VIRTIO_NET_F_MRG_RXBUF cannot be negotiated even if both sides
> support it.
> 
> Signed-off-by: Damjan Marion <damarion@cisco.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Cc: qemu-stable@nongnu.org

> ---
>  hw/net/vhost_net.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index b21e7a4..77bb93e 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -163,11 +163,11 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>      if (r < 0) {
>          goto fail;
>      }
> -    if (!qemu_has_vnet_hdr_len(options->net_backend,
> -                               sizeof(struct virtio_net_hdr_mrg_rxbuf))) {
> -        net->dev.features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
> -    }
>      if (backend_kernel) {
> +        if (!qemu_has_vnet_hdr_len(options->net_backend,
> +                               sizeof(struct virtio_net_hdr_mrg_rxbuf))) {
> +            net->dev.features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
> +        }
>          if (~net->dev.features & net->dev.backend_features) {
>              fprintf(stderr, "vhost lacks feature mask %" PRIu64
>                     " for backend\n",
> -- 
> MST
> 

Re: [Qemu-devel] [PULL 00/12] pci, pc, virtio, misc bugfixes

[Peter Maydell]

On 14 September 2014 11:41, Michael S. Tsirkin <mst@redhat.com> wrote:
> The following changes since commit 4c24f4004089a308c5de8ed720cf6bd1746aedd8:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140912' into staging (2014-09-12 15:12:26 +0100)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 36a315ef05921aef7a386ec08d866fbe3f626e08:
>
>   vhost-user: fix VIRTIO_NET_F_MRG_RXBUF negotiation (2014-09-14 21:33:01 +0300)
>
> ----------------------------------------------------------------
> pci, pc, virtio, misc bugfixes
>
> A bunch of bugfixes - some of these will make sense for 2.1.2
> Cc: qemu-stable included where appropriate.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>

Hi. I'm afraid this doesn't build for me:

/root/qemu/tests/test-qdev-global-props.c: In function ‘test_static_prop’:
/root/qemu/tests/test-qdev-global-props.c:80:5: error: implicit
declaration of function ‘g_test_trap_subprocess’
[-Werror=implicit-function-declaration]
/root/qemu/tests/test-qdev-global-props.c:80:5: error: nested extern
declaration of ‘g_test_trap_subprocess’ [-Werror=nested-externs]

This function was only added in glib 2.38, and our
minimum version is 2.12.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/12] pci, pc, virtio, misc bugfixes

[Michael S. Tsirkin]

On Mon, Sep 15, 2014 at 01:30:40PM -0700, Peter Maydell wrote:
> On 14 September 2014 11:41, Michael S. Tsirkin <mst@redhat.com> wrote:
> > The following changes since commit 4c24f4004089a308c5de8ed720cf6bd1746aedd8:
> >
> >   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140912' into staging (2014-09-12 15:12:26 +0100)
> >
> > are available in the git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to 36a315ef05921aef7a386ec08d866fbe3f626e08:
> >
> >   vhost-user: fix VIRTIO_NET_F_MRG_RXBUF negotiation (2014-09-14 21:33:01 +0300)
> >
> > ----------------------------------------------------------------
> > pci, pc, virtio, misc bugfixes
> >
> > A bunch of bugfixes - some of these will make sense for 2.1.2
> > Cc: qemu-stable included where appropriate.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> 
> Hi. I'm afraid this doesn't build for me:
> 
> /root/qemu/tests/test-qdev-global-props.c: In function ‘test_static_prop’:
> /root/qemu/tests/test-qdev-global-props.c:80:5: error: implicit
> declaration of function ‘g_test_trap_subprocess’
> [-Werror=implicit-function-declaration]
> /root/qemu/tests/test-qdev-global-props.c:80:5: error: nested extern
> declaration of ‘g_test_trap_subprocess’ [-Werror=nested-externs]
> 
> This function was only added in glib 2.38, and our
> minimum version is 2.12.
> 
> thanks
> -- PMM

The following should help?

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

diff --git a/configure b/configure
index 961bf6f..6f1284a 100755
--- a/configure
+++ b/configure
@@ -2716,6 +2716,12 @@ for i in $glib_modules; do
     fi
 done
 
+# g_test_trap_subprocess added in 2.38. Used by some tests.
+glib_subprocess=yes
+if ! $pkg_config --atleast-version=2.38 glib-2.0; then
+    glib_subprocess=no
+fi
+
 ##########################################
 # SHA command probe for modules
 if test "$modules" = yes; then
@@ -4585,6 +4591,9 @@ if test "$bluez" = "yes" ; then
   echo "CONFIG_BLUEZ=y" >> $config_host_mak
   echo "BLUEZ_CFLAGS=$bluez_cflags" >> $config_host_mak
 fi
+if test "glib_subprocess" = "yes" ; then
+  echo "CONFIG_HAS_GLIB_SUBPROCESS_TESTS=y" >> $config_host_mak
+fi
 echo "GLIB_CFLAGS=$glib_cflags" >> $config_host_mak
 if test "$gtk" = "yes" ; then
   echo "CONFIG_GTK=y" >> $config_host_mak
diff --git a/tests/Makefile b/tests/Makefile
index d5db97b..a5e3d0c 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -58,7 +58,7 @@ check-unit-y += tests/test-int128$(EXESUF)
 # all code tested by test-int128 is inside int128.h
 gcov-files-test-int128-y =
 check-unit-y += tests/test-bitops$(EXESUF)
-check-unit-y += tests/test-qdev-global-props$(EXESUF)
+check-unit-$(CONFIG_HAS_GLIB_SUBPROCESS_TESTS) += tests/test-qdev-global-props$(EXESUF)
 check-unit-y += tests/check-qom-interface$(EXESUF)
 gcov-files-check-qom-interface-y = qom/object.c
 check-unit-$(CONFIG_POSIX) += tests/test-vmstate$(EXESUF)

Re: [Qemu-devel] [PULL 00/12] pci, pc, virtio, misc bugfixes

[Paolo Bonzini]

Il 16/09/2014 16:43, Michael S. Tsirkin ha scritto:
>> > 
>> > /root/qemu/tests/test-qdev-global-props.c: In function ‘test_static_prop’:
>> > /root/qemu/tests/test-qdev-global-props.c:80:5: error: implicit
>> > declaration of function ‘g_test_trap_subprocess’
>> > [-Werror=implicit-function-declaration]
>> > /root/qemu/tests/test-qdev-global-props.c:80:5: error: nested extern
>> > declaration of ‘g_test_trap_subprocess’ [-Werror=nested-externs]
>> > 
>> > This function was only added in glib 2.38, and our
>> > minimum version is 2.12.
>> > 
>> > thanks
>> > -- PMM
> The following should help?
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

We can still run a smaller suite of tests if subprocesses are not
available.  That's a better option, and doesn't require build system tests.

Paolo

Re: [Qemu-devel] [PULL 00/12] pci, pc, virtio, misc bugfixes

[Michael S. Tsirkin]

On Tue, Sep 16, 2014 at 04:07:35PM +0200, Paolo Bonzini wrote:
> Il 16/09/2014 16:43, Michael S. Tsirkin ha scritto:
> >> > 
> >> > /root/qemu/tests/test-qdev-global-props.c: In function ‘test_static_prop’:
> >> > /root/qemu/tests/test-qdev-global-props.c:80:5: error: implicit
> >> > declaration of function ‘g_test_trap_subprocess’
> >> > [-Werror=implicit-function-declaration]
> >> > /root/qemu/tests/test-qdev-global-props.c:80:5: error: nested extern
> >> > declaration of ‘g_test_trap_subprocess’ [-Werror=nested-externs]
> >> > 
> >> > This function was only added in glib 2.38, and our
> >> > minimum version is 2.12.
> >> > 
> >> > thanks
> >> > -- PMM
> > The following should help?
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> We can still run a smaller suite of tests if subprocesses are not
> available.  That's a better option, and doesn't require build system tests.
> 
> Paolo

Not sure I understand. Patch?

Re: [Qemu-devel] [PULL 00/12] pci, pc, virtio, misc bugfixes

[Michael Roth]

Quoting Michael S. Tsirkin (2014-09-16 09:52:25)
> On Tue, Sep 16, 2014 at 04:07:35PM +0200, Paolo Bonzini wrote:
> > Il 16/09/2014 16:43, Michael S. Tsirkin ha scritto:
> > >> > 
> > >> > /root/qemu/tests/test-qdev-global-props.c: In function ‘test_static_prop’:
> > >> > /root/qemu/tests/test-qdev-global-props.c:80:5: error: implicit
> > >> > declaration of function ‘g_test_trap_subprocess’
> > >> > [-Werror=implicit-function-declaration]
> > >> > /root/qemu/tests/test-qdev-global-props.c:80:5: error: nested extern
> > >> > declaration of ‘g_test_trap_subprocess’ [-Werror=nested-externs]
> > >> > 
> > >> > This function was only added in glib 2.38, and our
> > >> > minimum version is 2.12.
> > >> > 
> > >> > thanks
> > >> > -- PMM
> > > The following should help?
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > We can still run a smaller suite of tests if subprocesses are not
> > available.  That's a better option, and doesn't require build system tests.
> > 
> > Paolo
> 
> Not sure I understand. Patch?

Maybe wrapping the test cases that depend on subprocesses with:

#if !GLIB_CHECK_VERSION(2, 38, 0)
    g_test_add(...)
#endif

?

If there's still some discussion to be had around the qdev stuff would you
consider sending a v2 without them? I'd like to make sure these are in before
the slirp CVE fix goes upstream so we can cut the 2.1.2 release soon after.