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

[Martin Kletzander]

On Fri, Feb 20, 2015 at 07:36:53PM +0100, Martin Kletzander wrote:
>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.
>

Well, from the vast majority of replies, I think there is not that
much of disagreement.  Although if there was a thread where this was
decided and I missed that, feel free to leave it as-is.

>>>
>>> 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.
>

And since this is dealt with, ACK to this patch with the
aforementioned decision left to rest upon your shoulders.

>Thanks for the clarification,
>Martin



>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150224/1989c15b/attachment-0001.sig>