Re: [Qemu-devel] [PATCH RFC 00/14] vhost-user: shutdown and reconnection

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

On Fri, Mar 25, 2016 at 07:00:15PM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Mar 24, 2016 at 8:10 AM, Yuanhan Liu
> <yuanhan.liu@linux.intel.com> wrote:
> >> > The following series starts from the idea that the slave can request a
> >> > "managed" shutdown instead and later recover (I guess the use case for
> >> > this is to allow for example to update static dispatching/filter rules
> >> > etc)
> >
> > What if the backend crashes, that no such request will be sent? And
> > I'm wondering why this request is needed, as we are able to detect
> > the disconnect now (with your patches).

Hi, sorry for late and thanks for the input.

(CC'ed more guys, as they are the hungry users that wish this feature
 to be done ASAP; they may provide more valuable inputs)

> 
> I don't think trying to handle backend crashes is really a thing we
> need to take care of.

Yes, me neither. The reason I mentioned backend crash is that it's
one of the reasons why we need vhost-user reconnect stuff. And the
another reason is the backend restart.

Backend crash may be not that normal in production usage, it is in
development stage. It would be handy if we can handle that as well,
as it's a painful experience for me that every time I do a VM2VM test
and once my changes to backend causes a crash (unfortunately, it
happens often in early stage), I have to reboot the two VMs.

If we have reconnect, life could be easier. I then don't need worry
that I will mess the backend and crash it any more.

And again, the reason I mentioned crash here again is not because
we need handle it, specially. Instead, I was thinking we might be
able to handle the two reasons both.

> If the backend is bad enough to crash, it may as
> well corrupt the guest memory (mst: my understanding of vhost-user is
> that backend must be trusted, or it could just throw garbage in the
> queue descriptors with surprising consequences or elsewhere in the
> guest memory actually, right?).
> 
> > BTW, you meant to let QEMU as the server and the backend as the client
> > here, right? Honestly, that's what we've thought of, too, in the first
> > time.
> > However, I'm wondering could we still go with the QEMU as the client
> > and the backend as the server (the default and the only way DPDK
> > supports), and let QEMU to try to reconnect when the backend crashes
> > and restarts. In such case, we need enable the "reconnect" option
> > for vhost-user, and once I have done that, it basically works in my
> > test:
> >
> 
> Conceptually, I think if we allow the backend to disconnect, it makes
> sense that qemu is actually the socket server. But it doesn't matter
> much, it's simple to teach qemu to reconnect a timer...

We already have that, right? I mean, the char-dev "reconnect" option.

> So we should
> probably allow both cases anyway.

Yes, I think both should work. I may be wrong (that I may miss
something), but it seems it's (far) easier to me to keep QEMU
as the client, and adding the "reconnect" option, judging that
we have all stuff to make it work ready. In this way,

- if backend crashes/quits, you just need restart the backend,
  and QEMU will retry the connection.

- if QEMU crashes/quits, you just need restart the QEMU, and
  then QEMU will start a fresh connection.


However, if let QEMU as the server, there are 2 major works need
to be done in the backend side (take DPDK as example, that just
acts as vhost-user server only so far):

- Introducing a new API or extending current API to let it to
  connect the server socket from QEMU.

- If QEMU crashes/quits, we need add code to backend to keep
  reconnecting unless connection is established, which is something
  similar to the "reconnect" stuff in QEMU.

As you can see, it needs more effort (though it's not something
you care :). And it has duplicate work.

> 
> > - start DPDK vhost-switch example
> >
> > - start QEMU, which will connect to DPDK vhost-user
> >
> >   link is good now.
> >
> > - kill DPDK vhost-switch
> >
> >   link is broken at this stage
> >
> > - start DPDK vhost-switch again
> >
> >   you will find that the link is back again.
> >
> >
> > Will that makes sense to you? If so, we may need do nothing (or just
> > very few) changes at all to DPDK to get the reconnect work.
> 
> The main issue with handling crashes (gone at any time) is that the
> backend my not have time to sync the used idx (at the least). It may
> already have processed incoming packets, so on reconnect, it may
> duplicate the receiving/dispatching work.

That's not the case for DPDK backend implementation: incoming packets
won't be delivered for processing before we update the used idx.

> Similarly, on the backend
> receiving end, some packets may be lost, never received by the VM, and
> later overwritten by the backend after reconnect (for the same used
> idx update reason). This may not be a big deal for unreliable
> protocols, but I am not familiar enough with network usage to know if
> that's fine in all cases. It may be fine for some packets, such as
> udp.
> 
> However, in general, vhost-user should not be specific to network
> transmission, and it would be nice to have a reliable way for the the
> backend to reconnect. That's what I try to do in this series.

Agreed.

> I'll
> repost it after I have done more testing.

Great, and thanks for the work. I will also take some time to give
closer review.

	--yliu