Re: [Qemu-devel] [PATCH v4 1/5] virtio: add "use-started" property

2019-06-26 Thread Yongji Xie
On Wed, 26 Jun 2019 at 18:18, Greg Kurz  wrote:
>
> On Wed, 26 Jun 2019 10:31:26 +0800
> elohi...@gmail.com wrote:
>
> > From: Xie Yongji 
> >
> > In order to avoid migration issues, we introduce a "use-started"
> > property to the base virtio device to indicate whether use
> > "started" flag or not. This property will be true by default and
> > set to false when machine type <= 4.0.
> >
> > Suggested-by: Greg Kurz 
> > Signed-off-by: Xie Yongji 
> > ---
>
> LGTM,
>
> Reviewed-by: Greg Kurz 
>
> This fixes the backward migration breakage I was observing.
>
> Tested-by: Greg Kurz 
>

Thanks for the testing.



Re: [Qemu-devel] [PATCH v4 1/5] virtio: add "use-started" property

2019-06-26 Thread Greg Kurz
On Wed, 26 Jun 2019 10:31:26 +0800
elohi...@gmail.com wrote:

> From: Xie Yongji 
> 
> In order to avoid migration issues, we introduce a "use-started"
> property to the base virtio device to indicate whether use
> "started" flag or not. This property will be true by default and
> set to false when machine type <= 4.0.
> 
> Suggested-by: Greg Kurz 
> Signed-off-by: Xie Yongji 
> ---

LGTM,

Reviewed-by: Greg Kurz 

This fixes the backward migration breakage I was observing.

Tested-by: Greg Kurz 

>  hw/block/vhost-user-blk.c  |  4 ++--
>  hw/core/machine.c  |  1 +
>  hw/virtio/virtio.c | 18 +++---
>  include/hw/virtio/virtio.h | 21 +
>  4 files changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 9cb61336a6..85bc4017e7 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -191,7 +191,7 @@ 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 = virtio_device_started(vdev, status);
>  int ret;
>  
>  if (!vdev->vm_running) {
> @@ -317,7 +317,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
>  }
>  
>  /* restore vhost state */
> -if (vdev->started) {
> +if (virtio_device_started(vdev, vdev->status)) {
>  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 ea5a01aa49..ea84bd6788 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -30,6 +30,7 @@ GlobalProperty hw_compat_4_0[] = {
>  { "bochs-display",  "edid", "false" },
>  { "virtio-vga", "edid", "false" },
>  { "virtio-gpu-pci", "edid", "false" },
> +{ "virtio-device", "use-started", "false" },
>  };
>  const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
>  
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index e1e90fcfd6..c9a6ca04b8 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1162,10 +1162,8 @@ 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;
> -}
> +
> +virtio_set_started(vdev, val & VIRTIO_CONFIG_S_DRIVER_OK);
>  
>  if (k->set_status) {
>  k->set_status(vdev, val);
> @@ -1536,8 +1534,7 @@ static bool virtio_queue_notify_aio_vq(VirtQueue *vq)
>  ret = vq->handle_aio_output(vdev, vq);
>  
>  if (unlikely(vdev->start_on_kick)) {
> -vdev->started = true;
> -vdev->start_on_kick = false;
> +virtio_set_started(vdev, true);
>  }
>  }
>  
> @@ -1557,8 +1554,7 @@ static void virtio_queue_notify_vq(VirtQueue *vq)
>  vq->handle_output(vdev, vq);
>  
>  if (unlikely(vdev->start_on_kick)) {
> -vdev->started = true;
> -vdev->start_on_kick = false;
> +virtio_set_started(vdev, true);
>  }
>  }
>  }
> @@ -1579,8 +1575,7 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
>  }
>  
>  if (unlikely(vdev->start_on_kick)) {
> -vdev->started = true;
> -vdev->start_on_kick = false;
> +virtio_set_started(vdev, true);
>  }
>  }
>  
> @@ -2291,7 +2286,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->status);
>  vdev->vm_running = running;
>  
>  if (backend_run) {
> @@ -2669,6 +2664,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..15d5366939 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,24 @@ static inline bool virtio_is_big_endian(VirtIODevice 
> *vdev)
>  /* Devices conforming to VIRTIO 1.0 or later are always LE. */

[Qemu-devel] [PATCH v4 1/5] virtio: add "use-started" property

2019-06-25 Thread elohimes
From: Xie Yongji 

In order to avoid migration issues, we introduce a "use-started"
property to the base virtio device to indicate whether use
"started" flag or not. This property will be true by default and
set to false when machine type <= 4.0.

Suggested-by: Greg Kurz 
Signed-off-by: Xie Yongji 
---
 hw/block/vhost-user-blk.c  |  4 ++--
 hw/core/machine.c  |  1 +
 hw/virtio/virtio.c | 18 +++---
 include/hw/virtio/virtio.h | 21 +
 4 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9cb61336a6..85bc4017e7 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -191,7 +191,7 @@ 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 = virtio_device_started(vdev, status);
 int ret;
 
 if (!vdev->vm_running) {
@@ -317,7 +317,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
 }
 
 /* restore vhost state */
-if (vdev->started) {
+if (virtio_device_started(vdev, vdev->status)) {
 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 ea5a01aa49..ea84bd6788 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -30,6 +30,7 @@ GlobalProperty hw_compat_4_0[] = {
 { "bochs-display",  "edid", "false" },
 { "virtio-vga", "edid", "false" },
 { "virtio-gpu-pci", "edid", "false" },
+{ "virtio-device", "use-started", "false" },
 };
 const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
 
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index e1e90fcfd6..c9a6ca04b8 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1162,10 +1162,8 @@ 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;
-}
+
+virtio_set_started(vdev, val & VIRTIO_CONFIG_S_DRIVER_OK);
 
 if (k->set_status) {
 k->set_status(vdev, val);
@@ -1536,8 +1534,7 @@ static bool virtio_queue_notify_aio_vq(VirtQueue *vq)
 ret = vq->handle_aio_output(vdev, vq);
 
 if (unlikely(vdev->start_on_kick)) {
-vdev->started = true;
-vdev->start_on_kick = false;
+virtio_set_started(vdev, true);
 }
 }
 
@@ -1557,8 +1554,7 @@ static void virtio_queue_notify_vq(VirtQueue *vq)
 vq->handle_output(vdev, vq);
 
 if (unlikely(vdev->start_on_kick)) {
-vdev->started = true;
-vdev->start_on_kick = false;
+virtio_set_started(vdev, true);
 }
 }
 }
@@ -1579,8 +1575,7 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
 }
 
 if (unlikely(vdev->start_on_kick)) {
-vdev->started = true;
-vdev->start_on_kick = false;
+virtio_set_started(vdev, true);
 }
 }
 
@@ -2291,7 +2286,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->status);
 vdev->vm_running = running;
 
 if (backend_run) {
@@ -2669,6 +2664,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..15d5366939 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,24 @@ 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, uint8_t status)
+{
+if (vdev->use_started) {
+return vdev->started;
+}
+
+return status & VIRTIO_CONFIG_S_DRIVER_OK;
+}
+
+static inline void virtio_set_started(VirtIODevice *vdev, bool started)
+{
+if (started) {
+vdev->start_on_kick = false;
+}
+
+if (vdev->use_started) {
+vdev->started = started;
+