On 06/12/2015 08:05 AM, 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 <[email protected]> > CC: Don Slutz <[email protected]> > ---
> +typedef int FindVMPortRpcDeviceFunc(VMPortRpcState *s, const void *arg);
> +
> +/*
> + * The input and output argument that find_VMPortRpc_device() uses
> + * to do it's work.
s/it's/its/ (the only time "it's" is correct is if "it is" would also be
correct)
>
> +static int get_qmp_guestinfo(VMPortRpcState *s,
> + unsigned int a_key_len, const char *a_info_key,
> + unsigned int *a_value_len, void ** a_value_data)
No space after **.
> +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,
> + "OS_Reboot");
> + break;
> + case VMPORT_ACTION_HALT:
> + rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send,
> + "OS_Halt");
> + break;
> + case VMPORT_ACTION_MAX:
> + assert(action != VMPORT_ACTION_MAX);
> + abort(); /* Should be impossible to get here. */
> + break;
Don't know if any static checkers will complain about the break being
dead code.
> +
> +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."));
Cast here...
> + key_value.key_len = strlen(key) - strlen("guestinfo.");
> + } else {
> + key_value.key_data = (void *)key;
...and here...
> + key_value.key_len = strlen(key);
> + }
> + if (has_format && (format == DATA_FORMAT_BASE64)) {
> + gsize write_count;
> +
> + key_value.value_data = g_base64_decode(value, &write_count);
> + key_value.value_len = write_count;
> + } else {
> + key_value.value_data = (void *)value;
...and here is only needed to get rid of const. Would it help if struct
keyValue declared 'const char *key_data' instead of 'void *key_data'?
> + key_value.value_len = strlen(value);
> + }
> +
> + rc = foreach_dynamic_vmport_rpc_device(find_set, (void *)&key_value);
This cast is spurious (it does not get rid of const, and any non-const
pointer in C can already be passed to void* without a cast).
> +VmportGuestInfoValue *qmp_vmport_guestinfo_get(const char *key,
> + bool has_format,
> + enum DataFormat format,
> + Error **errp)
> +{
> + int rc;
> + keyValue key_value;
> + VmportGuestInfoValue *ret_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;
More casts that might be prevented with correct const in the callback
struct.
> + key_value.key_len = strlen(key);
> + }
> +
> + rc = foreach_dynamic_vmport_rpc_device(find_get, (void *)&key_value);
Another spurious cast.
> + if (rc) {
> + convert_local_rc(errp, rc);
> + return NULL;
> + }
> +
> + ret_value = g_malloc(sizeof(VmportGuestInfoKey));
Do you want g_malloc0, or even the additional type-safety (and less
typing) of g_new0(VmportGuestInfoKey, 1)? Otherwise, if you later add
fields, but forget to initialize them, you'll have random garbage.
> + if (has_format && (format == DATA_FORMAT_BASE64)) {
> + ret_value->value = g_base64_encode(key_value.value_data,
> + key_value.value_len);
> + } else {
> + /*
> + * FIXME should check for complete, valid UTF-8 characters.
> + * Invalid sequences should be replaced by a suitable
> + * replacement character.
Or cause an error to be raised.
Looking better, though.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
