Commit Graph

29 Commits

Author SHA1 Message Date
Peter Maydell
a309b290aa Error reporting patches for 2017-07-13
-----BEGIN PGP SIGNATURE-----
 
 iQIcBAABAgAGBQJZZ1/BAAoJEDhwtADrkYZTo7oP+gLj4B4kkp/DJnkzfuMMD1Ce
 ZPddZ8Z9RyXE4fS66sq1ODBQo5U+aQQZO7K234+jf8V4cKWW98lpVzLc3YdAHm2U
 ZF6Z9Rji5K4414ZsUcg92Zlovvdaji+mY0ooINav+4mqlONYrz29ntApWc0e0tGc
 e3tj4XDLhJrOM+mIx8vzixFlgSYj+6HgEiybYwolEK5svQbIQao3Y2omyb+zy0w0
 RDT3XQnAAaZSOQAXcJGkhekkyMe0jMHOF0tULLx1uDQYctg9mUGlAGTZ5oTLgSve
 TCpSJwWCAx8XAJMkXyDRrdRFDLeUh6yGY7NTqAL3OuPSoAw9ygKrHyhTavxBJL+W
 rX7Qit3dmVrlZLviwNFQplAKYb10d08vBoKXmrnW5oVCmPEDvJIQfncbucpA/CNS
 ucdJ3RMLuDbbWdl+5tsL7jfiZAG7oSgAePTjN1rm0bDe5JN7NAU8WzHnKfE83iZq
 R+I3hofqGoiXSByYRLamZb+6nsURAxWPhcqcw7hdMsk7UI6dyZwWl9Fnm72w0BZK
 M5LHLkX0LYc+kZjiLKXlNK7Z50bXY0zKQpPCLH3nHA69iMiwVoozrjwa9iCKIxE+
 7ZlOfsu4ztExuicEyTr8b27CBrHjJjYDuFP0hroEOzqCKXUzegoq3oYMGP0doXxe
 o3xcwXVKT/1PudddyR4z
 =tByN
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2017-07-13' into staging

Error reporting patches for 2017-07-13

# gpg: Signature made Thu 13 Jul 2017 12:55:45 BST
# gpg:                using RSA key 0x3870B400EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"
# Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867  4E5F 3870 B400 EB91 8653

* remotes/armbru/tags/pull-error-2017-07-13:
  Convert error_report*_err() to warn_report*_err()
  error: Implement the warn and free Error functions
  char-socket: Report TCP socket waiting as information
  Convert error_report() to warn_report()
  error: Functions to report warnings and informational messages
  util/qemu-error: Rename error_print_loc() to be more generic
  websock: Don't try to set *errp directly
  block: Don't try to set *errp directly
  xilinx: Fix latent error handling bug

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-07-14 09:36:40 +01:00
Alistair Francis
e43ead1d0b error: Implement the warn and free Error functions
Implement warn_report_err() and warn_reportf_err() functions which
are the same as the error_report_err() and error_reportf_err()
functions except report a warning instead of an error.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <276ff93eadc0b01b8243cc61ffc331f77922c0d0.1499866456.git.alistair.francis@xilinx.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-07-13 13:50:19 +02:00
Daniel P. Berrange
c01c214b69 block: remove all encryption handling APIs
Now that all encryption keys must be provided upfront via
the QCryptoSecret API and associated block driver properties
there is no need for any explicit encryption handling APIs
in the block layer. Encryption can be handled transparently
within the block driver. We only retain an API for querying
whether an image is encrypted or not, since that is a
potentially useful piece of metadata to report to the user.

Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170623162419.26068-18-berrange@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11 17:44:56 +02:00
Sascha Silbe
98cb89af4d error: error_setg_errno(): errno gets preserved
C11 allows errno to be clobbered by pretty much any library function
call, so in general callers need to take care to save errno before
calling other functions.

However, for error reporting functions this is rather awkward and can
make the code on the caller side more complicated than
necessary. error_setg_errno() already takes care of preserving errno
and some functions rely on that, so just promise that we continue to
do so in the future.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Message-Id: <1469611466-31574-1-git-send-email-silbe@linux.vnet.ibm.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-01-19 15:42:36 +01:00
Daniel P. Berrange
d59ce6f344 migration: add reporting of errors for outgoing migration
Currently if an application initiates an outgoing migration,
it may or may not, get an error reported back on failure. If
the error occurs synchronously to the 'migrate' command
execution, the client app will see the error message. This
is the case for DNS lookup failures. If the error occurs
asynchronously to the monitor command though, the error
will be thrown away and the client left guessing about
what went wrong. This is the case for failure to connect
to the TCP server (eg due to wrong port, or firewall
rules, or other similar errors).

In the future we'll be adding more scope for errors to
happen asynchronously with the TLS protocol handshake.
TLS errors are hard to diagnose even when they are well
reported, so discarding errors entirely will make it
impossible to debug TLS connection problems.

Management apps which do migration are already using
'query-migrate' / 'info migrate' to check up on progress
of background migration operations and to see their end
status. This is a fine place to also include the error
message when things go wrong.

This patch thus adds an 'error-desc' field to the
MigrationInfo struct, which will be populated when
the 'status' is set to 'failed':

(qemu) migrate -d tcp:localhost:9001
(qemu) info migrate
capabilities: xbzrle: off rdma-pin-all: off auto-converge: off zero-blocks: off compress: off events: off x-postcopy-ram: off
Migration status: failed (Error connecting to socket: Connection refused)
total time: 0 milliseconds

In the HMP, when doing non-detached migration, it is
also possible to display this error message directly
to the app.

(qemu) migrate tcp:localhost:9001
Error connecting to socket: Connection refused

Or with QMP

  {
    "execute": "query-migrate",
    "arguments": {}
  }
  {
    "return": {
      "status": "failed",
      "error-desc": "address resolution failed for myhost:9000: No address associated with hostname"
    }
  }

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1461751518-12128-11-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
2016-05-26 11:31:30 +05:30
Markus Armbruster
da34e65cb4 include/qemu/osdep.h: Don't include qapi/error.h
Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the
Error typedef.  Since then, we've moved to include qemu/osdep.h
everywhere.  Its file comment explains: "To avoid getting into
possible circular include dependencies, this file should not include
any other QEMU headers, with the exceptions of config-host.h,
compiler.h, os-posix.h and os-win32.h, all of which are doing a
similar job to this file and are under similar constraints."
qapi/error.h doesn't do a similar job, and it doesn't adhere to
similar constraints: it includes qapi-types.h.  That's in excess of
100KiB of crap most .c files don't actually need.

Add the typedef to qemu/typedefs.h, and include that instead of
qapi/error.h.  Include qapi/error.h in .c files that need it and don't
get it now.  Include qapi-types.h in qom/object.h for uint16List.

Update scripts/clean-includes accordingly.  Update it further to match
reality: replace config.h by config-target.h, add sysemu/os-posix.h,
sysemu/os-win32.h.  Update the list of includes in the qemu/osdep.h
comment quoted above similarly.

This reduces the number of objects depending on qapi/error.h from "all
of them" to less than a third.  Unfortunately, the number depending on
qapi-types.h shrinks only a little.  More work is needed for that one.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
[Fix compilation without the spice devel packages. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-03-22 22:20:15 +01:00
Peter Maydell
90ce6e2644 include: Clean up includes
Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

NB: If this commit breaks compilation for your out-of-tree
patchseries or fork, then you need to make sure you add
#include "qemu/osdep.h" to any new .c files that you have.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
2016-02-23 12:43:05 +00:00
Markus Armbruster
10303f04b9 error: Improve documentation some more
Don't claim error_report_err() always reports to stderr.  It actually
reports to the current monitor when we have one.

Clarify intended use of error_abort and error_fatal.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1454522628-28294-2-git-send-email-armbru@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
2016-02-09 13:19:41 +01:00
Markus Armbruster
8277d2aa58 error: New error_prepend(), error_reportf_err()
Instead of simply propagating an error verbatim, we sometimes want to
add to its message, like this:

    frobnicate(arg, &err);
    error_setg(errp, "Can't frobnicate %s: %s",
                     arg, error_get_pretty(err));
    error_free(err);

This is suboptimal, because it loses err's hint (if any).  Moreover,
when errp is &error_abort or is subsequently propagated to
&error_abort, the abort message points to the place where we last
added to the error, not to the place where it originated.

To avoid these issues, provide means to add to an error's message in
place:

    frobnicate(arg, errp);
    error_prepend(errp, "Can't frobnicate %s: ", arg);

Likewise, reporting an error like

    frobnicate(arg, &err);
    error_report("Can't frobnicate %s: %s", arg, error_get_pretty(err));

can lose err's hint.  To avoid:

    error_reportf_err(err, "Can't frobnicate %s: ", arg);

The next commits will put these functions to use.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-10-git-send-email-armbru@redhat.com>
2016-01-13 15:16:17 +01:00
Markus Armbruster
f4d0064afc error: Improve documentation
While there, tighten error_append_hint()'s assertion.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1450452927-8346-6-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2016-01-13 15:16:16 +01:00
Markus Armbruster
8d780f4392 error: Document how to accumulate multiple errors
Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447776349-2344-1-git-send-email-armbru@redhat.com>
2016-01-13 11:58:57 +01:00
Eric Blake
d20a580bc0 qapi: Change munging of CamelCase enum values
When munging enum values, the fact that we were passing the entire
prefix + value through camel_to_upper() meant that enum values
spelled with CamelCase could be turned into CAMEL_CASE.  However,
this provides a potential collision (both OneTwo and One-Two would
munge into ONE_TWO) for enum types, when the same two names are
valid side-by-side as QAPI member names.  By changing the generation
of enum constants to always be prefix + '_' + c_name(value,
False).upper(), and ensuring that there are no case collisions (in
the next patches), we no longer have to worry about names that
would be distinct as QAPI members but collide as variant tag names,
without having to think about what munging the heuristics in
camel_to_upper() will actually perform on an enum value.

Making the change will affect enums that did not follow coding
conventions, using 'CamelCase' rather than desired 'lower-case'.

Thankfully, there are only two culprits: InputButton and ErrorClass.
We already tweaked ErrorClass to make it an alias of QapiErrorClass,
where only the alias needs changing rather than the whole tree.  So
the bulk of this change is modifying INPUT_BUTTON_WHEEL_UP to the
new INPUT_BUTTON_WHEELUP (and likewise for WHEELDOWN).  That part
of this commit may later need reverting if we rename the enum
constants from 'WheelUp' to 'wheel-up' as part of moving
x-input-send-event to a stable interface; but at least we have
documentation bread crumbs in place to remind us (commit 513e7cd),
and it matches the fact that SDL constants are also spelled
SDL_BUTTON_WHEELUP.

Suggested by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-27-git-send-email-eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17 08:21:28 +01:00
Eric Blake
f22a28b898 qapi: Add alias for ErrorClass
The qapi enum ErrorClass is unusual that it uses 'CamelCase' names,
contrary to our documented convention of preferring 'lower-case'.
However, this enum is entrenched in the API; we cannot change
what strings QMP outputs.  Meanwhile, we want to simplify how
c_enum_const() is used to generate enum constants, by moving away
from the heuristics of camel_to_upper() to a more straightforward
c_name(N).upper() - but doing so will rename all of the ErrorClass
constants and cause churn to all client files, where the new names
are aesthetically less pleasing (ERROR_CLASS_DEVICENOTFOUND looks
like we can't make up our minds on whether to break between words).

So as always in computer science, solve the problem by some more
indirection: rename the qapi type to QapiErrorClass, and add a
new enum ErrorClass in error.h whose members are aliases of the
qapi type, but with the spelling expected elsewhere in the tree.
Then, when c_enum_const() changes the munging, we only have to
adjust the one alias spot.

Suggested by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-26-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17 08:21:28 +01:00
Eric Blake
a12a5a1a01 qapi: Simplify error cleanup in test-qmp-*
We have several tests that perform multiple sub-actions that are
expected to fail.  Asserting that an error occurred, then clearing
it up to prepare for the next action, turned into enough
boilerplate that it was sometimes forgotten (for example, a number
of tests added to test-qmp-input-visitor.c in d88f5fd leaked err).
Worse, if an error is not reset to NULL, we risk invalidating
later use of that error (passing a non-NULL err into a function
is generally a bad idea).  Encapsulate the boilerplate into a
single helper function error_free_or_abort(), and consistently
use it.

The new function is added into error.c for use everywhere,
although it is anticipated that testsuites will be the main
client.

Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-11-10 08:08:21 +01:00
Markus Armbruster
a29a37b994 error: New error_fatal
Similar to error_abort, but doesn't report where the error was
created, and terminates the process with exit(1) rather than abort().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1441983105-26376-2-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
2015-09-18 14:38:08 +02:00
Eric Blake
50b7b000c9 hmp: Allow for error message hints on HMP
Commits 7216ae3d and d2828429 disabled some error message hints,
all because a change to use modern error reporting meant that the
hint would be output prior to the actual error.  Fix this by making
hints a first-class member of Error.

For example, we are now back to the pleasant:

 $ qemu-system-x86_64 --nodefaults -S --vnc :0 --chardev null,id=,
 qemu-system-x86_64: --chardev null,id=,: Parameter 'id' expects an identifier
 Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <1441901956-21991-1-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-09-18 14:34:39 +02:00
Markus Armbruster
1e9b65bb1b error: On abort, report where the error was created
This is particularly useful when we abort in error_propagate(),
because there the stack backtrace doesn't lead to where the error was
created.  Looks like this:

    Unexpected error in parse_block_error_action() at .../qemu/blockdev.c:322:
    qemu-system-x86_64: -drive if=none,werror=foo: 'foo' invalid write error action
    Aborted (core dumped)

Note: to get this example output, I monkey-patched drive_new() to pass
&error_abort to blockdev_init().

To keep the error handling boiler plate from growing even more, all
error_setFOO() become macros expanding into error_setFOO_internal()
with additional __FILE__, __LINE__, __func__ arguments.  Not exactly
pretty, but it works.

The macro trickery breaks down when you take the address of an
error_setFOO().  Fortunately, we do that in just one place: qemu-ga's
Windows VSS provider and requester DLL wants to call
error_setg_win32() through a function pointer "to avoid linking glib
to the DLL".  Use error_setg_win32_internal() there.  The use of the
function pointer is already wrapped in a macro, so the churn isn't
bad.

Code size increases by some 35KiB for me (0.7%).  Tolerable.  Could be
less if we passed relative rather than absolute source file names to
the compiler, or forwent reporting __func__.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
2015-09-10 13:48:06 +02:00
Markus Armbruster
edf6f3b335 error: Revamp interface documentation
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2015-09-10 13:48:06 +02:00
Markus Armbruster
4463dcb85c error: error_set_errno() is unused, drop
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2015-09-10 13:48:06 +02:00
Markus Armbruster
e7cf59e847 qga: Clean up unnecessarily dirty casts
qga_vss_fsfreeze() casts error_set_win32() from

    void (*)(Error **, int, ErrorClass, const char *, ...)

to

    void (*)(void **, int, int, const char *, ...)

The result is later called.  Since the two types are not compatible,
the call is undefined behavior.  It works in practice anyway.

However, there's no real need for trickery here.  Clean it up as
follows:

* Declare struct Error, and fix the first parameter.

* Switch to error_setg_win32().  This gets rid of the troublesome
  ErrorClass parameter.  Requires converting error_setg_win32() from
  macro to function, but that's trivially easy, because this is the
  only user of error_set_win32().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2015-09-10 13:48:06 +02:00
Markus Armbruster
a9499ddd82 error: Make error_setg() a function
Saves a tiny amount of code at every call site.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2015-09-10 13:48:05 +02:00
Markus Armbruster
2ee2f1e415 error: New convenience function error_report_err()
I've typed error_report("%s", error_get_pretty(ERR)) too many times
already, and I've fixed too many instances of qerror_report_err(ERR)
to error_report("%s", error_get_pretty(ERR)) as well.  Capture the
pattern in a convenience function.

Since it's almost invariably followed by error_free(), stuff that into
the convenience function as well.

The next patch will put it to use.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2015-02-18 10:50:43 +01:00
Markus Armbruster
d2e064a73e error: error_is_set() is finally unused; remove
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2014-05-21 11:57:58 +02:00
Markus Armbruster
64dfefed16 error: Consistently name Error ** objects errp, and not err
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
2014-05-09 09:11:30 -04:00
Peter Crosthwaite
5d24ee70bc error: Add error_abort
Add a special Error * that can be passed to error handling APIs to
signal that any errors are fatal and should abort QEMU. There are two
advantages to this:

- allows for brevity when wishing to assert success of Error **
  accepting APIs. No need for this pattern:
        Error * local_err = NULL;
        api_call(foo, bar, &local_err);
        assert_no_error(local_err);
  This also removes the need for _nofail variants of APIs with
  asserting call sites now reduced to 1LOC.
- SIGABRT happens from within the offending API. When a fatal error
  occurs in an API call (when the caller is asserting sucess) failure
  often means the API itself is broken. With the abort happening in the
  API call now, the stack frames into the call are available at debug
  time. In the assert_no_error scheme the abort happens after the fact.

The exact semantic is that when an error is raised, if the argument
Error ** matches &error_abort, then the abort occurs immediately. The
error messaged is reported.

For error_propagate, if the destination error is &error_abort, then
the abort happens at propagation time.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
2014-01-06 14:01:53 -05:00
Tomoki Sekiyama
20840d4cfe error: Add error_set_win32 and error_setg_win32
These functions help maintaining homogeneous formatting of error messages
with Windows error code and description (generated by
g_win32_error_message()).

Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
2013-09-09 14:17:57 -05:00
Luiz Capitulino
54028d7542 error: add error_setg_file_open() helper
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
2013-06-17 11:01:14 -04:00
Paolo Bonzini
1de7afc984 misc: move include files to include/qemu/
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2012-12-19 08:32:39 +01:00
Paolo Bonzini
7b1b5d1913 qapi: move include files to include/qobject/
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2012-12-19 08:31:31 +01:00