[PATCH v2 03/15] net/vhost-vdpa: Fix device compatibility check

Kevin Wolf posted 15 patches 1 month, 2 weeks ago

[PATCH v2 03/15] net/vhost-vdpa: Fix device compatibility check

Posted by Kevin Wolf 1 month, 2 weeks ago
vhost-vdpa works only with specific devices. At startup, it second
guesses what the command line option handling will do and error out if
it thinks a non-virtio device will attach to them.

This second guessing is not only ugly, it can lead to wrong error
messages ('-device floppy,netdev=foo' should complain about an unknown
property, not about the wrong kind of network device being attached) and
completely ignores hotplugging.

Drop the old checks and implement .check_peer_type() instead to fix
this. As a nice side effect, it also removes one more dependency on the
legacy QemuOpts infrastructure and even reduces the code size.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 net/vhost-vdpa.c | 37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 912686457c..6dc68d8677 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -147,12 +147,26 @@ static bool vhost_vdpa_has_ufo(NetClientState *nc)
 
 }
 
+static bool vhost_vdpa_check_peer_type(NetClientState *nc, ObjectClass *oc,
+                                       Error **errp)
+{
+    const char *driver = object_class_get_name(oc);
+
+    if (!g_str_has_prefix(driver, "virtio-net-")) {
+        error_setg(errp, "vhost-vdpa requires frontend driver virtio-net-*");
+        return false;
+    }
+
+    return true;
+}
+
 static NetClientInfo net_vhost_vdpa_info = {
         .type = NET_CLIENT_DRIVER_VHOST_VDPA,
         .size = sizeof(VhostVDPAState),
         .cleanup = vhost_vdpa_cleanup,
         .has_vnet_hdr = vhost_vdpa_has_vnet_hdr,
         .has_ufo = vhost_vdpa_has_ufo,
+        .check_peer_type = vhost_vdpa_check_peer_type,
 };
 
 static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
@@ -179,24 +193,6 @@ static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
     return ret;
 }
 
-static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
-{
-    const char *name = opaque;
-    const char *driver, *netdev;
-
-    driver = qemu_opt_get(opts, "driver");
-    netdev = qemu_opt_get(opts, "netdev");
-    if (!driver || !netdev) {
-        return 0;
-    }
-    if (strcmp(netdev, name) == 0 &&
-        !g_str_has_prefix(driver, "virtio-net-")) {
-        error_setg(errp, "vhost-vdpa requires frontend driver virtio-net-*");
-        return -1;
-    }
-    return 0;
-}
-
 int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
                         NetClientState *peer, Error **errp)
 {
@@ -204,10 +200,5 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
 
     assert(netdev->type == NET_CLIENT_DRIVER_VHOST_VDPA);
     opts = &netdev->u.vhost_vdpa;
-    /* verify net frontend */
-    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
-                          (char *)name, errp)) {
-        return -1;
-    }
     return net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name, opts->vhostdev);
 }
-- 
2.31.1

Re: [PATCH v2 03/15] net/vhost-vdpa: Fix device compatibility check

Posted by Jason Wang 1 month, 1 week ago
在 2021/10/8 下午9:34, Kevin Wolf 写道:
> vhost-vdpa works only with specific devices. At startup, it second
> guesses what the command line option handling will do and error out if
> it thinks a non-virtio device will attach to them.
>
> This second guessing is not only ugly, it can lead to wrong error
> messages ('-device floppy,netdev=foo' should complain about an unknown
> property, not about the wrong kind of network device being attached) and
> completely ignores hotplugging.
>
> Drop the old checks and implement .check_peer_type() instead to fix
> this. As a nice side effect, it also removes one more dependency on the
> legacy QemuOpts infrastructure and even reduces the code size.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   net/vhost-vdpa.c | 37 ++++++++++++++-----------------------
>   1 file changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 912686457c..6dc68d8677 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -147,12 +147,26 @@ static bool vhost_vdpa_has_ufo(NetClientState *nc)
>   
>   }
>   
> +static bool vhost_vdpa_check_peer_type(NetClientState *nc, ObjectClass *oc,
> +                                       Error **errp)
> +{
> +    const char *driver = object_class_get_name(oc);
> +
> +    if (!g_str_has_prefix(driver, "virtio-net-")) {
> +        error_setg(errp, "vhost-vdpa requires frontend driver virtio-net-*");
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>   static NetClientInfo net_vhost_vdpa_info = {
>           .type = NET_CLIENT_DRIVER_VHOST_VDPA,
>           .size = sizeof(VhostVDPAState),
>           .cleanup = vhost_vdpa_cleanup,
>           .has_vnet_hdr = vhost_vdpa_has_vnet_hdr,
>           .has_ufo = vhost_vdpa_has_ufo,
> +        .check_peer_type = vhost_vdpa_check_peer_type,
>   };
>   
>   static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
> @@ -179,24 +193,6 @@ static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
>       return ret;
>   }
>   
> -static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
> -{
> -    const char *name = opaque;
> -    const char *driver, *netdev;
> -
> -    driver = qemu_opt_get(opts, "driver");
> -    netdev = qemu_opt_get(opts, "netdev");
> -    if (!driver || !netdev) {
> -        return 0;
> -    }
> -    if (strcmp(netdev, name) == 0 &&
> -        !g_str_has_prefix(driver, "virtio-net-")) {
> -        error_setg(errp, "vhost-vdpa requires frontend driver virtio-net-*");
> -        return -1;
> -    }
> -    return 0;
> -}
> -
>   int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
>                           NetClientState *peer, Error **errp)
>   {
> @@ -204,10 +200,5 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
>   
>       assert(netdev->type == NET_CLIENT_DRIVER_VHOST_VDPA);
>       opts = &netdev->u.vhost_vdpa;
> -    /* verify net frontend */
> -    if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
> -                          (char *)name, errp)) {
> -        return -1;
> -    }
>       return net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name, opts->vhostdev);
>   }

Re: [PATCH v2 03/15] net/vhost-vdpa: Fix device compatibility check

Posted by Damien Hedde 1 month, 2 weeks ago

On 10/8/21 15:34, Kevin Wolf wrote:
> vhost-vdpa works only with specific devices. At startup, it second
> guesses what the command line option handling will do and error out if
> it thinks a non-virtio device will attach to them.
> 
> This second guessing is not only ugly, it can lead to wrong error
> messages ('-device floppy,netdev=foo' should complain about an unknown
> property, not about the wrong kind of network device being attached) and
> completely ignores hotplugging.
> 
> Drop the old checks and implement .check_peer_type() instead to fix
> this. As a nice side effect, it also removes one more dependency on the
> legacy QemuOpts infrastructure and even reduces the code size.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>