json: Clean up headers

The JSON parser has three public headers, json-lexer.h, json-parser.h,
json-streamer.h. They all contain stuff that is of no interest
outside qobject/json-*.c.

Collect the p

json: Clean up headers

The JSON parser has three public headers, json-lexer.h, json-parser.h,
json-streamer.h. They all contain stuff that is of no interest
outside qobject/json-*.c.

Collect the public interface in include/qapi/qmp/json-parser.h, and
everything else in qobject/json-parser-int.h.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-54-armbru@redhat.com>

show more ...

json: Pass lexical errors and limit violations to callback

The callback to consume JSON values takes QObject *json, Error *err.
If both are null, the callback is supposed to make up an error by
itse

json: Pass lexical errors and limit violations to callback

The callback to consume JSON values takes QObject *json, Error *err.
If both are null, the callback is supposed to make up an error by
itself. This sucks.

qjson.c's consume_json() neglects to do so, which makes
qobject_from_json() null instead of failing. I consider that a bug.

The culprit is json_message_process_token(): it passes two null
pointers when it runs into a lexical error or a limit violation. Fix
it to pass a proper Error object then. Update the callbacks:

* monitor.c's handle_qmp_command(): the code to make up an error is
now dead, drop it.

* qga/main.c's process_event(): lumps the "both null" case together
with the "not a JSON object" case. The former is now gone. The
error message "Invalid JSON syntax" is misleading for the latter.
Improve it to "Input must be a JSON object".

* qobject/qjson.c's consume_json(): no update; check-qjson
demonstrates qobject_from_json() now sets an error on lexical
errors, but still doesn't on some other errors.

* tests/libqtest.c's qmp_response(): the Error object is now reliable,
so use it to improve the error message.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-40-armbru@redhat.com>

show more ...

json: Redesign the callback to consume JSON values

The classical way to structure parser and lexer is to have the client
call the parser to get an abstract syntax tree, the parser call the
lexer to

json: Redesign the callback to consume JSON values

The classical way to structure parser and lexer is to have the client
call the parser to get an abstract syntax tree, the parser call the
lexer to get the next token, and the lexer call some function to get
input characters.

Another way to structure them would be to have the client feed
characters to the lexer, the lexer feed tokens to the parser, and the
parser feed abstract syntax trees to some callback provided by the
client. This way is more easily integrated into an event loop that
dispatches input characters as they arrive.

Our JSON parser is kind of between the two. The lexer feeds tokens to
a "streamer" instead of a real parser. The streamer accumulates
tokens until it got the sequence of tokens that comprise a single JSON
value (it counts curly braces and square brackets to decide). It
feeds those token sequences to a callback provided by the client. The
callback passes each token sequence to the parser, and gets back an
abstract syntax tree.

I figure it was done that way to make a straightforward recursive
descent parser possible. "Get next token" becomes "pop the first
token off the token sequence". Drawback: we need to store a complete
token sequence. Each token eats 13 + input characters + malloc
overhead bytes.

Observations:

1. This is not the only way to use recursive descent. If we replaced
"get next token" by a coroutine yield, we could do without a
streamer.

2. The lexer reports errors by passing a JSON_ERROR token to the
streamer. This communicates the offending input characters and
their location, but no more.

3. The streamer reports errors by passing a null token sequence to the
callback. The (already poor) lexical error information is thrown
away.

4. Having the callback receive a token sequence duplicates the code to
convert token sequence to abstract syntax tree in every callback.

5. Known bug: the streamer silently drops incomplete token sequences.

This commit rectifies 4. by lifting the call of the parser from the
callbacks into the streamer. Later commits will address 3. and 5.

The lifting removes a bug from qjson.c's parse_json(): it passed a
pointer to a non-null Error * in certain cases, as demonstrated by
check-qjson.c.

json_parser_parse() is now unused. It's a stupid wrapper around
json_parser_parse_err(). Drop it, and rename json_parser_parse_err()
to json_parser_parse().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-35-armbru@redhat.com>

show more ...

qga: process_event() simplification and leak fix

json_parser_parse_err() may return something else than a QDict, in
which case we loose the object. Let's keep track of the original
object to avoid l

qga: process_event() simplification and leak fix

json_parser_parse_err() may return something else than a QDict, in
which case we loose the object. Let's keep track of the original
object to avoid leaks.

When an error occurs, "qdict" contains the response, but we still
check the "execute" key there. Untangle a bit this code, by having a
clear error path.

CC: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga-win: Handle fstrim for OSes lower than Win8

The defrag.exe tool which is used for executing the fstrim command
on Windows doesn't support retrim for OSes lower than Win8. This
commit handles thi

qga-win: Handle fstrim for OSes lower than Win8

The defrag.exe tool which is used for executing the fstrim command
on Windows doesn't support retrim for OSes lower than Win8. This
commit handles this case and returns a suitable error.

Output of fstrim before this commit:
{"execute":"guest-fstrim"}
{"return": {"paths": [{"path": "C:\\", "error": "An invalid command line option
was specified. (0x89000008)"}, {"path": "F:\\", "error": "An invalid command
line option was specified. (0x89000008)"}, {"path": "S:\\", "error": "An
invalid command line option was specified. (0x89000008)"}]}}

Reported on:
https://bugzilla.redhat.com/show_bug.cgi?id=1594113

Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
* use alternative version query code proposed by Sameeh
* fix up version check logic
* avoid CamelCase variable names when possible
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga: fix file descriptor leak

The file descriptor for /sys/power/state was never closed. Reported
by Coverity.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daud

qga: fix file descriptor leak

The file descriptor for /sys/power/state was never closed. Reported
by Coverity.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga: fix 'driver' leak in guest-get-fsinfo

'driver' is leaked when the loop is not broken.

Leak introduced by commit 743c71d03c20d64f2bae5fba6f26cdf5e4b1bda6,
spotted by ASAN.

Signed-off-by: Marc-

qga: fix 'driver' leak in guest-get-fsinfo

'driver' is leaked when the loop is not broken.

Leak introduced by commit 743c71d03c20d64f2bae5fba6f26cdf5e4b1bda6,
spotted by ASAN.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qmp: Use QDict * instead of QObject * for response objects

By using the more specific type, we get fewer downcasts. The
downcasts are safe, but not obviously so, at least not locally.

Signed-off-b

qmp: Use QDict * instead of QObject * for response objects

By using the more specific type, we get fewer downcasts. The
downcasts are safe, but not obviously so, at least not locally.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-24-armbru@redhat.com>

show more ...

qmp: De-duplicate error response building

All callers of qmp_build_error_object() duplicate the code to wrap it
in a response object. Replace it by qmp_error_response() that
captures the duplicated

qmp: De-duplicate error response building

All callers of qmp_build_error_object() duplicate the code to wrap it
in a response object. Replace it by qmp_error_response() that
captures the duplicated code, including error_free().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-23-armbru@redhat.com>

show more ...

qmp qemu-ga: Fix qemu-ga not to accept "control"

Commit cf869d53172 "qmp: support out-of-band (oob) execution"
accidentally made qemu-ga accept and ignore "control". Fix that.

Out-of-band executio

qmp qemu-ga: Fix qemu-ga not to accept "control"

Commit cf869d53172 "qmp: support out-of-band (oob) execution"
accidentally made qemu-ga accept and ignore "control". Fix that.

Out-of-band execution in a monitor that doesn't support it now fails
with

{"error": {"class": "GenericError", "desc": "QMP input member 'control' is unexpected"}}

instead of

{"error": {"class": "GenericError", "desc": "Please enable out-of-band first for the session during capabilities negotiation"}}

The old description is suboptimal when out-of-band cannot not be
enabled, or the command doesn't support out-of-band execution.

The new description is a bit unspecific, but it'll do.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-12-armbru@redhat.com>

show more ...

qga: removing bios_supports_mode

bios_support_mode verifies if the guest has support for a certain
suspend mode but it doesn't inform back which suspend tool
provides it. The caller, guest_suspend,

qga: removing bios_supports_mode

bios_support_mode verifies if the guest has support for a certain
suspend mode but it doesn't inform back which suspend tool
provides it. The caller, guest_suspend, executes all suspend
strategies in order again.

After adding systemd suspend support, bios_support_mode now will
verify for support for systemd, then pmutils, then Linux sys state
file. In a worst case scenario where both systemd and pmutils isn't
supported but Linux sys state is:

- bios_supports_mode will check for systemd, then pmutils, then
Linux sys state. It will tell guest_suspend that there is support,
but it will not tell who provides it;

- guest_suspend will try to execute (and fail) systemd suspend,
then pmutils suspend, to only then use the Linux sys suspend.
The time spent executing systemd and pmutils suspend was wasted
and could be avoided, but only bios_support_mode knew it but
didn't inform it back.

A quicker approach is to nuke bios_supports_mode and control
whether we found support at all with a bool flag inside
guest_suspend. guest_suspend will search for suspend support
and execute it as soon as possible. If the a given suspend
mechanism fails, continue to the next. If no suspend
support is found, the "not supported" message is still being
sent back to the user.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga: systemd hibernate/suspend/hybrid-sleep support

pmutils isn't being supported by newer OSes like Fedora 27
or Mint. This means that the only suspend option QGA offers
for these guests are writin

qga: systemd hibernate/suspend/hybrid-sleep support

pmutils isn't being supported by newer OSes like Fedora 27
or Mint. This means that the only suspend option QGA offers
for these guests are writing directly into the Linux sys state
file. This also means that QGA also loses the ability to do
hybrid suspend in those guests - this suspend mode is only
available when using pmutils.

Newer guests can use systemd facilities to do all the suspend
types QGA supports. The mapping in comparison with pmutils is:

- pm-hibernate -> systemctl hibernate
- pm-suspend -> systemctl suspend
- pm-suspend-hybrid -> systemctl hybrid-sleep

To discover whether systemd supports these functions, we inspect
the status of the services that implements them.

With this patch, we can offer hybrid suspend again for newer
guests that do not have pmutils support anymore.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga: removing switch statements, adding run_process_child

This is a cleanup of the resulting code after detaching
pmutils and Linux sys state file logic:

- remove the SUSPEND_MODE_* macros and use

qga: removing switch statements, adding run_process_child

This is a cleanup of the resulting code after detaching
pmutils and Linux sys state file logic:

- remove the SUSPEND_MODE_* macros and use an enumeration
instead. At the same time, drop the switch statements
at the start of each function and use the enumeration
index to get the right binary/argument;

- create a new function called run_process_child(). This
function uses g_spawn_sync() to execute a shell command,
returning the exit code. This is a common operation in the
pmutils functions and will be used in the systemd implementation
as well, so this function will avoid code repetition.

There are more places inside commands-posix.c where this new
run_process_child function can also be used, but one step
at a time.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
*check/propagate local_err before setting errp directly
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga: guest_suspend: decoupling pm-utils and sys logic

Following the same logic of the previous patch, let's also
decouple the suspend logic from guest_suspend into specialized
functions, one for eac

qga: guest_suspend: decoupling pm-utils and sys logic

Following the same logic of the previous patch, let's also
decouple the suspend logic from guest_suspend into specialized
functions, one for each strategy we support at this moment.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga: bios_supports_mode: decoupling pm-utils and sys logic

In bios_supports_mode there is a verification to assert if
the chosen suspend mode is supported by the pmutils tools and,
if not, we see if

qga: bios_supports_mode: decoupling pm-utils and sys logic

In bios_supports_mode there is a verification to assert if
the chosen suspend mode is supported by the pmutils tools and,
if not, we see if the Linux sys state files supports it.

This verification is done in the same function, one after
the other, and it works for now. But, when adding a new
suspend mechanism that will not necessarily follow the same
return 0 or 1 logic of pmutils, this code will be hard
to deal with.

This patch decouple the two existing logics into their own
functions, pmutils_supports_mode and linux_sys_state_supports_mode,
which in turn are used inside bios_support_mode. The existing
logic is kept but now it's easier to extend it.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga: refactoring qmp_guest_suspend_* functions

To be able to add new suspend mechanisms we need to detach
the existing QMP functions from the current implementation
specifics.

At this moment we hav

qga: refactoring qmp_guest_suspend_* functions

To be able to add new suspend mechanisms we need to detach
the existing QMP functions from the current implementation
specifics.

At this moment we have functions such as qmp_guest_suspend_ram
calling bios_suspend_mode and guest_suspend passing the
pmutils command and arguments as parameters. This patch
removes this logic from the QMP functions, moving them to
the respective functions that will have to deal with which
binary to use.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qemu-ga: make get-fsinfo work over pci bridges

Iterate over the PCI bridges to lookup the PCI device associated with
the block device.

This allows to lookup the driver under the following syspath:

qemu-ga: make get-fsinfo work over pci bridges

Iterate over the PCI bridges to lookup the PCI device associated with
the block device.

This allows to lookup the driver under the following syspath:
/sys/devices/pci0000:00/0000:00:02.2/0000:03:00.0/virtio2/block/vda/vda3

It also works with an "old-style" Q35 libvirt hierarchy: root complex
-> DMI-PCI bridge -> PCI-PCI bridge -> virtio controller, ex:
/sys/devices/pci0000:00/0000:00:03.0/0000:01:01.0/0000:02:01.0/virtio1/block/vda/vda3

The setup can be reproduced with the following qemu command line
(Thanks Marcel for help):

qemu-system-x86_64 -M q35 \
-device i82801b11-bridge,id=dmi2pci_bridge,bus=pcie.0
-device pci-bridge,id=pci_bridge,bus=dmi2pci_bridge,addr=0x1,chassis_nr=1
-device virtio-blk-pci,scsi=off,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,bus=pci_bridge,addr=0x1

For consistency with other syspath-related debug messages, replace a
\"%s\" in the message with '%s'.

Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1567041

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga-win: Fixing msi upgrade disallow in WiX file

Issue: When upgrading qemu-ga using the msi from an old version
to a newer one, the upgrade is not allowed by the msi
showing this erro

qga-win: Fixing msi upgrade disallow in WiX file

Issue: When upgrading qemu-ga using the msi from an old version
to a newer one, the upgrade is not allowed by the msi
showing this error message "Another version of this product
is already installed."

BZ# 1536331: https://bugzilla.redhat.com/show_bug.cgi?id=1536331

Fix: For the upgrade to be allowed by the msi the WiX file must
provide three things:
1. Changing product's Id. (assigning it to "*")
2. Constant product's UpgradeId. (exists)
3. Changing version. (exists)

Reference: http://wixtoolset.org/documentation/manual/v3/howtos/updates/major_upgrade.html

Signed-off-by: Bishara AbuHattoum <bishara@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga/schema: fix documentation for GuestOSInfo

The documentation for kernel-version and kernel-release on Windows was
swapped.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Ma

qga/schema: fix documentation for GuestOSInfo

The documentation for kernel-version and kernel-release on Windows was
swapped.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga-win: add driver path usage to GuestFilesystemInfo

This patch adds support for getting the usage of
windows driver path.
The usage of fs stored as used_bytes and total_bytes.

Cc: Michael Roth <m

qga-win: add driver path usage to GuestFilesystemInfo

This patch adds support for getting the usage of
windows driver path.
The usage of fs stored as used_bytes and total_bytes.

Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga: add mountpoint usage info to GuestFilesystemInfo

This patch adds support for getting the usage of mounted
filesystem.
The usage of fs stored as used_bytes and total_bytes.
It's very useful when

qga: add mountpoint usage info to GuestFilesystemInfo

This patch adds support for getting the usage of mounted
filesystem.
The usage of fs stored as used_bytes and total_bytes.
It's very useful when we try to monitor guest's filesystem.

Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga: check bytes count read by guest-file-read

While reading file content via 'guest-file-read' command,
'qmp_guest_file_read' routine allocates buffer of count+1
bytes. It could overflow for large

qga: check bytes count read by guest-file-read

While reading file content via 'guest-file-read' command,
'qmp_guest_file_read' routine allocates buffer of count+1
bytes. It could overflow for large values of 'count'.
Add check to avoid it.

Reported-by: Fakhri Zulkifli <mohdfakhrizulkifli@gmail.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

qga: unset frozen state if no mount points are frozen

If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
we may got nothing to freeze as all mountpoints are
not valid.
So call ga_unset_frozen

qga: unset frozen state if no mount points are frozen

If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
we may got nothing to freeze as all mountpoints are
not valid.
So call ga_unset_frozen in this senario.

Also, if we return 0 frozen fs, there is no need to call
guest-fsfreeze-thaw.

Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

show more ...

glib: bump min required glib library version to 2.40

Per supported platforms doc[1], the various min glib on relevant distros is:

RHEL-7: 2.50.3
Debian (Stretch): 2.50.3
Debian (Jessie): 2.42

glib: bump min required glib library version to 2.40

Per supported platforms doc[1], the various min glib on relevant distros is:

RHEL-7: 2.50.3
Debian (Stretch): 2.50.3
Debian (Jessie): 2.42.1
OpenBSD (Ports): 2.54.3
FreeBSD (Ports): 2.50.3
OpenSUSE Leap 15: 2.54.3
SLE12-SP2: 2.48.2
Ubuntu (Xenial): 2.48.0
macOS (Homebrew): 2.56.0

This suggests that a minimum glib of 2.42 is a reasonable target.

The GLibC compile farm, however, uses Ubuntu 14.04 (Trusty) which only
has glib 2.40.0, and this is needed for testing during merge. Thus an
exception is made to the documented platform support policy to allow for
all three current LTS releases to be supported.

Docker jobs that not longer satisfy this new min version are removed.

[1] https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

show more ...

qga: use local path for local headers

When pulling in headers that are in the same directory as the C file (as
opposed to one in include/), we should use its relative path, without a
directory.

Sig

qga: use local path for local headers

When pulling in headers that are in the same directory as the C file (as
opposed to one in include/), we should use its relative path, without a
directory.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

show more ...