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

[Marc-André Lureau]

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.



-- 
Marc-André Lureau