* [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
@ 2015-06-12 14:05 Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [BUGFIX][PATCH v7 1/9] vmport: The io memory region needs to be at least a size of 4 Don Slutz
` (10 more replies)
0 siblings, 11 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-12 14:05 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Don Slutz, Luiz Capitulino,
Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
Changes v6 to v7:
Rebase to master
Fixed a bug caused by commit c3c1bb99d1c11978d9ce94d1bdcf0705378c1459 now patch #1
Added patch #2 to switch to using trace in vm,port.c.
Delay call on g_strndup till after key length check.
Switched e-mail address in MAINTAINERS.
Eric Blake
Why not assert(find) instead of leaving it to the comment?
Switch to assert.
Is it worth marking arg const here and in the VMPortRpcFind struct
Switch to const.
I'd rather abort() if someone compiled with -NDEBUG
Done.
Still mismatches on ---- line length (several sites).
Done
Changes v5 to v6:
Rebase to master
Eric Blake
Returning a non-dictionary is not extensible.
Added new type VmportGuestInfoValue.
s/VmportGuestInfo/VmportGuestInfoKey/
s/type/struct/
Issues with examples
Fixed.
Changes v4 to v5:
Paolo Bonzini
What is VMPORT_SHORT about?
Dropped this.
Why not use a bool in CPUX86State?
Took his sugestion and moved to a bool in X86CPU.
Changes v3 to v4:
Paolo Bonzini on "vmort_rpc: Add QMP access to vmport_rpc"
Does this compile on non-x86 targets?
Nope. Fixed.
Changes v2 to v3:
s/2.3/2.4
Changes v1 to v2:
Added live migration code.
Adjust data structures for migration.
Switch to GHashTable.
Eric Blake
s/spawened/spawned/
Done
s/traceing/tracing/
Done
Change "error_set(errp, ERROR_CLASS_GENERIC_ERROR, " to
"error_setg(errp, "
Done
Why two commands (inject-vmport-reboot, inject-vmport-halt)?
Switched to inject-vmport-action.
format=base64 "bug" statements.
Dropped.
Much more on format=base64:
If there is a bug it is in GLIB. However the Glib reference manual
refers to RFC 1421 and RFC 2045 and MIME encoding. Based on all
that (which seems to match:
http://en.wikipedia.org/wiki/Base64
) MIME states that all characters outside the (base64) alphabet are
to be ignored. Testing shows that g_base64_decode() does this.
The confusion is that most non-MIME uses reject a base64 string that
contain characters outside the alphabet. I was just following the
other uses of base64 in this file.
DataFormat refers to RFC 3548, which has the info:
"
Implementations MUST reject the encoding if it contains
characters outside the base alphabet when interpreting base
encoded data, unless the specification referring to this document
explicitly states otherwise. Such specifications may, as MIME
does, instead state that characters outside the base encoding
alphabet should simply be ignored when interpreting data ("be
liberal in what you accept").
"
So with GLIB going the MIME way, I do not think this is a QEMU bug
(you could consider this a GLIB bug, but the document I found says
that GLIB goes the MIME way and so does not reject anything).
---
The support included is enough to allow VMware tools to install in a
guest and provide guestinfo support. guestinfo support is provided
by what is known as VMware RPC support.
One of the better on-line references is:
https://sites.google.com/site/chitchatvmback/backdoor
As a place to get more accurate information by studying:
http://open-vm-tools.sourceforge.net/
With vmware tools installed, you can do:
-------------------------------------------------------------------------------
Last login: Fri Jan 30 16:03:08 2015
[root@C63-min-tools ~]# vmtoolsd --cmd "info-get guestinfo.joejoel"
No value found
[root@C63-min-tools ~]# vmtoolsd --cmd "info-set guestinfo.joejoel bar"
[root@C63-min-tools ~]# vmtoolsd --cmd "info-get guestinfo.joejoel"
bar
[root@C63-min-tools ~]#
-------------------------------------------------------------------------------
to access guest info. QMP access is also provided.
The live migration code is still in progress.
Don Slutz (9):
vmport: The io memory region needs to be at least a size of 4
vmport: Switch to trace
vmport: Fix vmport_cmd_ram_size
vmport_rpc: Add the object vmport_rpc
vmport_rpc: Add limited support of VMware's hyper-call rpc
vmport_rpc: Add QMP access to vmport_rpc object.
vmport_rpc: Add migration
vmport: Add VMware all ring hack
MAINTAINERS: add VMware port
MAINTAINERS | 7 +
hw/i386/pc.c | 32 +-
hw/i386/pc_piix.c | 2 +-
hw/i386/pc_q35.c | 2 +-
hw/misc/Makefile.objs | 1 +
hw/misc/vmport.c | 15 +-
hw/misc/vmport_rpc.c | 1457 ++++++++++++++++++++++++++++++++++++++++++++++
include/hw/i386/pc.h | 6 +-
monitor.c | 24 +
qapi-schema.json | 101 ++++
qmp-commands.hx | 119 ++++
target-i386/cpu-qom.h | 3 +
target-i386/seg_helper.c | 9 +
trace-events | 28 +
14 files changed, 1796 insertions(+), 10 deletions(-)
create mode 100644 hw/misc/vmport_rpc.c
--
1.8.4
^ permalink raw reply [flat|nested] 44+ messages in thread
* [Qemu-devel] [BUGFIX][PATCH v7 1/9] vmport: The io memory region needs to be at least a size of 4
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
@ 2015-06-12 14:05 ` Don Slutz
2015-06-12 22:38 ` Eric Blake
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 2/9] vmport: Switch to trace Don Slutz
` (9 subsequent siblings)
10 siblings, 1 reply; 44+ messages in thread
From: Don Slutz @ 2015-06-12 14:05 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Don Slutz, Luiz Capitulino,
Don Slutz, Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
Before:
commit c3c1bb99d1c11978d9ce94d1bdcf0705378c1459
Author: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Date: Mon Mar 16 22:35:54 2015 -0700
exec: Respect as_tranlsate_internal length clamp
it did not matter. Only accept I/O that starts on 1st
port.
Signed-off-by: Don Slutz <dslutz@verizon.com>
CC: Don Slutz <don.slutz@gmail.com>
---
hw/misc/vmport.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/misc/vmport.c b/hw/misc/vmport.c
index 7fcc00d..51b64bc 100644
--- a/hw/misc/vmport.c
+++ b/hw/misc/vmport.c
@@ -69,6 +69,10 @@ static uint64_t vmport_ioport_read(void *opaque, hwaddr addr,
unsigned char command;
uint32_t eax;
+ /* Only support 1 address */
+ if (addr) {
+ return ~0U;
+ }
cpu_synchronize_state(cs);
eax = env->regs[R_EAX];
@@ -159,7 +163,7 @@ static void vmport_realizefn(DeviceState *dev, Error **errp)
ISADevice *isadev = ISA_DEVICE(dev);
VMPortState *s = VMPORT(dev);
- memory_region_init_io(&s->io, OBJECT(s), &vmport_ops, s, "vmport", 1);
+ memory_region_init_io(&s->io, OBJECT(s), &vmport_ops, s, "vmport", 4);
isa_register_ioport(isadev, &s->io, 0x5658);
port_state = s;
--
1.8.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Qemu-devel] [PATCH v7 2/9] vmport: Switch to trace
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [BUGFIX][PATCH v7 1/9] vmport: The io memory region needs to be at least a size of 4 Don Slutz
@ 2015-06-12 14:05 ` Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [BUGFIX][PATCH v7 3/9] vmport: Fix vmport_cmd_ram_size Don Slutz
` (8 subsequent siblings)
10 siblings, 0 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-12 14:05 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Don Slutz, Luiz Capitulino,
Don Slutz, Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
Signed-off-by: Don Slutz <dslutz@verizon.com>
CC: Don Slutz <don.slutz@gmail.com>
---
hw/misc/vmport.c | 7 +++----
trace-events | 4 ++++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/misc/vmport.c b/hw/misc/vmport.c
index 51b64bc..203e892 100644
--- a/hw/misc/vmport.c
+++ b/hw/misc/vmport.c
@@ -27,7 +27,7 @@
#include "sysemu/kvm.h"
#include "hw/qdev.h"
-//#define VMPORT_DEBUG
+#include "trace.h"
#define VMPORT_CMD_GETVERSION 0x0a
#define VMPORT_CMD_GETRAMSIZE 0x14
@@ -71,6 +71,7 @@ static uint64_t vmport_ioport_read(void *opaque, hwaddr addr,
/* Only support 1 address */
if (addr) {
+ trace_vmport_ioport_ignored(addr, size, cs);
return ~0U;
}
cpu_synchronize_state(cs);
@@ -84,9 +85,7 @@ static uint64_t vmport_ioport_read(void *opaque, hwaddr addr,
return eax;
if (!s->func[command])
{
-#ifdef VMPORT_DEBUG
- fprintf(stderr, "vmport: unknown command %x\n", command);
-#endif
+ trace_vmport_ioport_unknown_command(command);
return eax;
}
diff --git a/trace-events b/trace-events
index d6c73e6..e229aac 100644
--- a/trace-events
+++ b/trace-events
@@ -883,6 +883,10 @@ pc87312_info_ide(uint32_t base) "base 0x%x"
pc87312_info_parallel(uint32_t base, uint32_t irq) "base 0x%x, irq %u"
pc87312_info_serial(int n, uint32_t base, uint32_t irq) "id=%d, base 0x%x, irq %u"
+# hw/misc/vmport.c
+vmport_ioport_ignored(uint64_t addr, uint32_t size, void *cs) "addr=%#" PRIx64 " size=%u cs=%p"
+vmport_ioport_unknown_command(unsigned char command) "%x"
+
# hw/scsi/vmw_pvscsi.c
pvscsi_ring_init_data(uint32_t txr_len_log2, uint32_t rxr_len_log2) "TX/RX rings logarithms set to %d/%d"
pvscsi_ring_init_msg(uint32_t len_log2) "MSG ring logarithm set to %d"
--
1.8.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Qemu-devel] [BUGFIX][PATCH v7 3/9] vmport: Fix vmport_cmd_ram_size
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [BUGFIX][PATCH v7 1/9] vmport: The io memory region needs to be at least a size of 4 Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 2/9] vmport: Switch to trace Don Slutz
@ 2015-06-12 14:05 ` Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 4/9] vmport_rpc: Add the object vmport_rpc Don Slutz
` (7 subsequent siblings)
10 siblings, 0 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-12 14:05 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Don Slutz, Luiz Capitulino,
Don Slutz, Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
Based on
https://sites.google.com/site/chitchatvmback/backdoor
and testing on ESXi, this should be in MB not bytes.
Signed-off-by: Don Slutz <dslutz@verizon.com>
CC: Don Slutz <don.slutz@gmail.com>
---
hw/misc/vmport.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/misc/vmport.c b/hw/misc/vmport.c
index 203e892..000af18 100644
--- a/hw/misc/vmport.c
+++ b/hw/misc/vmport.c
@@ -113,7 +113,7 @@ static uint32_t vmport_cmd_ram_size(void *opaque, uint32_t addr)
X86CPU *cpu = X86_CPU(current_cpu);
cpu->env.regs[R_EBX] = 0x1177;
- return ram_size;
+ return ram_size >> 20; /* in MB */
}
static uint32_t vmport_cmd_xtest(void *opaque, uint32_t addr)
--
1.8.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Qemu-devel] [PATCH v7 4/9] vmport_rpc: Add the object vmport_rpc
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
` (2 preceding siblings ...)
2015-06-12 14:05 ` [Qemu-devel] [BUGFIX][PATCH v7 3/9] vmport: Fix vmport_cmd_ram_size Don Slutz
@ 2015-06-12 14:05 ` Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 5/9] vmport_rpc: Add limited support of VMware's hyper-call rpc Don Slutz
` (6 subsequent siblings)
10 siblings, 0 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-12 14:05 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Don Slutz, Luiz Capitulino,
Don Slutz, Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
This is the 1st part of "Add limited support of VMware's hyper-call
rpc".
This patch uses existing infrastructure used by vmmouse.c (provided
by vmport.c) to handle the VMware backdoor command 30.
One of the better on-line references is:
https://sites.google.com/site/chitchatvmback/backdoor
More in next patch.
Signed-off-by: Don Slutz <dslutz@verizon.com>
CC: Don Slutz <don.slutz@gmail.com>
---
hw/i386/pc.c | 6 +++
hw/misc/Makefile.objs | 1 +
hw/misc/vmport_rpc.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 133 insertions(+)
create mode 100644 hw/misc/vmport_rpc.c
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3f0d435..48711c6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1478,8 +1478,14 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
i8042 = isa_create_simple(isa_bus, "i8042");
i8042_setup_a20_line(i8042, &a20_line[0]);
if (!no_vmport) {
+ ISADevice *vmport_rpc;
+
vmport_init(isa_bus);
vmmouse = isa_try_create(isa_bus, "vmmouse");
+ vmport_rpc = isa_try_create(isa_bus, "vmport_rpc");
+ if (vmport_rpc) {
+ qdev_init_nofail(DEVICE(vmport_rpc));
+ }
} else {
vmmouse = NULL;
}
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 4aa76ff..e04c8ac 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -7,6 +7,7 @@ common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o
common-obj-$(CONFIG_PCI_TESTDEV) += pci-testdev.o
obj-$(CONFIG_VMPORT) += vmport.o
+obj-$(CONFIG_VMPORT) += vmport_rpc.o
# ARM devices
common-obj-$(CONFIG_PL310) += arm_l2x0.o
diff --git a/hw/misc/vmport_rpc.c b/hw/misc/vmport_rpc.c
new file mode 100644
index 0000000..b7cd355
--- /dev/null
+++ b/hw/misc/vmport_rpc.c
@@ -0,0 +1,126 @@
+/*
+ * QEMU VMPORT RPC emulation
+ *
+ * Copyright (C) 2015 Verizon Corporation
+ *
+ * This file is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License Version 2 (GPLv2)
+ * as published by the Free Software Foundation.
+ *
+ * This file is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details. <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * One of the better on-line references is:
+ *
+ * https://sites.google.com/site/chitchatvmback/backdoor
+ *
+ * Which points you to:
+ *
+ * http://open-vm-tools.sourceforge.net/
+ *
+ * as a place to get more accurate information by studying.
+ */
+
+#include "hw/hw.h"
+#include "hw/i386/pc.h"
+#include "hw/qdev.h"
+#include "trace.h"
+#include "qmp-commands.h"
+#include "qapi/qmp/qerror.h"
+
+/* #define VMPORT_RPC_DEBUG */
+
+#define TYPE_VMPORT_RPC "vmport_rpc"
+#define VMPORT_RPC(obj) OBJECT_CHECK(VMPortRpcState, (obj), TYPE_VMPORT_RPC)
+
+/* VMPORT RPC Command */
+#define VMPORT_RPC_COMMAND 30
+
+/* The vmport_rpc object. */
+typedef struct VMPortRpcState {
+ ISADevice parent_obj;
+
+ /* Properties */
+ uint64_t reset_time;
+ uint64_t build_number_value;
+ uint64_t build_number_time;
+
+ /* Private data */
+} VMPortRpcState;
+
+typedef struct {
+ uint32_t eax;
+ uint32_t ebx;
+ uint32_t ecx;
+ uint32_t edx;
+ uint32_t esi;
+ uint32_t edi;
+} vregs;
+
+static uint32_t vmport_rpc_ioport_read(void *opaque, uint32_t addr)
+{
+ VMPortRpcState *s = opaque;
+ union {
+ uint32_t data[6];
+ vregs regs;
+ } ur;
+
+ vmmouse_get_data(ur.data);
+
+ s->build_number_time++;
+
+ vmmouse_set_data(ur.data);
+ return ur.data[0];
+}
+
+static void vmport_rpc_reset(DeviceState *d)
+{
+ VMPortRpcState *s = VMPORT_RPC(d);
+
+ s->reset_time = 14;
+ s->build_number_value = 0;
+ s->build_number_time = 0;
+}
+
+static void vmport_rpc_realize(DeviceState *dev, Error **errp)
+{
+ VMPortRpcState *s = VMPORT_RPC(dev);
+
+ vmport_register(VMPORT_RPC_COMMAND, vmport_rpc_ioport_read, s);
+}
+
+static Property vmport_rpc_properties[] = {
+ DEFINE_PROP_UINT64("reset-time", VMPortRpcState, reset_time, 14),
+ DEFINE_PROP_UINT64("build-number-value", VMPortRpcState,
+ build_number_value, 0),
+ DEFINE_PROP_UINT64("build-number-time", VMPortRpcState,
+ build_number_time, 0),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vmport_rpc_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = vmport_rpc_realize;
+ dc->reset = vmport_rpc_reset;
+ dc->props = vmport_rpc_properties;
+}
+
+static const TypeInfo vmport_rpc_info = {
+ .name = TYPE_VMPORT_RPC,
+ .parent = TYPE_ISA_DEVICE,
+ .instance_size = sizeof(VMPortRpcState),
+ .class_init = vmport_rpc_class_init,
+};
+
+static void vmport_rpc_register_types(void)
+{
+ type_register_static(&vmport_rpc_info);
+}
+
+type_init(vmport_rpc_register_types)
--
1.8.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Qemu-devel] [PATCH v7 5/9] vmport_rpc: Add limited support of VMware's hyper-call rpc
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
` (3 preceding siblings ...)
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 4/9] vmport_rpc: Add the object vmport_rpc Don Slutz
@ 2015-06-12 14:05 ` Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 6/9] vmport_rpc: Add QMP access to vmport_rpc object Don Slutz
` (5 subsequent siblings)
10 siblings, 0 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-12 14:05 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Don Slutz, Luiz Capitulino,
Don Slutz, Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
The support included is enough to allow VMware tools to install in a
guest and provide guestinfo support. guestinfo support is provided
by what is known as VMware RPC support.
If the guest is running VMware tools, then the "build version" of
the tools is also available via the property build-number-value of
the vmport_rpc object. The build-number-time property is changed
every time the VMware tools running in the guest sends the
"build version" via the rpc.
The property reset-time controls how often to request them in
seconds minus 1. The minus 1 is to handle to 0 case. I.E. the
fastest that can be selected is every second. The default is 4
times a minute.
The VMware RPC support includes the notion of channels that are
opened, active and closed. All RPC messages sent via a channel
starts with normal ASCII text. The message some times does include
binary data.
Currently there are 2 protocols defined for VMware RPC. They
determine the direction for data flow, guest to tool stack or tool
stack to guest.
There is no provided interrupt for VMware RPC.
Getting VMPORT_RPC_DEBUG will provide a higher level trace calls
that are simpler to understand.
Signed-off-by: Don Slutz <dslutz@verizon.com>
CC: Don Slutz <don.slutz@gmail.com>
---
hw/misc/vmport_rpc.c | 796 ++++++++++++++++++++++++++++++++++++++++++++++++++-
trace-events | 22 ++
2 files changed, 817 insertions(+), 1 deletion(-)
diff --git a/hw/misc/vmport_rpc.c b/hw/misc/vmport_rpc.c
index b7cd355..e2d5c36 100644
--- a/hw/misc/vmport_rpc.c
+++ b/hw/misc/vmport_rpc.c
@@ -40,6 +40,123 @@
/* VMPORT RPC Command */
#define VMPORT_RPC_COMMAND 30
+/* Limits on amount of non guest memory to use */
+#define MAX_KEY_LEN 128
+#define MIN_VAL_LEN 64
+#define MAX_VAL_LEN 8192
+#define MAX_NUM_KEY 256
+#define MAX_BKTS 4
+/* Max number of channels. */
+#define GUESTMSG_MAX_CHANNEL 8
+
+/*
+ * All of VMware's rpc is based on 32bit registers. So this is the
+ * number of bytes in ebx.
+ */
+#define CHAR_PER_CALL sizeof(uint32_t)
+/* Room for basic commands */
+#define EXTRA_SEND 22
+/* Status code and NULL */
+#define EXTRA_RECV 2
+#define MAX_SEND_BUF DIV_ROUND_UP(EXTRA_SEND + MAX_KEY_LEN + MAX_VAL_LEN, \
+ CHAR_PER_CALL)
+#define MAX_RECV_BUF DIV_ROUND_UP(EXTRA_RECV + MAX_VAL_LEN, CHAR_PER_CALL)
+#define MIN_SEND_BUF DIV_ROUND_UP(EXTRA_SEND + MAX_KEY_LEN + MIN_VAL_LEN, \
+ CHAR_PER_CALL)
+
+/* Reply statuses */
+/* The basic request succeeded */
+#define MESSAGE_STATUS_SUCCESS 0x0001
+/* vmware has a message available for its party */
+#define MESSAGE_STATUS_DORECV 0x0002
+/* The channel has been closed */
+#define MESSAGE_STATUS_CLOSED 0x0004
+/* vmware removed the message before the party fetched it */
+#define MESSAGE_STATUS_UNSENT 0x0008
+/* A checkpoint occurred */
+#define MESSAGE_STATUS_CPT 0x0010
+/* An underlying device is powering off */
+#define MESSAGE_STATUS_POWEROFF 0x0020
+/* vmware has detected a timeout on the channel */
+#define MESSAGE_STATUS_TIMEOUT 0x0040
+/* vmware supports high-bandwidth for sending and receiving the payload */
+#define MESSAGE_STATUS_HB 0x0080
+
+/* Max number of channels. */
+#define GUESTMSG_MAX_CHANNEL 8
+
+/* Flags to open a channel. */
+#define GUESTMSG_FLAG_COOKIE 0x80000000
+
+/* Data to guest */
+#define VMWARE_PROTO_TO_GUEST 0x4f4c4354
+/* Data from guest */
+#define VMWARE_PROTO_FROM_GUEST 0x49435052
+
+/*
+ * Error return values used only in this file. The routine
+ * convert_local_rc() is used to convert these to an Error
+ * object.
+ */
+#define VMPORT_DEVICE_NOT_FOUND -1
+#define SEND_NOT_OPEN -2
+#define SEND_SKIPPED -3
+#define SEND_TRUCATED -4
+#define SEND_NO_MEMORY -5
+#define GUESTINFO_NOTFOUND -6
+#define GUESTINFO_VALTOOLONG -7
+#define GUESTINFO_KEYTOOLONG -8
+#define GUESTINFO_TOOMANYKEYS -9
+#define GUESTINFO_NO_MEMORY -10
+
+
+/* The VMware RPC guest info storage . */
+typedef struct {
+ char *val_data;
+ uint16_t val_len;
+ uint16_t val_max;
+} guestinfo_t;
+
+/* The VMware RPC bucket control. */
+typedef struct {
+ uint16_t recv_len;
+ uint16_t recv_idx;
+ uint16_t recv_buf_max;
+} bucket_control_t;
+
+/* The VMware RPC bucket info. */
+typedef struct {
+ union {
+ uint32_t *words;
+ char *bytes;
+ } recv;
+ bucket_control_t ctl;
+} bucket_t;
+
+
+/* The VMware RPC channel control. */
+typedef struct {
+ uint64_t active_time;
+ uint32_t chan_id;
+ uint32_t cookie;
+ uint32_t proto_num;
+ uint16_t send_len;
+ uint16_t send_idx;
+ uint16_t send_buf_max;
+ uint8_t recv_read;
+ uint8_t recv_write;
+} channel_control_t;
+
+/* The VMware RPC channel info. */
+typedef struct {
+ union {
+ uint32_t *words;
+ char *bytes;
+ } send;
+ channel_control_t ctl;
+ bucket_t recv_bkt[MAX_BKTS];
+} channel_t;
+
/* The vmport_rpc object. */
typedef struct VMPortRpcState {
ISADevice parent_obj;
@@ -50,8 +167,32 @@ typedef struct VMPortRpcState {
uint64_t build_number_time;
/* Private data */
+ uint64_t ping_time;
+ uint32_t open_cookie;
+ channel_t chans[GUESTMSG_MAX_CHANNEL];
+ GHashTable *guestinfo;
+#ifdef VMPORT_RPC_DEBUG
+ unsigned int end;
+ unsigned int last;
+ char out[2048];
+#endif
} VMPortRpcState;
+/* Basic request types */
+typedef enum {
+ MESSAGE_TYPE_OPEN,
+ MESSAGE_TYPE_SENDSIZE,
+ MESSAGE_TYPE_SENDPAYLOAD,
+ MESSAGE_TYPE_RECVSIZE,
+ MESSAGE_TYPE_RECVPAYLOAD,
+ MESSAGE_TYPE_RECVSTATUS,
+ MESSAGE_TYPE_CLOSE,
+} MessageType;
+
+/*
+ * Overlay on the array that vmmouse_get_data() returns. The code is
+ * easier to read using register names.
+ */
typedef struct {
uint32_t eax;
uint32_t ebx;
@@ -61,6 +202,636 @@ typedef struct {
uint32_t edi;
} vregs;
+#ifdef VMPORT_RPC_DEBUG
+/*
+ * Add helper function for tracing. This routine will convert
+ * binary data into more normal characters so that the trace data is
+ * earier to read and will not have nulls in it.
+ */
+static void vmport_rpc_safe_print(VMPortRpcState *s, int len, const char *msg)
+{
+ unsigned char c;
+ unsigned int i, k;
+
+ s->end = len;
+ /* 1 byte can cnvert to 3 bytes, and save room at the end. */
+ if (s->end > (sizeof(s->out) / 3 - 6)) {
+ s->end = sizeof(s->out) / 3 - 6;
+ }
+ s->out[0] = '<';
+ k = 1;
+ for (i = 0; i < s->end; ++i) {
+ c = msg[i];
+ if ((c == '^') || (c == '\\') || (c == '>')) {
+ s->out[k++] = '\\';
+ s->out[k++] = c;
+ } else if ((c >= ' ') && (c <= '~')) {
+ s->out[k++] = c;
+ } else if (c < ' ') {
+ s->out[k++] = '^';
+ s->out[k++] = c ^ 0x40;
+ } else {
+ snprintf(&s->out[k], sizeof(s->out) - k, "\\%02x", c);
+ k += 3;
+ }
+ }
+ s->out[k++] = '>';
+ if (len > s->end) {
+ s->out[k++] = '.';
+ s->out[k++] = '.';
+ s->out[k++] = '.';
+ }
+ s->out[k++] = 0;
+ s->last = k;
+}
+#endif
+
+/*
+ * Copy message into a free receive bucket buffer which vmtools will
+ * use to read from 4 (CHAR_PER_CALL) bytes at a time until done
+ * with it.
+ */
+static int vmport_rpc_send(VMPortRpcState *s, channel_t *c,
+ const char *msg, unsigned int cur_recv_len)
+{
+ int rc;
+ unsigned int my_bkt = c->ctl.recv_write;
+ unsigned int next_bkt = my_bkt + 1;
+ bucket_t *b;
+
+ if (next_bkt >= MAX_BKTS) {
+ next_bkt = 0;
+ }
+
+ if (next_bkt == c->ctl.recv_read) {
+#ifdef VMPORT_RPC_DEBUG
+ {
+ char prefix[30];
+
+ snprintf(prefix, sizeof(prefix),
+ "VMware _send skipped %d (%d, %d) ",
+ c->ctl.chan_id, my_bkt, c->ctl.recv_read);
+ prefix[sizeof(prefix) - 1] = 0;
+ vmport_rpc_safe_print(s, cur_recv_len, msg);
+ trace_vmport_rpc_send_skip(prefix, s->end, cur_recv_len, s->last,
+ sizeof(s->out), s->out);
+ }
+#endif
+ return SEND_SKIPPED;
+ }
+
+ c->ctl.recv_write = next_bkt;
+ b = &c->recv_bkt[my_bkt];
+#ifdef VMPORT_RPC_DEBUG
+ {
+ char prefix[30];
+
+ snprintf(prefix, sizeof(prefix), "VMware _send %d (%d) ",
+ c->ctl.chan_id, my_bkt);
+ prefix[sizeof(prefix) - 1] = 0;
+ vmport_rpc_safe_print(s, cur_recv_len, msg);
+ trace_vmport_rpc_send_normal(prefix, s->end, cur_recv_len, s->last,
+ sizeof(s->out), s->out);
+ }
+#endif
+
+ if (b->ctl.recv_buf_max < MAX_RECV_BUF) {
+ size_t new_recv_buf_max = DIV_ROUND_UP(cur_recv_len, CHAR_PER_CALL);
+
+ if (new_recv_buf_max > b->ctl.recv_buf_max) {
+ uint32_t *new_recv_buf =
+ g_try_malloc((new_recv_buf_max + 1) * CHAR_PER_CALL);
+
+ if (new_recv_buf) {
+ g_free(b->recv.words);
+ b->recv.words = new_recv_buf;
+ b->ctl.recv_buf_max = new_recv_buf_max;
+ }
+ }
+ }
+ if (!b->recv.words) {
+ return SEND_NO_MEMORY;
+ }
+ b->ctl.recv_len = cur_recv_len;
+ b->ctl.recv_idx = 0;
+ rc = 0;
+ if (cur_recv_len > (b->ctl.recv_buf_max * CHAR_PER_CALL)) {
+ trace_vmport_rpc_send_big(cur_recv_len,
+ b->ctl.recv_buf_max * CHAR_PER_CALL);
+ cur_recv_len = b->ctl.recv_buf_max * CHAR_PER_CALL;
+ b->recv.words[b->ctl.recv_buf_max] = 0;
+ rc = SEND_TRUCATED;
+ } else {
+ b->recv.words[cur_recv_len / CHAR_PER_CALL] = 0;
+ b->recv.words[DIV_ROUND_UP(cur_recv_len, CHAR_PER_CALL)] = 0;
+ }
+ memcpy(b->recv.words, msg, cur_recv_len);
+ return rc;
+}
+
+static int vmport_rpc_ctrl_send(VMPortRpcState *s, char *msg)
+{
+ int rc = SEND_NOT_OPEN;
+ unsigned int i;
+
+ if (!s) {
+ return rc;
+ }
+ s->ping_time = get_clock() / 1000000000LL;
+ for (i = 0; i < GUESTMSG_MAX_CHANNEL; ++i) {
+ if (s->chans[i].ctl.proto_num == VMWARE_PROTO_TO_GUEST) {
+ rc = vmport_rpc_send(s, &s->chans[i], msg, strlen(msg) + 1);
+ }
+ }
+ return rc;
+}
+
+static void vmport_rpc_sweep(VMPortRpcState *s, unsigned long now_time)
+{
+ unsigned int i;
+
+ for (i = 0; i < GUESTMSG_MAX_CHANNEL; ++i) {
+ if (s->chans[i].ctl.proto_num) {
+ channel_t *c = &s->chans[i];
+ long delta = now_time - c->ctl.active_time;
+
+ if (delta >= 80) {
+ trace_vmport_rpc_sweep(c->ctl.chan_id, delta);
+ /* Return channel to free pool */
+ c->ctl.proto_num = 0;
+ }
+ }
+ }
+}
+
+static channel_t *vmport_rpc_new_chan(VMPortRpcState *s, unsigned long now_time)
+{
+ unsigned int i;
+
+ for (i = 0; i < GUESTMSG_MAX_CHANNEL; ++i) {
+ if (!s->chans[i].ctl.proto_num) {
+ channel_t *c = &s->chans[i];
+
+ c->ctl.chan_id = i;
+ c->ctl.cookie = s->open_cookie++;
+ c->ctl.active_time = now_time;
+ c->ctl.send_len = 0;
+ c->ctl.send_idx = 0;
+ c->ctl.recv_read = 0;
+ c->ctl.recv_write = 0;
+ if (!c->send.words) {
+ uint32_t *new_send_buf =
+ g_try_malloc0((MIN_SEND_BUF + 1) * CHAR_PER_CALL);
+ if (new_send_buf) {
+ c->send.words = new_send_buf;
+ c->ctl.send_buf_max = MIN_SEND_BUF;
+ }
+ }
+ if (!c->send.words) {
+ return NULL;
+ }
+ return c;
+ }
+ }
+ return NULL;
+}
+
+static void process_send_size(VMPortRpcState *s, channel_t *c,
+ vregs *ur)
+{
+ /* vmware tools often send a 0 byte request size. */
+ c->ctl.send_len = ur->ebx;
+ c->ctl.send_idx = 0;
+
+ if (c->ctl.send_buf_max < MAX_SEND_BUF) {
+ size_t new_send_max = DIV_ROUND_UP(c->ctl.send_len, CHAR_PER_CALL);
+
+ if (new_send_max > c->ctl.send_buf_max) {
+ uint32_t *new_send_buf =
+ g_try_malloc0((new_send_max + 1) * CHAR_PER_CALL);
+
+ if (new_send_buf) {
+ g_free(c->send.words);
+ c->send.words = new_send_buf;
+ c->ctl.send_buf_max = new_send_max;
+ }
+ }
+ }
+ ur->ecx = (MESSAGE_STATUS_SUCCESS << 16) | (ur->ecx & 0xffff);
+ trace_vmport_detail_rpc_process_send_size(c->ctl.chan_id,
+ c->ctl.send_len);
+}
+
+/* ret_buffer is in/out param */
+static int get_guestinfo(VMPortRpcState *s,
+ char *a_info_key, unsigned int a_key_len,
+ char *ret_buffer, unsigned int ret_buffer_len)
+{
+ guestinfo_t *gi = NULL;
+
+ trace_vmport_rpc_get_guestinfo(a_key_len, a_key_len, a_info_key);
+
+ if (a_key_len <= MAX_KEY_LEN) {
+ gpointer key = g_strndup(a_info_key, a_key_len);
+
+ gi = (guestinfo_t *)g_hash_table_lookup(s->guestinfo, key);
+ g_free(key);
+ } else {
+ return GUESTINFO_KEYTOOLONG;
+ }
+ if (gi) {
+ unsigned int ret_len = 2 + gi->val_len;
+
+ trace_vmport_rpc_get_guestinfo_found(gi->val_len, gi->val_len,
+ gi->val_data);
+
+ ret_buffer[0] = '1';
+ ret_buffer[1] = ' ';
+ if (ret_len > ret_buffer_len - 1) {
+ ret_len = ret_buffer_len - 1;
+ }
+ memcpy(ret_buffer + 2, gi->val_data, ret_len);
+ return ret_len;
+ }
+
+ return GUESTINFO_NOTFOUND;
+}
+
+static int set_guestinfo(VMPortRpcState *s, int a_key_len,
+ unsigned int a_val_len, const char *a_info_key,
+ char *val)
+{
+ gpointer key = NULL;
+ int rc = 0;
+
+ trace_vmport_rpc_set_guestinfo(a_key_len, a_key_len, a_info_key,
+ a_val_len, a_val_len, val);
+
+ if (a_key_len <= MAX_KEY_LEN) {
+ guestinfo_t *gi;
+
+ key = g_strndup(a_info_key, a_key_len);
+ gi = (guestinfo_t *)g_hash_table_lookup(s->guestinfo, key);
+ if (a_val_len <= MAX_VAL_LEN) {
+ if (gi) {
+ if (a_val_len > gi->val_max) {
+ char *new_val = g_try_malloc0(a_val_len);
+
+ if (!new_val) {
+ g_free(key);
+ return GUESTINFO_NO_MEMORY;
+ }
+ g_free(gi->val_data);
+ gi->val_max = a_val_len;
+ gi->val_data = new_val;
+ }
+ gi->val_len = a_val_len;
+ memcpy(gi->val_data, val, a_val_len);
+ } else {
+ int new_val_len = a_val_len;
+
+ if (new_val_len < MIN_VAL_LEN) {
+ new_val_len = MIN_VAL_LEN;
+ }
+ if (g_hash_table_size(s->guestinfo) >= MAX_NUM_KEY) {
+ g_free(key);
+ return GUESTINFO_TOOMANYKEYS;
+ }
+ gi = g_try_malloc(sizeof(guestinfo_t));
+ if (!gi) {
+ g_free(key);
+ return GUESTINFO_NO_MEMORY;
+ }
+ gi->val_data = g_try_malloc0(new_val_len);
+ if (!gi->val_data) {
+ g_free(gi);
+ g_free(key);
+ return GUESTINFO_NO_MEMORY;
+ }
+ gi->val_len = a_val_len;
+ gi->val_max = new_val_len;
+ memcpy(gi->val_data, val, a_val_len);
+ g_hash_table_insert(s->guestinfo, key, gi);
+ key = NULL; /* Do not free key below */
+ }
+ } else {
+ rc = GUESTINFO_VALTOOLONG;
+ }
+ } else {
+ rc = GUESTINFO_KEYTOOLONG;
+ }
+ g_free(key);
+ return rc;
+}
+
+static void process_send_payload(VMPortRpcState *s,
+ channel_t *c,
+ vregs *ur,
+ unsigned long now_time)
+{
+ /* Accumulate 4 (CHAR_PER_CALL) bytes of paload into send_buf
+ * using offset */
+ if (c->ctl.send_idx < c->ctl.send_buf_max) {
+ c->send.words[c->ctl.send_idx] = ur->ebx;
+ }
+
+ c->ctl.send_idx++;
+ ur->ecx = (MESSAGE_STATUS_SUCCESS << 16) | (ur->ecx & 0xffff);
+
+ if (c->ctl.send_idx * CHAR_PER_CALL >= c->ctl.send_len) {
+
+ /* We are done accumulating so handle the command */
+
+ if (c->ctl.send_idx <= c->ctl.send_buf_max) {
+ c->send.words[c->ctl.send_idx] = 0;
+ }
+#ifdef VMPORT_RPC_DEBUG
+ {
+ char prefix[30];
+
+ snprintf(prefix, sizeof(prefix),
+ "VMware RECV %d (%d) ",
+ c->ctl.chan_id, c->ctl.recv_read);
+ prefix[sizeof(prefix) - 1] = 0;
+ vmport_rpc_safe_print(s, MIN(c->ctl.send_len,
+ c->ctl.send_buf_max * CHAR_PER_CALL),
+ c->send.bytes);
+ trace_vmport_rpc_recv_normal(prefix, s->end, c->ctl.send_len,
+ s->last, sizeof(s->out), s->out);
+ }
+#endif
+ if (c->ctl.proto_num == VMWARE_PROTO_FROM_GUEST) {
+ /*
+ * Eaxmples of messages:
+ *
+ * log toolbox: Version: build-341836
+ * SetGuestInfo 4 build-341836
+ * info-get guestinfo.ip
+ * info-set guestinfo.ip joe
+ *
+ */
+
+ char *build = NULL;
+ char *info_key = NULL;
+ char *ret_msg = (char *)"1 ";
+ char ret_buffer[2 + MAX_VAL_LEN + 2];
+ unsigned int ret_len = strlen(ret_msg) + 1;
+
+ if (strncmp(c->send.bytes, "log toolbox: Version: build-",
+ strlen("log toolbox: Version: build-")) == 0) {
+ build =
+ c->send.bytes + strlen("log toolbox: Version: build-");
+ } else if (strncmp(c->send.bytes, "SetGuestInfo 4 build-",
+ strlen("SetGuestInfo 4 build-")) == 0) {
+ build = c->send.bytes + strlen("SetGuestInfo 4 build-");
+ } else if (strncmp(c->send.bytes, "info-get guestinfo.",
+ strlen("info-get guestinfo.")) == 0) {
+ unsigned int a_key_len =
+ c->ctl.send_len - strlen("info-get guestinfo.");
+ int rc;
+
+ info_key = c->send.bytes + strlen("info-get guestinfo.");
+ if (a_key_len <= MAX_KEY_LEN) {
+
+ rc = get_guestinfo(s, info_key, a_key_len,
+ ret_buffer, sizeof(ret_buffer));
+ if (rc == GUESTINFO_NOTFOUND) {
+ ret_msg = (char *)"0 No value found";
+ ret_len = strlen(ret_msg) + 1;
+ } else {
+ ret_msg = ret_buffer;
+ ret_len = rc;
+ }
+ } else {
+ ret_msg = (char *)"0 Key is too long";
+ ret_len = strlen(ret_msg) + 1;
+ }
+ } else if (strncmp(c->send.bytes, "info-set guestinfo.",
+ strlen("info-set guestinfo.")) == 0) {
+ char *val;
+ unsigned int rest_len =
+ c->ctl.send_len - strlen("info-set guestinfo.");
+
+ info_key = c->send.bytes + strlen("info-set guestinfo.");
+ val = strstr(info_key, " ");
+ if (val) {
+ unsigned int a_key_len = val - info_key;
+ unsigned int a_val_len = rest_len - a_key_len - 1;
+ int rc;
+
+ val++;
+ rc = set_guestinfo(s, a_key_len, a_val_len,
+ info_key, val);
+ switch (rc) {
+ case 0:
+ ret_msg = (char *)"1 ";
+ break;
+ case GUESTINFO_VALTOOLONG:
+ ret_msg = (char *)"0 Value too long";
+ break;
+ case GUESTINFO_KEYTOOLONG:
+ ret_msg = (char *)"0 Key is too long";
+ break;
+ case GUESTINFO_TOOMANYKEYS:
+ ret_msg = (char *)"0 Too many keys";
+ break;
+ case GUESTINFO_NO_MEMORY:
+ ret_msg = (char *)"0 Out of memory";
+ break;
+ }
+ ret_len = strlen(ret_msg) + 1;
+ } else {
+ ret_msg =
+ (char *)"0 Two and exactly two arguments expected";
+ ret_len = strlen(ret_msg) + 1;
+ }
+ }
+
+ vmport_rpc_send(s, c, ret_msg, ret_len);
+
+ if (build) {
+ s->build_number_value = strtol(build, NULL, 10);
+ s->build_number_time = now_time;
+ }
+ } else {
+ unsigned int my_bkt = c->ctl.recv_read - 1;
+ bucket_t *b;
+
+ if (my_bkt >= MAX_BKTS) {
+ my_bkt = MAX_BKTS - 1;
+ }
+ b = &c->recv_bkt[my_bkt];
+ b->ctl.recv_len = 0;
+ }
+ }
+}
+
+static void process_recv_size(VMPortRpcState *s, channel_t *c,
+ vregs *ur)
+{
+ bucket_t *b;
+ int16_t recv_len;
+
+ b = &c->recv_bkt[c->ctl.recv_read];
+ recv_len = b->ctl.recv_len;
+ if (recv_len) {
+ ur->ecx = ((MESSAGE_STATUS_DORECV | MESSAGE_STATUS_SUCCESS) << 16) |
+ (ur->ecx & 0xffff);
+ ur->edx = (ur->edx & 0xffff) | (MESSAGE_TYPE_SENDSIZE << 16);
+ ur->ebx = recv_len;
+ } else {
+ ur->ecx = (MESSAGE_STATUS_SUCCESS << 16) | (ur->ecx & 0xffff);
+ }
+ trace_vmport_detail_rpc_process_recv_size(c->ctl.chan_id, recv_len);
+}
+
+static void process_recv_payload(VMPortRpcState *s,
+ channel_t *c,
+ vregs *ur)
+{
+ bucket_t *b;
+
+ b = &c->recv_bkt[c->ctl.recv_read];
+ if (b->ctl.recv_idx < b->ctl.recv_buf_max) {
+ ur->ebx = b->recv.words[b->ctl.recv_idx++];
+ } else {
+ ur->ebx = 0;
+ }
+ ur->ecx = (MESSAGE_STATUS_SUCCESS << 16) | (ur->ecx & 0xffff);
+ ur->edx = (ur->edx & 0xffff) | (MESSAGE_TYPE_SENDPAYLOAD << 16);
+}
+
+static void process_recv_status(VMPortRpcState *s,
+ channel_t *c,
+ vregs *ur)
+{
+ ur->ecx = (MESSAGE_STATUS_SUCCESS << 16) | (ur->ecx & 0xffff);
+ c->ctl.recv_read++;
+ if (c->ctl.recv_read >= MAX_BKTS) {
+ c->ctl.recv_read = 0;
+ }
+}
+
+static void process_close(VMPortRpcState *s, channel_t *c,
+ vregs *ur)
+{
+ /* Return channel to free pool */
+ c->ctl.proto_num = 0;
+ ur->ecx = (MESSAGE_STATUS_SUCCESS << 16) | (ur->ecx & 0xffff);
+ trace_vmport_rpc_process_close(c->ctl.chan_id);
+}
+
+static void process_packet(VMPortRpcState *s, channel_t *c,
+ vregs *ur, unsigned int sub_cmd,
+ unsigned long now_time)
+{
+ c->ctl.active_time = now_time;
+
+ switch (sub_cmd) {
+ case MESSAGE_TYPE_SENDSIZE:
+ process_send_size(s, c, ur);
+ break;
+
+ case MESSAGE_TYPE_SENDPAYLOAD:
+ process_send_payload(s, c, ur, now_time);
+ break;
+
+ case MESSAGE_TYPE_RECVSIZE:
+ process_recv_size(s, c, ur);
+ break;
+
+ case MESSAGE_TYPE_RECVPAYLOAD:
+ process_recv_payload(s, c, ur);
+ break;
+
+ case MESSAGE_TYPE_RECVSTATUS:
+ process_recv_status(s, c, ur);
+ break;
+
+ case MESSAGE_TYPE_CLOSE:
+ process_close(s, c, ur);
+ break;
+
+ default:
+ ur->ecx = 0;
+ break;
+ }
+}
+
+static void vmport_rpc(VMPortRpcState *s , vregs *ur)
+{
+ unsigned int sub_cmd = (ur->ecx >> 16) & 0xffff;
+ channel_t *c = NULL;
+ uint16_t msg_id;
+ uint32_t msg_cookie;
+ unsigned long now_time = get_clock() / 1000000000LL;
+ long delta = now_time - s->ping_time;
+
+ trace_vmport_detail_rpc_start(sub_cmd, ur->eax, ur->ebx, ur->ecx, ur->edx,
+ ur->esi, ur->edi);
+
+ if (!s) {
+ return;
+ }
+ if (delta > s->reset_time) {
+ trace_vmport_rpc_ping(delta);
+ vmport_rpc_ctrl_send(s, (char *)"reset");
+ }
+ vmport_rpc_sweep(s, now_time);
+ do {
+ /* Check to see if a new open request is happening... */
+ if (MESSAGE_TYPE_OPEN == sub_cmd) {
+ c = vmport_rpc_new_chan(s, now_time);
+ if (!c) {
+ trace_vmport_rpc_nofree();
+ break;
+ }
+
+ /* Attach the apropriate protocol the the channel */
+ c->ctl.proto_num = ur->ebx & ~GUESTMSG_FLAG_COOKIE;
+ ur->ecx = (MESSAGE_STATUS_SUCCESS << 16) | (ur->ecx & 0xffff);
+ ur->edx = (ur->edx & 0xffff) | (c->ctl.chan_id << 16);
+ ur->edi = c->ctl.cookie & 0xffff;
+ ur->esi = (c->ctl.cookie >> 16) & 0xffff;
+ trace_vmport_rpc_process_open(c->ctl.chan_id, c->ctl.proto_num);
+ if (c->ctl.proto_num == VMWARE_PROTO_TO_GUEST) {
+ vmport_rpc_send(s, c, "reset", strlen("reset") + 1);
+ }
+ break;
+ }
+
+ msg_id = (ur->edx >> 16) & 0xffff;
+ msg_cookie = (ur->edi & 0xffff) | (ur->esi << 16);
+ if (msg_id >= GUESTMSG_MAX_CHANNEL) {
+ trace_vmport_rpc_bad_chan(msg_id, GUESTMSG_MAX_CHANNEL);
+ break;
+ }
+ c = &s->chans[msg_id];
+ if (!c->ctl.proto_num) {
+ trace_vmport_rpc_chan_not_open(msg_id);
+ break;
+ }
+
+ /* We check the cookie here since it's possible that the
+ * connection timed out on us and another channel was opened
+ * if this happens, return error and the vmware tool will
+ * need to reopen the connection
+ */
+ if (msg_cookie != c->ctl.cookie) {
+ trace_vmport_rpc_bad_cookie(msg_cookie, c->ctl.cookie);
+ break;
+ }
+ process_packet(s, c, ur, sub_cmd, now_time);
+ } while (0);
+
+ if (!c) {
+ ur->ecx &= 0xffff;
+ }
+
+ trace_vmport_detail_rpc_end(sub_cmd, ur->eax, ur->ebx, ur->ecx, ur->edx,
+ ur->esi, ur->edi);
+}
+
static uint32_t vmport_rpc_ioport_read(void *opaque, uint32_t addr)
{
VMPortRpcState *s = opaque;
@@ -71,7 +842,7 @@ static uint32_t vmport_rpc_ioport_read(void *opaque, uint32_t addr)
vmmouse_get_data(ur.data);
- s->build_number_time++;
+ vmport_rpc(s, &ur.regs);
vmmouse_set_data(ur.data);
return ur.data[0];
@@ -79,11 +850,32 @@ static uint32_t vmport_rpc_ioport_read(void *opaque, uint32_t addr)
static void vmport_rpc_reset(DeviceState *d)
{
+ unsigned int i;
VMPortRpcState *s = VMPORT_RPC(d);
s->reset_time = 14;
s->build_number_value = 0;
s->build_number_time = 0;
+ for (i = 0; i < GUESTMSG_MAX_CHANNEL; ++i) {
+ unsigned int j;
+ channel_t *c = &s->chans[i];
+
+ for (j = 0; j < MAX_BKTS; ++j) {
+ bucket_t *b = &c->recv_bkt[j];
+
+ g_free(b->recv.words);
+ }
+ g_free(c->send.words);
+ memset(c, 0, sizeof(*c));
+ }
+ g_hash_table_remove_all(s->guestinfo);
+}
+
+static void free_guestinfo(gpointer opaque)
+{
+ guestinfo_t *gi = (guestinfo_t *)opaque;
+ g_free(gi->val_data);
+ g_free(gi);
}
static void vmport_rpc_realize(DeviceState *dev, Error **errp)
@@ -91,6 +883,8 @@ static void vmport_rpc_realize(DeviceState *dev, Error **errp)
VMPortRpcState *s = VMPORT_RPC(dev);
vmport_register(VMPORT_RPC_COMMAND, vmport_rpc_ioport_read, s);
+ s->guestinfo = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
+ free_guestinfo);
}
static Property vmport_rpc_properties[] = {
diff --git a/trace-events b/trace-events
index e229aac..cfb3ffa 100644
--- a/trace-events
+++ b/trace-events
@@ -887,6 +887,28 @@ pc87312_info_serial(int n, uint32_t base, uint32_t irq) "id=%d, base 0x%x, irq %
vmport_ioport_ignored(uint64_t addr, uint32_t size, void *cs) "addr=%#" PRIx64 " size=%u cs=%p"
vmport_ioport_unknown_command(unsigned char command) "%x"
+# hw/misc/vmport_rpc.c
+vmport_detail_rpc_start(int sub_cmd, unsigned int ax, unsigned int bx, unsigned int cx, unsigned int dx, unsigned int si, unsigned int di) "sub_cmd=0x%x ax=%x bx=%x cx=%x dx=%x si=%x di=%x"
+vmport_detail_rpc_end(int sub_cmd, unsigned int ax, unsigned int bx, unsigned int cx, unsigned int dx, unsigned int si, unsigned int di) "sub_cmd=0x%x ax=%x bx=%x cx=%x dx=%x si=%x di=%x"
+vmport_rpc_send_skip(char *prefix, unsigned int end, unsigned int len, unsigned int k, size_t out_size, char *out) "%s%d[%d,%d,%zu]%s"
+vmport_rpc_send_normal(char *prefix, unsigned int end, unsigned int len, unsigned int k, size_t out_size, char *out) "%s%d[%d,%d,%zu]%s"
+vmport_rpc_recv_normal(char *prefix, unsigned int end, unsigned int len, unsigned int k, size_t out_size, char *out) "%s%d[%d,%d,%zu]%s"
+vmport_rpc_get_guestinfo(short klen, int kmaxlen, char *key) "klen=%d key=%.*s"
+vmport_rpc_get_guestinfo_found(short vlen, int vmaxlen, char *val) "vlen=%d val=%.*s"
+vmport_rpc_set_guestinfo(short klen, int kmaxlen, const char *key, short vlen, int vmaxlen, char *val) "klen=%d key=%.*s vlen=%d val=%.*s"
+vmport_rpc_set_guestinfo_bad(int i, int used_guestinfo) "i=%d not allocated, but used_guestinfo=%d"
+vmport_rpc_send_big(int len, size_t maxlen) "recv_len=%d >= %zd"
+vmport_rpc_sweep(int chan_id, long delta) "flush %d. delta=%ld"
+vmport_rpc_ping(long delta) "delta=%ld"
+vmport_rpc_nofree(void) "failed to find a free channel"
+vmport_rpc_bad_chan(int chan_id, int max_chan_id) "chan_id=%d >= %d"
+vmport_rpc_chan_not_open(int chan_id) "chan_id=%d"
+vmport_rpc_bad_cookie(int msg_cookie, int ctl_cookie) "%x vs %x"
+vmport_detail_rpc_process_send_size(int chan_id, int send_len) "chan %d is %d"
+vmport_detail_rpc_process_recv_size(int chan_id, int send_len) "chan %d is %d"
+vmport_rpc_process_close(int chan_id) "chan %d"
+vmport_rpc_process_open(int chan_id, int proto_num) "chan %d proto 0x%x"
+
# hw/scsi/vmw_pvscsi.c
pvscsi_ring_init_data(uint32_t txr_len_log2, uint32_t rxr_len_log2) "TX/RX rings logarithms set to %d/%d"
pvscsi_ring_init_msg(uint32_t len_log2) "MSG ring logarithm set to %d"
--
1.8.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Qemu-devel] [PATCH v7 6/9] vmport_rpc: Add QMP access to vmport_rpc object.
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
` (4 preceding siblings ...)
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 5/9] vmport_rpc: Add limited support of VMware's hyper-call rpc Don Slutz
@ 2015-06-12 14:05 ` Don Slutz
2015-06-15 15:53 ` Eric Blake
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 7/9] vmport_rpc: Add migration Don Slutz
` (4 subsequent siblings)
10 siblings, 1 reply; 44+ messages in thread
From: Don Slutz @ 2015-06-12 14:05 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Don Slutz, Luiz Capitulino,
Don Slutz, Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
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>
CC: Don Slutz <don.slutz@gmail.com>
---
v7:
Delay call on g_strndup till after key length check.
Why not assert(find) instead of leaving it to the comment?
Switch to assert.
Is it worth marking arg const here and in the VMPortRpcFind struct
Switch to const.
I'd rather abort() if someone compiled with -NDEBUG
Done.
Still mismatches on ---- line length (several sites).
Done
hw/misc/vmport_rpc.c | 291 ++++++++++++++++++++++++++++++++++++++++++++++++++-
monitor.c | 24 +++++
qapi-schema.json | 101 ++++++++++++++++++
qmp-commands.hx | 119 +++++++++++++++++++++
4 files changed, 533 insertions(+), 2 deletions(-)
diff --git a/hw/misc/vmport_rpc.c b/hw/misc/vmport_rpc.c
index e2d5c36..e122bad 100644
--- a/hw/misc/vmport_rpc.c
+++ b/hw/misc/vmport_rpc.c
@@ -178,6 +178,19 @@ typedef struct VMPortRpcState {
#endif
} VMPortRpcState;
+typedef int FindVMPortRpcDeviceFunc(VMPortRpcState *s, const void *arg);
+
+/*
+ * The input and output argument that find_VMPortRpc_device() uses
+ * to do it's work.
+ */
+typedef struct VMPortRpcFind {
+ const void *arg;
+ FindVMPortRpcDeviceFunc *func;
+ int found;
+ int rc;
+} VMPortRpcFind;
+
/* Basic request types */
typedef enum {
MESSAGE_TYPE_OPEN,
@@ -202,6 +215,56 @@ typedef struct {
uint32_t edi;
} vregs;
+/*
+ * Run func() for every VMPortRpc device, traverse the tree for
+ * everything else.
+ */
+static int find_VMPortRpc_device(Object *obj, void *opaque)
+{
+ VMPortRpcFind *find = opaque;
+ Object *dev;
+ VMPortRpcState *s;
+
+ assert(find);
+ 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,
+ const 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
@@ -329,7 +392,7 @@ static int vmport_rpc_send(VMPortRpcState *s, channel_t *c,
return rc;
}
-static int vmport_rpc_ctrl_send(VMPortRpcState *s, char *msg)
+static int vmport_rpc_ctrl_send(VMPortRpcState *s, const char *msg)
{
int rc = SEND_NOT_OPEN;
unsigned int i;
@@ -457,6 +520,29 @@ static int get_guestinfo(VMPortRpcState *s,
return GUESTINFO_NOTFOUND;
}
+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)
+{
+ guestinfo_t *gi = NULL;
+
+ if (a_key_len <= MAX_KEY_LEN) {
+ gpointer key = g_strndup(a_info_key, a_key_len);
+
+ gi = (guestinfo_t *)g_hash_table_lookup(s->guestinfo, key);
+ g_free(key);
+ } else {
+ return GUESTINFO_KEYTOOLONG;
+ }
+ 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, const char *a_info_key,
char *val)
@@ -775,7 +861,7 @@ static void vmport_rpc(VMPortRpcState *s , vregs *ur)
}
if (delta > s->reset_time) {
trace_vmport_rpc_ping(delta);
- vmport_rpc_ctrl_send(s, (char *)"reset");
+ vmport_rpc_ctrl_send(s, "reset");
}
vmport_rpc_sweep(s, now_time);
do {
@@ -848,6 +934,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, const 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,
+ "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;
+ }
+ 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, const void *arg)
+{
+ const 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, const void *arg)
+{
+ keyValue *key_value = (void *)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, const void *arg)
+{
+ g_hash_table_foreach(s->guestinfo, vmport_rpc_find_list_one, (gpointer)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 9afee7b..50bc610 100644
--- a/monitor.c
+++ b/monitor.c
@@ -5367,4 +5367,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 6e17a5c..6646f2b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1429,6 +1429,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 867a21f..2035dcf 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -490,6 +490,125 @@ 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 value
+ contains valid UTF-8 text. 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,
--
1.8.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Qemu-devel] [PATCH v7 7/9] vmport_rpc: Add migration
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
` (5 preceding siblings ...)
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 6/9] vmport_rpc: Add QMP access to vmport_rpc object Don Slutz
@ 2015-06-12 14:05 ` Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 8/9] vmport: Add VMware all ring hack Don Slutz
` (3 subsequent siblings)
10 siblings, 0 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-12 14:05 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Don Slutz, Luiz Capitulino,
Don Slutz, Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
Signed-off-by: Don Slutz <dslutz@verizon.com>
CC: Don Slutz <don.slutz@gmail.com>
---
hw/misc/vmport_rpc.c | 250 +++++++++++++++++++++++++++++++++++++++++++++++++++
trace-events | 2 +
2 files changed, 252 insertions(+)
diff --git a/hw/misc/vmport_rpc.c b/hw/misc/vmport_rpc.c
index e122bad..4984193 100644
--- a/hw/misc/vmport_rpc.c
+++ b/hw/misc/vmport_rpc.c
@@ -171,6 +171,14 @@ typedef struct VMPortRpcState {
uint32_t open_cookie;
channel_t chans[GUESTMSG_MAX_CHANNEL];
GHashTable *guestinfo;
+ /* Temporary cache for migration purposes */
+ int32_t mig_chan_num;
+ int32_t mig_bucket_num;
+ uint32_t mig_guestinfo_size;
+ uint32_t mig_guestinfo_off;
+ uint8_t *mig_guestinfo_buf;
+ channel_control_t *mig_chans;
+ bucket_control_t *mig_buckets;
#ifdef VMPORT_RPC_DEBUG
unsigned int end;
unsigned int last;
@@ -1183,6 +1191,247 @@ static Property vmport_rpc_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static const VMStateDescription vmstate_vmport_rpc_chan = {
+ .name = "vmport_rpc/chan",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField [])
+ {
+ VMSTATE_UINT64(active_time, channel_control_t),
+ VMSTATE_UINT32(chan_id, channel_control_t),
+ VMSTATE_UINT32(cookie, channel_control_t),
+ VMSTATE_UINT32(proto_num, channel_control_t),
+ VMSTATE_UINT16(send_len, channel_control_t),
+ VMSTATE_UINT16(send_idx, channel_control_t),
+ VMSTATE_UINT16(send_buf_max, channel_control_t),
+ VMSTATE_UINT8(recv_read, channel_control_t),
+ VMSTATE_UINT8(recv_write, channel_control_t),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
+static const VMStateDescription vmstate_vmport_rpc_bucket = {
+ .name = "vmport_rpc/bucket",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField [])
+ {
+ VMSTATE_UINT16(recv_len, bucket_control_t),
+ VMSTATE_UINT16(recv_idx, bucket_control_t),
+ VMSTATE_UINT16(recv_buf_max, bucket_control_t),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
+static void vmport_rpc_size_mig_guestinfo(gpointer key, gpointer value,
+ gpointer opaque)
+{
+ VMPortRpcState *s = opaque;
+ unsigned int key_len = strlen(key) + 1;
+ guestinfo_t *gi = value;
+
+ s->mig_guestinfo_size += 1 + key_len + 4 + gi->val_max;
+}
+
+static void vmport_rpc_fill_mig_guestinfo(gpointer key, gpointer value,
+ gpointer opaque)
+{
+ VMPortRpcState *s = opaque;
+ unsigned int key_len = strlen(key) + 1;
+ guestinfo_t *gi = value;
+
+ assert(gi->val_len <= gi->val_max);
+ trace_vmport_rpc_fill_mig_guestinfo(key_len, key_len, key, gi->val_len,
+ gi->val_len, gi->val_data);
+ s->mig_guestinfo_buf[s->mig_guestinfo_off++] = key_len;
+ memcpy(s->mig_guestinfo_buf + s->mig_guestinfo_off, key, key_len);
+ s->mig_guestinfo_off += key_len;
+ s->mig_guestinfo_buf[s->mig_guestinfo_off++] = gi->val_len >> 8;
+ s->mig_guestinfo_buf[s->mig_guestinfo_off++] = gi->val_len;
+ s->mig_guestinfo_buf[s->mig_guestinfo_off++] = gi->val_max >> 8;
+ s->mig_guestinfo_buf[s->mig_guestinfo_off++] = gi->val_max;
+ memcpy(s->mig_guestinfo_buf + s->mig_guestinfo_off, gi->val_data,
+ gi->val_max);
+ s->mig_guestinfo_off += gi->val_max;
+}
+
+static int vmport_rpc_pre_load(void *opaque)
+{
+ VMPortRpcState *s = opaque;
+
+ g_free(s->mig_guestinfo_buf);
+ s->mig_guestinfo_buf = NULL;
+ s->mig_guestinfo_size = 0;
+ s->mig_guestinfo_off = 0;
+ g_free(s->mig_chans);
+ s->mig_chans = NULL;
+ s->mig_chan_num = 0;
+ g_free(s->mig_buckets);
+ s->mig_buckets = NULL;
+ s->mig_bucket_num = 0;
+
+ return 0;
+}
+
+static void vmport_rpc_pre_save(void *opaque)
+{
+ VMPortRpcState *s = opaque;
+ unsigned int i;
+ unsigned int mig_chan_idx = 0;
+ unsigned int mig_bucket_idx = 0;
+
+ (void)vmport_rpc_pre_load(opaque);
+ for (i = 0; i < GUESTMSG_MAX_CHANNEL; ++i) {
+ channel_t *c = &s->chans[i];
+
+ if (c->ctl.proto_num) {
+ unsigned int j;
+
+ s->mig_chan_num++;
+ for (j = 0; j < MAX_BKTS; ++j) {
+ bucket_t *b = &c->recv_bkt[j];
+
+ s->mig_bucket_num++;
+ s->mig_guestinfo_size +=
+ (b->ctl.recv_buf_max + 1) * CHAR_PER_CALL;
+ }
+ s->mig_guestinfo_size += (c->ctl.send_buf_max + 1) * CHAR_PER_CALL;
+ }
+ }
+ g_hash_table_foreach(s->guestinfo, vmport_rpc_size_mig_guestinfo, s);
+ s->mig_guestinfo_size++;
+ s->mig_guestinfo_buf = g_malloc(s->mig_guestinfo_size);
+ s->mig_chans = g_malloc(s->mig_chan_num * sizeof(channel_control_t));
+ s->mig_buckets = g_malloc(s->mig_bucket_num * sizeof(bucket_control_t));
+
+ for (i = 0; i < GUESTMSG_MAX_CHANNEL; ++i) {
+ channel_t *c = &s->chans[i];
+
+ if (c->ctl.proto_num) {
+ unsigned int j;
+ channel_control_t *cm = s->mig_chans + mig_chan_idx++;
+ unsigned int send_chars = (c->ctl.send_buf_max + 1) * CHAR_PER_CALL;
+
+ *cm = c->ctl;
+ for (j = 0; j < MAX_BKTS; ++j) {
+ bucket_t *b = &c->recv_bkt[j];
+ bucket_control_t *bm = s->mig_buckets + mig_bucket_idx++;
+ unsigned int recv_chars =
+ (b->ctl.recv_buf_max + 1) * CHAR_PER_CALL;
+
+ *bm = b->ctl;
+ memcpy(s->mig_guestinfo_buf + s->mig_guestinfo_off,
+ b->recv.words, recv_chars);
+ s->mig_guestinfo_off += recv_chars;
+ }
+ memcpy(s->mig_guestinfo_buf + s->mig_guestinfo_off,
+ c->send.words, send_chars);
+ s->mig_guestinfo_off += send_chars;
+ }
+ }
+ g_hash_table_foreach(s->guestinfo, vmport_rpc_fill_mig_guestinfo, s);
+ s->mig_guestinfo_buf[s->mig_guestinfo_off++] = 0;
+ assert(s->mig_guestinfo_size == s->mig_guestinfo_off);
+ assert(s->mig_chan_num == mig_chan_idx);
+ assert(s->mig_bucket_num == mig_bucket_idx);
+}
+
+static int vmport_rpc_post_load(void *opaque, int version_id)
+{
+ VMPortRpcState *s = opaque;
+ unsigned int i;
+ unsigned int key_len;
+ unsigned int mig_bucket_idx = 0;
+
+ s->mig_guestinfo_off = 0;
+ for (i = 0; i < s->mig_chan_num; ++i) {
+ channel_control_t *cm = s->mig_chans + i;
+ channel_t *c = &s->chans[cm->chan_id];
+ unsigned int j;
+ unsigned int send_chars;
+
+ c->ctl = *cm;
+ for (j = 0; j < MAX_BKTS; ++j) {
+ bucket_t *b = &c->recv_bkt[j];
+ bucket_control_t *bm = s->mig_buckets + mig_bucket_idx++;
+ unsigned int recv_chars;
+
+ b->ctl = *bm;
+ recv_chars = (b->ctl.recv_buf_max + 1) * CHAR_PER_CALL;
+ b->recv.words =
+ g_memdup(s->mig_guestinfo_buf + s->mig_guestinfo_off,
+ recv_chars);
+ s->mig_guestinfo_off += recv_chars;
+ }
+ send_chars = (c->ctl.send_buf_max + 1) * CHAR_PER_CALL;
+ c->send.words = g_memdup(s->mig_guestinfo_buf + s->mig_guestinfo_off,
+ send_chars);
+ s->mig_guestinfo_off += send_chars;
+ }
+ assert(s->mig_bucket_num == mig_bucket_idx);
+
+ do {
+ key_len = s->mig_guestinfo_buf[s->mig_guestinfo_off++];
+ if (key_len) {
+ gpointer key = g_memdup(s->mig_guestinfo_buf + s->mig_guestinfo_off,
+ key_len);
+ guestinfo_t *gi = g_malloc(sizeof(guestinfo_t));
+ unsigned int bhi, blow;
+
+ s->mig_guestinfo_off += key_len;
+ bhi = s->mig_guestinfo_buf[s->mig_guestinfo_off++];
+ blow = s->mig_guestinfo_buf[s->mig_guestinfo_off++];
+ gi->val_len = (bhi << 8) + blow;
+ bhi = s->mig_guestinfo_buf[s->mig_guestinfo_off++];
+ blow = s->mig_guestinfo_buf[s->mig_guestinfo_off++];
+ gi->val_max = (bhi << 8) + blow;
+ assert(gi->val_len <= gi->val_max);
+ gi->val_data = g_memdup(s->mig_guestinfo_buf +
+ s->mig_guestinfo_off,
+ gi->val_max);
+ s->mig_guestinfo_off += gi->val_max;
+ trace_vmport_rpc_post_load(key_len, key_len, key, gi->val_len,
+ gi->val_len, gi->val_data);
+ assert(!g_hash_table_lookup(s->guestinfo, key));
+ g_hash_table_insert(s->guestinfo, key, gi);
+ }
+ } while (key_len);
+ assert(s->mig_guestinfo_size == s->mig_guestinfo_off);
+
+ (void)vmport_rpc_pre_load(opaque);
+ return 0;
+}
+
+static const VMStateDescription vmstate_vmport_rpc = {
+ .name = "vmport_rpc",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .pre_save = vmport_rpc_pre_save,
+ .pre_load = vmport_rpc_pre_load,
+ .post_load = vmport_rpc_post_load,
+ .fields = (VMStateField[])
+ {
+ VMSTATE_UINT64(reset_time, VMPortRpcState),
+ VMSTATE_UINT64(build_number_value, VMPortRpcState),
+ VMSTATE_UINT64(build_number_time, VMPortRpcState),
+ VMSTATE_UINT64(ping_time, VMPortRpcState),
+ VMSTATE_UINT32(open_cookie, VMPortRpcState),
+ VMSTATE_INT32(mig_chan_num, VMPortRpcState),
+ VMSTATE_STRUCT_VARRAY_ALLOC(mig_chans, VMPortRpcState, mig_chan_num,
+ 0, vmstate_vmport_rpc_chan,
+ channel_control_t),
+ VMSTATE_INT32(mig_bucket_num, VMPortRpcState),
+ VMSTATE_STRUCT_VARRAY_ALLOC(mig_buckets, VMPortRpcState,
+ mig_bucket_num, 0,
+ vmstate_vmport_rpc_bucket,
+ bucket_control_t),
+ VMSTATE_UINT32(mig_guestinfo_size, VMPortRpcState),
+ VMSTATE_VBUFFER_ALLOC_UINT32(mig_guestinfo_buf, VMPortRpcState, 1,
+ NULL, 0, mig_guestinfo_size),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
static void vmport_rpc_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1190,6 +1439,7 @@ static void vmport_rpc_class_init(ObjectClass *klass, void *data)
dc->realize = vmport_rpc_realize;
dc->reset = vmport_rpc_reset;
dc->props = vmport_rpc_properties;
+ dc->vmsd = &vmstate_vmport_rpc;
}
static const TypeInfo vmport_rpc_info = {
diff --git a/trace-events b/trace-events
index cfb3ffa..fbfcd86 100644
--- a/trace-events
+++ b/trace-events
@@ -908,6 +908,8 @@ vmport_detail_rpc_process_send_size(int chan_id, int send_len) "chan %d is %d"
vmport_detail_rpc_process_recv_size(int chan_id, int send_len) "chan %d is %d"
vmport_rpc_process_close(int chan_id) "chan %d"
vmport_rpc_process_open(int chan_id, int proto_num) "chan %d proto 0x%x"
+vmport_rpc_fill_mig_guestinfo(short klen, int kmaxlen, char *key, short vlen, int vmaxlen, char *val) "klen=%d key=%.*s vlen=%d val=%.*s"
+vmport_rpc_post_load(short klen, int kmaxlen, char *key, short vlen, int vmaxlen, char *val) "klen=%d key=%.*s vlen=%d val=%.*s"
# hw/scsi/vmw_pvscsi.c
pvscsi_ring_init_data(uint32_t txr_len_log2, uint32_t rxr_len_log2) "TX/RX rings logarithms set to %d/%d"
--
1.8.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Qemu-devel] [PATCH v7 8/9] vmport: Add VMware all ring hack
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
` (6 preceding siblings ...)
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 7/9] vmport_rpc: Add migration Don Slutz
@ 2015-06-12 14:05 ` Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 9/9] MAINTAINERS: add VMware port Don Slutz
` (2 subsequent siblings)
10 siblings, 0 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-12 14:05 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Don Slutz, Luiz Capitulino,
Don Slutz, Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
This is done by adding a new machine property vmware-port-ring3 that
needs to be enabled to have any effect. It only effects accel=tcg
mode. It is needed if you want to use VMware tools in accel=tcg
mode.
Signed-off-by: Don Slutz <dslutz@verizon.com>
CC: Don Slutz <don.slutz@gmail.com>
---
hw/i386/pc.c | 26 +++++++++++++++++++++++++-
hw/i386/pc_piix.c | 2 +-
hw/i386/pc_q35.c | 2 +-
include/hw/i386/pc.h | 6 +++++-
target-i386/cpu-qom.h | 3 +++
target-i386/seg_helper.c | 9 +++++++++
6 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 48711c6..40d4e33 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1042,7 +1042,9 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
object_unref(OBJECT(cpu));
}
-void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
+/* vmware_port_ring3 true says enable VMware port access in ring3. */
+void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge,
+ bool vmware_port_ring3)
{
int i;
X86CPU *cpu = NULL;
@@ -1073,6 +1075,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
error_report_err(error);
exit(1);
}
+ cpu->allow_vmport_ring3 = vmware_port_ring3;
object_unref(OBJECT(cpu));
}
@@ -1841,6 +1844,21 @@ static bool pc_machine_get_aligned_dimm(Object *obj, Error **errp)
return pcms->enforce_aligned_dimm;
}
+static bool pc_machine_get_vmware_port_ring3(Object *obj, Error **errp)
+{
+ PCMachineState *pcms = PC_MACHINE(obj);
+
+ return pcms->vmware_port_ring3;
+}
+
+static void pc_machine_set_vmware_port_ring3(Object *obj, bool value,
+ Error **errp)
+{
+ PCMachineState *pcms = PC_MACHINE(obj);
+
+ pcms->vmware_port_ring3 = value;
+}
+
static void pc_machine_initfn(Object *obj)
{
PCMachineState *pcms = PC_MACHINE(obj);
@@ -1871,6 +1889,12 @@ static void pc_machine_initfn(Object *obj)
object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALIGNED_DIMM,
pc_machine_get_aligned_dimm,
NULL, NULL);
+
+ pcms->vmware_port_ring3 = false;
+ object_property_add_bool(obj, PC_MACHINE_VMWARE_PORT_RING3,
+ pc_machine_get_vmware_port_ring3,
+ pc_machine_set_vmware_port_ring3,
+ NULL);
}
static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e142f75..890c45e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -146,7 +146,7 @@ static void pc_init1(MachineState *machine)
object_property_add_child(qdev_get_machine(), "icc-bridge",
OBJECT(icc_bridge), NULL);
- pc_cpus_init(machine->cpu_model, icc_bridge);
+ pc_cpus_init(machine->cpu_model, icc_bridge, pc_machine->vmware_port_ring3);
if (kvm_enabled() && kvmclock_enabled) {
kvmclock_create();
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index b68263d..3883872 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -137,7 +137,7 @@ static void pc_q35_init(MachineState *machine)
object_property_add_child(qdev_get_machine(), "icc-bridge",
OBJECT(icc_bridge), NULL);
- pc_cpus_init(machine->cpu_model, icc_bridge);
+ pc_cpus_init(machine->cpu_model, icc_bridge, pc_machine->vmware_port_ring3);
pc_acpi_init("q35-acpi-dsdt.aml");
kvmclock_create();
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 86c5651..9e648d2 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -40,6 +40,7 @@ struct PCMachineState {
uint64_t max_ram_below_4g;
OnOffAuto vmport;
+ bool vmware_port_ring3;
bool enforce_aligned_dimm;
};
@@ -48,6 +49,7 @@ struct PCMachineState {
#define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
#define PC_MACHINE_VMPORT "vmport"
#define PC_MACHINE_ENFORCE_ALIGNED_DIMM "enforce-aligned-dimm"
+#define PC_MACHINE_VMWARE_PORT_RING3 "vmware-port-ring3"
/**
* PCMachineClass:
@@ -161,7 +163,9 @@ extern int fd_bootchk;
void pc_register_ferr_irq(qemu_irq irq);
void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
-void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
+/* vmware_port_ring3 true says enable VMware port access in ring3. */
+void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge,
+ bool vmware_port_ring3);
void pc_hot_add_cpu(const int64_t id, Error **errp);
void pc_acpi_init(const char *default_dsdt);
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 7a4fddd..ccbc5bb 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -109,6 +109,9 @@ typedef struct X86CPU {
*/
bool enable_pmu;
+ /* allow_vmport_ring3 true says enable VMware port access in ring3 */
+ bool allow_vmport_ring3;
+
/* in order to simplify APIC support, we leave this pointer to the
user */
struct DeviceState *apic_state;
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 8a4271e..2ddb84f 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -2566,6 +2566,15 @@ static inline void check_io(CPUX86State *env, int addr, int size)
{
int io_offset, val, mask;
+ /* vmport hack: skip iopl checking for VMware port 0x5658 (see
+ * vmport_realizefn()) */
+ if (addr == 0x5658) {
+ X86CPU *cpu = x86_env_get_cpu(env);
+ if (cpu->allow_vmport_ring3) {
+ return;
+ }
+ }
+
/* TSS must be a valid 32 bit one */
if (!(env->tr.flags & DESC_P_MASK) ||
((env->tr.flags >> DESC_TYPE_SHIFT) & 0xf) != 9 ||
--
1.8.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Qemu-devel] [PATCH v7 9/9] MAINTAINERS: add VMware port
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
` (7 preceding siblings ...)
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 8/9] vmport: Add VMware all ring hack Don Slutz
@ 2015-06-12 14:05 ` Don Slutz
2015-06-17 13:44 ` [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Paolo Bonzini
2015-06-17 14:11 ` Michael S. Tsirkin
10 siblings, 0 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-12 14:05 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Don Slutz, Luiz Capitulino,
Don Slutz, Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
From: Don Slutz <don.slutz@gmail.com>
Signed-off-by: Don Slutz <don.slutz@gmail.com>
CC: Don Slutz <dslutz@verizon.com>
---
v7:
Switched e-mail address in MAINTAINERS.
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index e728d3a..5155128 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -767,6 +767,13 @@ M: Jiri Pirko <jiri@resnulli.us>
S: Maintained
F: hw/net/rocker/
+VMware port
+M: Don Slutz <Don.Slutz@Gmail.com>
+S: Maintained
+F: hw/misc/vmport.c
+F: hw/input/vmmouse.c
+F: hw/misc/vmport_rpc.c
+
Subsystems
----------
Audio
--
1.8.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [BUGFIX][PATCH v7 1/9] vmport: The io memory region needs to be at least a size of 4
2015-06-12 14:05 ` [Qemu-devel] [BUGFIX][PATCH v7 1/9] vmport: The io memory region needs to be at least a size of 4 Don Slutz
@ 2015-06-12 22:38 ` Eric Blake
2015-06-15 13:53 ` Don Slutz
0 siblings, 1 reply; 44+ messages in thread
From: Eric Blake @ 2015-06-12 22:38 UTC (permalink / raw)
To: Don Slutz, qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Luiz Capitulino, Don Slutz,
Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1205 bytes --]
On 06/12/2015 08:05 AM, Don Slutz wrote:
> Before:
>
> commit c3c1bb99d1c11978d9ce94d1bdcf0705378c1459
> Author: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Date: Mon Mar 16 22:35:54 2015 -0700
>
> exec: Respect as_tranlsate_internal length clamp
>
> it did not matter. Only accept I/O that starts on 1st
> port.
>
> Signed-off-by: Don Slutz <dslutz@verizon.com>
> CC: Don Slutz <don.slutz@gmail.com>
> ---
> hw/misc/vmport.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/misc/vmport.c b/hw/misc/vmport.c
> index 7fcc00d..51b64bc 100644
> --- a/hw/misc/vmport.c
> +++ b/hw/misc/vmport.c
> @@ -69,6 +69,10 @@ static uint64_t vmport_ioport_read(void *opaque, hwaddr addr,
> unsigned char command;
> uint32_t eax;
>
> + /* Only support 1 address */
> + if (addr) {
> + return ~0U;
> + }
Different answer on 32-bit platforms (there, ~0U is 0xffffffff, which
then 0-extends to uint64_t rather than your desired result of
0xffffffffffffffffULL).
Why can't you just 'return -1;'?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [BUGFIX][PATCH v7 1/9] vmport: The io memory region needs to be at least a size of 4
2015-06-12 22:38 ` Eric Blake
@ 2015-06-15 13:53 ` Don Slutz
2015-06-15 15:09 ` Eric Blake
0 siblings, 1 reply; 44+ messages in thread
From: Don Slutz @ 2015-06-15 13:53 UTC (permalink / raw)
To: Eric Blake, qemu-devel@nongnu.org
Cc: Michael S. Tsirkin, Markus Armbruster, Luiz Capitulino, Don Slutz,
Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
On 06/12/15 18:38, Eric Blake wrote:
> On 06/12/2015 08:05 AM, Don Slutz wrote:
>> Before:
>>
>> commit c3c1bb99d1c11978d9ce94d1bdcf0705378c1459
>> Author: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> Date: Mon Mar 16 22:35:54 2015 -0700
>>
>> exec: Respect as_tranlsate_internal length clamp
>>
>> it did not matter. Only accept I/O that starts on 1st
>> port.
>>
>> Signed-off-by: Don Slutz <dslutz@verizon.com>
>> CC: Don Slutz <don.slutz@gmail.com>
>> ---
>> hw/misc/vmport.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/misc/vmport.c b/hw/misc/vmport.c
>> index 7fcc00d..51b64bc 100644
>> --- a/hw/misc/vmport.c
>> +++ b/hw/misc/vmport.c
>> @@ -69,6 +69,10 @@ static uint64_t vmport_ioport_read(void *opaque, hwaddr addr,
>> unsigned char command;
>> uint32_t eax;
>>
>> + /* Only support 1 address */
>> + if (addr) {
>> + return ~0U;
>> + }
>
> Different answer on 32-bit platforms (there, ~0U is 0xffffffff, which
> then 0-extends to uint64_t rather than your desired result of
> 0xffffffffffffffffULL).
>
This is not true:
Using:
build1:~/tmp>cat zr64.c
#include <stdio.h>
#include <stdint.h>
uint64_t vmport_ioport_read(void)
{
return ~0U;
}
int
main(void)
{
uint64_t res = vmport_ioport_read();
printf("res=0x%llx\n", res);
}
On 32-bits:
build1:~/tmp>file zr64
zr64: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV),
dynamically linked (uses shared libs), for GNU/Linux 2.6.18, not stripped
build1:~/tmp>./zr64
res=0xffffffff
on 64-bits:
build2:~/tmp>file zr64
zr64: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically
linked (uses shared libs), for GNU/Linux 2.6.18, not stripped
build2:~/tmp>./zr64
res=0xffffffff
> Why can't you just 'return -1;'?
>
I/O instructions on x86 are limited to 32bits max. Also when EAX is
changed via inl, the high 32bits are 0. So the correct result is ~0U
not -1.
-Don Slutz
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [BUGFIX][PATCH v7 1/9] vmport: The io memory region needs to be at least a size of 4
2015-06-15 13:53 ` Don Slutz
@ 2015-06-15 15:09 ` Eric Blake
2015-06-15 16:15 ` Don Slutz
0 siblings, 1 reply; 44+ messages in thread
From: Eric Blake @ 2015-06-15 15:09 UTC (permalink / raw)
To: Don Slutz, qemu-devel@nongnu.org
Cc: Michael S. Tsirkin, Markus Armbruster, Luiz Capitulino, Don Slutz,
Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]
On 06/15/2015 07:53 AM, Don Slutz wrote:
> On 06/12/15 18:38, Eric Blake wrote:
>>>
>>> + /* Only support 1 address */
>>> + if (addr) {
>>> + return ~0U;
>>> + }
>>
>> Different answer on 32-bit platforms (there, ~0U is 0xffffffff, which
>> then 0-extends to uint64_t rather than your desired result of
>> 0xffffffffffffffffULL).
>>
>
> This is not true:
Oh, I was confusing ~0UL (where sign extension on 32- vs 64-bit matters)
and ~0U (which you used).
>
>> Why can't you just 'return -1;'?
>>
>
> I/O instructions on x86 are limited to 32bits max. Also when EAX is
> changed via inl, the high 32bits are 0. So the correct result is ~0U
> not -1.
Still, it might be better to write an explicit 0xffffffff or even have a
named constant, rather than making people reason about whether ~0U
promotes into a 64-bit value with only 32 bits set.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 6/9] vmport_rpc: Add QMP access to vmport_rpc object.
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 6/9] vmport_rpc: Add QMP access to vmport_rpc object Don Slutz
@ 2015-06-15 15:53 ` Eric Blake
2015-06-17 16:42 ` Don Slutz
0 siblings, 1 reply; 44+ messages in thread
From: Eric Blake @ 2015-06-15 15:53 UTC (permalink / raw)
To: Don Slutz, qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Luiz Capitulino, Don Slutz,
Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 4809 bytes --]
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 <dslutz@verizon.com>
> CC: Don Slutz <don.slutz@gmail.com>
> ---
> +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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [BUGFIX][PATCH v7 1/9] vmport: The io memory region needs to be at least a size of 4
2015-06-15 15:09 ` Eric Blake
@ 2015-06-15 16:15 ` Don Slutz
0 siblings, 0 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-15 16:15 UTC (permalink / raw)
To: Eric Blake, qemu-devel@nongnu.org
Cc: Michael S. Tsirkin, Markus Armbruster, Luiz Capitulino, Don Slutz,
Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
On 06/15/15 11:09, Eric Blake wrote:
> On 06/15/2015 07:53 AM, Don Slutz wrote:
>> On 06/12/15 18:38, Eric Blake wrote:
>
>>>>
>>>> + /* Only support 1 address */
>>>> + if (addr) {
>>>> + return ~0U;
>>>> + }
>>>
>>> Different answer on 32-bit platforms (there, ~0U is 0xffffffff, which
>>> then 0-extends to uint64_t rather than your desired result of
>>> 0xffffffffffffffffULL).
>>>
>>
>> This is not true:
>
> Oh, I was confusing ~0UL (where sign extension on 32- vs 64-bit matters)
> and ~0U (which you used).
>
>>
>>> Why can't you just 'return -1;'?
>>>
>>
>> I/O instructions on x86 are limited to 32bits max. Also when EAX is
>> changed via inl, the high 32bits are 0. So the correct result is ~0U
>> not -1.
>
> Still, it might be better to write an explicit 0xffffffff or even have a
> named constant, rather than making people reason about whether ~0U
> promotes into a 64-bit value with only 32 bits set.
>
Ok, Will switch to 0xffffffff.
-Don Slutz
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
` (8 preceding siblings ...)
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 9/9] MAINTAINERS: add VMware port Don Slutz
@ 2015-06-17 13:44 ` Paolo Bonzini
2015-06-17 22:26 ` Don Slutz
2015-06-17 14:11 ` Michael S. Tsirkin
10 siblings, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2015-06-17 13:44 UTC (permalink / raw)
To: Don Slutz, qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 12/06/2015 16:05, Don Slutz wrote:
> Changes v6 to v7:
> Rebase to master
>
> Fixed a bug caused by commit c3c1bb99d1c11978d9ce94d1bdcf0705378c1459 now patch #1
>
> Added patch #2 to switch to using trace in vm,port.c.
>
> Delay call on g_strndup till after key length check.
>
> Switched e-mail address in MAINTAINERS.
>
> Eric Blake
> Why not assert(find) instead of leaving it to the comment?
> Switch to assert.
> Is it worth marking arg const here and in the VMPortRpcFind struct
> Switch to const.
> I'd rather abort() if someone compiled with -NDEBUG
> Done.
> Still mismatches on ---- line length (several sites).
> Done
>
>
> Changes v5 to v6:
>
> Rebase to master
>
> Eric Blake
> Returning a non-dictionary is not extensible.
> Added new type VmportGuestInfoValue.
> s/VmportGuestInfo/VmportGuestInfoKey/
> s/type/struct/
> Issues with examples
> Fixed.
>
>
> Changes v4 to v5:
>
> Paolo Bonzini
> What is VMPORT_SHORT about?
> Dropped this.
> Why not use a bool in CPUX86State?
> Took his sugestion and moved to a bool in X86CPU.
>
> Changes v3 to v4:
>
> Paolo Bonzini on "vmort_rpc: Add QMP access to vmport_rpc"
> Does this compile on non-x86 targets?
> Nope. Fixed.
>
> Changes v2 to v3:
>
> s/2.3/2.4
>
> Changes v1 to v2:
>
> Added live migration code.
> Adjust data structures for migration.
> Switch to GHashTable.
>
> Eric Blake
> s/spawened/spawned/
> Done
> s/traceing/tracing/
> Done
> Change "error_set(errp, ERROR_CLASS_GENERIC_ERROR, " to
> "error_setg(errp, "
> Done
> Why two commands (inject-vmport-reboot, inject-vmport-halt)?
> Switched to inject-vmport-action.
> format=base64 "bug" statements.
> Dropped.
>
> Much more on format=base64:
>
> If there is a bug it is in GLIB. However the Glib reference manual
> refers to RFC 1421 and RFC 2045 and MIME encoding. Based on all
> that (which seems to match:
>
> http://en.wikipedia.org/wiki/Base64
>
> ) MIME states that all characters outside the (base64) alphabet are
> to be ignored. Testing shows that g_base64_decode() does this.
>
> The confusion is that most non-MIME uses reject a base64 string that
> contain characters outside the alphabet. I was just following the
> other uses of base64 in this file.
>
> DataFormat refers to RFC 3548, which has the info:
>
> "
> Implementations MUST reject the encoding if it contains
> characters outside the base alphabet when interpreting base
> encoded data, unless the specification referring to this document
> explicitly states otherwise. Such specifications may, as MIME
> does, instead state that characters outside the base encoding
> alphabet should simply be ignored when interpreting data ("be
> liberal in what you accept").
> "
>
> So with GLIB going the MIME way, I do not think this is a QEMU bug
> (you could consider this a GLIB bug, but the document I found says
> that GLIB goes the MIME way and so does not reject anything).
>
> ---
>
>
> The support included is enough to allow VMware tools to install in a
> guest and provide guestinfo support. guestinfo support is provided
> by what is known as VMware RPC support.
>
> One of the better on-line references is:
>
> https://sites.google.com/site/chitchatvmback/backdoor
>
> As a place to get more accurate information by studying:
>
> http://open-vm-tools.sourceforge.net/
>
> With vmware tools installed, you can do:
>
> -------------------------------------------------------------------------------
> Last login: Fri Jan 30 16:03:08 2015
> [root@C63-min-tools ~]# vmtoolsd --cmd "info-get guestinfo.joejoel"
> No value found
> [root@C63-min-tools ~]# vmtoolsd --cmd "info-set guestinfo.joejoel bar"
>
> [root@C63-min-tools ~]# vmtoolsd --cmd "info-get guestinfo.joejoel"
> bar
> [root@C63-min-tools ~]#
> -------------------------------------------------------------------------------
>
> to access guest info. QMP access is also provided.
>
> The live migration code is still in progress.
>
> Don Slutz (9):
> vmport: The io memory region needs to be at least a size of 4
> vmport: Switch to trace
> vmport: Fix vmport_cmd_ram_size
> vmport_rpc: Add the object vmport_rpc
> vmport_rpc: Add limited support of VMware's hyper-call rpc
> vmport_rpc: Add QMP access to vmport_rpc object.
> vmport_rpc: Add migration
> vmport: Add VMware all ring hack
> MAINTAINERS: add VMware port
>
> MAINTAINERS | 7 +
> hw/i386/pc.c | 32 +-
> hw/i386/pc_piix.c | 2 +-
> hw/i386/pc_q35.c | 2 +-
> hw/misc/Makefile.objs | 1 +
> hw/misc/vmport.c | 15 +-
> hw/misc/vmport_rpc.c | 1457 ++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 6 +-
> monitor.c | 24 +
> qapi-schema.json | 101 ++++
> qmp-commands.hx | 119 ++++
> target-i386/cpu-qom.h | 3 +
> target-i386/seg_helper.c | 9 +
> trace-events | 28 +
> 14 files changed, 1796 insertions(+), 10 deletions(-)
> create mode 100644 hw/misc/vmport_rpc.c
>
Looks good, feel free to send out patches 1+2+3+9 in a pull request if
you want.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
` (9 preceding siblings ...)
2015-06-17 13:44 ` [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Paolo Bonzini
@ 2015-06-17 14:11 ` Michael S. Tsirkin
2015-06-17 14:13 ` Paolo Bonzini
10 siblings, 1 reply; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-17 14:11 UTC (permalink / raw)
To: Don Slutz
Cc: qemu-devel, Markus Armbruster, Luiz Capitulino, Anthony Liguori,
Paolo Bonzini, Andreas Färber, Richard Henderson
On Fri, Jun 12, 2015 at 10:05:47AM -0400, Don Slutz wrote:
> Changes v6 to v7:
> Rebase to master
>
> Fixed a bug caused by commit c3c1bb99d1c11978d9ce94d1bdcf0705378c1459 now patch #1
> Added patch #2 to switch to using trace in vm,port.c.
>
> Delay call on g_strndup till after key length check.
>
> Switched e-mail address in MAINTAINERS.
>
> Eric Blake
> Why not assert(find) instead of leaving it to the comment?
> Switch to assert.
> Is it worth marking arg const here and in the VMPortRpcFind struct
> Switch to const.
> I'd rather abort() if someone compiled with -NDEBUG
> Done.
> Still mismatches on ---- line length (several sites).
> Done
>
>
> Changes v5 to v6:
>
> Rebase to master
>
> Eric Blake
> Returning a non-dictionary is not extensible.
> Added new type VmportGuestInfoValue.
> s/VmportGuestInfo/VmportGuestInfoKey/
> s/type/struct/
> Issues with examples
> Fixed.
>
>
> Changes v4 to v5:
>
> Paolo Bonzini
> What is VMPORT_SHORT about?
> Dropped this.
> Why not use a bool in CPUX86State?
> Took his sugestion and moved to a bool in X86CPU.
>
> Changes v3 to v4:
>
> Paolo Bonzini on "vmort_rpc: Add QMP access to vmport_rpc"
> Does this compile on non-x86 targets?
> Nope. Fixed.
>
> Changes v2 to v3:
>
> s/2.3/2.4
>
> Changes v1 to v2:
>
> Added live migration code.
> Adjust data structures for migration.
> Switch to GHashTable.
>
> Eric Blake
> s/spawened/spawned/
> Done
> s/traceing/tracing/
> Done
> Change "error_set(errp, ERROR_CLASS_GENERIC_ERROR, " to
> "error_setg(errp, "
> Done
> Why two commands (inject-vmport-reboot, inject-vmport-halt)?
> Switched to inject-vmport-action.
> format=base64 "bug" statements.
> Dropped.
>
> Much more on format=base64:
>
> If there is a bug it is in GLIB. However the Glib reference manual
> refers to RFC 1421 and RFC 2045 and MIME encoding. Based on all
> that (which seems to match:
>
> http://en.wikipedia.org/wiki/Base64
>
> ) MIME states that all characters outside the (base64) alphabet are
> to be ignored. Testing shows that g_base64_decode() does this.
>
> The confusion is that most non-MIME uses reject a base64 string that
> contain characters outside the alphabet. I was just following the
> other uses of base64 in this file.
>
> DataFormat refers to RFC 3548, which has the info:
>
> "
> Implementations MUST reject the encoding if it contains
> characters outside the base alphabet when interpreting base
> encoded data, unless the specification referring to this document
> explicitly states otherwise. Such specifications may, as MIME
> does, instead state that characters outside the base encoding
> alphabet should simply be ignored when interpreting data ("be
> liberal in what you accept").
> "
>
> So with GLIB going the MIME way, I do not think this is a QEMU bug
> (you could consider this a GLIB bug, but the document I found says
> that GLIB goes the MIME way and so does not reject anything).
To me it looks like this will break cross-version migration as you are
adding a bunch of devices unconditionally. Will it not?
> ---
>
>
> The support included is enough to allow VMware tools to install in a
> guest and provide guestinfo support. guestinfo support is provided
> by what is known as VMware RPC support.
>
> One of the better on-line references is:
>
> https://sites.google.com/site/chitchatvmback/backdoor
>
> As a place to get more accurate information by studying:
>
> http://open-vm-tools.sourceforge.net/
>
> With vmware tools installed, you can do:
>
> -------------------------------------------------------------------------------
> Last login: Fri Jan 30 16:03:08 2015
> [root@C63-min-tools ~]# vmtoolsd --cmd "info-get guestinfo.joejoel"
> No value found
> [root@C63-min-tools ~]# vmtoolsd --cmd "info-set guestinfo.joejoel bar"
>
> [root@C63-min-tools ~]# vmtoolsd --cmd "info-get guestinfo.joejoel"
> bar
> [root@C63-min-tools ~]#
> -------------------------------------------------------------------------------
>
> to access guest info. QMP access is also provided.
>
> The live migration code is still in progress.
>
> Don Slutz (9):
> vmport: The io memory region needs to be at least a size of 4
> vmport: Switch to trace
> vmport: Fix vmport_cmd_ram_size
> vmport_rpc: Add the object vmport_rpc
> vmport_rpc: Add limited support of VMware's hyper-call rpc
> vmport_rpc: Add QMP access to vmport_rpc object.
> vmport_rpc: Add migration
> vmport: Add VMware all ring hack
> MAINTAINERS: add VMware port
>
> MAINTAINERS | 7 +
> hw/i386/pc.c | 32 +-
> hw/i386/pc_piix.c | 2 +-
> hw/i386/pc_q35.c | 2 +-
> hw/misc/Makefile.objs | 1 +
> hw/misc/vmport.c | 15 +-
> hw/misc/vmport_rpc.c | 1457 ++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 6 +-
> monitor.c | 24 +
> qapi-schema.json | 101 ++++
> qmp-commands.hx | 119 ++++
> target-i386/cpu-qom.h | 3 +
> target-i386/seg_helper.c | 9 +
> trace-events | 28 +
> 14 files changed, 1796 insertions(+), 10 deletions(-)
> create mode 100644 hw/misc/vmport_rpc.c
>
> --
> 1.8.4
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 14:11 ` Michael S. Tsirkin
@ 2015-06-17 14:13 ` Paolo Bonzini
2015-06-17 14:18 ` Michael S. Tsirkin
0 siblings, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2015-06-17 14:13 UTC (permalink / raw)
To: Michael S. Tsirkin, Don Slutz
Cc: qemu-devel, Markus Armbruster, Luiz Capitulino, Anthony Liguori,
Andreas Färber, Richard Henderson
On 17/06/2015 16:11, Michael S. Tsirkin wrote:
> On Fri, Jun 12, 2015 at 10:05:47AM -0400, Don Slutz wrote:
>> Changes v6 to v7:
>> Rebase to master
>>
>> Fixed a bug caused by commit c3c1bb99d1c11978d9ce94d1bdcf0705378c1459 now patch #1
>
>> Added patch #2 to switch to using trace in vm,port.c.
>>
>> Delay call on g_strndup till after key length check.
>>
>> Switched e-mail address in MAINTAINERS.
>>
>> Eric Blake
>> Why not assert(find) instead of leaving it to the comment?
>> Switch to assert.
>> Is it worth marking arg const here and in the VMPortRpcFind struct
>> Switch to const.
>> I'd rather abort() if someone compiled with -NDEBUG
>> Done.
>> Still mismatches on ---- line length (several sites).
>> Done
>>
>>
>> Changes v5 to v6:
>>
>> Rebase to master
>>
>> Eric Blake
>> Returning a non-dictionary is not extensible.
>> Added new type VmportGuestInfoValue.
>> s/VmportGuestInfo/VmportGuestInfoKey/
>> s/type/struct/
>> Issues with examples
>> Fixed.
>>
>>
>> Changes v4 to v5:
>>
>> Paolo Bonzini
>> What is VMPORT_SHORT about?
>> Dropped this.
>> Why not use a bool in CPUX86State?
>> Took his sugestion and moved to a bool in X86CPU.
>>
>> Changes v3 to v4:
>>
>> Paolo Bonzini on "vmort_rpc: Add QMP access to vmport_rpc"
>> Does this compile on non-x86 targets?
>> Nope. Fixed.
>>
>> Changes v2 to v3:
>>
>> s/2.3/2.4
>>
>> Changes v1 to v2:
>>
>> Added live migration code.
>> Adjust data structures for migration.
>> Switch to GHashTable.
>>
>> Eric Blake
>> s/spawened/spawned/
>> Done
>> s/traceing/tracing/
>> Done
>> Change "error_set(errp, ERROR_CLASS_GENERIC_ERROR, " to
>> "error_setg(errp, "
>> Done
>> Why two commands (inject-vmport-reboot, inject-vmport-halt)?
>> Switched to inject-vmport-action.
>> format=base64 "bug" statements.
>> Dropped.
>>
>> Much more on format=base64:
>>
>> If there is a bug it is in GLIB. However the Glib reference manual
>> refers to RFC 1421 and RFC 2045 and MIME encoding. Based on all
>> that (which seems to match:
>>
>> http://en.wikipedia.org/wiki/Base64
>>
>> ) MIME states that all characters outside the (base64) alphabet are
>> to be ignored. Testing shows that g_base64_decode() does this.
>>
>> The confusion is that most non-MIME uses reject a base64 string that
>> contain characters outside the alphabet. I was just following the
>> other uses of base64 in this file.
>>
>> DataFormat refers to RFC 3548, which has the info:
>>
>> "
>> Implementations MUST reject the encoding if it contains
>> characters outside the base alphabet when interpreting base
>> encoded data, unless the specification referring to this document
>> explicitly states otherwise. Such specifications may, as MIME
>> does, instead state that characters outside the base encoding
>> alphabet should simply be ignored when interpreting data ("be
>> liberal in what you accept").
>> "
>>
>> So with GLIB going the MIME way, I do not think this is a QEMU bug
>> (you could consider this a GLIB bug, but the document I found says
>> that GLIB goes the MIME way and so does not reject anything).
>
> To me it looks like this will break cross-version migration as you are
> adding a bunch of devices unconditionally. Will it not?
Yes, that's what was done for parallel and pcspk as well. There's no
infrastructure to avoid it.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 14:13 ` Paolo Bonzini
@ 2015-06-17 14:18 ` Michael S. Tsirkin
2015-06-17 14:27 ` Paolo Bonzini
0 siblings, 1 reply; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-17 14:18 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On Wed, Jun 17, 2015 at 04:13:52PM +0200, Paolo Bonzini wrote:
>
>
> On 17/06/2015 16:11, Michael S. Tsirkin wrote:
> > On Fri, Jun 12, 2015 at 10:05:47AM -0400, Don Slutz wrote:
> >> Changes v6 to v7:
> >> Rebase to master
> >>
> >> Fixed a bug caused by commit c3c1bb99d1c11978d9ce94d1bdcf0705378c1459 now patch #1
> >
> >> Added patch #2 to switch to using trace in vm,port.c.
> >>
> >> Delay call on g_strndup till after key length check.
> >>
> >> Switched e-mail address in MAINTAINERS.
> >>
> >> Eric Blake
> >> Why not assert(find) instead of leaving it to the comment?
> >> Switch to assert.
> >> Is it worth marking arg const here and in the VMPortRpcFind struct
> >> Switch to const.
> >> I'd rather abort() if someone compiled with -NDEBUG
> >> Done.
> >> Still mismatches on ---- line length (several sites).
> >> Done
> >>
> >>
> >> Changes v5 to v6:
> >>
> >> Rebase to master
> >>
> >> Eric Blake
> >> Returning a non-dictionary is not extensible.
> >> Added new type VmportGuestInfoValue.
> >> s/VmportGuestInfo/VmportGuestInfoKey/
> >> s/type/struct/
> >> Issues with examples
> >> Fixed.
> >>
> >>
> >> Changes v4 to v5:
> >>
> >> Paolo Bonzini
> >> What is VMPORT_SHORT about?
> >> Dropped this.
> >> Why not use a bool in CPUX86State?
> >> Took his sugestion and moved to a bool in X86CPU.
> >>
> >> Changes v3 to v4:
> >>
> >> Paolo Bonzini on "vmort_rpc: Add QMP access to vmport_rpc"
> >> Does this compile on non-x86 targets?
> >> Nope. Fixed.
> >>
> >> Changes v2 to v3:
> >>
> >> s/2.3/2.4
> >>
> >> Changes v1 to v2:
> >>
> >> Added live migration code.
> >> Adjust data structures for migration.
> >> Switch to GHashTable.
> >>
> >> Eric Blake
> >> s/spawened/spawned/
> >> Done
> >> s/traceing/tracing/
> >> Done
> >> Change "error_set(errp, ERROR_CLASS_GENERIC_ERROR, " to
> >> "error_setg(errp, "
> >> Done
> >> Why two commands (inject-vmport-reboot, inject-vmport-halt)?
> >> Switched to inject-vmport-action.
> >> format=base64 "bug" statements.
> >> Dropped.
> >>
> >> Much more on format=base64:
> >>
> >> If there is a bug it is in GLIB. However the Glib reference manual
> >> refers to RFC 1421 and RFC 2045 and MIME encoding. Based on all
> >> that (which seems to match:
> >>
> >> http://en.wikipedia.org/wiki/Base64
> >>
> >> ) MIME states that all characters outside the (base64) alphabet are
> >> to be ignored. Testing shows that g_base64_decode() does this.
> >>
> >> The confusion is that most non-MIME uses reject a base64 string that
> >> contain characters outside the alphabet. I was just following the
> >> other uses of base64 in this file.
> >>
> >> DataFormat refers to RFC 3548, which has the info:
> >>
> >> "
> >> Implementations MUST reject the encoding if it contains
> >> characters outside the base alphabet when interpreting base
> >> encoded data, unless the specification referring to this document
> >> explicitly states otherwise. Such specifications may, as MIME
> >> does, instead state that characters outside the base encoding
> >> alphabet should simply be ignored when interpreting data ("be
> >> liberal in what you accept").
> >> "
> >>
> >> So with GLIB going the MIME way, I do not think this is a QEMU bug
> >> (you could consider this a GLIB bug, but the document I found says
> >> that GLIB goes the MIME way and so does not reject anything).
> >
> > To me it looks like this will break cross-version migration as you are
> > adding a bunch of devices unconditionally. Will it not?
>
> Yes, that's what was done for parallel and pcspk as well. There's no
> infrastructure to avoid it.
>
> Paolo
How do you mean? We have multiple ways to keep devices
compatible with old versions.
Set a new property to skip the extra stuff.
Since we enable this thing by default (why do we?)
this seems like a big deal ...
--
MST
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 14:18 ` Michael S. Tsirkin
@ 2015-06-17 14:27 ` Paolo Bonzini
2015-06-17 14:29 ` Michael S. Tsirkin
0 siblings, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2015-06-17 14:27 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 17/06/2015 16:18, Michael S. Tsirkin wrote:
>> > Yes, that's what was done for parallel and pcspk as well. There's no
>> > infrastructure to avoid it.
>> >
>> > Paolo
> How do you mean? We have multiple ways to keep devices
> compatible with old versions.
> Set a new property to skip the extra stuff.
Not if the device didn't have a vmstate at all, unfortunately.
> Since we enable this thing by default (why do we?)
> this seems like a big deal ...
The PC speaker device is also enabled by default.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 14:27 ` Paolo Bonzini
@ 2015-06-17 14:29 ` Michael S. Tsirkin
2015-06-17 16:17 ` Paolo Bonzini
0 siblings, 1 reply; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-17 14:29 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
>
>
> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
> >> > Yes, that's what was done for parallel and pcspk as well. There's no
> >> > infrastructure to avoid it.
> >> >
> >> > Paolo
> > How do you mean? We have multiple ways to keep devices
> > compatible with old versions.
> > Set a new property to skip the extra stuff.
>
> Not if the device didn't have a vmstate at all, unfortunately.
?
Skip creating the device completely for old machine types.
> > Since we enable this thing by default (why do we?)
> > this seems like a big deal ...
>
> The PC speaker device is also enabled by default.
>
> Paolo
This is historical, isn't it?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 14:29 ` Michael S. Tsirkin
@ 2015-06-17 16:17 ` Paolo Bonzini
2015-06-17 16:29 ` Michael S. Tsirkin
2015-06-18 7:03 ` Gerd Hoffmann
0 siblings, 2 replies; 44+ messages in thread
From: Paolo Bonzini @ 2015-06-17 16:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 17/06/2015 16:29, Michael S. Tsirkin wrote:
> On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
>>
>>
>> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
>>>>> Yes, that's what was done for parallel and pcspk as well. There's no
>>>>> infrastructure to avoid it.
>>>>>
>>>>> Paolo
>>> How do you mean? We have multiple ways to keep devices
>>> compatible with old versions.
>>> Set a new property to skip the extra stuff.
>>
>> Not if the device didn't have a vmstate at all, unfortunately.
>
> Skip creating the device completely for old machine types.
Which device? The vmstate is tied to the same device that has always
been created.
we enable this thing by default (why do we?)
>>> this seems like a big deal ...
>>
>> The PC speaker device is also enabled by default.
>
> This is historical, isn't it?
Yes, but it has broken 2.3->2.2 migration.
Let's just stop fighting windmills.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 16:17 ` Paolo Bonzini
@ 2015-06-17 16:29 ` Michael S. Tsirkin
2015-06-17 16:48 ` Paolo Bonzini
2015-06-17 17:03 ` Don Slutz
2015-06-18 7:03 ` Gerd Hoffmann
1 sibling, 2 replies; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-17 16:29 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On Wed, Jun 17, 2015 at 06:17:19PM +0200, Paolo Bonzini wrote:
>
>
> On 17/06/2015 16:29, Michael S. Tsirkin wrote:
> > On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
> >>
> >>
> >> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
> >>>>> Yes, that's what was done for parallel and pcspk as well. There's no
> >>>>> infrastructure to avoid it.
> >>>>>
> >>>>> Paolo
> >>> How do you mean? We have multiple ways to keep devices
> >>> compatible with old versions.
> >>> Set a new property to skip the extra stuff.
> >>
> >> Not if the device didn't have a vmstate at all, unfortunately.
> >
> > Skip creating the device completely for old machine types.
>
> Which device? The vmstate is tied to the same device that has always
> been created.
Just disable the new functionality. Make it behave in
a compatible way.
> we enable this thing by default (why do we?)
Sigh. There is a very simple way to add a device in qemu: let user
request it with -device. If one does this, one gets to maintain the
resulting mess without bothering with pc maintainers in any way.
But of course, everyone implementing a new feature feels it's such a
great thing, and completel zero risk, it must be part of the default
machine. Guess what, one then gets to bother with versioning from day 0.
> >>> this seems like a big deal ...
> >>
> >> The PC speaker device is also enabled by default.
> >
> > This is historical, isn't it?
>
> Yes, but it has broken 2.3->2.2 migration.
>
> Let's just stop fighting windmills.
>
> Paolo
I don't see what you are saying. Suddenly guest visible
changes within a machine type are ok?
So we have a bug, need to fix it, preferably before piling up
more features. The best way imho is for 2.4 to avoid
this device unless requested explicitly.
--
MST
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 6/9] vmport_rpc: Add QMP access to vmport_rpc object.
2015-06-15 15:53 ` Eric Blake
@ 2015-06-17 16:42 ` Don Slutz
0 siblings, 0 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-17 16:42 UTC (permalink / raw)
To: Eric Blake, Don Slutz, qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Luiz Capitulino,
Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
On 06/15/15 11:53, Eric Blake wrote:
> 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 <dslutz@verizon.com>
>> CC: Don Slutz <don.slutz@gmail.com>
>> ---
>
>> +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)
>
Will fix.
>
>>
>> +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 **.
>
Will fix.
>
>> +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.
>
>
I do not know. However I can just delete it, and so will do so.
>> +
>> +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'?
This is value_data not key_data. So this cast is needed with 'const char
*key_data', the other 2 do go away.
>
>> + 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).
>
Yup fixed.
>> +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.
>
these 2 do get fixed.
>> + key_value.key_len = strlen(key);
>> + }
>> +
>> + rc = foreach_dynamic_vmport_rpc_device(find_get, (void *)&key_value);
>
> Another spurious cast.
>
Fixed.
>> + 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.
>
Switched to g_new0.
>> + 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.
Added that to the comment.
>
> Looking better, though.
>
-Don Slutz
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 16:29 ` Michael S. Tsirkin
@ 2015-06-17 16:48 ` Paolo Bonzini
2015-06-17 18:43 ` Michael S. Tsirkin
2015-06-17 17:03 ` Don Slutz
1 sibling, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2015-06-17 16:48 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 17/06/2015 18:29, Michael S. Tsirkin wrote:
> On Wed, Jun 17, 2015 at 06:17:19PM +0200, Paolo Bonzini wrote:
>>
>>
>> On 17/06/2015 16:29, Michael S. Tsirkin wrote:
>>> On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
>>>>
>>>>
>>>> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
>>>>>>> Yes, that's what was done for parallel and pcspk as well. There's no
>>>>>>> infrastructure to avoid it.
>>>>>>>
>>>>>>> Paolo
>>>>> How do you mean? We have multiple ways to keep devices
>>>>> compatible with old versions.
>>>>> Set a new property to skip the extra stuff.
>>>>
>>>> Not if the device didn't have a vmstate at all, unfortunately.
>>>
>>> Skip creating the device completely for old machine types.
>>
>> Which device? The vmstate is tied to the same device that has always
>> been created.
>
> Just disable the new functionality. Make it behave in
> a compatible way.
Ok. Doesn't solve the fact that the migration stream changes just
because dc->vmsd was NULL and now isn't. We cannot add a subsection:
there is no toplevel section for it to hang it from.
>> we enable this thing by default (why do we?)
>
> Sigh. There is a very simple way to add a device in qemu: let user
> request it with -device. If one does this, one gets to maintain the
> resulting mess without bothering with pc maintainers in any way.
>
> But of course, everyone implementing a new feature feels it's such a
> great thing, and completel zero risk, it must be part of the default
> machine. Guess what, one then gets to bother with versioning from day 0.
>
> I don't see what you are saying. Suddenly guest visible
> changes within a machine type are ok?
Some are. They have always been. For example, the 0xcf9 reset was
added without backwards compatibility. Bug fixes are also guest visible.
Also, commands are added unconditionally, with only the configuration
bits that govern discovery being masked. That's also guest visible if
the guest doesn't follow the spec.
> So we have a bug, need to fix it, preferably before piling up
> more features. The best way imho is for 2.4 to avoid
> this device unless requested explicitly.
The device is already enabled by default. It grew new functionality
(before it was basically just for the mouse). It didn't have state. It
now has.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 16:29 ` Michael S. Tsirkin
2015-06-17 16:48 ` Paolo Bonzini
@ 2015-06-17 17:03 ` Don Slutz
2015-06-17 17:14 ` Paolo Bonzini
2015-06-17 18:45 ` Michael S. Tsirkin
1 sibling, 2 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-17 17:03 UTC (permalink / raw)
To: Michael S. Tsirkin, Paolo Bonzini
Cc: qemu-devel, Markus Armbruster, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 06/17/15 12:29, Michael S. Tsirkin wrote:
> On Wed, Jun 17, 2015 at 06:17:19PM +0200, Paolo Bonzini wrote:
>>
>>
>> On 17/06/2015 16:29, Michael S. Tsirkin wrote:
>>> On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
>>>>
>>>>
>>>> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
>>>>>>> Yes, that's what was done for parallel and pcspk as well. There's no
>>>>>>> infrastructure to avoid it.
>>>>>>>
>>>>>>> Paolo
>>>>> How do you mean? We have multiple ways to keep devices
>>>>> compatible with old versions.
>>>>> Set a new property to skip the extra stuff.
>>>>
>>>> Not if the device didn't have a vmstate at all, unfortunately.
>>>
>>> Skip creating the device completely for old machine types.
>>
>> Which device? The vmstate is tied to the same device that has always
>> been created.
>
> Just disable the new functionality. Make it behave in
> a compatible way.
>
>> we enable this thing by default (why do we?)
>
> Sigh. There is a very simple way to add a device in qemu: let user
> request it with -device. If one does this, one gets to maintain the
> resulting mess without bothering with pc maintainers in any way.
>
> But of course, everyone implementing a new feature feels it's such a
> great thing, and completel zero risk, it must be part of the default
> machine. Guess what, one then gets to bother with versioning from day 0.
>
>>>>> this seems like a big deal ...
>>>>
>>>> The PC speaker device is also enabled by default.
>>>
>>> This is historical, isn't it?
>>
>> Yes, but it has broken 2.3->2.2 migration.
>>
>> Let's just stop fighting windmills.
>>
>> Paolo
>
> I don't see what you are saying. Suddenly guest visible
> changes within a machine type are ok?
>
> So we have a bug, need to fix it, preferably before piling up
> more features. The best way imho is for 2.4 to avoid
> this device unless requested explicitly.
>
My take on this is that Michael would like me to have a vmport_rpc=on
option, just like vmport=on (which already exists). With a default of off.
I have no problem adding it.
-Don Slutz
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 17:03 ` Don Slutz
@ 2015-06-17 17:14 ` Paolo Bonzini
2015-06-17 17:25 ` Paolo Bonzini
2015-06-17 18:45 ` Michael S. Tsirkin
2015-06-17 18:45 ` Michael S. Tsirkin
1 sibling, 2 replies; 44+ messages in thread
From: Paolo Bonzini @ 2015-06-17 17:14 UTC (permalink / raw)
To: Don Slutz, Michael S. Tsirkin
Cc: qemu-devel, Markus Armbruster, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 17/06/2015 19:03, Don Slutz wrote:
> On 06/17/15 12:29, Michael S. Tsirkin wrote:
>> On Wed, Jun 17, 2015 at 06:17:19PM +0200, Paolo Bonzini wrote:
>>>
>>>
>>> On 17/06/2015 16:29, Michael S. Tsirkin wrote:
>>>> On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
>>>>>
>>>>>
>>>>> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
>>>>>>>> Yes, that's what was done for parallel and pcspk as well. There's no
>>>>>>>> infrastructure to avoid it.
>>>>>>>>
>>>>>>>> Paolo
>>>>>> How do you mean? We have multiple ways to keep devices
>>>>>> compatible with old versions.
>>>>>> Set a new property to skip the extra stuff.
>>>>>
>>>>> Not if the device didn't have a vmstate at all, unfortunately.
>>>>
>>>> Skip creating the device completely for old machine types.
>>>
>>> Which device? The vmstate is tied to the same device that has always
>>> been created.
>>
>> Just disable the new functionality. Make it behave in
>> a compatible way.
>>
>>> we enable this thing by default (why do we?)
>>
>> Sigh. There is a very simple way to add a device in qemu: let user
>> request it with -device. If one does this, one gets to maintain the
>> resulting mess without bothering with pc maintainers in any way.
>>
>> But of course, everyone implementing a new feature feels it's such a
>> great thing, and completel zero risk, it must be part of the default
>> machine. Guess what, one then gets to bother with versioning from day 0.
>>
>>>>>> this seems like a big deal ...
>>>>>
>>>>> The PC speaker device is also enabled by default.
>>>>
>>>> This is historical, isn't it?
>>>
>>> Yes, but it has broken 2.3->2.2 migration.
>>>
>>> Let's just stop fighting windmills.
>>>
>>> Paolo
>>
>> I don't see what you are saying. Suddenly guest visible
>> changes within a machine type are ok?
>>
>> So we have a bug, need to fix it, preferably before piling up
>> more features. The best way imho is for 2.4 to avoid
>> this device unless requested explicitly.
>>
>
> My take on this is that Michael would like me to have a vmport_rpc=on
> option, just like vmport=on (which already exists). With a default of off.
It wouldn't be enough, because dc->vmsd would be non-NULL anyway.
(But yes, that option would be a good thing anyway).
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 17:14 ` Paolo Bonzini
@ 2015-06-17 17:25 ` Paolo Bonzini
2015-06-17 17:34 ` Don Slutz
2015-06-17 18:45 ` Michael S. Tsirkin
1 sibling, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2015-06-17 17:25 UTC (permalink / raw)
To: Don Slutz, Michael S. Tsirkin
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 17/06/2015 19:14, Paolo Bonzini wrote:
>
>
> On 17/06/2015 19:03, Don Slutz wrote:
>> On 06/17/15 12:29, Michael S. Tsirkin wrote:
>>> On Wed, Jun 17, 2015 at 06:17:19PM +0200, Paolo Bonzini wrote:
>>>>
>>>>
>>>> On 17/06/2015 16:29, Michael S. Tsirkin wrote:
>>>>> On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
>>>>>>
>>>>>>
>>>>>> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
>>>>>>>>> Yes, that's what was done for parallel and pcspk as well. There's no
>>>>>>>>> infrastructure to avoid it.
>>>>>>>>>
>>>>>>>>> Paolo
>>>>>>> How do you mean? We have multiple ways to keep devices
>>>>>>> compatible with old versions.
>>>>>>> Set a new property to skip the extra stuff.
>>>>>>
>>>>>> Not if the device didn't have a vmstate at all, unfortunately.
>>>>>
>>>>> Skip creating the device completely for old machine types.
>>>>
>>>> Which device? The vmstate is tied to the same device that has always
>>>> been created.
>>>
>>> Just disable the new functionality. Make it behave in
>>> a compatible way.
>>>
>>>> we enable this thing by default (why do we?)
>>>
>>> Sigh. There is a very simple way to add a device in qemu: let user
>>> request it with -device. If one does this, one gets to maintain the
>>> resulting mess without bothering with pc maintainers in any way.
>>>
>>> But of course, everyone implementing a new feature feels it's such a
>>> great thing, and completel zero risk, it must be part of the default
>>> machine. Guess what, one then gets to bother with versioning from day 0.
>>>
>>>>>>> this seems like a big deal ...
>>>>>>
>>>>>> The PC speaker device is also enabled by default.
>>>>>
>>>>> This is historical, isn't it?
>>>>
>>>> Yes, but it has broken 2.3->2.2 migration.
>>>>
>>>> Let's just stop fighting windmills.
>>>>
>>>> Paolo
>>>
>>> I don't see what you are saying. Suddenly guest visible
>>> changes within a machine type are ok?
>>>
>>> So we have a bug, need to fix it, preferably before piling up
>>> more features. The best way imho is for 2.4 to avoid
>>> this device unless requested explicitly.
>>>
>>
>> My take on this is that Michael would like me to have a vmport_rpc=on
>> option, just like vmport=on (which already exists). With a default of off.
>
> It wouldn't be enough, because dc->vmsd would be non-NULL anyway.
>
> (But yes, that option would be a good thing anyway).
Even better would be to have a "-global vmport.rpc=no" option. It would
be simpler to disable it in existing machine types.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 17:25 ` Paolo Bonzini
@ 2015-06-17 17:34 ` Don Slutz
2015-06-17 18:58 ` Michael S. Tsirkin
0 siblings, 1 reply; 44+ messages in thread
From: Don Slutz @ 2015-06-17 17:34 UTC (permalink / raw)
To: Paolo Bonzini, Michael S. Tsirkin
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 06/17/15 13:25, Paolo Bonzini wrote:
>
>
> On 17/06/2015 19:14, Paolo Bonzini wrote:
>>
>>
>> On 17/06/2015 19:03, Don Slutz wrote:
>>> On 06/17/15 12:29, Michael S. Tsirkin wrote:
>>>> On Wed, Jun 17, 2015 at 06:17:19PM +0200, Paolo Bonzini wrote:
>>>>>
>>>>>
>>>>> On 17/06/2015 16:29, Michael S. Tsirkin wrote:
>>>>>> On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
>>>>>>>>>> Yes, that's what was done for parallel and pcspk as well. There's no
>>>>>>>>>> infrastructure to avoid it.
>>>>>>>>>>
>>>>>>>>>> Paolo
>>>>>>>> How do you mean? We have multiple ways to keep devices
>>>>>>>> compatible with old versions.
>>>>>>>> Set a new property to skip the extra stuff.
>>>>>>>
>>>>>>> Not if the device didn't have a vmstate at all, unfortunately.
>>>>>>
>>>>>> Skip creating the device completely for old machine types.
>>>>>
>>>>> Which device? The vmstate is tied to the same device that has always
>>>>> been created.
>>>>
>>>> Just disable the new functionality. Make it behave in
>>>> a compatible way.
>>>>
>>>>> we enable this thing by default (why do we?)
>>>>
>>>> Sigh. There is a very simple way to add a device in qemu: let user
>>>> request it with -device. If one does this, one gets to maintain the
>>>> resulting mess without bothering with pc maintainers in any way.
>>>>
>>>> But of course, everyone implementing a new feature feels it's such a
>>>> great thing, and completel zero risk, it must be part of the default
>>>> machine. Guess what, one then gets to bother with versioning from day 0.
>>>>
>>>>>>>> this seems like a big deal ...
>>>>>>>
>>>>>>> The PC speaker device is also enabled by default.
>>>>>>
>>>>>> This is historical, isn't it?
>>>>>
>>>>> Yes, but it has broken 2.3->2.2 migration.
>>>>>
>>>>> Let's just stop fighting windmills.
>>>>>
>>>>> Paolo
>>>>
>>>> I don't see what you are saying. Suddenly guest visible
>>>> changes within a machine type are ok?
>>>>
>>>> So we have a bug, need to fix it, preferably before piling up
>>>> more features. The best way imho is for 2.4 to avoid
>>>> this device unless requested explicitly.
>>>>
>>>
>>> My take on this is that Michael would like me to have a vmport_rpc=on
>>> option, just like vmport=on (which already exists). With a default of off.
>>
>> It wouldn't be enough, because dc->vmsd would be non-NULL anyway.
>>
>> (But yes, that option would be a good thing anyway).
>
> Even better would be to have a "-global vmport.rpc=no" option. It would
> be simpler to disable it in existing machine types.
>
Either way I can avoid the device creation... Unless I hear otherwise I
will go the global way. Since the default would be no, should I also
make the default =yes for the 2.4 pc?
-Don Slutz
-Don Slutz
> Paolo
>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 16:48 ` Paolo Bonzini
@ 2015-06-17 18:43 ` Michael S. Tsirkin
2015-06-17 19:15 ` Paolo Bonzini
0 siblings, 1 reply; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-17 18:43 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On Wed, Jun 17, 2015 at 06:48:40PM +0200, Paolo Bonzini wrote:
>
>
> On 17/06/2015 18:29, Michael S. Tsirkin wrote:
> > On Wed, Jun 17, 2015 at 06:17:19PM +0200, Paolo Bonzini wrote:
> >>
> >>
> >> On 17/06/2015 16:29, Michael S. Tsirkin wrote:
> >>> On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
> >>>>
> >>>>
> >>>> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
> >>>>>>> Yes, that's what was done for parallel and pcspk as well. There's no
> >>>>>>> infrastructure to avoid it.
> >>>>>>>
> >>>>>>> Paolo
> >>>>> How do you mean? We have multiple ways to keep devices
> >>>>> compatible with old versions.
> >>>>> Set a new property to skip the extra stuff.
> >>>>
> >>>> Not if the device didn't have a vmstate at all, unfortunately.
> >>>
> >>> Skip creating the device completely for old machine types.
> >>
> >> Which device? The vmstate is tied to the same device that has always
> >> been created.
> >
> > Just disable the new functionality. Make it behave in
> > a compatible way.
>
> Ok. Doesn't solve the fact that the migration stream changes just
> because dc->vmsd was NULL and now isn't. We cannot add a subsection:
> there is no toplevel section for it to hang it from.
It seems pretty obvious:
+ vmport_rpc = isa_try_create(isa_bus, "vmport_rpc");
+ if (vmport_rpc) {
+ qdev_init_nofail(DEVICE(vmport_rpc));
+ }
Don't do this. Let user specify the device using -device vmport_rpc.
Then all issues just go away. I don't see any reason not to do it like
this - we should have done the same for other vmport things.
--
MST
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 17:03 ` Don Slutz
2015-06-17 17:14 ` Paolo Bonzini
@ 2015-06-17 18:45 ` Michael S. Tsirkin
1 sibling, 0 replies; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-17 18:45 UTC (permalink / raw)
To: Don Slutz
Cc: qemu-devel, Markus Armbruster, Don Slutz, Luiz Capitulino,
Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
On Wed, Jun 17, 2015 at 01:03:00PM -0400, Don Slutz wrote:
> On 06/17/15 12:29, Michael S. Tsirkin wrote:
> > On Wed, Jun 17, 2015 at 06:17:19PM +0200, Paolo Bonzini wrote:
> >>
> >>
> >> On 17/06/2015 16:29, Michael S. Tsirkin wrote:
> >>> On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
> >>>>
> >>>>
> >>>> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
> >>>>>>> Yes, that's what was done for parallel and pcspk as well. There's no
> >>>>>>> infrastructure to avoid it.
> >>>>>>>
> >>>>>>> Paolo
> >>>>> How do you mean? We have multiple ways to keep devices
> >>>>> compatible with old versions.
> >>>>> Set a new property to skip the extra stuff.
> >>>>
> >>>> Not if the device didn't have a vmstate at all, unfortunately.
> >>>
> >>> Skip creating the device completely for old machine types.
> >>
> >> Which device? The vmstate is tied to the same device that has always
> >> been created.
> >
> > Just disable the new functionality. Make it behave in
> > a compatible way.
> >
> >> we enable this thing by default (why do we?)
> >
> > Sigh. There is a very simple way to add a device in qemu: let user
> > request it with -device. If one does this, one gets to maintain the
> > resulting mess without bothering with pc maintainers in any way.
> >
> > But of course, everyone implementing a new feature feels it's such a
> > great thing, and completel zero risk, it must be part of the default
> > machine. Guess what, one then gets to bother with versioning from day 0.
> >
> >>>>> this seems like a big deal ...
> >>>>
> >>>> The PC speaker device is also enabled by default.
> >>>
> >>> This is historical, isn't it?
> >>
> >> Yes, but it has broken 2.3->2.2 migration.
> >>
> >> Let's just stop fighting windmills.
> >>
> >> Paolo
> >
> > I don't see what you are saying. Suddenly guest visible
> > changes within a machine type are ok?
> >
> > So we have a bug, need to fix it, preferably before piling up
> > more features. The best way imho is for 2.4 to avoid
> > this device unless requested explicitly.
> >
>
> My take on this is that Michael would like me to have a vmport_rpc=on
> option, just like vmport=on (which already exists). With a default of off.
>
> I have no problem adding it.
>
> -Don Slutz
I'd prefer -device instead. This way we don't need to deal
with it in PC code at all.
--
MST
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 17:14 ` Paolo Bonzini
2015-06-17 17:25 ` Paolo Bonzini
@ 2015-06-17 18:45 ` Michael S. Tsirkin
1 sibling, 0 replies; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-17 18:45 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Don Slutz, Anthony Liguori, Andreas Färber,
Richard Henderson
On Wed, Jun 17, 2015 at 07:14:24PM +0200, Paolo Bonzini wrote:
>
>
> On 17/06/2015 19:03, Don Slutz wrote:
> > On 06/17/15 12:29, Michael S. Tsirkin wrote:
> >> On Wed, Jun 17, 2015 at 06:17:19PM +0200, Paolo Bonzini wrote:
> >>>
> >>>
> >>> On 17/06/2015 16:29, Michael S. Tsirkin wrote:
> >>>> On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
> >>>>>
> >>>>>
> >>>>> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
> >>>>>>>> Yes, that's what was done for parallel and pcspk as well. There's no
> >>>>>>>> infrastructure to avoid it.
> >>>>>>>>
> >>>>>>>> Paolo
> >>>>>> How do you mean? We have multiple ways to keep devices
> >>>>>> compatible with old versions.
> >>>>>> Set a new property to skip the extra stuff.
> >>>>>
> >>>>> Not if the device didn't have a vmstate at all, unfortunately.
> >>>>
> >>>> Skip creating the device completely for old machine types.
> >>>
> >>> Which device? The vmstate is tied to the same device that has always
> >>> been created.
> >>
> >> Just disable the new functionality. Make it behave in
> >> a compatible way.
> >>
> >>> we enable this thing by default (why do we?)
> >>
> >> Sigh. There is a very simple way to add a device in qemu: let user
> >> request it with -device. If one does this, one gets to maintain the
> >> resulting mess without bothering with pc maintainers in any way.
> >>
> >> But of course, everyone implementing a new feature feels it's such a
> >> great thing, and completel zero risk, it must be part of the default
> >> machine. Guess what, one then gets to bother with versioning from day 0.
> >>
> >>>>>> this seems like a big deal ...
> >>>>>
> >>>>> The PC speaker device is also enabled by default.
> >>>>
> >>>> This is historical, isn't it?
> >>>
> >>> Yes, but it has broken 2.3->2.2 migration.
> >>>
> >>> Let's just stop fighting windmills.
> >>>
> >>> Paolo
> >>
> >> I don't see what you are saying. Suddenly guest visible
> >> changes within a machine type are ok?
> >>
> >> So we have a bug, need to fix it, preferably before piling up
> >> more features. The best way imho is for 2.4 to avoid
> >> this device unless requested explicitly.
> >>
> >
> > My take on this is that Michael would like me to have a vmport_rpc=on
> > option, just like vmport=on (which already exists). With a default of off.
>
> It wouldn't be enough, because dc->vmsd would be non-NULL anyway.
For which device class?
> (But yes, that option would be a good thing anyway).
>
> Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 17:34 ` Don Slutz
@ 2015-06-17 18:58 ` Michael S. Tsirkin
2015-06-18 18:40 ` Don Slutz
0 siblings, 1 reply; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-17 18:58 UTC (permalink / raw)
To: Don Slutz
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
On Wed, Jun 17, 2015 at 01:34:33PM -0400, Don Slutz wrote:
> On 06/17/15 13:25, Paolo Bonzini wrote:
> >
> >
> > On 17/06/2015 19:14, Paolo Bonzini wrote:
> >>
> >>
> >> On 17/06/2015 19:03, Don Slutz wrote:
> >>> On 06/17/15 12:29, Michael S. Tsirkin wrote:
> >>>> On Wed, Jun 17, 2015 at 06:17:19PM +0200, Paolo Bonzini wrote:
> >>>>>
> >>>>>
> >>>>> On 17/06/2015 16:29, Michael S. Tsirkin wrote:
> >>>>>> On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
> >>>>>>>>>> Yes, that's what was done for parallel and pcspk as well. There's no
> >>>>>>>>>> infrastructure to avoid it.
> >>>>>>>>>>
> >>>>>>>>>> Paolo
> >>>>>>>> How do you mean? We have multiple ways to keep devices
> >>>>>>>> compatible with old versions.
> >>>>>>>> Set a new property to skip the extra stuff.
> >>>>>>>
> >>>>>>> Not if the device didn't have a vmstate at all, unfortunately.
> >>>>>>
> >>>>>> Skip creating the device completely for old machine types.
> >>>>>
> >>>>> Which device? The vmstate is tied to the same device that has always
> >>>>> been created.
> >>>>
> >>>> Just disable the new functionality. Make it behave in
> >>>> a compatible way.
> >>>>
> >>>>> we enable this thing by default (why do we?)
> >>>>
> >>>> Sigh. There is a very simple way to add a device in qemu: let user
> >>>> request it with -device. If one does this, one gets to maintain the
> >>>> resulting mess without bothering with pc maintainers in any way.
> >>>>
> >>>> But of course, everyone implementing a new feature feels it's such a
> >>>> great thing, and completel zero risk, it must be part of the default
> >>>> machine. Guess what, one then gets to bother with versioning from day 0.
> >>>>
> >>>>>>>> this seems like a big deal ...
> >>>>>>>
> >>>>>>> The PC speaker device is also enabled by default.
> >>>>>>
> >>>>>> This is historical, isn't it?
> >>>>>
> >>>>> Yes, but it has broken 2.3->2.2 migration.
> >>>>>
> >>>>> Let's just stop fighting windmills.
> >>>>>
> >>>>> Paolo
> >>>>
> >>>> I don't see what you are saying. Suddenly guest visible
> >>>> changes within a machine type are ok?
> >>>>
> >>>> So we have a bug, need to fix it, preferably before piling up
> >>>> more features. The best way imho is for 2.4 to avoid
> >>>> this device unless requested explicitly.
> >>>>
> >>>
> >>> My take on this is that Michael would like me to have a vmport_rpc=on
> >>> option, just like vmport=on (which already exists). With a default of off.
> >>
> >> It wouldn't be enough, because dc->vmsd would be non-NULL anyway.
> >>
> >> (But yes, that option would be a good thing anyway).
> >
> > Even better would be to have a "-global vmport.rpc=no" option. It would
> > be simpler to disable it in existing machine types.
> >
>
> Either way I can avoid the device creation... Unless I hear otherwise I
> will go the global way. Since the default would be no, should I also
> make the default =yes for the 2.4 pc?
>
> -Don Slutz
>
> -Don Slutz
Can you use -device vmport_rpc, and avoid adding code to the default pc?
> > Paolo
> >
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 18:43 ` Michael S. Tsirkin
@ 2015-06-17 19:15 ` Paolo Bonzini
2015-06-17 19:22 ` Michael S. Tsirkin
0 siblings, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2015-06-17 19:15 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 17/06/2015 20:43, Michael S. Tsirkin wrote:
> It seems pretty obvious:
>
> + vmport_rpc = isa_try_create(isa_bus, "vmport_rpc");
> + if (vmport_rpc) {
> + qdev_init_nofail(DEVICE(vmport_rpc));
> + }
>
>
> Don't do this. Let user specify the device using -device vmport_rpc.
> Then all issues just go away. I don't see any reason not to do it like
> this - we should have done the same for other vmport things.
Does it make sense to have an ISA device that has no ports or MMIO
regions? It's a bit of hack modeling-wise, but sure it works.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 19:15 ` Paolo Bonzini
@ 2015-06-17 19:22 ` Michael S. Tsirkin
2015-06-17 19:24 ` Paolo Bonzini
0 siblings, 1 reply; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-17 19:22 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On Wed, Jun 17, 2015 at 09:15:56PM +0200, Paolo Bonzini wrote:
>
>
> On 17/06/2015 20:43, Michael S. Tsirkin wrote:
> > It seems pretty obvious:
> >
> > + vmport_rpc = isa_try_create(isa_bus, "vmport_rpc");
> > + if (vmport_rpc) {
> > + qdev_init_nofail(DEVICE(vmport_rpc));
> > + }
> >
> >
> > Don't do this. Let user specify the device using -device vmport_rpc.
> > Then all issues just go away. I don't see any reason not to do it like
> > this - we should have done the same for other vmport things.
>
> Does it make sense to have an ISA device that has no ports or MMIO
> regions? It's a bit of hack modeling-wise, but sure it works.
>
> Paolo
I didn't write this code :)
--
MST
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 19:22 ` Michael S. Tsirkin
@ 2015-06-17 19:24 ` Paolo Bonzini
2015-06-17 19:29 ` Michael S. Tsirkin
0 siblings, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2015-06-17 19:24 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 17/06/2015 21:22, Michael S. Tsirkin wrote:
> > Does it make sense to have an ISA device that has no ports or MMIO
> > regions? It's a bit of hack modeling-wise, but sure it works.
>
> I didn't write this code :)
Well, you did: :)
>>> + vmport_rpc = isa_try_create(isa_bus, "vmport_rpc");
>>> + if (vmport_rpc) {
>>> + qdev_init_nofail(DEVICE(vmport_rpc));
>>> + }
vmport does have a port. vmport_rpc doesn't, it would use the same port
as the main vmport device (which would dispatch to vmport_rpc).
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 19:24 ` Paolo Bonzini
@ 2015-06-17 19:29 ` Michael S. Tsirkin
0 siblings, 0 replies; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-17 19:29 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On Wed, Jun 17, 2015 at 09:24:44PM +0200, Paolo Bonzini wrote:
>
>
> On 17/06/2015 21:22, Michael S. Tsirkin wrote:
> > > Does it make sense to have an ISA device that has no ports or MMIO
> > > regions? It's a bit of hack modeling-wise, but sure it works.
> >
> > I didn't write this code :)
>
> Well, you did: :)
I merely copied a snippet from Don's patches.
> >>> + vmport_rpc = isa_try_create(isa_bus, "vmport_rpc");
> >>> + if (vmport_rpc) {
> >>> + qdev_init_nofail(DEVICE(vmport_rpc));
> >>> + }
>
> vmport does have a port. vmport_rpc doesn't, it would use the same port
> as the main vmport device (which would dispatch to vmport_rpc).
>
> Paolo
Right but that's how Don chose to model it anyway.
--
MST
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 13:44 ` [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Paolo Bonzini
@ 2015-06-17 22:26 ` Don Slutz
2015-06-18 7:58 ` Michael S. Tsirkin
2015-06-18 8:33 ` Paolo Bonzini
0 siblings, 2 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-17 22:26 UTC (permalink / raw)
To: Paolo Bonzini, Don Slutz, qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 06/17/15 09:44, Paolo Bonzini wrote:
>
> On 12/06/2015 16:05, Don Slutz wrote:
>> Changes v6 to v7:
...
> Looks good, feel free to send out patches 1+2+3+9 in a pull request if
> you want.
If I am reading this correctly, I should add
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
to these 4 patches.
Since I have never sent a pull request to QEMU before here is what I
think should be in it:
The following changes since commit f754c3c9cce3c4789733d9068394be4256dfe6a8:
Merge remote-tracking branch
'remotes/agraf/tags/signed-s390-for-upstream' into staging (2015-06-17
12:43:26 +0100)
are available in the git repository at:
git@github.com:dslutz/qemu.git vmware_pull_v7
for you to fetch changes up to a9e61af94c4452270521638c6bac11262ff2f2b7:
MAINTAINERS: add VMware port (2015-06-17 18:21:02 -0400)
- ----------------------------------------------------------------
Don Slutz (4):
vmport: The io memory region needs to be at least a size of 4
vmport: Switch to trace
vmport: Fix vmport_cmd_ram_size
MAINTAINERS: add VMware port
MAINTAINERS | 7 +++++++
hw/misc/vmport.c | 15 +++++++++------
trace-events | 4 ++++
3 files changed, 20 insertions(+), 6 deletions(-)
Not clear at all about signing this pull.
-Don Slutz
> Thanks,
>
> Paolo
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAEBAgAGBQJVgfP9AAoJEMH0lYjxq9KcR2YP/RRWoCXrROAdiJfqEKOdrccn
ZrIOF+DT2nDD7VvzYvRvPNXaCZk7ugwzqHeMBTBzwFHXpxsoFMWSQkO1rwl5S7GC
T4JHMwOBNcKY3DUasbAHjE/BPF2nJF+fkBmb6lhH7gbOSaSc/CB7JNc9UkMlBMFG
BQj0vpcgGJnhGj3eL+aQ6juulNyWioHSsSoLAMLr8wB6ywpxm2W3lrzQhgvp7v5U
Qn7r5Fp5AfmzP3Pxl2iS0bW7wligfPX0UgNw5GM1OIVnNHtZaRE0zKpdy2zvBfFs
XS/DlWK1v6VcUyb0ywi1uFmnprmluiycKBafozK8wHgi75THw1MKY6G1V2GTWt6O
PlCQnF9VOmKVzrg7YIa+7alyyJSeh7ELfBZOAl4zejbg1Mp7CoW7wvOOVX4TuJJ8
S5nAIYJa0XfkRsaNYRD3LR6i7u35CA0f2dzRSXepDIkTmFihYA3i/tBZyjIc8+vP
9W1txyWyi9i8S6pzC5UCpERvq5+bOYAi/LaOKnh5N2TiILIZ91vK+o8zTkc1J5e2
DBmE5cHRYHLf3HEPf5+wahQLKdCLV3cKF7LwkJyZEpfYW31jBlt2ePA2w5ebJenN
KJ+rS9SKswUbvgC+EdR9TQHvmSQjJyVx20dsZ0CALL3lwxFFbXRsPleKpCFmx2Q4
D2vNnwaa6GFzM48CNjcO
=xLeM
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 16:17 ` Paolo Bonzini
2015-06-17 16:29 ` Michael S. Tsirkin
@ 2015-06-18 7:03 ` Gerd Hoffmann
1 sibling, 0 replies; 44+ messages in thread
From: Gerd Hoffmann @ 2015-06-18 7:03 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Michael S. Tsirkin, Markus Armbruster, qemu-devel, Don Slutz,
Luiz Capitulino, Anthony Liguori, Andreas Färber,
Richard Henderson
Hi,
> we enable this thing by default (why do we?)
Historical reasons :(
At least we recently got an option
to turn it off (-machine $name,vmport=off)
cheers,
Gerd
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 22:26 ` Don Slutz
@ 2015-06-18 7:58 ` Michael S. Tsirkin
2015-06-18 8:33 ` Paolo Bonzini
2015-06-18 8:33 ` Paolo Bonzini
1 sibling, 1 reply; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-18 7:58 UTC (permalink / raw)
To: Don Slutz
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
On Wed, Jun 17, 2015 at 06:26:06PM -0400, Don Slutz wrote:
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 06/17/15 09:44, Paolo Bonzini wrote:
> >
> > On 12/06/2015 16:05, Don Slutz wrote:
> >> Changes v6 to v7:
> ...
> > Looks good, feel free to send out patches 1+2+3+9 in a pull request if
> > you want.
>
> If I am reading this correctly, I should add
>
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>
> to these 4 patches.
>
> Since I have never sent a pull request to QEMU before here is what I
> think should be in it:
I'd like to see a version with comments addressed first.
>
> The following changes since commit f754c3c9cce3c4789733d9068394be4256dfe6a8:
>
> Merge remote-tracking branch
> 'remotes/agraf/tags/signed-s390-for-upstream' into staging (2015-06-17
> 12:43:26 +0100)
>
> are available in the git repository at:
>
>
> git@github.com:dslutz/qemu.git vmware_pull_v7
>
> for you to fetch changes up to a9e61af94c4452270521638c6bac11262ff2f2b7:
>
> MAINTAINERS: add VMware port (2015-06-17 18:21:02 -0400)
>
> - ----------------------------------------------------------------
> Don Slutz (4):
> vmport: The io memory region needs to be at least a size of 4
> vmport: Switch to trace
> vmport: Fix vmport_cmd_ram_size
> MAINTAINERS: add VMware port
>
> MAINTAINERS | 7 +++++++
> hw/misc/vmport.c | 15 +++++++++------
> trace-events | 4 ++++
> 3 files changed, 20 insertions(+), 6 deletions(-)
>
>
> Not clear at all about signing this pull.
>
> -Don Slutz
>
> > Thanks,
> >
> > Paolo
> >
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.22 (GNU/Linux)
>
> iQIcBAEBAgAGBQJVgfP9AAoJEMH0lYjxq9KcR2YP/RRWoCXrROAdiJfqEKOdrccn
> ZrIOF+DT2nDD7VvzYvRvPNXaCZk7ugwzqHeMBTBzwFHXpxsoFMWSQkO1rwl5S7GC
> T4JHMwOBNcKY3DUasbAHjE/BPF2nJF+fkBmb6lhH7gbOSaSc/CB7JNc9UkMlBMFG
> BQj0vpcgGJnhGj3eL+aQ6juulNyWioHSsSoLAMLr8wB6ywpxm2W3lrzQhgvp7v5U
> Qn7r5Fp5AfmzP3Pxl2iS0bW7wligfPX0UgNw5GM1OIVnNHtZaRE0zKpdy2zvBfFs
> XS/DlWK1v6VcUyb0ywi1uFmnprmluiycKBafozK8wHgi75THw1MKY6G1V2GTWt6O
> PlCQnF9VOmKVzrg7YIa+7alyyJSeh7ELfBZOAl4zejbg1Mp7CoW7wvOOVX4TuJJ8
> S5nAIYJa0XfkRsaNYRD3LR6i7u35CA0f2dzRSXepDIkTmFihYA3i/tBZyjIc8+vP
> 9W1txyWyi9i8S6pzC5UCpERvq5+bOYAi/LaOKnh5N2TiILIZ91vK+o8zTkc1J5e2
> DBmE5cHRYHLf3HEPf5+wahQLKdCLV3cKF7LwkJyZEpfYW31jBlt2ePA2w5ebJenN
> KJ+rS9SKswUbvgC+EdR9TQHvmSQjJyVx20dsZ0CALL3lwxFFbXRsPleKpCFmx2Q4
> D2vNnwaa6GFzM48CNjcO
> =xLeM
> -----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-18 7:58 ` Michael S. Tsirkin
@ 2015-06-18 8:33 ` Paolo Bonzini
2015-06-18 9:40 ` Michael S. Tsirkin
0 siblings, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2015-06-18 8:33 UTC (permalink / raw)
To: Michael S. Tsirkin, Don Slutz
Cc: Markus Armbruster, qemu-devel, Don Slutz, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 18/06/2015 09:58, Michael S. Tsirkin wrote:
> > If I am reading this correctly, I should add
> >
> > Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> >
> > to these 4 patches.
> >
> > Since I have never sent a pull request to QEMU before here is what I
> > think should be in it:
>
> I'd like to see a version with comments addressed first.
These four patches are unrelated.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 22:26 ` Don Slutz
2015-06-18 7:58 ` Michael S. Tsirkin
@ 2015-06-18 8:33 ` Paolo Bonzini
1 sibling, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2015-06-18 8:33 UTC (permalink / raw)
To: Don Slutz, Don Slutz, qemu-devel
Cc: Michael S. Tsirkin, Markus Armbruster, Luiz Capitulino,
Anthony Liguori, Andreas Färber, Richard Henderson
On 18/06/2015 00:26, Don Slutz wrote:
>
> The following changes since commit
> f754c3c9cce3c4789733d9068394be4256dfe6a8:
>
> Merge remote-tracking branch
> 'remotes/agraf/tags/signed-s390-for-upstream' into staging
> (2015-06-17 12:43:26 +0100)
>
> are available in the git repository at:
>
>
> git@github.com:dslutz/qemu.git vmware_pull_v7
Almost. In your .gitconfig change
url = git@github.com:dslutz/qemu.git
to
url = https://github.com/dslutz/qemu.git
pushurl = git@github.com:dslutz/qemu.git
It will also be faster for everyday use.
> for you to fetch changes up to
> a9e61af94c4452270521638c6bac11262ff2f2b7:
>
> MAINTAINERS: add VMware port (2015-06-17 18:21:02 -0400)
>
> - ----------------------------------------------------------------
> Don Slutz (4): vmport: The io memory region needs to be at least a
> size of 4 vmport: Switch to trace vmport: Fix vmport_cmd_ram_size
> MAINTAINERS: add VMware port
>
> MAINTAINERS | 7 +++++++ hw/misc/vmport.c | 15
> +++++++++------ trace-events | 4 ++++ 3 files changed, 20
> insertions(+), 6 deletions(-)
Yes, this looks good.
> Not clear at all about signing this pull.
Instead of pushing a branch, do
git tag -s -f for-upstream
<type tag message>
git push --tags name-of-github-remote
and git request-pull should just work.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-18 8:33 ` Paolo Bonzini
@ 2015-06-18 9:40 ` Michael S. Tsirkin
0 siblings, 0 replies; 44+ messages in thread
From: Michael S. Tsirkin @ 2015-06-18 9:40 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, Markus Armbruster, Don Slutz, Luiz Capitulino,
Don Slutz, Anthony Liguori, Andreas Färber,
Richard Henderson
On Thu, Jun 18, 2015 at 10:33:00AM +0200, Paolo Bonzini wrote:
>
>
> On 18/06/2015 09:58, Michael S. Tsirkin wrote:
> > > If I am reading this correctly, I should add
> > >
> > > Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> > >
> > > to these 4 patches.
> > >
> > > Since I have never sent a pull request to QEMU before here is what I
> > > think should be in it:
> >
> > I'd like to see a version with comments addressed first.
>
> These four patches are unrelated.
>
> Paolo
Quite possibly but personally I'm confused.
May I see a series with either just patches intended for merge,
or the ones not intended for merge called out explicitly?
--
MST
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc
2015-06-17 18:58 ` Michael S. Tsirkin
@ 2015-06-18 18:40 ` Don Slutz
0 siblings, 0 replies; 44+ messages in thread
From: Don Slutz @ 2015-06-18 18:40 UTC (permalink / raw)
To: Michael S. Tsirkin, Don Slutz
Cc: Markus Armbruster, qemu-devel@nongnu.org, Luiz Capitulino,
Anthony Liguori, Paolo Bonzini, Andreas Färber,
Richard Henderson
On 06/17/15 14:58, Michael S. Tsirkin wrote:
> On Wed, Jun 17, 2015 at 01:34:33PM -0400, Don Slutz wrote:
>> On 06/17/15 13:25, Paolo Bonzini wrote:
>>>
>>>
>>> On 17/06/2015 19:14, Paolo Bonzini wrote:
>>>>
>>>>
>>>> On 17/06/2015 19:03, Don Slutz wrote:
>>>>> On 06/17/15 12:29, Michael S. Tsirkin wrote:
>>>>>> On Wed, Jun 17, 2015 at 06:17:19PM +0200, Paolo Bonzini wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 17/06/2015 16:29, Michael S. Tsirkin wrote:
>>>>>>>> On Wed, Jun 17, 2015 at 04:27:13PM +0200, Paolo Bonzini wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 17/06/2015 16:18, Michael S. Tsirkin wrote:
>>>>>>>>>>>> Yes, that's what was done for parallel and pcspk as well. There's no
>>>>>>>>>>>> infrastructure to avoid it.
>>>>>>>>>>>>
>>>>>>>>>>>> Paolo
>>>>>>>>>> How do you mean? We have multiple ways to keep devices
>>>>>>>>>> compatible with old versions.
>>>>>>>>>> Set a new property to skip the extra stuff.
>>>>>>>>>
>>>>>>>>> Not if the device didn't have a vmstate at all, unfortunately.
>>>>>>>>
>>>>>>>> Skip creating the device completely for old machine types.
>>>>>>>
>>>>>>> Which device? The vmstate is tied to the same device that has always
>>>>>>> been created.
>>>>>>
>>>>>> Just disable the new functionality. Make it behave in
>>>>>> a compatible way.
>>>>>>
>>>>>>> we enable this thing by default (why do we?)
>>>>>>
>>>>>> Sigh. There is a very simple way to add a device in qemu: let user
>>>>>> request it with -device. If one does this, one gets to maintain the
>>>>>> resulting mess without bothering with pc maintainers in any way.
>>>>>>
>>>>>> But of course, everyone implementing a new feature feels it's such a
>>>>>> great thing, and completel zero risk, it must be part of the default
>>>>>> machine. Guess what, one then gets to bother with versioning from day 0.
>>>>>>
>>>>>>>>>> this seems like a big deal ...
>>>>>>>>>
>>>>>>>>> The PC speaker device is also enabled by default.
>>>>>>>>
>>>>>>>> This is historical, isn't it?
>>>>>>>
>>>>>>> Yes, but it has broken 2.3->2.2 migration.
>>>>>>>
>>>>>>> Let's just stop fighting windmills.
>>>>>>>
>>>>>>> Paolo
>>>>>>
>>>>>> I don't see what you are saying. Suddenly guest visible
>>>>>> changes within a machine type are ok?
>>>>>>
>>>>>> So we have a bug, need to fix it, preferably before piling up
>>>>>> more features. The best way imho is for 2.4 to avoid
>>>>>> this device unless requested explicitly.
>>>>>>
>>>>>
>>>>> My take on this is that Michael would like me to have a vmport_rpc=on
>>>>> option, just like vmport=on (which already exists). With a default of off.
>>>>
>>>> It wouldn't be enough, because dc->vmsd would be non-NULL anyway.
>>>>
>>>> (But yes, that option would be a good thing anyway).
>>>
>>> Even better would be to have a "-global vmport.rpc=no" option. It would
>>> be simpler to disable it in existing machine types.
>>>
>>
>> Either way I can avoid the device creation... Unless I hear otherwise I
>> will go the global way. Since the default would be no, should I also
>> make the default =yes for the 2.4 pc?
>>
>> -Don Slutz
>>
>> -Don Slutz
>
> Can you use -device vmport_rpc, and avoid adding code to the default pc?
>
I have made this change (drop default creation, require -device to use)
and so far testing looks good.
-Don Slutz
>>> Paolo
>>>
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2015-06-18 18:40 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-12 14:05 [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [BUGFIX][PATCH v7 1/9] vmport: The io memory region needs to be at least a size of 4 Don Slutz
2015-06-12 22:38 ` Eric Blake
2015-06-15 13:53 ` Don Slutz
2015-06-15 15:09 ` Eric Blake
2015-06-15 16:15 ` Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 2/9] vmport: Switch to trace Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [BUGFIX][PATCH v7 3/9] vmport: Fix vmport_cmd_ram_size Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 4/9] vmport_rpc: Add the object vmport_rpc Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 5/9] vmport_rpc: Add limited support of VMware's hyper-call rpc Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 6/9] vmport_rpc: Add QMP access to vmport_rpc object Don Slutz
2015-06-15 15:53 ` Eric Blake
2015-06-17 16:42 ` Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 7/9] vmport_rpc: Add migration Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 8/9] vmport: Add VMware all ring hack Don Slutz
2015-06-12 14:05 ` [Qemu-devel] [PATCH v7 9/9] MAINTAINERS: add VMware port Don Slutz
2015-06-17 13:44 ` [Qemu-devel] [PATCH v7 0/9] Add limited support of VMware's hyper-call rpc Paolo Bonzini
2015-06-17 22:26 ` Don Slutz
2015-06-18 7:58 ` Michael S. Tsirkin
2015-06-18 8:33 ` Paolo Bonzini
2015-06-18 9:40 ` Michael S. Tsirkin
2015-06-18 8:33 ` Paolo Bonzini
2015-06-17 14:11 ` Michael S. Tsirkin
2015-06-17 14:13 ` Paolo Bonzini
2015-06-17 14:18 ` Michael S. Tsirkin
2015-06-17 14:27 ` Paolo Bonzini
2015-06-17 14:29 ` Michael S. Tsirkin
2015-06-17 16:17 ` Paolo Bonzini
2015-06-17 16:29 ` Michael S. Tsirkin
2015-06-17 16:48 ` Paolo Bonzini
2015-06-17 18:43 ` Michael S. Tsirkin
2015-06-17 19:15 ` Paolo Bonzini
2015-06-17 19:22 ` Michael S. Tsirkin
2015-06-17 19:24 ` Paolo Bonzini
2015-06-17 19:29 ` Michael S. Tsirkin
2015-06-17 17:03 ` Don Slutz
2015-06-17 17:14 ` Paolo Bonzini
2015-06-17 17:25 ` Paolo Bonzini
2015-06-17 17:34 ` Don Slutz
2015-06-17 18:58 ` Michael S. Tsirkin
2015-06-18 18:40 ` Don Slutz
2015-06-17 18:45 ` Michael S. Tsirkin
2015-06-17 18:45 ` Michael S. Tsirkin
2015-06-18 7:03 ` Gerd Hoffmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.