qemu-devel.nongnu.org archive mirror
 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 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).