On Thu, Jun 05, 2008 at 10:30:55PM +0100, Richard W.M. Jones wrote:
+/* Memory peeking flags. */
+typedef enum {
+ VIR_MEMORY_VIRTUAL = 1, /* addresses are virtual addresses */
+} virDomainMemoryFlags;
Since there is only one flag, and it is compulsory I'm rather inclined
to say that virtual memory addressing should be the default with a flags
value of 0. Unless there is another mode, not yet implemented, that you
think would be a better default in the future ? Obviously keep the flags
arg for expansion regardless though.
diff -u -p -r1.36 remote.c
--- qemud/remote.c 5 Jun 2008 21:12:26 -0000 1.36
+++ qemud/remote.c 5 Jun 2008 21:26:29 -0000
@@ -938,6 +938,52 @@ remoteDispatchDomainBlockPeek (struct qe
}
static int
+remoteDispatchDomainMemoryPeek (struct qemud_server *server ATTRIBUTE_UNUSED,
+ struct qemud_client *client,
+ remote_message_header *req,
+ remote_domain_memory_peek_args *args,
+ remote_domain_memory_peek_ret *ret)
+{
+ virDomainPtr dom;
+ unsigned long long offset;
+ size_t size;
+ unsigned int flags;
+ CHECK_CONN (client);
+
+ dom = get_nonnull_domain (client->conn, args->dom);
+ if (dom == NULL) {
+ remoteDispatchError (client, req, "%s", _("domain not
found"));
+ return -2;
+ }
+ offset = args->offset;
+ size = args->size;
+ flags = args->flags;
+
+ if (size > REMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX) {
+ remoteDispatchError (client, req,
+ "%s", _("size > maximum buffer
size"));
+ return -2;
+ }
+
+ ret->buffer.buffer_len = size;
+ ret->buffer.buffer_val = malloc (size);
+ if (!ret->buffer.buffer_val) {
+ remoteDispatchError (client, req, "%s", strerror (errno));
+ return -2;
+ }
The 'dom' object is leaking in the 2 error paths here & it'd be better
to use VIR_ALLOC_N(ret->buffer.buffer_val, size) here.
diff -u -p -r1.14 remote_protocol.x
--- qemud/remote_protocol.x 5 Jun 2008 21:12:27 -0000 1.14
+++ qemud/remote_protocol.x 5 Jun 2008 21:26:29 -0000
@@ -340,6 +340,17 @@ struct remote_domain_block_peek_ret {
opaque buffer<REMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX>;
};
+struct remote_domain_memory_peek_args {
+ remote_nonnull_domain dom;
+ unsigned hyper offset;
+ unsigned size;
+ unsigned flags;
+};
+
+struct remote_domain_memory_peek_ret {
+ opaque buffer<REMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX>;
+};
Is it worth having a separate constant for the max memory size, independant
of the block peek size.
+/**
+ * virDomainMemoryPeek:
+ * @dom: pointer to the domain object
+ * @start: start of memory to peek
+ * @size: size of memory to peek
+ * @buffer: return buffer (must be at least size bytes)
+ * @flags: flags, see below
+ *
+ * This function allows you to read the contents of a domain's
+ * memory.
+ *
+ * The memory which is read is controlled by the 'start', 'size'
+ * and 'flags' parameters.
+ *
+ * If 'flags' is VIR_MEMORY_VIRTUAL then the 'start' and 'size'
+ * parameters are interpreted as virtual memory addresses for
+ * whichever task happens to be running on the domain at the
+ * moment. Although this sounds haphazard it is in fact what
+ * you want in order to read Linux kernel state, because it
+ * ensures that pointers in the kernel image can be interpreted
+ * coherently.
+ *
+ * 'buffer' is the return buffer and must be at least 'size' bytes.
+ * 'size' may be 0 to test if the call would succeed.
Should we document and enforce a maximum value for size. The remote
driver at least has a maximum limit,, so perhaps we should enforce
that in the main API dispatcher here in virDomainMemoryPeek() impl
+int
+static int
+qemudDomainMemoryPeek (virDomainPtr dom,
+ unsigned long long offset, size_t size,
+ void *buffer,
+ unsigned int flags)
+{
+ struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
+ struct qemud_vm *vm = qemudFindVMByID (driver, dom->id);
+ char cmd[256], *info;
+ char tmp[] = "/tmp/qemumemXXXXXX";
It'd make the SELinux policy easier if we avoided the generic /tmp and put
the files in somewhere like /var/spool/libvirt/qemu/. Wouldn't even need to
use mkstemp() then - just have it named VMNAME.mem since it'd be in a safe
controlled directory
Regards,
Daniel.
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|