Re: [Qemu-devel] [PATCH v6 4/7] vmport_rpc: Add QMP access to vmport_rpc object.
On 05/12/2015 01:03 PM, Don Slutz wrote:
This adds one new inject command:
inject-vmport-action
And three guest info commands:
vmport-guestinfo-set
vmport-guestinfo-get
query-vmport-guestinfo
More details in qmp-commands.hx
Signed-off-by: Don Slutz dsl...@verizon.com
---
+/*
+ * Run func() for every VMPortRpc device, traverse the tree for
+ * everything else. Note: This routine expects that opaque is a
+ * VMPortRpcFind pointer and not NULL.
+ */
+static int find_VMPortRpc_device(Object *obj, void *opaque)
+{
+VMPortRpcFind *find = opaque;
+Object *dev;
+VMPortRpcState *s;
+
+if (find-found) {
Why not assert(find) instead of leaving it to the comment?
+/*
+ * Loop through all dynamically created VMPortRpc devices and call
+ * func() for each instance.
+ */
+static int foreach_dynamic_vmport_rpc_device(FindVMPortRpcDeviceFunc *func,
+ void *arg)
+{
+VMPortRpcFind find = {
+.func = func,
+.arg = arg,
Is it worth marking arg const here and in the VMPortRpcFind struct...
+void qmp_inject_vmport_action(enum VmportAction action, Error **errp)
+{
+int rc;
+
+switch (action) {
+case VMPORT_ACTION_REBOOT:
+rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send,
+ (void *)OS_Reboot);
...so that you don't have to cast away const here?
+break;
+case VMPORT_ACTION_HALT:
+rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send,
+ (void *)OS_Halt);
+break;
+case VMPORT_ACTION_MAX:
+assert(action != VMPORT_ACTION_MAX);
+rc = 0; /* Should be impossible to get here. */
I'd rather abort() if someone compiled with -NDEBUG.
+break;
+}
+convert_local_rc(errp, rc);
+}
+
+typedef struct keyValue {
+void *key_data;
+void *value_data;
+unsigned int key_len;
+unsigned int value_len;
Should these be size_t?
+void qmp_vmport_guestinfo_set(const char *key, const char *value,
+ bool has_format, enum DataFormat format,
+ Error **errp)
+{
+int rc;
+keyValue key_value;
+
+if (strncmp(key, guestinfo., strlen(guestinfo.)) == 0) {
+key_value.key_data = (void *)(key + strlen(guestinfo.));
+key_value.key_len = strlen(key) - strlen(guestinfo.);
+} else {
+key_value.key_data = (void *)key;
Casting to (void*) looks awkward; should key_data should be typed 'const
void *' to avoid the need for a cast? For that matter, why is it void*,
why not 'const char *'?
+++ b/qmp-commands.hx
@@ -490,6 +490,126 @@ Note: inject-nmi fails when the guest doesn't support
injecting.
EQMP
+SQMP
+vmport-guestinfo-set
+--
Still mismatches on line length (several sites).
+- { execute: vmport-guestinfo-get,
+arguments: { key: guestinfo.foo,
+ format: utf8 } }
+- {return: {value: abcdefgh}}
+
+
+EQMP
+
+{
+.name = query-vmport-guestinfo,
+.args_type = ,
+.mhandler.cmd_new = qmp_marshal_input_query_vmport_guestinfo,
+},
+
+SQMP
+query-vmport-guestinfo
+--
+
+Returns information about VMWare Tools guestinfo. The returned value is a
json-array
+of all keys.
+
+Example:
+
+- { execute: query-vmport-guestinfo }
+- {
+ return: [
+ {
+key: guestinfo.ip
+ },
So if I'm following this correctly, a user has to call
query-vmport-guestinfo to learn the key names, and then once per key
call vmport-guetsinfo-get to learn the key values. Why not just return
key names and values all at once, to save the multiple call overhead?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v6 4/7] vmport_rpc: Add QMP access to vmport_rpc object.
This adds one new inject command:
inject-vmport-action
And three guest info commands:
vmport-guestinfo-set
vmport-guestinfo-get
query-vmport-guestinfo
More details in qmp-commands.hx
Signed-off-by: Don Slutz dsl...@verizon.com
---
hw/misc/vmport_rpc.c | 268 +++
monitor.c| 24 +
qapi-schema.json | 101 +++
qmp-commands.hx | 120 +++
4 files changed, 513 insertions(+)
diff --git a/hw/misc/vmport_rpc.c b/hw/misc/vmport_rpc.c
index 927c0bf..9a32c6f 100644
--- a/hw/misc/vmport_rpc.c
+++ b/hw/misc/vmport_rpc.c
@@ -215,6 +215,56 @@ typedef struct {
uint32_t edi;
} vregs;
+/*
+ * Run func() for every VMPortRpc device, traverse the tree for
+ * everything else. Note: This routine expects that opaque is a
+ * VMPortRpcFind pointer and not NULL.
+ */
+static int find_VMPortRpc_device(Object *obj, void *opaque)
+{
+VMPortRpcFind *find = opaque;
+Object *dev;
+VMPortRpcState *s;
+
+if (find-found) {
+return 0;
+}
+dev = object_dynamic_cast(obj, TYPE_VMPORT_RPC);
+s = (VMPortRpcState *)dev;
+
+if (!s) {
+/* Container, traverse it for children */
+return object_child_foreach(obj, find_VMPortRpc_device, opaque);
+}
+
+find-found++;
+find-rc = find-func(s, find-arg);
+
+return 0;
+}
+
+/*
+ * Loop through all dynamically created VMPortRpc devices and call
+ * func() for each instance.
+ */
+static int foreach_dynamic_vmport_rpc_device(FindVMPortRpcDeviceFunc *func,
+ void *arg)
+{
+VMPortRpcFind find = {
+.func = func,
+.arg = arg,
+};
+
+/* Loop through all VMPortRpc devices that were spawned outside
+ * the machine */
+find_VMPortRpc_device(qdev_get_machine(), find);
+if (find.found) {
+return find.rc;
+} else {
+return VMPORT_DEVICE_NOT_FOUND;
+}
+}
+
#ifdef VMPORT_RPC_DEBUG
/*
* Add helper function for tracing. This routine will convert
@@ -464,6 +514,23 @@ static int get_guestinfo(VMPortRpcState *s,
return GUESTINFO_NOTFOUND;
}
+static int get_qmp_guestinfo(VMPortRpcState *s,
+ unsigned int a_key_len, char *a_info_key,
+ unsigned int *a_value_len, void **a_value_data)
+{
+gpointer key = g_strndup(a_info_key, a_key_len);
+guestinfo_t *gi = (guestinfo_t *)g_hash_table_lookup(s-guestinfo, key);
+
+g_free(key);
+if (gi) {
+*a_value_len = gi-val_len;
+*a_value_data = gi-val_data;
+return 0;
+}
+
+return GUESTINFO_NOTFOUND;
+}
+
static int set_guestinfo(VMPortRpcState *s, int a_key_len,
unsigned int a_val_len, char *a_info_key, char *val)
{
@@ -851,6 +918,207 @@ static uint32_t vmport_rpc_ioport_read(void *opaque,
uint32_t addr)
return ur.data[0];
}
+static int vmport_rpc_find_send(VMPortRpcState *s, void *arg)
+{
+return vmport_rpc_ctrl_send(s, arg);
+}
+
+static void convert_local_rc(Error **errp, int rc)
+{
+switch (rc) {
+case 0:
+break;
+case VMPORT_DEVICE_NOT_FOUND:
+error_set(errp, QERR_DEVICE_NOT_FOUND, TYPE_VMPORT_RPC);
+break;
+case SEND_NOT_OPEN:
+error_setg(errp, VMWare rpc not open);
+break;
+case SEND_SKIPPED:
+error_setg(errp, VMWare rpc send skipped);
+break;
+case SEND_TRUCATED:
+error_setg(errp, VMWare rpc send trucated);
+break;
+case SEND_NO_MEMORY:
+error_setg(errp, VMWare rpc send out of memory);
+break;
+case GUESTINFO_NOTFOUND:
+error_setg(errp, VMWare guestinfo not found);
+break;
+case GUESTINFO_VALTOOLONG:
+error_setg(errp, VMWare guestinfo value too long);
+break;
+case GUESTINFO_KEYTOOLONG:
+error_setg(errp, VMWare guestinfo key too long);
+break;
+case GUESTINFO_TOOMANYKEYS:
+error_setg(errp, VMWare guestinfo too many keys);
+break;
+case GUESTINFO_NO_MEMORY:
+error_setg(errp, VMWare guestinfo out of memory);
+break;
+default:
+error_setg(errp, VMWare rpc send rc=%d unknown, rc);
+break;
+}
+}
+
+void qmp_inject_vmport_action(enum VmportAction action, Error **errp)
+{
+int rc;
+
+switch (action) {
+case VMPORT_ACTION_REBOOT:
+rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send,
+ (void *)OS_Reboot);
+break;
+case VMPORT_ACTION_HALT:
+rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send,
+ (void *)OS_Halt);
+break;
+case VMPORT_ACTION_MAX:
+assert(action != VMPORT_ACTION_MAX);
+rc = 0; /* Should be impossible to get here. */
+break;
+}
+convert_local_rc(errp, rc);
+}
+
+typedef struct keyValue {
+void *key_data;
Re: [Qemu-devel] [PATCH v6 4/7] vmport_rpc: Add QMP access to vmport_rpc object.
On 05/12/15 15:50, Eric Blake wrote:
On 05/12/2015 01:03 PM, Don Slutz wrote:
This adds one new inject command:
inject-vmport-action
And three guest info commands:
vmport-guestinfo-set
vmport-guestinfo-get
query-vmport-guestinfo
vmport-guestinfo-get key=foo
More details in qmp-commands.hx
Signed-off-by: Don Slutz dsl...@verizon.com
---
+/*
+ * Run func() for every VMPortRpc device, traverse the tree for
+ * everything else. Note: This routine expects that opaque is a
+ * VMPortRpcFind pointer and not NULL.
+ */
+static int find_VMPortRpc_device(Object *obj, void *opaque)
+{
+VMPortRpcFind *find = opaque;
+Object *dev;
+VMPortRpcState *s;
+
+if (find-found) {
Why not assert(find) instead of leaving it to the comment?
My memory says that someone complained about too many asserts used,
so I just commented it.
I have no issue with adding an assert() and dropping the comment,
and so plan on doing this.
+/*
+ * Loop through all dynamically created VMPortRpc devices and call
+ * func() for each instance.
+ */
+static int foreach_dynamic_vmport_rpc_device(FindVMPortRpcDeviceFunc *func,
+ void *arg)
+{
+VMPortRpcFind find = {
+.func = func,
+.arg = arg,
Is it worth marking arg const here and in the VMPortRpcFind struct...
See below
+void qmp_inject_vmport_action(enum VmportAction action, Error **errp)
+{
+int rc;
+
+switch (action) {
+case VMPORT_ACTION_REBOOT:
+rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send,
+ (void *)OS_Reboot);
...so that you don't have to cast away const here?
Not sure. I still need 2 casts:
-static int vmport_rpc_find_list(VMPortRpcState *s, void *arg)
+static int vmport_rpc_find_list(VMPortRpcState *s, const void *arg)
{
-g_hash_table_foreach(s-guestinfo, vmport_rpc_find_list_one, arg);
+g_hash_table_foreach(s-guestinfo, vmport_rpc_find_list_one,
(gpointer)arg);
return 0;
}
and
-static int find_get(VMPortRpcState *s, void *arg)
+static int find_get(VMPortRpcState *s, const void *arg)
{
-keyValue *key_value = arg;
+keyValue *key_value = (void *)arg;
get_qmp_guestinfo() does change parts of key_value.
So are these casts better or worse?
+break;
+case VMPORT_ACTION_HALT:
+rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send,
+ (void *)OS_Halt);
+break;
+case VMPORT_ACTION_MAX:
+assert(action != VMPORT_ACTION_MAX);
+rc = 0; /* Should be impossible to get here. */
I'd rather abort() if someone compiled with -NDEBUG.
Ok, will change to abort.
+break;
+}
+convert_local_rc(errp, rc);
+}
+
+typedef struct keyValue {
+void *key_data;
+void *value_data;
+unsigned int key_len;
+unsigned int value_len;
Should these be size_t?
There is no need to. There is a limit on the max values that can be
used here (because without the max check it is possible for the guest
to request a lot of memory outside of the memory provided to the guest
as ram.
Currently using unsigned char for key_len and unsigned short for
value_len will work, but I went with unsigned int to avoid strange
issues if someone in the future changes the allowed maxes.
However this is not important to me and so if you want them
to be size_t, I am happy to change them.
+void qmp_vmport_guestinfo_set(const char *key, const char *value,
+ bool has_format, enum DataFormat format,
+ Error **errp)
+{
+int rc;
+keyValue key_value;
+
+if (strncmp(key, guestinfo., strlen(guestinfo.)) == 0) {
+key_value.key_data = (void *)(key + strlen(guestinfo.));
+key_value.key_len = strlen(key) - strlen(guestinfo.);
+} else {
+key_value.key_data = (void *)key;
Casting to (void*) looks awkward; should key_data should be typed 'const
void *' to avoid the need for a cast? For that matter, why is it void*,
why not 'const char *'?
Not sure. Best guess is that I was focused on the value being binary
data, and just did the same for the key. The change to
const char *key_data; works, and so I will do this.
+++ b/qmp-commands.hx
@@ -490,6 +490,126 @@ Note: inject-nmi fails when the guest doesn't support
injecting.
EQMP
+SQMP
+vmport-guestinfo-set
+--
Still mismatches on line length (several sites).
Sigh, will fix.
+- { execute: vmport-guestinfo-get,
+arguments: { key: guestinfo.foo,
+ format: utf8 } }
+- {return: {value: abcdefgh}}
+
+
+EQMP
+
+{
+.name = query-vmport-guestinfo,
+.args_type = ,
+.mhandler.cmd_new = qmp_marshal_input_query_vmport_guestinfo,
+},
+
+SQMP
+query-vmport-guestinfo
