qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
@ 2024-06-25 12:18 Vladimir Sementsov-Ogievskiy
  2024-06-25 12:18 ` [PATCH v5 1/3] qdev-monitor: add option to report GenericError from find_device_state Vladimir Sementsov-Ogievskiy
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-06-25 12:18 UTC (permalink / raw)
  To: qemu-block, raphael, mst
  Cc: qemu-devel, armbru, eblake, eduardo, berrange, pbonzini, hreitz,
	kwolf, vsementsov, yc-core

v5:
03: drop extra check on is is runstate running


Vladimir Sementsov-Ogievskiy (3):
  qdev-monitor: add option to report GenericError from find_device_state
  vhost-user-blk: split vhost_user_blk_sync_config()
  qapi: introduce device-sync-config

 hw/block/vhost-user-blk.c | 27 ++++++++++++++------
 hw/virtio/virtio-pci.c    |  9 +++++++
 include/hw/qdev-core.h    |  3 +++
 qapi/qdev.json            | 24 ++++++++++++++++++
 system/qdev-monitor.c     | 53 ++++++++++++++++++++++++++++++++++++---
 5 files changed, 105 insertions(+), 11 deletions(-)

-- 
2.34.1



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

* [PATCH v5 1/3] qdev-monitor: add option to report GenericError from find_device_state
  2024-06-25 12:18 [PATCH v5 0/3] vhost-user-blk: live resize additional APIs Vladimir Sementsov-Ogievskiy
@ 2024-06-25 12:18 ` Vladimir Sementsov-Ogievskiy
  2024-07-18  8:30   ` Markus Armbruster
  2024-06-25 12:18 ` [PATCH v5 2/3] vhost-user-blk: split vhost_user_blk_sync_config() Vladimir Sementsov-Ogievskiy
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-06-25 12:18 UTC (permalink / raw)
  To: qemu-block, raphael, mst
  Cc: qemu-devel, armbru, eblake, eduardo, berrange, pbonzini, hreitz,
	kwolf, vsementsov, yc-core

Here we just prepare for the following patch, making possible to report
GenericError as recommended.

This patch doesn't aim to prevent further use of DeviceNotFound by
future interfaces:

 - find_device_state() is used in blk_by_qdev_id() and qmp_get_blk()
   functions, which may lead to spread of DeviceNotFound anyway
 - also, nothing prevent simply copy-pasting find_device_state() calls
   with false argument

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 system/qdev-monitor.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 6af6ef7d66..264978aa40 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -879,13 +879,20 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
     object_unref(OBJECT(dev));
 }
 
-static DeviceState *find_device_state(const char *id, Error **errp)
+/*
+ * Note that creating new APIs using error classes other than GenericError is
+ * not recommended. Set use_generic_error=true for new interfaces.
+ */
+static DeviceState *find_device_state(const char *id, bool use_generic_error,
+                                      Error **errp)
 {
     Object *obj = object_resolve_path_at(qdev_get_peripheral(), id);
     DeviceState *dev;
 
     if (!obj) {
-        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+        error_set(errp,
+                  (use_generic_error ?
+                   ERROR_CLASS_GENERIC_ERROR : ERROR_CLASS_DEVICE_NOT_FOUND),
                   "Device '%s' not found", id);
         return NULL;
     }
@@ -950,7 +957,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
 
 void qmp_device_del(const char *id, Error **errp)
 {
-    DeviceState *dev = find_device_state(id, errp);
+    DeviceState *dev = find_device_state(id, false, errp);
     if (dev != NULL) {
         if (dev->pending_deleted_event &&
             (dev->pending_deleted_expires_ms == 0 ||
@@ -1070,7 +1077,7 @@ BlockBackend *blk_by_qdev_id(const char *id, Error **errp)
 
     GLOBAL_STATE_CODE();
 
-    dev = find_device_state(id, errp);
+    dev = find_device_state(id, false, errp);
     if (dev == NULL) {
         return NULL;
     }
-- 
2.34.1



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

* [PATCH v5 2/3] vhost-user-blk: split vhost_user_blk_sync_config()
  2024-06-25 12:18 [PATCH v5 0/3] vhost-user-blk: live resize additional APIs Vladimir Sementsov-Ogievskiy
  2024-06-25 12:18 ` [PATCH v5 1/3] qdev-monitor: add option to report GenericError from find_device_state Vladimir Sementsov-Ogievskiy
@ 2024-06-25 12:18 ` Vladimir Sementsov-Ogievskiy
  2024-06-25 12:18 ` [PATCH v5 3/3] qapi: introduce device-sync-config Vladimir Sementsov-Ogievskiy
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-06-25 12:18 UTC (permalink / raw)
  To: qemu-block, raphael, mst
  Cc: qemu-devel, armbru, eblake, eduardo, berrange, pbonzini, hreitz,
	kwolf, vsementsov, yc-core

Split vhost_user_blk_sync_config() out from
vhost_user_blk_handle_config_change(), to be reused in the following
commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 hw/block/vhost-user-blk.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9e6bbc6950..091d2c6acf 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -88,27 +88,39 @@ static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
     s->blkcfg.wce = blkcfg->wce;
 }
 
+static int vhost_user_blk_sync_config(DeviceState *dev, Error **errp)
+{
+    int ret;
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostUserBlk *s = VHOST_USER_BLK(vdev);
+
+    ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
+                               vdev->config_len, errp);
+    if (ret < 0) {
+        return ret;
+    }
+
+    memcpy(vdev->config, &s->blkcfg, vdev->config_len);
+    virtio_notify_config(vdev);
+
+    return 0;
+}
+
 static int vhost_user_blk_handle_config_change(struct vhost_dev *dev)
 {
     int ret;
-    VirtIODevice *vdev = dev->vdev;
-    VHostUserBlk *s = VHOST_USER_BLK(dev->vdev);
     Error *local_err = NULL;
 
     if (!dev->started) {
         return 0;
     }
 
-    ret = vhost_dev_get_config(dev, (uint8_t *)&s->blkcfg,
-                               vdev->config_len, &local_err);
+    ret = vhost_user_blk_sync_config(DEVICE(dev->vdev), &local_err);
     if (ret < 0) {
         error_report_err(local_err);
         return ret;
     }
 
-    memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len);
-    virtio_notify_config(dev->vdev);
-
     return 0;
 }
 
-- 
2.34.1



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

* [PATCH v5 3/3] qapi: introduce device-sync-config
  2024-06-25 12:18 [PATCH v5 0/3] vhost-user-blk: live resize additional APIs Vladimir Sementsov-Ogievskiy
  2024-06-25 12:18 ` [PATCH v5 1/3] qdev-monitor: add option to report GenericError from find_device_state Vladimir Sementsov-Ogievskiy
  2024-06-25 12:18 ` [PATCH v5 2/3] vhost-user-blk: split vhost_user_blk_sync_config() Vladimir Sementsov-Ogievskiy
@ 2024-06-25 12:18 ` Vladimir Sementsov-Ogievskiy
  2024-07-18  8:27   ` Markus Armbruster
  2024-07-01 12:42 ` [PATCH v5 0/3] vhost-user-blk: live resize additional APIs Raphael Norwitz
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-06-25 12:18 UTC (permalink / raw)
  To: qemu-block, raphael, mst
  Cc: qemu-devel, armbru, eblake, eduardo, berrange, pbonzini, hreitz,
	kwolf, vsementsov, yc-core

Add command to sync config from vhost-user backend to the device. It
may be helpful when VHOST_USER_SLAVE_CONFIG_CHANGE_MSG failed or not
triggered interrupt to the guest or just not available (not supported
by vhost-user server).

Command result is racy if allow it during migration. Let's allow the
sync only in RUNNING state.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 hw/block/vhost-user-blk.c |  1 +
 hw/virtio/virtio-pci.c    |  9 +++++++++
 include/hw/qdev-core.h    |  3 +++
 qapi/qdev.json            | 24 ++++++++++++++++++++++++
 system/qdev-monitor.c     | 38 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 75 insertions(+)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 091d2c6acf..2f301f380c 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -588,6 +588,7 @@ static void vhost_user_blk_class_init(ObjectClass *klass, void *data)
 
     device_class_set_props(dc, vhost_user_blk_properties);
     dc->vmsd = &vmstate_vhost_user_blk;
+    dc->sync_config = vhost_user_blk_sync_config;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     vdc->realize = vhost_user_blk_device_realize;
     vdc->unrealize = vhost_user_blk_device_unrealize;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index b1d02f4b3d..0d91e8b5dc 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2351,6 +2351,14 @@ static void virtio_pci_dc_realize(DeviceState *qdev, Error **errp)
     vpciklass->parent_dc_realize(qdev, errp);
 }
 
+static int virtio_pci_sync_config(DeviceState *dev, Error **errp)
+{
+    VirtIOPCIProxy *proxy = VIRTIO_PCI(dev);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
+    return qdev_sync_config(DEVICE(vdev), errp);
+}
+
 static void virtio_pci_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -2367,6 +2375,7 @@ static void virtio_pci_class_init(ObjectClass *klass, void *data)
     device_class_set_parent_realize(dc, virtio_pci_dc_realize,
                                     &vpciklass->parent_dc_realize);
     rc->phases.hold = virtio_pci_bus_reset_hold;
+    dc->sync_config = virtio_pci_sync_config;
 }
 
 static const TypeInfo virtio_pci_info = {
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 5336728a23..f992061919 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -95,6 +95,7 @@ typedef void (*DeviceUnrealize)(DeviceState *dev);
 typedef void (*DeviceReset)(DeviceState *dev);
 typedef void (*BusRealize)(BusState *bus, Error **errp);
 typedef void (*BusUnrealize)(BusState *bus);
+typedef int (*DeviceSyncConfig)(DeviceState *dev, Error **errp);
 
 /**
  * struct DeviceClass - The base class for all devices.
@@ -162,6 +163,7 @@ struct DeviceClass {
     DeviceReset reset;
     DeviceRealize realize;
     DeviceUnrealize unrealize;
+    DeviceSyncConfig sync_config;
 
     /**
      * @vmsd: device state serialisation description for
@@ -547,6 +549,7 @@ bool qdev_hotplug_allowed(DeviceState *dev, Error **errp);
  */
 HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
 void qdev_unplug(DeviceState *dev, Error **errp);
+int qdev_sync_config(DeviceState *dev, Error **errp);
 void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
                                   DeviceState *dev, Error **errp);
 void qdev_machine_creation_done(void);
diff --git a/qapi/qdev.json b/qapi/qdev.json
index facaa0bc6a..72e434bc45 100644
--- a/qapi/qdev.json
+++ b/qapi/qdev.json
@@ -161,3 +161,27 @@
 ##
 { 'event': 'DEVICE_UNPLUG_GUEST_ERROR',
   'data': { '*device': 'str', 'path': 'str' } }
+
+##
+# @device-sync-config:
+#
+# Synchronize device configuration from host to guest part.  First,
+# copy the configuration from the host part (backend) to the guest
+# part (frontend).  Then notify guest software that device
+# configuration changed.
+#
+# The command may be used to notify the guest about block device
+# capcity change.  Currently only vhost-user-blk device supports
+# this.
+#
+# @id: the device's ID or QOM path
+#
+# Features:
+#
+# @unstable: The command is experimental.
+#
+# Since: 9.1
+##
+{ 'command': 'device-sync-config',
+  'features': [ 'unstable' ],
+  'data': {'id': 'str'} }
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 264978aa40..1c29312b53 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -23,6 +23,7 @@
 #include "monitor/monitor.h"
 #include "monitor/qdev.h"
 #include "sysemu/arch_init.h"
+#include "sysemu/runstate.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-qdev.h"
 #include "qapi/qmp/dispatch.h"
@@ -971,6 +972,43 @@ void qmp_device_del(const char *id, Error **errp)
     }
 }
 
+int qdev_sync_config(DeviceState *dev, Error **errp)
+{
+    DeviceClass *dc = DEVICE_GET_CLASS(dev);
+
+    if (!dc->sync_config) {
+        error_setg(errp, "device-sync-config is not supported for '%s'",
+                   object_get_typename(OBJECT(dev)));
+        return -ENOTSUP;
+    }
+
+    return dc->sync_config(dev, errp);
+}
+
+void qmp_device_sync_config(const char *id, Error **errp)
+{
+    DeviceState *dev;
+
+    /*
+     * During migration there is a race between syncing`configuration
+     * and migrating it (if migrate first, that target would get
+     * outdated version), so let's just not allow it.
+     */
+
+    if (migration_is_running()) {
+        error_setg(errp, "Config synchronization is not allowed "
+                   "during migration");
+        return;
+    }
+
+    dev = find_device_state(id, true, errp);
+    if (!dev) {
+        return;
+    }
+
+    qdev_sync_config(dev, errp);
+}
+
 void hmp_device_add(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
-- 
2.34.1



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

* Re: [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
  2024-06-25 12:18 [PATCH v5 0/3] vhost-user-blk: live resize additional APIs Vladimir Sementsov-Ogievskiy
                   ` (2 preceding siblings ...)
  2024-06-25 12:18 ` [PATCH v5 3/3] qapi: introduce device-sync-config Vladimir Sementsov-Ogievskiy
@ 2024-07-01 12:42 ` Raphael Norwitz
  2024-07-01 20:55   ` Michael S. Tsirkin
  2024-07-11  8:53 ` Vladimir Sementsov-Ogievskiy
  2024-09-11  9:51 ` Michael S. Tsirkin
  5 siblings, 1 reply; 18+ messages in thread
From: Raphael Norwitz @ 2024-07-01 12:42 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: qemu-block, mst, qemu-devel, armbru, eblake, eduardo, berrange,
	pbonzini, hreitz, kwolf, yc-core

I have no issues with these APIs, but I'm not a QMP expert so others
should review those bits.

For the vhost-user-blk code:

Acked-by: Raphael Norwitz <raphael@enfabrica.net>

On Tue, Jun 25, 2024 at 8:19 AM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> v5:
> 03: drop extra check on is is runstate running
>
>
> Vladimir Sementsov-Ogievskiy (3):
>   qdev-monitor: add option to report GenericError from find_device_state
>   vhost-user-blk: split vhost_user_blk_sync_config()
>   qapi: introduce device-sync-config
>
>  hw/block/vhost-user-blk.c | 27 ++++++++++++++------
>  hw/virtio/virtio-pci.c    |  9 +++++++
>  include/hw/qdev-core.h    |  3 +++
>  qapi/qdev.json            | 24 ++++++++++++++++++
>  system/qdev-monitor.c     | 53 ++++++++++++++++++++++++++++++++++++---
>  5 files changed, 105 insertions(+), 11 deletions(-)
>
> --
> 2.34.1
>


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

* Re: [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
  2024-07-01 12:42 ` [PATCH v5 0/3] vhost-user-blk: live resize additional APIs Raphael Norwitz
@ 2024-07-01 20:55   ` Michael S. Tsirkin
  2024-08-01  8:35     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2024-07-01 20:55 UTC (permalink / raw)
  To: Raphael Norwitz
  Cc: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel, armbru,
	eblake, eduardo, berrange, pbonzini, hreitz, kwolf, yc-core

On Mon, Jul 01, 2024 at 08:42:39AM -0400, Raphael Norwitz wrote:
> I have no issues with these APIs, but I'm not a QMP expert so others
> should review those bits.
> 
> For the vhost-user-blk code:
> 
> Acked-by: Raphael Norwitz <raphael@enfabrica.net>

Could the relevant bits get ack from qapi maintainers please?



> On Tue, Jun 25, 2024 at 8:19 AM Vladimir Sementsov-Ogievskiy
> <vsementsov@yandex-team.ru> wrote:
> >
> > v5:
> > 03: drop extra check on is is runstate running
> >
> >
> > Vladimir Sementsov-Ogievskiy (3):
> >   qdev-monitor: add option to report GenericError from find_device_state
> >   vhost-user-blk: split vhost_user_blk_sync_config()
> >   qapi: introduce device-sync-config
> >
> >  hw/block/vhost-user-blk.c | 27 ++++++++++++++------
> >  hw/virtio/virtio-pci.c    |  9 +++++++
> >  include/hw/qdev-core.h    |  3 +++
> >  qapi/qdev.json            | 24 ++++++++++++++++++
> >  system/qdev-monitor.c     | 53 ++++++++++++++++++++++++++++++++++++---
> >  5 files changed, 105 insertions(+), 11 deletions(-)
> >
> > --
> > 2.34.1
> >



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

* Re: [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
  2024-06-25 12:18 [PATCH v5 0/3] vhost-user-blk: live resize additional APIs Vladimir Sementsov-Ogievskiy
                   ` (3 preceding siblings ...)
  2024-07-01 12:42 ` [PATCH v5 0/3] vhost-user-blk: live resize additional APIs Raphael Norwitz
@ 2024-07-11  8:53 ` Vladimir Sementsov-Ogievskiy
  2024-07-18  8:31   ` Markus Armbruster
  2024-09-11  9:51 ` Michael S. Tsirkin
  5 siblings, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-07-11  8:53 UTC (permalink / raw)
  To: qemu-block, raphael, mst, Markus Armbruster, Eric Blake
  Cc: qemu-devel, armbru, eblake, eduardo, berrange, pbonzini, hreitz,
	kwolf, yc-core

ping. Markus, Eric, could someone give an ACC for QAPI part?

On 25.06.24 15:18, Vladimir Sementsov-Ogievskiy wrote:
> v5:
> 03: drop extra check on is is runstate running
> 
> 
> Vladimir Sementsov-Ogievskiy (3):
>    qdev-monitor: add option to report GenericError from find_device_state
>    vhost-user-blk: split vhost_user_blk_sync_config()
>    qapi: introduce device-sync-config
> 
>   hw/block/vhost-user-blk.c | 27 ++++++++++++++------
>   hw/virtio/virtio-pci.c    |  9 +++++++
>   include/hw/qdev-core.h    |  3 +++
>   qapi/qdev.json            | 24 ++++++++++++++++++
>   system/qdev-monitor.c     | 53 ++++++++++++++++++++++++++++++++++++---
>   5 files changed, 105 insertions(+), 11 deletions(-)
> 

-- 
Best regards,
Vladimir



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

* Re: [PATCH v5 3/3] qapi: introduce device-sync-config
  2024-06-25 12:18 ` [PATCH v5 3/3] qapi: introduce device-sync-config Vladimir Sementsov-Ogievskiy
@ 2024-07-18  8:27   ` Markus Armbruster
  2024-07-19  8:31     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 18+ messages in thread
From: Markus Armbruster @ 2024-07-18  8:27 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: qemu-block, raphael, mst, qemu-devel, eblake, eduardo, berrange,
	pbonzini, hreitz, kwolf, yc-core

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> Add command to sync config from vhost-user backend to the device. It
> may be helpful when VHOST_USER_SLAVE_CONFIG_CHANGE_MSG failed or not
> triggered interrupt to the guest or just not available (not supported
> by vhost-user server).
>
> Command result is racy if allow it during migration. Let's allow the
> sync only in RUNNING state.

Is this still accurate?  The runstate_is_running() check is gone in
v4, the migration_is_running() check remains.

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

QAPI schema and QMP part:
Signed-off-by: Markus Armbruster <armbru@redhat.com>



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

* Re: [PATCH v5 1/3] qdev-monitor: add option to report GenericError from find_device_state
  2024-06-25 12:18 ` [PATCH v5 1/3] qdev-monitor: add option to report GenericError from find_device_state Vladimir Sementsov-Ogievskiy
@ 2024-07-18  8:30   ` Markus Armbruster
  0 siblings, 0 replies; 18+ messages in thread
From: Markus Armbruster @ 2024-07-18  8:30 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: qemu-block, raphael, mst, qemu-devel, armbru, eblake, eduardo,
	berrange, pbonzini, hreitz, kwolf, yc-core

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> Here we just prepare for the following patch, making possible to report
> GenericError as recommended.
>
> This patch doesn't aim to prevent further use of DeviceNotFound by
> future interfaces:
>
>  - find_device_state() is used in blk_by_qdev_id() and qmp_get_blk()
>    functions, which may lead to spread of DeviceNotFound anyway
>  - also, nothing prevent simply copy-pasting find_device_state() calls
>    with false argument

A possible way to reduce the likelihood of further spread:

1. Rename find_device_state() to find_device_state_legacy().

2. New find_device_state() that reports GenericError.

Could also be done in a follow-up.

>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

The patch does what it says on the tin, so
Reviewed-by: Markus Armbruster <armbru@redhat.com>



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

* Re: [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
  2024-07-11  8:53 ` Vladimir Sementsov-Ogievskiy
@ 2024-07-18  8:31   ` Markus Armbruster
  2024-07-19  8:33     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 18+ messages in thread
From: Markus Armbruster @ 2024-07-18  8:31 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: qemu-block, raphael, mst, Eric Blake, qemu-devel, eduardo,
	berrange, pbonzini, hreitz, kwolf, yc-core

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> ping. Markus, Eric, could someone give an ACC for QAPI part?

I apologize for the delay.  It was pretty bad.



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

* Re: [PATCH v5 3/3] qapi: introduce device-sync-config
  2024-07-18  8:27   ` Markus Armbruster
@ 2024-07-19  8:31     ` Vladimir Sementsov-Ogievskiy
  2024-07-19  9:10       ` Markus Armbruster
  0 siblings, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-07-19  8:31 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-block, raphael, mst, qemu-devel, eblake, eduardo, berrange,
	pbonzini, hreitz, kwolf, yc-core

On 18.07.24 11:27, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> Add command to sync config from vhost-user backend to the device. It
>> may be helpful when VHOST_USER_SLAVE_CONFIG_CHANGE_MSG failed or not
>> triggered interrupt to the guest or just not available (not supported
>> by vhost-user server).
>>
>> Command result is racy if allow it during migration. Let's allow the
>> sync only in RUNNING state.
> 
> Is this still accurate?  The runstate_is_running() check is gone in
> v4, the migration_is_running() check remains.

Right, better to fix commit message like:

Command result is racy if allow it during migration. Let's not allow it.

> 
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
> QAPI schema and QMP part:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> 

-- 
Best regards,
Vladimir



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

* Re: [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
  2024-07-18  8:31   ` Markus Armbruster
@ 2024-07-19  8:33     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-07-19  8:33 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-block, raphael, mst, Eric Blake, qemu-devel, eduardo,
	berrange, pbonzini, hreitz, kwolf, yc-core

On 18.07.24 11:31, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> ping. Markus, Eric, could someone give an ACC for QAPI part?
> 
> I apologize for the delay.  It was pretty bad.
> 

No problem, I myself make worse delays now when busy with other work, thanks for reviewing!

-- 
Best regards,
Vladimir



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

* Re: [PATCH v5 3/3] qapi: introduce device-sync-config
  2024-07-19  8:31     ` Vladimir Sementsov-Ogievskiy
@ 2024-07-19  9:10       ` Markus Armbruster
  0 siblings, 0 replies; 18+ messages in thread
From: Markus Armbruster @ 2024-07-19  9:10 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Markus Armbruster, qemu-block, raphael, mst, qemu-devel, eblake,
	eduardo, berrange, pbonzini, hreitz, kwolf, yc-core

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 18.07.24 11:27, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>> 
>>> Add command to sync config from vhost-user backend to the device. It
>>> may be helpful when VHOST_USER_SLAVE_CONFIG_CHANGE_MSG failed or not
>>> triggered interrupt to the guest or just not available (not supported
>>> by vhost-user server).
>>>
>>> Command result is racy if allow it during migration. Let's allow the
>>> sync only in RUNNING state.
>>
>> Is this still accurate?  The runstate_is_running() check is gone in
>> v4, the migration_is_running() check remains.
>
> Right, better to fix commit message like:
>
> Command result is racy if allow it during migration. Let's not allow it.

Suggest "Let's not allow that."

Thanks!

>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>
>> QAPI schema and QMP part:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>



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

* Re: [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
  2024-07-01 20:55   ` Michael S. Tsirkin
@ 2024-08-01  8:35     ` Vladimir Sementsov-Ogievskiy
  2024-08-01  8:37       ` Michael S. Tsirkin
  0 siblings, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-08-01  8:35 UTC (permalink / raw)
  To: Michael S. Tsirkin, Raphael Norwitz
  Cc: qemu-block, qemu-devel, armbru, eblake, eduardo, berrange,
	pbonzini, hreitz, kwolf, yc-core

On 01.07.24 23:55, Michael S. Tsirkin wrote:
> On Mon, Jul 01, 2024 at 08:42:39AM -0400, Raphael Norwitz wrote:
>> I have no issues with these APIs, but I'm not a QMP expert so others
>> should review those bits.
>>
>> For the vhost-user-blk code:
>>
>> Acked-by: Raphael Norwitz <raphael@enfabrica.net>
> 
> Could the relevant bits get ack from qapi maintainers please?
> 

We go them. Could you queue the patches please?

> 
> 
>> On Tue, Jun 25, 2024 at 8:19 AM Vladimir Sementsov-Ogievskiy
>> <vsementsov@yandex-team.ru> wrote:
>>>
>>> v5:
>>> 03: drop extra check on is is runstate running
>>>
>>>
>>> Vladimir Sementsov-Ogievskiy (3):
>>>    qdev-monitor: add option to report GenericError from find_device_state
>>>    vhost-user-blk: split vhost_user_blk_sync_config()
>>>    qapi: introduce device-sync-config
>>>
>>>   hw/block/vhost-user-blk.c | 27 ++++++++++++++------
>>>   hw/virtio/virtio-pci.c    |  9 +++++++
>>>   include/hw/qdev-core.h    |  3 +++
>>>   qapi/qdev.json            | 24 ++++++++++++++++++
>>>   system/qdev-monitor.c     | 53 ++++++++++++++++++++++++++++++++++++---
>>>   5 files changed, 105 insertions(+), 11 deletions(-)
>>>
>>> --
>>> 2.34.1
>>>
> 

-- 
Best regards,
Vladimir



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

* Re: [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
  2024-08-01  8:35     ` Vladimir Sementsov-Ogievskiy
@ 2024-08-01  8:37       ` Michael S. Tsirkin
  2024-08-01  8:42         ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2024-08-01  8:37 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Raphael Norwitz, qemu-block, qemu-devel, armbru, eblake, eduardo,
	berrange, pbonzini, hreitz, kwolf, yc-core

On Thu, Aug 01, 2024 at 11:35:19AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 01.07.24 23:55, Michael S. Tsirkin wrote:
> > On Mon, Jul 01, 2024 at 08:42:39AM -0400, Raphael Norwitz wrote:
> > > I have no issues with these APIs, but I'm not a QMP expert so others
> > > should review those bits.
> > > 
> > > For the vhost-user-blk code:
> > > 
> > > Acked-by: Raphael Norwitz <raphael@enfabrica.net>
> > 
> > Could the relevant bits get ack from qapi maintainers please?
> > 
> 
> We go them. Could you queue the patches please?


Tagged for after the freeze. Thanks!

> > 
> > 
> > > On Tue, Jun 25, 2024 at 8:19 AM Vladimir Sementsov-Ogievskiy
> > > <vsementsov@yandex-team.ru> wrote:
> > > > 
> > > > v5:
> > > > 03: drop extra check on is is runstate running
> > > > 
> > > > 
> > > > Vladimir Sementsov-Ogievskiy (3):
> > > >    qdev-monitor: add option to report GenericError from find_device_state
> > > >    vhost-user-blk: split vhost_user_blk_sync_config()
> > > >    qapi: introduce device-sync-config
> > > > 
> > > >   hw/block/vhost-user-blk.c | 27 ++++++++++++++------
> > > >   hw/virtio/virtio-pci.c    |  9 +++++++
> > > >   include/hw/qdev-core.h    |  3 +++
> > > >   qapi/qdev.json            | 24 ++++++++++++++++++
> > > >   system/qdev-monitor.c     | 53 ++++++++++++++++++++++++++++++++++++---
> > > >   5 files changed, 105 insertions(+), 11 deletions(-)
> > > > 
> > > > --
> > > > 2.34.1
> > > > 
> > 
> 
> -- 
> Best regards,
> Vladimir



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

* Re: [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
  2024-08-01  8:37       ` Michael S. Tsirkin
@ 2024-08-01  8:42         ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-08-01  8:42 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Raphael Norwitz, qemu-block, qemu-devel, armbru, eblake, eduardo,
	berrange, pbonzini, hreitz, kwolf, yc-core

On 01.08.24 11:37, Michael S. Tsirkin wrote:
> On Thu, Aug 01, 2024 at 11:35:19AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> On 01.07.24 23:55, Michael S. Tsirkin wrote:
>>> On Mon, Jul 01, 2024 at 08:42:39AM -0400, Raphael Norwitz wrote:
>>>> I have no issues with these APIs, but I'm not a QMP expert so others
>>>> should review those bits.
>>>>
>>>> For the vhost-user-blk code:
>>>>
>>>> Acked-by: Raphael Norwitz <raphael@enfabrica.net>
>>>
>>> Could the relevant bits get ack from qapi maintainers please?
>>>
>>
>> We go them. Could you queue the patches please?
> 
> 
> Tagged for after the freeze. Thanks!

Oh right, I missed the freeze. OK, thanks!

> 
>>>
>>>
>>>> On Tue, Jun 25, 2024 at 8:19 AM Vladimir Sementsov-Ogievskiy
>>>> <vsementsov@yandex-team.ru> wrote:
>>>>>
>>>>> v5:
>>>>> 03: drop extra check on is is runstate running
>>>>>
>>>>>
>>>>> Vladimir Sementsov-Ogievskiy (3):
>>>>>     qdev-monitor: add option to report GenericError from find_device_state
>>>>>     vhost-user-blk: split vhost_user_blk_sync_config()
>>>>>     qapi: introduce device-sync-config
>>>>>
>>>>>    hw/block/vhost-user-blk.c | 27 ++++++++++++++------
>>>>>    hw/virtio/virtio-pci.c    |  9 +++++++
>>>>>    include/hw/qdev-core.h    |  3 +++
>>>>>    qapi/qdev.json            | 24 ++++++++++++++++++
>>>>>    system/qdev-monitor.c     | 53 ++++++++++++++++++++++++++++++++++++---
>>>>>    5 files changed, 105 insertions(+), 11 deletions(-)
>>>>>
>>>>> --
>>>>> 2.34.1
>>>>>
>>>
>>
>> -- 
>> Best regards,
>> Vladimir
> 

-- 
Best regards,
Vladimir



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

* Re: [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
  2024-06-25 12:18 [PATCH v5 0/3] vhost-user-blk: live resize additional APIs Vladimir Sementsov-Ogievskiy
                   ` (4 preceding siblings ...)
  2024-07-11  8:53 ` Vladimir Sementsov-Ogievskiy
@ 2024-09-11  9:51 ` Michael S. Tsirkin
  2024-09-20  9:43   ` Vladimir Sementsov-Ogievskiy
  5 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2024-09-11  9:51 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: qemu-block, raphael, qemu-devel, armbru, eblake, eduardo,
	berrange, pbonzini, hreitz, kwolf, yc-core

On Tue, Jun 25, 2024 at 03:18:40PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> v5:
> 03: drop extra check on is is runstate running

Causes build failures when generating qdoc.

https://gitlab.com/mstredhat/qemu/-/jobs/7792086965


> 
> Vladimir Sementsov-Ogievskiy (3):
>   qdev-monitor: add option to report GenericError from find_device_state
>   vhost-user-blk: split vhost_user_blk_sync_config()
>   qapi: introduce device-sync-config
> 
>  hw/block/vhost-user-blk.c | 27 ++++++++++++++------
>  hw/virtio/virtio-pci.c    |  9 +++++++
>  include/hw/qdev-core.h    |  3 +++
>  qapi/qdev.json            | 24 ++++++++++++++++++
>  system/qdev-monitor.c     | 53 ++++++++++++++++++++++++++++++++++++---
>  5 files changed, 105 insertions(+), 11 deletions(-)
> 
> -- 
> 2.34.1



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

* Re: [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
  2024-09-11  9:51 ` Michael S. Tsirkin
@ 2024-09-20  9:43   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2024-09-20  9:43 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-block, raphael, qemu-devel, armbru, eblake, eduardo,
	berrange, pbonzini, hreitz, kwolf, yc-core

On 11.09.24 12:51, Michael S. Tsirkin wrote:
> On Tue, Jun 25, 2024 at 03:18:40PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> v5:
>> 03: drop extra check on is is runstate running
> 
> Causes build failures when generating qdoc.
> 
> https://gitlab.com/mstredhat/qemu/-/jobs/7792086965
> 
> 

Sorry for a delay, I'll send a v6 soon with fix for that.

-- 
Best regards,
Vladimir



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

end of thread, other threads:[~2024-09-20  9:44 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-25 12:18 [PATCH v5 0/3] vhost-user-blk: live resize additional APIs Vladimir Sementsov-Ogievskiy
2024-06-25 12:18 ` [PATCH v5 1/3] qdev-monitor: add option to report GenericError from find_device_state Vladimir Sementsov-Ogievskiy
2024-07-18  8:30   ` Markus Armbruster
2024-06-25 12:18 ` [PATCH v5 2/3] vhost-user-blk: split vhost_user_blk_sync_config() Vladimir Sementsov-Ogievskiy
2024-06-25 12:18 ` [PATCH v5 3/3] qapi: introduce device-sync-config Vladimir Sementsov-Ogievskiy
2024-07-18  8:27   ` Markus Armbruster
2024-07-19  8:31     ` Vladimir Sementsov-Ogievskiy
2024-07-19  9:10       ` Markus Armbruster
2024-07-01 12:42 ` [PATCH v5 0/3] vhost-user-blk: live resize additional APIs Raphael Norwitz
2024-07-01 20:55   ` Michael S. Tsirkin
2024-08-01  8:35     ` Vladimir Sementsov-Ogievskiy
2024-08-01  8:37       ` Michael S. Tsirkin
2024-08-01  8:42         ` Vladimir Sementsov-Ogievskiy
2024-07-11  8:53 ` Vladimir Sementsov-Ogievskiy
2024-07-18  8:31   ` Markus Armbruster
2024-07-19  8:33     ` Vladimir Sementsov-Ogievskiy
2024-09-11  9:51 ` Michael S. Tsirkin
2024-09-20  9:43   ` Vladimir Sementsov-Ogievskiy

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).