mbox series

[v4,for,7.2,00/22] virtio-gpio and various virtio cleanups

Message ID 20220802095010.3330793-1-alex.bennee@linaro.org
Headers show
Series virtio-gpio and various virtio cleanups | expand

Message

Alex Bennée Aug. 2, 2022, 9:49 a.m. UTC
Hi,

This is an update to the previous series which fixes the last few
niggling CI failures I was seeing.

   Subject: [PATCH v3 for 7.2 00/21] virtio-gpio and various virtio cleanups
   Date: Tue, 26 Jul 2022 20:21:29 +0100
   Message-Id: <20220726192150.2435175-1-alex.bennee@linaro.org>

The CI failures were tricky to track down because they didn't occur
locally but after patching to dump backtraces they all seem to involve
updates to virtio_set_status() as the machine was torn down. I think
patch that switches all users to use virtio_device_started() along
with consistent checking of vhost_dev->started stops this from
happening. The clean-up seems worthwhile in reducing boilerplate
anyway.

The following patches still need review:

  - tests/qtest: enable tests for virtio-gpio
  - tests/qtest: add a get_features op to vhost-user-test
  - tests/qtest: implement stub for VHOST_USER_GET_CONFIG
  - tests/qtest: add assert to catch bad features
  - tests/qtest: plain g_assert for VHOST_USER_F_PROTOCOL_FEATURES
  - tests/qtest: catch unhandled vhost-user messages
  - tests/qtest: use qos_printf instead of g_test_message
  - tests/qtest: pass stdout/stderr down to subtests
  - hw/virtio: move vhd->started check into helper and add FIXME
  - hw/virtio: move vm_running check to virtio_device_started
  - hw/virtio: add some vhost-user trace events
  - hw/virtio: log potentially buggy guest drivers
  - hw/virtio: fix some coding style issues
  - include/hw: document vhost_dev feature life-cycle
  - include/hw/virtio: more comment for VIRTIO_F_BAD_FEATURE
  - hw/virtio: fix vhost_user_read tracepoint
  - hw/virtio: handle un-configured shutdown in virtio-pci
  - hw/virtio: gracefully handle unset vhost_dev vdev
  - hw/virtio: incorporate backend features in features


Alex Bennée (20):
  hw/virtio: incorporate backend features in features
  hw/virtio: gracefully handle unset vhost_dev vdev
  hw/virtio: handle un-configured shutdown in virtio-pci
  hw/virtio: fix vhost_user_read tracepoint
  include/hw/virtio: more comment for VIRTIO_F_BAD_FEATURE
  include/hw: document vhost_dev feature life-cycle
  hw/virtio: fix some coding style issues
  hw/virtio: log potentially buggy guest drivers
  hw/virtio: add some vhost-user trace events
  hw/virtio: move vm_running check to virtio_device_started
  hw/virtio: move vhd->started check into helper and add FIXME
  tests/qtest: pass stdout/stderr down to subtests
  tests/qtest: add a timeout for subprocess_run_one_test
  tests/qtest: use qos_printf instead of g_test_message
  tests/qtest: catch unhandled vhost-user messages
  tests/qtest: plain g_assert for VHOST_USER_F_PROTOCOL_FEATURES
  tests/qtest: add assert to catch bad features
  tests/qtest: implement stub for VHOST_USER_GET_CONFIG
  tests/qtest: add a get_features op to vhost-user-test
  tests/qtest: enable tests for virtio-gpio

Viresh Kumar (2):
  hw/virtio: add boilerplate for vhost-user-gpio device
  hw/virtio: add vhost-user-gpio-pci boilerplate

 include/hw/virtio/vhost-user-gpio.h |  35 +++
 include/hw/virtio/vhost.h           |  15 +
 include/hw/virtio/virtio.h          |  12 +-
 tests/qtest/libqos/virtio-gpio.h    |  35 +++
 hw/block/vhost-user-blk.c           |  10 +-
 hw/scsi/vhost-scsi.c                |   4 +-
 hw/scsi/vhost-user-scsi.c           |   2 +-
 hw/virtio/vhost-user-fs.c           |   9 +-
 hw/virtio/vhost-user-gpio-pci.c     |  69 +++++
 hw/virtio/vhost-user-gpio.c         | 411 ++++++++++++++++++++++++++++
 hw/virtio/vhost-user-i2c.c          |  10 +-
 hw/virtio/vhost-user-rng.c          |  10 +-
 hw/virtio/vhost-user-vsock.c        |   8 +-
 hw/virtio/vhost-user.c              |  20 +-
 hw/virtio/vhost-vsock-common.c      |   3 +-
 hw/virtio/vhost-vsock.c             |   8 +-
 hw/virtio/vhost.c                   |  16 +-
 hw/virtio/virtio-pci.c              |   9 +-
 hw/virtio/virtio.c                  |   7 +
 tests/qtest/libqos/virtio-gpio.c    | 171 ++++++++++++
 tests/qtest/libqos/virtio.c         |   4 +-
 tests/qtest/qos-test.c              |   9 +-
 tests/qtest/vhost-user-test.c       | 175 ++++++++++--
 MAINTAINERS                         |   8 +
 hw/virtio/Kconfig                   |   5 +
 hw/virtio/meson.build               |   2 +
 hw/virtio/trace-events              |   9 +
 tests/qtest/libqos/meson.build      |   1 +
 28 files changed, 1007 insertions(+), 70 deletions(-)
 create mode 100644 include/hw/virtio/vhost-user-gpio.h
 create mode 100644 tests/qtest/libqos/virtio-gpio.h
 create mode 100644 hw/virtio/vhost-user-gpio-pci.c
 create mode 100644 hw/virtio/vhost-user-gpio.c
 create mode 100644 tests/qtest/libqos/virtio-gpio.c

Comments

Alex Bennée Sept. 16, 2022, 6:51 a.m. UTC | #1
Alex Bennée <alex.bennee@linaro.org> writes:

> Hi,
>
> This is an update to the previous series which fixes the last few
> niggling CI failures I was seeing.
>
>    Subject: [PATCH v3 for 7.2 00/21] virtio-gpio and various virtio cleanups
>    Date: Tue, 26 Jul 2022 20:21:29 +0100
>    Message-Id: <20220726192150.2435175-1-alex.bennee@linaro.org>
>
> The CI failures were tricky to track down because they didn't occur
> locally but after patching to dump backtraces they all seem to involve
> updates to virtio_set_status() as the machine was torn down. I think
> patch that switches all users to use virtio_device_started() along
> with consistent checking of vhost_dev->started stops this from
> happening. The clean-up seems worthwhile in reducing boilerplate
> anyway.
>
> The following patches still need review:
>
>   - tests/qtest: enable tests for virtio-gpio
>   - tests/qtest: add a get_features op to vhost-user-test
>   - tests/qtest: implement stub for VHOST_USER_GET_CONFIG
>   - tests/qtest: add assert to catch bad features
>   - tests/qtest: plain g_assert for VHOST_USER_F_PROTOCOL_FEATURES
>   - tests/qtest: catch unhandled vhost-user messages
>   - tests/qtest: use qos_printf instead of g_test_message
>   - tests/qtest: pass stdout/stderr down to subtests
>   - hw/virtio: move vhd->started check into helper and add FIXME
>   - hw/virtio: move vm_running check to virtio_device_started
>   - hw/virtio: add some vhost-user trace events
>   - hw/virtio: log potentially buggy guest drivers
>   - hw/virtio: fix some coding style issues
>   - include/hw: document vhost_dev feature life-cycle
>   - include/hw/virtio: more comment for VIRTIO_F_BAD_FEATURE
>   - hw/virtio: fix vhost_user_read tracepoint
>   - hw/virtio: handle un-configured shutdown in virtio-pci
>   - hw/virtio: gracefully handle unset vhost_dev vdev
>   - hw/virtio: incorporate backend features in features
<snip>

Ping?
Stefan Hajnoczi Sept. 19, 2022, 4:39 p.m. UTC | #2
On Fri, Sep 16, 2022 at 07:51:40AM +0100, Alex Bennée wrote:
> 
> Alex Bennée <alex.bennee@linaro.org> writes:
> 
> > Hi,
> >
> > This is an update to the previous series which fixes the last few
> > niggling CI failures I was seeing.
> >
> >    Subject: [PATCH v3 for 7.2 00/21] virtio-gpio and various virtio cleanups
> >    Date: Tue, 26 Jul 2022 20:21:29 +0100
> >    Message-Id: <20220726192150.2435175-1-alex.bennee@linaro.org>
> >
> > The CI failures were tricky to track down because they didn't occur
> > locally but after patching to dump backtraces they all seem to involve
> > updates to virtio_set_status() as the machine was torn down. I think
> > patch that switches all users to use virtio_device_started() along
> > with consistent checking of vhost_dev->started stops this from
> > happening. The clean-up seems worthwhile in reducing boilerplate
> > anyway.
> >
> > The following patches still need review:
> >
> >   - tests/qtest: enable tests for virtio-gpio
> >   - tests/qtest: add a get_features op to vhost-user-test
> >   - tests/qtest: implement stub for VHOST_USER_GET_CONFIG
> >   - tests/qtest: add assert to catch bad features
> >   - tests/qtest: plain g_assert for VHOST_USER_F_PROTOCOL_FEATURES
> >   - tests/qtest: catch unhandled vhost-user messages
> >   - tests/qtest: use qos_printf instead of g_test_message
> >   - tests/qtest: pass stdout/stderr down to subtests
> >   - hw/virtio: move vhd->started check into helper and add FIXME
> >   - hw/virtio: move vm_running check to virtio_device_started
> >   - hw/virtio: add some vhost-user trace events
> >   - hw/virtio: log potentially buggy guest drivers
> >   - hw/virtio: fix some coding style issues
> >   - include/hw: document vhost_dev feature life-cycle
> >   - include/hw/virtio: more comment for VIRTIO_F_BAD_FEATURE
> >   - hw/virtio: fix vhost_user_read tracepoint
> >   - hw/virtio: handle un-configured shutdown in virtio-pci
> >   - hw/virtio: gracefully handle unset vhost_dev vdev
> >   - hw/virtio: incorporate backend features in features
> <snip>
> 
> Ping?

Who are you pinging?

Only qemu-devel is on To and there are a bunch of people on Cc.

Stefan
Alex Bennée Sept. 20, 2022, 11:30 a.m. UTC | #3
Stefan Hajnoczi <stefanha@redhat.com> writes:

> [[PGP Signed Part:Undecided]]
> On Fri, Sep 16, 2022 at 07:51:40AM +0100, Alex Bennée wrote:
>> 
>> Alex Bennée <alex.bennee@linaro.org> writes:
>> 
>> > Hi,
>> >
>> > This is an update to the previous series which fixes the last few
>> > niggling CI failures I was seeing.
>> >
>> >    Subject: [PATCH v3 for 7.2 00/21] virtio-gpio and various virtio cleanups
>> >    Date: Tue, 26 Jul 2022 20:21:29 +0100
>> >    Message-Id: <20220726192150.2435175-1-alex.bennee@linaro.org>
>> >
>> > The CI failures were tricky to track down because they didn't occur
>> > locally but after patching to dump backtraces they all seem to involve
>> > updates to virtio_set_status() as the machine was torn down. I think
>> > patch that switches all users to use virtio_device_started() along
>> > with consistent checking of vhost_dev->started stops this from
>> > happening. The clean-up seems worthwhile in reducing boilerplate
>> > anyway.
>> >
>> > The following patches still need review:
>> >
>> >   - tests/qtest: enable tests for virtio-gpio
>> >   - tests/qtest: add a get_features op to vhost-user-test
>> >   - tests/qtest: implement stub for VHOST_USER_GET_CONFIG
>> >   - tests/qtest: add assert to catch bad features
>> >   - tests/qtest: plain g_assert for VHOST_USER_F_PROTOCOL_FEATURES
>> >   - tests/qtest: catch unhandled vhost-user messages
>> >   - tests/qtest: use qos_printf instead of g_test_message
>> >   - tests/qtest: pass stdout/stderr down to subtests
>> >   - hw/virtio: move vhd->started check into helper and add FIXME
>> >   - hw/virtio: move vm_running check to virtio_device_started
>> >   - hw/virtio: add some vhost-user trace events
>> >   - hw/virtio: log potentially buggy guest drivers
>> >   - hw/virtio: fix some coding style issues
>> >   - include/hw: document vhost_dev feature life-cycle
>> >   - include/hw/virtio: more comment for VIRTIO_F_BAD_FEATURE
>> >   - hw/virtio: fix vhost_user_read tracepoint
>> >   - hw/virtio: handle un-configured shutdown in virtio-pci
>> >   - hw/virtio: gracefully handle unset vhost_dev vdev
>> >   - hw/virtio: incorporate backend features in features
>> <snip>
>> 
>> Ping?
>
> Who are you pinging?
>
> Only qemu-devel is on To and there are a bunch of people on Cc.

Well I guess MST is the maintainer for the sub-system but I was hoping
other virtio contributors had some sort of view. The process of
up-streaming a simple vhost-user stub has flushed out all sorts of
stuff.

>
> Stefan
>
> [[End of PGP Signed Part]]
Stefan Hajnoczi Sept. 20, 2022, 6:25 p.m. UTC | #4
On Tue, 20 Sept 2022 at 10:18, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Stefan Hajnoczi <stefanha@redhat.com> writes:
>
> > [[PGP Signed Part:Undecided]]
> > On Fri, Sep 16, 2022 at 07:51:40AM +0100, Alex Bennée wrote:
> >>
> >> Alex Bennée <alex.bennee@linaro.org> writes:
> >>
> >> > Hi,
> >> >
> >> > This is an update to the previous series which fixes the last few
> >> > niggling CI failures I was seeing.
> >> >
> >> >    Subject: [PATCH v3 for 7.2 00/21] virtio-gpio and various virtio cleanups
> >> >    Date: Tue, 26 Jul 2022 20:21:29 +0100
> >> >    Message-Id: <20220726192150.2435175-1-alex.bennee@linaro.org>
> >> >
> >> > The CI failures were tricky to track down because they didn't occur
> >> > locally but after patching to dump backtraces they all seem to involve
> >> > updates to virtio_set_status() as the machine was torn down. I think
> >> > patch that switches all users to use virtio_device_started() along
> >> > with consistent checking of vhost_dev->started stops this from
> >> > happening. The clean-up seems worthwhile in reducing boilerplate
> >> > anyway.
> >> >
> >> > The following patches still need review:
> >> >
> >> >   - tests/qtest: enable tests for virtio-gpio
> >> >   - tests/qtest: add a get_features op to vhost-user-test
> >> >   - tests/qtest: implement stub for VHOST_USER_GET_CONFIG
> >> >   - tests/qtest: add assert to catch bad features
> >> >   - tests/qtest: plain g_assert for VHOST_USER_F_PROTOCOL_FEATURES
> >> >   - tests/qtest: catch unhandled vhost-user messages
> >> >   - tests/qtest: use qos_printf instead of g_test_message
> >> >   - tests/qtest: pass stdout/stderr down to subtests
> >> >   - hw/virtio: move vhd->started check into helper and add FIXME
> >> >   - hw/virtio: move vm_running check to virtio_device_started
> >> >   - hw/virtio: add some vhost-user trace events
> >> >   - hw/virtio: log potentially buggy guest drivers
> >> >   - hw/virtio: fix some coding style issues
> >> >   - include/hw: document vhost_dev feature life-cycle
> >> >   - include/hw/virtio: more comment for VIRTIO_F_BAD_FEATURE
> >> >   - hw/virtio: fix vhost_user_read tracepoint
> >> >   - hw/virtio: handle un-configured shutdown in virtio-pci
> >> >   - hw/virtio: gracefully handle unset vhost_dev vdev
> >> >   - hw/virtio: incorporate backend features in features
> >> <snip>
> >>
> >> Ping?
> >
> > Who are you pinging?
> >
> > Only qemu-devel is on To and there are a bunch of people on Cc.
>
> Well I guess MST is the maintainer for the sub-system but I was hoping
> other virtio contributors had some sort of view. The process of
> up-streaming a simple vhost-user stub has flushed out all sorts of
> stuff.

Okay, moving MST to To in case it helps get his attention.

Thanks,
Stefan
Michael S. Tsirkin Sept. 20, 2022, 7:10 p.m. UTC | #5
On Tue, Sep 20, 2022 at 02:25:48PM -0400, Stefan Hajnoczi wrote:
> On Tue, 20 Sept 2022 at 10:18, Alex Bennée <alex.bennee@linaro.org> wrote:
> >
> >
> > Stefan Hajnoczi <stefanha@redhat.com> writes:
> >
> > > [[PGP Signed Part:Undecided]]
> > > On Fri, Sep 16, 2022 at 07:51:40AM +0100, Alex Bennée wrote:
> > >>
> > >> Alex Bennée <alex.bennee@linaro.org> writes:
> > >>
> > >> > Hi,
> > >> >
> > >> > This is an update to the previous series which fixes the last few
> > >> > niggling CI failures I was seeing.
> > >> >
> > >> >    Subject: [PATCH v3 for 7.2 00/21] virtio-gpio and various virtio cleanups
> > >> >    Date: Tue, 26 Jul 2022 20:21:29 +0100
> > >> >    Message-Id: <20220726192150.2435175-1-alex.bennee@linaro.org>
> > >> >
> > >> > The CI failures were tricky to track down because they didn't occur
> > >> > locally but after patching to dump backtraces they all seem to involve
> > >> > updates to virtio_set_status() as the machine was torn down. I think
> > >> > patch that switches all users to use virtio_device_started() along
> > >> > with consistent checking of vhost_dev->started stops this from
> > >> > happening. The clean-up seems worthwhile in reducing boilerplate
> > >> > anyway.
> > >> >
> > >> > The following patches still need review:
> > >> >
> > >> >   - tests/qtest: enable tests for virtio-gpio
> > >> >   - tests/qtest: add a get_features op to vhost-user-test
> > >> >   - tests/qtest: implement stub for VHOST_USER_GET_CONFIG
> > >> >   - tests/qtest: add assert to catch bad features
> > >> >   - tests/qtest: plain g_assert for VHOST_USER_F_PROTOCOL_FEATURES
> > >> >   - tests/qtest: catch unhandled vhost-user messages
> > >> >   - tests/qtest: use qos_printf instead of g_test_message
> > >> >   - tests/qtest: pass stdout/stderr down to subtests
> > >> >   - hw/virtio: move vhd->started check into helper and add FIXME
> > >> >   - hw/virtio: move vm_running check to virtio_device_started
> > >> >   - hw/virtio: add some vhost-user trace events
> > >> >   - hw/virtio: log potentially buggy guest drivers
> > >> >   - hw/virtio: fix some coding style issues
> > >> >   - include/hw: document vhost_dev feature life-cycle
> > >> >   - include/hw/virtio: more comment for VIRTIO_F_BAD_FEATURE
> > >> >   - hw/virtio: fix vhost_user_read tracepoint
> > >> >   - hw/virtio: handle un-configured shutdown in virtio-pci
> > >> >   - hw/virtio: gracefully handle unset vhost_dev vdev
> > >> >   - hw/virtio: incorporate backend features in features
> > >> <snip>
> > >>
> > >> Ping?
> > >
> > > Who are you pinging?
> > >
> > > Only qemu-devel is on To and there are a bunch of people on Cc.
> >
> > Well I guess MST is the maintainer for the sub-system but I was hoping
> > other virtio contributors had some sort of view. The process of
> > up-streaming a simple vhost-user stub has flushed out all sorts of
> > stuff.
> 
> Okay, moving MST to To in case it helps get his attention.
> 
> Thanks,
> Stefan

thanks, it's in my queue, just pulling in backlog that built up
during the forum.
Alex Bennée Sept. 20, 2022, 9:18 p.m. UTC | #6
"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Tue, Sep 20, 2022 at 02:25:48PM -0400, Stefan Hajnoczi wrote:
>> On Tue, 20 Sept 2022 at 10:18, Alex Bennée <alex.bennee@linaro.org> wrote:
>> >
>> >
>> > Stefan Hajnoczi <stefanha@redhat.com> writes:
>> >
>> > > [[PGP Signed Part:Undecided]]
>> > > On Fri, Sep 16, 2022 at 07:51:40AM +0100, Alex Bennée wrote:
>> > >>
>> > >> Alex Bennée <alex.bennee@linaro.org> writes:
>> > >>
>> > >> > Hi,
>> > >> >
>> > >> > This is an update to the previous series which fixes the last few
>> > >> > niggling CI failures I was seeing.
>> > >> >
>> > >> >    Subject: [PATCH v3 for 7.2 00/21] virtio-gpio and various virtio cleanups
>> > >> >    Date: Tue, 26 Jul 2022 20:21:29 +0100
>> > >> >    Message-Id: <20220726192150.2435175-1-alex.bennee@linaro.org>
>> > >> >
>> > >> > The CI failures were tricky to track down because they didn't occur
>> > >> > locally but after patching to dump backtraces they all seem to involve
>> > >> > updates to virtio_set_status() as the machine was torn down. I think
>> > >> > patch that switches all users to use virtio_device_started() along
>> > >> > with consistent checking of vhost_dev->started stops this from
>> > >> > happening. The clean-up seems worthwhile in reducing boilerplate
>> > >> > anyway.
>> > >> >
>> > >> > The following patches still need review:
>> > >> >
>> > >> >   - tests/qtest: enable tests for virtio-gpio
>> > >> >   - tests/qtest: add a get_features op to vhost-user-test
>> > >> >   - tests/qtest: implement stub for VHOST_USER_GET_CONFIG
>> > >> >   - tests/qtest: add assert to catch bad features
>> > >> >   - tests/qtest: plain g_assert for VHOST_USER_F_PROTOCOL_FEATURES
>> > >> >   - tests/qtest: catch unhandled vhost-user messages
>> > >> >   - tests/qtest: use qos_printf instead of g_test_message
>> > >> >   - tests/qtest: pass stdout/stderr down to subtests
>> > >> >   - hw/virtio: move vhd->started check into helper and add FIXME
>> > >> >   - hw/virtio: move vm_running check to virtio_device_started
>> > >> >   - hw/virtio: add some vhost-user trace events
>> > >> >   - hw/virtio: log potentially buggy guest drivers
>> > >> >   - hw/virtio: fix some coding style issues
>> > >> >   - include/hw: document vhost_dev feature life-cycle
>> > >> >   - include/hw/virtio: more comment for VIRTIO_F_BAD_FEATURE
>> > >> >   - hw/virtio: fix vhost_user_read tracepoint
>> > >> >   - hw/virtio: handle un-configured shutdown in virtio-pci
>> > >> >   - hw/virtio: gracefully handle unset vhost_dev vdev
>> > >> >   - hw/virtio: incorporate backend features in features
>> > >> <snip>
>> > >>
>> > >> Ping?
>> > >
>> > > Who are you pinging?
>> > >
>> > > Only qemu-devel is on To and there are a bunch of people on Cc.
>> >
>> > Well I guess MST is the maintainer for the sub-system but I was hoping
>> > other virtio contributors had some sort of view. The process of
>> > up-streaming a simple vhost-user stub has flushed out all sorts of
>> > stuff.
>> 
>> Okay, moving MST to To in case it helps get his attention.
>> 
>> Thanks,
>> Stefan
>
> thanks, it's in my queue, just pulling in backlog that built up
> during the forum.

Thanks, doing the same myself ;-)