[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: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 08/24] vhost-user: return a read error |
Date: |
Tue, 5 Jul 2016 14:12:10 +0300 |
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.
> >> 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.
> >> > 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.
Protocol limits # of messages in flight so it should not trigger
even if backend is stuck.
> >
> >
> >>
> >> --
> >> Marc-André Lureau
>
>
>
> --
> Marc-André Lureau