[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 4/8] ACPI: Add Virtual Machine Generation ID
From: |
Ben Warren |
Subject: |
Re: [Qemu-devel] [PATCH v7 4/8] ACPI: Add Virtual Machine Generation ID support |
Date: |
Thu, 16 Feb 2017 11:05:33 -0800 |
> On Feb 16, 2017, at 11:03 AM, Laszlo Ersek <address@hidden> wrote:
>
> On 02/16/17 19:32, Ben Warren wrote:
>>
>>> On Feb 16, 2017, at 1:56 AM, Igor Mammedov <address@hidden> wrote:
>>>
>>> On Wed, 15 Feb 2017 22:18:14 -0800
>>> address@hidden wrote:
>>>
>>>> From: Ben Warren <address@hidden>
>>>>
>>>> This implements the VM Generation ID feature by passing a 128-bit
>>>> GUID to the guest via a fw_cfg blob.
>>>> Any time the GUID changes, an ACPI notify event is sent to the guest
>>>>
>>>> The user interface is a simple device with one parameter:
>>>> - guid (string, must be "auto" or in UUID format
>>>> xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx)
>>>>
>>>> Signed-off-by: Ben Warren <address@hidden>
>>> Reviewed-by: Igor Mammedov <address@hidden>
>>>
>>> Thing to get fixed on top is to check
>>> that there isn't vmgenid device present already
>>> into realizefn() and fail gracefully is there is.
>>>
>> I made the change as follows:
>> ===
>>
>> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
>> index c8465df..fa3de6d 100644
>> --- a/hw/acpi/vmgenid.c
>> +++ b/hw/acpi/vmgenid.c
>> @@ -207,8 +207,15 @@ static void vmgenid_handle_reset(void *opaque)
>>
>> static void vmgenid_realize(DeviceState *dev, Error **errp)
>> {
>> + static bool is_realized = false;
>> VmGenIdState *vms = VMGENID(dev);
>> +
>> + if (is_realized) {
>> + error_setg(errp, "Device %s may only be specified once",
>> + VMGENID_DEVICE);
>> + }
>> qemu_register_reset(vmgenid_handle_reset, vms);
>> + is_realized = true;
>> }
>>
>> ===
>> Sample output:
>> $ ../workspace/qemu/x86_64-softmmu/qemu-system-x86_64 -nographic -bios
>> ../workspace/seabios/out/bios.bin -machine type=q35 -hda
>> beiwe-with-vmgenid-driver.qcow2 -qmp unix:./qmp-sock,server,nowait -device
>> vmgenid,guid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb66" -device
>> vmgenid,guid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb67"
>> qemu-system-x86_64: -device
>> vmgenid,guid=324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb67: Device vmgenid may only
>> be specified once
>>
>>
>> Good enough? If so, I’ll roll it in. Otherwise, it’ll be a supplemental
>> patch.
>
> Ehm, I'd say let's postpone that patch. Function (or even file) scope
> static variables that plaster over QOM shortcomings (or, well,
> "non-trivial QOM patterns") are strongly frowned upon. I've done it
> myself before (totally as a last resort, really -- not kidding!), and it
> was real hard to get into the tree.
>
> This deserves more discussion, so please delay it.
>
> (While at it, please remember my other recommendation for realize():
> enforcing that the running machine type / version provides the guest
> with the DMA interface for fw_cfg. For that, based on prior consensus,
> you'll need another device property, to be set in parallel to
> fw_cfg_mem's and fw_cfg_io's "dma-enabled" property. So I think it makes
> sense to address all of these things in a separate, followup series.)
>
OK, makes sense.
> Thanks!
> Laszlo
—Ben
smime.p7s
Description: S/MIME cryptographic signature
[Qemu-devel] [PATCH v7 5/8] qmp/hmp: add query-vm-generation-id and 'info vm-generation-id' commands, ben, 2017/02/16
[Qemu-devel] [PATCH v7 6/8] tests: Move reusable ACPI code into a utility file, ben, 2017/02/16
[Qemu-devel] [PATCH v7 7/8] tests: Add unit tests for the VM Generation ID feature, ben, 2017/02/16
[Qemu-devel] [PATCH v7 8/8] MAINTAINERS: Add VM Generation ID entry, ben, 2017/02/16
Re: [Qemu-devel] [PATCH v7 0/8] Add support for VM Generation ID, Igor Mammedov, 2017/02/16