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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: pgp73dnYvRgtn.pgp
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]