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

Re: [libvirt] [PATCH] Support physical memory in virDomainMemoryPeek()



On Wed, Jul 22, 2009 at 4:44 PM, Daniel Veillard<veillard redhat com> wrote:
> On Wed, Jul 22, 2009 at 04:27:45PM +0900, Nguyen Anh Quynh wrote:
>> On Wed, Jul 22, 2009 at 4:19 PM, Nguyen Anh Quynh<aquynh gmail com> wrote:
>> Acutally, to avoid all those ugly sanity checks, it is best to define
>
>  S/ugly/sane/
>
>> VIR_MEMORY_* as an enum type, then redefine virDomainMemoryCheck() as
>> (note the last param is changed):
>>
>> int virDomainMemoryPeek (virDomainPtr dom,
>>                                              unsigned long long start,
>>                                              size_t size,
>>                                              void *buffer,
>>                                              enum virDomainMemoryFlags flags);
>>
>
>  That is ugly, it's also wrong, it break API and ABI compatibility,
> forget about it !
>
>> Let me know your idea about this.
>
>  If more C was implemented with defensive programming, and if people
> didn't broke API every time they think "it would be nicer" then it
> would be way easier to actually develop in C ! Please change your
> mindset that just doesn't work in the long term, sorry ...

Please take this new patch.

Thank you,
Quynh


$ diffstat pmemsave4.diff
 include/libvirt/libvirt.h.in |    1 +
 src/libvirt.c                |   14 ++++++++------
 src/qemu_driver.c            |   15 ++++++++++-----
 3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index ba2b6f0..e6536c7 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -619,6 +619,7 @@ int                     virDomainBlockPeek (virDomainPtr dom,
 /* Memory peeking flags. */
 typedef enum {
   VIR_MEMORY_VIRTUAL              = 1, /* addresses are virtual addresses */
+  VIR_MEMORY_PHYSICAL             = 2, /* addresses are physical addresses */
 } virDomainMemoryFlags;
 
 int                     virDomainMemoryPeek (virDomainPtr dom,
diff --git a/src/libvirt.c b/src/libvirt.c
index 8ee7741..8dc5e17 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3811,19 +3811,20 @@ virDomainMemoryPeek (virDomainPtr dom,
         goto error;
     }
 
-    /* Flags must be VIR_MEMORY_VIRTUAL at the moment.
-     *
-     * Note on access to physical memory: A VIR_MEMORY_PHYSICAL flag is
+    /* Note on access to physical memory: A VIR_MEMORY_PHYSICAL flag is
      * a possibility.  However it isn't really useful unless the caller
      * can also access registers, particularly CR3 on x86 in order to
      * get the Page Table Directory.  Since registers are different on
      * every architecture, that would imply another call to get the
      * machine registers.
      *
-     * The QEMU driver handles only VIR_MEMORY_VIRTUAL, mapping it
+     * The QEMU driver handles VIR_MEMORY_VIRTUAL, mapping it
      * to the qemu 'memsave' command which does the virtual to physical
      * mapping inside qemu.
      *
+     * The QEMU driver also handles VIR_MEMORY_PHYSICAL, mapping it
+     * to the qemu 'pmemsave' command.
+     *
      * At time of writing there is no Xen driver.  However the Xen
      * hypervisor only lets you map physical pages from other domains,
      * and so the Xen driver would have to do the virtual to physical
@@ -3832,9 +3833,10 @@ virDomainMemoryPeek (virDomainPtr dom,
      * which does this, although we cannot copy this code directly
      * because of incompatible licensing.
      */
-    if (flags != VIR_MEMORY_VIRTUAL) {
+
+    if (flags != VIR_MEMORY_VIRTUAL && flags != VIR_MEMORY_PHYSICAL) {
         virLibDomainError (dom, VIR_ERR_INVALID_ARG,
-                           _("flags parameter must be VIR_MEMORY_VIRTUAL"));
+                     _("flags parameter must be VIR_MEMORY_VIRTUAL or VIR_MEMORY_PHYSICAL"));
         goto error;
     }
 
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index d2db1a2..1d29d5f 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -5181,9 +5181,9 @@ qemudDomainMemoryPeek (virDomainPtr dom,
         goto cleanup;
     }
 
-    if (flags != VIR_MEMORY_VIRTUAL) {
+    if (flags != VIR_MEMORY_VIRTUAL && flags != VIR_MEMORY_PHYSICAL) {
         qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_ARG,
-                          "%s", _("QEMU driver only supports virtual memory addrs"));
+                     "%s", _("flags parameter must be VIR_MEMORY_VIRTUAL or VIR_MEMORY_PHYSICAL"));
         goto cleanup;
     }
 
@@ -5200,15 +5200,20 @@ qemudDomainMemoryPeek (virDomainPtr dom,
         goto cleanup;
     }
 
-    /* Issue the memsave command. */
-    snprintf (cmd, sizeof cmd, "memsave %llu %zi \"%s\"", offset, size, tmp);
+    if (flags == VIR_MEMORY_VIRTUAL)
+        /* Issue the memsave command. */
+        snprintf (cmd, sizeof cmd, "memsave %llu %zi \"%s\"", offset, size, tmp);
+    else
+        /* Issue the pmemsave command. */
+        snprintf (cmd, sizeof cmd, "pmemsave %llu %zi \"%s\"", offset, size, tmp);
+
     if (qemudMonitorCommand (vm, cmd, &info) < 0) {
         qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                           "%s", _("'memsave' command failed"));
         goto cleanup;
     }
 
-    DEBUG ("%s: memsave reply: %s", vm->def->name, info);
+    DEBUG ("%s: (p)memsave reply: %s", vm->def->name, info);
 
     /* Read the memory file into buffer. */
     if (saferead (fd, buffer, size) == (ssize_t) -1) {

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