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