Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY
On 4/25/19 22:00, Eduardo Habkost wrote: > Eduardo Habkost (4): > machine: Move gpio code to hw/core/gpio.c > move qdev hotplug code to qdev-hotplug.c > qdev: Don't compile hotplug code in user-mode emulation > qdev-hotplug: Don't check type of qdev_get_machine() Patch 1 queued to machine-next.
Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY
Eduardo Habkost writes:
> This series moves some qdev code outside qdev.o, so it can be
> compiled only in CONFIG_SOFTMMU.
>
> The code being moved includes two qdev_get_machine() calls, so
> this will make it easier to move qdev_get_machine() to
> CONFIG_SOFTMMU later.
>
> After this series, there's one remaining qdev_get_machine() call
> that seems more difficult to remove:
>
> static void device_set_realized(Object *obj, bool value, Error **errp)
> {
> /* [...] */
> if (!obj->parent) {
> gchar *name = g_strdup_printf("device[%d]", unattached_count++);
>
> object_property_add_child(container_get(qdev_get_machine(),
> "/unattached"),
> name, obj, _abort);
> unattached_parent = true;
> g_free(name);
> }
> /* [...] */
> }
>
> This one is tricky because on system emulation mode it needs
> "/machine" to already exist, but in user-only mode it needs to
> implicitly create a "/machine" container.
The patches look good to me on a glance. Looking forward to v2.
Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY
On Fri, Apr 26, 2019 at 04:55:17PM +0800, Like Xu wrote:
> On 2019/4/26 4:00, Eduardo Habkost wrote:
> > This series moves some qdev code outside qdev.o, so it can be
> > compiled only in CONFIG_SOFTMMU.
> >
> > The code being moved includes two qdev_get_machine() calls, so
> > this will make it easier to move qdev_get_machine() to
> > CONFIG_SOFTMMU later.
> >
> > After this series, there's one remaining qdev_get_machine() call
> > that seems more difficult to remove:
> >
> > static void device_set_realized(Object *obj, bool value, Error **errp)
> > {
> > /* [...] */
> > if (!obj->parent) {
> > gchar *name = g_strdup_printf("device[%d]",
> > unattached_count++);
> >
> > object_property_add_child(container_get(qdev_get_machine(),
> > "/unattached"),
> >name, obj, _abort);
> > unattached_parent = true;
> > g_free(name);
> > }
> > /* [...] */
> > }
> >
>
> I may have an experimental patch to fix device_set_realized issue:
>
> 1. in qdev_get_machine():
> replace
> dev = container_get(object_get_root(), "/machine");
> with
> dev = object_resolve_path("/machine", NULL);
>
> 2. in device_set_realized():
>
> Using
> Object *container = qdev_get_machine() ?
> qdev_get_machine() : object_get_root();
> and pass it to
> object_property_add_child(
> container_get(container, "/unattached"),
> name, obj, _abort);
I wouldn't like to call qdev_get_machine() here (see below[1]),
but trying "/machine" first and falling back to object_get_root()
sounds good, for two reasons:
* It won't require "/machine" to exist at all, on *-user;
* It will allow machines to create unattached devices on
instance_init without crashing (which is not possible today).
>
> With this fix, we could say the qdev_get_machine() does
> return the "/machine" object (or null) not a confused "/container".
>
> We could continue to use qdev_get_machine() in system emulation mode,
> getting rid of its surprising side effect as Markus said.
>
> The return value of qdev_get_machine() in user-only mode
> is the same object returned by object_get_root(),
> so no semantic changes.
[1] I wouldn't like to have a different qdev_get_machine()
function in user-only mode. I would like to simply not call
qdev_get_machine() at all unless we're in system emulation mode.
--
Eduardo
Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY
On 2019/4/26 4:00, Eduardo Habkost wrote:
This series moves some qdev code outside qdev.o, so it can be
compiled only in CONFIG_SOFTMMU.
The code being moved includes two qdev_get_machine() calls, so
this will make it easier to move qdev_get_machine() to
CONFIG_SOFTMMU later.
After this series, there's one remaining qdev_get_machine() call
that seems more difficult to remove:
static void device_set_realized(Object *obj, bool value, Error **errp)
{
/* [...] */
if (!obj->parent) {
gchar *name = g_strdup_printf("device[%d]", unattached_count++);
object_property_add_child(container_get(qdev_get_machine(),
"/unattached"),
name, obj, _abort);
unattached_parent = true;
g_free(name);
}
/* [...] */
}
I may have an experimental patch to fix device_set_realized issue:
1. in qdev_get_machine():
replace
dev = container_get(object_get_root(), "/machine");
with
dev = object_resolve_path("/machine", NULL);
2. in device_set_realized():
Using
Object *container = qdev_get_machine() ?
qdev_get_machine() : object_get_root();
and pass it to
object_property_add_child(
container_get(container, "/unattached"),
name, obj, _abort);
With this fix, we could say the qdev_get_machine() does
return the "/machine" object (or null) not a confused "/container".
We could continue to use qdev_get_machine() in system emulation mode,
getting rid of its surprising side effect as Markus said.
The return value of qdev_get_machine() in user-only mode
is the same object returned by object_get_root(),
so no semantic changes.
This one is tricky because on system emulation mode it needs
"/machine" to already exist, but in user-only mode it needs to
implicitly create a "/machine" container.
Eduardo Habkost (4):
machine: Move gpio code to hw/core/gpio.c
move qdev hotplug code to qdev-hotplug.c
qdev: Don't compile hotplug code in user-mode emulation
qdev-hotplug: Don't check type of qdev_get_machine()
hw/core/bus.c| 11 --
hw/core/gpio.c | 206
hw/core/qdev-hotplug-stubs.c | 44 +++
hw/core/qdev-hotplug.c | 64 ++
hw/core/qdev.c | 219 ---
hw/core/Makefile.objs| 5 +-
tests/Makefile.include | 3 +-
7 files changed, 320 insertions(+), 232 deletions(-)
create mode 100644 hw/core/gpio.c
create mode 100644 hw/core/qdev-hotplug-stubs.c
create mode 100644 hw/core/qdev-hotplug.c
Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY
Patchew URL: https://patchew.org/QEMU/20190425200051.19906-1-ehabk...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20190425200051.19906-1-ehabk...@redhat.com Subject: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY === 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 === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu t [tag update] patchew/20190424004700.12766-1-richardw.y...@linux.intel.com -> patchew/20190424004700.12766-1-richardw.y...@linux.intel.com * [new tag] patchew/20190425200051.19906-1-ehabk...@redhat.com -> patchew/20190425200051.19906-1-ehabk...@redhat.com Switched to a new branch 'test' c5c77ea2cc qdev-hotplug: Don't check type of qdev_get_machine() 854ded711d qdev: Don't compile hotplug code in user-mode emulation 1bd33307be move qdev hotplug code to qdev-hotplug.c 02b1f5db6f machine: Move gpio code to hw/core/gpio.c === OUTPUT BEGIN === 1/4 Checking commit 02b1f5db6fcd (machine: Move gpio code to hw/core/gpio.c) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #26: new file mode 100644 WARNING: Block comments use a leading /* on a separate line #60: FILE: hw/core/gpio.c:30: +/* NULL is a valid and matchable name, otherwise do a normal WARNING: Block comments use a leading /* on a separate line #156: FILE: hw/core/gpio.c:126: +/* We need a name for object_property_set_link to work. If the ERROR: "foo * bar" should be "foo *bar" #205: FILE: hw/core/gpio.c:175: +void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin) total: 1 errors, 3 warnings, 403 lines checked Patch 1/4 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 2/4 Checking commit 1bd33307be11 (move qdev hotplug code to qdev-hotplug.c) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #49: new file mode 100644 total: 0 errors, 1 warnings, 134 lines checked Patch 2/4 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/4 Checking commit 854ded711daa (qdev: Don't compile hotplug code in user-mode emulation) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #48: new file mode 100644 total: 0 errors, 1 warnings, 77 lines checked Patch 3/4 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 4/4 Checking commit c5c77ea2ccde (qdev-hotplug: Don't check type of qdev_get_machine()) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20190425200051.19906-1-ehabk...@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY
This series moves some qdev code outside qdev.o, so it can be
compiled only in CONFIG_SOFTMMU.
The code being moved includes two qdev_get_machine() calls, so
this will make it easier to move qdev_get_machine() to
CONFIG_SOFTMMU later.
After this series, there's one remaining qdev_get_machine() call
that seems more difficult to remove:
static void device_set_realized(Object *obj, bool value, Error **errp)
{
/* [...] */
if (!obj->parent) {
gchar *name = g_strdup_printf("device[%d]", unattached_count++);
object_property_add_child(container_get(qdev_get_machine(),
"/unattached"),
name, obj, _abort);
unattached_parent = true;
g_free(name);
}
/* [...] */
}
This one is tricky because on system emulation mode it needs
"/machine" to already exist, but in user-only mode it needs to
implicitly create a "/machine" container.
Eduardo Habkost (4):
machine: Move gpio code to hw/core/gpio.c
move qdev hotplug code to qdev-hotplug.c
qdev: Don't compile hotplug code in user-mode emulation
qdev-hotplug: Don't check type of qdev_get_machine()
hw/core/bus.c| 11 --
hw/core/gpio.c | 206
hw/core/qdev-hotplug-stubs.c | 44 +++
hw/core/qdev-hotplug.c | 64 ++
hw/core/qdev.c | 219 ---
hw/core/Makefile.objs| 5 +-
tests/Makefile.include | 3 +-
7 files changed, 320 insertions(+), 232 deletions(-)
create mode 100644 hw/core/gpio.c
create mode 100644 hw/core/qdev-hotplug-stubs.c
create mode 100644 hw/core/qdev-hotplug.c
--
2.18.0.rc1.1.g3f1ff2140
