Commit 510ef98dca made qdev_realize() support bus-less devices,
asserting either the device is bus-less or the device is created
on a bus. Commit 464a22c757 used qdev_realize() instead of
object_property_set_bool(). Since qdev_realize() now checks for
a bus, it is not possible to create hotplug devices unattached
to any bus anymore.

Fix by only asserting if the device is not hotpluggable.

Fixes: 464a22c757 "qdev: Use qdev_realize() in qdev_device_add()"
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/core/qdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 01796823b4..6c5540ecdc 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -393,7 +393,9 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp)
     if (bus) {
         qdev_set_parent_bus(dev, bus);
     } else {
-        assert(!DEVICE_GET_CLASS(dev)->bus_type);
+        DeviceClass *dc = DEVICE_GET_CLASS(dev);
+
+        assert(dc->hotpluggable || !dc->bus_type);
     }
 
     return object_property_set_bool(OBJECT(dev), "realized", true, errp);
-- 
2.21.3

Patchew URL: https://patchew.org/QEMU/20200719134329.21613-1-f4bug@amsat.org/



Hi,

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

Type: series
Message-id: 20200719134329.21613-1-f4bug@amsat.org
Subject: [PATCH-for-5.1] qdev: Allow to create hotplug device before plugging it to a bus

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

fatal: unable to write new index file
warning: Clone succeeded, but checkout failed.
You can inspect what was checked out with 'git status'
and retry the checkout with 'git checkout -f HEAD'

Traceback (most recent call last):
  File "patchew-tester2/src/patchew-cli", line 531, in test_one
    git_clone_repo(clone, r["repo"], r["head"], logf, True)
  File "patchew-tester2/src/patchew-cli", line 62, in git_clone_repo
    subprocess.check_call(clone_cmd, stderr=logf, stdout=logf)
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'clone', '-q', '/home/patchew2/.cache/patchew-git-cache/httpsgithubcompatchewprojectqemu-3c8cf5a9c21ff8782164d1def7f44bd888713384', '/var/tmp/patchew-tester-tmp-3_pdffy_/src']' returned non-zero exit status 128.



The full log is available at
http://patchew.org/logs/20200719134329.21613-1-f4bug@amsat.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Patchew URL: https://patchew.org/QEMU/20200719134329.21613-1-f4bug@amsat.org/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20200719134329.21613-1-f4bug@amsat.org/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Commit 510ef98dca made qdev_realize() support bus-less devices,
> asserting either the device is bus-less or the device is created
> on a bus. Commit 464a22c757 used qdev_realize() instead of
> object_property_set_bool(). Since qdev_realize() now checks for
> a bus, it is not possible to create hotplug devices unattached
> to any bus anymore.
>
> Fix by only asserting if the device is not hotpluggable.
>
> Fixes: 464a22c757 "qdev: Use qdev_realize() in qdev_device_add()"
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/core/qdev.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 01796823b4..6c5540ecdc 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -393,7 +393,9 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp)
>      if (bus) {
>          qdev_set_parent_bus(dev, bus);
>      } else {
> -        assert(!DEVICE_GET_CLASS(dev)->bus_type);
> +        DeviceClass *dc = DEVICE_GET_CLASS(dev);
> +
> +        assert(dc->hotpluggable || !dc->bus_type);
>      }
>  
>      return object_property_set_bool(OBJECT(dev), "realized", true, errp);

I think this is wrong.

Invariant about realized devices and their bus:

* realized "bus-full" devices are plugged into an appropriate bus, and

* realized bus-less devices are not plugged into any bus.

Since qdev_realize() goes from unrealized to realized, it needs to
establish the invariant, regardless of dc->hotpluggable.

I suspect the bug is in the caller.  Give me a concrete reproducer, and
I'll look.