[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] machine: Move acpi_nvdimm_state into struct
From: |
Auger Eric |
Subject: |
Re: [Qemu-devel] [PATCH v2] machine: Move acpi_nvdimm_state into struct MachineState |
Date: |
Thu, 7 Mar 2019 18:56:50 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
Hi Eduardo,
On 3/7/19 6:26 PM, Eduardo Habkost wrote:
> On Thu, Mar 07, 2019 at 10:06:39AM +0100, Eric Auger wrote:
>> As NVDIMM support is looming for ARM and SPAPR, let's
>> move the acpi_nvdimm_state to the generic machine struct
>> instead of duplicating the same code in several machines.
>> It is also renamed into nvdimms_state.
>>
>> nvdimm and nvdimm-persistence become generic machine options.
>> We also add a description for those options.
>>
>> We also remove the nvdimms_state.is_enabled initialization to
>> false as objects are guaranteed to be zero initialized.
>>
>> Signed-off-by: Eric Auger <address@hidden>
>> Suggested-by: Igor Mammedov <address@hidden>
>>
>> ---
>>
>> v1 -> v2:
>> - s/acpi_nvdimm_state/nvdimms_state
>> - remove ms->nvdimms_state.is_enabled initialization to false.
> [...]
>> @@ -765,6 +806,20 @@ static void machine_class_init(ObjectClass *oc, void
>> *data)
>> &error_abort);
>> object_class_property_set_description(oc, "memory-encryption",
>> "Set memory encryption object to use", &error_abort);
>> +
>> + object_class_property_add_bool(oc, "nvdimm",
>> + machine_get_nvdimm, machine_set_nvdimm, &error_abort);
>> + object_class_property_set_description(oc, "nvdimm",
>> + "Set on/off to enable/disable
>> NVDIMM "
>> + "instantiation", NULL);
>> +
>> + object_class_property_add_str(oc, "nvdimm-persistence",
>> + machine_get_nvdimm_persistence,
>> + machine_set_nvdimm_persistence,
>> &error_abort);
>> + object_class_property_set_description(oc, "nvdimm-persistence",
>> + "Set NVDIMM persistence"
>> + "Valid values are cpu and
>> mem-ctrl",
>> + NULL);
>
> As noted in another reply, I don't mind adding new MachineState
> fields, but now I noticed you are adding new user-visible
> options, which requires more care.
>
> This patch seems to make all machines except PC silently ignore
> the new nvdimm options. If the current machine doesn't support
> nvdimm, "nvdimm-persistence=..." and "nvdimm=on" should be
> rejected by QEMU instead of silently ignored.
>
> Probably the simplest way to do that is by making the
> registration of those QOM properties conditional.
>
> We could add a simple
> bool MachineClass::nvdimm_supported
Makes sense to me too. I will go that way.
Thanks
Eric
> field, or we could add a
> static void nvdimm_machine_class_init(MachineClass *mc);
> helper that would enable nvdimm support on the machine type.
>
- Re: [Qemu-devel] [PATCH v2] machine: Move acpi_nvdimm_state into struct MachineState, (continued)
Re: [Qemu-devel] [PATCH v2] machine: Move acpi_nvdimm_state into struct MachineState, Eduardo Habkost, 2019/03/07
- Re: [Qemu-devel] [PATCH v2] machine: Move acpi_nvdimm_state into struct MachineState,
Auger Eric <=