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

[Michael S. Tsirkin]

On Wed, Jan 22, 2025 at 05:20:06PM +0100, 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 :-)
> 
> Thanks,
> Stefano

reasonable, yes.

> > 
> > 
> > > but my understanding is that the message was to notify that the
> > > migration was finished (reading that commit).
> > > 
> > > If neither feature is supported, could this be a problem for the user
> > > and that's why we were printing the message?
> > > 
> > > Thanks,
> > > Stefano
> >