Re: [Qemu-devel] [PATCH 08/24] vhost-user: return a read error

[Marc-André Lureau]

Hi

On Tue, Jul 5, 2016 at 12:35 AM, Michael S. Tsirkin <address@hidden> wrote:
> On Mon, Jul 04, 2016 at 11:56:56PM +0200, Marc-André Lureau wrote:
>> Hi
>>
>> On Mon, Jul 4, 2016 at 5:47 PM, Michael S. Tsirkin <address@hidden> wrote:
>> > Why does vhost_user_set_log_base need to return error?
>> > If backend is not there to handle this message,
>> > then it is not changing memory so it's ok to ignore the error.
>>
>> How do you know it's not changing the memory?
>
> either it closed socket intentionally or it exited
> and kernel cleaned up.

And if it closed intentionally during migration, we want to catch this
as a bug since it may still modify the memory

>> Furthermore, if the migration happened, it's because backend claim
>> VHOST_F_LOG_ALL, thus it should really not fail
>
> I don't see why - could you explain pls?

If the backend claims migration support, it shouldn't have bad
migration behaviour such as closing the vhost-user socket.

>> > Same logic applies to many other messages.
>>
>> Pretty much all messages, the error can't be ignored, or operations
>> will just fail silentely randomly. I don't understand why vhost-user
>> io error can be ignored. Also it's quite inconsistent the way the code
>> is today, vhost_user_write() returns an error that is mostly ignored,
>> while vhost_user_read() is checked. Why having an error later when you
>> can report it earlier? I fail to understand the rationale of this
>> error handling.
>
> It's historical.  the way I see it, most errors due to disconnect
> can be ignored except
> maybe for the initial feature negotiation which is needed
> so we know what to tell guest.

The way I see it is that errors should not be ignored because it makes
it harder to track what is going on.

> Errors due to e.g. buffer being full should cause an assert
> as it's an internal qemu error.

I don't see why qemu would be responsible for say, a suspended backend.

>
>
>>
>> --
>> Marc-André Lureau



-- 
Marc-André Lureau