qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@gmail.com>
Cc: Tetsuya Mukawa <mukawa@igel.co.jp>,
	Yuanhan Liu <yuanhan.liu@linux.intel.com>,
	Victor Kaplansky <victork@redhat.com>,
	jonshin@cisco.com, QEMU <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH 00/24] vhost-user reconnect fixes
Date: Sat, 25 Jun 2016 01:19:22 +0300	[thread overview]
Message-ID: <20160625011545-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <CAJ+F1C+Sfvd1vWJwxFr7OnMsVKGcfoR3yqYpCLhx6W5o_OecBA@mail.gmail.com>

On Fri, Jun 24, 2016 at 03:22:19PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Jun 23, 2016 at 6:31 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > OK so if it's ok for read or write to fail, then I think
> > the callback should just return success.
> > This will make sure backends such as vhost net in kernel
> > where failure indicates an internal bug are handled
> > correctly, while vhost user ignores errors and attempts
> > to go on.
> >
> 
> That's what we are trying to do, but since it's not yet well defined
> how to disconnect/reconnect and there are many bugs around this, it
> would help a lot to add error checking and error reporting for now.

Just for debugging?  OK, how about a macro with #ifdef DEBUG or
something like that then?


> >> Since there is feature checks on reconnection, qemu should wait for
> >> the initial connection feature negotiation to complete. The test added
> >> demonstrates this.
> >
> >
> > So on disconnect, we keep the guest going? But then when connect is
> > attempted, then we for some reason block guest until negotiation
> > is done? Sounds rather strange to me.
> 
> When qemu is the server, it already waits for an initial connection
> from vhost-user backend. Here it is just enhanced to wait for the
> initial nego to complete, which is necessary for proper device
> initialization and also further reconnections checks.

OH, it's just for the initial connection? I didn't realize.
Will have to re-read it to figure it out. Some comments in commit log
and code pointing this out could also help.

> -- 
> Marc-André Lureau

  reply	other threads:[~2016-06-24 22:19 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-21 10:02 [Qemu-devel] [PATCH 00/24] vhost-user reconnect fixes marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 01/24] misc: indentation marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 02/24] vhost-user: minor simplification marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 03/24] qemu-char: check socket is actually connected marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 04/24] vhost-user: check qemu_chr_fe_set_msgfds() return value marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 05/24] vhost: change some assert() for error_report() or silent fail marcandre.lureau
2016-06-23  4:27   ` Michael S. Tsirkin
2016-06-23  9:19     ` Marc-André Lureau
2016-06-23 17:13       ` Michael S. Tsirkin
2016-06-24 13:25         ` Marc-André Lureau
2016-06-24 22:38           ` Michael S. Tsirkin
2016-06-27  9:01             ` Marc-André Lureau
2016-07-04 15:43               ` Michael S. Tsirkin
2016-07-06 13:47                 ` Marc-André Lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 06/24] vhost-user: check vhost_user_write() return value marcandre.lureau
2016-06-23  4:27   ` Michael S. Tsirkin
2016-06-23  9:16     ` Marc-André Lureau
2016-06-23 17:08       ` Michael S. Tsirkin
2016-06-24 12:49         ` Marc-André Lureau
2016-07-04 15:46           ` Michael S. Tsirkin
2016-07-04 22:01             ` Marc-André Lureau
2016-07-04 22:36               ` Michael S. Tsirkin
2016-06-21 10:02 ` [Qemu-devel] [PATCH 07/24] vhost: use error_report() instead of fprintf(stderr, ...) marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 08/24] vhost-user: return a read error marcandre.lureau
2016-06-23  4:27   ` Michael S. Tsirkin
2016-06-23  9:14     ` Marc-André Lureau
2016-06-23 17:03       ` Michael S. Tsirkin
2016-06-24 12:46         ` Marc-André Lureau
2016-07-04 15:47           ` Michael S. Tsirkin
2016-07-04 21:56             ` Marc-André Lureau
2016-07-04 22:35               ` Michael S. Tsirkin
2016-07-05  9:18                 ` Marc-André Lureau
2016-07-05 11:12                   ` Michael S. Tsirkin
2016-07-06 13:40                     ` Marc-André Lureau
2016-07-06 13:52                       ` Marc-André Lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 09/24] vhost: make vhost_log_put() idempotent marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 10/24] vhost: call vhost_log_put() on cleanup marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 11/24] vhost: add vhost device only after all success marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 12/24] vhost: make vhost_dev_cleanup() idempotent marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 13/24] vhost-net: always call vhost_dev_cleanup() on failure marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 14/24] vhost: don't assume opaque is a fd, use backend cleanup marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 15/24] vhost: fix calling vhost_dev_cleanup() after vhost_dev_init() marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 16/24] vhost-user: keep vhost_net after a disconnection marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 17/24] Revert "vhost-net: do not crash if backend is not present" marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 18/24] get_vhost_net() should be != null after vhost_user_init marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 19/24] vhost-net: success if backend has no ops->vhost_migration_done marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 20/24] vhost: add assert() to check runtime behaviour marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 21/24] char: add chr_wait_connected callback marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 22/24] char: add and use tcp_chr_wait_connected marcandre.lureau
2016-06-21 10:02 ` [Qemu-devel] [PATCH 23/24] vhost-user: wait until link is up marcandre.lureau
2016-06-23  4:25   ` Michael S. Tsirkin
2016-06-23  9:11     ` Marc-André Lureau
2016-06-23 16:45       ` Michael S. Tsirkin
2016-06-21 10:02 ` [Qemu-devel] [PATCH 24/24] tests: add /vhost-user/connect-fail test marcandre.lureau
2016-06-23  4:31 ` [Qemu-devel] [PATCH 00/24] vhost-user reconnect fixes Michael S. Tsirkin
2016-06-24 13:22   ` Marc-André Lureau
2016-06-24 22:19     ` Michael S. Tsirkin [this message]
2016-06-27 12:37       ` Marc-André Lureau
2016-06-23  4:32 ` Michael S. Tsirkin
2016-06-24 13:17   ` 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=20160625011545-mutt-send-email-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=jonshin@cisco.com \
    --cc=marcandre.lureau@gmail.com \
    --cc=mukawa@igel.co.jp \
    --cc=qemu-devel@nongnu.org \
    --cc=victork@redhat.com \
    --cc=yuanhan.liu@linux.intel.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).