Re: [PATCH] vhost-user: Silence unsupported VHOST_USER_PROTOCOL_F_RARP error

[Laurent Vivier]

On 22/01/2025 17:20, Stefano Garzarella wrote:
> On Wed, Jan 22, 2025 at 08:59:22AM -0500, Michael S. Tsirkin wrote:
> > On Wed, Jan 22, 2025 at 02:42:14PM +0100, Stefano Garzarella wrote:
> > > On Tue, Jan 21, 2025 at 11:00:29AM +0100, Laurent Vivier wrote:
> > > > In vhost_user_receive() if vhost_net_notify_migration_done() reports
> > > > an error we display on the console:
> > > >
> > > >  Vhost user backend fails to broadcast fake RARP
> > > >
> > > > This message can be useful if there is a problem to execute
> > > > VHOST_USER_SEND_RARP but it is useless if the backend doesn't
> > > > support VHOST_USER_PROTOCOL_F_RARP.
> > > >
> > > > Don't report the error if vhost_net_notify_migration_done()
> > > > returns -ENOTSUP (from vhost_user_migration_done())
> > > >
> > > > Update vhost_net-stub.c to return -ENOTSUP too.
> > > >
> > > > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > > > ---
> > > > hw/net/vhost_net-stub.c | 2 +-
> > > > net/vhost-user.c        | 2 +-
> > > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/hw/net/vhost_net-stub.c b/hw/net/vhost_net-stub.c
> > > > index 72df6d757e4d..875cd6c2b9c8 100644
> > > > --- a/hw/net/vhost_net-stub.c
> > > > +++ b/hw/net/vhost_net-stub.c
> > > > @@ -93,7 +93,7 @@ void vhost_net_config_mask(VHostNetState *net, VirtIODevice *dev, 
> > > bool mask)
> > > >
> > > > int vhost_net_notify_migration_done(struct vhost_net *net, char* mac_addr)
> > > > {
> > > > -    return -1;
> > > > +    return -ENOTSUP;
> > > > }
> > > >
> > > > VHostNetState *get_vhost_net(NetClientState *nc)
> > > > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > > > index 12555518e838..636fff8a84a2 100644
> > > > --- a/net/vhost-user.c
> > > > +++ b/net/vhost-user.c
> > > > @@ -146,7 +146,7 @@ static ssize_t vhost_user_receive(NetClientState *nc, const 
> > > uint8_t *buf,
> > > >
> > > >         r = vhost_net_notify_migration_done(s->vhost_net, mac_addr);
> > > >
> > > > -        if ((r != 0) && (display_rarp_failure)) {
> > > > +        if ((r != 0) && (r != -ENOTSUP) && (display_rarp_failure)) {
> > > >             fprintf(stderr,
> > > >                     "Vhost user backend fails to broadcast fake RARP\n");
> > > >             fflush(stderr);
> > > > --
> > > > 2.47.1
> > > >
> > >
> > > IIUC the message was there since the introduction about 10 years ago
> > > from commit 3e866365e1 ("vhost user: add rarp sending after live
> > > migration for legacy guest"). IIUC -ENOTSUP is returned when both F_RARP
> > > and F_GUEST_ANNOUNCE are not negotiated.
> > >
> > > That said, I honestly don't know what F_RARP or F_GUEST_ANNOUNCE is for,
> >
> > rarp is to have destination host broadcast a message with VM address
> > to update the network. Guest announce is when it will instead
> > ask the guest to do this.
>
> Okay, thanks for explaining to me.
> So if both features are not negotiated, no one is going to broadcast
> the message, right?
>
> Could that be a valid reason to print an error message in QEMU?
>
> To me it might be reasonable because the user might experience some
> network problems, but I'm not a network guy :-)

I'm working on adding vhost-user to passt[1], and in this case we don't need to broadcast 
any message.

So I don't implement VHOST_USER_SEND_RARP and I don't want the error message to spoil my 
console.

-ENOTSUP is an error message for developer not for user.

Thanks,
Laurent
[1] https://passt.top/passt/