Re: [Qemu-devel] [PATCH 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet

[Markus Armbruster]

Peter Maydell <address@hidden> writes:

> On 17 October 2013 14:54,  <address@hidden> wrote:
>> From: Markus Armbruster <address@hidden>
>>
>> In an ideal world, machines can be built by wiring devices together
>> with configuration, not code.  Unfortunately, that's not the world we
>> live in right now.  We still have quite a few devices that need to be
>> wired up by code.  If you try to device_add such a device, it'll fail
>> in sometimes mysterious ways.  If you're lucky, you get an
>> unmysterious immediate crash.
>>
>> +    /*
>> +     * Shall we hide this device model from -device / device_add?
>> +     * All devices should support instantiation with device_add, and
>> +     * this flag should not exist.  But we're not there, yet.  Some
>> +     * devices fail to instantiate with cryptic error messages.
>> +     * Others instantiate, but don't work.  Exposing users to such
>> +     * behavior would be cruel; this flag serves to protect them.  It
>> +     * should never be set without a comment explaining why it is set.
>> +     * TODO remove once we're there
>> +     */
>> +    bool cannot_instantiate_with_device_add_yet;
>
> So reading this I'm still not entirely sure what the scope of this
> flag is intended to be. I can think of two possibilities:
>
> (1) the minimal definition: this device would actually crash
> or cause QEMU to break if you created it with device_add
> (2) a larger definition, which includes also devices which
> are completely useless if created with device_add because
> there's no way for the user to wire them up properly.
>
> I think most sysbus devices are going to be in (2) but not (1),
> because they should be fine to create and initialize, but they'll
> just be sitting completely pointlessly totally disconnected from
> the machine model.
>
> Definition (1) is a harder boundary and more straightforward
> to check against, but definition (2) is arguably a bit more useful
> for the end user.

I agree, and I'd like us to adopt definition (2).  I tried to express
this when I wrote "instantiate, but don't work".  Care to suggest
clearer language for this comment?

Regarding (2) being less straightforward to check against: I think we
should try hard to make our cannot_instantiate_with_device_add_yet use
correct (any device we mark that way is actually useless with
device_add), but I view completeness (all the devices that are actually
useless with -device are marked) as not quite that important.