From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Fam Zheng <famz@redhat.com>, Kevin Wolf <kwolf@redhat.com>,
qemu-devel@nongnu.org, qemu-block@nongnu.org,
Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] file-posix: Consolidate the locking error message
Date: Thu, 7 Jun 2018 14:22:31 +0100 [thread overview]
Message-ID: <20180607132231.GR28827@redhat.com> (raw)
In-Reply-To: <87602uwx5z.fsf@dusky.pond.sub.org>
On Thu, Jun 07, 2018 at 03:20:24PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > On Fri, Jun 01, 2018 at 05:18:35PM +0800, Fam Zheng wrote:
> >> When hot-plugging a block device fails due to image locking errors,
> >> users won't see the helpful 'Is another process using the image?'
> >> message in QMP because currently the error hint is not carried over
> >> there.
> >>
> >> Even though extending QMP to include hint is a conceivably easy task,
> >> Libvirt will need some change to consume that data.
> >>
> >> Before that is fully sorted out, let's just do the easy fix by joining
> >> the two lines.
> >
> > There are many places in QEMU which uses error hints and these are all
> > invisible to libvirt. Arbitrarily picking one hint to remove, while
> > leaving everything else unfixed is not a very satisfactory approach.
> >
> > If QEMU passes the hint in QMP, it is trivial for libvirt to be changed
> > to append the hint when reporting its own error message, so can we just
> > focus on fixing the root cause instead of special casing file-posix.c
>
> Intended use of hints according to error.h:
>
> * Intended use is adding helpful hints on the human user interface,
> * e.g. a list of valid values. It's not for clarifying a confusing
> * error message.
>
> I admit this guidance is widely ignored.
Perhaps if we rename the function "error_append_hmp_hint" it would
make it obvious this is targetted at CLI users, and not QMP users,
and so encourage people to write better initial messages ?
>
> When used as intended, the hints need not make any sense in QMP!
> Consider this example in qemu-option.c:
>
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> "a non-negative number below 2^64");
> error_append_hint(errp, "Optional suffix k, M, G, T, P or E means"
> " kilo-, mega-, giga-, tera-, peta-\n"
> "and exabytes, respectively.\n");
>
> The suffixes are only available in the human interface.
>
> Aside: we have lots of code consuming input from both QMP and HMP / CLI.
> The error reporting is generally atrocious for at least one of the two.
>
> Perhaps we could use separate functions for providing syntax hints and
> for clarifying confusing error messages. Patches welcome, but they
> better convert all existing uses.
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 :|
prev parent reply other threads:[~2018-06-07 13:22 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-01 9:18 [Qemu-devel] [PATCH] file-posix: Consolidate the locking error message Fam Zheng
2018-06-01 12:32 ` Eric Blake
2018-06-01 13:38 ` Fam Zheng
2018-06-01 12:43 ` Daniel P. Berrangé
2018-06-01 13:33 ` Fam Zheng
2018-06-01 14:00 ` Daniel P. Berrangé
2018-06-01 14:17 ` Fam Zheng
2018-06-07 13:20 ` Markus Armbruster
2018-06-07 13:22 ` Daniel P. Berrangé [this message]
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=20180607132231.GR28827@redhat.com \
--to=berrange@redhat.com \
--cc=armbru@redhat.com \
--cc=famz@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.