qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Christian Schoenebeck <qemu_oss@crudebyte.com>
To: qemu-devel@nongnu.org
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>
Subject: Re: [PATCH] net: improve error message for missing netdev backend
Date: Mon, 03 Oct 2022 17:00:59 +0200	[thread overview]
Message-ID: <33977161.gUOJYfmdpQ@silver> (raw)
In-Reply-To: <YzrafHjJoFK/Orip@redhat.com>

On Montag, 3. Oktober 2022 14:50:04 CEST Daniel P. Berrangé wrote:
> On Mon, Oct 03, 2022 at 02:46:04PM +0200, Christian Schoenebeck wrote:
> > On Montag, 3. Oktober 2022 12:06:12 CEST 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)
> > 
> > Is this intended as alternative to Marc-André's previous patch?
> 
> This is a patch that should be applied regardless of any other change,
> because the error message we report here today is awful and needs
> improving.
> 
> >                                                                  If yes,
> >                                                                  then
> > 
> > same applies here: what about people not passing any networking arg to
> > QEMU? They would not get any error message at all, right?
> 
> Yes, I mentioned that in the text that you've quoted below....

Yeah, missed that one, sorry.

> > > 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
> > 
> > And why not naming the child, i.e. that QEMU was built without slirp?
> 
> There are several network backends that can be conditionally disabled
> at build time, and IMHO its overkill to give a different message for
> each one. This message is sufficient to show users where to go next.

Yes, but that is not a user friendly error message, especially for people who 
never dealt with QEMU's networking options before. That message does not make 
it obvious how to find the solution IMO.

What about a web link to the QEMU networking docs where this issue could then 
be clarified in a more user friendly manner? #anchors_are_cheap

> > > 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.
> 
> .... here.
> 
> 
> 
> With regards,
> Daniel




  reply	other threads:[~2022-10-03 15:04 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 [this message]
2022-10-04  7:23 ` Thomas Huth
2022-10-27 10:52 ` Daniel P. Berrangé
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=33977161.gUOJYfmdpQ@silver \
    --to=qemu_oss@crudebyte.com \
    --cc=berrange@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --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).