All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Coiby Xu <coiby.xu@gmail.com>
Cc: bharatlkmlkvm@gmail.com,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	qemu-devel@nongnu.org, "Stefan Hajnoczi" <stefanha@redhat.com>
Subject: Re: [PATCH v4 0/5] vhost-user block device backend implementation
Date: Thu, 27 Feb 2020 11:02:06 +0100	[thread overview]
Message-ID: <20200227100206.GA7493@linux.fritz.box> (raw)
In-Reply-To: <CAJAkqrWUJWLdT+6b_XmHFwnzhhbYei2SakCKVW0Rf92HJgoZDw@mail.gmail.com>

Am 27.02.2020 um 10:53 hat Coiby Xu geschrieben:
> Thank you for reminding me of this socket short read issue! It seems
> we still need customized vu_message_read because libvhost-user assumes
> we will always get a full-size VhostUserMsg and hasn't taken care of
> this short read case. I will improve libvhost-user's vu_message_read
> by making it keep reading from socket util getting enough bytes. I
> assume short read is a rare case thus introduced performance penalty
> would be negligible.

In any case, please make sure that we use the QIOChannel functions
called from a coroutine in QEMU so that it will never block, but the
coroutine can just yield while it's waiting for more bytes.

Kevin

> On Thu, Feb 27, 2020 at 3:41 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Wed, Feb 26, 2020 at 11:18:41PM +0800, Coiby Xu wrote:
> > > Hi Stefan,
> > >
> > > Thank you for reviewing my code!
> > >
> > > I tried to reach you on IRC. But somehow either you missed my message
> > > or I missed your reply. So I will reply by email instead.
> > >
> > > If we use qio_channel_set_aio_fd_handler to monitor G_IO_IN event,
> > > i.e. use vu_dispatch as the read handler, then we can re-use
> > > vu_message_read. And "removing the blocking recv from libvhost-user"
> > > isn't necessary because "the operation of poll() and ppoll() is not
> > > affected by the O_NONBLOCK flag" despite that we use
> > > qio_channel_set_blocking before calling qio_channel_set_aio_fd_handler
> > > to make recv non-blocking.
> >
> > I'm not sure I understand.  poll() just says whether the file descriptor
> > is readable.  It does not say whether enough bytes are readable :).  So
> > our callback will be invoked if there is 1 byte ready, but when we try
> > to read 20 bytes either it will block (without O_NONBLOCK) or return
> > only 1 byte (with O_NONBLOCK).  Neither case is okay, so I expect that
> > code changes will be necessary.
> >
> > But please go ahead and send the next revision and I'll take a look.
> >
> > Stefan
> 
> 
> 
> --
> Best regards,
> Coiby
> 



  reply	other threads:[~2020-02-27 10:04 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-18  5:07 [PATCH v4 0/5] vhost-user block device backend implementation Coiby Xu
2020-02-18  5:07 ` [PATCH v4 1/5] extend libvhost to support IOThread and coroutine Coiby Xu
2020-02-19 16:33   ` Stefan Hajnoczi
2020-02-25 14:55   ` Kevin Wolf
2020-02-18  5:07 ` [PATCH v4 2/5] generic vhost user server Coiby Xu
2020-02-25 15:44   ` Kevin Wolf
2020-02-28  4:23     ` Coiby Xu
2020-02-18  5:07 ` [PATCH v4 3/5] vhost-user block device backend server Coiby Xu
2020-02-25 16:09   ` Kevin Wolf
2020-02-18  5:07 ` [PATCH v4 4/5] a standone-alone tool to directly share disk image file via vhost-user protocol Coiby Xu
2020-02-18  5:07 ` [PATCH v4 5/5] new qTest case to test the vhost-user-blk-server Coiby Xu
2020-02-19 16:38 ` [PATCH v4 0/5] vhost-user block device backend implementation Stefan Hajnoczi
2020-02-26 15:18   ` Coiby Xu
2020-02-27  7:41     ` Stefan Hajnoczi
2020-02-27  9:53       ` Coiby Xu
2020-02-27 10:02         ` Kevin Wolf [this message]
2020-02-27 10:28           ` Coiby Xu
2020-02-27 10:55             ` Kevin Wolf
2020-02-27 11:07               ` Marc-André Lureau
2020-02-27 11:19                 ` Kevin Wolf
2020-02-27 11:38                   ` Daniel P. Berrangé
2020-02-27 13:07                     ` Marc-André Lureau

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=20200227100206.GA7493@linux.fritz.box \
    --to=kwolf@redhat.com \
    --cc=bharatlkmlkvm@gmail.com \
    --cc=coiby.xu@gmail.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.