* [Qemu-devel] [PATCH 0/5] virtio: fix some issues of "started" and "start_on_kick" flag
@ 2019-05-29 7:09 elohimes
2019-05-29 7:09 ` [Qemu-devel] [PATCH 1/5] virtio: Set "start_on_kick" on virtio_set_features() elohimes
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: elohimes @ 2019-05-29 7:09 UTC (permalink / raw)
To: mst, groug, dgilbert; +Cc: Xie Yongji, qemu-devel
From: Xie Yongji <xieyongji@baidu.com>
We introduced two flags "started" and "start_on_kick" to indicate virtio
device's state before. But there still are some problems with them. So
we try to fixup them in this patchset.
The patch 1 fixes a regression bug that old guest is not able to boot with
vhost-user-blk device.
The patch 2,3,4 fix some problems with "started" and "start_on_kick" flag.
The patch 5 introduces a "use-started" property to avoid a migration
issue under Greg Kurz's suggestion [1].
[1] https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg06247.html
Xie Yongji (5):
virtio: Set "start_on_kick" on virtio_set_features()
virtio: Migrate the "start_on_kick" flag
virtio: Make sure we get correct state of device on
handle_aio_output()
virtio: Don't change "started" flag on virtio_vmstate_change()
virtio: add "use-started" property
hw/block/vhost-user-blk.c | 8 +++--
hw/core/machine.c | 4 ++-
hw/virtio/virtio.c | 67 +++++++++++++++++++++++++++-----------
include/hw/virtio/virtio.h | 10 ++++++
4 files changed, 67 insertions(+), 22 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 1/5] virtio: Set "start_on_kick" on virtio_set_features()
2019-05-29 7:09 [Qemu-devel] [PATCH 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
@ 2019-05-29 7:09 ` elohimes
2019-06-03 16:53 ` Greg Kurz
2019-05-29 7:09 ` [Qemu-devel] [PATCH 2/5] virtio: Migrate the "start_on_kick" flag elohimes
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: elohimes @ 2019-05-29 7:09 UTC (permalink / raw)
To: mst, groug, dgilbert; +Cc: Xie Yongji, qemu-devel
From: Xie Yongji <xieyongji@baidu.com>
The guest feature is not set correctly on virtio_reset() and
virtio_init(). So we should not use it to set "start_on_kick" at that
point. This patch set "start_on_kick" on virtio_set_features() instead.
Signed-off-by: Xie Yongji <xieyongji@baidu.com>
---
hw/virtio/virtio.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 4805727b53..fc8fca81ad 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1214,8 +1214,7 @@ void virtio_reset(void *opaque)
k->reset(vdev);
}
- vdev->start_on_kick = (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
- !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1));
+ vdev->start_on_kick = false;
vdev->started = false;
vdev->broken = false;
vdev->guest_features = 0;
@@ -2069,14 +2068,21 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
return -EINVAL;
}
ret = virtio_set_features_nocheck(vdev, val);
- if (!ret && virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
- /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
- int i;
- for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
- if (vdev->vq[i].vring.num != 0) {
- virtio_init_region_cache(vdev, i);
+ if (!ret) {
+ if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
+ /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
+ int i;
+ for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+ if (vdev->vq[i].vring.num != 0) {
+ virtio_init_region_cache(vdev, i);
+ }
}
}
+
+ if (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
+ !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+ vdev->start_on_kick = true;
+ }
}
return ret;
}
@@ -2331,8 +2337,7 @@ void virtio_init(VirtIODevice *vdev, const char *name,
g_malloc0(sizeof(*vdev->vector_queues) * nvectors);
}
- vdev->start_on_kick = (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
- !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1));
+ vdev->start_on_kick = false;
vdev->started = false;
vdev->device_id = device_id;
vdev->status = 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 2/5] virtio: Migrate the "start_on_kick" flag
2019-05-29 7:09 [Qemu-devel] [PATCH 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
2019-05-29 7:09 ` [Qemu-devel] [PATCH 1/5] virtio: Set "start_on_kick" on virtio_set_features() elohimes
@ 2019-05-29 7:09 ` elohimes
2019-06-03 20:16 ` Greg Kurz
2019-05-29 7:09 ` [Qemu-devel] [PATCH 3/5] virtio: Make sure we get correct state of device on handle_aio_output() elohimes
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: elohimes @ 2019-05-29 7:09 UTC (permalink / raw)
To: mst, groug, dgilbert; +Cc: Xie Yongji, qemu-devel
From: Xie Yongji <xieyongji@baidu.com>
We should migrate the "start_on_kick" flag so that we
would not miss starting device on kicking at startup
after migration.
Signed-off-by: Xie Yongji <xieyongji@baidu.com>
---
hw/virtio/virtio.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index fc8fca81ad..4d4ff67791 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1802,6 +1802,13 @@ static bool virtio_started_needed(void *opaque)
return vdev->started;
}
+static bool virtio_start_on_kick_needed(void *opaque)
+{
+ VirtIODevice *vdev = opaque;
+
+ return vdev->start_on_kick;
+}
+
static const VMStateDescription vmstate_virtqueue = {
.name = "virtqueue_state",
.version_id = 1,
@@ -1941,6 +1948,17 @@ static const VMStateDescription vmstate_virtio_started = {
}
};
+static const VMStateDescription vmstate_virtio_start_on_kick = {
+ .name = "virtio/start_on_kick",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = &virtio_start_on_kick_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_BOOL(start_on_kick, VirtIODevice),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_virtio = {
.name = "virtio",
.version_id = 1,
@@ -1957,6 +1975,7 @@ static const VMStateDescription vmstate_virtio = {
&vmstate_virtio_broken,
&vmstate_virtio_extra_state,
&vmstate_virtio_started,
+ &vmstate_virtio_start_on_kick,
NULL
}
};
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 3/5] virtio: Make sure we get correct state of device on handle_aio_output()
2019-05-29 7:09 [Qemu-devel] [PATCH 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
2019-05-29 7:09 ` [Qemu-devel] [PATCH 1/5] virtio: Set "start_on_kick" on virtio_set_features() elohimes
2019-05-29 7:09 ` [Qemu-devel] [PATCH 2/5] virtio: Migrate the "start_on_kick" flag elohimes
@ 2019-05-29 7:09 ` elohimes
2019-05-29 7:09 ` [Qemu-devel] [PATCH 4/5] virtio: Don't change "started" flag on virtio_vmstate_change() elohimes
2019-05-29 7:09 ` [Qemu-devel] [PATCH 5/5] virtio: add "use-started" property elohimes
4 siblings, 0 replies; 15+ messages in thread
From: elohimes @ 2019-05-29 7:09 UTC (permalink / raw)
To: mst, groug, dgilbert; +Cc: Xie Yongji, qemu-devel
From: Xie Yongji <xieyongji@baidu.com>
We should set the flags: "start_on_kick" and "started" after we call
the kick functions (handle_aio_output() and handle_output()).
Signed-off-by: Xie Yongji <xieyongji@baidu.com>
---
hw/virtio/virtio.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 4d4ff67791..8a81374492 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1575,11 +1575,11 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
event_notifier_set(&vq->host_notifier);
} else if (vq->handle_output) {
vq->handle_output(vdev, vq);
- }
- if (unlikely(vdev->start_on_kick)) {
- vdev->started = true;
- vdev->start_on_kick = false;
+ if (unlikely(vdev->start_on_kick)) {
+ vdev->started = true;
+ vdev->start_on_kick = false;
+ }
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 4/5] virtio: Don't change "started" flag on virtio_vmstate_change()
2019-05-29 7:09 [Qemu-devel] [PATCH 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
` (2 preceding siblings ...)
2019-05-29 7:09 ` [Qemu-devel] [PATCH 3/5] virtio: Make sure we get correct state of device on handle_aio_output() elohimes
@ 2019-05-29 7:09 ` elohimes
2019-05-29 7:09 ` [Qemu-devel] [PATCH 5/5] virtio: add "use-started" property elohimes
4 siblings, 0 replies; 15+ messages in thread
From: elohimes @ 2019-05-29 7:09 UTC (permalink / raw)
To: mst, groug, dgilbert; +Cc: Xie Yongji, qemu-devel
From: Xie Yongji <xieyongji@baidu.com>
We will call virtio_set_status() on virtio_vmstate_change().
The "started" flag should not be changed in this case. Otherwise,
we may get an incorrect value when we set "started" flag but
not set DRIVER_OK in source VM.
Signed-off-by: Xie Yongji <xieyongji@baidu.com>
---
hw/virtio/virtio.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8a81374492..9e17293d46 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1162,9 +1162,13 @@ int virtio_set_status(VirtIODevice *vdev, uint8_t val)
}
}
}
- vdev->started = val & VIRTIO_CONFIG_S_DRIVER_OK;
- if (unlikely(vdev->start_on_kick && vdev->started)) {
- vdev->start_on_kick = false;
+
+ if ((vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) !=
+ (val & VIRTIO_CONFIG_S_DRIVER_OK)) {
+ vdev->started = val & VIRTIO_CONFIG_S_DRIVER_OK;
+ if (unlikely(vdev->start_on_kick && vdev->started)) {
+ vdev->start_on_kick = false;
+ }
}
if (k->set_status) {
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 5/5] virtio: add "use-started" property
2019-05-29 7:09 [Qemu-devel] [PATCH 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
` (3 preceding siblings ...)
2019-05-29 7:09 ` [Qemu-devel] [PATCH 4/5] virtio: Don't change "started" flag on virtio_vmstate_change() elohimes
@ 2019-05-29 7:09 ` elohimes
2019-06-03 20:49 ` Greg Kurz
4 siblings, 1 reply; 15+ messages in thread
From: elohimes @ 2019-05-29 7:09 UTC (permalink / raw)
To: mst, groug, dgilbert; +Cc: Xie Yongji, qemu-devel
From: Xie Yongji <xieyongji@baidu.com>
In order to avoid migration issues, we introduce a "use-started"
property to the base virtio device to indicate whether "started"
and "start_on_kick" flag could be used. This property will be
true by default and set to false when machine type <= 4.0.
Suggested-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Xie Yongji <xieyongji@baidu.com>
---
hw/block/vhost-user-blk.c | 8 ++++++--
hw/core/machine.c | 4 +++-
hw/virtio/virtio.c | 7 ++++---
include/hw/virtio/virtio.h | 10 ++++++++++
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9cb61336a6..520c5d3d4b 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -191,9 +191,13 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status)
{
VHostUserBlk *s = VHOST_USER_BLK(vdev);
- bool should_start = vdev->started;
+ bool should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;
int ret;
+ if (virtio_device_started(vdev)) {
+ should_start = true;
+ }
+
if (!vdev->vm_running) {
should_start = false;
}
@@ -317,7 +321,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
}
/* restore vhost state */
- if (vdev->started) {
+ if (virtio_device_started(vdev)) {
ret = vhost_user_blk_start(vdev);
if (ret < 0) {
error_report("vhost-user-blk: vhost start failed: %s",
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 934c1bcceb..1730d28c1b 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -24,7 +24,9 @@
#include "hw/pci/pci.h"
#include "hw/mem/nvdimm.h"
-GlobalProperty hw_compat_4_0[] = {};
+GlobalProperty hw_compat_4_0[] = {
+ { "virtio-device", "use-started", "false" },
+};
const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
GlobalProperty hw_compat_3_1[] = {
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 9e17293d46..4c05a9efe3 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1803,14 +1803,14 @@ static bool virtio_started_needed(void *opaque)
{
VirtIODevice *vdev = opaque;
- return vdev->started;
+ return vdev->started && vdev->use_started;
}
static bool virtio_start_on_kick_needed(void *opaque)
{
VirtIODevice *vdev = opaque;
- return vdev->start_on_kick;
+ return vdev->start_on_kick && vdev->use_started;
}
static const VMStateDescription vmstate_virtqueue = {
@@ -2320,7 +2320,7 @@ static void virtio_vmstate_change(void *opaque, int running, RunState state)
VirtIODevice *vdev = opaque;
BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
- bool backend_run = running && vdev->started;
+ bool backend_run = running && virtio_device_started(vdev);
vdev->vm_running = running;
if (backend_run) {
@@ -2698,6 +2698,7 @@ static void virtio_device_instance_finalize(Object *obj)
static Property virtio_properties[] = {
DEFINE_VIRTIO_COMMON_FEATURES(VirtIODevice, host_features),
+ DEFINE_PROP_BOOL("use-started", VirtIODevice, use_started, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 27c0efc3d0..ba4dbd7480 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -105,6 +105,7 @@ struct VirtIODevice
uint16_t device_id;
bool vm_running;
bool broken; /* device in invalid state, needs reset */
+ bool use_started;
bool started;
bool start_on_kick; /* virtio 1.0 transitional devices support that */
VMChangeStateEntry *vmstate;
@@ -351,4 +352,13 @@ static inline bool virtio_is_big_endian(VirtIODevice *vdev)
/* Devices conforming to VIRTIO 1.0 or later are always LE. */
return false;
}
+
+static inline bool virtio_device_started(VirtIODevice *vdev)
+{
+ if (vdev->use_started) {
+ return vdev->started;
+ }
+
+ return vdev->status & VIRTIO_CONFIG_S_DRIVER_OK;
+}
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] virtio: Set "start_on_kick" on virtio_set_features()
2019-05-29 7:09 ` [Qemu-devel] [PATCH 1/5] virtio: Set "start_on_kick" on virtio_set_features() elohimes
@ 2019-06-03 16:53 ` Greg Kurz
2019-06-04 1:41 ` Yongji Xie
0 siblings, 1 reply; 15+ messages in thread
From: Greg Kurz @ 2019-06-03 16:53 UTC (permalink / raw)
To: elohimes; +Cc: qemu-devel, Xie Yongji, dgilbert, mst
On Wed, 29 May 2019 15:09:51 +0800
elohimes@gmail.com wrote:
> From: Xie Yongji <xieyongji@baidu.com>
>
> The guest feature is not set correctly on virtio_reset() and
> virtio_init(). So we should not use it to set "start_on_kick" at that
> point. This patch set "start_on_kick" on virtio_set_features() instead.
>
> Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> ---
Maybe add a Fixes: badaf79cfdbd3 ?
> hw/virtio/virtio.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 4805727b53..fc8fca81ad 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1214,8 +1214,7 @@ void virtio_reset(void *opaque)
> k->reset(vdev);
> }
>
> - vdev->start_on_kick = (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> - !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1));
> + vdev->start_on_kick = false;
> vdev->started = false;
> vdev->broken = false;
> vdev->guest_features = 0;
> @@ -2069,14 +2068,21 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
> return -EINVAL;
> }
> ret = virtio_set_features_nocheck(vdev, val);
> - if (!ret && virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
> - /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
> - int i;
> - for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> - if (vdev->vq[i].vring.num != 0) {
> - virtio_init_region_cache(vdev, i);
> + if (!ret) {
> + if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
> + /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
> + int i;
> + for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> + if (vdev->vq[i].vring.num != 0) {
> + virtio_init_region_cache(vdev, i);
> + }
> }
> }
> +
> + if (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> + !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> + vdev->start_on_kick = true;
> + }
> }
> return ret;
> }
> @@ -2331,8 +2337,7 @@ void virtio_init(VirtIODevice *vdev, const char *name,
> g_malloc0(sizeof(*vdev->vector_queues) * nvectors);
> }
>
> - vdev->start_on_kick = (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> - !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1));
> + vdev->start_on_kick = false;
virtio_init() is called at realize on an object that was just allocated with
g_malloc0(). You shouldn't need to set anything to 0 or false... or I'm
missing something ?
Anyway, I guess this doesn't hurt, so:
Reviewed-by: Greg Kurz <groug@kaod.org>
> vdev->started = false;
> vdev->device_id = device_id;
> vdev->status = 0;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] virtio: Migrate the "start_on_kick" flag
2019-05-29 7:09 ` [Qemu-devel] [PATCH 2/5] virtio: Migrate the "start_on_kick" flag elohimes
@ 2019-06-03 20:16 ` Greg Kurz
2019-06-03 21:03 ` Michael S. Tsirkin
2019-06-04 2:15 ` Yongji Xie
0 siblings, 2 replies; 15+ messages in thread
From: Greg Kurz @ 2019-06-03 20:16 UTC (permalink / raw)
To: elohimes; +Cc: qemu-devel, Xie Yongji, dgilbert, mst
On Wed, 29 May 2019 15:09:52 +0800
elohimes@gmail.com wrote:
> From: Xie Yongji <xieyongji@baidu.com>
>
> We should migrate the "start_on_kick" flag so that we
> would not miss starting device on kicking at startup
> after migration.
>
Hmm... IIUC "start_on_kick" means "virtio 1.0 transitional device that has
not been started yet", ie:
!vdev->started &&
(virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1))
If so, not sure why you need this extra field in the first place, but
you probably don't need to migrate it. Just recalculate in a post load
callback.
> Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> ---
> hw/virtio/virtio.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index fc8fca81ad..4d4ff67791 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1802,6 +1802,13 @@ static bool virtio_started_needed(void *opaque)
> return vdev->started;
> }
>
> +static bool virtio_start_on_kick_needed(void *opaque)
> +{
> + VirtIODevice *vdev = opaque;
> +
> + return vdev->start_on_kick;
> +}
> +
> static const VMStateDescription vmstate_virtqueue = {
> .name = "virtqueue_state",
> .version_id = 1,
> @@ -1941,6 +1948,17 @@ static const VMStateDescription vmstate_virtio_started = {
> }
> };
>
> +static const VMStateDescription vmstate_virtio_start_on_kick = {
> + .name = "virtio/start_on_kick",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = &virtio_start_on_kick_needed,
> + .fields = (VMStateField[]) {
> + VMSTATE_BOOL(start_on_kick, VirtIODevice),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> static const VMStateDescription vmstate_virtio = {
> .name = "virtio",
> .version_id = 1,
> @@ -1957,6 +1975,7 @@ static const VMStateDescription vmstate_virtio = {
> &vmstate_virtio_broken,
> &vmstate_virtio_extra_state,
> &vmstate_virtio_started,
> + &vmstate_virtio_start_on_kick,
> NULL
> }
> };
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 5/5] virtio: add "use-started" property
2019-05-29 7:09 ` [Qemu-devel] [PATCH 5/5] virtio: add "use-started" property elohimes
@ 2019-06-03 20:49 ` Greg Kurz
2019-06-04 2:37 ` Yongji Xie
0 siblings, 1 reply; 15+ messages in thread
From: Greg Kurz @ 2019-06-03 20:49 UTC (permalink / raw)
To: elohimes; +Cc: qemu-devel, Xie Yongji, dgilbert, mst
On Wed, 29 May 2019 15:09:55 +0800
elohimes@gmail.com wrote:
> From: Xie Yongji <xieyongji@baidu.com>
>
> In order to avoid migration issues, we introduce a "use-started"
> property to the base virtio device to indicate whether "started"
> and "start_on_kick" flag could be used. This property will be
> true by default and set to false when machine type <= 4.0.
>
> Suggested-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> ---
> hw/block/vhost-user-blk.c | 8 ++++++--
> hw/core/machine.c | 4 +++-
> hw/virtio/virtio.c | 7 ++++---
> include/hw/virtio/virtio.h | 10 ++++++++++
> 4 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 9cb61336a6..520c5d3d4b 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -191,9 +191,13 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
> static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status)
> {
> VHostUserBlk *s = VHOST_USER_BLK(vdev);
> - bool should_start = vdev->started;
> + bool should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;
I guess you need the above because the set_status callback is called
before setting vdev->status, and virtio_device_started() thus doesn't
return the expected result for older machine types ?
What about adding a status argument to virtio_device_started() ?
bool should_start = virtio_device_started(vdev, status);
> int ret;
>
> + if (virtio_device_started(vdev)) {
> + should_start = true;
> + }
> +
> if (!vdev->vm_running) {
> should_start = false;
> }
> @@ -317,7 +321,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
> }
>
> /* restore vhost state */
> - if (vdev->started) {
> + if (virtio_device_started(vdev)) {
if (virtio_device_started(vdev, vdev->status)) {
and so on...
> ret = vhost_user_blk_start(vdev);
> if (ret < 0) {
> error_report("vhost-user-blk: vhost start failed: %s",
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 934c1bcceb..1730d28c1b 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -24,7 +24,9 @@
> #include "hw/pci/pci.h"
> #include "hw/mem/nvdimm.h"
>
> -GlobalProperty hw_compat_4_0[] = {};
> +GlobalProperty hw_compat_4_0[] = {
> + { "virtio-device", "use-started", "false" },
> +};
> const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
>
> GlobalProperty hw_compat_3_1[] = {
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 9e17293d46..4c05a9efe3 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1803,14 +1803,14 @@ static bool virtio_started_needed(void *opaque)
> {
> VirtIODevice *vdev = opaque;
>
> - return vdev->started;
> + return vdev->started && vdev->use_started;
Hmm... the idea is that vdev->started should never be set when
"use-started" is "off". Instead of checking vdev->use_started
here, you should rather assign it to vdev->started everywhere
you currently assign true.
> }
>
> static bool virtio_start_on_kick_needed(void *opaque)
> {
> VirtIODevice *vdev = opaque;
>
> - return vdev->start_on_kick;
> + return vdev->start_on_kick && vdev->use_started;
> }
>
> static const VMStateDescription vmstate_virtqueue = {
> @@ -2320,7 +2320,7 @@ static void virtio_vmstate_change(void *opaque, int running, RunState state)
> VirtIODevice *vdev = opaque;
> BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> - bool backend_run = running && vdev->started;
> + bool backend_run = running && virtio_device_started(vdev);
> vdev->vm_running = running;
>
> if (backend_run) {
> @@ -2698,6 +2698,7 @@ static void virtio_device_instance_finalize(Object *obj)
>
> static Property virtio_properties[] = {
> DEFINE_VIRTIO_COMMON_FEATURES(VirtIODevice, host_features),
> + DEFINE_PROP_BOOL("use-started", VirtIODevice, use_started, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 27c0efc3d0..ba4dbd7480 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -105,6 +105,7 @@ struct VirtIODevice
> uint16_t device_id;
> bool vm_running;
> bool broken; /* device in invalid state, needs reset */
> + bool use_started;
> bool started;
> bool start_on_kick; /* virtio 1.0 transitional devices support that */
> VMChangeStateEntry *vmstate;
> @@ -351,4 +352,13 @@ static inline bool virtio_is_big_endian(VirtIODevice *vdev)
> /* Devices conforming to VIRTIO 1.0 or later are always LE. */
> return false;
> }
> +
> +static inline bool virtio_device_started(VirtIODevice *vdev)
> +{
> + if (vdev->use_started) {
> + return vdev->started;
> + }
> +
> + return vdev->status & VIRTIO_CONFIG_S_DRIVER_OK;
> +}
> #endif
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] virtio: Migrate the "start_on_kick" flag
2019-06-03 20:16 ` Greg Kurz
@ 2019-06-03 21:03 ` Michael S. Tsirkin
2019-06-03 21:25 ` Greg Kurz
2019-06-04 2:15 ` Yongji Xie
1 sibling, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2019-06-03 21:03 UTC (permalink / raw)
To: Greg Kurz; +Cc: elohimes, Xie Yongji, dgilbert, qemu-devel
On Mon, Jun 03, 2019 at 10:16:39PM +0200, Greg Kurz wrote:
> On Wed, 29 May 2019 15:09:52 +0800
> elohimes@gmail.com wrote:
>
> > From: Xie Yongji <xieyongji@baidu.com>
> >
> > We should migrate the "start_on_kick" flag so that we
> > would not miss starting device on kicking at startup
> > after migration.
> >
>
> Hmm... IIUC "start_on_kick" means "virtio 1.0 transitional device that has
> not been started yet", ie:
>
> !vdev->started &&
> (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1))
Or a legacy device I guess?
> If so, not sure why you need this extra field in the first place, but
> you probably don't need to migrate it. Just recalculate in a post load
> callback.
>
> > Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> > ---
> > hw/virtio/virtio.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index fc8fca81ad..4d4ff67791 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -1802,6 +1802,13 @@ static bool virtio_started_needed(void *opaque)
> > return vdev->started;
> > }
> >
> > +static bool virtio_start_on_kick_needed(void *opaque)
> > +{
> > + VirtIODevice *vdev = opaque;
> > +
> > + return vdev->start_on_kick;
> > +}
> > +
> > static const VMStateDescription vmstate_virtqueue = {
> > .name = "virtqueue_state",
> > .version_id = 1,
> > @@ -1941,6 +1948,17 @@ static const VMStateDescription vmstate_virtio_started = {
> > }
> > };
> >
> > +static const VMStateDescription vmstate_virtio_start_on_kick = {
> > + .name = "virtio/start_on_kick",
> > + .version_id = 1,
> > + .minimum_version_id = 1,
> > + .needed = &virtio_start_on_kick_needed,
> > + .fields = (VMStateField[]) {
> > + VMSTATE_BOOL(start_on_kick, VirtIODevice),
> > + VMSTATE_END_OF_LIST()
> > + }
> > +};
> > +
> > static const VMStateDescription vmstate_virtio = {
> > .name = "virtio",
> > .version_id = 1,
> > @@ -1957,6 +1975,7 @@ static const VMStateDescription vmstate_virtio = {
> > &vmstate_virtio_broken,
> > &vmstate_virtio_extra_state,
> > &vmstate_virtio_started,
> > + &vmstate_virtio_start_on_kick,
> > NULL
> > }
> > };
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] virtio: Migrate the "start_on_kick" flag
2019-06-03 21:03 ` Michael S. Tsirkin
@ 2019-06-03 21:25 ` Greg Kurz
2019-06-03 21:51 ` Michael S. Tsirkin
0 siblings, 1 reply; 15+ messages in thread
From: Greg Kurz @ 2019-06-03 21:25 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: elohimes, Xie Yongji, dgilbert, qemu-devel
On Mon, 3 Jun 2019 17:03:06 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Mon, Jun 03, 2019 at 10:16:39PM +0200, Greg Kurz wrote:
> > On Wed, 29 May 2019 15:09:52 +0800
> > elohimes@gmail.com wrote:
> >
> > > From: Xie Yongji <xieyongji@baidu.com>
> > >
> > > We should migrate the "start_on_kick" flag so that we
> > > would not miss starting device on kicking at startup
> > > after migration.
> > >
> >
> > Hmm... IIUC "start_on_kick" means "virtio 1.0 transitional device that has
> > not been started yet", ie:
> >
> > !vdev->started &&
> > (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> > !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1))
>
> Or a legacy device I guess?
>
Do you mean "start_on_kick" should be set for both legacy
and virtio 1.0 transitional devices ?
> > If so, not sure why you need this extra field in the first place, but
> > you probably don't need to migrate it. Just recalculate in a post load
> > callback.
> >
> > > Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> > > ---
> > > hw/virtio/virtio.c | 19 +++++++++++++++++++
> > > 1 file changed, 19 insertions(+)
> > >
> > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > index fc8fca81ad..4d4ff67791 100644
> > > --- a/hw/virtio/virtio.c
> > > +++ b/hw/virtio/virtio.c
> > > @@ -1802,6 +1802,13 @@ static bool virtio_started_needed(void *opaque)
> > > return vdev->started;
> > > }
> > >
> > > +static bool virtio_start_on_kick_needed(void *opaque)
> > > +{
> > > + VirtIODevice *vdev = opaque;
> > > +
> > > + return vdev->start_on_kick;
> > > +}
> > > +
> > > static const VMStateDescription vmstate_virtqueue = {
> > > .name = "virtqueue_state",
> > > .version_id = 1,
> > > @@ -1941,6 +1948,17 @@ static const VMStateDescription vmstate_virtio_started = {
> > > }
> > > };
> > >
> > > +static const VMStateDescription vmstate_virtio_start_on_kick = {
> > > + .name = "virtio/start_on_kick",
> > > + .version_id = 1,
> > > + .minimum_version_id = 1,
> > > + .needed = &virtio_start_on_kick_needed,
> > > + .fields = (VMStateField[]) {
> > > + VMSTATE_BOOL(start_on_kick, VirtIODevice),
> > > + VMSTATE_END_OF_LIST()
> > > + }
> > > +};
> > > +
> > > static const VMStateDescription vmstate_virtio = {
> > > .name = "virtio",
> > > .version_id = 1,
> > > @@ -1957,6 +1975,7 @@ static const VMStateDescription vmstate_virtio = {
> > > &vmstate_virtio_broken,
> > > &vmstate_virtio_extra_state,
> > > &vmstate_virtio_started,
> > > + &vmstate_virtio_start_on_kick,
> > > NULL
> > > }
> > > };
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] virtio: Migrate the "start_on_kick" flag
2019-06-03 21:25 ` Greg Kurz
@ 2019-06-03 21:51 ` Michael S. Tsirkin
0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2019-06-03 21:51 UTC (permalink / raw)
To: Greg Kurz; +Cc: elohimes, Xie Yongji, dgilbert, qemu-devel
On Mon, Jun 03, 2019 at 11:25:23PM +0200, Greg Kurz wrote:
> On Mon, 3 Jun 2019 17:03:06 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Mon, Jun 03, 2019 at 10:16:39PM +0200, Greg Kurz wrote:
> > > On Wed, 29 May 2019 15:09:52 +0800
> > > elohimes@gmail.com wrote:
> > >
> > > > From: Xie Yongji <xieyongji@baidu.com>
> > > >
> > > > We should migrate the "start_on_kick" flag so that we
> > > > would not miss starting device on kicking at startup
> > > > after migration.
> > > >
> > >
> > > Hmm... IIUC "start_on_kick" means "virtio 1.0 transitional device that has
> > > not been started yet", ie:
> > >
> > > !vdev->started &&
> > > (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> > > !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1))
> >
> > Or a legacy device I guess?
> >
>
> Do you mean "start_on_kick" should be set for both legacy
> and virtio 1.0 transitional devices ?
Yes - generally when virtio 1 feature has not
been negotiated.
> > > If so, not sure why you need this extra field in the first place, but
> > > you probably don't need to migrate it. Just recalculate in a post load
> > > callback.
> > >
> > > > Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> > > > ---
> > > > hw/virtio/virtio.c | 19 +++++++++++++++++++
> > > > 1 file changed, 19 insertions(+)
> > > >
> > > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > > index fc8fca81ad..4d4ff67791 100644
> > > > --- a/hw/virtio/virtio.c
> > > > +++ b/hw/virtio/virtio.c
> > > > @@ -1802,6 +1802,13 @@ static bool virtio_started_needed(void *opaque)
> > > > return vdev->started;
> > > > }
> > > >
> > > > +static bool virtio_start_on_kick_needed(void *opaque)
> > > > +{
> > > > + VirtIODevice *vdev = opaque;
> > > > +
> > > > + return vdev->start_on_kick;
> > > > +}
> > > > +
> > > > static const VMStateDescription vmstate_virtqueue = {
> > > > .name = "virtqueue_state",
> > > > .version_id = 1,
> > > > @@ -1941,6 +1948,17 @@ static const VMStateDescription vmstate_virtio_started = {
> > > > }
> > > > };
> > > >
> > > > +static const VMStateDescription vmstate_virtio_start_on_kick = {
> > > > + .name = "virtio/start_on_kick",
> > > > + .version_id = 1,
> > > > + .minimum_version_id = 1,
> > > > + .needed = &virtio_start_on_kick_needed,
> > > > + .fields = (VMStateField[]) {
> > > > + VMSTATE_BOOL(start_on_kick, VirtIODevice),
> > > > + VMSTATE_END_OF_LIST()
> > > > + }
> > > > +};
> > > > +
> > > > static const VMStateDescription vmstate_virtio = {
> > > > .name = "virtio",
> > > > .version_id = 1,
> > > > @@ -1957,6 +1975,7 @@ static const VMStateDescription vmstate_virtio = {
> > > > &vmstate_virtio_broken,
> > > > &vmstate_virtio_extra_state,
> > > > &vmstate_virtio_started,
> > > > + &vmstate_virtio_start_on_kick,
> > > > NULL
> > > > }
> > > > };
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] virtio: Set "start_on_kick" on virtio_set_features()
2019-06-03 16:53 ` Greg Kurz
@ 2019-06-04 1:41 ` Yongji Xie
0 siblings, 0 replies; 15+ messages in thread
From: Yongji Xie @ 2019-06-04 1:41 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, Xie Yongji, Dr. David Alan Gilbert, Michael S. Tsirkin
On Tue, 4 Jun 2019 at 00:53, Greg Kurz <groug@kaod.org> wrote:
>
> On Wed, 29 May 2019 15:09:51 +0800
> elohimes@gmail.com wrote:
>
> > From: Xie Yongji <xieyongji@baidu.com>
> >
> > The guest feature is not set correctly on virtio_reset() and
> > virtio_init(). So we should not use it to set "start_on_kick" at that
> > point. This patch set "start_on_kick" on virtio_set_features() instead.
> >
> > Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> > ---
>
> Maybe add a Fixes: badaf79cfdbd3 ?
>
Will add it in v2. Thanks for review.
Thanks,
Yongji
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] virtio: Migrate the "start_on_kick" flag
2019-06-03 20:16 ` Greg Kurz
2019-06-03 21:03 ` Michael S. Tsirkin
@ 2019-06-04 2:15 ` Yongji Xie
1 sibling, 0 replies; 15+ messages in thread
From: Yongji Xie @ 2019-06-04 2:15 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, Xie Yongji, Dr. David Alan Gilbert, Michael S. Tsirkin
On Tue, 4 Jun 2019 at 04:16, Greg Kurz <groug@kaod.org> wrote:
>
> On Wed, 29 May 2019 15:09:52 +0800
> elohimes@gmail.com wrote:
>
> > From: Xie Yongji <xieyongji@baidu.com>
> >
> > We should migrate the "start_on_kick" flag so that we
> > would not miss starting device on kicking at startup
> > after migration.
> >
>
> Hmm... IIUC "start_on_kick" means "virtio 1.0 transitional device that has
> not been started yet", ie:
>
> !vdev->started &&
> (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1))
>
> If so, not sure why you need this extra field in the first place, but
> you probably don't need to migrate it. Just recalculate in a post load
> callback.
>
Good idea! Will recalculate this in virtio_load() in v2. Thank you.
Thanks,
Yongji
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 5/5] virtio: add "use-started" property
2019-06-03 20:49 ` Greg Kurz
@ 2019-06-04 2:37 ` Yongji Xie
0 siblings, 0 replies; 15+ messages in thread
From: Yongji Xie @ 2019-06-04 2:37 UTC (permalink / raw)
To: Greg Kurz
Cc: qemu-devel, Xie Yongji, Dr. David Alan Gilbert, Michael S. Tsirkin
On Tue, 4 Jun 2019 at 04:49, Greg Kurz <groug@kaod.org> wrote:
>
> On Wed, 29 May 2019 15:09:55 +0800
> elohimes@gmail.com wrote:
>
> > From: Xie Yongji <xieyongji@baidu.com>
> >
> > In order to avoid migration issues, we introduce a "use-started"
> > property to the base virtio device to indicate whether "started"
> > and "start_on_kick" flag could be used. This property will be
> > true by default and set to false when machine type <= 4.0.
> >
> > Suggested-by: Greg Kurz <groug@kaod.org>
> > Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> > ---
> > hw/block/vhost-user-blk.c | 8 ++++++--
> > hw/core/machine.c | 4 +++-
> > hw/virtio/virtio.c | 7 ++++---
> > include/hw/virtio/virtio.h | 10 ++++++++++
> > 4 files changed, 23 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > index 9cb61336a6..520c5d3d4b 100644
> > --- a/hw/block/vhost-user-blk.c
> > +++ b/hw/block/vhost-user-blk.c
> > @@ -191,9 +191,13 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
> > static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status)
> > {
> > VHostUserBlk *s = VHOST_USER_BLK(vdev);
> > - bool should_start = vdev->started;
> > + bool should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;
>
> I guess you need the above because the set_status callback is called
> before setting vdev->status, and virtio_device_started() thus doesn't
> return the expected result for older machine types ?
>
Yes, that's case.
> What about adding a status argument to virtio_device_started() ?
>
> bool should_start = virtio_device_started(vdev, status);
>
Looks fine to me.
> > int ret;
> >
> > + if (virtio_device_started(vdev)) {
> > + should_start = true;
> > + }
> > +
> > if (!vdev->vm_running) {
> > should_start = false;
> > }
> > @@ -317,7 +321,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
> > }
> >
> > /* restore vhost state */
> > - if (vdev->started) {
> > + if (virtio_device_started(vdev)) {
>
> if (virtio_device_started(vdev, vdev->status)) {
>
> and so on...
>
> > ret = vhost_user_blk_start(vdev);
> > if (ret < 0) {
> > error_report("vhost-user-blk: vhost start failed: %s",
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 934c1bcceb..1730d28c1b 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -24,7 +24,9 @@
> > #include "hw/pci/pci.h"
> > #include "hw/mem/nvdimm.h"
> >
> > -GlobalProperty hw_compat_4_0[] = {};
> > +GlobalProperty hw_compat_4_0[] = {
> > + { "virtio-device", "use-started", "false" },
> > +};
> > const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
> >
> > GlobalProperty hw_compat_3_1[] = {
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 9e17293d46..4c05a9efe3 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -1803,14 +1803,14 @@ static bool virtio_started_needed(void *opaque)
> > {
> > VirtIODevice *vdev = opaque;
> >
> > - return vdev->started;
> > + return vdev->started && vdev->use_started;
>
> Hmm... the idea is that vdev->started should never be set when
> "use-started" is "off". Instead of checking vdev->use_started
> here, you should rather assign it to vdev->started everywhere
> you currently assign true.
>
Will do it in v2.
Thanks,
Yongji
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-06-04 2:38 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29 7:09 [Qemu-devel] [PATCH 0/5] virtio: fix some issues of "started" and "start_on_kick" flag elohimes
2019-05-29 7:09 ` [Qemu-devel] [PATCH 1/5] virtio: Set "start_on_kick" on virtio_set_features() elohimes
2019-06-03 16:53 ` Greg Kurz
2019-06-04 1:41 ` Yongji Xie
2019-05-29 7:09 ` [Qemu-devel] [PATCH 2/5] virtio: Migrate the "start_on_kick" flag elohimes
2019-06-03 20:16 ` Greg Kurz
2019-06-03 21:03 ` Michael S. Tsirkin
2019-06-03 21:25 ` Greg Kurz
2019-06-03 21:51 ` Michael S. Tsirkin
2019-06-04 2:15 ` Yongji Xie
2019-05-29 7:09 ` [Qemu-devel] [PATCH 3/5] virtio: Make sure we get correct state of device on handle_aio_output() elohimes
2019-05-29 7:09 ` [Qemu-devel] [PATCH 4/5] virtio: Don't change "started" flag on virtio_vmstate_change() elohimes
2019-05-29 7:09 ` [Qemu-devel] [PATCH 5/5] virtio: add "use-started" property elohimes
2019-06-03 20:49 ` Greg Kurz
2019-06-04 2:37 ` Yongji Xie
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.