On Tue, 2 Apr 2019 21:09:39 +0800 Like Xu <[email protected]> wrote:
> On 2019/4/2 19:27, Markus Armbruster wrote: > > Like Xu <[email protected]> writes: > > > >> This patch makes the remaining dozen or so uses of the global > >> current_machine outside vl.c use qdev_get_machine() instead, > >> and then make current_machine local to vl.c instead of global. > >> > >> Signed-off-by: Like Xu <[email protected]> > > > > You effectively replace > > > > current_machine > > > > by > > > > MACHINE(qdev_get_machine()) > > > > qdev_get_machine() uses container_get(), which has a side effect: any > > path component that doesn't exist already gets created as "container" > > object. In case of qdev_get_machine(), that's just "/machine". > > > > Creating "/machine" as "container" is of course wrong. You therefore > > must not use qdev_get_machine() before main() creates "/machine". It > > does like this: > > > > object_property_add_child(object_get_root(), "machine", > > OBJECT(current_machine), &error_abort); > > > > I recently had several cases of code rearrangements explode because the > > reordered code called qdev_get_machine() too early. Makes me rather > > skeptical about this patch. To be frank, I consider qdev_get_machine() > > a trap for the unwary. container_get(), too. > > > > If we decide using it to make current_machine static a good idea anyway, > > we need to check the new uses carefully to make sure they can't run > > before main() creates "/machine". maybe we can assert in qdev_get_machine() if machine hasn't been created yet? with this at least it will be hard to misuse function or catch invalid users. (but it still might miss some use cases/CLI options which are not tested) > > > > Thanks for your comments and this issue may not exist in this patch. > > I am curious when and where we would call qdev_get_machine() before > main() initializes current_machine and adds it to QOM store which is > called very early.
