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

[Martin Kletzander]

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
-------------- 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/20150220/d87131d2/attachment-0001.sig>