[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 5/5] virtio: add "use-started" property
From: |
Yongji Xie |
Subject: |
Re: [Qemu-devel] [PATCH v2 5/5] virtio: add "use-started" property |
Date: |
Wed, 5 Jun 2019 17:14:23 +0800 |
On Wed, 5 Jun 2019 at 17:00, Greg Kurz <address@hidden> wrote:
>
> On Tue, 4 Jun 2019 15:34:59 +0800
> address@hidden wrote:
>
> > From: Xie Yongji <address@hidden>
> >
> > 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.1.
> >
> > Signed-off-by: Xie Yongji <address@hidden>
> > ---
> > hw/block/vhost-user-blk.c | 4 ++--
> > hw/core/machine.c | 4 +++-
> > hw/virtio/virtio.c | 21 ++++++++-------------
> > include/hw/virtio/virtio.h | 21 +++++++++++++++++++++
> > 4 files changed, 34 insertions(+), 16 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 f1a0f45f9c..133c113ebf 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_1[] = {};
> > +GlobalProperty hw_compat_4_0_1[] = {
> > + { "virtio-device", "use-started", "false" },
> > +};
> > const size_t hw_compat_4_0_1_len = G_N_ELEMENTS(hw_compat_4_0_1);
>
> I'm discovering hw_compat_4_0_1, which seems to be only used by the
> pc-q35-4.0.1 machine type...
>
Oops, my mistake.
> >
> > GlobalProperty hw_compat_4_0[] = {};
>
> Not sure if it's the way to go but the same line should at least be added
> here for all other machine types that use hw_compat_4_0[] eg. pseries-4.0
> and older, which are the ones I need this fix for.
>
I agree.
> Cc'ing core machine code maintainers for advice.
>
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 3960619bd4..9af2e339af 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -1165,10 +1165,7 @@ int virtio_set_status(VirtIODevice *vdev, uint8_t
> > val)
> >
> > 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;
> > - }
> > + virtio_set_started(vdev, val & VIRTIO_CONFIG_S_DRIVER_OK);
>
> virtio_set_started() takes a bool as second argument, so this should
> rather be !!(val & VIRTIO_CONFIG_S_DRIVER_OK) to avoid potential
> warnings from picky compilers.
>
Will fix it in v3.
> The rest looks good, but I'm wondering if this patch should be the first
> one in the series to narrow the range of commits where backward migration
> is broken.
>
It's OK to me.
Thanks,
Yongji
- [Qemu-devel] [PATCH v2 1/5] virtio: Set "start_on_kick" on virtio_set_features(), (continued)
[Qemu-devel] [PATCH v2 3/5] virtio: Make sure we get correct state of device on handle_aio_output(), elohimes, 2019/06/04
[Qemu-devel] [PATCH v2 4/5] virtio: Don't change "started" flag on virtio_vmstate_change(), elohimes, 2019/06/04
[Qemu-devel] [PATCH v2 5/5] virtio: add "use-started" property, elohimes, 2019/06/04