Trying to squeeze the validator output into the monitor reply message
doesn't make sense and doesn't work well as it's not well formed JSON:
54) qemuMonitorJSONAddNetdev ... libvirt: error : internal error: cannot parse json { "error": { "desc": "failed to validate arguments of 'netdev_add' against QAPI schema: {
ERROR: variant 'test' for discriminator 'type' not found
", "class": "UnexpectedCommand" } }: lexical error: invalid character inside string.
ev_add' against QAPI schema: { ERROR: variant 'test' for
(right here) ------^
FAILED
Output it to stderr if requested and just note that schema validation
failed in the error message:
54) qemuMonitorJSONAddNetdev ...
failed to validate arguments of 'netdev_add' against QAPI schema
args:
{
"id": "net0",
"type": "test"
}
validator output:
{
ERROR: variant 'test' for discriminator 'type' not found
libvirt: QEMU Driver error : internal error: unable to execute QEMU command 'netdev_add': failed to validate arguments of 'netdev_add' against QAPI schema (to see debug output use VIR_TEST_DEBUG=2)
FAILED
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Sync was introduced in [1] to check for ga presence. This
check is racy but in the era before serial events are available
there was not better solution I guess.
In case we have the events the sync function is different. It allows us
to flush stateless ga channel from remnants of previous communications.
But we need to do it only once. Until we get timeout on issued command
channel state is ok.
[1] qemu_agent: Issue guest-sync prior to every command
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This converts the QEMU agent APIs to use the per-VM
event loop, which involves switching from virEvent APIs
to GMainContext / GSource APIs.
A GSocket is used as a convenient way to create a GSource
for a socket, but is not yet used for actual I/O.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This converts the QEMU monitor APIs to use the per-VM
event loop, which involves switching from virEvent APIs
to GMainContext / GSource APIs.
A GSocket is used as a convenient way to create a GSource
for a socket, but is not yet used for actual I/O.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Tests which are using the QEMU monitor / agent need to have an
event thread running a private GMainContext.
There is already a thread running the main libvirt event loop
but this can't be eliminated yet as it is used for more than
just the monitor client I/O.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Make it obvious that the function always returns a valid pointer and fix
all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The function now does not return an error so we can drop it fully.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Replace all occurrences of
if (VIR_STRDUP(a, b) < 0)
/* effectively dead code */
with:
a = g_strdup(b);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit 44e7f02915
util: rewrite auto cleanup macros to use glib's equivalent
VIR_AUTOPTR aliases to g_autoptr. Replace all of its use by the GLib
macro version.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit 44e7f02915
util: rewrite auto cleanup macros to use glib's equivalent
VIR_AUTOFREE is just an alias for g_autofree. Use the GLib macros
directly instead of our custom aliases.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit 44e7f02915
util: rewrite auto cleanup macros to use glib's equivalent
VIR_AUTOCLEAN is just an alias for g_auto. Use the GLib macros
directly instead of our custom aliases.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
g_strerror is offers the safety/correctness benefits of strerror_r, with
the API design convenience of strerror.
Use of virStrerror should be eliminated through the codebase in favour
of g_strerror.
commandhelper.c is a special case as its a tiny single threaded test
program, not linked to glib, so it just uses traditional strerror().
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When connecting to qemu's monitor the @vm object is unlocked.
This is justified - connecting may take a long time and we don't
want to wait with the domain object locked. However, just before
the domain object is locked again, the monitor's FD is registered
in the event loop. Therefore, there is a small window where the
event loop has a chance to call a handler for an event that
occurred on the monitor FD but vm is not initalized properly just
yet (i.e. priv->mon is not set). For instance, if there's an
incoming migration, qemu creates its socket but then fails to
initialize (for various reasons, I'm reproducing this by using
hugepages but leaving the HP pool empty) then the following may
happen:
1) qemuConnectMonitor() unlocks @vm
2) qemuMonitorOpen() connects to the monitor socket and by
calling qemuMonitorOpenInternal() which subsequently calls
qemuMonitorRegister() the event handler is installed
3) qemu fails to initialize and exit()-s, which closes the
monitor
4) The even loop sees EOF on the monitor and the control gets to
qemuProcessEventHandler() which locks @vm and calls
processMonitorEOFEvent() which then calls
qemuMonitorLastError(priv->mon). But priv->mon is not set just
yet.
5) qemuMonitorLastError() dereferences NULL pointer
The solution is to unlock the domain object for a shorter time
and most importantly, register event handler with domain object
locked so that any possible event processing is done only after
@vm's private data was properly initialized.
This issue is also mentioned in v4.2.0-99-ga5a777a8ba.
Since we are unlocking @vm and locking it back, another thread
might have destroyed the domain meanwhile. Therefore we have to
check if domain is still active, and we have to do it at the
same place where domain lock is acquired back, i.e. in
qemuMonitorOpen(). This creates a small problem for our test
suite which calls qemuMonitorOpen() directly and passes @vm which
has no definition. This makes virDomainObjIsActive() call crash.
Fortunately, allocating empty domain definition is sufficient.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
We don't need to escape the commands any more since we use QMP
passthrough, which means we can delete the functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Always assume JSON monitor was requested, since all the callers
pass true anyway.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
Now that we no longer support testing HMP monitor,
the json field is pointless.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
We return success when running this function for either non-JSON monitor
testing or guest agent testing.
However we no longer test HMP monitor and we do not try to validate
the guest agent interaction.
Drop the test->json check and report a proper error if someone tries
to run this function for the guest agent without properly wiring it up.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The QMP monitor only uses a newline to separate lines,
while HMP and the guest agent also use a carriage return.
In preparation to dropping support for testing HMP interaction,
only skip the carriage return if we're dealing with the guest agent,
removing the need to check the 'json' field.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
There is no obvious benefit in putting the escaped message
back into msg while tmp holds the original message.
Remove the assignment and use 'tmp' directly'.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Now that all the callers call qemuMonitorTestNew with json=true,
remove the argument and always assume JSON.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
The only user of the qemuMonitorTestNewSimple macro is using JSON.
Always pass 'true' to qemuMonitorTestNew and remove the 'json'
argument.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Pass in the schema data from the caller if QMP schema testing is
desired.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
In case when we are testing a QMP command we can try to schema check it
so that we catch inconsistencies.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Require that all headers are guarded by a symbol named
LIBVIRT_$FILENAME
where $FILENAME is the uppercased filename, with all characters
outside a-z changed into '_'.
Note we do not use a leading __ because that is technically a
namespace reserved for the toolchain.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This introduces a syntax-check script that validates header files use a
common layout:
/*
...copyright header...
*/
<one blank line>
#ifndef SYMBOL
# define SYMBOL
....content....
#endif /* SYMBOL */
For any file ending priv.h, before the #ifndef, we will require a
guard to prevent bogus imports:
#ifndef SYMBOL_ALLOW
# error ....
#endif /* SYMBOL_ALLOW */
<one blank line>
The many mistakes this script identifies are then fixed.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
All of the ones being removed are pulled in by internal.h. The only
exception is sanlock which expects the application to include <stdint.h>
before sanlock's headers, because sanlock prototypes use fixed width
int, but they don't include stdint.h themselves, so we have to leave
that one in place.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
It doesn't really make sense for us to have stdlib.h and string.h but
not stdio.h in the internal.h header.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
virQEMUQAPISchemaPathGet returns success when a given schema path was
not found but the returned object is set to NULL. This meant that we'd
call testQEMUSchemaValidate with the schemaroot being NULL which lead to
a crash if a mistyped monitor command was tested.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
The test file can be broken up by newlines and is automatically
concatenated back. Fix the control flow so that the concatenation code
'continues' the loop rather than branching out.
Also add an anotation to the concatenation code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
On EOF, the loop can be terminated right away since most of it is
skipped anyways and the handling of the last command is repeated after
the loop.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Since libvirt called bind() and listen() on the UNIX socket, it is
guaranteed that connect() will immediately succeed, if QEMU is running
normally. It will only fail if QEMU has closed the monitor socket by
mistake or if QEMU has exited, letting the kernel close it.
With this in mind we can remove the retry loop and timeout when
connecting to the QEMU monitor if we are doing FD passing. Libvirt can
go straight to sending the QMP greeting and will simply block waiting
for a reply until QEMU is ready.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Extended the json monitor test program with support for query-cpus-fast
and added a sample file set for x86 data obtained using the it.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Add infrastructure that will allow testing schema of the commands we
pass to the fake monitor object, so that we can make sure that it
actually does something.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Use the return value of virObjectRef directly. This way, it's easier
for another reader to identify the reason why the additional reference
is required.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
There were couple of reports on the list (e.g. [1]) that guests
with huge amounts of RAM are unable to start because libvirt
kills qemu in the initialization phase. The problem is that if
guest is configured to use hugepages kernel has to zero them all
out before handing over to qemu process. For instance, 402GiB
worth of 1GiB pages took around 105 seconds (~3.8GiB/s). Since we
do not want to make the timeout for connecting to monitor
configurable, we have to teach libvirt to count with this
fact. This commit implements "1s per each 1GiB of RAM" approach
as suggested here [2].
1: https://www.redhat.com/archives/libvir-list/2017-March/msg00373.html
2: https://www.redhat.com/archives/libvir-list/2017-March/msg00405.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Similar to the existing qemuMonitorTestNewFromFile the *Full version
will allow to check both commands and supply responses for a better
monitor testing.
