Re: [libvirt] [PATCH 04/11] conf: Add device address type for dimm devices

On Fri, Feb 20, 2015 at 06:25:40PM +0100, Peter Krempa wrote: >On Fri, Feb 20, 2015 at 10:19:53 +0100, Martin Kletzander wrote: >>On Thu, Feb 19, 2015 at 04:38:29PM +0100, Peter Krempa wrote: >>>ACPI Dimm devices are described by the slot and base address. Add a new >>>address type to be able to describe such address. >>>--- >>> docs/schemas/domaincommon.rng | 18 +++++++++++ >>> src/conf/domain_conf.c | 74 ++++++++++++++++++++++++++++++++++++++++++- >>> src/conf/domain_conf.h | 9 ++++++ >>> 3 files changed, 100 insertions(+), 1 deletion(-) >>> >>>diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng >>>index acfa16a..1824741 100644 >>>--- a/docs/schemas/domaincommon.rng >>>+++ b/docs/schemas/domaincommon.rng >>>@@ -3993,6 +3993,18 @@ >>> </attribute> >>> </optional> >>> </define> >>>+ <define name="acpidimmaddress"> >>>+ <optional> >>>+ <attribute name="slot"> >>>+ <ref name="unsignedInt"/> >>>+ </attribute> >>>+ </optional> >>>+ <optional> >>>+ <attribute name="base"> >>>+ <ref name="hexuint"/> >>>+ </attribute> >>>+ </optional> >>>+ </define> >>> <define name="devices"> >>> <element name="devices"> >>> <interleave> >>>@@ -4407,6 +4419,12 @@ >>> </attribute> >>> <ref name="isaaddress"/> >>> </group> >>>+ <group> >>>+ <attribute name="type"> >>>+ <value>acpi-dimm</value> >>>+ </attribute> >>>+ <ref name="acpidimmaddress"/> >>>+ </group> >>> </choice> >>> </element> >>> </define> >> >>I've got 2 questions here: >> >> 1) Why not just "dimm"? I feel like the "acpi" complicates >> everything. > >That is okay if upstream agrees. > Just a swift idea, not that it's needed. I'd wonder about others' opinions.

>> >> 2) It looks like we won't do any address validation or allocation, is >> that planned?. I hope this won't end up like other address types >> where we just wait for qemu to fail. Also, if base[n+1] is just >> base[n]+size[n], then there should be no problem assigning proper >> addresses automatically. I think it'd be much less pain to >> automatically assign them in libvirt then making it mandatory for >> the management application.a > >As I've explained a few times already. The management apps ideally >shouldn't pass anything in the address and the data are then recalled >from qemu. I want to avoid by all means doing the magic alignment done >by qemu here since we can recall the data after the module is used. > >The only reason the address is required is to allow migrations without >moving the modules around. This is the main reason this is stashed under >the address field and users shouldn't need to set it ... ever. > That's even better than I meant. Maybe we'll have the same possibility for other devices, too. That would deal with some of our current problems.

Thanks for the clarification, Martin

-- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list