Re: [PATCH] block/vhost-user-blk: Fix hang on boot for some odd guests

[Michael S. Tsirkin]

On Tue, Apr 18, 2023 at 07:20:29PM +0200, Andrey Ryabinin wrote:
> 
> 
> On 4/18/23 08:17, Michael S. Tsirkin wrote:
> > On Tue, Apr 18, 2023 at 05:13:11AM +0000, Raphael Norwitz wrote:
> >> Hey Andrey - apologies for the late reply here.
> >>
> >> It sounds like you are dealing with a buggy guest, rather than a QEMU 
> >> issue.
> >>
> >>> On Apr 10, 2023, at 11:39 AM, Andrey Ryabinin <arbn@yandex-team.com> 
> >>> wrote:
> >>>
> >>>
> >>>
> >>> On 4/10/23 10:35, Andrey Ryabinin wrote:
> >>>> Some guests hang on boot when using the vhost-user-blk-pci device,
> >>>> but boot normally when using the virtio-blk device. The problem occurs
> >>>> because the guest advertises VIRTIO_F_VERSION_1 but kicks the virtqueue
> >>>> before setting VIRTIO_CONFIG_S_DRIVER_OK, causing vdev->start_on_kick to
> >>
> >> Virtio 1.1 Section 3.1.1, says during setup “[t]he driver MUST NOT notify 
> >> the device before setting DRIVER_OK.”
> >>
> >> Therefore what you are describing is buggy guest behavior. Sounds like the 
> >> driver should be made to either
> >> - not advertise VIRTIO_F_VERSION_1
> >> - not kick before setting VIRTIO_CONFIG_S_DRIVER_OK
> >>
> >> If anything, the virtio-blk virtio_blk_handle_output() function should 
> >> probably check start_on_kick?
> > 
> > Question is, how easy is this guest to fix.
> > 
> 
> I wouldn't count on that.
> 
> In this case the guest is Foritgate firewall, apparently from this guys 
> https://www.fortinet.com/
> It seems that the kernel they use claims itself as 3.2.16 Linux kernel, 
> however it looks like
> it's not vanilla kernel, but modified with some backports. I'm guessing that 
> they backported
> the patches introducing VIRTIO_F_VERSTION_1, but they didn't add this patch 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7a11370e5e6c26566904bb7f08281093a3002ff2
> 
> I've tried to look the sources of the kernel they use but failed to find any.
> Found only some news about gpl voilation from 2005 )

All I remember is this is a can of worms, the change seems risky
frankly. If you like, I'm open to an unstable property
(starting with "x-") that forces start on kick.
Will be handy for debugging too.


-- 
MST