All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
@ 2021-10-04 15:07 Denis Plotnikov
  2021-10-04 15:07 ` [PATCH v0 1/2] vhost-user-blk: add a new vhost-user-virtio-blk type Denis Plotnikov
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Denis Plotnikov @ 2021-10-04 15:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, mst, yc-core, qemu-block, raphael.norwitz

It might be useful for the cases when a slow block layer should be replaced
with a more performant one on running VM without stopping, i.e. with very low
downtime comparable with the one on migration.

It's possible to achive that for two reasons:

1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the same.
  They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
  each other in the values of migration service fields only.
2.The device driver used in the guest is the same: virtio-blk

In the series cross-migration is achieved by adding a new type.
The new type uses virtio-blk VMState instead of vhost-user-blk specific
VMstate, also it implements migration save/load callbacks to be compatible
with migration stream produced by "virtio-blk" device.

Adding the new type instead of modifying the existing one is convenent.
It ease to differ the new virtio-blk-compatible vhost-user-blk
device from the existing non-compatible one using qemu machinery without any
other modifiactions. That gives all the variety of qemu device related
constraints out of box.

0001: adds new type "vhost-user-virtio-blk"
0002: add new type "vhost-user-virtio-blk-pci"

Denis Plotnikov (2):
  vhost-user-blk: add a new vhost-user-virtio-blk type
  vhost-user-blk-pci: add new pci device type to support
    vhost-user-virtio-blk

 hw/block/vhost-user-blk.c          | 63 ++++++++++++++++++++++++++++++
 hw/virtio/vhost-user-blk-pci.c     | 43 ++++++++++++++++++--
 include/hw/virtio/vhost-user-blk.h |  2 +
 3 files changed, 105 insertions(+), 3 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v0 1/2] vhost-user-blk: add a new vhost-user-virtio-blk type
  2021-10-04 15:07 [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration Denis Plotnikov
@ 2021-10-04 15:07 ` Denis Plotnikov
  2021-10-04 15:16   ` Michael S. Tsirkin
  2021-10-04 15:07 ` [PATCH v0 2/2] vhost-user-blk-pci: add new pci device type to support vhost-user-virtio-blk Denis Plotnikov
  2021-10-04 15:11 ` [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration Michael S. Tsirkin
  2 siblings, 1 reply; 20+ messages in thread
From: Denis Plotnikov @ 2021-10-04 15:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, mst, yc-core, qemu-block, raphael.norwitz

The main reason of adding a new type is to make cross-device live migration
between "virtio-blk" and "vhost-user-blk" devices possible in both directions.

It might be useful for the cases when a slow block layer should be replaced
with a more performant one on running VM without stopping, i.e. with very low
downtime comparable with the one on migration.

It's possible to achive that for two reasons:

1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the same.
  They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
  each other in the values of migration service fields only.
2.The device driver used in the guest is the same: virtio-blk

The new type uses virtio-blk VMState instead of vhost-user-blk specific
VMstate, also it implements migration save/load callbacks to be compatible
with migration stream produced by "virtio-blk" device.

Adding the new vhost-user-blk type instead of modifying the existing one
is convenent. It ease to differ the new virtio-blk-compatible vhost-user-blk
device from the existing non-compatible one using qemu machinery without any
other modifiactions. That gives all the variety of qemu device related
constraints out of box.

Signed-off-by: Denis Plotnikov <den-plotnikov@yandex-team.ru>
---
 hw/block/vhost-user-blk.c          | 63 ++++++++++++++++++++++++++++++
 include/hw/virtio/vhost-user-blk.h |  2 +
 2 files changed, 65 insertions(+)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index ba13cb87e520..877fe54e891f 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -30,6 +30,7 @@
 #include "hw/virtio/virtio-access.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/runstate.h"
+#include "migration/qemu-file-types.h"
 
 #define REALIZE_CONNECTION_RETRIES 3
 
@@ -612,9 +613,71 @@ static const TypeInfo vhost_user_blk_info = {
     .class_init = vhost_user_blk_class_init,
 };
 
+/*
+ * this is the same as vmstate_virtio_blk
+ * we use it to allow virtio-blk <-> vhost-user-virtio-blk migration
+ */
+static const VMStateDescription vmstate_vhost_user_virtio_blk = {
+    .name = "virtio-blk",
+    .minimum_version_id = 2,
+    .version_id = 2,
+    .fields = (VMStateField[]) {
+        VMSTATE_VIRTIO_DEVICE,
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+static void vhost_user_virtio_blk_save(VirtIODevice *vdev, QEMUFile *f)
+{
+    /*
+     * put a zero byte in the stream to be compatible with virtio-blk
+     */
+    qemu_put_sbyte(f, 0);
+}
+
+static int vhost_user_virtio_blk_load(VirtIODevice *vdev, QEMUFile *f,
+                                      int version_id)
+{
+    if (qemu_get_sbyte(f)) {
+        /*
+         * on virtio-blk -> vhost-user-virtio-blk migration we don't expect
+         * to get any infilght requests in the migration stream because
+         * we can't load them yet.
+         * TODO: consider putting those inflight requests to inflight region
+         */
+        error_report("%s: can't load in-flight requests",
+                     TYPE_VHOST_USER_VIRTIO_BLK);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+static void vhost_user_virtio_blk_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+    /* override vmstate of vhost_user_blk */
+    dc->vmsd = &vmstate_vhost_user_virtio_blk;
+
+    /* adding callbacks to be compatible with virtio-blk migration stream */
+    vdc->save = vhost_user_virtio_blk_save;
+    vdc->load = vhost_user_virtio_blk_load;
+}
+
+static const TypeInfo vhost_user_virtio_blk_info = {
+    .name = TYPE_VHOST_USER_VIRTIO_BLK,
+    .parent = TYPE_VHOST_USER_BLK,
+    .instance_size = sizeof(VHostUserBlk),
+    /* instance_init is the same as in parent type */
+    .class_init = vhost_user_virtio_blk_class_init,
+};
+
 static void virtio_register_types(void)
 {
     type_register_static(&vhost_user_blk_info);
+    type_register_static(&vhost_user_virtio_blk_info);
 }
 
 type_init(virtio_register_types)
diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
index 7c91f15040eb..d81f18d22596 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -23,6 +23,8 @@
 #include "qom/object.h"
 
 #define TYPE_VHOST_USER_BLK "vhost-user-blk"
+#define TYPE_VHOST_USER_VIRTIO_BLK "vhost-user-virtio-blk"
+
 OBJECT_DECLARE_SIMPLE_TYPE(VHostUserBlk, VHOST_USER_BLK)
 
 #define VHOST_USER_BLK_AUTO_NUM_QUEUES UINT16_MAX
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v0 2/2] vhost-user-blk-pci: add new pci device type to support vhost-user-virtio-blk
  2021-10-04 15:07 [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration Denis Plotnikov
  2021-10-04 15:07 ` [PATCH v0 1/2] vhost-user-blk: add a new vhost-user-virtio-blk type Denis Plotnikov
@ 2021-10-04 15:07 ` Denis Plotnikov
  2021-10-04 15:11 ` [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration Michael S. Tsirkin
  2 siblings, 0 replies; 20+ messages in thread
From: Denis Plotnikov @ 2021-10-04 15:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, mst, yc-core, qemu-block, raphael.norwitz

To allow the recently added vhost-user-virtio-blk work via virtio-pci.

This patch refactors the vhost-user-blk-pci object model to reuse
the existing code.

Signed-off-by: Denis Plotnikov <den-plotnikov@yandex-team.ru>
---
 hw/virtio/vhost-user-blk-pci.c | 43 +++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost-user-blk-pci.c b/hw/virtio/vhost-user-blk-pci.c
index 33b404d8a225..2f68296af22f 100644
--- a/hw/virtio/vhost-user-blk-pci.c
+++ b/hw/virtio/vhost-user-blk-pci.c
@@ -34,10 +34,18 @@ typedef struct VHostUserBlkPCI VHostUserBlkPCI;
 /*
  * vhost-user-blk-pci: This extends VirtioPCIProxy.
  */
+#define TYPE_VHOST_USER_BLK_PCI_ABSTRACT "vhost-user-blk-pci-abstract-base"
+#define VHOST_USER_BLK_PCI_ABSTRACT(obj) \
+        OBJECT_CHECK(VHostUserBlkPCI, (obj), TYPE_VHOST_USER_BLK_PCI_ABSTRACT)
+
 #define TYPE_VHOST_USER_BLK_PCI "vhost-user-blk-pci-base"
 DECLARE_INSTANCE_CHECKER(VHostUserBlkPCI, VHOST_USER_BLK_PCI,
                          TYPE_VHOST_USER_BLK_PCI)
 
+#define TYPE_VHOST_USER_VIRTIO_BLK_PCI "vhost-user-virtio-blk-pci-base"
+#define VHOST_USER_VIRTIO_BLK_PCI(obj) \
+        OBJECT_CHECK(VHostUserBlkPCI, (obj), TYPE_VHOST_USER_VIRTIO_BLK_PCI)
+
 struct VHostUserBlkPCI {
     VirtIOPCIProxy parent_obj;
     VHostUserBlk vdev;
@@ -52,7 +60,7 @@ static Property vhost_user_blk_pci_properties[] = {
 
 static void vhost_user_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
 {
-    VHostUserBlkPCI *dev = VHOST_USER_BLK_PCI(vpci_dev);
+    VHostUserBlkPCI *dev = VHOST_USER_BLK_PCI_ABSTRACT(vpci_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
 
     if (dev->vdev.num_queues == VHOST_USER_BLK_AUTO_NUM_QUEUES) {
@@ -66,7 +74,8 @@ static void vhost_user_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
 }
 
-static void vhost_user_blk_pci_class_init(ObjectClass *klass, void *data)
+static void vhost_user_blk_pci_abstract_class_init(ObjectClass *klass,
+                                                   void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
@@ -81,6 +90,12 @@ static void vhost_user_blk_pci_class_init(ObjectClass *klass, void *data)
     pcidev_k->class_id = PCI_CLASS_STORAGE_SCSI;
 }
 
+static const VirtioPCIDeviceTypeInfo vhost_user_blk_pci_abstract_info = {
+    .base_name      = TYPE_VHOST_USER_BLK_PCI_ABSTRACT,
+    .instance_size  = sizeof(VHostUserBlkPCI),
+    .class_init     = vhost_user_blk_pci_abstract_class_init,
+};
+
 static void vhost_user_blk_pci_instance_init(Object *obj)
 {
     VHostUserBlkPCI *dev = VHOST_USER_BLK_PCI(obj);
@@ -92,18 +107,40 @@ static void vhost_user_blk_pci_instance_init(Object *obj)
 }
 
 static const VirtioPCIDeviceTypeInfo vhost_user_blk_pci_info = {
+    .parent                  = TYPE_VHOST_USER_BLK_PCI_ABSTRACT,
     .base_name               = TYPE_VHOST_USER_BLK_PCI,
     .generic_name            = "vhost-user-blk-pci",
     .transitional_name       = "vhost-user-blk-pci-transitional",
     .non_transitional_name   = "vhost-user-blk-pci-non-transitional",
     .instance_size  = sizeof(VHostUserBlkPCI),
     .instance_init  = vhost_user_blk_pci_instance_init,
-    .class_init     = vhost_user_blk_pci_class_init,
+};
+
+static void vhost_user_virtio_blk_pci_instance_init(Object *obj)
+{
+    VHostUserBlkPCI *dev = VHOST_USER_VIRTIO_BLK_PCI(obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VHOST_USER_VIRTIO_BLK);
+    object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
+                              "bootindex");
+}
+
+static const VirtioPCIDeviceTypeInfo vhost_user_virtio_blk_pci_info = {
+    .parent                  = TYPE_VHOST_USER_BLK_PCI_ABSTRACT,
+    .base_name               = TYPE_VHOST_USER_VIRTIO_BLK_PCI,
+    .generic_name            = "vhost-user-virtio-blk-pci",
+    .transitional_name       = "vhost-user-virtio-blk-pci-transitional",
+    .non_transitional_name   = "vhost-user-virtio-blk-pci-non-transitional",
+    .instance_size  = sizeof(VHostUserBlkPCI),
+    .instance_init  = vhost_user_virtio_blk_pci_instance_init,
 };
 
 static void vhost_user_blk_pci_register(void)
 {
+    virtio_pci_types_register(&vhost_user_blk_pci_abstract_info);
     virtio_pci_types_register(&vhost_user_blk_pci_info);
+    virtio_pci_types_register(&vhost_user_virtio_blk_pci_info);
 }
 
 type_init(vhost_user_blk_pci_register)
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-04 15:07 [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration Denis Plotnikov
  2021-10-04 15:07 ` [PATCH v0 1/2] vhost-user-blk: add a new vhost-user-virtio-blk type Denis Plotnikov
  2021-10-04 15:07 ` [PATCH v0 2/2] vhost-user-blk-pci: add new pci device type to support vhost-user-virtio-blk Denis Plotnikov
@ 2021-10-04 15:11 ` Michael S. Tsirkin
  2021-10-04 23:18   ` Roman Kagan
  2 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2021-10-04 15:11 UTC (permalink / raw)
  To: Denis Plotnikov; +Cc: kwolf, yc-core, qemu-devel, qemu-block, raphael.norwitz

On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote:
> It might be useful for the cases when a slow block layer should be replaced
> with a more performant one on running VM without stopping, i.e. with very low
> downtime comparable with the one on migration.
> 
> It's possible to achive that for two reasons:
> 
> 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the same.
>   They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
>   each other in the values of migration service fields only.
> 2.The device driver used in the guest is the same: virtio-blk
> 
> In the series cross-migration is achieved by adding a new type.
> The new type uses virtio-blk VMState instead of vhost-user-blk specific
> VMstate, also it implements migration save/load callbacks to be compatible
> with migration stream produced by "virtio-blk" device.
> 
> Adding the new type instead of modifying the existing one is convenent.
> It ease to differ the new virtio-blk-compatible vhost-user-blk
> device from the existing non-compatible one using qemu machinery without any
> other modifiactions. That gives all the variety of qemu device related
> constraints out of box.

Hmm I'm not sure I understand. What is the advantage for the user?
What if vhost-user-blk became an alias for vhost-user-virtio-blk?
We could add some hacks to make it compatible for old machine types.

> 0001: adds new type "vhost-user-virtio-blk"
> 0002: add new type "vhost-user-virtio-blk-pci"
> 
> Denis Plotnikov (2):
>   vhost-user-blk: add a new vhost-user-virtio-blk type
>   vhost-user-blk-pci: add new pci device type to support
>     vhost-user-virtio-blk
> 
>  hw/block/vhost-user-blk.c          | 63 ++++++++++++++++++++++++++++++
>  hw/virtio/vhost-user-blk-pci.c     | 43 ++++++++++++++++++--
>  include/hw/virtio/vhost-user-blk.h |  2 +
>  3 files changed, 105 insertions(+), 3 deletions(-)
> 
> -- 
> 2.25.1



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 1/2] vhost-user-blk: add a new vhost-user-virtio-blk type
  2021-10-04 15:07 ` [PATCH v0 1/2] vhost-user-blk: add a new vhost-user-virtio-blk type Denis Plotnikov
@ 2021-10-04 15:16   ` Michael S. Tsirkin
  0 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2021-10-04 15:16 UTC (permalink / raw)
  To: Denis Plotnikov; +Cc: kwolf, yc-core, qemu-devel, qemu-block, raphael.norwitz

On Mon, Oct 04, 2021 at 06:07:30PM +0300, Denis Plotnikov wrote:
> Adding the new vhost-user-blk type instead of modifying the existing one
> is convenent. It ease to differ the new virtio-blk-compatible vhost-user-blk
> device from the existing non-compatible one using qemu machinery without any
> other modifiactions. That gives all the variety of qemu device related
> constraints out of box.

Convenient for the developer maybe, but isn't it confusing for the user?
I don't really understand this paragraph. E.g. what are the constraints
here?



> 
> Signed-off-by: Denis Plotnikov <den-plotnikov@yandex-team.ru>
> ---
>  hw/block/vhost-user-blk.c          | 63 ++++++++++++++++++++++++++++++
>  include/hw/virtio/vhost-user-blk.h |  2 +
>  2 files changed, 65 insertions(+)
> 
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index ba13cb87e520..877fe54e891f 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -30,6 +30,7 @@
>  #include "hw/virtio/virtio-access.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/runstate.h"
> +#include "migration/qemu-file-types.h"
>  
>  #define REALIZE_CONNECTION_RETRIES 3
>  
> @@ -612,9 +613,71 @@ static const TypeInfo vhost_user_blk_info = {
>      .class_init = vhost_user_blk_class_init,
>  };
>  
> +/*
> + * this is the same as vmstate_virtio_blk
> + * we use it to allow virtio-blk <-> vhost-user-virtio-blk migration
> + */
> +static const VMStateDescription vmstate_vhost_user_virtio_blk = {
> +    .name = "virtio-blk",
> +    .minimum_version_id = 2,
> +    .version_id = 2,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_VIRTIO_DEVICE,
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
> +static void vhost_user_virtio_blk_save(VirtIODevice *vdev, QEMUFile *f)
> +{
> +    /*
> +     * put a zero byte in the stream to be compatible with virtio-blk
> +     */
> +    qemu_put_sbyte(f, 0);
> +}
> +
> +static int vhost_user_virtio_blk_load(VirtIODevice *vdev, QEMUFile *f,
> +                                      int version_id)
> +{
> +    if (qemu_get_sbyte(f)) {
> +        /*
> +         * on virtio-blk -> vhost-user-virtio-blk migration we don't expect
> +         * to get any infilght requests in the migration stream because
> +         * we can't load them yet.
> +         * TODO: consider putting those inflight requests to inflight region
> +         */
> +        error_report("%s: can't load in-flight requests",
> +                     TYPE_VHOST_USER_VIRTIO_BLK);
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +static void vhost_user_virtio_blk_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> +
> +    /* override vmstate of vhost_user_blk */
> +    dc->vmsd = &vmstate_vhost_user_virtio_blk;

So can't we do something like this in vhost_user_blk_class_init:

	if qemu >= 6.2
		dc->vmsd = &vmstate_vhost_user_virtio_blk;
	else
		dc->vmsd = &vmstate_vhost_user_blk

?

> +
> +    /* adding callbacks to be compatible with virtio-blk migration stream */
> +    vdc->save = vhost_user_virtio_blk_save;
> +    vdc->load = vhost_user_virtio_blk_load;
> +}
> +
> +static const TypeInfo vhost_user_virtio_blk_info = {
> +    .name = TYPE_VHOST_USER_VIRTIO_BLK,
> +    .parent = TYPE_VHOST_USER_BLK,
> +    .instance_size = sizeof(VHostUserBlk),
> +    /* instance_init is the same as in parent type */
> +    .class_init = vhost_user_virtio_blk_class_init,
> +};
> +
>  static void virtio_register_types(void)
>  {
>      type_register_static(&vhost_user_blk_info);
> +    type_register_static(&vhost_user_virtio_blk_info);
>  }
>  
>  type_init(virtio_register_types)
> diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
> index 7c91f15040eb..d81f18d22596 100644
> --- a/include/hw/virtio/vhost-user-blk.h
> +++ b/include/hw/virtio/vhost-user-blk.h
> @@ -23,6 +23,8 @@
>  #include "qom/object.h"
>  
>  #define TYPE_VHOST_USER_BLK "vhost-user-blk"
> +#define TYPE_VHOST_USER_VIRTIO_BLK "vhost-user-virtio-blk"
> +
>  OBJECT_DECLARE_SIMPLE_TYPE(VHostUserBlk, VHOST_USER_BLK)
>  
>  #define VHOST_USER_BLK_AUTO_NUM_QUEUES UINT16_MAX
> -- 
> 2.25.1



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-04 15:11 ` [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration Michael S. Tsirkin
@ 2021-10-04 23:18   ` Roman Kagan
  2021-10-05  6:51     ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Roman Kagan @ 2021-10-04 23:18 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kwolf, qemu-block, qemu-devel, raphael.norwitz, Denis Plotnikov, yc-core

On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote:
> On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote:
> > It might be useful for the cases when a slow block layer should be replaced
> > with a more performant one on running VM without stopping, i.e. with very low
> > downtime comparable with the one on migration.
> > 
> > It's possible to achive that for two reasons:
> > 
> > 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the same.
> >   They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
> >   each other in the values of migration service fields only.
> > 2.The device driver used in the guest is the same: virtio-blk
> > 
> > In the series cross-migration is achieved by adding a new type.
> > The new type uses virtio-blk VMState instead of vhost-user-blk specific
> > VMstate, also it implements migration save/load callbacks to be compatible
> > with migration stream produced by "virtio-blk" device.
> > 
> > Adding the new type instead of modifying the existing one is convenent.
> > It ease to differ the new virtio-blk-compatible vhost-user-blk
> > device from the existing non-compatible one using qemu machinery without any
> > other modifiactions. That gives all the variety of qemu device related
> > constraints out of box.
> 
> Hmm I'm not sure I understand. What is the advantage for the user?
> What if vhost-user-blk became an alias for vhost-user-virtio-blk?
> We could add some hacks to make it compatible for old machine types.

The point is that virtio-blk and vhost-user-blk are not
migration-compatible ATM.  OTOH they are the same device from the guest
POV so there's nothing fundamentally preventing the migration between
the two.  In particular, we see it as a means to switch between the
storage backend transports via live migration without disrupting the
guest.

Migration-wise virtio-blk and vhost-user-blk have in common

- the content of the VMState -- VMSTATE_VIRTIO_DEVICE

The two differ in

- the name and the version of the VMStateDescription

- virtio-blk has an extra migration section (via .save/.load callbacks
  on VirtioDeviceClass) containing requests in flight

It looks like to become migration-compatible with virtio-blk,
vhost-user-blk has to start using VMStateDescription of virtio-blk and
provide compatible .save/.load callbacks.  It isn't entirely obvious how
to make this machine-type-dependent, so we came up with a simpler idea
of defining a new device that shares most of the implementation with the
original vhost-user-blk except for the migration stuff.  We're certainly
open to suggestions on how to reconcile this under a single
vhost-user-blk device, as this would be more user-friendly indeed.

We considered using a class property for this and defining the
respective compat clause, but IIUC the class constructors (where .vmsd
and .save/.load are defined) are not supposed to depend on class
properties.

Thanks,
Roman.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-04 23:18   ` Roman Kagan
@ 2021-10-05  6:51     ` Michael S. Tsirkin
  2021-10-05 14:01       ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2021-10-05  6:51 UTC (permalink / raw)
  To: Roman Kagan, Denis Plotnikov, qemu-devel, qemu-block,
	raphael.norwitz, kwolf, yc-core
  Cc: pbonzini, ehabkost, dgilbert

On Tue, Oct 05, 2021 at 02:18:40AM +0300, Roman Kagan wrote:
> On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote:
> > On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote:
> > > It might be useful for the cases when a slow block layer should be replaced
> > > with a more performant one on running VM without stopping, i.e. with very low
> > > downtime comparable with the one on migration.
> > > 
> > > It's possible to achive that for two reasons:
> > > 
> > > 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the same.
> > >   They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
> > >   each other in the values of migration service fields only.
> > > 2.The device driver used in the guest is the same: virtio-blk
> > > 
> > > In the series cross-migration is achieved by adding a new type.
> > > The new type uses virtio-blk VMState instead of vhost-user-blk specific
> > > VMstate, also it implements migration save/load callbacks to be compatible
> > > with migration stream produced by "virtio-blk" device.
> > > 
> > > Adding the new type instead of modifying the existing one is convenent.
> > > It ease to differ the new virtio-blk-compatible vhost-user-blk
> > > device from the existing non-compatible one using qemu machinery without any
> > > other modifiactions. That gives all the variety of qemu device related
> > > constraints out of box.
> > 
> > Hmm I'm not sure I understand. What is the advantage for the user?
> > What if vhost-user-blk became an alias for vhost-user-virtio-blk?
> > We could add some hacks to make it compatible for old machine types.
> 
> The point is that virtio-blk and vhost-user-blk are not
> migration-compatible ATM.  OTOH they are the same device from the guest
> POV so there's nothing fundamentally preventing the migration between
> the two.  In particular, we see it as a means to switch between the
> storage backend transports via live migration without disrupting the
> guest.
> 
> Migration-wise virtio-blk and vhost-user-blk have in common
> 
> - the content of the VMState -- VMSTATE_VIRTIO_DEVICE
> 
> The two differ in
> 
> - the name and the version of the VMStateDescription
> 
> - virtio-blk has an extra migration section (via .save/.load callbacks
>   on VirtioDeviceClass) containing requests in flight
> 
> It looks like to become migration-compatible with virtio-blk,
> vhost-user-blk has to start using VMStateDescription of virtio-blk and
> provide compatible .save/.load callbacks.  It isn't entirely obvious how
> to make this machine-type-dependent, so we came up with a simpler idea
> of defining a new device that shares most of the implementation with the
> original vhost-user-blk except for the migration stuff.  We're certainly
> open to suggestions on how to reconcile this under a single
> vhost-user-blk device, as this would be more user-friendly indeed.
> 
> We considered using a class property for this and defining the
> respective compat clause, but IIUC the class constructors (where .vmsd
> and .save/.load are defined) are not supposed to depend on class
> properties.
> 
> Thanks,
> Roman.

So the question is how to make vmsd depend on machine type.
CC Eduardo who poked at this kind of compat stuff recently,
paolo who looked at qom things most recently and dgilbert
for advice on migration.

-- 
MST



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-05  6:51     ` Michael S. Tsirkin
@ 2021-10-05 14:01       ` Dr. David Alan Gilbert
  2021-10-05 16:10         ` Eduardo Habkost
  0 siblings, 1 reply; 20+ messages in thread
From: Dr. David Alan Gilbert @ 2021-10-05 14:01 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kwolf, Denis Plotnikov, ehabkost, qemu-block, qemu-devel,
	raphael.norwitz, Roman Kagan, yc-core, pbonzini

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Tue, Oct 05, 2021 at 02:18:40AM +0300, Roman Kagan wrote:
> > On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote:
> > > On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote:
> > > > It might be useful for the cases when a slow block layer should be replaced
> > > > with a more performant one on running VM without stopping, i.e. with very low
> > > > downtime comparable with the one on migration.
> > > > 
> > > > It's possible to achive that for two reasons:
> > > > 
> > > > 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the same.
> > > >   They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
> > > >   each other in the values of migration service fields only.
> > > > 2.The device driver used in the guest is the same: virtio-blk
> > > > 
> > > > In the series cross-migration is achieved by adding a new type.
> > > > The new type uses virtio-blk VMState instead of vhost-user-blk specific
> > > > VMstate, also it implements migration save/load callbacks to be compatible
> > > > with migration stream produced by "virtio-blk" device.
> > > > 
> > > > Adding the new type instead of modifying the existing one is convenent.
> > > > It ease to differ the new virtio-blk-compatible vhost-user-blk
> > > > device from the existing non-compatible one using qemu machinery without any
> > > > other modifiactions. That gives all the variety of qemu device related
> > > > constraints out of box.
> > > 
> > > Hmm I'm not sure I understand. What is the advantage for the user?
> > > What if vhost-user-blk became an alias for vhost-user-virtio-blk?
> > > We could add some hacks to make it compatible for old machine types.
> > 
> > The point is that virtio-blk and vhost-user-blk are not
> > migration-compatible ATM.  OTOH they are the same device from the guest
> > POV so there's nothing fundamentally preventing the migration between
> > the two.  In particular, we see it as a means to switch between the
> > storage backend transports via live migration without disrupting the
> > guest.
> > 
> > Migration-wise virtio-blk and vhost-user-blk have in common
> > 
> > - the content of the VMState -- VMSTATE_VIRTIO_DEVICE
> > 
> > The two differ in
> > 
> > - the name and the version of the VMStateDescription
> > 
> > - virtio-blk has an extra migration section (via .save/.load callbacks
> >   on VirtioDeviceClass) containing requests in flight
> > 
> > It looks like to become migration-compatible with virtio-blk,
> > vhost-user-blk has to start using VMStateDescription of virtio-blk and
> > provide compatible .save/.load callbacks.  It isn't entirely obvious how
> > to make this machine-type-dependent, so we came up with a simpler idea
> > of defining a new device that shares most of the implementation with the
> > original vhost-user-blk except for the migration stuff.  We're certainly
> > open to suggestions on how to reconcile this under a single
> > vhost-user-blk device, as this would be more user-friendly indeed.
> > 
> > We considered using a class property for this and defining the
> > respective compat clause, but IIUC the class constructors (where .vmsd
> > and .save/.load are defined) are not supposed to depend on class
> > properties.
> > 
> > Thanks,
> > Roman.
> 
> So the question is how to make vmsd depend on machine type.
> CC Eduardo who poked at this kind of compat stuff recently,
> paolo who looked at qom things most recently and dgilbert
> for advice on migration.

I don't think I've seen anyone change vmsd name dependent on machine
type; making fields appear/disappear is easy - that just ends up as a
property on the device that's checked;  I guess if that property is
global (rather than per instance) then you can check it in
vhost_user_blk_class_init and swing the dc->vmsd pointer?

Dave


> -- 
> MST
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-05 14:01       ` Dr. David Alan Gilbert
@ 2021-10-05 16:10         ` Eduardo Habkost
  2021-10-05 22:06           ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Eduardo Habkost @ 2021-10-05 16:10 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: kwolf, Denis Plotnikov, qemu-block, Michael S. Tsirkin,
	qemu-devel, raphael.norwitz, Roman Kagan, yc-core, pbonzini,
	Philippe Mathieu-Daudé

On Tue, Oct 05, 2021 at 03:01:05PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Tue, Oct 05, 2021 at 02:18:40AM +0300, Roman Kagan wrote:
> > > On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote:
> > > > On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote:
> > > > > It might be useful for the cases when a slow block layer should be replaced
> > > > > with a more performant one on running VM without stopping, i.e. with very low
> > > > > downtime comparable with the one on migration.
> > > > > 
> > > > > It's possible to achive that for two reasons:
> > > > > 
> > > > > 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the same.
> > > > >   They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
> > > > >   each other in the values of migration service fields only.
> > > > > 2.The device driver used in the guest is the same: virtio-blk
> > > > > 
> > > > > In the series cross-migration is achieved by adding a new type.
> > > > > The new type uses virtio-blk VMState instead of vhost-user-blk specific
> > > > > VMstate, also it implements migration save/load callbacks to be compatible
> > > > > with migration stream produced by "virtio-blk" device.
> > > > > 
> > > > > Adding the new type instead of modifying the existing one is convenent.
> > > > > It ease to differ the new virtio-blk-compatible vhost-user-blk
> > > > > device from the existing non-compatible one using qemu machinery without any
> > > > > other modifiactions. That gives all the variety of qemu device related
> > > > > constraints out of box.
> > > > 
> > > > Hmm I'm not sure I understand. What is the advantage for the user?
> > > > What if vhost-user-blk became an alias for vhost-user-virtio-blk?
> > > > We could add some hacks to make it compatible for old machine types.
> > > 
> > > The point is that virtio-blk and vhost-user-blk are not
> > > migration-compatible ATM.  OTOH they are the same device from the guest
> > > POV so there's nothing fundamentally preventing the migration between
> > > the two.  In particular, we see it as a means to switch between the
> > > storage backend transports via live migration without disrupting the
> > > guest.
> > > 
> > > Migration-wise virtio-blk and vhost-user-blk have in common
> > > 
> > > - the content of the VMState -- VMSTATE_VIRTIO_DEVICE
> > > 
> > > The two differ in
> > > 
> > > - the name and the version of the VMStateDescription
> > > 
> > > - virtio-blk has an extra migration section (via .save/.load callbacks
> > >   on VirtioDeviceClass) containing requests in flight
> > > 
> > > It looks like to become migration-compatible with virtio-blk,
> > > vhost-user-blk has to start using VMStateDescription of virtio-blk and
> > > provide compatible .save/.load callbacks.  It isn't entirely obvious how
> > > to make this machine-type-dependent, so we came up with a simpler idea
> > > of defining a new device that shares most of the implementation with the
> > > original vhost-user-blk except for the migration stuff.  We're certainly
> > > open to suggestions on how to reconcile this under a single
> > > vhost-user-blk device, as this would be more user-friendly indeed.
> > > 
> > > We considered using a class property for this and defining the
> > > respective compat clause, but IIUC the class constructors (where .vmsd
> > > and .save/.load are defined) are not supposed to depend on class
> > > properties.
> > > 
> > > Thanks,
> > > Roman.
> > 
> > So the question is how to make vmsd depend on machine type.
> > CC Eduardo who poked at this kind of compat stuff recently,
> > paolo who looked at qom things most recently and dgilbert
> > for advice on migration.
> 
> I don't think I've seen anyone change vmsd name dependent on machine
> type; making fields appear/disappear is easy - that just ends up as a
> property on the device that's checked;  I guess if that property is
> global (rather than per instance) then you can check it in
> vhost_user_blk_class_init and swing the dc->vmsd pointer?

class_init can be called very early during QEMU initialization,
so it's too early to make decisions based on machine type.

Making a specific vmsd appear/disappear based on machine
configuration or state is "easy", by implementing
VMStateDescription.needed.  But this would require registering
both vmsds (one of them would need to be registered manually
instead of using DeviceClass.vmsd).

I don't remember what are the consequences of not using
DeviceClass.vmsd to register a vmsd, I only remember it was
subtle.  See commit b170fce3dd06 ("cpu: Register
VMStateDescription through CPUState") and related threads.  CCing
Philippe, who might remember the details here.

If that's an important use case, I would suggest allowing devices
to implement a DeviceClass.get_vmsd method, which would override
DeviceClass.vmsd if necessary.  Is the problem we're trying to
address worth the additional complexity?

-- 
Eduardo



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-05 16:10         ` Eduardo Habkost
@ 2021-10-05 22:06           ` Michael S. Tsirkin
  2021-10-06  8:09             ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2021-10-05 22:06 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: kwolf, Denis Plotnikov, qemu-block, qemu-devel,
	Dr. David Alan Gilbert, raphael.norwitz, Roman Kagan, yc-core,
	pbonzini, Philippe Mathieu-Daudé

On Tue, Oct 05, 2021 at 12:10:08PM -0400, Eduardo Habkost wrote:
> On Tue, Oct 05, 2021 at 03:01:05PM +0100, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > On Tue, Oct 05, 2021 at 02:18:40AM +0300, Roman Kagan wrote:
> > > > On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote:
> > > > > On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote:
> > > > > > It might be useful for the cases when a slow block layer should be replaced
> > > > > > with a more performant one on running VM without stopping, i.e. with very low
> > > > > > downtime comparable with the one on migration.
> > > > > > 
> > > > > > It's possible to achive that for two reasons:
> > > > > > 
> > > > > > 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the same.
> > > > > >   They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
> > > > > >   each other in the values of migration service fields only.
> > > > > > 2.The device driver used in the guest is the same: virtio-blk
> > > > > > 
> > > > > > In the series cross-migration is achieved by adding a new type.
> > > > > > The new type uses virtio-blk VMState instead of vhost-user-blk specific
> > > > > > VMstate, also it implements migration save/load callbacks to be compatible
> > > > > > with migration stream produced by "virtio-blk" device.
> > > > > > 
> > > > > > Adding the new type instead of modifying the existing one is convenent.
> > > > > > It ease to differ the new virtio-blk-compatible vhost-user-blk
> > > > > > device from the existing non-compatible one using qemu machinery without any
> > > > > > other modifiactions. That gives all the variety of qemu device related
> > > > > > constraints out of box.
> > > > > 
> > > > > Hmm I'm not sure I understand. What is the advantage for the user?
> > > > > What if vhost-user-blk became an alias for vhost-user-virtio-blk?
> > > > > We could add some hacks to make it compatible for old machine types.
> > > > 
> > > > The point is that virtio-blk and vhost-user-blk are not
> > > > migration-compatible ATM.  OTOH they are the same device from the guest
> > > > POV so there's nothing fundamentally preventing the migration between
> > > > the two.  In particular, we see it as a means to switch between the
> > > > storage backend transports via live migration without disrupting the
> > > > guest.
> > > > 
> > > > Migration-wise virtio-blk and vhost-user-blk have in common
> > > > 
> > > > - the content of the VMState -- VMSTATE_VIRTIO_DEVICE
> > > > 
> > > > The two differ in
> > > > 
> > > > - the name and the version of the VMStateDescription
> > > > 
> > > > - virtio-blk has an extra migration section (via .save/.load callbacks
> > > >   on VirtioDeviceClass) containing requests in flight
> > > > 
> > > > It looks like to become migration-compatible with virtio-blk,
> > > > vhost-user-blk has to start using VMStateDescription of virtio-blk and
> > > > provide compatible .save/.load callbacks.  It isn't entirely obvious how
> > > > to make this machine-type-dependent, so we came up with a simpler idea
> > > > of defining a new device that shares most of the implementation with the
> > > > original vhost-user-blk except for the migration stuff.  We're certainly
> > > > open to suggestions on how to reconcile this under a single
> > > > vhost-user-blk device, as this would be more user-friendly indeed.
> > > > 
> > > > We considered using a class property for this and defining the
> > > > respective compat clause, but IIUC the class constructors (where .vmsd
> > > > and .save/.load are defined) are not supposed to depend on class
> > > > properties.
> > > > 
> > > > Thanks,
> > > > Roman.
> > > 
> > > So the question is how to make vmsd depend on machine type.
> > > CC Eduardo who poked at this kind of compat stuff recently,
> > > paolo who looked at qom things most recently and dgilbert
> > > for advice on migration.
> > 
> > I don't think I've seen anyone change vmsd name dependent on machine
> > type; making fields appear/disappear is easy - that just ends up as a
> > property on the device that's checked;  I guess if that property is
> > global (rather than per instance) then you can check it in
> > vhost_user_blk_class_init and swing the dc->vmsd pointer?
> 
> class_init can be called very early during QEMU initialization,
> so it's too early to make decisions based on machine type.
> 
> Making a specific vmsd appear/disappear based on machine
> configuration or state is "easy", by implementing
> VMStateDescription.needed.  But this would require registering
> both vmsds (one of them would need to be registered manually
> instead of using DeviceClass.vmsd).
> 
> I don't remember what are the consequences of not using
> DeviceClass.vmsd to register a vmsd, I only remember it was
> subtle.  See commit b170fce3dd06 ("cpu: Register
> VMStateDescription through CPUState") and related threads.  CCing
> Philippe, who might remember the details here.
> 
> If that's an important use case, I would suggest allowing devices
> to implement a DeviceClass.get_vmsd method, which would override
> DeviceClass.vmsd if necessary.  Is the problem we're trying to
> address worth the additional complexity?

The tricky part is that we generally dont support migration when
command line is different on source and destination ...

So maybe the actual answer is that vhost-user-blk should really
be a drive supplied to a virtio blk device, not a device
itself?
This way it's sane, and also matches what we do e.g. for net.

-- 
MST



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-05 22:06           ` Michael S. Tsirkin
@ 2021-10-06  8:09             ` Dr. David Alan Gilbert
  2021-10-06  8:17               ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Dr. David Alan Gilbert @ 2021-10-06  8:09 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kwolf, Denis Plotnikov, Eduardo Habkost, qemu-block, qemu-devel,
	raphael.norwitz, Roman Kagan, yc-core, pbonzini,
	Philippe Mathieu-Daudé

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Tue, Oct 05, 2021 at 12:10:08PM -0400, Eduardo Habkost wrote:
> > On Tue, Oct 05, 2021 at 03:01:05PM +0100, Dr. David Alan Gilbert wrote:
> > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > On Tue, Oct 05, 2021 at 02:18:40AM +0300, Roman Kagan wrote:
> > > > > On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote:
> > > > > > On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote:
> > > > > > > It might be useful for the cases when a slow block layer should be replaced
> > > > > > > with a more performant one on running VM without stopping, i.e. with very low
> > > > > > > downtime comparable with the one on migration.
> > > > > > > 
> > > > > > > It's possible to achive that for two reasons:
> > > > > > > 
> > > > > > > 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the same.
> > > > > > >   They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
> > > > > > >   each other in the values of migration service fields only.
> > > > > > > 2.The device driver used in the guest is the same: virtio-blk
> > > > > > > 
> > > > > > > In the series cross-migration is achieved by adding a new type.
> > > > > > > The new type uses virtio-blk VMState instead of vhost-user-blk specific
> > > > > > > VMstate, also it implements migration save/load callbacks to be compatible
> > > > > > > with migration stream produced by "virtio-blk" device.
> > > > > > > 
> > > > > > > Adding the new type instead of modifying the existing one is convenent.
> > > > > > > It ease to differ the new virtio-blk-compatible vhost-user-blk
> > > > > > > device from the existing non-compatible one using qemu machinery without any
> > > > > > > other modifiactions. That gives all the variety of qemu device related
> > > > > > > constraints out of box.
> > > > > > 
> > > > > > Hmm I'm not sure I understand. What is the advantage for the user?
> > > > > > What if vhost-user-blk became an alias for vhost-user-virtio-blk?
> > > > > > We could add some hacks to make it compatible for old machine types.
> > > > > 
> > > > > The point is that virtio-blk and vhost-user-blk are not
> > > > > migration-compatible ATM.  OTOH they are the same device from the guest
> > > > > POV so there's nothing fundamentally preventing the migration between
> > > > > the two.  In particular, we see it as a means to switch between the
> > > > > storage backend transports via live migration without disrupting the
> > > > > guest.
> > > > > 
> > > > > Migration-wise virtio-blk and vhost-user-blk have in common
> > > > > 
> > > > > - the content of the VMState -- VMSTATE_VIRTIO_DEVICE
> > > > > 
> > > > > The two differ in
> > > > > 
> > > > > - the name and the version of the VMStateDescription
> > > > > 
> > > > > - virtio-blk has an extra migration section (via .save/.load callbacks
> > > > >   on VirtioDeviceClass) containing requests in flight
> > > > > 
> > > > > It looks like to become migration-compatible with virtio-blk,
> > > > > vhost-user-blk has to start using VMStateDescription of virtio-blk and
> > > > > provide compatible .save/.load callbacks.  It isn't entirely obvious how
> > > > > to make this machine-type-dependent, so we came up with a simpler idea
> > > > > of defining a new device that shares most of the implementation with the
> > > > > original vhost-user-blk except for the migration stuff.  We're certainly
> > > > > open to suggestions on how to reconcile this under a single
> > > > > vhost-user-blk device, as this would be more user-friendly indeed.
> > > > > 
> > > > > We considered using a class property for this and defining the
> > > > > respective compat clause, but IIUC the class constructors (where .vmsd
> > > > > and .save/.load are defined) are not supposed to depend on class
> > > > > properties.
> > > > > 
> > > > > Thanks,
> > > > > Roman.
> > > > 
> > > > So the question is how to make vmsd depend on machine type.
> > > > CC Eduardo who poked at this kind of compat stuff recently,
> > > > paolo who looked at qom things most recently and dgilbert
> > > > for advice on migration.
> > > 
> > > I don't think I've seen anyone change vmsd name dependent on machine
> > > type; making fields appear/disappear is easy - that just ends up as a
> > > property on the device that's checked;  I guess if that property is
> > > global (rather than per instance) then you can check it in
> > > vhost_user_blk_class_init and swing the dc->vmsd pointer?
> > 
> > class_init can be called very early during QEMU initialization,
> > so it's too early to make decisions based on machine type.
> > 
> > Making a specific vmsd appear/disappear based on machine
> > configuration or state is "easy", by implementing
> > VMStateDescription.needed.  But this would require registering
> > both vmsds (one of them would need to be registered manually
> > instead of using DeviceClass.vmsd).
> > 
> > I don't remember what are the consequences of not using
> > DeviceClass.vmsd to register a vmsd, I only remember it was
> > subtle.  See commit b170fce3dd06 ("cpu: Register
> > VMStateDescription through CPUState") and related threads.  CCing
> > Philippe, who might remember the details here.
> > 
> > If that's an important use case, I would suggest allowing devices
> > to implement a DeviceClass.get_vmsd method, which would override
> > DeviceClass.vmsd if necessary.  Is the problem we're trying to
> > address worth the additional complexity?
> 
> The tricky part is that we generally dont support migration when
> command line is different on source and destination ...

The reality has always been a bit more subtle than that.
For example, it's fine if the path to a block device is different on the
source and destination; or if it's accessed by iSCSI on the destination
say.  As long as what the guest sees, and the migration stream carries
are the same, then in principal it's OK - but that does start getting
trickier; also it would prboably get interesting to let libvirt know
that this combo is OK.

> So maybe the actual answer is that vhost-user-blk should really
> be a drive supplied to a virtio blk device, not a device
> itself?
> This way it's sane, and also matches what we do e.g. for net.

Hmm a bit of a fudge; it's not quite the same as a drive is it; there's
almost another layer split in there.

Dave

> -- 
> MST
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-06  8:09             ` Dr. David Alan Gilbert
@ 2021-10-06  8:17               ` Michael S. Tsirkin
  2021-10-06  8:28                 ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2021-10-06  8:17 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: kwolf, Denis Plotnikov, Eduardo Habkost, qemu-block, qemu-devel,
	raphael.norwitz, Roman Kagan, yc-core, pbonzini,
	Philippe Mathieu-Daudé

On Wed, Oct 06, 2021 at 09:09:30AM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Tue, Oct 05, 2021 at 12:10:08PM -0400, Eduardo Habkost wrote:
> > > On Tue, Oct 05, 2021 at 03:01:05PM +0100, Dr. David Alan Gilbert wrote:
> > > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > > On Tue, Oct 05, 2021 at 02:18:40AM +0300, Roman Kagan wrote:
> > > > > > On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote:
> > > > > > > On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote:
> > > > > > > > It might be useful for the cases when a slow block layer should be replaced
> > > > > > > > with a more performant one on running VM without stopping, i.e. with very low
> > > > > > > > downtime comparable with the one on migration.
> > > > > > > > 
> > > > > > > > It's possible to achive that for two reasons:
> > > > > > > > 
> > > > > > > > 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the same.
> > > > > > > >   They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
> > > > > > > >   each other in the values of migration service fields only.
> > > > > > > > 2.The device driver used in the guest is the same: virtio-blk
> > > > > > > > 
> > > > > > > > In the series cross-migration is achieved by adding a new type.
> > > > > > > > The new type uses virtio-blk VMState instead of vhost-user-blk specific
> > > > > > > > VMstate, also it implements migration save/load callbacks to be compatible
> > > > > > > > with migration stream produced by "virtio-blk" device.
> > > > > > > > 
> > > > > > > > Adding the new type instead of modifying the existing one is convenent.
> > > > > > > > It ease to differ the new virtio-blk-compatible vhost-user-blk
> > > > > > > > device from the existing non-compatible one using qemu machinery without any
> > > > > > > > other modifiactions. That gives all the variety of qemu device related
> > > > > > > > constraints out of box.
> > > > > > > 
> > > > > > > Hmm I'm not sure I understand. What is the advantage for the user?
> > > > > > > What if vhost-user-blk became an alias for vhost-user-virtio-blk?
> > > > > > > We could add some hacks to make it compatible for old machine types.
> > > > > > 
> > > > > > The point is that virtio-blk and vhost-user-blk are not
> > > > > > migration-compatible ATM.  OTOH they are the same device from the guest
> > > > > > POV so there's nothing fundamentally preventing the migration between
> > > > > > the two.  In particular, we see it as a means to switch between the
> > > > > > storage backend transports via live migration without disrupting the
> > > > > > guest.
> > > > > > 
> > > > > > Migration-wise virtio-blk and vhost-user-blk have in common
> > > > > > 
> > > > > > - the content of the VMState -- VMSTATE_VIRTIO_DEVICE
> > > > > > 
> > > > > > The two differ in
> > > > > > 
> > > > > > - the name and the version of the VMStateDescription
> > > > > > 
> > > > > > - virtio-blk has an extra migration section (via .save/.load callbacks
> > > > > >   on VirtioDeviceClass) containing requests in flight
> > > > > > 
> > > > > > It looks like to become migration-compatible with virtio-blk,
> > > > > > vhost-user-blk has to start using VMStateDescription of virtio-blk and
> > > > > > provide compatible .save/.load callbacks.  It isn't entirely obvious how
> > > > > > to make this machine-type-dependent, so we came up with a simpler idea
> > > > > > of defining a new device that shares most of the implementation with the
> > > > > > original vhost-user-blk except for the migration stuff.  We're certainly
> > > > > > open to suggestions on how to reconcile this under a single
> > > > > > vhost-user-blk device, as this would be more user-friendly indeed.
> > > > > > 
> > > > > > We considered using a class property for this and defining the
> > > > > > respective compat clause, but IIUC the class constructors (where .vmsd
> > > > > > and .save/.load are defined) are not supposed to depend on class
> > > > > > properties.
> > > > > > 
> > > > > > Thanks,
> > > > > > Roman.
> > > > > 
> > > > > So the question is how to make vmsd depend on machine type.
> > > > > CC Eduardo who poked at this kind of compat stuff recently,
> > > > > paolo who looked at qom things most recently and dgilbert
> > > > > for advice on migration.
> > > > 
> > > > I don't think I've seen anyone change vmsd name dependent on machine
> > > > type; making fields appear/disappear is easy - that just ends up as a
> > > > property on the device that's checked;  I guess if that property is
> > > > global (rather than per instance) then you can check it in
> > > > vhost_user_blk_class_init and swing the dc->vmsd pointer?
> > > 
> > > class_init can be called very early during QEMU initialization,
> > > so it's too early to make decisions based on machine type.
> > > 
> > > Making a specific vmsd appear/disappear based on machine
> > > configuration or state is "easy", by implementing
> > > VMStateDescription.needed.  But this would require registering
> > > both vmsds (one of them would need to be registered manually
> > > instead of using DeviceClass.vmsd).
> > > 
> > > I don't remember what are the consequences of not using
> > > DeviceClass.vmsd to register a vmsd, I only remember it was
> > > subtle.  See commit b170fce3dd06 ("cpu: Register
> > > VMStateDescription through CPUState") and related threads.  CCing
> > > Philippe, who might remember the details here.
> > > 
> > > If that's an important use case, I would suggest allowing devices
> > > to implement a DeviceClass.get_vmsd method, which would override
> > > DeviceClass.vmsd if necessary.  Is the problem we're trying to
> > > address worth the additional complexity?
> > 
> > The tricky part is that we generally dont support migration when
> > command line is different on source and destination ...
> 
> The reality has always been a bit more subtle than that.
> For example, it's fine if the path to a block device is different on the
> source and destination; or if it's accessed by iSCSI on the destination
> say.  As long as what the guest sees, and the migration stream carries
> are the same, then in principal it's OK - but that does start getting
> trickier; also it would prboably get interesting to let libvirt know
> that this combo is OK.

I agree, but that's not the same as specifying a different
device. Yes we internally they are compatible, but
this is a detail users/tools generally won't be able to
figure out.

> > So maybe the actual answer is that vhost-user-blk should really
> > be a drive supplied to a virtio blk device, not a device
> > itself?
> > This way it's sane, and also matches what we do e.g. for net.
> 
> Hmm a bit of a fudge; it's not quite the same as a drive is it; there's
> almost another layer split in there.
> 
> Dave

We can make it something else, not "drive=". Maybe simply "vhost-user=" ?
Point is if we promise it looks the same to guest it should be the
same -device.


> > -- 
> > MST
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-06  8:17               ` Michael S. Tsirkin
@ 2021-10-06  8:28                 ` Dr. David Alan Gilbert
  2021-10-06  8:36                   ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Dr. David Alan Gilbert @ 2021-10-06  8:28 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kwolf, Denis Plotnikov, Eduardo Habkost, qemu-block, qemu-devel,
	raphael.norwitz, Roman Kagan, yc-core, pbonzini,
	Philippe Mathieu-Daudé

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Wed, Oct 06, 2021 at 09:09:30AM +0100, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > On Tue, Oct 05, 2021 at 12:10:08PM -0400, Eduardo Habkost wrote:
> > > > On Tue, Oct 05, 2021 at 03:01:05PM +0100, Dr. David Alan Gilbert wrote:
> > > > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > > > On Tue, Oct 05, 2021 at 02:18:40AM +0300, Roman Kagan wrote:
> > > > > > > On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote:
> > > > > > > > On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote:
> > > > > > > > > It might be useful for the cases when a slow block layer should be replaced
> > > > > > > > > with a more performant one on running VM without stopping, i.e. with very low
> > > > > > > > > downtime comparable with the one on migration.
> > > > > > > > > 
> > > > > > > > > It's possible to achive that for two reasons:
> > > > > > > > > 
> > > > > > > > > 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the same.
> > > > > > > > >   They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
> > > > > > > > >   each other in the values of migration service fields only.
> > > > > > > > > 2.The device driver used in the guest is the same: virtio-blk
> > > > > > > > > 
> > > > > > > > > In the series cross-migration is achieved by adding a new type.
> > > > > > > > > The new type uses virtio-blk VMState instead of vhost-user-blk specific
> > > > > > > > > VMstate, also it implements migration save/load callbacks to be compatible
> > > > > > > > > with migration stream produced by "virtio-blk" device.
> > > > > > > > > 
> > > > > > > > > Adding the new type instead of modifying the existing one is convenent.
> > > > > > > > > It ease to differ the new virtio-blk-compatible vhost-user-blk
> > > > > > > > > device from the existing non-compatible one using qemu machinery without any
> > > > > > > > > other modifiactions. That gives all the variety of qemu device related
> > > > > > > > > constraints out of box.
> > > > > > > > 
> > > > > > > > Hmm I'm not sure I understand. What is the advantage for the user?
> > > > > > > > What if vhost-user-blk became an alias for vhost-user-virtio-blk?
> > > > > > > > We could add some hacks to make it compatible for old machine types.
> > > > > > > 
> > > > > > > The point is that virtio-blk and vhost-user-blk are not
> > > > > > > migration-compatible ATM.  OTOH they are the same device from the guest
> > > > > > > POV so there's nothing fundamentally preventing the migration between
> > > > > > > the two.  In particular, we see it as a means to switch between the
> > > > > > > storage backend transports via live migration without disrupting the
> > > > > > > guest.
> > > > > > > 
> > > > > > > Migration-wise virtio-blk and vhost-user-blk have in common
> > > > > > > 
> > > > > > > - the content of the VMState -- VMSTATE_VIRTIO_DEVICE
> > > > > > > 
> > > > > > > The two differ in
> > > > > > > 
> > > > > > > - the name and the version of the VMStateDescription
> > > > > > > 
> > > > > > > - virtio-blk has an extra migration section (via .save/.load callbacks
> > > > > > >   on VirtioDeviceClass) containing requests in flight
> > > > > > > 
> > > > > > > It looks like to become migration-compatible with virtio-blk,
> > > > > > > vhost-user-blk has to start using VMStateDescription of virtio-blk and
> > > > > > > provide compatible .save/.load callbacks.  It isn't entirely obvious how
> > > > > > > to make this machine-type-dependent, so we came up with a simpler idea
> > > > > > > of defining a new device that shares most of the implementation with the
> > > > > > > original vhost-user-blk except for the migration stuff.  We're certainly
> > > > > > > open to suggestions on how to reconcile this under a single
> > > > > > > vhost-user-blk device, as this would be more user-friendly indeed.
> > > > > > > 
> > > > > > > We considered using a class property for this and defining the
> > > > > > > respective compat clause, but IIUC the class constructors (where .vmsd
> > > > > > > and .save/.load are defined) are not supposed to depend on class
> > > > > > > properties.
> > > > > > > 
> > > > > > > Thanks,
> > > > > > > Roman.
> > > > > > 
> > > > > > So the question is how to make vmsd depend on machine type.
> > > > > > CC Eduardo who poked at this kind of compat stuff recently,
> > > > > > paolo who looked at qom things most recently and dgilbert
> > > > > > for advice on migration.
> > > > > 
> > > > > I don't think I've seen anyone change vmsd name dependent on machine
> > > > > type; making fields appear/disappear is easy - that just ends up as a
> > > > > property on the device that's checked;  I guess if that property is
> > > > > global (rather than per instance) then you can check it in
> > > > > vhost_user_blk_class_init and swing the dc->vmsd pointer?
> > > > 
> > > > class_init can be called very early during QEMU initialization,
> > > > so it's too early to make decisions based on machine type.
> > > > 
> > > > Making a specific vmsd appear/disappear based on machine
> > > > configuration or state is "easy", by implementing
> > > > VMStateDescription.needed.  But this would require registering
> > > > both vmsds (one of them would need to be registered manually
> > > > instead of using DeviceClass.vmsd).
> > > > 
> > > > I don't remember what are the consequences of not using
> > > > DeviceClass.vmsd to register a vmsd, I only remember it was
> > > > subtle.  See commit b170fce3dd06 ("cpu: Register
> > > > VMStateDescription through CPUState") and related threads.  CCing
> > > > Philippe, who might remember the details here.
> > > > 
> > > > If that's an important use case, I would suggest allowing devices
> > > > to implement a DeviceClass.get_vmsd method, which would override
> > > > DeviceClass.vmsd if necessary.  Is the problem we're trying to
> > > > address worth the additional complexity?
> > > 
> > > The tricky part is that we generally dont support migration when
> > > command line is different on source and destination ...
> > 
> > The reality has always been a bit more subtle than that.
> > For example, it's fine if the path to a block device is different on the
> > source and destination; or if it's accessed by iSCSI on the destination
> > say.  As long as what the guest sees, and the migration stream carries
> > are the same, then in principal it's OK - but that does start getting
> > trickier; also it would prboably get interesting to let libvirt know
> > that this combo is OK.
> 
> I agree, but that's not the same as specifying a different
> device. Yes we internally they are compatible, but
> this is a detail users/tools generally won't be able to
> figure out.

Yeh.

> > > So maybe the actual answer is that vhost-user-blk should really
> > > be a drive supplied to a virtio blk device, not a device
> > > itself?
> > > This way it's sane, and also matches what we do e.g. for net.
> > 
> > Hmm a bit of a fudge; it's not quite the same as a drive is it; there's
> > almost another layer split in there.
> > 
> > Dave
> 
> We can make it something else, not "drive=". Maybe simply "vhost-user=" ?
> Point is if we promise it looks the same to guest it should be the
> same -device.

To me it feels the same as the distinction between vhost-kernel and qemu
backended virtio that we get in net and others - in principal it's just 
another implementation.
A tricky part is guaranteeing the set of visible virtio features between
implementations; we have that problem when we use vhost-kernel and run
on a newer/older kernel and gain virtio features; the same will be true
with vhost-user implementations.

But this would make the structure of a vhost-user implementation quite
different.

Dave

> 
> > > -- 
> > > MST
> > > 
> > -- 
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-06  8:28                 ` Dr. David Alan Gilbert
@ 2021-10-06  8:36                   ` Michael S. Tsirkin
  2021-10-06  8:43                     ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2021-10-06  8:36 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: kwolf, Denis Plotnikov, Eduardo Habkost, qemu-block, qemu-devel,
	raphael.norwitz, Roman Kagan, yc-core, pbonzini,
	Philippe Mathieu-Daudé

On Wed, Oct 06, 2021 at 09:28:50AM +0100, Dr. David Alan Gilbert wrote:
> To me it feels the same as the distinction between vhost-kernel and qemu
> backended virtio that we get in net and others - in principal it's just 
> another implementation.

In net it's actually like this. Same -device, a different netdev.

> A tricky part is guaranteeing the set of visible virtio features between
> implementations; we have that problem when we use vhost-kernel and run
> on a newer/older kernel and gain virtio features; the same will be true
> with vhost-user implementations.

That's not new but yes we need to work on this.

> But this would make the structure of a vhost-user implementation quite
> different.
> 
> Dave

Right. That's why I'm reluctant to just add a new device type that
has special compatibility requirements.

> > 
> > > > -- 
> > > > MST
> > > > 
> > > -- 
> > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-06  8:36                   ` Michael S. Tsirkin
@ 2021-10-06  8:43                     ` Dr. David Alan Gilbert
  2021-10-06 12:18                       ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Dr. David Alan Gilbert @ 2021-10-06  8:43 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kwolf, Denis Plotnikov, Eduardo Habkost, qemu-block, qemu-devel,
	raphael.norwitz, Roman Kagan, yc-core, pbonzini,
	Philippe Mathieu-Daudé

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Wed, Oct 06, 2021 at 09:28:50AM +0100, Dr. David Alan Gilbert wrote:
> > To me it feels the same as the distinction between vhost-kernel and qemu
> > backended virtio that we get in net and others - in principal it's just 
> > another implementation.
> 
> In net it's actually like this. Same -device, a different netdev.
> 
> > A tricky part is guaranteeing the set of visible virtio features between
> > implementations; we have that problem when we use vhost-kernel and run
> > on a newer/older kernel and gain virtio features; the same will be true
> > with vhost-user implementations.
> 
> That's not new but yes we need to work on this.
> 
> > But this would make the structure of a vhost-user implementation quite
> > different.
> > 
> > Dave
> 
> Right. That's why I'm reluctant to just add a new device type that
> has special compatibility requirements.

Hmm but there's already another layer of hack^Wabstraction in there isn't there -
there's already:
    virtio-blk-pci
    virtio-blk-device

created when the user specifies a virtio-blk device?

Dave


> > > 
> > > > > -- 
> > > > > MST
> > > > > 
> > > > -- 
> > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > 
> > -- 
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-06  8:43                     ` Dr. David Alan Gilbert
@ 2021-10-06 12:18                       ` Michael S. Tsirkin
  2021-10-06 13:29                         ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2021-10-06 12:18 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: kwolf, Denis Plotnikov, Eduardo Habkost, qemu-block, qemu-devel,
	raphael.norwitz, Roman Kagan, yc-core, pbonzini,
	Philippe Mathieu-Daudé

On Wed, Oct 06, 2021 at 09:43:52AM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Wed, Oct 06, 2021 at 09:28:50AM +0100, Dr. David Alan Gilbert wrote:
> > > To me it feels the same as the distinction between vhost-kernel and qemu
> > > backended virtio that we get in net and others - in principal it's just 
> > > another implementation.
> > 
> > In net it's actually like this. Same -device, a different netdev.
> > 
> > > A tricky part is guaranteeing the set of visible virtio features between
> > > implementations; we have that problem when we use vhost-kernel and run
> > > on a newer/older kernel and gain virtio features; the same will be true
> > > with vhost-user implementations.
> > 
> > That's not new but yes we need to work on this.
> > 
> > > But this would make the structure of a vhost-user implementation quite
> > > different.
> > > 
> > > Dave
> > 
> > Right. That's why I'm reluctant to just add a new device type that
> > has special compatibility requirements.
> 
> Hmm but there's already another layer of hack^Wabstraction in there isn't there -
> there's already:
>     virtio-blk-pci
>     virtio-blk-device
> 
> created when the user specifies a virtio-blk device?
> 
> Dave

virtio-*-pci is there because it was felt these devices look
differently from e.g. virtio-ccw so should have a different name.
virtio-blk-device is an internal thingy, users and guests have no idea.


> 
> > > > 
> > > > > > -- 
> > > > > > MST
> > > > > > 
> > > > > -- 
> > > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > > 
> > > -- 
> > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-06 12:18                       ` Michael S. Tsirkin
@ 2021-10-06 13:29                         ` Dr. David Alan Gilbert
  2021-10-06 13:39                           ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Dr. David Alan Gilbert @ 2021-10-06 13:29 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kwolf, Denis Plotnikov, Eduardo Habkost, qemu-block, qemu-devel,
	raphael.norwitz, Roman Kagan, yc-core, pbonzini,
	Philippe Mathieu-Daudé

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Wed, Oct 06, 2021 at 09:43:52AM +0100, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > On Wed, Oct 06, 2021 at 09:28:50AM +0100, Dr. David Alan Gilbert wrote:
> > > > To me it feels the same as the distinction between vhost-kernel and qemu
> > > > backended virtio that we get in net and others - in principal it's just 
> > > > another implementation.
> > > 
> > > In net it's actually like this. Same -device, a different netdev.
> > > 
> > > > A tricky part is guaranteeing the set of visible virtio features between
> > > > implementations; we have that problem when we use vhost-kernel and run
> > > > on a newer/older kernel and gain virtio features; the same will be true
> > > > with vhost-user implementations.
> > > 
> > > That's not new but yes we need to work on this.
> > > 
> > > > But this would make the structure of a vhost-user implementation quite
> > > > different.
> > > > 
> > > > Dave
> > > 
> > > Right. That's why I'm reluctant to just add a new device type that
> > > has special compatibility requirements.
> > 
> > Hmm but there's already another layer of hack^Wabstraction in there isn't there -
> > there's already:
> >     virtio-blk-pci
> >     virtio-blk-device
> > 
> > created when the user specifies a virtio-blk device?
> > 
> > Dave
> 
> virtio-*-pci is there because it was felt these devices look
> differently from e.g. virtio-ccw so should have a different name.
> virtio-blk-device is an internal thingy, users and guests have no idea.

Right, so to do what we're asking here, should we keep the
virtio-blk-pci and instantiate virtio-blk-vhost-user instead of
virtio-blk-device?

Dave

> 
> > 
> > > > > 
> > > > > > > -- 
> > > > > > > MST
> > > > > > > 
> > > > > > -- 
> > > > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > > > 
> > > > -- 
> > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > 
> > -- 
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-06 13:29                         ` Dr. David Alan Gilbert
@ 2021-10-06 13:39                           ` Michael S. Tsirkin
  2021-10-06 14:27                             ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2021-10-06 13:39 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: kwolf, Denis Plotnikov, Eduardo Habkost, qemu-block, qemu-devel,
	raphael.norwitz, Roman Kagan, yc-core, pbonzini,
	Philippe Mathieu-Daudé

On Wed, Oct 06, 2021 at 02:29:37PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Wed, Oct 06, 2021 at 09:43:52AM +0100, Dr. David Alan Gilbert wrote:
> > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > On Wed, Oct 06, 2021 at 09:28:50AM +0100, Dr. David Alan Gilbert wrote:
> > > > > To me it feels the same as the distinction between vhost-kernel and qemu
> > > > > backended virtio that we get in net and others - in principal it's just 
> > > > > another implementation.
> > > > 
> > > > In net it's actually like this. Same -device, a different netdev.
> > > > 
> > > > > A tricky part is guaranteeing the set of visible virtio features between
> > > > > implementations; we have that problem when we use vhost-kernel and run
> > > > > on a newer/older kernel and gain virtio features; the same will be true
> > > > > with vhost-user implementations.
> > > > 
> > > > That's not new but yes we need to work on this.
> > > > 
> > > > > But this would make the structure of a vhost-user implementation quite
> > > > > different.
> > > > > 
> > > > > Dave
> > > > 
> > > > Right. That's why I'm reluctant to just add a new device type that
> > > > has special compatibility requirements.
> > > 
> > > Hmm but there's already another layer of hack^Wabstraction in there isn't there -
> > > there's already:
> > >     virtio-blk-pci
> > >     virtio-blk-device
> > > 
> > > created when the user specifies a virtio-blk device?
> > > 
> > > Dave
> > 
> > virtio-*-pci is there because it was felt these devices look
> > differently from e.g. virtio-ccw so should have a different name.
> > virtio-blk-device is an internal thingy, users and guests have no idea.
> 
> Right, so to do what we're asking here, should we keep the
> virtio-blk-pci and instantiate virtio-blk-vhost-user instead of
> virtio-blk-device?
> 
> Dave

I guess that's possible, but we need to pass a bunch of parameters.
-drive is probably the right want to do that, right?

> > 
> > > 
> > > > > > 
> > > > > > > > -- 
> > > > > > > > MST
> > > > > > > > 
> > > > > > > -- 
> > > > > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > > > > 
> > > > > -- 
> > > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > > 
> > > -- 
> > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-06 13:39                           ` Michael S. Tsirkin
@ 2021-10-06 14:27                             ` Dr. David Alan Gilbert
  2021-10-06 14:37                               ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Dr. David Alan Gilbert @ 2021-10-06 14:27 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: kwolf, Denis Plotnikov, Eduardo Habkost, qemu-block, qemu-devel,
	raphael.norwitz, Roman Kagan, yc-core, pbonzini,
	Philippe Mathieu-Daudé

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Wed, Oct 06, 2021 at 02:29:37PM +0100, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > On Wed, Oct 06, 2021 at 09:43:52AM +0100, Dr. David Alan Gilbert wrote:
> > > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > > On Wed, Oct 06, 2021 at 09:28:50AM +0100, Dr. David Alan Gilbert wrote:
> > > > > > To me it feels the same as the distinction between vhost-kernel and qemu
> > > > > > backended virtio that we get in net and others - in principal it's just 
> > > > > > another implementation.
> > > > > 
> > > > > In net it's actually like this. Same -device, a different netdev.
> > > > > 
> > > > > > A tricky part is guaranteeing the set of visible virtio features between
> > > > > > implementations; we have that problem when we use vhost-kernel and run
> > > > > > on a newer/older kernel and gain virtio features; the same will be true
> > > > > > with vhost-user implementations.
> > > > > 
> > > > > That's not new but yes we need to work on this.
> > > > > 
> > > > > > But this would make the structure of a vhost-user implementation quite
> > > > > > different.
> > > > > > 
> > > > > > Dave
> > > > > 
> > > > > Right. That's why I'm reluctant to just add a new device type that
> > > > > has special compatibility requirements.
> > > > 
> > > > Hmm but there's already another layer of hack^Wabstraction in there isn't there -
> > > > there's already:
> > > >     virtio-blk-pci
> > > >     virtio-blk-device
> > > > 
> > > > created when the user specifies a virtio-blk device?
> > > > 
> > > > Dave
> > > 
> > > virtio-*-pci is there because it was felt these devices look
> > > differently from e.g. virtio-ccw so should have a different name.
> > > virtio-blk-device is an internal thingy, users and guests have no idea.
> > 
> > Right, so to do what we're asking here, should we keep the
> > virtio-blk-pci and instantiate virtio-blk-vhost-user instead of
> > virtio-blk-device?
> > 
> > Dave
> 
> I guess that's possible, but we need to pass a bunch of parameters.
> -drive is probably the right want to do that, right?

I'm not sure about -drive - isn't that very tied into the block layer?

Dave

> > > 
> > > > 
> > > > > > > 
> > > > > > > > > -- 
> > > > > > > > > MST
> > > > > > > > > 
> > > > > > > > -- 
> > > > > > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > > > > > 
> > > > > > -- 
> > > > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > > > 
> > > > -- 
> > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > 
> > -- 
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
  2021-10-06 14:27                             ` Dr. David Alan Gilbert
@ 2021-10-06 14:37                               ` Michael S. Tsirkin
  0 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2021-10-06 14:37 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: kwolf, Denis Plotnikov, Eduardo Habkost, qemu-block, qemu-devel,
	raphael.norwitz, Roman Kagan, yc-core, pbonzini,
	Philippe Mathieu-Daudé

On Wed, Oct 06, 2021 at 03:27:59PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Wed, Oct 06, 2021 at 02:29:37PM +0100, Dr. David Alan Gilbert wrote:
> > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > On Wed, Oct 06, 2021 at 09:43:52AM +0100, Dr. David Alan Gilbert wrote:
> > > > > * Michael S. Tsirkin (mst@redhat.com) wrote:
> > > > > > On Wed, Oct 06, 2021 at 09:28:50AM +0100, Dr. David Alan Gilbert wrote:
> > > > > > > To me it feels the same as the distinction between vhost-kernel and qemu
> > > > > > > backended virtio that we get in net and others - in principal it's just 
> > > > > > > another implementation.
> > > > > > 
> > > > > > In net it's actually like this. Same -device, a different netdev.
> > > > > > 
> > > > > > > A tricky part is guaranteeing the set of visible virtio features between
> > > > > > > implementations; we have that problem when we use vhost-kernel and run
> > > > > > > on a newer/older kernel and gain virtio features; the same will be true
> > > > > > > with vhost-user implementations.
> > > > > > 
> > > > > > That's not new but yes we need to work on this.
> > > > > > 
> > > > > > > But this would make the structure of a vhost-user implementation quite
> > > > > > > different.
> > > > > > > 
> > > > > > > Dave
> > > > > > 
> > > > > > Right. That's why I'm reluctant to just add a new device type that
> > > > > > has special compatibility requirements.
> > > > > 
> > > > > Hmm but there's already another layer of hack^Wabstraction in there isn't there -
> > > > > there's already:
> > > > >     virtio-blk-pci
> > > > >     virtio-blk-device
> > > > > 
> > > > > created when the user specifies a virtio-blk device?
> > > > > 
> > > > > Dave
> > > > 
> > > > virtio-*-pci is there because it was felt these devices look
> > > > differently from e.g. virtio-ccw so should have a different name.
> > > > virtio-blk-device is an internal thingy, users and guests have no idea.
> > > 
> > > Right, so to do what we're asking here, should we keep the
> > > virtio-blk-pci and instantiate virtio-blk-vhost-user instead of
> > > virtio-blk-device?
> > > 
> > > Dave
> > 
> > I guess that's possible, but we need to pass a bunch of parameters.
> > -drive is probably the right want to do that, right?
> 
> I'm not sure about -drive - isn't that very tied into the block layer?
> 
> Dave


If not, we'll need to add some other object, and tie it's id to the device.

> > > > 
> > > > > 
> > > > > > > > 
> > > > > > > > > > -- 
> > > > > > > > > > MST
> > > > > > > > > > 
> > > > > > > > > -- 
> > > > > > > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > > > > > > 
> > > > > > > -- 
> > > > > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > > > > 
> > > > > -- 
> > > > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > > > 
> > > -- 
> > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-10-06 14:38 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-04 15:07 [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration Denis Plotnikov
2021-10-04 15:07 ` [PATCH v0 1/2] vhost-user-blk: add a new vhost-user-virtio-blk type Denis Plotnikov
2021-10-04 15:16   ` Michael S. Tsirkin
2021-10-04 15:07 ` [PATCH v0 2/2] vhost-user-blk-pci: add new pci device type to support vhost-user-virtio-blk Denis Plotnikov
2021-10-04 15:11 ` [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration Michael S. Tsirkin
2021-10-04 23:18   ` Roman Kagan
2021-10-05  6:51     ` Michael S. Tsirkin
2021-10-05 14:01       ` Dr. David Alan Gilbert
2021-10-05 16:10         ` Eduardo Habkost
2021-10-05 22:06           ` Michael S. Tsirkin
2021-10-06  8:09             ` Dr. David Alan Gilbert
2021-10-06  8:17               ` Michael S. Tsirkin
2021-10-06  8:28                 ` Dr. David Alan Gilbert
2021-10-06  8:36                   ` Michael S. Tsirkin
2021-10-06  8:43                     ` Dr. David Alan Gilbert
2021-10-06 12:18                       ` Michael S. Tsirkin
2021-10-06 13:29                         ` Dr. David Alan Gilbert
2021-10-06 13:39                           ` Michael S. Tsirkin
2021-10-06 14:27                             ` Dr. David Alan Gilbert
2021-10-06 14:37                               ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.