[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] net: improve error message for missing netdev backend
From: |
Jason Wang |
Subject: |
Re: [PATCH] net: improve error message for missing netdev backend |
Date: |
Fri, 28 Oct 2022 09:59:22 +0800 |
On Thu, Oct 27, 2022 at 6:52 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> ping: Jason, are you willing to queue this since it has two
> positive reviews.
>
Yes, I've queued this.
Thanks
> 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 :|
>