On Wed, Jul 22, 2009 at 4:44 PM, Daniel Veillard<veillard@xxxxxxxxxx> 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@xxxxxxxxx> 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) {
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list