qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PULL v2 01/24] qdev: Replace no_user by cannot_instantiate_with_device_add_yet
Date: Tue, 07 Jan 2014 11:28:26 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux)

"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 :)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]