On 2019/4/18 1:10, Eduardo Habkost wrote:
On Wed, Apr 17, 2019 at 07:14:10AM +0200, Markus Armbruster wrote:
Eduardo Habkost <ehabk...@redhat.com> writes:
On Mon, Apr 15, 2019 at 03:59:45PM +0800, Like Xu wrote:
To avoid the misuse of qdev_get_machine() if machine hasn't been created yet,
this patch uses qdev_get_machine_uncheck() for obj-common (share with user-only
mode) and adds type assertion to qdev_get_machine() in system-emulation mode.
Suggested-by: Igor Mammedov <imamm...@redhat.com>
Signed-off-by: Like Xu <like...@linux.intel.com>
Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
I'm queueing the series on machine-next, thanks!
Hold your horses, please.
I dislike the name qdev_get_machine_uncheck(). I could live with
qdev_get_machine_unchecked().
However, I doubt this is the right approach.
The issue at hand is undisciplined creation of QOM object /machine.
This patch adds an asseertion "undisciplined creation of /machine didn't
create crap", but only in some places.
I think we should never create /machine as (surprising!) side effect of
qdev_get_machine(). Create it explicitly instead, and have
qdev_get_machine() use object_resolve_path("/machine", NULL) to get it.
Look ma, no side effects.
OK, I'm dropping this one while we discuss it.
I really miss a good explanation why qdev_get_machine_unchecked()
needs to exist. When exactly do we want /machine to exist but
not be TYPE_MACHINE? Why?
AFAICT, there is no such "/machine" that is not of type TYPE_MACHINE.
The original qdev_get_machine() would always return a "/container"
object in user-only mode and there is none TYPE_MACHINE object.
In system emulation mode, it returns the same "/container" object at the
beginning, until we initialize and add a TYPE_MACHINE object to the
"/container" as a child and it would return OBJECT(current_machine)
for later usages.
The starting point is to avoid using the legacy qdev_get_machine()
in system emulation mode when we haven't added the "/machine" object.
As a result, we introduced type checking assertions to avoid premature
invocations.
In this proposal, the qdev_get_machine_unchecked() is only used
in user-only mode, part of which shares with system emulation mode
(such as device_set_realized, cpu_common_realizefn). The new
qdev_get_machine() is only used in system emulation mode and type
checking assertion does reduce the irrational use of this function (if
any in the future).
We all agree to use this qdev_get_machine() as little as possible
and this patch could make future clean up work easier.
Once the expectations and use cases are explained, we can choose
a better name for qdev_get_machine_unchecked() and document it
properly.
