All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony PERARD <anthony.perard@citrix.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: xen-devel@lists.xenproject.org,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Wei Liu <wei.liu2@citrix.com>
Subject: Re: [PATCH v4 22/32] libxl_qmp: Handle messages from QEMU
Date: Mon, 6 Aug 2018 14:14:51 +0100	[thread overview]
Message-ID: <20180806131451.GM1327@perard.uk.xensource.com> (raw)
In-Reply-To: <20180806112522.tivasi6rlucnruks@mac>

On Mon, Aug 06, 2018 at 01:25:22PM +0200, Roger Pau Monné wrote:
> On Fri, Aug 03, 2018 at 06:25:00PM +0100, Anthony PERARD wrote:
> > On Thu, Aug 02, 2018 at 01:17:37PM +0200, Roger Pau Monné wrote:
> > > On Fri, Jul 27, 2018 at 03:06:04PM +0100, Anthony PERARD wrote:
> > > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > > > index 4a385801ba..e104fea941 100644
> > > > --- a/tools/libxl/libxl_types.idl
> > > > +++ b/tools/libxl/libxl_types.idl
> > > > @@ -69,6 +69,10 @@ libxl_error = Enumeration("error", [
> > > >      (-23, "NOTFOUND"),
> > > >      (-24, "DOMAIN_DESTROYED"), # Target domain ceased to exist during op
> > > >      (-25, "FEATURE_REMOVED"), # For functionality that has been removed
> > > > +    (-26, "QMP_GENERIC_ERROR"), # unspecified qmp error
> > > > +    (-27, "QMP_COMMAND_NOT_FOUND"), # the requested command has not been found
> > > > +    (-28, "QMP_DEVICE_NOT_ACTIVE"), # a device has failed to be become active
> > > > +    (-29, "QMP_DEVICE_NOT_FOUND"), # the requested device has not been found
> > > 
> > > Do we really need such granularity for QMP errors? Isn't it enough to
> > > have a single ERROR_QMP or similar?
> > 
> > No I don't think so. QMP_COMMAND_NOT_FOUND can be useful when a qmp
> > command is removed (there is already one that is deprecated and we use).
> > 
> > The last two could be useful to user of libxl as they could provide
> > better error messages. xl don't care because whatever error message is
> > attach to the error, it will be printed.
> 
> I wonder if this set of errors will change very often, at the end of
> day it's not controlled by ourselves but QEMU, and then how much
> chasing will libxl have to do in order to keep up with new types of
> QMP errors.

libxl doesn't have to know all the possible error code. Whenever there's
a code that is not known, I simple return ERROR_FAIL to the caller. So
it doesn't matter how often new error codes are added. (Also, I've
already left out one error code which is KVM specific.)

> IMO I don't see a lot of value in returning specific QMP error codes
> from libxl, and I would even consider whether it's worth to have
> something like ERROR_QMP, because I don't think there's much the
> caller can do in case of QMP failure. It's an internal libxl QMP
> error, and it's not like the caller is going to start it's own QMP
> connection in order to speak with QEMU directly in order to solve
> whatever QMP issue it got from libxl.

Those error codes aren't "internal libxl QMP error", the QMP connection
is working perfectly fine. All those error comes from QEMU.

All those error codes are part of possible responses to a command.

Here are the possible way that a caller can solve an issue with each
error code:

GenericError:
  The command failed, there's a description that a Human can read. Not
  much else can be done.

CommandNotFound:
  Will, the QMP command that the caller is trying to use is unknown to
  the current QEMU, it's maybe a command introduced in newer version of
  QEMU, or a deprecated command that is now removed.
  Maybe the caller try to use a newer command that replace a deprecated
  one, on error, it could call the older version. And vice versa.

DeviceNotActive:
  The device that libxl is trying to add via the command wasn't bring up
  properly, the caller could try again with different parameters or
  inverstigate with "query-*" commands to find out what's missing.

DeviceNotFound:
  The device that the caller is trying to modify (like a cdrom drive) is
  not found in QEMU, the caller could try to add it or let the user know
  that the device doesn't exist.


None of those error code are QMP issue.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply	other threads:[~2018-08-06 13:14 UTC|newest]

Thread overview: 96+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-27 14:05 [PATCH v4 00/32] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
2018-07-27 14:05 ` [PATCH v4 01/32] libxl_event: Fix DEBUG prints Anthony PERARD
2018-07-27 14:05 ` [PATCH v4 02/32] libxl_qmp: Documentation of the logic of the QMP client Anthony PERARD
2018-07-27 14:05 ` [PATCH v4 03/32] libxl_qmp: Fix use of DEBUG_RECEIVED Anthony PERARD
2018-07-27 14:05 ` [PATCH v4 04/32] libxl_json: fix build with DEBUG_ANSWER Anthony PERARD
2018-07-27 14:05 ` [PATCH v4 05/32] libxl_qmp: Move the buffer realloc to the same scope level as read Anthony PERARD
2018-07-27 14:05 ` [PATCH v4 06/32] libxl_qmp: Add a warning to not trust QEMU Anthony PERARD
2018-07-27 14:05 ` [PATCH v4 07/32] libxl_qmp: Move struct sockaddr_un variable to qmp_open() Anthony PERARD
2018-08-02  8:26   ` Roger Pau Monné
2018-08-20 14:52   ` Wei Liu
2018-07-27 14:05 ` [PATCH v4 08/32] libxl: Add libxl__prepare_sockaddr_un() helper Anthony PERARD
2018-08-02  8:36   ` Roger Pau Monné
2018-08-20 14:56   ` Wei Liu
2018-07-27 14:05 ` [PATCH v4 09/32] libxl_qmp: Remove unused yajl_ctx from handler Anthony PERARD
2018-08-20 14:56   ` Wei Liu
2018-07-27 14:05 ` [PATCH v4 10/32] libxl_json: constify libxl__json_object_to_yajl_gen arguments Anthony PERARD
2018-07-27 14:05 ` [PATCH v4 11/32] libxl_dm: Add libxl__qemu_qmp_path() Anthony PERARD
2018-08-02  8:41   ` Roger Pau Monné
2018-08-20 14:57   ` Wei Liu
2018-07-27 14:05 ` [PATCH v4 12/32] libxl: Design of an async API to issue QMP commands to QEMU Anthony PERARD
2018-08-02  9:01   ` [PATCH v4 12/32] libxl: Design of an async API to issue QMP commands to QEMUç Roger Pau Monné
2018-08-03 11:18     ` Anthony PERARD
2018-08-03 13:55       ` Roger Pau Monné
2018-08-03 14:45         ` Anthony PERARD
2018-08-03 15:30           ` Roger Pau Monné
2018-07-27 14:05 ` [PATCH v4 13/32] libxl_qmp: Connect to QMP socket Anthony PERARD
2018-08-02  9:35   ` Roger Pau Monné
2018-08-03 13:54     ` Anthony PERARD
2018-08-03 14:03       ` Roger Pau Monné
2018-08-21  8:22         ` Wei Liu
2018-07-27 14:05 ` [PATCH v4 14/32] libxl_qmp: Implement fd callback and read data Anthony PERARD
2018-08-02  9:56   ` Roger Pau Monné
2018-08-03 14:32     ` Anthony PERARD
2018-08-03 15:24       ` Roger Pau Monné
2018-08-06 15:01         ` Anthony PERARD
2018-07-27 14:05 ` [PATCH v4 15/32] libxl_json: Enable yajl_allow_trailing_garbage Anthony PERARD
2018-08-02 10:01   ` Roger Pau Monné
2018-08-21  8:26     ` Wei Liu
2018-07-27 14:05 ` [PATCH v4 16/32] libxl_json: libxl__json_object_to_json Anthony PERARD
2018-08-02 10:10   ` Roger Pau Monné
2018-08-21  8:26   ` Wei Liu
2018-07-27 14:05 ` [PATCH v4 17/32] libxl_qmp: Parse JSON input from QMP Anthony PERARD
2018-08-02 10:25   ` Roger Pau Monné
2018-08-03 15:33     ` Anthony PERARD
2018-08-06 11:17       ` Roger Pau Monné
2018-07-27 14:06 ` [PATCH v4 18/32] libxl_qmp: Separate QMP message generation from qmp_send_prepare Anthony PERARD
2018-08-02 10:34   ` Roger Pau Monné
2018-08-03 15:43     ` Anthony PERARD
2018-07-27 14:06 ` [PATCH v4 19/32] libxl_qmp: Prepare the command to be sent Anthony PERARD
2018-08-02 10:41   ` Roger Pau Monné
2018-08-03 16:35     ` Anthony PERARD
2018-07-27 14:06 ` [PATCH v4 20/32] libxl_qmp: Handle write to QMP socket Anthony PERARD
2018-08-02 11:02   ` Roger Pau Monné
2018-08-03 16:50     ` Anthony PERARD
2018-07-27 14:06 ` [PATCH v4 21/32] libxl_qmp: Simplify qmp_response_type() prototype Anthony PERARD
2018-08-02 11:03   ` Roger Pau Monné
2018-08-21  8:53   ` Wei Liu
2018-07-27 14:06 ` [PATCH v4 22/32] libxl_qmp: Handle messages from QEMU Anthony PERARD
2018-08-02 11:17   ` Roger Pau Monné
2018-08-03 17:25     ` Anthony PERARD
2018-08-06 11:25       ` Roger Pau Monné
2018-08-06 13:14         ` Anthony PERARD [this message]
2018-08-06 15:03           ` Roger Pau Monné
2018-08-21  8:58       ` Wei Liu
2018-08-21 12:50         ` Anthony PERARD
2018-08-21 14:56           ` Ian Jackson
2018-07-27 14:06 ` [PATCH v4 23/32] libxl_qmp: Respond to QMP greeting Anthony PERARD
2018-08-02 11:26   ` Roger Pau Monné
2018-08-06 17:41     ` Anthony PERARD
2018-08-21  9:00   ` Wei Liu
2018-08-21 10:51     ` Anthony PERARD
2018-07-27 14:06 ` [PATCH v4 24/32] libxl_qmp: Disable beautify for QMP generated cmd Anthony PERARD
2018-08-21  9:00   ` Wei Liu
2018-07-27 14:06 ` [PATCH v4 25/32] libxl_exec: Add libxl__spawn_initiate_failure Anthony PERARD
2018-08-02 11:34   ` Roger Pau Monné
2018-08-06 15:58     ` Anthony PERARD
2018-07-27 14:06 ` [PATCH v4 26/32] libxl_dm: Pre-open QMP socket for QEMU Anthony PERARD
2018-08-02 15:00   ` Roger Pau Monné
2018-07-27 14:06 ` [PATCH v4 27/32] libxl: QEMU startup sync based on QMP Anthony PERARD
2018-08-02 15:06   ` Roger Pau Monné
2018-07-27 14:06 ` [PATCH v4 28/32] libxl_qmp: Store advertised QEMU version in libxl__ev_qmp Anthony PERARD
2018-08-02 15:08   ` Roger Pau Monné
2018-08-06 16:52     ` Anthony PERARD
2018-07-27 14:06 ` [PATCH v4 29/32] libxl: Change libxl__domain_suspend_device_model() to be async Anthony PERARD
2018-08-02 15:15   ` Roger Pau Monné
2018-07-27 14:06 ` [PATCH v4 30/32] libxl: Re-implement domain_suspend_device_model using libxl__ev_qmp Anthony PERARD
2018-08-02 15:38   ` Roger Pau Monné
2018-08-06 17:07     ` Anthony PERARD
2018-07-27 14:06 ` [PATCH v4 31/32] libxl_disk: Cut libxl_cdrom_insert into step Anthony PERARD
2018-08-02 15:50   ` Roger Pau Monné
2018-08-06 17:20     ` Anthony PERARD
2018-08-07 14:18       ` Roger Pau Monné
2018-08-07 14:40         ` Anthony PERARD
2018-08-21  9:08           ` Wei Liu
2018-08-21 12:58             ` Anthony PERARD
2018-07-27 14:06 ` [PATCH v4 32/32] libxl_disk: Have libxl_cdrom_insert use libxl__ev_qmp Anthony PERARD

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=20180806131451.GM1327@perard.uk.xensource.com \
    --to=anthony.perard@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.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.