qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Christian Schoenebeck" <qemu_oss@crudebyte.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>
Subject: Re: [PATCH] net: improve error message for missing netdev backend
Date: Thu, 27 Oct 2022 11:52:30 +0100	[thread overview]
Message-ID: <Y1pi7iAuehFGZc6w@redhat.com> (raw)
In-Reply-To: <20221003100612.596845-1-berrange@redhat.com>

ping: Jason, are you willing to queue this since it has two
positive reviews.

On Mon, Oct 03, 2022 at 11:06:12AM +0100, Daniel P. Berrangé wrote:
> The current message when using '-net user...' with SLIRP disabled at
> compile time is:
> 
>   qemu-system-x86_64: -net user: Parameter 'type' expects a net backend type (maybe it is not compiled into this binary)
> 
> An observation is that we're using the 'netdev->type' field here which
> is an enum value, produced after QAPI has converted from its string
> form.
> 
> IOW, at this point in the code, we know that the user's specified
> type name was a valid network backend. The only possible scenario that
> can make the backend init function be NULL, is if support for that
> backend was disabled at build time. Given this, we don't need to caveat
> our error message with a 'maybe' hint, we can be totally explicit.
> 
> The use of QERR_INVALID_PARAMETER_VALUE doesn't really lend itself to
> user friendly error message text. Since this is not used to set a
> specific QAPI error class, we can simply stop using this pre-formatted
> error text and provide something better.
> 
> Thus the new message is:
> 
>   qemu-system-x86_64: -net user: network backend 'user' is not compiled into this binary
> 
> The case of passing 'hubport' for -net is also given a message reminding
> people they should have used -netdev/-nic instead, as this backend type
> is only valid for the modern syntax.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> 
> NB, this does not make any difference to people who were relying on the
> QEMU built-in default hub that was created if you don't list any -net /
> -netdev / -nic argument, only those using explicit args.
> 
>  net/net.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/net/net.c b/net/net.c
> index 2db160e063..8ddafacf13 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1036,19 +1036,23 @@ static int net_client_init1(const Netdev *netdev, bool is_netdev, Error **errp)
>      if (is_netdev) {
>          if (netdev->type == NET_CLIENT_DRIVER_NIC ||
>              !net_client_init_fun[netdev->type]) {
> -            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
> -                       "a netdev backend type");
> +            error_setg(errp, "network backend '%s' is not compiled into this binary",
> +                       NetClientDriver_str(netdev->type));
>              return -1;
>          }
>      } else {
>          if (netdev->type == NET_CLIENT_DRIVER_NONE) {
>              return 0; /* nothing to do */
>          }
> -        if (netdev->type == NET_CLIENT_DRIVER_HUBPORT ||
> -            !net_client_init_fun[netdev->type]) {
> -            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
> -                       "a net backend type (maybe it is not compiled "
> -                       "into this binary)");
> +        if (netdev->type == NET_CLIENT_DRIVER_HUBPORT) {
> +            error_setg(errp, "network backend '%s' is only supported with -netdev/-nic",
> +                       NetClientDriver_str(netdev->type));
> +            return -1;
> +        }
> +
> +        if (!net_client_init_fun[netdev->type]) {
> +            error_setg(errp, "network backend '%s' is not compiled into this binary",
> +                       NetClientDriver_str(netdev->type));
>              return -1;
>          }
>  
> -- 
> 2.37.3
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  parent reply	other threads:[~2022-10-27 10:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-03 10:06 [PATCH] net: improve error message for missing netdev backend Daniel P. Berrangé
2022-10-03 10:13 ` Marc-André Lureau
2022-10-03 12:46 ` Christian Schoenebeck
2022-10-03 12:50   ` Daniel P. Berrangé
2022-10-03 15:00     ` Christian Schoenebeck
2022-10-04  7:23 ` Thomas Huth
2022-10-27 10:52 ` Daniel P. Berrangé [this message]
2022-10-28  1:59   ` Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y1pi7iAuehFGZc6w@redhat.com \
    --to=berrange@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu_oss@crudebyte.com \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).