qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Fiona Ebner <f.ebner@proxmox.com>,
	qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>,
	qemu-block@nongnu.org, Hanna Reitz <hreitz@redhat.com>,
	hujian <hu.jian@zte.com.cn>
Subject: Re: [PULL 1/1] virtio-blk: don't start dataplane during the stop of dataplane
Date: Tue, 17 Oct 2023 09:46:53 -0400	[thread overview]
Message-ID: <20231017134653.GB211967@fedora> (raw)
In-Reply-To: <ZS5bstofOtQIdz84@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

On Tue, Oct 17, 2023 at 12:02:26PM +0200, Kevin Wolf wrote:
> Am 17.10.2023 um 11:01 hat Fiona Ebner geschrieben:
> > Am 16.10.23 um 21:40 schrieb Stefan Hajnoczi:
> > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > > index 39e7f23fab..c2d59389cb 100644
> > > --- a/hw/block/virtio-blk.c
> > > +++ b/hw/block/virtio-blk.c
> > > @@ -1166,7 +1166,7 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> > >  {
> > >      VirtIOBlock *s = (VirtIOBlock *)vdev;
> > >  
> > > -    if (s->dataplane && !s->dataplane_started) {
> > > +    if (s->dataplane && !s->dataplane_started && !s->stopping) {
> > 
> > Isn't 'stopping' a property of VirtIOBlockDataPlane? Here, s is VirtIOBlock.
> 
> Indeed, this patch doesn't even build for me.
> 
> However, even if we wrote !s->dataplane->stopping, would it really be
> right to be handling I/O in the main thread while the dataplane hasn't
> stopped yet? At least without all the multiqueue changes, it's not
> obvious to me that it can't cause problems. Unfortunately, the commit
> message doesn't say anything about why it's safe.

Thanks for pointing these things out, Fiona and Kevin. I've dropped the
patch for now.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

      reply	other threads:[~2023-10-17 13:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-16 19:40 [PULL 0/1] Block patches Stefan Hajnoczi
2023-10-16 19:40 ` [PULL 1/1] virtio-blk: don't start dataplane during the stop of dataplane Stefan Hajnoczi
2023-10-17  9:01   ` Fiona Ebner
2023-10-17 10:02     ` Kevin Wolf
2023-10-17 13:46       ` Stefan Hajnoczi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231017134653.GB211967@fedora \
    --to=stefanha@redhat.com \
    --cc=f.ebner@proxmox.com \
    --cc=hreitz@redhat.com \
    --cc=hu.jian@zte.com.cn \
    --cc=kwolf@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).