qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Marc-André Lureau
Subject: Re: [Qemu-devel] [PATCH 08/24] vhost-user: return a read error
Date: Wed, 6 Jul 2016 15:52:01 +0200

On Wed, Jul 6, 2016 at 3:40 PM, Marc-André Lureau
<address@hidden> wrote:
> Hi
>
> On Tue, Jul 5, 2016 at 1:12 PM, Michael S. Tsirkin <address@hidden> wrote:
>> On Tue, Jul 05, 2016 at 11:18:38AM +0200, Marc-André Lureau wrote:
>>> 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
>>
>> You can't prevent backend bugs I think.
>
> Right, but it's best to provide an error when you can detect backend bugs.
>
>>> >> 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.
>>
>> But I don't see why it's bad. If it's not modifying memory then
>> it does not need to log any changes.
>
> "if it's not modifying memory"...
>
> I fail to understand why some code path check error code, and some
> don't. Ignoring error and running further may lead to wrong
> assumptions and later issues. I also fail to understand why providing
> more useful error messages is bad. I feel quite strongly about having
> more consistent error checking in vhost-user, I don't get why you
> don't.
>

Fwiw, Gonglei came up with the same patch in "vhost-user: fix
unreasonable return value when vhost-user read failed" and he gave
extra reasons for it that are hard to deny.



-- 
Marc-André Lureau



reply via email to

[Prev in Thread] Current Thread [Next in Thread]