[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instan
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet |
Date: |
Wed, 30 Oct 2013 13:21:21 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Marcel Apfelbaum <address@hidden> writes:
> On Tue, 2013-10-29 at 17:08 +0100, 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.
>>
>> 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>
[...]
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index e191ca0..2b571d7 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -97,7 +97,18 @@ typedef struct DeviceClass {
>> const char *fw_name;
>> const char *desc;
>> Property *props;
>> - int no_user;
>> +
>> + /*
>> + * 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;
>>
>> /* callbacks */
>> void (*reset)(DeviceState *dev);
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index a02c925..36f6f09 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -87,7 +87,7 @@ static void qdev_print_devinfo(DeviceClass *dc)
>> if (dc->desc) {
>> error_printf(", desc \"%s\"", dc->desc);
>> }
>> - if (dc->no_user) {
>> + if (dc->cannot_instantiate_with_device_add_yet) {
>> error_printf(", no-user");
> Maybe also the message can be changed here?
I'd rather not change output of "info qdm" without good reason.
>> }
>> error_printf("\n");
>> @@ -127,7 +127,8 @@ static void qdev_print_devinfos(bool show_no_user)
> Same question about show_no_user parameter, maybe give it a "better"
> name?
My excuse for keeping show_no_user is it controls whether "no-user" is
printed.
Regardless, I'm willing to rename if folks think it's useful.
> Seems OK to me.
>
> Reviewed-by: Marcel Apfelbaum <address@hidden>
Thanks!
[...]
- [Qemu-devel] [PATCH v2 02/10] sysbus: Set cannot_instantiate_with_device_add_yet, (continued)
[Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet, armbru, 2013/10/29
[Qemu-devel] [PATCH v2 09/10] isa: Clean up use of cannot_instantiate_with_device_add_yet, armbru, 2013/10/29
[Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet, armbru, 2013/10/29
[Qemu-devel] [PATCH v2 07/10] piix3 piix4: Clean up use of cannot_instantiate_with_device_add_yet, armbru, 2013/10/29