Re: [Qemu-devel] [PATCH 00/24] vhost-user reconnect fixes

[Marc-André Lureau]

On Sat, Jun 25, 2016 at 12:19 AM, Michael S. Tsirkin <address@hidden> wrote:
>> On Thu, Jun 23, 2016 at 6:31 AM, Michael S. Tsirkin <address@hidden> 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?

As long as the disconnection is not well specified and tested, I would
rather see errors. I think we have a fundamental disagreement on how
to deal with disconnections and the previous series. To me, it is a
best effort so far (works if you are lucky), it is not the way
qemu/vhost-user should support disconnect, which I always though
should be handled by an initial request from the backend, something we
don't have yet.

>> >> 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.

Sorry if the patch comments aren't clear, what about?


vhost-user: wait until backend init is completed

The chardev waits for an initial connection before starting qemu, and
vhost-user should wait for the backend negotiation to be completed
before starting qemu too.

vhost-user is started in the net_vhost_user_event callback, which is
synchronously called after the socket is connected. Use a
VhostUserState.started flag to indicate vhost-user init completed
successfully and qemu can be started.


 (feel free to suggest improvements)

thanks

-- 
Marc-André Lureau