[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmge
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device |
Date: |
Mon, 20 Mar 2017 16:13:42 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 03/20/17 15:16, Michael S. Tsirkin wrote:
> On Mon, Mar 20, 2017 at 12:59:51PM +0100, Laszlo Ersek wrote:
>> Multiple instances make no sense.
>>
>> Cc: "Michael S. Tsirkin" <address@hidden>
>> Cc: Ben Warren <address@hidden>
>> Cc: Igor Mammedov <address@hidden>
>> Signed-off-by: Laszlo Ersek <address@hidden>
>
> find_vmgenid_dev would be a better place for this.
> This is where the single instance assumption comes from ATM.
object_resolve_path_type() -- used internally in find_vmgenid_dev() --
returns NULL in either of two cases: there is no such device, or there
are multiple devices. You can tell them apart by looking at the last
parameter (called "ambiguous"), but find_vmgenid_dev() doesn't use that
parameter.
By the time we are in the vmgenid_realize() function, at least one
vmgenid device is guaranteed to exist (the one which we are realizing).
Therefore, this patch could be simplified as:
if (find_vmgenid_dev() == NULL) {
error_setg(errp, "at most one %s device is permitted", VMGENID_DEVICE);
return;
}
I found that confusing, and wanted to spell out "ambiguous" with the
assert(). If you prefer the above simpler (but harder to understand)
check, I can do that too.
Thanks
Laszlo
>
>
>> ---
>> hw/acpi/vmgenid.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
>> index c3ddcc8e7cb0..b5c0dfcf19e1 100644
>> --- a/hw/acpi/vmgenid.c
>> +++ b/hw/acpi/vmgenid.c
>> @@ -214,6 +214,8 @@ static Property vmgenid_properties[] = {
>> static void vmgenid_realize(DeviceState *dev, Error **errp)
>> {
>> VmGenIdState *vms = VMGENID(dev);
>> + Object *one_vmgenid;
>> + bool ambiguous;
>>
>> if (!vms->write_pointer_available) {
>> error_setg(errp, "%s requires DMA write support in fw_cfg, "
>> @@ -221,6 +223,14 @@ static void vmgenid_realize(DeviceState *dev, Error
>> **errp)
>> return;
>> }
>>
>> + one_vmgenid = object_resolve_path_type("", VMGENID_DEVICE, &ambiguous);
>> + if (one_vmgenid == NULL) {
>> + assert(ambiguous);
>> + error_setg(errp, "at most one %s device is permitted",
>> VMGENID_DEVICE);
>> + return;
>> + }
>> + assert(one_vmgenid == OBJECT(vms));
>> +
>> qemu_register_reset(vmgenid_handle_reset, vms);
>> }
>>
>> --
>> 2.9.3
[Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Laszlo Ersek, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Michael S. Tsirkin, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device,
Laszlo Ersek <=
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Laszlo Ersek, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Michael S. Tsirkin, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Laszlo Ersek, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Michael S. Tsirkin, 2017/03/20
- Re: [Qemu-devel] [PATCH 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device, Laszlo Ersek, 2017/03/20