[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 4/7] vmport_rpc: Add QMP access to vmport_rpc
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v6 4/7] vmport_rpc: Add QMP access to vmport_rpc object. |
Date: |
Tue, 12 May 2015 13:50:34 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
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 <address@hidden>
> ---
> +/*
> + * 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 0/7] Add limited support of VMware's hyper-call rpc, Don Slutz, 2015/05/12
- [Qemu-devel] [PATCH v6 1/7] vmport.c: Fix vmport_cmd_ram_size, Don Slutz, 2015/05/12
- [Qemu-devel] [PATCH v6 2/7] vmport_rpc: Add the object vmport_rpc, Don Slutz, 2015/05/12
- [Qemu-devel] [PATCH v6 3/7] vmport_rpc: Add limited support of VMware's hyper-call rpc, Don Slutz, 2015/05/12
- [Qemu-devel] [PATCH v6 6/7] vmport: Add VMware all ring hack, Don Slutz, 2015/05/12
- [Qemu-devel] [PATCH v6 5/7] vmport_rpc: Add migration, Don Slutz, 2015/05/12
- [Qemu-devel] [PATCH v6 4/7] vmport_rpc: Add QMP access to vmport_rpc object., Don Slutz, 2015/05/12
- Re: [Qemu-devel] [PATCH v6 4/7] vmport_rpc: Add QMP access to vmport_rpc object.,
Eric Blake <=
- [Qemu-devel] [PATCH v6 7/7] MAINTAINERS: add VMware port, Don Slutz, 2015/05/12