Re: [Qemu-devel] [PULL v2 01/24] qdev: Replace no_user by cannot_instantiate_with_device_add_yet

["Andreas Färber" <afaerber@suse.de>]

Am 07.01.2014 11:28, schrieb Markus Armbruster:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
>> On Tue, Dec 24, 2013 at 06:04:12PM +0100, Andreas Färber wrote:
>>> From: Markus Armbruster <armbru@redhat.com>
>>>
>>> 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.
>>>
>>> To protect users from such badness, DeviceClass member no_user used to
>>> make device models unavailable with -device / device_add, but that
>>> regressed in commit 18b6dad.  The device model is still omitted from
>>> help, but is available anyway.
>>>
>>> Attempts to fix the regression have been rejected with the argument
>>> that the purpose of no_user isn't clear, and it's prone to misuse.
>>>
>>> This commit clarifies no_user's purpose.  Anthony suggested to rename
>>> it cannot_instantiate_with_device_add_yet_due_to_internal_bugs, which
>>> I shorten somewhat to keep checkpatch happy.  While there, make it
>>> bool.
>>>
>>> Every use of cannot_instantiate_with_device_add_yet gets a FIXME
>>> comment asking for rationale.  The next few commits will clean them
>>> all up, either by providing a rationale, or by getting rid of the use.
>>>
>>> With that done, the regression fix is hopefully acceptable.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>
>> Sorry about not commenting on this earlier.
>> It looks like a bunch of devices will have this flag
>> (whatever we call it) forever.
> 
> Anthony has argued otherwise.
> 
>> If so, _yet seems confusing, should be just
>> cannot_instantiate_with_device_add.
> 
> I don't care.  In fact, I was perfectly happy with 'no_user'.  I renamed
> it (and added additional value) just to make my "protect users from
> device_add that's known not to work" regression palatable for Anthony.
> I recognize the result is an improvement, so Anthony wasn't entirely
> wrong ;)
> 
>> Doesn't have to block this patchset, we can
>> rename it all in one patch easily.
> 
> Thanks :)

Sorry for not explicitly having responded to that yet. In short, I
regarded this patchset as an improvement, not blocking further
improvements on top. It addresses feedback that blocked an earlier,
simpler no_user-checking solution in qdev-monitor.c.

Let's take further discussions off this pull request (that's still
waiting to be processed) and back to where it last sparked!

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg