[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PULL v2 01/24] qdev: Replace no_user by can
From: |
Andreas Färber |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PULL v2 01/24] qdev: Replace no_user by cannot_instantiate_with_device_add_yet |
Date: |
Tue, 07 Jan 2014 12:56:35 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 |
Am 07.01.2014 11:28, schrieb Markus Armbruster:
> "Michael S. Tsirkin" <address@hidden> writes:
>
>> On Tue, Dec 24, 2013 at 06:04:12PM +0100, Andreas Färber 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.
>>>
>>> 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 <address@hidden>
>>> Reviewed-by: Marcel Apfelbaum <address@hidden>
>>> Signed-off-by: Andreas Färber <address@hidden>
>>
>> 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