Re: [Qemu-devel] [RFC 13/19] unimplemented-device: Remove user_creatable flag

[Peter Maydell <peter.maydell@linaro.org>]

On 3 April 2017 at 19:30, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Apr 03, 2017 at 03:08:06PM +0100, Peter Maydell wrote:
>> On 3 April 2017 at 14:54, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> > This, on the other hand, currently works:
>> >   $ qemu-system-x86_64 -M q35 -device unimplemented-device,size=1024,name=foo
>>
>> That's a bug in the q35 machine or the handling of -device
>> on non-platform-bus systems, though, isn't it? The default
>> for all sysbus devices has always been "you can't use
>> -device with this", [...]
>
> This was true until 2014, only. commit
> 33cd52b5d7b9adfd009e95f07e6c64dd88ae2a31 changed sys-bus-device
> to have cannot_instantiate_with_device_add_yet=false. See patch
> 03/19 for more detailed info.

That commit shouldn't cause this problem -- unless the
machine sets has_dynamic_sysbus then the machine_init_notify()
that commit adds will cause it to complain.

I think the bug was introduced much later, in bf8d4924 in 2016,
when q35 had the has_dynamic_sysbus flag added but without
any code to restruct this to a whitelist of working
devices. (In fact you can see in that commit a very
minimal attempt to blacklist a few devices.)

The commit message says only intel-iommu was supposed to
be -device creatable, so it should have been the only
thing on the whitelist. Commit 9e3f9733 shows how this
should be done (that's where spapr got has_dynamic_sysbus).

Maybe we should just fix the q35 bug first?

>>               [...] there's never been a requirement to
>> mark them as such separately (because it would require
>> touching the files for a huge number of devices for no
>> particularly good reason when you can default the whole
>> set of devices subclassing SysBusDevice).
>
> Yes, and this is the whole point of this series. At the end of
> this series, only the few devices that are actually usable with
> some machine will have an explicit user_creatable=true
> assignment. See cover letter for details.

...OK, but in that case why not just set that where it should
be set, rather than having a big long patchset that touches
a lot of devices that in the end are right back where
they started? I think a lot of why I'm confused by
this patchset is that it seems like it's changing
behaviour on devices like this one which don't need
any changes...

thanks
-- PMM