qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH v3 2/2] core/qdev: refactor qdev_


From: Eduardo Habkost
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH v3 2/2] core/qdev: refactor qdev_get_machine() with type assertion
Date: Wed, 17 Apr 2019 14:10:59 -0300
User-agent: Mutt/1.10.1 (2018-07-13)

On Wed, Apr 17, 2019 at 07:14:10AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <address@hidden> 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 <address@hidden>
> >> Signed-off-by: Like Xu <address@hidden>
> >
> > Reviewed-by: Eduardo Habkost <address@hidden>
> >
> > 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?

Once the expectations and use cases are explained, we can choose
a better name for qdev_get_machine_unchecked() and document it
properly.

-- 
Eduardo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]