diff mbox

[v6,4/7] vmport_rpc: Add QMP access to vmport_rpc object.

Message ID 1431457421-31877-5-git-send-email-dslutz@verizon.com
State New
Headers show

Commit Message

Don Slutz May 12, 2015, 7:03 p.m. UTC
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 <dslutz@verizon.com>
---
 hw/misc/vmport_rpc.c | 268 +++++++++++++++++++++++++++++++++++++++++++++++++++
 monitor.c            |  24 +++++
 qapi-schema.json     | 101 +++++++++++++++++++
 qmp-commands.hx      | 120 +++++++++++++++++++++++
 4 files changed, 513 insertions(+)

Comments

Eric Blake May 12, 2015, 7:50 p.m. UTC | #1
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 <dslutz@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?
Don Slutz May 12, 2015, 10:10 p.m. UTC | #2
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 <dslutz@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
>> +----------
>> +
>> +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?
> 

You are following this correctly.  The issue is with "*format" (i.e.
DataFormat).  So the format arg would need to be added, and while I
expect most uses will all be base64, I do not see this as hard requirement.

So if the user of QMP what to use base64 for some keys and utf8 for
others, there is no way to express this in 1 arg.  This is the way
I came up with to handle this case.

   -Don Slutz
diff mbox

Patch

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;
+    void *value_data;
+    unsigned int key_len;
+    unsigned int value_len;
+} keyValue;
+
+static int find_set(VMPortRpcState *s, void *arg)
+{
+    keyValue *key_value = arg;
+
+    return set_guestinfo(s, key_value->key_len, key_value->value_len,
+                         key_value->key_data, key_value->value_data);
+}
+
+static int find_get(VMPortRpcState *s, void *arg)
+{
+    keyValue *key_value = arg;
+
+    return get_qmp_guestinfo(s, key_value->key_len, key_value->key_data,
+                             &key_value->value_len, &key_value->value_data);
+}
+
+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;
+        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;
+        key_value.value_len = strlen(value);
+    }
+
+    rc = foreach_dynamic_vmport_rpc_device(find_set, (void *)&key_value);
+
+    if (key_value.value_data != value) {
+        g_free(key_value.value_data);
+    }
+
+    if (rc) {
+        convert_local_rc(errp, rc);
+        return;
+    }
+}
+
+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;
+        key_value.key_len = strlen(key);
+    }
+
+    rc = foreach_dynamic_vmport_rpc_device(find_get, (void *)&key_value);
+    if (rc) {
+        convert_local_rc(errp, rc);
+        return NULL;
+    }
+
+    ret_value = g_malloc(sizeof(VmportGuestInfoKey));
+    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.
+         */
+        ret_value->value = g_malloc(key_value.value_len + 1);
+        memcpy(ret_value->value, key_value.value_data, key_value.value_len);
+        ret_value->value[key_value.value_len] = 0;
+    }
+
+    return ret_value;
+}
+
+
+static void vmport_rpc_find_list_one(gpointer key, gpointer value,
+                                     gpointer opaque)
+{
+    VmportGuestInfoKeyList **keys = opaque;
+    VmportGuestInfoKeyList *info = g_malloc0(sizeof(*info));
+    char *ckey = key;
+
+    info->value = g_malloc0(sizeof(*info->value));
+    info->value->key = g_strdup_printf("guestinfo.%s", ckey);
+    info->next = *keys;
+    *keys = info;
+}
+
+static int vmport_rpc_find_list(VMPortRpcState *s, void *arg)
+{
+    g_hash_table_foreach(s->guestinfo, vmport_rpc_find_list_one, arg);
+    return 0;
+}
+
+VmportGuestInfoKeyList *qmp_query_vmport_guestinfo(Error **errp)
+{
+    VmportGuestInfoKeyList *keys = NULL;
+    int rc = foreach_dynamic_vmport_rpc_device(vmport_rpc_find_list,
+                                               (void *)&keys);
+
+    if (rc) {
+        convert_local_rc(errp, rc);
+    }
+
+    return keys;
+}
+
+
 static void vmport_rpc_reset(DeviceState *d)
 {
     unsigned int i;
diff --git a/monitor.c b/monitor.c
index b2561e1..22e2a43 100644
--- a/monitor.c
+++ b/monitor.c
@@ -5484,4 +5484,28 @@  void qmp_rtc_reset_reinjection(Error **errp)
 {
     error_set(errp, QERR_FEATURE_DISABLED, "rtc-reset-reinjection");
 }
+void qmp_inject_vmport_action(enum VmportAction action, Error **errp)
+{
+    error_set(errp, QERR_FEATURE_DISABLED, "inject-vmport-action");
+}
+void qmp_vmport_guestinfo_set(const char *key, const char *value,
+                              bool has_format, enum DataFormat format,
+                              Error **errp)
+{
+    error_set(errp, QERR_FEATURE_DISABLED, "vmport-guestinfo-set");
+}
+VmportGuestInfoValue *qmp_vmport_guestinfo_get(const char *key,
+                                               bool has_format,
+                                               enum DataFormat format,
+                                               Error **errp)
+{
+    error_set(errp, QERR_FEATURE_DISABLED, "vmport-guestinfo-get");
+    return NULL;
+}
+VmportGuestInfoKeyList *qmp_query_vmport_guestinfo(Error **errp)
+{
+    error_set(errp, QERR_FEATURE_DISABLED, "query-vmport-guestinfo");
+    return NULL;
+}
 #endif
+
diff --git a/qapi-schema.json b/qapi-schema.json
index 9c92482..893c905 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1406,6 +1406,107 @@ 
 { 'command': 'inject-nmi' }
 
 ##
+# @VmportAction:
+#
+# An enumeration of actions that can be requested via vmport RPC.
+#
+# @reboot: Ask the guest via VMware tools to reboot
+#
+# @halt: Ask the guest via VMware tools to halt
+#
+# Since: 2.4
+##
+{ 'enum': 'VmportAction',
+  'data': [ 'reboot', 'halt' ] }
+
+##
+# @inject-vmport-action:
+#
+# Injects a VMWare Tools action to the guest.
+#
+# Returns:  If successful, nothing
+#
+# Since:  2.4
+#
+##
+{ 'command': 'inject-vmport-action',
+  'data': {'action': 'VmportAction'} }
+
+##
+# @vmport-guestinfo-set:
+#
+# Set a VMWare Tools guestinfo key to a value
+#
+# @key: the key to set
+#
+# @value: The data to set the key to
+#
+# @format: #optional value encoding (default 'utf8').
+#          - base64: value is assumed to be base64 encoded text.  Its binary
+#            decoding gets set.
+#          - utf8: value's UTF-8 encoding is used to set.
+#
+# Returns: Nothing on success
+#
+# Since: 2.4
+##
+{ 'command': 'vmport-guestinfo-set',
+  'data': {'key': 'str', 'value': 'str',
+           '*format': 'DataFormat'} }
+
+##
+# @VmportGuestInfoValue:
+#
+# Information about a single VMWare Tools guestinfo
+#
+# @value: The value for a key
+#
+# Since: 2.4
+##
+{ 'struct': 'VmportGuestInfoValue', 'data': {'value': 'str'} }
+
+##
+# @vmport-guestinfo-get:
+#
+# Get a VMWare Tools guestinfo value for a key
+#
+# @key: the key to get
+#
+# @format: #optional data encoding (default 'utf8').
+#          - base64: the value is returned in base64 encoding.
+#          - utf8: the value is interpreted as UTF-8.
+#
+# Returns: value for the guest info key
+#
+# Since: 2.4
+##
+{ 'command': 'vmport-guestinfo-get',
+  'data': {'key': 'str', '*format': 'DataFormat'},
+  'returns': 'VmportGuestInfoValue' }
+
+##
+# @VmportGuestInfoKey:
+#
+# Information about a single VMWare Tools guestinfo
+#
+# @key: The known key
+#
+# Since: 2.4
+##
+{ 'struct': 'VmportGuestInfoKey', 'data': {'key': 'str'} }
+
+##
+# @query-vmport-guestinfo:
+#
+# Returns information about VMWare Tools guestinfo
+#
+# Returns: a list of @VmportGuestInfoKey
+#
+# Since: 2.4
+##
+{ 'command': 'query-vmport-guestinfo', 'returns': ['VmportGuestInfoKey'] }
+
+##
 # @set_link:
 #
 # Sets the link status of a virtual network adapter.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 7506774..5b28972 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -490,6 +490,126 @@  Note: inject-nmi fails when the guest doesn't support injecting.
 EQMP
 
     {
+        .name       = "inject-vmport-action",
+        .args_type  = "action:s",
+        .mhandler.cmd_new = qmp_marshal_input_inject_vmport_action,
+    },
+
+SQMP
+inject-vmport-action
+--------------------
+
+Inject a VMWare Tools action to the guest.
+
+Arguments:
+
+- "action": vmport action (json-string)
+          - Possible values: "reboot", "halt"
+
+Example:
+
+-> { "execute": "inject-vmport-action",
+                "arguments": { "action": "reboot" } }
+<- { "return": {} }
+
+Note: inject-vmport-action fails when the guest doesn't support injecting.
+
+EQMP
+
+    {
+        .name       = "vmport-guestinfo-set",
+        .args_type  = "key:s,value:s,format:s?",
+        .mhandler.cmd_new = qmp_marshal_input_vmport_guestinfo_set,
+    },
+
+SQMP
+vmport-guestinfo-set
+----------
+
+Set a VMWare Tools guestinfo key to a value
+
+Arguments:
+
+- "key": the key to set (json-string)
+- "value": data to write (json-string)
+- "format": data format (json-string, optional)
+          - Possible values: "utf8" (default), "base64"
+
+Example:
+
+-> { "execute": "vmport-guestinfo-set",
+                "arguments": { "key": "guestinfo.foo",
+                               "value": "abcdefgh",
+                               "format": "utf8" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "vmport-guestinfo-get",
+        .args_type  = "key:s,format:s?",
+        .mhandler.cmd_new = qmp_marshal_input_vmport_guestinfo_get,
+    },
+
+SQMP
+vmport-guestinfo-get
+----------
+
+Get a VMWare Tools guestinfo value for a key
+
+Arguments:
+
+- "key": the key to get (json-string)
+- "format": data format (json-string, optional)
+          - Possible values: "utf8" (default), "base64"
+          - Naturally, format "utf8" works only when the ring buffer
+            contains valid UTF-8 text.  Invalid UTF-8 sequences get
+            replaced.  Bug: replacement doesn't work.  Bug: can screw
+            up on encountering NUL characters.
+
+Example:
+
+-> { "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"
+         },
+         {
+            "key": "guestinfo.foo"
+         },
+         {
+            "key": "guestinfo.long"
+         }
+      ]
+   }
+
+EQMP
+
+    {
         .name       = "ringbuf-write",
         .args_type  = "device:s,data:s,format:s?",
         .mhandler.cmd_new = qmp_marshal_input_ringbuf_write,