* [Qemu-devel] [PATCH v4 0/4] *** Introduce a new vhost-user-blk host device to Qemu ***
@ 2017-10-19 5:24 Changpeng Liu
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space Changpeng Liu
` (3 more replies)
0 siblings, 4 replies; 36+ messages in thread
From: Changpeng Liu @ 2017-10-19 5:24 UTC (permalink / raw)
To: changpeng.liu, qemu-devel
Cc: stefanha, pbonzini, mst, marcandre.lureau, felipe, james.r.harris
Although virtio scsi specification was designed as a replacement for virtio_blk,
there are still many users using virtio_blk. Qemu 2.9 introduced a new device
vhost user scsi which can process I/O in user space for virtio_scsi, this commit
introduces a new vhost user block host device, which can support virtio_blk in
Guest OS, and I/O processing in another I/O target.
Due to the limitation for virtio_blk specification, virtio_blk device cannot get
block information such as capacity, block size etc via the specification, several
new vhost user messages were added to deliver virtio config space
information between Qemu and I/O target, VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG
messages used for get/set config space from/to I/O target, VHOST_USER_SET_CONFIG_FD
was added for event notifier in case the change of virtio config space. Also, those
messages can be used for vhost device live migration as well.
v3-v4: refactoring the vhost user block example patch based on new libvhost-user library.
v2-v3: add new vhost user message to get/set virtio config space
Changpeng Liu (4):
vhost-user: add new vhost user messages to support virtio config space
vhost-user-blk: introduce a new vhost-user-blk host device
contrib/libvhost-user: enable virtio config space messages
contrib/vhost-user-blk: introduce a vhost-user-blk sample application
.gitignore | 1 +
Makefile | 3 +
Makefile.objs | 1 +
configure | 11 +
contrib/libvhost-user/libvhost-user.c | 75 +++++
contrib/libvhost-user/libvhost-user.h | 15 +-
contrib/vhost-user-blk/Makefile.objs | 1 +
contrib/vhost-user-blk/vhost-user-blk.c | 492 ++++++++++++++++++++++++++++++++
docs/interop/vhost-user.txt | 33 +++
hw/block/Makefile.objs | 3 +
hw/block/vhost-user-blk.c | 360 +++++++++++++++++++++++
hw/virtio/vhost-user.c | 97 +++++++
hw/virtio/vhost.c | 63 ++++
hw/virtio/virtio-pci.c | 55 ++++
hw/virtio/virtio-pci.h | 18 ++
include/hw/virtio/vhost-backend.h | 8 +
include/hw/virtio/vhost-user-blk.h | 40 +++
include/hw/virtio/vhost.h | 16 ++
18 files changed, 1291 insertions(+), 1 deletion(-)
create mode 100644 contrib/vhost-user-blk/Makefile.objs
create mode 100644 contrib/vhost-user-blk/vhost-user-blk.c
create mode 100644 hw/block/vhost-user-blk.c
create mode 100644 include/hw/virtio/vhost-user-blk.h
--
1.9.3
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-19 5:24 [Qemu-devel] [PATCH v4 0/4] *** Introduce a new vhost-user-blk host device to Qemu *** Changpeng Liu
@ 2017-10-19 5:24 ` Changpeng Liu
2017-10-19 11:37 ` Paolo Bonzini
` (2 more replies)
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device Changpeng Liu
` (2 subsequent siblings)
3 siblings, 3 replies; 36+ messages in thread
From: Changpeng Liu @ 2017-10-19 5:24 UTC (permalink / raw)
To: changpeng.liu, qemu-devel
Cc: stefanha, pbonzini, mst, marcandre.lureau, felipe, james.r.harris
Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages which can be
used for live migration of vhost user devices, also vhost user devices
can benefit from the messages to get/set virtio config space from/to the
I/O target. For the purpose to support virtio config space change,
VHOST_USER_SET_CONFIG_FD message is added as the event notifier
in case virtio config space change in the I/O target.
Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
---
docs/interop/vhost-user.txt | 33 +++++++++++++
hw/virtio/vhost-user.c | 97 +++++++++++++++++++++++++++++++++++++++
hw/virtio/vhost.c | 63 +++++++++++++++++++++++++
include/hw/virtio/vhost-backend.h | 8 ++++
include/hw/virtio/vhost.h | 16 +++++++
5 files changed, 217 insertions(+)
diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
index 954771d..ea4c5ca 100644
--- a/docs/interop/vhost-user.txt
+++ b/docs/interop/vhost-user.txt
@@ -116,6 +116,10 @@ Depending on the request type, payload can be:
- 3: IOTLB invalidate
- 4: IOTLB access fail
+ * Virtio device config space
+
+ 256 Bytes static virito device config space
+
In QEMU the vhost-user message is implemented with the following struct:
typedef struct VhostUserMsg {
@@ -129,6 +133,7 @@ typedef struct VhostUserMsg {
VhostUserMemory memory;
VhostUserLog log;
struct vhost_iotlb_msg iotlb;
+ uint8_t config[256];
};
} QEMU_PACKED VhostUserMsg;
@@ -596,6 +601,34 @@ Master message types
and expect this message once (per VQ) during device configuration
(ie. before the master starts the VQ).
+ * VHOST_USER_GET_CONFIG
+ Id: 24
+ Equivalent ioctl: N/A
+ Master payload: virtio device config space
+
+ Submitted by the vhost-user master to fetch the contents of the virtio
+ device config space. The vhost-user master may cache the contents to
+ avoid repeated VHOST_USER_GET_CONFIG calls.
+
+* VHOST_USER_SET_CONFIG
+ Id: 25
+ Equivalent ioctl: N/A
+ Master payload: virtio device config space
+
+ Submitted by the vhost-user master when the Guest changes the virtio
+ device config space and also can be used for live migration on the
+ destination host.
+
+* VHOST_USER_SET_CONFIG_FD
+ Id: 26
+ Equivalent ioctl: N/A
+ Master payload: N/A
+
+ Sets the notifier file descriptor, which is passed as ancillary data.
+ Vhost-user slave uses the file descriptor to notify vhost-user master
+ when the virtio config space changed. The vhost-user master can read
+ the virtio config space to get the latest update.
+
Slave message types
-------------------
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 093675e..4e7bafc 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -26,6 +26,11 @@
#define VHOST_MEMORY_MAX_NREGIONS 8
#define VHOST_USER_F_PROTOCOL_FEATURES 30
+/*
+ * Maximum size of virtio device config space
+ */
+#define VHOST_USER_MAX_CONFIG_SIZE 256
+
enum VhostUserProtocolFeature {
VHOST_USER_PROTOCOL_F_MQ = 0,
VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
@@ -65,6 +70,9 @@ typedef enum VhostUserRequest {
VHOST_USER_SET_SLAVE_REQ_FD = 21,
VHOST_USER_IOTLB_MSG = 22,
VHOST_USER_SET_VRING_ENDIAN = 23,
+ VHOST_USER_GET_CONFIG = 24,
+ VHOST_USER_SET_CONFIG = 25,
+ VHOST_USER_SET_CONFIG_FD = 26,
VHOST_USER_MAX
} VhostUserRequest;
@@ -109,6 +117,7 @@ typedef struct VhostUserMsg {
VhostUserMemory memory;
VhostUserLog log;
struct vhost_iotlb_msg iotlb;
+ uint8_t config[VHOST_USER_MAX_CONFIG_SIZE];
} payload;
} QEMU_PACKED VhostUserMsg;
@@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
/* No-op as the receive channel is not dedicated to IOTLB messages. */
}
+static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
+ size_t config_len)
+{
+ VhostUserMsg msg = {
+ .request = VHOST_USER_GET_CONFIG,
+ .flags = VHOST_USER_VERSION,
+ .size = config_len,
+ };
+
+ if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
+ error_report("bad config length");
+ return -1;
+ }
+
+ if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+ return -1;
+ }
+
+ if (vhost_user_read(dev, &msg) < 0) {
+ return -1;
+ }
+
+ if (msg.request != VHOST_USER_GET_CONFIG) {
+ error_report("Received unexpected msg type. Expected %d received %d",
+ VHOST_USER_GET_CONFIG, msg.request);
+ return -1;
+ }
+
+ if (msg.size != config_len) {
+ error_report("Received bad msg size.");
+ return -1;
+ }
+
+ memcpy(config, &msg.payload.config, config_len);
+
+ return 0;
+}
+
+static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *config,
+ size_t config_len)
+{
+ bool reply_supported = virtio_has_feature(dev->protocol_features,
+ VHOST_USER_PROTOCOL_F_REPLY_ACK);
+
+ VhostUserMsg msg = {
+ .request = VHOST_USER_SET_CONFIG,
+ .flags = VHOST_USER_VERSION,
+ .size = config_len,
+ };
+
+ if (reply_supported) {
+ msg.flags |= VHOST_USER_NEED_REPLY_MASK;
+ }
+
+ if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
+ error_report("bad config length");
+ return -1;
+ }
+
+ memcpy(&msg.payload.config, config, config_len);
+ if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+ return -1;
+ }
+
+ if (reply_supported) {
+ return process_message_reply(dev, &msg);
+ }
+
+ return 0;
+}
+
+static int vhost_user_set_config_fd(struct vhost_dev *dev, int fd)
+{
+ VhostUserMsg msg = {
+ .request = VHOST_USER_SET_CONFIG_FD,
+ .flags = VHOST_USER_VERSION,
+ };
+
+ if (vhost_user_write(dev, &msg, &fd, 1) < 0) {
+ return -1;
+ }
+
+ return 0;
+}
+
const VhostOps user_ops = {
.backend_type = VHOST_BACKEND_TYPE_USER,
.vhost_backend_init = vhost_user_init,
@@ -948,4 +1042,7 @@ const VhostOps user_ops = {
.vhost_net_set_mtu = vhost_user_net_set_mtu,
.vhost_set_iotlb_callback = vhost_user_set_iotlb_callback,
.vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg,
+ .vhost_get_config = vhost_user_get_config,
+ .vhost_set_config = vhost_user_set_config,
+ .vhost_set_config_fd = vhost_user_set_config_fd,
};
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index ddc42f0..8079f46 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1354,6 +1354,9 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
for (i = 0; i < hdev->nvqs; ++i) {
vhost_virtqueue_cleanup(hdev->vqs + i);
}
+ if (hdev->config_ops) {
+ event_notifier_cleanup(&hdev->config_notifier);
+ }
if (hdev->mem) {
/* those are only safe after successful init */
memory_listener_unregister(&hdev->memory_listener);
@@ -1501,6 +1504,66 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
}
}
+int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
+ size_t config_len)
+{
+ assert(hdev->vhost_ops);
+
+ if (hdev->vhost_ops->vhost_get_config) {
+ return hdev->vhost_ops->vhost_get_config(hdev, config, config_len);
+ }
+
+ return 0;
+}
+
+int vhost_dev_set_config(struct vhost_dev *hdev, const uint8_t *config,
+ size_t config_len)
+{
+ assert(hdev->vhost_ops);
+
+ if (hdev->vhost_ops->vhost_set_config) {
+ return hdev->vhost_ops->vhost_set_config(hdev, config, config_len);
+ }
+
+ return 0;
+}
+
+static void vhost_dev_config_notifier_read(EventNotifier *n)
+{
+ struct vhost_dev *hdev = container_of(n, struct vhost_dev,
+ config_notifier);
+
+ if (event_notifier_test_and_clear(n)) {
+ if (hdev->config_ops) {
+ hdev->config_ops->vhost_dev_config_notifier(hdev);
+ }
+ }
+}
+
+int vhost_dev_set_config_notifier(struct vhost_dev *hdev,
+ const VhostDevConfigOps *ops)
+{
+ int r, fd;
+
+ assert(hdev->vhost_ops);
+
+ r = event_notifier_init(&hdev->config_notifier, 0);
+ if (r < 0) {
+ return r;
+ }
+
+ hdev->config_ops = ops;
+ event_notifier_set_handler(&hdev->config_notifier,
+ vhost_dev_config_notifier_read);
+
+ if (hdev->vhost_ops->vhost_set_config_fd) {
+ fd = event_notifier_get_fd(&hdev->config_notifier);
+ return hdev->vhost_ops->vhost_set_config_fd(hdev, fd);
+ }
+
+ return 0;
+}
+
/* Host notifiers must be enabled at this point. */
int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
{
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index a7a5f22..df6769e 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -84,6 +84,11 @@ typedef void (*vhost_set_iotlb_callback_op)(struct vhost_dev *dev,
int enabled);
typedef int (*vhost_send_device_iotlb_msg_op)(struct vhost_dev *dev,
struct vhost_iotlb_msg *imsg);
+typedef int (*vhost_set_config_op)(struct vhost_dev *dev, const uint8_t *config,
+ size_t config_len);
+typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config,
+ size_t config_len);
+typedef int (*vhost_set_config_fd_op)(struct vhost_dev *dev, int fd);
typedef struct VhostOps {
VhostBackendType backend_type;
@@ -118,6 +123,9 @@ typedef struct VhostOps {
vhost_vsock_set_running_op vhost_vsock_set_running;
vhost_set_iotlb_callback_op vhost_set_iotlb_callback;
vhost_send_device_iotlb_msg_op vhost_send_device_iotlb_msg;
+ vhost_get_config_op vhost_get_config;
+ vhost_set_config_op vhost_set_config;
+ vhost_set_config_fd_op vhost_set_config_fd;
} VhostOps;
extern const VhostOps user_ops;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 467dc77..ff172f2 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -46,6 +46,12 @@ struct vhost_iommu {
QLIST_ENTRY(vhost_iommu) iommu_next;
};
+typedef struct VhostDevConfigOps {
+ /* Vhost device config space changed callback
+ */
+ void (*vhost_dev_config_notifier)(struct vhost_dev *dev);
+} VhostDevConfigOps;
+
struct vhost_memory;
struct vhost_dev {
VirtIODevice *vdev;
@@ -76,6 +82,8 @@ struct vhost_dev {
QLIST_ENTRY(vhost_dev) entry;
QLIST_HEAD(, vhost_iommu) iommu_list;
IOMMUNotifier n;
+ EventNotifier config_notifier;
+ const VhostDevConfigOps *config_ops;
};
int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
@@ -106,4 +114,12 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
struct vhost_vring_file *file);
int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
+int vhost_dev_get_config(struct vhost_dev *dev, uint8_t *config,
+ size_t config_len);
+int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *config,
+ size_t config_len);
+/* notifier callback in case vhost device config space changed
+ */
+int vhost_dev_set_config_notifier(struct vhost_dev *dev,
+ const VhostDevConfigOps *ops);
#endif
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-19 5:24 [Qemu-devel] [PATCH v4 0/4] *** Introduce a new vhost-user-blk host device to Qemu *** Changpeng Liu
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space Changpeng Liu
@ 2017-10-19 5:24 ` Changpeng Liu
2017-10-19 11:33 ` Paolo Bonzini
2017-10-19 15:17 ` Stefan Hajnoczi
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 3/4] contrib/libvhost-user: enable virtio config space messages Changpeng Liu
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 4/4] contrib/vhost-user-blk: introduce a vhost-user-blk sample application Changpeng Liu
3 siblings, 2 replies; 36+ messages in thread
From: Changpeng Liu @ 2017-10-19 5:24 UTC (permalink / raw)
To: changpeng.liu, qemu-devel
Cc: stefanha, pbonzini, mst, marcandre.lureau, felipe, james.r.harris
This commit introduces a new vhost-user device for block, it uses a
chardev to connect with the backend, same with Qemu virito-blk device,
Guest OS still uses the virtio-blk frontend driver.
To use it, start Qemu with command line like this:
qemu-system-x86_64 \
-chardev socket,id=char0,path=/path/vhost.socket \
-device vhost-user-blk-pci,chardev=char0,num_queues=1, \
bootindex=2... \
Users can use different parameters for `num_queues` and `bootindex`.
Different with exist Qemu virtio-blk host device, it makes more easy
for users to implement their own I/O processing logic, such as all
user space I/O stack against hardware block device. It uses the new
vhost messages(VHOST_USER_GET_CONFIG) to get block virtio config
information from backend process.
Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
---
configure | 11 ++
hw/block/Makefile.objs | 3 +
hw/block/vhost-user-blk.c | 360 +++++++++++++++++++++++++++++++++++++
hw/virtio/virtio-pci.c | 55 ++++++
hw/virtio/virtio-pci.h | 18 ++
include/hw/virtio/vhost-user-blk.h | 40 +++++
6 files changed, 487 insertions(+)
create mode 100644 hw/block/vhost-user-blk.c
create mode 100644 include/hw/virtio/vhost-user-blk.h
diff --git a/configure b/configure
index 663e908..f2b348f 100755
--- a/configure
+++ b/configure
@@ -318,6 +318,7 @@ tcg="yes"
vhost_net="no"
vhost_scsi="no"
+vhost_user_blk="no"
vhost_vsock="no"
vhost_user=""
kvm="no"
@@ -782,6 +783,7 @@ Linux)
kvm="yes"
vhost_net="yes"
vhost_scsi="yes"
+ vhost_user_blk="yes"
vhost_vsock="yes"
QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers $QEMU_INCLUDES"
supported_os="yes"
@@ -1139,6 +1141,10 @@ for opt do
;;
--enable-vhost-scsi) vhost_scsi="yes"
;;
+ --disable-vhost-user-blk) vhost_user_blk="no"
+ ;;
+ --enable-vhost-user-blk) vhost_user_blk="yes"
+ ;;
--disable-vhost-vsock) vhost_vsock="no"
;;
--enable-vhost-vsock) vhost_vsock="yes"
@@ -1511,6 +1517,7 @@ disabled with --disable-FEATURE, default is enabled if available:
cap-ng libcap-ng support
attr attr and xattr support
vhost-net vhost-net acceleration support
+ vhost-user-blk VM virtio-blk acceleration in user space
spice spice
rbd rados block device (rbd)
libiscsi iscsi support
@@ -5417,6 +5424,7 @@ echo "posix_madvise $posix_madvise"
echo "libcap-ng support $cap_ng"
echo "vhost-net support $vhost_net"
echo "vhost-scsi support $vhost_scsi"
+echo "vhost-user-blk support $vhost_user_blk"
echo "vhost-vsock support $vhost_vsock"
echo "vhost-user support $vhost_user"
echo "Trace backends $trace_backends"
@@ -5845,6 +5853,9 @@ fi
if test "$vhost_scsi" = "yes" ; then
echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak
fi
+if test "$vhost_user_blk" = "yes" -a "$vhost_user" = "yes"; then
+ echo "CONFIG_VHOST_USER_BLK=y" >> $config_host_mak
+fi
if test "$vhost_net" = "yes" -a "$vhost_user" = "yes"; then
echo "CONFIG_VHOST_NET_USED=y" >> $config_host_mak
fi
diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
index e0ed980..4c19a58 100644
--- a/hw/block/Makefile.objs
+++ b/hw/block/Makefile.objs
@@ -13,3 +13,6 @@ obj-$(CONFIG_SH4) += tc58128.o
obj-$(CONFIG_VIRTIO) += virtio-blk.o
obj-$(CONFIG_VIRTIO) += dataplane/
+ifeq ($(CONFIG_VIRTIO),y)
+obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk.o
+endif
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
new file mode 100644
index 0000000..8aa9fa9
--- /dev/null
+++ b/hw/block/vhost-user-blk.c
@@ -0,0 +1,360 @@
+/*
+ * vhost-user-blk host device
+ *
+ * Copyright IBM, Corp. 2011
+ * Copyright(C) 2017 Intel Corporation.
+ *
+ * Authors:
+ * Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
+ * Changpeng Liu <changpeng.liu@intel.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/typedefs.h"
+#include "qemu/cutils.h"
+#include "qom/object.h"
+#include "hw/qdev-core.h"
+#include "hw/virtio/vhost.h"
+#include "hw/virtio/vhost-user-blk.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-access.h"
+
+static const int user_feature_bits[] = {
+ VIRTIO_BLK_F_SIZE_MAX,
+ VIRTIO_BLK_F_SEG_MAX,
+ VIRTIO_BLK_F_GEOMETRY,
+ VIRTIO_BLK_F_BLK_SIZE,
+ VIRTIO_BLK_F_TOPOLOGY,
+ VIRTIO_BLK_F_SCSI,
+ VIRTIO_BLK_F_MQ,
+ VIRTIO_BLK_F_RO,
+ VIRTIO_BLK_F_FLUSH,
+ VIRTIO_BLK_F_BARRIER,
+ VIRTIO_BLK_F_WCE,
+ VIRTIO_F_VERSION_1,
+ VIRTIO_RING_F_INDIRECT_DESC,
+ VIRTIO_RING_F_EVENT_IDX,
+ VIRTIO_F_NOTIFY_ON_EMPTY,
+ VHOST_INVALID_FEATURE_BIT
+};
+
+static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
+{
+ VHostUserBlk *s = VHOST_USER_BLK(vdev);
+
+ memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
+}
+
+static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
+{
+ VHostUserBlk *s = VHOST_USER_BLK(vdev);
+ struct virtio_blk_config *blkcfg = (struct virtio_blk_config *)config;
+ int ret;
+
+ if (blkcfg->wce == s->blkcfg.wce) {
+ return;
+ }
+
+ ret = vhost_dev_set_config(&s->dev, config,
+ sizeof(struct virtio_blk_config));
+ if (ret) {
+ error_report("set device config space failed");
+ return;
+ }
+
+ s->blkcfg.wce = blkcfg->wce;
+}
+
+static void vhost_user_blk_handle_config_change(struct vhost_dev *dev)
+{
+ int ret;
+ struct virtio_blk_config blkcfg;
+ VHostUserBlk *s = VHOST_USER_BLK(dev->vdev);
+
+ ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg,
+ sizeof(struct virtio_blk_config));
+ if (ret < 0) {
+ error_report("get config space failed");
+ return;
+ }
+
+ memcpy(&s->blkcfg, &blkcfg, sizeof(struct virtio_blk_config));
+ memcpy(dev->vdev->config, &blkcfg, sizeof(struct virtio_blk_config));
+
+ virtio_notify_config(dev->vdev);
+}
+
+const VhostDevConfigOps blk_ops = {
+ .vhost_dev_config_notifier = vhost_user_blk_handle_config_change,
+};
+
+static void vhost_user_blk_start(VirtIODevice *vdev)
+{
+ VHostUserBlk *s = VHOST_USER_BLK(vdev);
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+ int i, ret;
+
+ if (!k->set_guest_notifiers) {
+ error_report("binding does not support guest notifiers");
+ return;
+ }
+
+ ret = vhost_dev_enable_notifiers(&s->dev, vdev);
+ if (ret < 0) {
+ error_report("Error enabling host notifiers: %d", -ret);
+ return;
+ }
+
+ ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, true);
+ if (ret < 0) {
+ error_report("Error binding guest notifier: %d", -ret);
+ goto err_host_notifiers;
+ }
+
+ s->dev.acked_features = vdev->guest_features;
+ ret = vhost_dev_start(&s->dev, vdev);
+ if (ret < 0) {
+ error_report("Error starting vhost: %d", -ret);
+ goto err_guest_notifiers;
+ }
+
+ /* guest_notifier_mask/pending not used yet, so just unmask
+ * everything here. virtio-pci will do the right thing by
+ * enabling/disabling irqfd.
+ */
+ for (i = 0; i < s->dev.nvqs; i++) {
+ vhost_virtqueue_mask(&s->dev, vdev, i, false);
+ }
+
+ return;
+
+err_guest_notifiers:
+ k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
+err_host_notifiers:
+ vhost_dev_disable_notifiers(&s->dev, vdev);
+}
+
+static void vhost_user_blk_stop(VirtIODevice *vdev)
+{
+ VHostUserBlk *s = VHOST_USER_BLK(vdev);
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+ int ret;
+
+ if (!k->set_guest_notifiers) {
+ return;
+ }
+
+ vhost_dev_stop(&s->dev, vdev);
+
+ ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
+ if (ret < 0) {
+ error_report("vhost guest notifier cleanup failed: %d", ret);
+ return;
+ }
+
+ vhost_dev_disable_notifiers(&s->dev, vdev);
+}
+
+static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status)
+{
+ VHostUserBlk *s = VHOST_USER_BLK(vdev);
+ bool should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;
+
+ if (!vdev->vm_running) {
+ should_start = false;
+ }
+
+ if (s->dev.started == should_start) {
+ return;
+ }
+
+ if (should_start) {
+ vhost_user_blk_start(vdev);
+ } else {
+ vhost_user_blk_stop(vdev);
+ }
+
+}
+
+static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev,
+ uint64_t features,
+ Error **errp)
+{
+ VHostUserBlk *s = VHOST_USER_BLK(vdev);
+ uint64_t get_features;
+
+ /* Turn on pre-defined features */
+ features |= s->host_features;
+
+ get_features = vhost_get_features(&s->dev, user_feature_bits, features);
+
+ return get_features;
+}
+
+static void vhost_user_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
+{
+
+}
+
+static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VHostUserBlk *s = VHOST_USER_BLK(vdev);
+ int i, ret;
+
+ if (!s->chardev.chr) {
+ error_setg(errp, "vhost-user-blk: chardev is mandatory");
+ return;
+ }
+
+ if (!s->num_queues || s->num_queues > VIRTIO_QUEUE_MAX) {
+ error_setg(errp, "vhost-user-blk: invalid number of IO queues");
+ return;
+ }
+
+ if (!s->queue_size) {
+ error_setg(errp, "vhost-user-blk: queue size must be non-zero");
+ return;
+ }
+
+ virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
+ sizeof(struct virtio_blk_config));
+
+ for (i = 0; i < s->num_queues; i++) {
+ virtio_add_queue(vdev, s->queue_size,
+ vhost_user_blk_handle_output);
+ }
+
+ s->dev.nvqs = s->num_queues;
+ s->dev.vqs = g_new(struct vhost_virtqueue, s->dev.nvqs);
+ s->dev.vq_index = 0;
+ s->dev.backend_features = 0;
+
+ ret = vhost_dev_init(&s->dev, &s->chardev, VHOST_BACKEND_TYPE_USER, 0);
+ if (ret < 0) {
+ error_setg(errp, "vhost-user-blk: vhost initialization failed: %s",
+ strerror(-ret));
+ goto virtio_err;
+ }
+
+ ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
+ sizeof(struct virtio_blk_config));
+ if (ret < 0) {
+ error_setg(errp, "vhost-user-blk: get block config failed");
+ goto vhost_err;
+ }
+
+ if (s->blkcfg.num_queues != s->num_queues) {
+ s->blkcfg.num_queues = s->num_queues;
+ }
+
+ vhost_dev_set_config_notifier(&s->dev, &blk_ops);
+
+ return;
+
+vhost_err:
+ vhost_dev_cleanup(&s->dev);
+virtio_err:
+ g_free(s->dev.vqs);
+ virtio_cleanup(vdev);
+}
+
+static void vhost_user_blk_device_unrealize(DeviceState *dev, Error **errp)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VHostUserBlk *s = VHOST_USER_BLK(dev);
+
+ vhost_user_blk_set_status(vdev, 0);
+ vhost_dev_cleanup(&s->dev);
+ g_free(s->dev.vqs);
+ virtio_cleanup(vdev);
+}
+
+static void vhost_user_blk_instance_init(Object *obj)
+{
+ VHostUserBlk *s = VHOST_USER_BLK(obj);
+
+ device_add_bootindex_property(obj, &s->bootindex, "bootindex",
+ "/disk@0,0", DEVICE(obj), NULL);
+}
+
+static const VMStateDescription vmstate_vhost_user_blk = {
+ .name = "vhost-user-blk",
+ .minimum_version_id = 1,
+ .version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_VIRTIO_DEVICE,
+ VMSTATE_END_OF_LIST()
+ },
+};
+
+static Property vhost_user_blk_properties[] = {
+ DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
+ DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues, 1),
+ DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128),
+ DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_SIZE_MAX, true),
+ DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_SIZE_MAX, true),
+ DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_SEG_MAX, true),
+ DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_GEOMETRY, true),
+ DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_RO, false),
+ DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_BLK_SIZE, true),
+ DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_TOPOLOGY, true),
+ DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_MQ, true),
+ DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_FLUSH, true),
+ DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_BARRIER, false),
+ DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_SCSI, false),
+ DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
+ VIRTIO_BLK_F_WCE, false),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vhost_user_blk_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+ dc->props = vhost_user_blk_properties;
+ dc->vmsd = &vmstate_vhost_user_blk;
+ set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+ vdc->realize = vhost_user_blk_device_realize;
+ vdc->unrealize = vhost_user_blk_device_unrealize;
+ vdc->get_config = vhost_user_blk_update_config;
+ vdc->set_config = vhost_user_blk_set_config;
+ vdc->get_features = vhost_user_blk_get_features;
+ vdc->set_status = vhost_user_blk_set_status;
+}
+
+static const TypeInfo vhost_user_blk_info = {
+ .name = TYPE_VHOST_USER_BLK,
+ .parent = TYPE_VIRTIO_DEVICE,
+ .instance_size = sizeof(VHostUserBlk),
+ .instance_init = vhost_user_blk_instance_init,
+ .class_init = vhost_user_blk_class_init,
+};
+
+static void virtio_register_types(void)
+{
+ type_register_static(&vhost_user_blk_info);
+}
+
+type_init(virtio_register_types)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index e92837c..174939f 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1976,6 +1976,58 @@ static const TypeInfo virtio_blk_pci_info = {
.class_init = virtio_blk_pci_class_init,
};
+#ifdef CONFIG_VHOST_USER_BLK
+/* vhost-user-blk */
+
+static Property vhost_user_blk_pci_properties[] = {
+ DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
+ DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vhost_user_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+ VHostUserBlkPCI *dev = VHOST_USER_BLK_PCI(vpci_dev);
+ DeviceState *vdev = DEVICE(&dev->vdev);
+
+ qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
+ object_property_set_bool(OBJECT(vdev), true, "realized", errp);
+}
+
+static void vhost_user_blk_pci_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+ PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+
+ set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+ dc->props = vhost_user_blk_pci_properties;
+ k->realize = vhost_user_blk_pci_realize;
+ pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+ pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_BLOCK;
+ pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
+ pcidev_k->class_id = PCI_CLASS_STORAGE_SCSI;
+}
+
+static void vhost_user_blk_pci_instance_init(Object *obj)
+{
+ VHostUserBlkPCI *dev = VHOST_USER_BLK_PCI(obj);
+
+ virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+ TYPE_VHOST_USER_BLK);
+ object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
+ "bootindex", &error_abort);
+}
+
+static const TypeInfo vhost_user_blk_pci_info = {
+ .name = TYPE_VHOST_USER_BLK_PCI,
+ .parent = TYPE_VIRTIO_PCI,
+ .instance_size = sizeof(VHostUserBlkPCI),
+ .instance_init = vhost_user_blk_pci_instance_init,
+ .class_init = vhost_user_blk_pci_class_init,
+};
+#endif
+
/* virtio-scsi-pci */
static Property virtio_scsi_pci_properties[] = {
@@ -2622,6 +2674,9 @@ static void virtio_pci_register_types(void)
type_register_static(&virtio_9p_pci_info);
#endif
type_register_static(&virtio_blk_pci_info);
+#ifdef CONFIG_VHOST_USER_BLK
+ type_register_static(&vhost_user_blk_pci_info);
+#endif
type_register_static(&virtio_scsi_pci_info);
type_register_static(&virtio_balloon_pci_info);
type_register_static(&virtio_serial_pci_info);
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 12d3a90..a72c1d4 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -27,6 +27,9 @@
#include "hw/virtio/virtio-gpu.h"
#include "hw/virtio/virtio-crypto.h"
#include "hw/virtio/vhost-user-scsi.h"
+#ifdef CONFIG_VHOST_USER_BLK
+#include "hw/virtio/vhost-user-blk.h"
+#endif
#ifdef CONFIG_VIRTFS
#include "hw/9pfs/virtio-9p.h"
@@ -46,6 +49,7 @@ typedef struct VirtIOSerialPCI VirtIOSerialPCI;
typedef struct VirtIONetPCI VirtIONetPCI;
typedef struct VHostSCSIPCI VHostSCSIPCI;
typedef struct VHostUserSCSIPCI VHostUserSCSIPCI;
+typedef struct VHostUserBlkPCI VHostUserBlkPCI;
typedef struct VirtIORngPCI VirtIORngPCI;
typedef struct VirtIOInputPCI VirtIOInputPCI;
typedef struct VirtIOInputHIDPCI VirtIOInputHIDPCI;
@@ -244,6 +248,20 @@ struct VHostUserSCSIPCI {
VHostUserSCSI vdev;
};
+#ifdef CONFIG_VHOST_USER_BLK
+/*
+ * vhost-user-blk-pci: This extends VirtioPCIProxy.
+ */
+#define TYPE_VHOST_USER_BLK_PCI "vhost-user-blk-pci"
+#define VHOST_USER_BLK_PCI(obj) \
+ OBJECT_CHECK(VHostUserBlkPCI, (obj), TYPE_VHOST_USER_BLK_PCI)
+
+struct VHostUserBlkPCI {
+ VirtIOPCIProxy parent_obj;
+ VHostUserBlk vdev;
+};
+#endif
+
/*
* virtio-blk-pci: This extends VirtioPCIProxy.
*/
diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
new file mode 100644
index 0000000..77d20f0
--- /dev/null
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -0,0 +1,40 @@
+/*
+ * vhost-user-blk host device
+ * Copyright IBM, Corp. 2011
+ * Copyright(C) 2017 Intel Corporation.
+ *
+ * Authors:
+ * Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
+ * Changpeng Liu <changpeng.liu@intel.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef VHOST_USER_BLK_H
+#define VHOST_USER_BLK_H
+
+#include "standard-headers/linux/virtio_blk.h"
+#include "qemu-common.h"
+#include "hw/qdev.h"
+#include "hw/block/block.h"
+#include "chardev/char-fe.h"
+#include "hw/virtio/vhost.h"
+
+#define TYPE_VHOST_USER_BLK "vhost-user-blk"
+#define VHOST_USER_BLK(obj) \
+ OBJECT_CHECK(VHostUserBlk, (obj), TYPE_VHOST_USER_BLK)
+
+typedef struct VHostUserBlk {
+ VirtIODevice parent_obj;
+ CharBackend chardev;
+ int32_t bootindex;
+ uint64_t host_features;
+ struct virtio_blk_config blkcfg;
+ uint16_t num_queues;
+ uint32_t queue_size;
+ struct vhost_dev dev;
+} VHostUserBlk;
+
+#endif
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v4 3/4] contrib/libvhost-user: enable virtio config space messages
2017-10-19 5:24 [Qemu-devel] [PATCH v4 0/4] *** Introduce a new vhost-user-blk host device to Qemu *** Changpeng Liu
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space Changpeng Liu
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device Changpeng Liu
@ 2017-10-19 5:24 ` Changpeng Liu
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 4/4] contrib/vhost-user-blk: introduce a vhost-user-blk sample application Changpeng Liu
3 siblings, 0 replies; 36+ messages in thread
From: Changpeng Liu @ 2017-10-19 5:24 UTC (permalink / raw)
To: changpeng.liu, qemu-devel
Cc: stefanha, pbonzini, mst, marcandre.lureau, felipe, james.r.harris
Enable VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG/VHOST_USER_SET_CONFIG_FD
messages in libvhost-user library, users can implement their own I/O target
based on the library. This enable the virtio config space delivered between
Qemu host device and the I/O target, also event notifier is added in case
of virtio config space changed.
Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
---
contrib/libvhost-user/libvhost-user.c | 75 +++++++++++++++++++++++++++++++++++
contrib/libvhost-user/libvhost-user.h | 15 ++++++-
2 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
index f409bd3..0343d4b 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -84,6 +84,9 @@ vu_request_to_string(unsigned int req)
REQ(VHOST_USER_SET_SLAVE_REQ_FD),
REQ(VHOST_USER_IOTLB_MSG),
REQ(VHOST_USER_SET_VRING_ENDIAN),
+ REQ(VHOST_USER_GET_CONFIG),
+ REQ(VHOST_USER_SET_CONFIG),
+ REQ(VHOST_USER_SET_CONFIG_FD),
REQ(VHOST_USER_MAX),
};
#undef REQ
@@ -798,6 +801,66 @@ vu_set_slave_req_fd(VuDev *dev, VhostUserMsg *vmsg)
}
static bool
+vu_get_config(VuDev *dev, VhostUserMsg *vmsg)
+{
+ int ret = -1;
+
+ if (dev->iface->get_config) {
+ ret = dev->iface->get_config(dev, vmsg->payload.config, vmsg->size);
+ }
+
+ if (ret) {
+ /* resize to zero to indicate an error to master */
+ vmsg->size = 0;
+ }
+
+ return true;
+}
+
+static bool
+vu_set_config(VuDev *dev, VhostUserMsg *vmsg)
+{
+ int ret = -1;
+ bool reply_supported = !!(dev->protocol_features &
+ (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK));
+
+ if (dev->iface->set_config) {
+ ret = dev->iface->set_config(dev, vmsg->payload.config, vmsg->size);
+ }
+
+ vmsg->size = sizeof(vmsg->payload.u64);
+ if (!ret) {
+ vmsg->payload.u64 = 0;
+ } else {
+ /* indicate an error in case reply supported */
+ vmsg->payload.u64 = 1;
+ }
+
+ if (reply_supported) {
+ return true;
+ }
+
+ return false;
+}
+
+static bool
+vu_set_config_fd(VuDev *dev, VhostUserMsg *vmsg)
+{
+ if (vmsg->fd_num != 1) {
+ vu_panic(dev, "Invalid config_fd message");
+ return false;
+ }
+
+ if (dev->config_fd != -1) {
+ close(dev->config_fd);
+ }
+ dev->config_fd = vmsg->fds[0];
+ DPRINT("Got config_fd: %d\n", vmsg->fds[0]);
+
+ return false;
+}
+
+static bool
vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
{
int do_reply = 0;
@@ -862,6 +925,12 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
return vu_set_vring_enable_exec(dev, vmsg);
case VHOST_USER_SET_SLAVE_REQ_FD:
return vu_set_slave_req_fd(dev, vmsg);
+ case VHOST_USER_GET_CONFIG:
+ return vu_get_config(dev, vmsg);
+ case VHOST_USER_SET_CONFIG:
+ return vu_set_config(dev, vmsg);
+ case VHOST_USER_SET_CONFIG_FD:
+ return vu_set_config_fd(dev, vmsg);
case VHOST_USER_NONE:
break;
default:
@@ -940,6 +1009,10 @@ vu_deinit(VuDev *dev)
dev->slave_fd = -1;
}
+ if (dev->config_fd != -1) {
+ close(dev->config_fd);
+ }
+
if (dev->sock != -1) {
close(dev->sock);
}
@@ -970,6 +1043,8 @@ vu_init(VuDev *dev,
dev->iface = iface;
dev->log_call_fd = -1;
dev->slave_fd = -1;
+ dev->config_fd = -1;
+
for (i = 0; i < VHOST_MAX_NR_VIRTQUEUE; i++) {
dev->vq[i] = (VuVirtq) {
.call_fd = -1, .kick_fd = -1, .err_fd = -1,
diff --git a/contrib/libvhost-user/libvhost-user.h b/contrib/libvhost-user/libvhost-user.h
index 2f5864b..61e0dff 100644
--- a/contrib/libvhost-user/libvhost-user.h
+++ b/contrib/libvhost-user/libvhost-user.h
@@ -30,6 +30,8 @@
#define VHOST_MEMORY_MAX_NREGIONS 8
+#define VHOST_USER_MAX_CONFIG_SIZE 256
+
enum VhostUserProtocolFeature {
VHOST_USER_PROTOCOL_F_MQ = 0,
VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
@@ -38,7 +40,6 @@ enum VhostUserProtocolFeature {
VHOST_USER_PROTOCOL_F_NET_MTU = 4,
VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
-
VHOST_USER_PROTOCOL_F_MAX
};
@@ -69,6 +70,9 @@ typedef enum VhostUserRequest {
VHOST_USER_SET_SLAVE_REQ_FD = 21,
VHOST_USER_IOTLB_MSG = 22,
VHOST_USER_SET_VRING_ENDIAN = 23,
+ VHOST_USER_GET_CONFIG = 24,
+ VHOST_USER_SET_CONFIG = 25,
+ VHOST_USER_SET_CONFIG_FD = 26,
VHOST_USER_MAX
} VhostUserRequest;
@@ -112,6 +116,7 @@ typedef struct VhostUserMsg {
struct vhost_vring_addr addr;
VhostUserMemory memory;
VhostUserLog log;
+ uint8_t config[VHOST_USER_MAX_CONFIG_SIZE];
} payload;
int fds[VHOST_MEMORY_MAX_NREGIONS];
@@ -140,6 +145,9 @@ typedef int (*vu_process_msg_cb) (VuDev *dev, VhostUserMsg *vmsg,
int *do_reply);
typedef void (*vu_queue_set_started_cb) (VuDev *dev, int qidx, bool started);
typedef bool (*vu_queue_is_processed_in_order_cb) (VuDev *dev, int qidx);
+typedef int (*vu_get_config_cb) (VuDev *dev, uint8_t *config, size_t len);
+typedef int (*vu_set_config_cb) (VuDev *dev, const uint8_t *config,
+ size_t len);
typedef struct VuDevIface {
/* called by VHOST_USER_GET_FEATURES to get the features bitmask */
@@ -162,6 +170,10 @@ typedef struct VuDevIface {
* on unmanaged exit/crash.
*/
vu_queue_is_processed_in_order_cb queue_is_processed_in_order;
+ /* get the config space of the device */
+ vu_get_config_cb get_config;
+ /* set the config space of the device */
+ vu_set_config_cb set_config;
} VuDevIface;
typedef void (*vu_queue_handler_cb) (VuDev *dev, int qidx);
@@ -227,6 +239,7 @@ struct VuDev {
VuVirtq vq[VHOST_MAX_NR_VIRTQUEUE];
int log_call_fd;
int slave_fd;
+ int config_fd;
uint64_t log_size;
uint8_t *log_table;
uint64_t features;
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v4 4/4] contrib/vhost-user-blk: introduce a vhost-user-blk sample application
2017-10-19 5:24 [Qemu-devel] [PATCH v4 0/4] *** Introduce a new vhost-user-blk host device to Qemu *** Changpeng Liu
` (2 preceding siblings ...)
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 3/4] contrib/libvhost-user: enable virtio config space messages Changpeng Liu
@ 2017-10-19 5:24 ` Changpeng Liu
2017-10-19 11:43 ` Paolo Bonzini
3 siblings, 1 reply; 36+ messages in thread
From: Changpeng Liu @ 2017-10-19 5:24 UTC (permalink / raw)
To: changpeng.liu, qemu-devel
Cc: stefanha, pbonzini, mst, marcandre.lureau, felipe, james.r.harris
This commit introcudes a vhost-user-blk backend device, it uses UNIX
domain socket to communicate with Qemu. The vhost-user-blk sample
application should be used with Qemu vhost-user-blk-pci device.
To use it, complie with:
make vhost-user-blk
and start like this:
vhost-user-blk -b /dev/sdb -s /path/vhost.socket
Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
---
.gitignore | 1 +
Makefile | 3 +
Makefile.objs | 1 +
contrib/vhost-user-blk/Makefile.objs | 1 +
contrib/vhost-user-blk/vhost-user-blk.c | 492 ++++++++++++++++++++++++++++++++
5 files changed, 498 insertions(+)
create mode 100644 contrib/vhost-user-blk/Makefile.objs
create mode 100644 contrib/vhost-user-blk/vhost-user-blk.c
diff --git a/.gitignore b/.gitignore
index 620eec6..544276e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -53,6 +53,7 @@
/scsi/qemu-pr-helper
/vscclient
/vhost-user-scsi
+/vhost-user-blk
/fsdev/virtfs-proxy-helper
*.tmp
*.[1-9]
diff --git a/Makefile b/Makefile
index 90f91e5..be67a09 100644
--- a/Makefile
+++ b/Makefile
@@ -318,6 +318,7 @@ dummy := $(call unnest-vars,, \
ivshmem-server-obj-y \
libvhost-user-obj-y \
vhost-user-scsi-obj-y \
+ vhost-user-blk-obj-y \
qga-vss-dll-obj-y \
block-obj-y \
block-obj-m \
@@ -534,6 +535,8 @@ ivshmem-server$(EXESUF): $(ivshmem-server-obj-y) $(COMMON_LDADDS)
endif
vhost-user-scsi$(EXESUF): $(vhost-user-scsi-obj-y) libvhost-user.a
$(call LINK, $^)
+vhost-user-blk$(EXESUF): $(vhost-user-blk-obj-y) libvhost-user.a
+ $(call LINK, $^)
module_block.h: $(SRC_PATH)/scripts/modules/module_block.py config-host.mak
$(call quiet-command,$(PYTHON) $< $@ \
diff --git a/Makefile.objs b/Makefile.objs
index d4f973a..3af989d 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -115,6 +115,7 @@ libvhost-user-obj-y = contrib/libvhost-user/
vhost-user-scsi.o-cflags := $(LIBISCSI_CFLAGS)
vhost-user-scsi.o-libs := $(LIBISCSI_LIBS)
vhost-user-scsi-obj-y = contrib/vhost-user-scsi/
+vhost-user-blk-obj-y = contrib/vhost-user-blk/
######################################################################
trace-events-subdirs =
diff --git a/contrib/vhost-user-blk/Makefile.objs b/contrib/vhost-user-blk/Makefile.objs
new file mode 100644
index 0000000..72e2cdc
--- /dev/null
+++ b/contrib/vhost-user-blk/Makefile.objs
@@ -0,0 +1 @@
+vhost-user-blk-obj-y = vhost-user-blk.o
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
new file mode 100644
index 0000000..45050bc
--- /dev/null
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -0,0 +1,492 @@
+/*
+ * vhost-user-blk sample application
+ *
+ * Copyright IBM, Corp. 2007
+ * Copyright (c) 2016 Nutanix Inc. All rights reserved.
+ * Copyright (c) 2017 Intel Corporation. All rights reserved.
+ *
+ * Author:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ * Felipe Franciosi <felipe@nutanix.com>
+ * Changpeng Liu <changpeng.liu@intel.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 only.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "standard-headers/linux/virtio_blk.h"
+#include "contrib/libvhost-user/libvhost-user-glib.h"
+#include "contrib/libvhost-user/libvhost-user.h"
+
+#include <glib.h>
+
+struct virtio_blk_inhdr {
+ unsigned char status;
+};
+
+/* vhost user block device */
+typedef struct VubDev {
+ VugDev parent;
+ int blk_fd;
+ struct virtio_blk_config blkcfg;
+ char *blk_name;
+ GMainLoop *loop;
+} VubDev;
+
+typedef struct VubReq {
+ VuVirtqElement *elem;
+ int64_t sector_num;
+ size_t size;
+ struct virtio_blk_inhdr *in;
+ struct virtio_blk_outhdr *out;
+ VubDev *vdev_blk;
+ struct VuVirtq *vq;
+} VubReq;
+
+/** refer util/iov.c **/
+static size_t vub_iov_size(const struct iovec *iov,
+ const unsigned int iov_cnt)
+{
+ size_t len;
+ unsigned int i;
+
+ len = 0;
+ for (i = 0; i < iov_cnt; i++) {
+ len += iov[i].iov_len;
+ }
+ return len;
+}
+
+static void vub_panic_cb(VuDev *vu_dev, const char *buf)
+{
+ VugDev *gdev;
+ VubDev *vdev_blk;
+
+ assert(vu_dev);
+
+ gdev = container_of(vu_dev, VugDev, parent);
+ vdev_blk = container_of(gdev, VubDev, parent);
+ if (buf) {
+ g_warning("vu_panic: %s", buf);
+ }
+
+ g_main_loop_quit(vdev_blk->loop);
+}
+
+static void vub_req_complete(VubReq *req)
+{
+ VugDev *gdev = &req->vdev_blk->parent;
+ VuDev *vu_dev = &gdev->parent;
+
+ /* IO size with 1 extra status byte */
+ vu_queue_push(vu_dev, req->vq, req->elem,
+ req->size + 1);
+ vu_queue_notify(vu_dev, req->vq);
+
+ if (req->elem) {
+ free(req->elem);
+ }
+ g_free(req);
+}
+
+static int vub_open(const char *file_name)
+{
+ int fd;
+
+ fd = open(file_name, O_RDWR | O_DIRECT);
+ if (fd < 0) {
+ fprintf(stderr, "Cannot open file %s, %s\n", file_name,
+ strerror(errno));
+ return -1;
+ }
+
+ return fd;
+}
+
+static void vub_close(int fd)
+{
+ if (fd >= 0) {
+ close(fd);
+ }
+}
+
+static ssize_t
+vub_readv(VubReq *req, struct iovec *iov, uint32_t iovcnt)
+{
+ VubDev *vdev_blk = req->vdev_blk;
+ ssize_t rc;
+
+ if (!iovcnt) {
+ fprintf(stderr, "Invalid Read IOV count\n");
+ return -1;
+ }
+
+ req->size = vub_iov_size(iov, iovcnt);
+ rc = preadv(vdev_blk->blk_fd, iov, iovcnt, req->sector_num * 512);
+ if (rc < 0) {
+ fprintf(stderr, "Block %s, Sector %"PRIu64", Size %lu Read Failed\n",
+ vdev_blk->blk_name, req->sector_num, req->size);
+ return -1;
+ }
+
+ return rc;
+}
+
+static ssize_t
+vub_writev(VubReq *req, struct iovec *iov, uint32_t iovcnt)
+{
+ VubDev *vdev_blk = req->vdev_blk;
+ ssize_t rc;
+
+ if (!iovcnt) {
+ fprintf(stderr, "Invalid Write IOV count\n");
+ return -1;
+ }
+
+ req->size = vub_iov_size(iov, iovcnt);
+ rc = pwritev(vdev_blk->blk_fd, iov, iovcnt, req->sector_num * 512);
+ if (rc < 0) {
+ fprintf(stderr, "Block %s, Sector %"PRIu64", Size %lu Write Failed\n",
+ vdev_blk->blk_name, req->sector_num, req->size);
+ return -1;
+ }
+
+ return rc;
+}
+
+static void
+vub_flush(VubReq *req)
+{
+ VubDev *vdev_blk = req->vdev_blk;
+
+ if (vdev_blk->blk_fd) {
+ fsync(vdev_blk->blk_fd);
+ }
+}
+
+
+static int vub_virtio_process_req(VubDev *vdev_blk,
+ VuVirtq *vq)
+{
+ VugDev *gdev = &vdev_blk->parent;
+ VuDev *vu_dev = &gdev->parent;
+ VuVirtqElement *elem;
+ uint32_t type;
+ unsigned in_num;
+ unsigned out_num;
+ VubReq *req;
+
+ elem = vu_queue_pop(vu_dev, vq, sizeof(VuVirtqElement));
+ if (!elem) {
+ return -1;
+ }
+
+ /* refer to hw/block/virtio_blk.c */
+ if (elem->out_num < 1 || elem->in_num < 1) {
+ fprintf(stderr, "virtio-blk request missing headers\n");
+ free(elem);
+ return -1;
+ }
+
+ req = g_new0(VubReq, 1);
+ req->vdev_blk = vdev_blk;
+ req->vq = vq;
+ req->elem = elem;
+
+ in_num = elem->in_num;
+ out_num = elem->out_num;
+
+ /* don't support VIRTIO_F_ANY_LAYOUT and virtio 1.0 only */
+ if (elem->out_sg[0].iov_len < sizeof(struct virtio_blk_outhdr)) {
+ fprintf(stderr, "Invalid outhdr size\n");
+ goto err;
+ }
+ req->out = (struct virtio_blk_outhdr *)elem->out_sg[0].iov_base;
+ out_num--;
+
+ if (elem->in_sg[in_num - 1].iov_len < sizeof(struct virtio_blk_inhdr)) {
+ fprintf(stderr, "Invalid inhdr size\n");
+ goto err;
+ }
+ req->in = (struct virtio_blk_inhdr *)elem->in_sg[in_num - 1].iov_base;
+ in_num--;
+
+ type = le32toh(req->out->type);
+ switch (type & ~(VIRTIO_BLK_T_OUT | VIRTIO_BLK_T_BARRIER)) {
+ case VIRTIO_BLK_T_IN: {
+ ssize_t ret = 0;
+ bool is_write = type & VIRTIO_BLK_T_OUT;
+ req->sector_num = le64toh(req->out->sector);
+ if (is_write) {
+ ret = vub_writev(req, &elem->out_sg[1], out_num);
+ } else {
+ ret = vub_readv(req, &elem->in_sg[0], in_num);
+ }
+ if (ret >= 0) {
+ req->in->status = VIRTIO_BLK_S_OK;
+ } else {
+ req->in->status = VIRTIO_BLK_S_IOERR;
+ }
+ vub_req_complete(req);
+ break;
+ }
+ case VIRTIO_BLK_T_FLUSH: {
+ vub_flush(req);
+ req->in->status = VIRTIO_BLK_S_OK;
+ vub_req_complete(req);
+ break;
+ }
+ case VIRTIO_BLK_T_GET_ID: {
+ size_t size = MIN(vub_iov_size(&elem->in_sg[0], in_num),
+ VIRTIO_BLK_ID_BYTES);
+ snprintf(elem->in_sg[0].iov_base, size, "%s", "vhost_user_blk");
+ req->in->status = VIRTIO_BLK_S_OK;
+ req->size = elem->in_sg[0].iov_len;
+ vub_req_complete(req);
+ break;
+ }
+ default: {
+ req->in->status = VIRTIO_BLK_S_UNSUPP;
+ vub_req_complete(req);
+ break;
+ }
+ }
+
+ return 0;
+
+err:
+ free(elem);
+ g_free(req);
+ return -1;
+}
+
+static void vub_process_vq(VuDev *vu_dev, int idx)
+{
+ VugDev *gdev;
+ VubDev *vdev_blk;
+ VuVirtq *vq;
+ int ret;
+
+ if ((idx < 0) || (idx >= VHOST_MAX_NR_VIRTQUEUE)) {
+ fprintf(stderr, "VQ Index out of range: %d\n", idx);
+ vub_panic_cb(vu_dev, NULL);
+ return;
+ }
+
+ gdev = container_of(vu_dev, VugDev, parent);
+ vdev_blk = container_of(gdev, VubDev, parent);
+ assert(vdev_blk);
+
+ vq = vu_get_queue(vu_dev, idx);
+ assert(vq);
+
+ while (1) {
+ ret = vub_virtio_process_req(vdev_blk, vq);
+ if (ret) {
+ break;
+ }
+ }
+}
+
+static void vub_queue_set_started(VuDev *vu_dev, int idx, bool started)
+{
+ VuVirtq *vq;
+
+ assert(vu_dev);
+
+ if ((idx < 0) || (idx >= VHOST_MAX_NR_VIRTQUEUE)) {
+ fprintf(stderr, "VQ Index out of range: %d\n", idx);
+ vub_panic_cb(vu_dev, NULL);
+ return;
+ }
+
+ vq = vu_get_queue(vu_dev, idx);
+ vu_set_queue_handler(vu_dev, vq, started ? vub_process_vq : NULL);
+}
+
+static uint64_t
+vub_get_features(VuDev *dev)
+{
+ return 1ull << VIRTIO_BLK_F_SIZE_MAX |
+ 1ull << VIRTIO_BLK_F_SEG_MAX |
+ 1ull << VIRTIO_BLK_F_TOPOLOGY |
+ 1ull << VIRTIO_BLK_F_BLK_SIZE |
+ 1ull << VIRTIO_F_VERSION_1 |
+ 1ull << VHOST_USER_F_PROTOCOL_FEATURES;
+}
+
+static int
+vub_get_config(VuDev *vu_dev, uint8_t *config, size_t len)
+{
+ VugDev *gdev;
+ VubDev *vdev_blk;
+
+ if (len != sizeof(struct virtio_blk_config)) {
+ return -1;
+ }
+ gdev = container_of(vu_dev, VugDev, parent);
+ vdev_blk = container_of(gdev, VubDev, parent);
+ memcpy(config, &vdev_blk->blkcfg, len);
+
+ return 0;
+}
+
+static const VuDevIface vub_iface = {
+ .get_features = vub_get_features,
+ .queue_set_started = vub_queue_set_started,
+ .get_config = vub_get_config,
+};
+
+static int unix_sock_new(char *unix_fn)
+{
+ int sock;
+ struct sockaddr_un un;
+ size_t len;
+
+ assert(unix_fn);
+
+ sock = socket(AF_UNIX, SOCK_STREAM, 0);
+ if (sock <= 0) {
+ perror("socket");
+ return -1;
+ }
+
+ un.sun_family = AF_UNIX;
+ (void)snprintf(un.sun_path, sizeof(un.sun_path), "%s", unix_fn);
+ len = sizeof(un.sun_family) + strlen(un.sun_path);
+
+ (void)unlink(unix_fn);
+ if (bind(sock, (struct sockaddr *)&un, len) < 0) {
+ perror("bind");
+ goto fail;
+ }
+
+ if (listen(sock, 1) < 0) {
+ perror("listen");
+ goto fail;
+ }
+
+ return sock;
+
+fail:
+ (void)close(sock);
+
+ return -1;
+}
+
+static void vub_deinit(struct VubDev *vdev_blk)
+{
+ if (!vdev_blk) {
+ return;
+ }
+
+ if (vdev_blk->blk_fd) {
+ vub_close(vdev_blk->blk_fd);
+ }
+}
+
+static void
+vub_initialize_config(int fd, struct virtio_blk_config *config)
+{
+ off64_t capacity;
+
+ capacity = lseek64(fd, 0, SEEK_END);
+ config->capacity = capacity >> 9;
+ config->blk_size = 512;
+ config->size_max = 65536;
+ config->seg_max = 128 - 2;
+ config->min_io_size = 1;
+ config->opt_io_size = 1;
+ config->num_queues = 1;
+}
+
+static int
+vub_new_block(VubDev *vdev_blk, char *blk_file)
+{
+ vdev_blk->blk_fd = vub_open(blk_file);
+ if (vdev_blk->blk_fd < 0) {
+ fprintf(stderr, "Error to open block device %s\n", blk_file);
+ return -1;
+ }
+ vdev_blk->blk_name = blk_file;
+
+ /* fill virtio_blk_config with block parameters */
+ vub_initialize_config(vdev_blk->blk_fd, &vdev_blk->blkcfg);
+
+ return 0;
+}
+
+int main(int argc, char **argv)
+{
+ int opt;
+ char *unix_socket = NULL;
+ char *blk_file = NULL;
+ int lsock = -1, csock = -1, rc;
+ VubDev *vdev_blk = NULL;
+
+ while ((opt = getopt(argc, argv, "b:h:s:")) != -1) {
+ switch (opt) {
+ case 'b':
+ blk_file = g_strdup(optarg);
+ break;
+ case 's':
+ unix_socket = g_strdup(optarg);
+ break;
+ case 'h':
+ default:
+ printf("Usage: %s [-b block device or file, -s UNIX domain socket]"
+ " | [ -h ]\n", argv[0]);
+ return -1;
+ }
+ }
+
+ if (!unix_socket || !blk_file) {
+ printf("Usage: %s [-b block device or file, -s UNIX domain socket] |"
+ " [ -h ]\n", argv[0]);
+ return -1;
+ }
+
+ lsock = unix_sock_new(unix_socket);
+ if (lsock < 0) {
+ goto err;
+ }
+
+ csock = accept(lsock, (void *)0, (void *)0);
+ if (csock < 0) {
+ fprintf(stderr, "Accept error %s\n", strerror(errno));
+ goto err;
+ }
+
+ vdev_blk = g_new0(VubDev, 1);
+ vdev_blk->loop = g_main_loop_new(NULL, FALSE);
+
+ rc = vub_new_block(vdev_blk, blk_file);
+ if (rc) {
+ goto err;
+ }
+
+ vug_init(&vdev_blk->parent, csock, vub_panic_cb, &vub_iface);
+
+ g_main_loop_run(vdev_blk->loop);
+
+ vug_deinit(&vdev_blk->parent);
+
+err:
+ if (vdev_blk) {
+ g_main_loop_unref(vdev_blk->loop);
+ vub_deinit(vdev_blk);
+ g_free(vdev_blk);
+ }
+ if (csock >= 0) {
+ close(csock);
+ }
+ if (lsock >= 0) {
+ close(lsock);
+ }
+ g_free(unix_socket);
+ g_free(blk_file);
+
+ return 0;
+}
+
--
1.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device Changpeng Liu
@ 2017-10-19 11:33 ` Paolo Bonzini
2017-10-20 1:38 ` Liu, Changpeng
2017-10-19 15:17 ` Stefan Hajnoczi
1 sibling, 1 reply; 36+ messages in thread
From: Paolo Bonzini @ 2017-10-19 11:33 UTC (permalink / raw)
To: Changpeng Liu, qemu-devel
Cc: stefanha, mst, marcandre.lureau, felipe, james.r.harris
On 19/10/2017 07:24, Changpeng Liu wrote:
> ;;
> --enable-vhost-scsi) vhost_scsi="yes"
> ;;
> + --disable-vhost-user-blk) vhost_user_blk="no"
> + ;;
> + --enable-vhost-user-blk) vhost_user_blk="yes"
> + ;;
> --disable-vhost-vsock) vhost_vsock="no"
> ;;
> --enable-vhost-vsock) vhost_vsock="yes"
> @@ -1511,6 +1517,7 @@ disabled with --disable-FEATURE, default is enabled if available:
> cap-ng libcap-ng support
> attr attr and xattr support
> vhost-net vhost-net acceleration support
> + vhost-user-blk VM virtio-blk acceleration in user space
Please use default-configs instead of a new configure switch. See how
CONFIG_VHOST_USER_SCSI is used in default-configs/pci.mak and
default-configs/s390x-softmmu.mak.
>
> +static const int user_feature_bits[] = {
> + VIRTIO_BLK_F_SIZE_MAX,
> + VIRTIO_BLK_F_SEG_MAX,
> + VIRTIO_BLK_F_GEOMETRY,
> + VIRTIO_BLK_F_BLK_SIZE,
> + VIRTIO_BLK_F_TOPOLOGY,
> + VIRTIO_BLK_F_SCSI,
Please omit VIRTIO_BLK_F_SCSI, it's a legacy option that is not anymore
part of virtio 1.0.
> + VIRTIO_BLK_F_MQ,
> + VIRTIO_BLK_F_RO,
> + VIRTIO_BLK_F_FLUSH,
> + VIRTIO_BLK_F_BARRIER,
Same for VIRTIO_BLK_F_BARRIER.
> + VIRTIO_BLK_F_WCE,
And VIRTIO_BLK_F_WCE is the same as VIRTIO_BLK_F_FLUSH, so it can be
removed too. Please include VIRTIO_BLK_F_CONFIG_WCE instead, since you
are supporting it in vhost_user_blk_set_config.
> + VIRTIO_F_VERSION_1,
> + VIRTIO_RING_F_INDIRECT_DESC,
> + VIRTIO_RING_F_EVENT_IDX,
> + VIRTIO_F_NOTIFY_ON_EMPTY,
> + VHOST_INVALID_FEATURE_BIT
> +};
>
> +static const TypeInfo vhost_user_blk_info = {
> + .name = TYPE_VHOST_USER_BLK,
> + .parent = TYPE_VIRTIO_DEVICE,
> + .instance_size = sizeof(VHostUserBlk),
> + .instance_init = vhost_user_blk_instance_init,
> + .class_init = vhost_user_blk_class_init,
> +};
> +
There is some code duplication, so maybe it's worth introducing a common
superclass like TYPE_VIRTIO_SCSI_COMMON. I'll let others comment on
whether this is a requirement.
Paolo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space Changpeng Liu
@ 2017-10-19 11:37 ` Paolo Bonzini
2017-10-19 14:09 ` Stefan Hajnoczi
2017-10-19 15:39 ` Michael S. Tsirkin
2 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2017-10-19 11:37 UTC (permalink / raw)
To: Changpeng Liu, qemu-devel
Cc: stefanha, mst, marcandre.lureau, felipe, james.r.harris
On 19/10/2017 07:24, Changpeng Liu wrote:
> Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages which can be
> used for live migration of vhost user devices, also vhost user devices
> can benefit from the messages to get/set virtio config space from/to the
> I/O target. For the purpose to support virtio config space change,
> VHOST_USER_SET_CONFIG_FD message is added as the event notifier
> in case virtio config space change in the I/O target.
>
> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> ---
> docs/interop/vhost-user.txt | 33 +++++++++++++
> hw/virtio/vhost-user.c | 97 +++++++++++++++++++++++++++++++++++++++
> hw/virtio/vhost.c | 63 +++++++++++++++++++++++++
> include/hw/virtio/vhost-backend.h | 8 ++++
> include/hw/virtio/vhost.h | 16 +++++++
> 5 files changed, 217 insertions(+)
>
> diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> index 954771d..ea4c5ca 100644
> --- a/docs/interop/vhost-user.txt
> +++ b/docs/interop/vhost-user.txt
> @@ -116,6 +116,10 @@ Depending on the request type, payload can be:
> - 3: IOTLB invalidate
> - 4: IOTLB access fail
>
> + * Virtio device config space
> +
> + 256 Bytes static virito device config space
"A 256-bytes array holding the contents of the virtio device's
configuration space"
> +* VHOST_USER_SET_CONFIG_FD
> + Id: 26
> + Equivalent ioctl: N/A
> + Master payload: N/A
> +
> + Sets the notifier file descriptor, which is passed as ancillary data.
> + Vhost-user slave uses the file descriptor to notify vhost-user master
"The vhost-user slave uses the file descriptor to notify the vhost-user
master of changes to the virtio configuration space."
Paolo
> + when the virtio config space changed. The vhost-user master can read
> + the virtio config space to get the latest update.
> +
> Slave message types
> -------------------
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 093675e..4e7bafc 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -26,6 +26,11 @@
> #define VHOST_MEMORY_MAX_NREGIONS 8
> #define VHOST_USER_F_PROTOCOL_FEATURES 30
>
> +/*
> + * Maximum size of virtio device config space
> + */
> +#define VHOST_USER_MAX_CONFIG_SIZE 256
> +
> enum VhostUserProtocolFeature {
> VHOST_USER_PROTOCOL_F_MQ = 0,
> VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
> @@ -65,6 +70,9 @@ typedef enum VhostUserRequest {
> VHOST_USER_SET_SLAVE_REQ_FD = 21,
> VHOST_USER_IOTLB_MSG = 22,
> VHOST_USER_SET_VRING_ENDIAN = 23,
> + VHOST_USER_GET_CONFIG = 24,
> + VHOST_USER_SET_CONFIG = 25,
> + VHOST_USER_SET_CONFIG_FD = 26,
> VHOST_USER_MAX
> } VhostUserRequest;
>
> @@ -109,6 +117,7 @@ typedef struct VhostUserMsg {
> VhostUserMemory memory;
> VhostUserLog log;
> struct vhost_iotlb_msg iotlb;
> + uint8_t config[VHOST_USER_MAX_CONFIG_SIZE];
> } payload;
> } QEMU_PACKED VhostUserMsg;
>
> @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
> /* No-op as the receive channel is not dedicated to IOTLB messages. */
> }
>
> +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
> + size_t config_len)
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_GET_CONFIG,
> + .flags = VHOST_USER_VERSION,
> + .size = config_len,
> + };
> +
> + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
> + error_report("bad config length");
> + return -1;
> + }
> +
> + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> + return -1;
> + }
> +
> + if (vhost_user_read(dev, &msg) < 0) {
> + return -1;
> + }
> +
> + if (msg.request != VHOST_USER_GET_CONFIG) {
> + error_report("Received unexpected msg type. Expected %d received %d",
> + VHOST_USER_GET_CONFIG, msg.request);
> + return -1;
> + }
> +
> + if (msg.size != config_len) {
> + error_report("Received bad msg size.");
> + return -1;
> + }
> +
> + memcpy(config, &msg.payload.config, config_len);
> +
> + return 0;
> +}
> +
> +static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *config,
> + size_t config_len)
> +{
> + bool reply_supported = virtio_has_feature(dev->protocol_features,
> + VHOST_USER_PROTOCOL_F_REPLY_ACK);
> +
> + VhostUserMsg msg = {
> + .request = VHOST_USER_SET_CONFIG,
> + .flags = VHOST_USER_VERSION,
> + .size = config_len,
> + };
> +
> + if (reply_supported) {
> + msg.flags |= VHOST_USER_NEED_REPLY_MASK;
> + }
> +
> + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
> + error_report("bad config length");
> + return -1;
> + }
> +
> + memcpy(&msg.payload.config, config, config_len);
> + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> + return -1;
> + }
> +
> + if (reply_supported) {
> + return process_message_reply(dev, &msg);
> + }
> +
> + return 0;
> +}
> +
> +static int vhost_user_set_config_fd(struct vhost_dev *dev, int fd)
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_SET_CONFIG_FD,
> + .flags = VHOST_USER_VERSION,
> + };
> +
> + if (vhost_user_write(dev, &msg, &fd, 1) < 0) {
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> const VhostOps user_ops = {
> .backend_type = VHOST_BACKEND_TYPE_USER,
> .vhost_backend_init = vhost_user_init,
> @@ -948,4 +1042,7 @@ const VhostOps user_ops = {
> .vhost_net_set_mtu = vhost_user_net_set_mtu,
> .vhost_set_iotlb_callback = vhost_user_set_iotlb_callback,
> .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg,
> + .vhost_get_config = vhost_user_get_config,
> + .vhost_set_config = vhost_user_set_config,
> + .vhost_set_config_fd = vhost_user_set_config_fd,
> };
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index ddc42f0..8079f46 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1354,6 +1354,9 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
> for (i = 0; i < hdev->nvqs; ++i) {
> vhost_virtqueue_cleanup(hdev->vqs + i);
> }
> + if (hdev->config_ops) {
> + event_notifier_cleanup(&hdev->config_notifier);
> + }
> if (hdev->mem) {
> /* those are only safe after successful init */
> memory_listener_unregister(&hdev->memory_listener);
> @@ -1501,6 +1504,66 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
> }
> }
>
> +int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
> + size_t config_len)
> +{
> + assert(hdev->vhost_ops);
> +
> + if (hdev->vhost_ops->vhost_get_config) {
> + return hdev->vhost_ops->vhost_get_config(hdev, config, config_len);
> + }
> +
> + return 0;
> +}
> +
> +int vhost_dev_set_config(struct vhost_dev *hdev, const uint8_t *config,
> + size_t config_len)
> +{
> + assert(hdev->vhost_ops);
> +
> + if (hdev->vhost_ops->vhost_set_config) {
> + return hdev->vhost_ops->vhost_set_config(hdev, config, config_len);
> + }
> +
> + return 0;
> +}
> +
> +static void vhost_dev_config_notifier_read(EventNotifier *n)
> +{
> + struct vhost_dev *hdev = container_of(n, struct vhost_dev,
> + config_notifier);
> +
> + if (event_notifier_test_and_clear(n)) {
> + if (hdev->config_ops) {
> + hdev->config_ops->vhost_dev_config_notifier(hdev);
> + }
> + }
> +}
> +
> +int vhost_dev_set_config_notifier(struct vhost_dev *hdev,
> + const VhostDevConfigOps *ops)
> +{
> + int r, fd;
> +
> + assert(hdev->vhost_ops);
> +
> + r = event_notifier_init(&hdev->config_notifier, 0);
> + if (r < 0) {
> + return r;
> + }
> +
> + hdev->config_ops = ops;
> + event_notifier_set_handler(&hdev->config_notifier,
> + vhost_dev_config_notifier_read);
> +
> + if (hdev->vhost_ops->vhost_set_config_fd) {
> + fd = event_notifier_get_fd(&hdev->config_notifier);
> + return hdev->vhost_ops->vhost_set_config_fd(hdev, fd);
> + }
> +
> + return 0;
> +}
> +
> /* Host notifiers must be enabled at this point. */
> int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> {
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index a7a5f22..df6769e 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -84,6 +84,11 @@ typedef void (*vhost_set_iotlb_callback_op)(struct vhost_dev *dev,
> int enabled);
> typedef int (*vhost_send_device_iotlb_msg_op)(struct vhost_dev *dev,
> struct vhost_iotlb_msg *imsg);
> +typedef int (*vhost_set_config_op)(struct vhost_dev *dev, const uint8_t *config,
> + size_t config_len);
> +typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config,
> + size_t config_len);
> +typedef int (*vhost_set_config_fd_op)(struct vhost_dev *dev, int fd);
>
> typedef struct VhostOps {
> VhostBackendType backend_type;
> @@ -118,6 +123,9 @@ typedef struct VhostOps {
> vhost_vsock_set_running_op vhost_vsock_set_running;
> vhost_set_iotlb_callback_op vhost_set_iotlb_callback;
> vhost_send_device_iotlb_msg_op vhost_send_device_iotlb_msg;
> + vhost_get_config_op vhost_get_config;
> + vhost_set_config_op vhost_set_config;
> + vhost_set_config_fd_op vhost_set_config_fd;
> } VhostOps;
>
> extern const VhostOps user_ops;
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 467dc77..ff172f2 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -46,6 +46,12 @@ struct vhost_iommu {
> QLIST_ENTRY(vhost_iommu) iommu_next;
> };
>
> +typedef struct VhostDevConfigOps {
> + /* Vhost device config space changed callback
> + */
> + void (*vhost_dev_config_notifier)(struct vhost_dev *dev);
> +} VhostDevConfigOps;
> +
> struct vhost_memory;
> struct vhost_dev {
> VirtIODevice *vdev;
> @@ -76,6 +82,8 @@ struct vhost_dev {
> QLIST_ENTRY(vhost_dev) entry;
> QLIST_HEAD(, vhost_iommu) iommu_list;
> IOMMUNotifier n;
> + EventNotifier config_notifier;
> + const VhostDevConfigOps *config_ops;
> };
>
> int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> @@ -106,4 +114,12 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
> struct vhost_vring_file *file);
>
> int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
> +int vhost_dev_get_config(struct vhost_dev *dev, uint8_t *config,
> + size_t config_len);
> +int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *config,
> + size_t config_len);
> +/* notifier callback in case vhost device config space changed
> + */
> +int vhost_dev_set_config_notifier(struct vhost_dev *dev,
> + const VhostDevConfigOps *ops);
> #endif
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/4] contrib/vhost-user-blk: introduce a vhost-user-blk sample application
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 4/4] contrib/vhost-user-blk: introduce a vhost-user-blk sample application Changpeng Liu
@ 2017-10-19 11:43 ` Paolo Bonzini
2017-10-20 1:39 ` Liu, Changpeng
0 siblings, 1 reply; 36+ messages in thread
From: Paolo Bonzini @ 2017-10-19 11:43 UTC (permalink / raw)
To: Changpeng Liu, qemu-devel
Cc: stefanha, mst, marcandre.lureau, felipe, james.r.harris
On 19/10/2017 07:24, Changpeng Liu wrote:
>
> +}
> +
> +static void
> +vub_flush(VubReq *req)
> +{
> + VubDev *vdev_blk = req->vdev_blk;
> +
> + if (vdev_blk->blk_fd) {
> + fsync(vdev_blk->blk_fd);
> + }
> +}
> +
No need to check the file descriptor---vub_readv and vub_writev aren't
checking it either. Also please use fdatasync instead of fsync.
> +static uint64_t
> +vub_get_features(VuDev *dev)
> +{
> + return 1ull << VIRTIO_BLK_F_SIZE_MAX |
> + 1ull << VIRTIO_BLK_F_SEG_MAX |
> + 1ull << VIRTIO_BLK_F_TOPOLOGY |
> + 1ull << VIRTIO_BLK_F_BLK_SIZE |
> + 1ull << VIRTIO_F_VERSION_1 |
> + 1ull << VHOST_USER_F_PROTOCOL_FEATURES;
> +}
VIRTIO_BLK_F_FLUSH is missing.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space Changpeng Liu
2017-10-19 11:37 ` Paolo Bonzini
@ 2017-10-19 14:09 ` Stefan Hajnoczi
2017-10-19 15:36 ` Michael S. Tsirkin
2017-10-19 15:39 ` Michael S. Tsirkin
2 siblings, 1 reply; 36+ messages in thread
From: Stefan Hajnoczi @ 2017-10-19 14:09 UTC (permalink / raw)
To: Changpeng Liu
Cc: qemu-devel, pbonzini, mst, marcandre.lureau, felipe,
james.r.harris
On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote:
> @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
> /* No-op as the receive channel is not dedicated to IOTLB messages. */
> }
>
> +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
> + size_t config_len)
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_GET_CONFIG,
> + .flags = VHOST_USER_VERSION,
> + .size = config_len,
> + };
> +
> + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
config_len should be limited to 256 bytes:
if (config_len == 0 || config_len > sizeof(msg.payload.config) {
> + error_report("bad config length");
> + return -1;
> + }
> +
> + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> + return -1;
> + }
> +
> + if (vhost_user_read(dev, &msg) < 0) {
> + return -1;
> + }
> +
> + if (msg.request != VHOST_USER_GET_CONFIG) {
> + error_report("Received unexpected msg type. Expected %d received %d",
> + VHOST_USER_GET_CONFIG, msg.request);
> + return -1;
> + }
> +
> + if (msg.size != config_len) {
> + error_report("Received bad msg size.");
> + return -1;
> + }
> +
> + memcpy(config, &msg.payload.config, config_len);
There is some complexity here: different virtio devices use different
amounts of config space. Devices may append new fields to the config
space to support new features.
Therefore I think the simplest protocol is to always fetch the full
256-byte configuration space. This way the vhost-user slave process can
implement feature bits that the master process does not know about.
In other words, I don't think the master process knows how much of the
config space is used so it should always request 256 bytes.
> + return 0;
> +}
> +
> +static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *config,
> + size_t config_len)
> +{
> + bool reply_supported = virtio_has_feature(dev->protocol_features,
> + VHOST_USER_PROTOCOL_F_REPLY_ACK);
> +
> + VhostUserMsg msg = {
> + .request = VHOST_USER_SET_CONFIG,
> + .flags = VHOST_USER_VERSION,
> + .size = config_len,
> + };
> +
> + if (reply_supported) {
> + msg.flags |= VHOST_USER_NEED_REPLY_MASK;
> + }
> +
> + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
Same thing here: config_len > sizeof(msg.payload.config)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device Changpeng Liu
2017-10-19 11:33 ` Paolo Bonzini
@ 2017-10-19 15:17 ` Stefan Hajnoczi
2017-10-20 1:47 ` Liu, Changpeng
1 sibling, 1 reply; 36+ messages in thread
From: Stefan Hajnoczi @ 2017-10-19 15:17 UTC (permalink / raw)
To: Changpeng Liu
Cc: qemu-devel, pbonzini, mst, marcandre.lureau, felipe,
james.r.harris
On Thu, Oct 19, 2017 at 01:24:08PM +0800, Changpeng Liu wrote:
> This commit introduces a new vhost-user device for block, it uses a
> chardev to connect with the backend, same with Qemu virito-blk device,
> Guest OS still uses the virtio-blk frontend driver.
>
> To use it, start Qemu with command line like this:
>
> qemu-system-x86_64 \
> -chardev socket,id=char0,path=/path/vhost.socket \
> -device vhost-user-blk-pci,chardev=char0,num_queues=1, \
> bootindex=2... \
>
> Users can use different parameters for `num_queues` and `bootindex`.
>
> Different with exist Qemu virtio-blk host device, it makes more easy
> for users to implement their own I/O processing logic, such as all
> user space I/O stack against hardware block device. It uses the new
> vhost messages(VHOST_USER_GET_CONFIG) to get block virtio config
> information from backend process.
>
> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> ---
> configure | 11 ++
> hw/block/Makefile.objs | 3 +
> hw/block/vhost-user-blk.c | 360 +++++++++++++++++++++++++++++++++++++
> hw/virtio/virtio-pci.c | 55 ++++++
> hw/virtio/virtio-pci.h | 18 ++
> include/hw/virtio/vhost-user-blk.h | 40 +++++
> 6 files changed, 487 insertions(+)
> create mode 100644 hw/block/vhost-user-blk.c
> create mode 100644 include/hw/virtio/vhost-user-blk.h
>
> diff --git a/configure b/configure
> index 663e908..f2b348f 100755
> --- a/configure
> +++ b/configure
> @@ -318,6 +318,7 @@ tcg="yes"
>
> vhost_net="no"
> vhost_scsi="no"
> +vhost_user_blk="no"
> vhost_vsock="no"
> vhost_user=""
> kvm="no"
> @@ -782,6 +783,7 @@ Linux)
> kvm="yes"
> vhost_net="yes"
> vhost_scsi="yes"
> + vhost_user_blk="yes"
> vhost_vsock="yes"
> QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers $QEMU_INCLUDES"
> supported_os="yes"
> @@ -1139,6 +1141,10 @@ for opt do
> ;;
> --enable-vhost-scsi) vhost_scsi="yes"
> ;;
> + --disable-vhost-user-blk) vhost_user_blk="no"
> + ;;
> + --enable-vhost-user-blk) vhost_user_blk="yes"
> + ;;
> --disable-vhost-vsock) vhost_vsock="no"
> ;;
> --enable-vhost-vsock) vhost_vsock="yes"
> @@ -1511,6 +1517,7 @@ disabled with --disable-FEATURE, default is enabled if available:
> cap-ng libcap-ng support
> attr attr and xattr support
> vhost-net vhost-net acceleration support
> + vhost-user-blk VM virtio-blk acceleration in user space
> spice spice
> rbd rados block device (rbd)
> libiscsi iscsi support
> @@ -5417,6 +5424,7 @@ echo "posix_madvise $posix_madvise"
> echo "libcap-ng support $cap_ng"
> echo "vhost-net support $vhost_net"
> echo "vhost-scsi support $vhost_scsi"
> +echo "vhost-user-blk support $vhost_user_blk"
> echo "vhost-vsock support $vhost_vsock"
> echo "vhost-user support $vhost_user"
> echo "Trace backends $trace_backends"
> @@ -5845,6 +5853,9 @@ fi
> if test "$vhost_scsi" = "yes" ; then
> echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak
> fi
> +if test "$vhost_user_blk" = "yes" -a "$vhost_user" = "yes"; then
> + echo "CONFIG_VHOST_USER_BLK=y" >> $config_host_mak
> +fi
> if test "$vhost_net" = "yes" -a "$vhost_user" = "yes"; then
> echo "CONFIG_VHOST_NET_USED=y" >> $config_host_mak
> fi
> diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
> index e0ed980..4c19a58 100644
> --- a/hw/block/Makefile.objs
> +++ b/hw/block/Makefile.objs
> @@ -13,3 +13,6 @@ obj-$(CONFIG_SH4) += tc58128.o
>
> obj-$(CONFIG_VIRTIO) += virtio-blk.o
> obj-$(CONFIG_VIRTIO) += dataplane/
> +ifeq ($(CONFIG_VIRTIO),y)
> +obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk.o
> +endif
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> new file mode 100644
> index 0000000..8aa9fa9
> --- /dev/null
> +++ b/hw/block/vhost-user-blk.c
> @@ -0,0 +1,360 @@
> +/*
> + * vhost-user-blk host device
> + *
> + * Copyright IBM, Corp. 2011
> + * Copyright(C) 2017 Intel Corporation.
> + *
> + * Authors:
> + * Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> + * Changpeng Liu <changpeng.liu@intel.com>
This gives the impression that IBM originally authored this code but
little copied code is actually in this file. Feel free to put your own
copyright and authorship information at the top and then add a statement
like "Based on foo.c, Copyright IBM, Corp. 2011" instead. That way it's
clear that this is new code that you wrote with reference to the
original file you used as a starting point.
> +static Property vhost_user_blk_properties[] = {
> + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> + DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues, 1),
> + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128),
> + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_SIZE_MAX, true),
> + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_SIZE_MAX, true),
> + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_SEG_MAX, true),
> + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_GEOMETRY, true),
> + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_RO, false),
> + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_BLK_SIZE, true),
> + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_TOPOLOGY, true),
> + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_MQ, true),
> + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_FLUSH, true),
> + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_BARRIER, false),
> + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_SCSI, false),
> + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> + VIRTIO_BLK_F_WCE, false),
Please explain how feature negotation works. The vhost-user slave
advertises support features in the return value from
VHOST_USER_GET_FEATURES. How does this additional feature mask work and
why is it useful?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-19 14:09 ` Stefan Hajnoczi
@ 2017-10-19 15:36 ` Michael S. Tsirkin
2017-10-20 10:00 ` Stefan Hajnoczi
0 siblings, 1 reply; 36+ messages in thread
From: Michael S. Tsirkin @ 2017-10-19 15:36 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Changpeng Liu, qemu-devel, pbonzini, marcandre.lureau, felipe,
james.r.harris
On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote:
> On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote:
> > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
> > /* No-op as the receive channel is not dedicated to IOTLB messages. */
> > }
> >
> > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
> > + size_t config_len)
> > +{
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_GET_CONFIG,
> > + .flags = VHOST_USER_VERSION,
> > + .size = config_len,
> > + };
> > +
> > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
>
> config_len should be limited to 256 bytes:
>
> if (config_len == 0 || config_len > sizeof(msg.payload.config) {
I would just limit it to a reasonable value, acceptable to
both master and slave, not fail if it's bigger.
> > + error_report("bad config length");
> > + return -1;
> > + }
> > +
> > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> > + return -1;
> > + }
> > +
> > + if (vhost_user_read(dev, &msg) < 0) {
> > + return -1;
> > + }
> > +
> > + if (msg.request != VHOST_USER_GET_CONFIG) {
> > + error_report("Received unexpected msg type. Expected %d received %d",
> > + VHOST_USER_GET_CONFIG, msg.request);
> > + return -1;
> > + }
> > +
> > + if (msg.size != config_len) {
> > + error_report("Received bad msg size.");
> > + return -1;
> > + }
> > +
> > + memcpy(config, &msg.payload.config, config_len);
>
> There is some complexity here: different virtio devices use different
> amounts of config space. Devices may append new fields to the config
> space to support new features.
>
> Therefore I think the simplest protocol is to always fetch the full
> 256-byte configuration space. This way the vhost-user slave process can
> implement feature bits that the master process does not know about.
>
> In other words, I don't think the master process knows how much of the
> config space is used so it should always request 256 bytes.
Each device knows the max config space size.
vdev->config_len = config_size;
I don't think we need to hard-code 256 bytes in there.
> > + return 0;
> > +}
> > +
> > +static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *config,
> > + size_t config_len)
> > +{
> > + bool reply_supported = virtio_has_feature(dev->protocol_features,
> > + VHOST_USER_PROTOCOL_F_REPLY_ACK);
> > +
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_SET_CONFIG,
> > + .flags = VHOST_USER_VERSION,
> > + .size = config_len,
> > + };
> > +
> > + if (reply_supported) {
> > + msg.flags |= VHOST_USER_NEED_REPLY_MASK;
> > + }
> > +
> > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
>
> Same thing here: config_len > sizeof(msg.payload.config)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space Changpeng Liu
2017-10-19 11:37 ` Paolo Bonzini
2017-10-19 14:09 ` Stefan Hajnoczi
@ 2017-10-19 15:39 ` Michael S. Tsirkin
2017-10-19 15:43 ` Paolo Bonzini
2 siblings, 1 reply; 36+ messages in thread
From: Michael S. Tsirkin @ 2017-10-19 15:39 UTC (permalink / raw)
To: Changpeng Liu
Cc: qemu-devel, stefanha, pbonzini, marcandre.lureau, felipe,
james.r.harris
On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote:
> Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages which can be
> used for live migration of vhost user devices, also vhost user devices
> can benefit from the messages to get/set virtio config space from/to the
> I/O target. For the purpose to support virtio config space change,
> VHOST_USER_SET_CONFIG_FD message is added as the event notifier
> in case virtio config space change in the I/O target.
>
> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
I don't much like it that config is getting passed through.
IMO this makes managing things harder not easier.
How about specific messages about specific parts of
config space that you want to get from the backend?
> ---
> docs/interop/vhost-user.txt | 33 +++++++++++++
> hw/virtio/vhost-user.c | 97 +++++++++++++++++++++++++++++++++++++++
> hw/virtio/vhost.c | 63 +++++++++++++++++++++++++
> include/hw/virtio/vhost-backend.h | 8 ++++
> include/hw/virtio/vhost.h | 16 +++++++
> 5 files changed, 217 insertions(+)
>
> diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> index 954771d..ea4c5ca 100644
> --- a/docs/interop/vhost-user.txt
> +++ b/docs/interop/vhost-user.txt
> @@ -116,6 +116,10 @@ Depending on the request type, payload can be:
> - 3: IOTLB invalidate
> - 4: IOTLB access fail
>
> + * Virtio device config space
> +
> + 256 Bytes static virito device config space
> +
> In QEMU the vhost-user message is implemented with the following struct:
>
> typedef struct VhostUserMsg {
> @@ -129,6 +133,7 @@ typedef struct VhostUserMsg {
> VhostUserMemory memory;
> VhostUserLog log;
> struct vhost_iotlb_msg iotlb;
> + uint8_t config[256];
> };
> } QEMU_PACKED VhostUserMsg;
>
> @@ -596,6 +601,34 @@ Master message types
> and expect this message once (per VQ) during device configuration
> (ie. before the master starts the VQ).
>
> + * VHOST_USER_GET_CONFIG
> + Id: 24
> + Equivalent ioctl: N/A
> + Master payload: virtio device config space
> +
> + Submitted by the vhost-user master to fetch the contents of the virtio
> + device config space. The vhost-user master may cache the contents to
> + avoid repeated VHOST_USER_GET_CONFIG calls.
> +
> +* VHOST_USER_SET_CONFIG
> + Id: 25
> + Equivalent ioctl: N/A
> + Master payload: virtio device config space
> +
> + Submitted by the vhost-user master when the Guest changes the virtio
> + device config space and also can be used for live migration on the
> + destination host.
> +
> +* VHOST_USER_SET_CONFIG_FD
> + Id: 26
> + Equivalent ioctl: N/A
> + Master payload: N/A
> +
> + Sets the notifier file descriptor, which is passed as ancillary data.
> + Vhost-user slave uses the file descriptor to notify vhost-user master
> + when the virtio config space changed. The vhost-user master can read
> + the virtio config space to get the latest update.
> +
> Slave message types
> -------------------
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 093675e..4e7bafc 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -26,6 +26,11 @@
> #define VHOST_MEMORY_MAX_NREGIONS 8
> #define VHOST_USER_F_PROTOCOL_FEATURES 30
>
> +/*
> + * Maximum size of virtio device config space
> + */
> +#define VHOST_USER_MAX_CONFIG_SIZE 256
> +
> enum VhostUserProtocolFeature {
> VHOST_USER_PROTOCOL_F_MQ = 0,
> VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
> @@ -65,6 +70,9 @@ typedef enum VhostUserRequest {
> VHOST_USER_SET_SLAVE_REQ_FD = 21,
> VHOST_USER_IOTLB_MSG = 22,
> VHOST_USER_SET_VRING_ENDIAN = 23,
> + VHOST_USER_GET_CONFIG = 24,
> + VHOST_USER_SET_CONFIG = 25,
> + VHOST_USER_SET_CONFIG_FD = 26,
> VHOST_USER_MAX
> } VhostUserRequest;
>
> @@ -109,6 +117,7 @@ typedef struct VhostUserMsg {
> VhostUserMemory memory;
> VhostUserLog log;
> struct vhost_iotlb_msg iotlb;
> + uint8_t config[VHOST_USER_MAX_CONFIG_SIZE];
> } payload;
> } QEMU_PACKED VhostUserMsg;
>
> @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
> /* No-op as the receive channel is not dedicated to IOTLB messages. */
> }
>
> +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
> + size_t config_len)
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_GET_CONFIG,
> + .flags = VHOST_USER_VERSION,
> + .size = config_len,
> + };
> +
> + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
> + error_report("bad config length");
> + return -1;
> + }
> +
> + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> + return -1;
> + }
> +
> + if (vhost_user_read(dev, &msg) < 0) {
> + return -1;
> + }
> +
> + if (msg.request != VHOST_USER_GET_CONFIG) {
> + error_report("Received unexpected msg type. Expected %d received %d",
> + VHOST_USER_GET_CONFIG, msg.request);
> + return -1;
> + }
> +
> + if (msg.size != config_len) {
> + error_report("Received bad msg size.");
> + return -1;
> + }
> +
> + memcpy(config, &msg.payload.config, config_len);
> +
> + return 0;
> +}
> +
> +static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *config,
> + size_t config_len)
> +{
> + bool reply_supported = virtio_has_feature(dev->protocol_features,
> + VHOST_USER_PROTOCOL_F_REPLY_ACK);
> +
> + VhostUserMsg msg = {
> + .request = VHOST_USER_SET_CONFIG,
> + .flags = VHOST_USER_VERSION,
> + .size = config_len,
> + };
> +
> + if (reply_supported) {
> + msg.flags |= VHOST_USER_NEED_REPLY_MASK;
> + }
> +
> + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
> + error_report("bad config length");
> + return -1;
> + }
> +
> + memcpy(&msg.payload.config, config, config_len);
> + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> + return -1;
> + }
> +
> + if (reply_supported) {
> + return process_message_reply(dev, &msg);
> + }
> +
> + return 0;
> +}
> +
> +static int vhost_user_set_config_fd(struct vhost_dev *dev, int fd)
> +{
> + VhostUserMsg msg = {
> + .request = VHOST_USER_SET_CONFIG_FD,
> + .flags = VHOST_USER_VERSION,
> + };
> +
> + if (vhost_user_write(dev, &msg, &fd, 1) < 0) {
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> const VhostOps user_ops = {
> .backend_type = VHOST_BACKEND_TYPE_USER,
> .vhost_backend_init = vhost_user_init,
> @@ -948,4 +1042,7 @@ const VhostOps user_ops = {
> .vhost_net_set_mtu = vhost_user_net_set_mtu,
> .vhost_set_iotlb_callback = vhost_user_set_iotlb_callback,
> .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg,
> + .vhost_get_config = vhost_user_get_config,
> + .vhost_set_config = vhost_user_set_config,
> + .vhost_set_config_fd = vhost_user_set_config_fd,
> };
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index ddc42f0..8079f46 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1354,6 +1354,9 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
> for (i = 0; i < hdev->nvqs; ++i) {
> vhost_virtqueue_cleanup(hdev->vqs + i);
> }
> + if (hdev->config_ops) {
> + event_notifier_cleanup(&hdev->config_notifier);
> + }
> if (hdev->mem) {
> /* those are only safe after successful init */
> memory_listener_unregister(&hdev->memory_listener);
> @@ -1501,6 +1504,66 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
> }
> }
>
> +int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
> + size_t config_len)
> +{
> + assert(hdev->vhost_ops);
> +
> + if (hdev->vhost_ops->vhost_get_config) {
> + return hdev->vhost_ops->vhost_get_config(hdev, config, config_len);
> + }
> +
> + return 0;
> +}
> +
> +int vhost_dev_set_config(struct vhost_dev *hdev, const uint8_t *config,
> + size_t config_len)
> +{
> + assert(hdev->vhost_ops);
> +
> + if (hdev->vhost_ops->vhost_set_config) {
> + return hdev->vhost_ops->vhost_set_config(hdev, config, config_len);
> + }
> +
> + return 0;
> +}
> +
> +static void vhost_dev_config_notifier_read(EventNotifier *n)
> +{
> + struct vhost_dev *hdev = container_of(n, struct vhost_dev,
> + config_notifier);
> +
> + if (event_notifier_test_and_clear(n)) {
> + if (hdev->config_ops) {
> + hdev->config_ops->vhost_dev_config_notifier(hdev);
> + }
> + }
> +}
> +
> +int vhost_dev_set_config_notifier(struct vhost_dev *hdev,
> + const VhostDevConfigOps *ops)
> +{
> + int r, fd;
> +
> + assert(hdev->vhost_ops);
> +
> + r = event_notifier_init(&hdev->config_notifier, 0);
> + if (r < 0) {
> + return r;
> + }
> +
> + hdev->config_ops = ops;
> + event_notifier_set_handler(&hdev->config_notifier,
> + vhost_dev_config_notifier_read);
> +
> + if (hdev->vhost_ops->vhost_set_config_fd) {
> + fd = event_notifier_get_fd(&hdev->config_notifier);
> + return hdev->vhost_ops->vhost_set_config_fd(hdev, fd);
> + }
> +
> + return 0;
> +}
> +
> /* Host notifiers must be enabled at this point. */
> int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
> {
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index a7a5f22..df6769e 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -84,6 +84,11 @@ typedef void (*vhost_set_iotlb_callback_op)(struct vhost_dev *dev,
> int enabled);
> typedef int (*vhost_send_device_iotlb_msg_op)(struct vhost_dev *dev,
> struct vhost_iotlb_msg *imsg);
> +typedef int (*vhost_set_config_op)(struct vhost_dev *dev, const uint8_t *config,
> + size_t config_len);
> +typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config,
> + size_t config_len);
> +typedef int (*vhost_set_config_fd_op)(struct vhost_dev *dev, int fd);
>
> typedef struct VhostOps {
> VhostBackendType backend_type;
> @@ -118,6 +123,9 @@ typedef struct VhostOps {
> vhost_vsock_set_running_op vhost_vsock_set_running;
> vhost_set_iotlb_callback_op vhost_set_iotlb_callback;
> vhost_send_device_iotlb_msg_op vhost_send_device_iotlb_msg;
> + vhost_get_config_op vhost_get_config;
> + vhost_set_config_op vhost_set_config;
> + vhost_set_config_fd_op vhost_set_config_fd;
> } VhostOps;
>
> extern const VhostOps user_ops;
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 467dc77..ff172f2 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -46,6 +46,12 @@ struct vhost_iommu {
> QLIST_ENTRY(vhost_iommu) iommu_next;
> };
>
> +typedef struct VhostDevConfigOps {
> + /* Vhost device config space changed callback
> + */
> + void (*vhost_dev_config_notifier)(struct vhost_dev *dev);
> +} VhostDevConfigOps;
> +
> struct vhost_memory;
> struct vhost_dev {
> VirtIODevice *vdev;
> @@ -76,6 +82,8 @@ struct vhost_dev {
> QLIST_ENTRY(vhost_dev) entry;
> QLIST_HEAD(, vhost_iommu) iommu_list;
> IOMMUNotifier n;
> + EventNotifier config_notifier;
> + const VhostDevConfigOps *config_ops;
> };
>
> int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> @@ -106,4 +114,12 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
> struct vhost_vring_file *file);
>
> int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
> +int vhost_dev_get_config(struct vhost_dev *dev, uint8_t *config,
> + size_t config_len);
> +int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *config,
> + size_t config_len);
> +/* notifier callback in case vhost device config space changed
> + */
> +int vhost_dev_set_config_notifier(struct vhost_dev *dev,
> + const VhostDevConfigOps *ops);
> #endif
> --
> 1.9.3
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-19 15:39 ` Michael S. Tsirkin
@ 2017-10-19 15:43 ` Paolo Bonzini
2017-10-19 17:43 ` Michael S. Tsirkin
0 siblings, 1 reply; 36+ messages in thread
From: Paolo Bonzini @ 2017-10-19 15:43 UTC (permalink / raw)
To: Michael S. Tsirkin, Changpeng Liu
Cc: qemu-devel, stefanha, marcandre.lureau, felipe, james.r.harris
On 19/10/2017 17:39, Michael S. Tsirkin wrote:
>> Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages which can be
>> used for live migration of vhost user devices, also vhost user devices
>> can benefit from the messages to get/set virtio config space from/to the
>> I/O target. For the purpose to support virtio config space change,
>> VHOST_USER_SET_CONFIG_FD message is added as the event notifier
>> in case virtio config space change in the I/O target.
>>
>> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> I don't much like it that config is getting passed through.
>
> IMO this makes managing things harder not easier.
>
> How about specific messages about specific parts of
> config space that you want to get from the backend?
In the case of virtio-blk that would be all of it. Do you have a case
in mind where some part of the configuration space is owned by QEMU?
Paolo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-19 15:43 ` Paolo Bonzini
@ 2017-10-19 17:43 ` Michael S. Tsirkin
2017-10-19 21:04 ` Paolo Bonzini
0 siblings, 1 reply; 36+ messages in thread
From: Michael S. Tsirkin @ 2017-10-19 17:43 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Changpeng Liu, qemu-devel, stefanha, marcandre.lureau, felipe,
james.r.harris
On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote:
> On 19/10/2017 17:39, Michael S. Tsirkin wrote:
> >> Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages which can be
> >> used for live migration of vhost user devices, also vhost user devices
> >> can benefit from the messages to get/set virtio config space from/to the
> >> I/O target. For the purpose to support virtio config space change,
> >> VHOST_USER_SET_CONFIG_FD message is added as the event notifier
> >> in case virtio config space change in the I/O target.
> >>
> >> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > I don't much like it that config is getting passed through.
> >
> > IMO this makes managing things harder not easier.
> >
> > How about specific messages about specific parts of
> > config space that you want to get from the backend?
>
> In the case of virtio-blk that would be all of it. Do you have a case
> in mind where some part of the configuration space is owned by QEMU?
>
> Paolo
Yes. seg_max
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-19 17:43 ` Michael S. Tsirkin
@ 2017-10-19 21:04 ` Paolo Bonzini
2017-10-20 0:27 ` Michael S. Tsirkin
0 siblings, 1 reply; 36+ messages in thread
From: Paolo Bonzini @ 2017-10-19 21:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Changpeng Liu, qemu-devel, stefanha, marcandre.lureau, felipe,
james.r.harris
On 19/10/2017 19:43, Michael S. Tsirkin wrote:
> On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote:
>> On 19/10/2017 17:39, Michael S. Tsirkin wrote:
>>>> Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages which can be
>>>> used for live migration of vhost user devices, also vhost user devices
>>>> can benefit from the messages to get/set virtio config space from/to the
>>>> I/O target. For the purpose to support virtio config space change,
>>>> VHOST_USER_SET_CONFIG_FD message is added as the event notifier
>>>> in case virtio config space change in the I/O target.
>>>>
>>>> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
>>> I don't much like it that config is getting passed through.
>>>
>>> IMO this makes managing things harder not easier.
>>>
>>> How about specific messages about specific parts of
>>> config space that you want to get from the backend?
>>
>> In the case of virtio-blk that would be all of it. Do you have a case
>> in mind where some part of the configuration space is owned by QEMU?
>>
>> Paolo
>
> Yes. seg_max
The seg_max limit is established by whoever reads buffers from the vring
and passes them down to the lower layer. For vhost-blk that's the
device server, not QEMU.
Paolo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-19 21:04 ` Paolo Bonzini
@ 2017-10-20 0:27 ` Michael S. Tsirkin
2017-10-20 1:55 ` Liu, Changpeng
0 siblings, 1 reply; 36+ messages in thread
From: Michael S. Tsirkin @ 2017-10-20 0:27 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Changpeng Liu, qemu-devel, stefanha, marcandre.lureau, felipe,
james.r.harris
On Thu, Oct 19, 2017 at 11:04:48PM +0200, Paolo Bonzini wrote:
> On 19/10/2017 19:43, Michael S. Tsirkin wrote:
> > On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote:
> >> On 19/10/2017 17:39, Michael S. Tsirkin wrote:
> >>>> Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages which can be
> >>>> used for live migration of vhost user devices, also vhost user devices
> >>>> can benefit from the messages to get/set virtio config space from/to the
> >>>> I/O target. For the purpose to support virtio config space change,
> >>>> VHOST_USER_SET_CONFIG_FD message is added as the event notifier
> >>>> in case virtio config space change in the I/O target.
> >>>>
> >>>> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> >>> I don't much like it that config is getting passed through.
> >>>
> >>> IMO this makes managing things harder not easier.
> >>>
> >>> How about specific messages about specific parts of
> >>> config space that you want to get from the backend?
> >>
> >> In the case of virtio-blk that would be all of it. Do you have a case
> >> in mind where some part of the configuration space is owned by QEMU?
> >>
> >> Paolo
> >
> > Yes. seg_max
>
> The seg_max limit is established by whoever reads buffers from the vring
> and passes them down to the lower layer. For vhost-blk that's the
> device server, not QEMU.
>
> Paolo
Good point. How about num_queues though?
Also why is there SET_CONFIG? Does not look like blk uses it.
And I wonder how do we do it for other devices.
E.g. for net there's a bit in the middle of the
config field that deals with migration.
--
MST
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-19 11:33 ` Paolo Bonzini
@ 2017-10-20 1:38 ` Liu, Changpeng
0 siblings, 0 replies; 36+ messages in thread
From: Liu, Changpeng @ 2017-10-20 1:38 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel@nongnu.org
Cc: stefanha@gmail.com, mst@redhat.com, marcandre.lureau@redhat.com,
felipe@nutanix.com, Harris, James R
> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Thursday, October 19, 2017 7:33 PM
> To: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org
> Cc: stefanha@gmail.com; mst@redhat.com; marcandre.lureau@redhat.com;
> felipe@nutanix.com; Harris, James R <james.r.harris@intel.com>
> Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host
> device
>
> On 19/10/2017 07:24, Changpeng Liu wrote:
> > ;;
> > --enable-vhost-scsi) vhost_scsi="yes"
> > ;;
> > + --disable-vhost-user-blk) vhost_user_blk="no"
> > + ;;
> > + --enable-vhost-user-blk) vhost_user_blk="yes"
> > + ;;
> > --disable-vhost-vsock) vhost_vsock="no"
> > ;;
> > --enable-vhost-vsock) vhost_vsock="yes"
> > @@ -1511,6 +1517,7 @@ disabled with --disable-FEATURE, default is enabled if
> available:
> > cap-ng libcap-ng support
> > attr attr and xattr support
> > vhost-net vhost-net acceleration support
> > + vhost-user-blk VM virtio-blk acceleration in user space
>
> Please use default-configs instead of a new configure switch. See how
> CONFIG_VHOST_USER_SCSI is used in default-configs/pci.mak and
> default-configs/s390x-softmmu.mak.
Ok, thanks.
>
> >
> > +static const int user_feature_bits[] = {
> > + VIRTIO_BLK_F_SIZE_MAX,
> > + VIRTIO_BLK_F_SEG_MAX,
> > + VIRTIO_BLK_F_GEOMETRY,
> > + VIRTIO_BLK_F_BLK_SIZE,
> > + VIRTIO_BLK_F_TOPOLOGY,
> > + VIRTIO_BLK_F_SCSI,
>
> Please omit VIRTIO_BLK_F_SCSI, it's a legacy option that is not anymore
> part of virtio 1.0.
ok
>
> > + VIRTIO_BLK_F_MQ,
> > + VIRTIO_BLK_F_RO,
> > + VIRTIO_BLK_F_FLUSH,
> > + VIRTIO_BLK_F_BARRIER,
>
> Same for VIRTIO_BLK_F_BARRIER.
>
> > + VIRTIO_BLK_F_WCE,
>
> And VIRTIO_BLK_F_WCE is the same as VIRTIO_BLK_F_FLUSH, so it can be
> removed too. Please include VIRTIO_BLK_F_CONFIG_WCE instead, since you
> are supporting it in vhost_user_blk_set_config.
Ok.
>
> > + VIRTIO_F_VERSION_1,
> > + VIRTIO_RING_F_INDIRECT_DESC,
> > + VIRTIO_RING_F_EVENT_IDX,
> > + VIRTIO_F_NOTIFY_ON_EMPTY,
> > + VHOST_INVALID_FEATURE_BIT
> > +};
>
> >
> > +static const TypeInfo vhost_user_blk_info = {
> > + .name = TYPE_VHOST_USER_BLK,
> > + .parent = TYPE_VIRTIO_DEVICE,
> > + .instance_size = sizeof(VHostUserBlk),
> > + .instance_init = vhost_user_blk_instance_init,
> > + .class_init = vhost_user_blk_class_init,
> > +};
> > +
>
> There is some code duplication, so maybe it's worth introducing a common
> superclass like TYPE_VIRTIO_SCSI_COMMON. I'll let others comment on
> whether this is a requirement.
>
> Paolo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/4] contrib/vhost-user-blk: introduce a vhost-user-blk sample application
2017-10-19 11:43 ` Paolo Bonzini
@ 2017-10-20 1:39 ` Liu, Changpeng
0 siblings, 0 replies; 36+ messages in thread
From: Liu, Changpeng @ 2017-10-20 1:39 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel@nongnu.org
Cc: stefanha@gmail.com, mst@redhat.com, marcandre.lureau@redhat.com,
felipe@nutanix.com, Harris, James R
> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Thursday, October 19, 2017 7:44 PM
> To: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org
> Cc: stefanha@gmail.com; mst@redhat.com; marcandre.lureau@redhat.com;
> felipe@nutanix.com; Harris, James R <james.r.harris@intel.com>
> Subject: Re: [PATCH v4 4/4] contrib/vhost-user-blk: introduce a vhost-user-blk
> sample application
>
> On 19/10/2017 07:24, Changpeng Liu wrote:
> >
> > +}
> > +
> > +static void
> > +vub_flush(VubReq *req)
> > +{
> > + VubDev *vdev_blk = req->vdev_blk;
> > +
> > + if (vdev_blk->blk_fd) {
> > + fsync(vdev_blk->blk_fd);
> > + }
> > +}
> > +
>
> No need to check the file descriptor---vub_readv and vub_writev aren't
> checking it either. Also please use fdatasync instead of fsync.
Ok.
>
> > +static uint64_t
> > +vub_get_features(VuDev *dev)
> > +{
> > + return 1ull << VIRTIO_BLK_F_SIZE_MAX |
> > + 1ull << VIRTIO_BLK_F_SEG_MAX |
> > + 1ull << VIRTIO_BLK_F_TOPOLOGY |
> > + 1ull << VIRTIO_BLK_F_BLK_SIZE |
> > + 1ull << VIRTIO_F_VERSION_1 |
> > + 1ull << VHOST_USER_F_PROTOCOL_FEATURES;
> > +}
>
> VIRTIO_BLK_F_FLUSH is missing.
Yes, will add.
>
> Thanks,
>
> Paolo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-19 15:17 ` Stefan Hajnoczi
@ 2017-10-20 1:47 ` Liu, Changpeng
2017-10-20 9:54 ` Stefan Hajnoczi
0 siblings, 1 reply; 36+ messages in thread
From: Liu, Changpeng @ 2017-10-20 1:47 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, mst@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
> -----Original Message-----
> From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> Sent: Thursday, October 19, 2017 11:18 PM
> To: Liu, Changpeng <changpeng.liu@intel.com>
> Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> <james.r.harris@intel.com>
> Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host
> device
>
> On Thu, Oct 19, 2017 at 01:24:08PM +0800, Changpeng Liu wrote:
> > This commit introduces a new vhost-user device for block, it uses a
> > chardev to connect with the backend, same with Qemu virito-blk device,
> > Guest OS still uses the virtio-blk frontend driver.
> >
> > To use it, start Qemu with command line like this:
> >
> > qemu-system-x86_64 \
> > -chardev socket,id=char0,path=/path/vhost.socket \
> > -device vhost-user-blk-pci,chardev=char0,num_queues=1, \
> > bootindex=2... \
> >
> > Users can use different parameters for `num_queues` and `bootindex`.
> >
> > Different with exist Qemu virtio-blk host device, it makes more easy
> > for users to implement their own I/O processing logic, such as all
> > user space I/O stack against hardware block device. It uses the new
> > vhost messages(VHOST_USER_GET_CONFIG) to get block virtio config
> > information from backend process.
> >
> > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > ---
> > configure | 11 ++
> > hw/block/Makefile.objs | 3 +
> > hw/block/vhost-user-blk.c | 360
> +++++++++++++++++++++++++++++++++++++
> > hw/virtio/virtio-pci.c | 55 ++++++
> > hw/virtio/virtio-pci.h | 18 ++
> > include/hw/virtio/vhost-user-blk.h | 40 +++++
> > 6 files changed, 487 insertions(+)
> > create mode 100644 hw/block/vhost-user-blk.c
> > create mode 100644 include/hw/virtio/vhost-user-blk.h
> >
> > diff --git a/configure b/configure
> > index 663e908..f2b348f 100755
> > --- a/configure
> > +++ b/configure
> > @@ -318,6 +318,7 @@ tcg="yes"
> >
> > vhost_net="no"
> > vhost_scsi="no"
> > +vhost_user_blk="no"
> > vhost_vsock="no"
> > vhost_user=""
> > kvm="no"
> > @@ -782,6 +783,7 @@ Linux)
> > kvm="yes"
> > vhost_net="yes"
> > vhost_scsi="yes"
> > + vhost_user_blk="yes"
> > vhost_vsock="yes"
> > QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers
> $QEMU_INCLUDES"
> > supported_os="yes"
> > @@ -1139,6 +1141,10 @@ for opt do
> > ;;
> > --enable-vhost-scsi) vhost_scsi="yes"
> > ;;
> > + --disable-vhost-user-blk) vhost_user_blk="no"
> > + ;;
> > + --enable-vhost-user-blk) vhost_user_blk="yes"
> > + ;;
> > --disable-vhost-vsock) vhost_vsock="no"
> > ;;
> > --enable-vhost-vsock) vhost_vsock="yes"
> > @@ -1511,6 +1517,7 @@ disabled with --disable-FEATURE, default is enabled if
> available:
> > cap-ng libcap-ng support
> > attr attr and xattr support
> > vhost-net vhost-net acceleration support
> > + vhost-user-blk VM virtio-blk acceleration in user space
> > spice spice
> > rbd rados block device (rbd)
> > libiscsi iscsi support
> > @@ -5417,6 +5424,7 @@ echo "posix_madvise $posix_madvise"
> > echo "libcap-ng support $cap_ng"
> > echo "vhost-net support $vhost_net"
> > echo "vhost-scsi support $vhost_scsi"
> > +echo "vhost-user-blk support $vhost_user_blk"
> > echo "vhost-vsock support $vhost_vsock"
> > echo "vhost-user support $vhost_user"
> > echo "Trace backends $trace_backends"
> > @@ -5845,6 +5853,9 @@ fi
> > if test "$vhost_scsi" = "yes" ; then
> > echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak
> > fi
> > +if test "$vhost_user_blk" = "yes" -a "$vhost_user" = "yes"; then
> > + echo "CONFIG_VHOST_USER_BLK=y" >> $config_host_mak
> > +fi
> > if test "$vhost_net" = "yes" -a "$vhost_user" = "yes"; then
> > echo "CONFIG_VHOST_NET_USED=y" >> $config_host_mak
> > fi
> > diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
> > index e0ed980..4c19a58 100644
> > --- a/hw/block/Makefile.objs
> > +++ b/hw/block/Makefile.objs
> > @@ -13,3 +13,6 @@ obj-$(CONFIG_SH4) += tc58128.o
> >
> > obj-$(CONFIG_VIRTIO) += virtio-blk.o
> > obj-$(CONFIG_VIRTIO) += dataplane/
> > +ifeq ($(CONFIG_VIRTIO),y)
> > +obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk.o
> > +endif
> > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > new file mode 100644
> > index 0000000..8aa9fa9
> > --- /dev/null
> > +++ b/hw/block/vhost-user-blk.c
> > @@ -0,0 +1,360 @@
> > +/*
> > + * vhost-user-blk host device
> > + *
> > + * Copyright IBM, Corp. 2011
> > + * Copyright(C) 2017 Intel Corporation.
> > + *
> > + * Authors:
> > + * Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > + * Changpeng Liu <changpeng.liu@intel.com>
>
> This gives the impression that IBM originally authored this code but
> little copied code is actually in this file. Feel free to put your own
> copyright and authorship information at the top and then add a statement
> like "Based on foo.c, Copyright IBM, Corp. 2011" instead. That way it's
> clear that this is new code that you wrote with reference to the
> original file you used as a starting point.
Ok, will change it, thanks.
>
> > +static Property vhost_user_blk_properties[] = {
> > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > + DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues, 1),
> > + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128),
> > + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_SIZE_MAX, true),
> > + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_SIZE_MAX, true),
> > + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_SEG_MAX, true),
> > + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_GEOMETRY, true),
> > + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_RO, false),
> > + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_BLK_SIZE, true),
> > + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_TOPOLOGY, true),
> > + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_MQ, true),
> > + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_FLUSH, true),
> > + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_BARRIER, false),
> > + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_SCSI, false),
> > + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> > + VIRTIO_BLK_F_WCE, false),
>
> Please explain how feature negotation works. The vhost-user slave
> advertises support features in the return value from
> VHOST_USER_GET_FEATURES. How does this additional feature mask work and
> why is it useful?
According to Paolo's previous comments, VIRTIO_BLK_F_WCE/ VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER
should be removed. Here I added all the feature flags just want to avoid the case that vhost-user slave target
can support but Qemu vhost block driver cannot support it.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-20 0:27 ` Michael S. Tsirkin
@ 2017-10-20 1:55 ` Liu, Changpeng
2017-10-20 2:12 ` Michael S. Tsirkin
0 siblings, 1 reply; 36+ messages in thread
From: Liu, Changpeng @ 2017-10-20 1:55 UTC (permalink / raw)
To: Michael S. Tsirkin, Paolo Bonzini
Cc: qemu-devel@nongnu.org, stefanha@gmail.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Friday, October 20, 2017 8:28 AM
> To: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org;
> stefanha@gmail.com; marcandre.lureau@redhat.com; felipe@nutanix.com; Harris,
> James R <james.r.harris@intel.com>
> Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support
> virtio config space
>
> On Thu, Oct 19, 2017 at 11:04:48PM +0200, Paolo Bonzini wrote:
> > On 19/10/2017 19:43, Michael S. Tsirkin wrote:
> > > On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote:
> > >> On 19/10/2017 17:39, Michael S. Tsirkin wrote:
> > >>>> Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages
> which can be
> > >>>> used for live migration of vhost user devices, also vhost user devices
> > >>>> can benefit from the messages to get/set virtio config space from/to the
> > >>>> I/O target. For the purpose to support virtio config space change,
> > >>>> VHOST_USER_SET_CONFIG_FD message is added as the event notifier
> > >>>> in case virtio config space change in the I/O target.
> > >>>>
> > >>>> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > >>> I don't much like it that config is getting passed through.
> > >>>
> > >>> IMO this makes managing things harder not easier.
> > >>>
> > >>> How about specific messages about specific parts of
> > >>> config space that you want to get from the backend?
> > >>
> > >> In the case of virtio-blk that would be all of it. Do you have a case
> > >> in mind where some part of the configuration space is owned by QEMU?
> > >>
> > >> Paolo
> > >
> > > Yes. seg_max
> >
> > The seg_max limit is established by whoever reads buffers from the vring
> > and passes them down to the lower layer. For vhost-blk that's the
> > device server, not QEMU.
> >
> > Paolo
>
> Good point. How about num_queues though?
num_queues is part of virtio_blk config, vhost-user slave can set it,
and Qemu driver can rewrite it if user want less IO queues.
>
> Also why is there SET_CONFIG? Does not look like blk uses it.
Only one possible usage when disable write cache to vhost-user slave device.
>
> And I wonder how do we do it for other devices.
>
> E.g. for net there's a bit in the middle of the
> config field that deals with migration.
Well, I'm okay to make those messages only valid for virtio block device, because it's enough
for virtio block to be started with vhost-user slave target.
>
>
> --
> MST
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-20 1:55 ` Liu, Changpeng
@ 2017-10-20 2:12 ` Michael S. Tsirkin
2017-10-20 2:34 ` Liu, Changpeng
0 siblings, 1 reply; 36+ messages in thread
From: Michael S. Tsirkin @ 2017-10-20 2:12 UTC (permalink / raw)
To: Liu, Changpeng
Cc: Paolo Bonzini, qemu-devel@nongnu.org, stefanha@gmail.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
On Fri, Oct 20, 2017 at 01:55:20AM +0000, Liu, Changpeng wrote:
>
>
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Friday, October 20, 2017 8:28 AM
> > To: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org;
> > stefanha@gmail.com; marcandre.lureau@redhat.com; felipe@nutanix.com; Harris,
> > James R <james.r.harris@intel.com>
> > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support
> > virtio config space
> >
> > On Thu, Oct 19, 2017 at 11:04:48PM +0200, Paolo Bonzini wrote:
> > > On 19/10/2017 19:43, Michael S. Tsirkin wrote:
> > > > On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote:
> > > >> On 19/10/2017 17:39, Michael S. Tsirkin wrote:
> > > >>>> Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages
> > which can be
> > > >>>> used for live migration of vhost user devices, also vhost user devices
> > > >>>> can benefit from the messages to get/set virtio config space from/to the
> > > >>>> I/O target. For the purpose to support virtio config space change,
> > > >>>> VHOST_USER_SET_CONFIG_FD message is added as the event notifier
> > > >>>> in case virtio config space change in the I/O target.
> > > >>>>
> > > >>>> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > > >>> I don't much like it that config is getting passed through.
> > > >>>
> > > >>> IMO this makes managing things harder not easier.
> > > >>>
> > > >>> How about specific messages about specific parts of
> > > >>> config space that you want to get from the backend?
> > > >>
> > > >> In the case of virtio-blk that would be all of it. Do you have a case
> > > >> in mind where some part of the configuration space is owned by QEMU?
> > > >>
> > > >> Paolo
> > > >
> > > > Yes. seg_max
> > >
> > > The seg_max limit is established by whoever reads buffers from the vring
> > > and passes them down to the lower layer. For vhost-blk that's the
> > > device server, not QEMU.
> > >
> > > Paolo
> >
> > Good point. How about num_queues though?
> num_queues is part of virtio_blk config, vhost-user slave can set it,
> and Qemu driver can rewrite it if user want less IO queues.
Fundamentally QEMU needs to support this # of queues for this
device.
So whenever QEMU doesn't always expose config space as-is,
need to document the exact semantics.
Also, does backend need to know?
> >
> > Also why is there SET_CONFIG? Does not look like blk uses it.
> Only one possible usage when disable write cache to vhost-user slave device.
Again need to add documentation what can be written.
> >
> > And I wonder how do we do it for other devices.
> >
> > E.g. for net there's a bit in the middle of the
> > config field that deals with migration.
> Well, I'm okay to make those messages only valid for virtio block device, because it's enough
> for virtio block to be started with vhost-user slave target.
OK but I'd rather make them at least somewhat generic so we can reuse
them down the road. It looks like adding offset/size pair would solve
most of the issues. Thoughts?
> >
> >
> > --
> > MST
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-20 2:12 ` Michael S. Tsirkin
@ 2017-10-20 2:34 ` Liu, Changpeng
0 siblings, 0 replies; 36+ messages in thread
From: Liu, Changpeng @ 2017-10-20 2:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, qemu-devel@nongnu.org, stefanha@gmail.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Friday, October 20, 2017 10:12 AM
> To: Liu, Changpeng <changpeng.liu@intel.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; qemu-devel@nongnu.org;
> stefanha@gmail.com; marcandre.lureau@redhat.com; felipe@nutanix.com; Harris,
> James R <james.r.harris@intel.com>
> Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support
> virtio config space
>
> On Fri, Oct 20, 2017 at 01:55:20AM +0000, Liu, Changpeng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > Sent: Friday, October 20, 2017 8:28 AM
> > > To: Paolo Bonzini <pbonzini@redhat.com>
> > > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org;
> > > stefanha@gmail.com; marcandre.lureau@redhat.com; felipe@nutanix.com;
> Harris,
> > > James R <james.r.harris@intel.com>
> > > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support
> > > virtio config space
> > >
> > > On Thu, Oct 19, 2017 at 11:04:48PM +0200, Paolo Bonzini wrote:
> > > > On 19/10/2017 19:43, Michael S. Tsirkin wrote:
> > > > > On Thu, Oct 19, 2017 at 05:43:18PM +0200, Paolo Bonzini wrote:
> > > > >> On 19/10/2017 17:39, Michael S. Tsirkin wrote:
> > > > >>>> Add VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG messages
> > > which can be
> > > > >>>> used for live migration of vhost user devices, also vhost user devices
> > > > >>>> can benefit from the messages to get/set virtio config space from/to the
> > > > >>>> I/O target. For the purpose to support virtio config space change,
> > > > >>>> VHOST_USER_SET_CONFIG_FD message is added as the event notifier
> > > > >>>> in case virtio config space change in the I/O target.
> > > > >>>>
> > > > >>>> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > > > >>> I don't much like it that config is getting passed through.
> > > > >>>
> > > > >>> IMO this makes managing things harder not easier.
> > > > >>>
> > > > >>> How about specific messages about specific parts of
> > > > >>> config space that you want to get from the backend?
> > > > >>
> > > > >> In the case of virtio-blk that would be all of it. Do you have a case
> > > > >> in mind where some part of the configuration space is owned by QEMU?
> > > > >>
> > > > >> Paolo
> > > > >
> > > > > Yes. seg_max
> > > >
> > > > The seg_max limit is established by whoever reads buffers from the vring
> > > > and passes them down to the lower layer. For vhost-blk that's the
> > > > device server, not QEMU.
> > > >
> > > > Paolo
> > >
> > > Good point. How about num_queues though?
> > num_queues is part of virtio_blk config, vhost-user slave can set it,
> > and Qemu driver can rewrite it if user want less IO queues.
>
> Fundamentally QEMU needs to support this # of queues for this
> device.
>
> So whenever QEMU doesn't always expose config space as-is,
> need to document the exact semantics.
Agreed, Qemu vhost block driver should always has a default value, so I also added the
value as one of the parameters for vhost block driver.
>
> Also, does backend need to know?
vhost-user slave does know how many queues are used, because vhost-user messages
such as SET_VRING_CALL/KICK are related with queues. Here the idea is vhost-user slave
provides the maximum io queues supported, and Qemu users can specify lower io queues.
>
>
> > >
> > > Also why is there SET_CONFIG? Does not look like blk uses it.
> > Only one possible usage when disable write cache to vhost-user slave device.
>
> Again need to add documentation what can be written.
Agreed.
>
>
> > >
> > > And I wonder how do we do it for other devices.
> > >
> > > E.g. for net there's a bit in the middle of the
> > > config field that deals with migration.
> > Well, I'm okay to make those messages only valid for virtio block device, because
> it's enough
> > for virtio block to be started with vhost-user slave target.
>
> OK but I'd rather make them at least somewhat generic so we can reuse
> them down the road. It looks like adding offset/size pair would solve
> most of the issues. Thoughts?
Do you mean SET_CONFIG message followed with offset/size to let vhost-user slave
Know which field the master want to change? Yes, sounds good to me.
>
> > >
> > >
> > > --
> > > MST
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-20 1:47 ` Liu, Changpeng
@ 2017-10-20 9:54 ` Stefan Hajnoczi
2017-10-23 4:26 ` Liu, Changpeng
0 siblings, 1 reply; 36+ messages in thread
From: Stefan Hajnoczi @ 2017-10-20 9:54 UTC (permalink / raw)
To: Liu, Changpeng
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, mst@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
On Fri, Oct 20, 2017 at 01:47:58AM +0000, Liu, Changpeng wrote:
> > > +static Property vhost_user_blk_properties[] = {
> > > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > > + DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues, 1),
> > > + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128),
> > > + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_SEG_MAX, true),
> > > + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_GEOMETRY, true),
> > > + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_RO, false),
> > > + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_BLK_SIZE, true),
> > > + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_TOPOLOGY, true),
> > > + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_MQ, true),
> > > + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_FLUSH, true),
> > > + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_BARRIER, false),
> > > + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_SCSI, false),
> > > + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> > > + VIRTIO_BLK_F_WCE, false),
> >
> > Please explain how feature negotation works. The vhost-user slave
> > advertises support features in the return value from
> > VHOST_USER_GET_FEATURES. How does this additional feature mask work and
> > why is it useful?
> According to Paolo's previous comments, VIRTIO_BLK_F_WCE/ VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER
> should be removed. Here I added all the feature flags just want to avoid the case that vhost-user slave target
> can support but Qemu vhost block driver cannot support it.
Please explain a bit more how these options can be used.
When I looked at the vhost code it seemed like the vhost slave can
report any feature bits it wishes (even things QEMU doesn't know about).
What is the purpose of override some of the feature bits on the QEMU
command-line?
Stefan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-19 15:36 ` Michael S. Tsirkin
@ 2017-10-20 10:00 ` Stefan Hajnoczi
2017-10-23 4:47 ` Liu, Changpeng
0 siblings, 1 reply; 36+ messages in thread
From: Stefan Hajnoczi @ 2017-10-20 10:00 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Changpeng Liu, qemu-devel, pbonzini, marcandre.lureau, felipe,
james.r.harris
On Thu, Oct 19, 2017 at 06:36:00PM +0300, Michael S. Tsirkin wrote:
> On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote:
> > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote:
> > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
> > > /* No-op as the receive channel is not dedicated to IOTLB messages. */
> > > }
> > >
> > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
> > > + size_t config_len)
> > > +{
> > > + VhostUserMsg msg = {
> > > + .request = VHOST_USER_GET_CONFIG,
> > > + .flags = VHOST_USER_VERSION,
> > > + .size = config_len,
> > > + };
> > > +
> > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
> >
> > config_len should be limited to 256 bytes:
> >
> > if (config_len == 0 || config_len > sizeof(msg.payload.config) {
>
> I would just limit it to a reasonable value, acceptable to
> both master and slave, not fail if it's bigger.
>
>
> > > + error_report("bad config length");
> > > + return -1;
> > > + }
> > > +
> > > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> > > + return -1;
> > > + }
> > > +
> > > + if (vhost_user_read(dev, &msg) < 0) {
> > > + return -1;
> > > + }
> > > +
> > > + if (msg.request != VHOST_USER_GET_CONFIG) {
> > > + error_report("Received unexpected msg type. Expected %d received %d",
> > > + VHOST_USER_GET_CONFIG, msg.request);
> > > + return -1;
> > > + }
> > > +
> > > + if (msg.size != config_len) {
> > > + error_report("Received bad msg size.");
> > > + return -1;
> > > + }
> > > +
> > > + memcpy(config, &msg.payload.config, config_len);
> >
> > There is some complexity here: different virtio devices use different
> > amounts of config space. Devices may append new fields to the config
> > space to support new features.
> >
> > Therefore I think the simplest protocol is to always fetch the full
> > 256-byte configuration space. This way the vhost-user slave process can
> > implement feature bits that the master process does not know about.
> >
> > In other words, I don't think the master process knows how much of the
> > config space is used so it should always request 256 bytes.
>
> Each device knows the max config space size.
>
> vdev->config_len = config_size;
I see you're referring to the field that is set in:
void virtio_init(VirtIODevice *vdev, const char *name,
uint16_t device_id, size_t config_size)
How does this work for vhost-user where different slave programs may
offer different config sizes?
The QEMU master process does not know the correct size ahead of time.
The size depends on the vhost-user slave process. This is why I suggest
using the full 256 bytes that the VIRTIO spec defines.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-20 9:54 ` Stefan Hajnoczi
@ 2017-10-23 4:26 ` Liu, Changpeng
2017-10-23 12:55 ` Michael S. Tsirkin
2017-10-23 17:11 ` Stefan Hajnoczi
0 siblings, 2 replies; 36+ messages in thread
From: Liu, Changpeng @ 2017-10-23 4:26 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, mst@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
> -----Original Message-----
> From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> Sent: Friday, October 20, 2017 5:55 PM
> To: Liu, Changpeng <changpeng.liu@intel.com>
> Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> <james.r.harris@intel.com>
> Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host
> device
>
> On Fri, Oct 20, 2017 at 01:47:58AM +0000, Liu, Changpeng wrote:
> > > > +static Property vhost_user_blk_properties[] = {
> > > > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > > > + DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues, 1),
> > > > + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128),
> > > > + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_SEG_MAX, true),
> > > > + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_GEOMETRY, true),
> > > > + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_RO, false),
> > > > + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_BLK_SIZE, true),
> > > > + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_TOPOLOGY, true),
> > > > + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_MQ, true),
> > > > + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_FLUSH, true),
> > > > + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_BARRIER, false),
> > > > + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_SCSI, false),
> > > > + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_WCE, false),
> > >
> > > Please explain how feature negotation works. The vhost-user slave
> > > advertises support features in the return value from
> > > VHOST_USER_GET_FEATURES. How does this additional feature mask work
> and
> > > why is it useful?
> > According to Paolo's previous comments, VIRTIO_BLK_F_WCE/
> VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER
> > should be removed. Here I added all the feature flags just want to avoid the
> case that vhost-user slave target
> > can support but Qemu vhost block driver cannot support it.
>
> Please explain a bit more how these options can be used.
>
> When I looked at the vhost code it seemed like the vhost slave can
> report any feature bits it wishes (even things QEMU doesn't know about).
> What is the purpose of override some of the feature bits on the QEMU
> command-line?
Hi Stefan,
Here I added a switch which can override vhost-slave's feature bits, for example, vhost-slave reported `VIRTIO_BLK_F_RO`,
but Qemu vhost-master can disable it through command line when started the Qemu. Users don't need to change any
vhost-slave's code to disable this feature, and this is also aligned with vhost-scsi and vhost-net's implementation.
>
> Stefan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-20 10:00 ` Stefan Hajnoczi
@ 2017-10-23 4:47 ` Liu, Changpeng
2017-10-23 17:26 ` Stefan Hajnoczi
0 siblings, 1 reply; 36+ messages in thread
From: Liu, Changpeng @ 2017-10-23 4:47 UTC (permalink / raw)
To: Stefan Hajnoczi, Michael S. Tsirkin
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
> -----Original Message-----
> From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> Sent: Friday, October 20, 2017 6:01 PM
> To: Michael S. Tsirkin <mst@redhat.com>
> Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org;
> pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com;
> Harris, James R <james.r.harris@intel.com>
> Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support
> virtio config space
>
> On Thu, Oct 19, 2017 at 06:36:00PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote:
> > > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote:
> > > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct
> vhost_dev *dev, int enabled)
> > > > /* No-op as the receive channel is not dedicated to IOTLB messages. */
> > > > }
> > > >
> > > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
> > > > + size_t config_len)
> > > > +{
> > > > + VhostUserMsg msg = {
> > > > + .request = VHOST_USER_GET_CONFIG,
> > > > + .flags = VHOST_USER_VERSION,
> > > > + .size = config_len,
> > > > + };
> > > > +
> > > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
> > >
> > > config_len should be limited to 256 bytes:
> > >
> > > if (config_len == 0 || config_len > sizeof(msg.payload.config) {
> >
> > I would just limit it to a reasonable value, acceptable to
> > both master and slave, not fail if it's bigger.
> >
> >
> > > > + error_report("bad config length");
> > > > + return -1;
> > > > + }
> > > > +
> > > > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> > > > + return -1;
> > > > + }
> > > > +
> > > > + if (vhost_user_read(dev, &msg) < 0) {
> > > > + return -1;
> > > > + }
> > > > +
> > > > + if (msg.request != VHOST_USER_GET_CONFIG) {
> > > > + error_report("Received unexpected msg type. Expected %d
> received %d",
> > > > + VHOST_USER_GET_CONFIG, msg.request);
> > > > + return -1;
> > > > + }
> > > > +
> > > > + if (msg.size != config_len) {
> > > > + error_report("Received bad msg size.");
> > > > + return -1;
> > > > + }
> > > > +
> > > > + memcpy(config, &msg.payload.config, config_len);
> > >
> > > There is some complexity here: different virtio devices use different
> > > amounts of config space. Devices may append new fields to the config
> > > space to support new features.
> > >
> > > Therefore I think the simplest protocol is to always fetch the full
> > > 256-byte configuration space. This way the vhost-user slave process can
> > > implement feature bits that the master process does not know about.
> > >
> > > In other words, I don't think the master process knows how much of the
> > > config space is used so it should always request 256 bytes.
> >
> > Each device knows the max config space size.
> >
> > vdev->config_len = config_size;
>
> I see you're referring to the field that is set in:
>
> void virtio_init(VirtIODevice *vdev, const char *name,
> uint16_t device_id, size_t config_size)
>
> How does this work for vhost-user where different slave programs may
> offer different config sizes?
Each Qemu vhost controller e.g: vhost-user-scsi-pci and vhost-user-blk-pci should has different char devices,
so vhost-slave knows those messages are from vhost-scsi or vhost-blk, of course, each UNIX domain socket
should be assigned by users with types: vhsot-scsi or vhost-blk.
>
> The QEMU master process does not know the correct size ahead of time.
> The size depends on the vhost-user slave process. This is why I suggest
> using the full 256 bytes that the VIRTIO spec defines.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-23 4:26 ` Liu, Changpeng
@ 2017-10-23 12:55 ` Michael S. Tsirkin
2017-10-24 0:40 ` Liu, Changpeng
2017-10-23 17:11 ` Stefan Hajnoczi
1 sibling, 1 reply; 36+ messages in thread
From: Michael S. Tsirkin @ 2017-10-23 12:55 UTC (permalink / raw)
To: Liu, Changpeng
Cc: Stefan Hajnoczi, qemu-devel@nongnu.org, pbonzini@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
On Mon, Oct 23, 2017 at 04:26:36AM +0000, Liu, Changpeng wrote:
>
>
> > -----Original Message-----
> > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > Sent: Friday, October 20, 2017 5:55 PM
> > To: Liu, Changpeng <changpeng.liu@intel.com>
> > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> > marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> > <james.r.harris@intel.com>
> > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host
> > device
> >
> > On Fri, Oct 20, 2017 at 01:47:58AM +0000, Liu, Changpeng wrote:
> > > > > +static Property vhost_user_blk_properties[] = {
> > > > > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > > > > + DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues, 1),
> > > > > + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128),
> > > > > + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_SEG_MAX, true),
> > > > > + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_GEOMETRY, true),
> > > > > + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_RO, false),
> > > > > + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_BLK_SIZE, true),
> > > > > + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_TOPOLOGY, true),
> > > > > + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_MQ, true),
> > > > > + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_FLUSH, true),
> > > > > + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_BARRIER, false),
> > > > > + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_SCSI, false),
> > > > > + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_WCE, false),
> > > >
> > > > Please explain how feature negotation works. The vhost-user slave
> > > > advertises support features in the return value from
> > > > VHOST_USER_GET_FEATURES. How does this additional feature mask work
> > and
> > > > why is it useful?
> > > According to Paolo's previous comments, VIRTIO_BLK_F_WCE/
> > VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER
> > > should be removed. Here I added all the feature flags just want to avoid the
> > case that vhost-user slave target
> > > can support but Qemu vhost block driver cannot support it.
> >
> > Please explain a bit more how these options can be used.
> >
> > When I looked at the vhost code it seemed like the vhost slave can
> > report any feature bits it wishes (even things QEMU doesn't know about).
> > What is the purpose of override some of the feature bits on the QEMU
> > command-line?
> Hi Stefan,
> Here I added a switch which can override vhost-slave's feature bits, for example, vhost-slave reported `VIRTIO_BLK_F_RO`,
> but Qemu vhost-master can disable it through command line when started the Qemu. Users don't need to change any
> vhost-slave's code to disable this feature, and this is also aligned with vhost-scsi and vhost-net's implementation.
Yes but I don't see these properties in virtio_blk_properties. Please
make the names consistent at least when virtio-blk has them.
I am pretty sure you don't want to expose e.g. _F_SCSI.
> > Stefan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-23 4:26 ` Liu, Changpeng
2017-10-23 12:55 ` Michael S. Tsirkin
@ 2017-10-23 17:11 ` Stefan Hajnoczi
2017-10-24 0:44 ` Liu, Changpeng
1 sibling, 1 reply; 36+ messages in thread
From: Stefan Hajnoczi @ 2017-10-23 17:11 UTC (permalink / raw)
To: Liu, Changpeng
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, mst@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
On Mon, Oct 23, 2017 at 04:26:36AM +0000, Liu, Changpeng wrote:
>
>
> > -----Original Message-----
> > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > Sent: Friday, October 20, 2017 5:55 PM
> > To: Liu, Changpeng <changpeng.liu@intel.com>
> > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> > marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> > <james.r.harris@intel.com>
> > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host
> > device
> >
> > On Fri, Oct 20, 2017 at 01:47:58AM +0000, Liu, Changpeng wrote:
> > > > > +static Property vhost_user_blk_properties[] = {
> > > > > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > > > > + DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues, 1),
> > > > > + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128),
> > > > > + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_SEG_MAX, true),
> > > > > + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_GEOMETRY, true),
> > > > > + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_RO, false),
> > > > > + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_BLK_SIZE, true),
> > > > > + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_TOPOLOGY, true),
> > > > > + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_MQ, true),
> > > > > + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_FLUSH, true),
> > > > > + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_BARRIER, false),
> > > > > + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_SCSI, false),
> > > > > + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> > > > > + VIRTIO_BLK_F_WCE, false),
> > > >
> > > > Please explain how feature negotation works. The vhost-user slave
> > > > advertises support features in the return value from
> > > > VHOST_USER_GET_FEATURES. How does this additional feature mask work
> > and
> > > > why is it useful?
> > > According to Paolo's previous comments, VIRTIO_BLK_F_WCE/
> > VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER
> > > should be removed. Here I added all the feature flags just want to avoid the
> > case that vhost-user slave target
> > > can support but Qemu vhost block driver cannot support it.
> >
> > Please explain a bit more how these options can be used.
> >
> > When I looked at the vhost code it seemed like the vhost slave can
> > report any feature bits it wishes (even things QEMU doesn't know about).
> > What is the purpose of override some of the feature bits on the QEMU
> > command-line?
> Hi Stefan,
> Here I added a switch which can override vhost-slave's feature bits, for example, vhost-slave reported `VIRTIO_BLK_F_RO`,
> but Qemu vhost-master can disable it through command line when started the Qemu. Users don't need to change any
> vhost-slave's code to disable this feature, and this is also aligned with vhost-scsi and vhost-net's implementation.
You said vhost-master can disable features but the code doesn't seem to
work that way:
+ /* Turn on pre-defined features */
+ features |= s->host_features;
If the use case isn't clear please remove these properties for now.
Stefan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-23 4:47 ` Liu, Changpeng
@ 2017-10-23 17:26 ` Stefan Hajnoczi
2017-10-24 0:52 ` Liu, Changpeng
0 siblings, 1 reply; 36+ messages in thread
From: Stefan Hajnoczi @ 2017-10-23 17:26 UTC (permalink / raw)
To: Liu, Changpeng
Cc: Michael S. Tsirkin, qemu-devel@nongnu.org, pbonzini@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
On Mon, Oct 23, 2017 at 04:47:00AM +0000, Liu, Changpeng wrote:
>
>
> > -----Original Message-----
> > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > Sent: Friday, October 20, 2017 6:01 PM
> > To: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org;
> > pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com;
> > Harris, James R <james.r.harris@intel.com>
> > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support
> > virtio config space
> >
> > On Thu, Oct 19, 2017 at 06:36:00PM +0300, Michael S. Tsirkin wrote:
> > > On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote:
> > > > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote:
> > > > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct
> > vhost_dev *dev, int enabled)
> > > > > /* No-op as the receive channel is not dedicated to IOTLB messages. */
> > > > > }
> > > > >
> > > > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
> > > > > + size_t config_len)
> > > > > +{
> > > > > + VhostUserMsg msg = {
> > > > > + .request = VHOST_USER_GET_CONFIG,
> > > > > + .flags = VHOST_USER_VERSION,
> > > > > + .size = config_len,
> > > > > + };
> > > > > +
> > > > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
> > > >
> > > > config_len should be limited to 256 bytes:
> > > >
> > > > if (config_len == 0 || config_len > sizeof(msg.payload.config) {
> > >
> > > I would just limit it to a reasonable value, acceptable to
> > > both master and slave, not fail if it's bigger.
> > >
> > >
> > > > > + error_report("bad config length");
> > > > > + return -1;
> > > > > + }
> > > > > +
> > > > > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> > > > > + return -1;
> > > > > + }
> > > > > +
> > > > > + if (vhost_user_read(dev, &msg) < 0) {
> > > > > + return -1;
> > > > > + }
> > > > > +
> > > > > + if (msg.request != VHOST_USER_GET_CONFIG) {
> > > > > + error_report("Received unexpected msg type. Expected %d
> > received %d",
> > > > > + VHOST_USER_GET_CONFIG, msg.request);
> > > > > + return -1;
> > > > > + }
> > > > > +
> > > > > + if (msg.size != config_len) {
> > > > > + error_report("Received bad msg size.");
> > > > > + return -1;
> > > > > + }
> > > > > +
> > > > > + memcpy(config, &msg.payload.config, config_len);
> > > >
> > > > There is some complexity here: different virtio devices use different
> > > > amounts of config space. Devices may append new fields to the config
> > > > space to support new features.
> > > >
> > > > Therefore I think the simplest protocol is to always fetch the full
> > > > 256-byte configuration space. This way the vhost-user slave process can
> > > > implement feature bits that the master process does not know about.
> > > >
> > > > In other words, I don't think the master process knows how much of the
> > > > config space is used so it should always request 256 bytes.
> > >
> > > Each device knows the max config space size.
> > >
> > > vdev->config_len = config_size;
> >
> > I see you're referring to the field that is set in:
> >
> > void virtio_init(VirtIODevice *vdev, const char *name,
> > uint16_t device_id, size_t config_size)
> >
> > How does this work for vhost-user where different slave programs may
> > offer different config sizes?
> Each Qemu vhost controller e.g: vhost-user-scsi-pci and vhost-user-blk-pci should has different char devices,
> so vhost-slave knows those messages are from vhost-scsi or vhost-blk, of course, each UNIX domain socket
> should be assigned by users with types: vhsot-scsi or vhost-blk.
We're talking about different things. Here is an example illustrating
my question:
vhost-user-blk slave A only knows about struct virtio_blk_config fields
up to wce (VIRTIO 1.0). See
http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-2070004.
vhost-user-blk slave B implements struct virtio_blk_config with the new
num_queues field. See
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/virtio_blk.h#n56.
Slaves A and B use different struct virtio_blk_config sizes!
Which config size should the vhost-master use? There is currently no
way to query the size from the slave.
What should slave programs do when the master requests configuration
space data that is the wrong size?
I think the simplest answer is that the master always uses 256 bytes.
Slaves also keep the full 256 bytes stored but their device
implementation may access fewer bytes.
> > The QEMU master process does not know the correct size ahead of time.
> > The size depends on the vhost-user slave process. This is why I suggest
> > using the full 256 bytes that the VIRTIO spec defines.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-23 12:55 ` Michael S. Tsirkin
@ 2017-10-24 0:40 ` Liu, Changpeng
0 siblings, 0 replies; 36+ messages in thread
From: Liu, Changpeng @ 2017-10-24 0:40 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stefan Hajnoczi, qemu-devel@nongnu.org, pbonzini@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, October 23, 2017 8:55 PM
> To: Liu, Changpeng <changpeng.liu@intel.com>
> Cc: Stefan Hajnoczi <stefanha@gmail.com>; qemu-devel@nongnu.org;
> pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com;
> Harris, James R <james.r.harris@intel.com>
> Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host
> device
>
> On Mon, Oct 23, 2017 at 04:26:36AM +0000, Liu, Changpeng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > > Sent: Friday, October 20, 2017 5:55 PM
> > > To: Liu, Changpeng <changpeng.liu@intel.com>
> > > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> > > marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> > > <james.r.harris@intel.com>
> > > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk
> host
> > > device
> > >
> > > On Fri, Oct 20, 2017 at 01:47:58AM +0000, Liu, Changpeng wrote:
> > > > > > +static Property vhost_user_blk_properties[] = {
> > > > > > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > > > > > + DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues,
> 1),
> > > > > > + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128),
> > > > > > + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > > + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > > + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_SEG_MAX, true),
> > > > > > + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_GEOMETRY, true),
> > > > > > + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_RO, false),
> > > > > > + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_BLK_SIZE, true),
> > > > > > + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_TOPOLOGY, true),
> > > > > > + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_MQ, true),
> > > > > > + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_FLUSH, true),
> > > > > > + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_BARRIER, false),
> > > > > > + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_SCSI, false),
> > > > > > + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_WCE, false),
> > > > >
> > > > > Please explain how feature negotation works. The vhost-user slave
> > > > > advertises support features in the return value from
> > > > > VHOST_USER_GET_FEATURES. How does this additional feature mask
> work
> > > and
> > > > > why is it useful?
> > > > According to Paolo's previous comments, VIRTIO_BLK_F_WCE/
> > > VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER
> > > > should be removed. Here I added all the feature flags just want to avoid
> the
> > > case that vhost-user slave target
> > > > can support but Qemu vhost block driver cannot support it.
> > >
> > > Please explain a bit more how these options can be used.
> > >
> > > When I looked at the vhost code it seemed like the vhost slave can
> > > report any feature bits it wishes (even things QEMU doesn't know about).
> > > What is the purpose of override some of the feature bits on the QEMU
> > > command-line?
> > Hi Stefan,
> > Here I added a switch which can override vhost-slave's feature bits, for
> example, vhost-slave reported `VIRTIO_BLK_F_RO`,
> > but Qemu vhost-master can disable it through command line when started the
> Qemu. Users don't need to change any
> > vhost-slave's code to disable this feature, and this is also aligned with vhost-
> scsi and vhost-net's implementation.
>
> Yes but I don't see these properties in virtio_blk_properties. Please
> make the names consistent at least when virtio-blk has them.
> I am pretty sure you don't want to expose e.g. _F_SCSI.
Yes, I should remove F_SCSI/F_WCE/F_BARRIER features, Virtio-blk hardcoded 4 features: VIRTIO_BLK_F_SEG_MAX/VIRTIO_BLK_F_GEOMETRY/
VIRTIO_BLK_F_TOPOLOGY/VIRTIO_BLK_F_BLK_SIZE, and extra several configuration parameters for VIRTIO_BLK_F_CONFIG_WCE/VIRTIO_BLK_F_MQ,
I can change those feature bits same as virtio-blk.
>
>
> > > Stefan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-23 17:11 ` Stefan Hajnoczi
@ 2017-10-24 0:44 ` Liu, Changpeng
2017-10-24 12:53 ` Stefan Hajnoczi
0 siblings, 1 reply; 36+ messages in thread
From: Liu, Changpeng @ 2017-10-24 0:44 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, mst@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
> -----Original Message-----
> From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> Sent: Tuesday, October 24, 2017 1:12 AM
> To: Liu, Changpeng <changpeng.liu@intel.com>
> Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> <james.r.harris@intel.com>
> Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host
> device
>
> On Mon, Oct 23, 2017 at 04:26:36AM +0000, Liu, Changpeng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > > Sent: Friday, October 20, 2017 5:55 PM
> > > To: Liu, Changpeng <changpeng.liu@intel.com>
> > > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> > > marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> > > <james.r.harris@intel.com>
> > > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk
> host
> > > device
> > >
> > > On Fri, Oct 20, 2017 at 01:47:58AM +0000, Liu, Changpeng wrote:
> > > > > > +static Property vhost_user_blk_properties[] = {
> > > > > > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > > > > > + DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues,
> 1),
> > > > > > + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128),
> > > > > > + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > > + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > > + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_SEG_MAX, true),
> > > > > > + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_GEOMETRY, true),
> > > > > > + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_RO, false),
> > > > > > + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_BLK_SIZE, true),
> > > > > > + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_TOPOLOGY, true),
> > > > > > + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_MQ, true),
> > > > > > + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_FLUSH, true),
> > > > > > + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_BARRIER, false),
> > > > > > + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_SCSI, false),
> > > > > > + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> > > > > > + VIRTIO_BLK_F_WCE, false),
> > > > >
> > > > > Please explain how feature negotation works. The vhost-user slave
> > > > > advertises support features in the return value from
> > > > > VHOST_USER_GET_FEATURES. How does this additional feature mask
> work
> > > and
> > > > > why is it useful?
> > > > According to Paolo's previous comments, VIRTIO_BLK_F_WCE/
> > > VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER
> > > > should be removed. Here I added all the feature flags just want to avoid
> the
> > > case that vhost-user slave target
> > > > can support but Qemu vhost block driver cannot support it.
> > >
> > > Please explain a bit more how these options can be used.
> > >
> > > When I looked at the vhost code it seemed like the vhost slave can
> > > report any feature bits it wishes (even things QEMU doesn't know about).
> > > What is the purpose of override some of the feature bits on the QEMU
> > > command-line?
> > Hi Stefan,
> > Here I added a switch which can override vhost-slave's feature bits, for
> example, vhost-slave reported `VIRTIO_BLK_F_RO`,
> > but Qemu vhost-master can disable it through command line when started the
> Qemu. Users don't need to change any
> > vhost-slave's code to disable this feature, and this is also aligned with vhost-
> scsi and vhost-net's implementation.
>
> You said vhost-master can disable features but the code doesn't seem to
> work that way:
>
> + /* Turn on pre-defined features */
> + features |= s->host_features;
User can append parameter when started Qemu: e.g: f_readonly=false to disable it.
>
> If the use case isn't clear please remove these properties for now.
I can make it the same with virtio-blk, hardcoded the mandatory features, and put
VIRTIO_BLK_F_MQ/VIRTIO_BLK_F_RO/VIRTIO_BLK_F_CONFIG_WCE configurable. Thoughts?
>
> Stefan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-23 17:26 ` Stefan Hajnoczi
@ 2017-10-24 0:52 ` Liu, Changpeng
2017-10-24 13:00 ` Stefan Hajnoczi
0 siblings, 1 reply; 36+ messages in thread
From: Liu, Changpeng @ 2017-10-24 0:52 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Michael S. Tsirkin, qemu-devel@nongnu.org, pbonzini@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
> -----Original Message-----
> From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> Sent: Tuesday, October 24, 2017 1:26 AM
> To: Liu, Changpeng <changpeng.liu@intel.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>; qemu-devel@nongnu.org;
> pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com;
> Harris, James R <james.r.harris@intel.com>
> Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support
> virtio config space
>
> On Mon, Oct 23, 2017 at 04:47:00AM +0000, Liu, Changpeng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > > Sent: Friday, October 20, 2017 6:01 PM
> > > To: Michael S. Tsirkin <mst@redhat.com>
> > > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org;
> > > pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com;
> > > Harris, James R <james.r.harris@intel.com>
> > > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to
> support
> > > virtio config space
> > >
> > > On Thu, Oct 19, 2017 at 06:36:00PM +0300, Michael S. Tsirkin wrote:
> > > > On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote:
> > > > > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote:
> > > > > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct
> > > vhost_dev *dev, int enabled)
> > > > > > /* No-op as the receive channel is not dedicated to IOTLB messages.
> */
> > > > > > }
> > > > > >
> > > > > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
> > > > > > + size_t config_len)
> > > > > > +{
> > > > > > + VhostUserMsg msg = {
> > > > > > + .request = VHOST_USER_GET_CONFIG,
> > > > > > + .flags = VHOST_USER_VERSION,
> > > > > > + .size = config_len,
> > > > > > + };
> > > > > > +
> > > > > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
> > > > >
> > > > > config_len should be limited to 256 bytes:
> > > > >
> > > > > if (config_len == 0 || config_len > sizeof(msg.payload.config) {
> > > >
> > > > I would just limit it to a reasonable value, acceptable to
> > > > both master and slave, not fail if it's bigger.
> > > >
> > > >
> > > > > > + error_report("bad config length");
> > > > > > + return -1;
> > > > > > + }
> > > > > > +
> > > > > > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> > > > > > + return -1;
> > > > > > + }
> > > > > > +
> > > > > > + if (vhost_user_read(dev, &msg) < 0) {
> > > > > > + return -1;
> > > > > > + }
> > > > > > +
> > > > > > + if (msg.request != VHOST_USER_GET_CONFIG) {
> > > > > > + error_report("Received unexpected msg type. Expected %d
> > > received %d",
> > > > > > + VHOST_USER_GET_CONFIG, msg.request);
> > > > > > + return -1;
> > > > > > + }
> > > > > > +
> > > > > > + if (msg.size != config_len) {
> > > > > > + error_report("Received bad msg size.");
> > > > > > + return -1;
> > > > > > + }
> > > > > > +
> > > > > > + memcpy(config, &msg.payload.config, config_len);
> > > > >
> > > > > There is some complexity here: different virtio devices use different
> > > > > amounts of config space. Devices may append new fields to the config
> > > > > space to support new features.
> > > > >
> > > > > Therefore I think the simplest protocol is to always fetch the full
> > > > > 256-byte configuration space. This way the vhost-user slave process can
> > > > > implement feature bits that the master process does not know about.
> > > > >
> > > > > In other words, I don't think the master process knows how much of the
> > > > > config space is used so it should always request 256 bytes.
> > > >
> > > > Each device knows the max config space size.
> > > >
> > > > vdev->config_len = config_size;
> > >
> > > I see you're referring to the field that is set in:
> > >
> > > void virtio_init(VirtIODevice *vdev, const char *name,
> > > uint16_t device_id, size_t config_size)
> > >
> > > How does this work for vhost-user where different slave programs may
> > > offer different config sizes?
> > Each Qemu vhost controller e.g: vhost-user-scsi-pci and vhost-user-blk-pci
> should has different char devices,
> > so vhost-slave knows those messages are from vhost-scsi or vhost-blk, of
> course, each UNIX domain socket
> > should be assigned by users with types: vhsot-scsi or vhost-blk.
>
> We're talking about different things. Here is an example illustrating
> my question:
>
> vhost-user-blk slave A only knows about struct virtio_blk_config fields
> up to wce (VIRTIO 1.0). See
> http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-
> 2070004.
>
> vhost-user-blk slave B implements struct virtio_blk_config with the new
> num_queues field. See
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/u
> api/linux/virtio_blk.h#n56.
>
> Slaves A and B use different struct virtio_blk_config sizes!
>
> Which config size should the vhost-master use? There is currently no
> way to query the size from the slave.
>
> What should slave programs do when the master requests configuration
> space data that is the wrong size?
>
> I think the simplest answer is that the master always uses 256 bytes.
> Slaves also keep the full 256 bytes stored but their device
> implementation may access fewer bytes.
Yes, clear now. How about the following configuration:
struct vhost_dev_config {
unsigned int offset;
unsigned int size;
uint8_t config[256];
};
The master always uses 256 Bytes, but with additional 2 parameters to locate detailed configuration fields, I think this combined
Michael and your comments about this patch.
>
> > > The QEMU master process does not know the correct size ahead of time.
> > > The size depends on the vhost-user slave process. This is why I suggest
> > > using the full 256 bytes that the VIRTIO spec defines.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-24 0:44 ` Liu, Changpeng
@ 2017-10-24 12:53 ` Stefan Hajnoczi
2017-10-25 1:34 ` Liu, Changpeng
0 siblings, 1 reply; 36+ messages in thread
From: Stefan Hajnoczi @ 2017-10-24 12:53 UTC (permalink / raw)
To: Liu, Changpeng
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, mst@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
On Tue, Oct 24, 2017 at 12:44:30AM +0000, Liu, Changpeng wrote:
>
>
> > -----Original Message-----
> > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > Sent: Tuesday, October 24, 2017 1:12 AM
> > To: Liu, Changpeng <changpeng.liu@intel.com>
> > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> > marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> > <james.r.harris@intel.com>
> > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host
> > device
> >
> > On Mon, Oct 23, 2017 at 04:26:36AM +0000, Liu, Changpeng wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > > > Sent: Friday, October 20, 2017 5:55 PM
> > > > To: Liu, Changpeng <changpeng.liu@intel.com>
> > > > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> > > > marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> > > > <james.r.harris@intel.com>
> > > > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk
> > host
> > > > device
> > > >
> > > > On Fri, Oct 20, 2017 at 01:47:58AM +0000, Liu, Changpeng wrote:
> > > > > > > +static Property vhost_user_blk_properties[] = {
> > > > > > > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > > > > > > + DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues,
> > 1),
> > > > > > > + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128),
> > > > > > > + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > > > + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > > > + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_SEG_MAX, true),
> > > > > > > + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_GEOMETRY, true),
> > > > > > > + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_RO, false),
> > > > > > > + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_BLK_SIZE, true),
> > > > > > > + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_TOPOLOGY, true),
> > > > > > > + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_MQ, true),
> > > > > > > + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_FLUSH, true),
> > > > > > > + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_BARRIER, false),
> > > > > > > + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_SCSI, false),
> > > > > > > + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> > > > > > > + VIRTIO_BLK_F_WCE, false),
> > > > > >
> > > > > > Please explain how feature negotation works. The vhost-user slave
> > > > > > advertises support features in the return value from
> > > > > > VHOST_USER_GET_FEATURES. How does this additional feature mask
> > work
> > > > and
> > > > > > why is it useful?
> > > > > According to Paolo's previous comments, VIRTIO_BLK_F_WCE/
> > > > VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER
> > > > > should be removed. Here I added all the feature flags just want to avoid
> > the
> > > > case that vhost-user slave target
> > > > > can support but Qemu vhost block driver cannot support it.
> > > >
> > > > Please explain a bit more how these options can be used.
> > > >
> > > > When I looked at the vhost code it seemed like the vhost slave can
> > > > report any feature bits it wishes (even things QEMU doesn't know about).
> > > > What is the purpose of override some of the feature bits on the QEMU
> > > > command-line?
> > > Hi Stefan,
> > > Here I added a switch which can override vhost-slave's feature bits, for
> > example, vhost-slave reported `VIRTIO_BLK_F_RO`,
> > > but Qemu vhost-master can disable it through command line when started the
> > Qemu. Users don't need to change any
> > > vhost-slave's code to disable this feature, and this is also aligned with vhost-
> > scsi and vhost-net's implementation.
> >
> > You said vhost-master can disable features but the code doesn't seem to
> > work that way:
> >
> > + /* Turn on pre-defined features */
> > + features |= s->host_features;
> User can append parameter when started Qemu: e.g: f_readonly=false to disable it.
No, f_readonly=false has no effect if the vhost slave replies with f_readonly
to the VHOST_USER_GET_FEATURES message.
Take a look at the hw/virtio/vhost.c code:
uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
uint64_t features)
{
const int *bit = feature_bits;
while (*bit != VHOST_INVALID_FEATURE_BIT) {
uint64_t bit_mask = (1ULL << *bit);
if (!(hdev->features & bit_mask)) {
features &= ~bit_mask;
}
bit++;
}
return features;
}
If f_readonly=false then features |= s->host_features will not contain
f_readonly but it doesn't matter because if hdev->features (this value
comes from the vhost slave) has f_readonly we will not clear the bit!
So again, what is the purpose of the f_* properties? I don't understand
the semantics, it seems useless.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
2017-10-24 0:52 ` Liu, Changpeng
@ 2017-10-24 13:00 ` Stefan Hajnoczi
0 siblings, 0 replies; 36+ messages in thread
From: Stefan Hajnoczi @ 2017-10-24 13:00 UTC (permalink / raw)
To: Liu, Changpeng
Cc: Michael S. Tsirkin, qemu-devel@nongnu.org, pbonzini@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
On Tue, Oct 24, 2017 at 12:52:28AM +0000, Liu, Changpeng wrote:
>
>
> > -----Original Message-----
> > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > Sent: Tuesday, October 24, 2017 1:26 AM
> > To: Liu, Changpeng <changpeng.liu@intel.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>; qemu-devel@nongnu.org;
> > pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com;
> > Harris, James R <james.r.harris@intel.com>
> > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support
> > virtio config space
> >
> > On Mon, Oct 23, 2017 at 04:47:00AM +0000, Liu, Changpeng wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > > > Sent: Friday, October 20, 2017 6:01 PM
> > > > To: Michael S. Tsirkin <mst@redhat.com>
> > > > Cc: Liu, Changpeng <changpeng.liu@intel.com>; qemu-devel@nongnu.org;
> > > > pbonzini@redhat.com; marcandre.lureau@redhat.com; felipe@nutanix.com;
> > > > Harris, James R <james.r.harris@intel.com>
> > > > Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to
> > support
> > > > virtio config space
> > > >
> > > > On Thu, Oct 19, 2017 at 06:36:00PM +0300, Michael S. Tsirkin wrote:
> > > > > On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote:
> > > > > > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote:
> > > > > > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct
> > > > vhost_dev *dev, int enabled)
> > > > > > > /* No-op as the receive channel is not dedicated to IOTLB messages.
> > */
> > > > > > > }
> > > > > > >
> > > > > > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
> > > > > > > + size_t config_len)
> > > > > > > +{
> > > > > > > + VhostUserMsg msg = {
> > > > > > > + .request = VHOST_USER_GET_CONFIG,
> > > > > > > + .flags = VHOST_USER_VERSION,
> > > > > > > + .size = config_len,
> > > > > > > + };
> > > > > > > +
> > > > > > > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
> > > > > >
> > > > > > config_len should be limited to 256 bytes:
> > > > > >
> > > > > > if (config_len == 0 || config_len > sizeof(msg.payload.config) {
> > > > >
> > > > > I would just limit it to a reasonable value, acceptable to
> > > > > both master and slave, not fail if it's bigger.
> > > > >
> > > > >
> > > > > > > + error_report("bad config length");
> > > > > > > + return -1;
> > > > > > > + }
> > > > > > > +
> > > > > > > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> > > > > > > + return -1;
> > > > > > > + }
> > > > > > > +
> > > > > > > + if (vhost_user_read(dev, &msg) < 0) {
> > > > > > > + return -1;
> > > > > > > + }
> > > > > > > +
> > > > > > > + if (msg.request != VHOST_USER_GET_CONFIG) {
> > > > > > > + error_report("Received unexpected msg type. Expected %d
> > > > received %d",
> > > > > > > + VHOST_USER_GET_CONFIG, msg.request);
> > > > > > > + return -1;
> > > > > > > + }
> > > > > > > +
> > > > > > > + if (msg.size != config_len) {
> > > > > > > + error_report("Received bad msg size.");
> > > > > > > + return -1;
> > > > > > > + }
> > > > > > > +
> > > > > > > + memcpy(config, &msg.payload.config, config_len);
> > > > > >
> > > > > > There is some complexity here: different virtio devices use different
> > > > > > amounts of config space. Devices may append new fields to the config
> > > > > > space to support new features.
> > > > > >
> > > > > > Therefore I think the simplest protocol is to always fetch the full
> > > > > > 256-byte configuration space. This way the vhost-user slave process can
> > > > > > implement feature bits that the master process does not know about.
> > > > > >
> > > > > > In other words, I don't think the master process knows how much of the
> > > > > > config space is used so it should always request 256 bytes.
> > > > >
> > > > > Each device knows the max config space size.
> > > > >
> > > > > vdev->config_len = config_size;
> > > >
> > > > I see you're referring to the field that is set in:
> > > >
> > > > void virtio_init(VirtIODevice *vdev, const char *name,
> > > > uint16_t device_id, size_t config_size)
> > > >
> > > > How does this work for vhost-user where different slave programs may
> > > > offer different config sizes?
> > > Each Qemu vhost controller e.g: vhost-user-scsi-pci and vhost-user-blk-pci
> > should has different char devices,
> > > so vhost-slave knows those messages are from vhost-scsi or vhost-blk, of
> > course, each UNIX domain socket
> > > should be assigned by users with types: vhsot-scsi or vhost-blk.
> >
> > We're talking about different things. Here is an example illustrating
> > my question:
> >
> > vhost-user-blk slave A only knows about struct virtio_blk_config fields
> > up to wce (VIRTIO 1.0). See
> > http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-
> > 2070004.
> >
> > vhost-user-blk slave B implements struct virtio_blk_config with the new
> > num_queues field. See
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/u
> > api/linux/virtio_blk.h#n56.
> >
> > Slaves A and B use different struct virtio_blk_config sizes!
> >
> > Which config size should the vhost-master use? There is currently no
> > way to query the size from the slave.
> >
> > What should slave programs do when the master requests configuration
> > space data that is the wrong size?
> >
> > I think the simplest answer is that the master always uses 256 bytes.
> > Slaves also keep the full 256 bytes stored but their device
> > implementation may access fewer bytes.
> Yes, clear now. How about the following configuration:
> struct vhost_dev_config {
> unsigned int offset;
> unsigned int size;
> uint8_t config[256];
> };
> The master always uses 256 Bytes, but with additional 2 parameters to locate detailed configuration fields, I think this combined
> Michael and your comments about this patch.
Yes, that's probably a good solution.
Stefan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-24 12:53 ` Stefan Hajnoczi
@ 2017-10-25 1:34 ` Liu, Changpeng
2017-11-03 14:50 ` Stefan Hajnoczi
0 siblings, 1 reply; 36+ messages in thread
From: Liu, Changpeng @ 2017-10-25 1:34 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, mst@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
> -----Original Message-----
> From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> Sent: Tuesday, October 24, 2017 8:53 PM
> To: Liu, Changpeng <changpeng.liu@intel.com>
> Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> <james.r.harris@intel.com>
> Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host
> device
>
> On Tue, Oct 24, 2017 at 12:44:30AM +0000, Liu, Changpeng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > > Sent: Tuesday, October 24, 2017 1:12 AM
> > > To: Liu, Changpeng <changpeng.liu@intel.com>
> > > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> > > marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> > > <james.r.harris@intel.com>
> > > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk
> host
> > > device
> > >
> > > On Mon, Oct 23, 2017 at 04:26:36AM +0000, Liu, Changpeng wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > > > > Sent: Friday, October 20, 2017 5:55 PM
> > > > > To: Liu, Changpeng <changpeng.liu@intel.com>
> > > > > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> > > > > marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> > > > > <james.r.harris@intel.com>
> > > > > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk
> > > host
> > > > > device
> > > > >
> > > > > On Fri, Oct 20, 2017 at 01:47:58AM +0000, Liu, Changpeng wrote:
> > > > > > > > +static Property vhost_user_blk_properties[] = {
> > > > > > > > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > > > > > > > + DEFINE_PROP_UINT16("num_queues", VHostUserBlk,
> num_queues,
> > > 1),
> > > > > > > > + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size,
> 128),
> > > > > > > > + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> > > > > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > > > > + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> > > > > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > > > > + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> > > > > > > > + VIRTIO_BLK_F_SEG_MAX, true),
> > > > > > > > + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> > > > > > > > + VIRTIO_BLK_F_GEOMETRY, true),
> > > > > > > > + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> > > > > > > > + VIRTIO_BLK_F_RO, false),
> > > > > > > > + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> > > > > > > > + VIRTIO_BLK_F_BLK_SIZE, true),
> > > > > > > > + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> > > > > > > > + VIRTIO_BLK_F_TOPOLOGY, true),
> > > > > > > > + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk,
> host_features,
> > > > > > > > + VIRTIO_BLK_F_MQ, true),
> > > > > > > > + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> > > > > > > > + VIRTIO_BLK_F_FLUSH, true),
> > > > > > > > + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> > > > > > > > + VIRTIO_BLK_F_BARRIER, false),
> > > > > > > > + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> > > > > > > > + VIRTIO_BLK_F_SCSI, false),
> > > > > > > > + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> > > > > > > > + VIRTIO_BLK_F_WCE, false),
> > > > > > >
> > > > > > > Please explain how feature negotation works. The vhost-user slave
> > > > > > > advertises support features in the return value from
> > > > > > > VHOST_USER_GET_FEATURES. How does this additional feature mask
> > > work
> > > > > and
> > > > > > > why is it useful?
> > > > > > According to Paolo's previous comments, VIRTIO_BLK_F_WCE/
> > > > > VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER
> > > > > > should be removed. Here I added all the feature flags just want to avoid
> > > the
> > > > > case that vhost-user slave target
> > > > > > can support but Qemu vhost block driver cannot support it.
> > > > >
> > > > > Please explain a bit more how these options can be used.
> > > > >
> > > > > When I looked at the vhost code it seemed like the vhost slave can
> > > > > report any feature bits it wishes (even things QEMU doesn't know about).
> > > > > What is the purpose of override some of the feature bits on the QEMU
> > > > > command-line?
> > > > Hi Stefan,
> > > > Here I added a switch which can override vhost-slave's feature bits, for
> > > example, vhost-slave reported `VIRTIO_BLK_F_RO`,
> > > > but Qemu vhost-master can disable it through command line when started
> the
> > > Qemu. Users don't need to change any
> > > > vhost-slave's code to disable this feature, and this is also aligned with
> vhost-
> > > scsi and vhost-net's implementation.
> > >
> > > You said vhost-master can disable features but the code doesn't seem to
> > > work that way:
> > >
> > > + /* Turn on pre-defined features */
> > > + features |= s->host_features;
> > User can append parameter when started Qemu: e.g: f_readonly=false to
> disable it.
>
> No, f_readonly=false has no effect if the vhost slave replies with f_readonly
> to the VHOST_USER_GET_FEATURES message.
>
> Take a look at the hw/virtio/vhost.c code:
>
> uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
> uint64_t features)
> {
> const int *bit = feature_bits;
> while (*bit != VHOST_INVALID_FEATURE_BIT) {
> uint64_t bit_mask = (1ULL << *bit);
> if (!(hdev->features & bit_mask)) {
> features &= ~bit_mask;
> }
> bit++;
> }
> return features;
> }
>
> If f_readonly=false then features |= s->host_features will not contain
> f_readonly but it doesn't matter because if hdev->features (this value
> comes from the vhost slave) has f_readonly we will not clear the bit!
>
> So again, what is the purpose of the f_* properties? I don't understand
> the semantics, it seems useless.
Hi Stefan,
I hate this piece of code too, :). There are several feature definitions in VirtIODevice and vhost_dev,
I printed the features in the function: vhost_user_blk_get_features, my test vhost-slave also provides RO feature, and
I started Qemu with f_readonly=false:
static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev,
uint64_t features,
Error **errp)
{
VHostUserBlk *s = VHOST_USER_BLK(vdev);
uint64_t get_features;
/* Turn on pre-defined host features */
fprintf(stdout, "Features %"PRIx64"\n", features);
features |= s->host_features;
fprintf(stdout, "Turn On Predefined Features %"PRIx64"\n", features);
fprintf(stdout, "VHOST Features %"PRIx64"\n", s->dev.features);
get_features = vhost_get_features(&s->dev, user_feature_bits, features);
fprintf(stdout, "Get Features %"PRIx64"\n", get_features);
return get_features;
}
Here is the result, RO feature is bit5:
Features 179000000, This is VirtIODevice->host_features.
Turn On Predefined Features 179001ed7, RO bit is masked here.
VHOST Features 141001466, vhost_dev-> features, you can see that RO bit is set here.
Get Features 149001446, this is the final result, RO bit is masked finally.
Similar with vhost-scsi and vhost-net, I think this part of logic is correct.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
2017-10-25 1:34 ` Liu, Changpeng
@ 2017-11-03 14:50 ` Stefan Hajnoczi
0 siblings, 0 replies; 36+ messages in thread
From: Stefan Hajnoczi @ 2017-11-03 14:50 UTC (permalink / raw)
To: Liu, Changpeng
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, mst@redhat.com,
marcandre.lureau@redhat.com, felipe@nutanix.com, Harris, James R
[-- Attachment #1: Type: text/plain, Size: 8593 bytes --]
On Wed, Oct 25, 2017 at 01:34:39AM +0000, Liu, Changpeng wrote:
>
>
> > -----Original Message-----
> > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > Sent: Tuesday, October 24, 2017 8:53 PM
> > To: Liu, Changpeng <changpeng.liu@intel.com>
> > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> > marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> > <james.r.harris@intel.com>
> > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host
> > device
> >
> > On Tue, Oct 24, 2017 at 12:44:30AM +0000, Liu, Changpeng wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > > > Sent: Tuesday, October 24, 2017 1:12 AM
> > > > To: Liu, Changpeng <changpeng.liu@intel.com>
> > > > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> > > > marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> > > > <james.r.harris@intel.com>
> > > > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk
> > host
> > > > device
> > > >
> > > > On Mon, Oct 23, 2017 at 04:26:36AM +0000, Liu, Changpeng wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> > > > > > Sent: Friday, October 20, 2017 5:55 PM
> > > > > > To: Liu, Changpeng <changpeng.liu@intel.com>
> > > > > > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com; mst@redhat.com;
> > > > > > marcandre.lureau@redhat.com; felipe@nutanix.com; Harris, James R
> > > > > > <james.r.harris@intel.com>
> > > > > > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk
> > > > host
> > > > > > device
> > > > > >
> > > > > > On Fri, Oct 20, 2017 at 01:47:58AM +0000, Liu, Changpeng wrote:
> > > > > > > > > +static Property vhost_user_blk_properties[] = {
> > > > > > > > > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > > > > > > > > + DEFINE_PROP_UINT16("num_queues", VHostUserBlk,
> > num_queues,
> > > > 1),
> > > > > > > > > + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size,
> > 128),
> > > > > > > > > + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> > > > > > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > > > > > + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> > > > > > > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > > > > > > + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> > > > > > > > > + VIRTIO_BLK_F_SEG_MAX, true),
> > > > > > > > > + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> > > > > > > > > + VIRTIO_BLK_F_GEOMETRY, true),
> > > > > > > > > + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> > > > > > > > > + VIRTIO_BLK_F_RO, false),
> > > > > > > > > + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> > > > > > > > > + VIRTIO_BLK_F_BLK_SIZE, true),
> > > > > > > > > + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> > > > > > > > > + VIRTIO_BLK_F_TOPOLOGY, true),
> > > > > > > > > + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk,
> > host_features,
> > > > > > > > > + VIRTIO_BLK_F_MQ, true),
> > > > > > > > > + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> > > > > > > > > + VIRTIO_BLK_F_FLUSH, true),
> > > > > > > > > + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> > > > > > > > > + VIRTIO_BLK_F_BARRIER, false),
> > > > > > > > > + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> > > > > > > > > + VIRTIO_BLK_F_SCSI, false),
> > > > > > > > > + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> > > > > > > > > + VIRTIO_BLK_F_WCE, false),
> > > > > > > >
> > > > > > > > Please explain how feature negotation works. The vhost-user slave
> > > > > > > > advertises support features in the return value from
> > > > > > > > VHOST_USER_GET_FEATURES. How does this additional feature mask
> > > > work
> > > > > > and
> > > > > > > > why is it useful?
> > > > > > > According to Paolo's previous comments, VIRTIO_BLK_F_WCE/
> > > > > > VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER
> > > > > > > should be removed. Here I added all the feature flags just want to avoid
> > > > the
> > > > > > case that vhost-user slave target
> > > > > > > can support but Qemu vhost block driver cannot support it.
> > > > > >
> > > > > > Please explain a bit more how these options can be used.
> > > > > >
> > > > > > When I looked at the vhost code it seemed like the vhost slave can
> > > > > > report any feature bits it wishes (even things QEMU doesn't know about).
> > > > > > What is the purpose of override some of the feature bits on the QEMU
> > > > > > command-line?
> > > > > Hi Stefan,
> > > > > Here I added a switch which can override vhost-slave's feature bits, for
> > > > example, vhost-slave reported `VIRTIO_BLK_F_RO`,
> > > > > but Qemu vhost-master can disable it through command line when started
> > the
> > > > Qemu. Users don't need to change any
> > > > > vhost-slave's code to disable this feature, and this is also aligned with
> > vhost-
> > > > scsi and vhost-net's implementation.
> > > >
> > > > You said vhost-master can disable features but the code doesn't seem to
> > > > work that way:
> > > >
> > > > + /* Turn on pre-defined features */
> > > > + features |= s->host_features;
> > > User can append parameter when started Qemu: e.g: f_readonly=false to
> > disable it.
> >
> > No, f_readonly=false has no effect if the vhost slave replies with f_readonly
> > to the VHOST_USER_GET_FEATURES message.
> >
> > Take a look at the hw/virtio/vhost.c code:
> >
> > uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
> > uint64_t features)
> > {
> > const int *bit = feature_bits;
> > while (*bit != VHOST_INVALID_FEATURE_BIT) {
> > uint64_t bit_mask = (1ULL << *bit);
> > if (!(hdev->features & bit_mask)) {
> > features &= ~bit_mask;
> > }
> > bit++;
> > }
> > return features;
> > }
> >
> > If f_readonly=false then features |= s->host_features will not contain
> > f_readonly but it doesn't matter because if hdev->features (this value
> > comes from the vhost slave) has f_readonly we will not clear the bit!
> >
> > So again, what is the purpose of the f_* properties? I don't understand
> > the semantics, it seems useless.
> Hi Stefan,
> I hate this piece of code too, :). There are several feature definitions in VirtIODevice and vhost_dev,
> I printed the features in the function: vhost_user_blk_get_features, my test vhost-slave also provides RO feature, and
> I started Qemu with f_readonly=false:
>
> static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev,
> uint64_t features,
> Error **errp)
> {
> VHostUserBlk *s = VHOST_USER_BLK(vdev);
> uint64_t get_features;
>
> /* Turn on pre-defined host features */
> fprintf(stdout, "Features %"PRIx64"\n", features);
> features |= s->host_features;
> fprintf(stdout, "Turn On Predefined Features %"PRIx64"\n", features);
> fprintf(stdout, "VHOST Features %"PRIx64"\n", s->dev.features);
>
> get_features = vhost_get_features(&s->dev, user_feature_bits, features);
> fprintf(stdout, "Get Features %"PRIx64"\n", get_features);
>
> return get_features;
> }
>
> Here is the result, RO feature is bit5:
> Features 179000000, This is VirtIODevice->host_features.
> Turn On Predefined Features 179001ed7, RO bit is masked here.
> VHOST Features 141001466, vhost_dev-> features, you can see that RO bit is set here.
> Get Features 149001446, this is the final result, RO bit is masked finally.
>
> Similar with vhost-scsi and vhost-net, I think this part of logic is correct.
I see. I incorrectly thought vhost_dev->features is ORed at some stage.
This means that vhost slaves can only influence the feature bits in one
way: they can disable feature bits that QEMU knows about. Basically the
feature bits are controlled by QEMU, not the slave.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2017-11-03 14:50 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-19 5:24 [Qemu-devel] [PATCH v4 0/4] *** Introduce a new vhost-user-blk host device to Qemu *** Changpeng Liu
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space Changpeng Liu
2017-10-19 11:37 ` Paolo Bonzini
2017-10-19 14:09 ` Stefan Hajnoczi
2017-10-19 15:36 ` Michael S. Tsirkin
2017-10-20 10:00 ` Stefan Hajnoczi
2017-10-23 4:47 ` Liu, Changpeng
2017-10-23 17:26 ` Stefan Hajnoczi
2017-10-24 0:52 ` Liu, Changpeng
2017-10-24 13:00 ` Stefan Hajnoczi
2017-10-19 15:39 ` Michael S. Tsirkin
2017-10-19 15:43 ` Paolo Bonzini
2017-10-19 17:43 ` Michael S. Tsirkin
2017-10-19 21:04 ` Paolo Bonzini
2017-10-20 0:27 ` Michael S. Tsirkin
2017-10-20 1:55 ` Liu, Changpeng
2017-10-20 2:12 ` Michael S. Tsirkin
2017-10-20 2:34 ` Liu, Changpeng
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device Changpeng Liu
2017-10-19 11:33 ` Paolo Bonzini
2017-10-20 1:38 ` Liu, Changpeng
2017-10-19 15:17 ` Stefan Hajnoczi
2017-10-20 1:47 ` Liu, Changpeng
2017-10-20 9:54 ` Stefan Hajnoczi
2017-10-23 4:26 ` Liu, Changpeng
2017-10-23 12:55 ` Michael S. Tsirkin
2017-10-24 0:40 ` Liu, Changpeng
2017-10-23 17:11 ` Stefan Hajnoczi
2017-10-24 0:44 ` Liu, Changpeng
2017-10-24 12:53 ` Stefan Hajnoczi
2017-10-25 1:34 ` Liu, Changpeng
2017-11-03 14:50 ` Stefan Hajnoczi
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 3/4] contrib/libvhost-user: enable virtio config space messages Changpeng Liu
2017-10-19 5:24 ` [Qemu-devel] [PATCH v4 4/4] contrib/vhost-user-blk: introduce a vhost-user-blk sample application Changpeng Liu
2017-10-19 11:43 ` Paolo Bonzini
2017-10-20 1:39 ` Liu, Changpeng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).