mbox series

[0/2] vhost-vdpa: improve parameters error management

Message ID 20200903185327.774708-1-lvivier@redhat.com
Headers show
Series vhost-vdpa: improve parameters error management | expand

Message

Laurent Vivier Sept. 3, 2020, 6:53 p.m. UTC 
If vhostdev is not provided QEMU crashes, and more generally
vhost-vdpa doesn't explain why parameters are wrong.

This series fixes that.

Laurent Vivier (2):
  vhost-vdpa: define and use default value for vhostdev
  vhost-vdpa: improve error reporting

 net/vhost-vdpa.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

-- 
2.26.2

Comments

no-reply@patchew.org Sept. 4, 2020, 1:28 p.m. UTC | #1 
Patchew URL: https://patchew.org/QEMU/20200903185327.774708-1-lvivier@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.






The full log is available at
http://patchew.org/logs/20200903185327.774708-1-lvivier@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Jason Wang Sept. 9, 2020, 1:50 a.m. UTC | #2 
On 2020/9/4 上午2:53, Laurent Vivier wrote:
> vhostdev is defined as optional in net.json, and if not set
> /dev/vhost-vdpa-0 should be used.
>
> The default value is not set and if vhostdev is not provided
> QEMU crashes with a SIGSEGV exception.
>
> Fixes: 1e0a84ea49b6 ("vhost-vdpa: introduce vhost-vdpa net client")
> Cc: lulu@redhat.com
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   net/vhost-vdpa.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index bc0e0d2d35b7..24103ef241e4 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -24,6 +24,9 @@
>   #include "monitor/monitor.h"
>   #include "hw/virtio/vhost.h"
>   
> +/* default vhostdev as defined in qapi/net.json */
> +#define VHOST_VDPA_DEFAULT_VHOSTDEV "/dev/vhost-vdpa-0"
> +
>   /* Todo:need to add the multiqueue support here */
>   typedef struct VhostVDPAState {
>       NetClientState nc;
> @@ -224,5 +227,7 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
>                             (char *)name, errp)) {
>           return -1;
>       }
> -    return net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name, opts->vhostdev);
> +    return net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name,
> +                               opts->has_vhostdev ?
> +                               opts->vhostdev : VHOST_VDPA_DEFAULT_VHOSTDEV);
>   }


Hi Laurent:

I think having a default path could introduce more confusion here.

So I post a patch to remove the default [1].

Thanks

[1] 
https://lore.kernel.org/qemu-devel/20200831082737.10983-2-jasowang@redhat.com/