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

[Laurent Vivier <lvivier@redhat.com>]

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/