]> xenbits.xensource.com Git - libvirt.git/log
libvirt.git
5 years agoqemuAgentGetOSInfo: expose 'report_unsupported' argument
Peter Krempa [Mon, 16 Mar 2020 07:37:13 +0000 (08:37 +0100)]
qemuAgentGetOSInfo: expose 'report_unsupported' argument

Use qemuAgentCommandFull so that callers of qemuAgentGetOSInfo can
suppress error reports if the function is not supported by the guest
agent.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuAgentGetUsers: expose 'report_unsupported' argument
Peter Krempa [Mon, 16 Mar 2020 07:37:13 +0000 (08:37 +0100)]
qemuAgentGetUsers: expose 'report_unsupported' argument

Use qemuAgentCommandFull so that callers of qemuAgentGetUsers can
suppress error reports if the function is not supported by the guest
agent.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuAgentGetHostname: expose 'report_unsupported' argument
Peter Krempa [Mon, 16 Mar 2020 07:26:34 +0000 (08:26 +0100)]
qemuAgentGetHostname: expose 'report_unsupported' argument

Use qemuAgentCommandFull in qemuAgentGetHostname so that we can suppress
error reports if the caller will not require them. Callers for now
always require error reporting but will be fixed later.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuAgentGetHostname: Refactor to remove cleanup section
Peter Krempa [Fri, 13 Mar 2020 09:55:22 +0000 (10:55 +0100)]
qemuAgentGetHostname: Refactor to remove cleanup section

Use g_autoptr instead.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuAgentGetUsers: Fix return value on success
Peter Krempa [Fri, 13 Mar 2020 09:02:48 +0000 (10:02 +0100)]
qemuAgentGetUsers: Fix return value on success

Return 0 on success to match the documentation. The callers only check
for negative values.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuAgentCommand: Wire up suppressing of error reporting for unsupported commands
Peter Krempa [Fri, 13 Mar 2020 08:49:35 +0000 (09:49 +0100)]
qemuAgentCommand: Wire up suppressing of error reporting for unsupported commands

In some cases we don't want to log errors if an agent command is
unsupported. Wire it up into qemuAgentCheckError via qemuAgentCommandFull
and provide a thin wrapper (qemuAgentCommand) to prevent having to fix
all callers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuAgentCheckError: use g_autofree
Peter Krempa [Fri, 13 Mar 2020 08:43:10 +0000 (09:43 +0100)]
qemuAgentCheckError: use g_autofree

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuDomainGetGuestInfo: Validate supported information types
Peter Krempa [Fri, 13 Mar 2020 08:05:48 +0000 (09:05 +0100)]
qemuDomainGetGuestInfo: Validate supported information types

'qemuDomainGetGuestInfoCheckSupport' despite its name was not checking
whether the info types are supported. Convert the function to return
integers and include the check.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoconf: domain_conf: remove virDomainVideoDefaultType
Rafael Fonseca [Tue, 24 Mar 2020 16:14:36 +0000 (17:14 +0100)]
conf: domain_conf: remove virDomainVideoDefaultType

The logic has been moved to the individual drivers.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agovbox: move video default logic to driver
Rafael Fonseca [Tue, 24 Mar 2020 16:14:35 +0000 (17:14 +0100)]
vbox: move video default logic to driver

The logic setting a device default should be in the post parse function
of individual driver code.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agotest: move video default logic to driver
Rafael Fonseca [Tue, 24 Mar 2020 16:14:34 +0000 (17:14 +0100)]
test: move video default logic to driver

The logic setting a device default should be in the post parse function
of individual driver code.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agovmx: vmware: move video default logic to driver
Rafael Fonseca [Tue, 24 Mar 2020 16:14:33 +0000 (17:14 +0100)]
vmx: vmware: move video default logic to driver

The logic setting a device default should be in the post parse function
of individual driver code.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agovz: openvz: move video default logic to driver
Rafael Fonseca [Tue, 24 Mar 2020 16:14:32 +0000 (17:14 +0100)]
vz: openvz: move video default logic to driver

The logic setting a device default should be in the post parse function
of individual driver code.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agolibxl: move video default logic to driver
Rafael Fonseca [Tue, 24 Mar 2020 16:14:31 +0000 (17:14 +0100)]
libxl: move video default logic to driver

The logic setting a device default should be in the post parse function
of individual driver code.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agobhyve: move video default logic to driver
Rafael Fonseca [Tue, 24 Mar 2020 16:14:30 +0000 (17:14 +0100)]
bhyve: move video default logic to driver

The logic setting a device default should be in the post parse function
of individual driver code.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoutil: keep the pidfile locked
Marc-André Lureau [Tue, 24 Mar 2020 23:58:00 +0000 (00:58 +0100)]
util: keep the pidfile locked

Unfortunately, advisory record locking lose the lock if any fd refering
to the file is closed. There doesn't seem to be a way to preserve the
lock atomically. We could eventually retake the lock if low pidfilefd
is required.

This fixes processes being leaked, as they are not killed in
virPidFileForceCleanupPath() if the lock can be taken. Here also, we may
consider this is not good enough, as a process may leak by simply
closing the pidfilefd.

Fixes commit d146105f1e4a9e0ab179f0b78c070ea38b9d5334 ("virCommand:
Actually acquire pidfile instead of just writing it")

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agodocs: news: fix typo
Ján Tomko [Tue, 24 Mar 2020 17:22:11 +0000 (18:22 +0100)]
docs: news: fix typo

s/ommited/omitted/

Signed-off-by: Ján Tomko <jtomko@redhat.com>
5 years agoWIP: qemu-slirp: update to follow current spec
Marc-André Lureau [Tue, 25 Feb 2020 09:55:13 +0000 (10:55 +0100)]
WIP: qemu-slirp: update to follow current spec

The WIP specification is hosted on slirp wiki at this point:
https://gitlab.freedesktop.org/slirp/libslirp/-/wikis/Slirp-Helper

We would need more feedback from various parties (including libvirt,
podman, and other developpers) before declaring a frozen version.

So for now, follow it, and feedback welcome!

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu-slirp: register helper for migration
Marc-André Lureau [Tue, 25 Feb 2020 09:55:12 +0000 (10:55 +0100)]
qemu-slirp: register helper for migration

When the helper supports DBus, connect it to the bus and set its ID.

If the helper supports migration, register its ID to the list of
dbus-vmstate ID to migrate, and specify --dbus-incoming when
restoring the VM.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: add dbus-vmstate helper migration support
Marc-André Lureau [Tue, 25 Feb 2020 09:55:11 +0000 (10:55 +0100)]
qemu: add dbus-vmstate helper migration support

Helper processes may have their state migrated with QEMU data stream
thanks to the QEMU "dbus-vmstate".

libvirt maintains the list of helpers to be migrated. The
"dbus-vmstate" is added when required, and given the list of helper
Ids that must be migrated, on save & load sides.

See also:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus-vmstate.rst

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: prepare and stop the dbus daemon
Marc-André Lureau [Tue, 25 Feb 2020 09:55:10 +0000 (10:55 +0100)]
qemu: prepare and stop the dbus daemon

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agodomain: save/restore the state of dbus-daemon running
Marc-André Lureau [Tue, 25 Feb 2020 09:55:09 +0000 (10:55 +0100)]
domain: save/restore the state of dbus-daemon running

This avoids trying to start a dbus-daemon when its already running.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: add a DBus daemon helper unit
Marc-André Lureau [Tue, 25 Feb 2020 09:55:08 +0000 (10:55 +0100)]
qemu: add a DBus daemon helper unit

Add a unit to start & stop a private dbus-daemon.

The daemon is meant to be started on demand, and associated with a
QEMU process. It should be stopped when the QEMU process is stopped.

The current policy is permissive like a session bus. Stricter
policies can be added later, following recommendations from:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus.rst

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu-conf: add dbusStateDir
Marc-André Lureau [Tue, 25 Feb 2020 09:55:07 +0000 (10:55 +0100)]
qemu-conf: add dbusStateDir

Location of DBus daemon state configuration, socket, pid...

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu-conf: add configurable dbus-daemon location
Marc-André Lureau [Tue, 25 Feb 2020 09:55:06 +0000 (10:55 +0100)]
qemu-conf: add configurable dbus-daemon location

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: remove dbus-vmstate code
Marc-André Lureau [Tue, 25 Feb 2020 09:55:05 +0000 (10:55 +0100)]
qemu: remove dbus-vmstate code

This code was based on a per-helper instance and peer-to-peer
connections. The code that landed in qemu master for v5.0 is relying
on a single instance and DBus bus.

Instead of trying to adapt the existing dbus-vmstate code, let's
remove it and resubmit. That should make reviewing easier.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agobridge_driver: Replace and drop networkKillDaemon
Michal Privoznik [Fri, 13 Mar 2020 16:06:19 +0000 (17:06 +0100)]
bridge_driver: Replace and drop networkKillDaemon

In the network driver code there's networkKillDaemon() which is
the same as virProcessKillPainfully(). Replace the former with
the later and drop what becomes unused function.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
5 years agoqemuVirtioFSStop: Simplify daemon kill
Michal Privoznik [Fri, 13 Mar 2020 16:03:10 +0000 (17:03 +0100)]
qemuVirtioFSStop: Simplify daemon kill

Now, that we know that the virtiofsd will have the pidfile open
and locked we can use virPidFileForceCleanupPath() to kill it and
unlink the pidfile.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
5 years agoqemuSlirpStop: Simplify helper kill
Michal Privoznik [Fri, 13 Mar 2020 15:12:59 +0000 (16:12 +0100)]
qemuSlirpStop: Simplify helper kill

Now, that we know that the slirp helper will have the pidfile
open and locked we can use virPidFileForceCleanupPath() to kill
it and unlink the pidfile.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
5 years agoqemuProcessStartManagedPRDaemon: Don't pass -f pidfile to the daemon
Michal Privoznik [Fri, 13 Mar 2020 14:57:24 +0000 (15:57 +0100)]
qemuProcessStartManagedPRDaemon: Don't pass -f pidfile to the daemon

Now, that our virCommandSetPidFile() is more intelligent we don't
need to rely on the daemon to create and lock the pidfile and use
virCommandSetPidFile() at the same time.

NOTE that as advertised in the previous commit, this was
temporarily broken, because both virCommand and
qemuProcessStartManagedPRDaemon() would try to lock the pidfile.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
5 years agovirCommand: Actually acquire pidfile instead of just writing it
Michal Privoznik [Fri, 13 Mar 2020 12:12:49 +0000 (13:12 +0100)]
virCommand: Actually acquire pidfile instead of just writing it

Our virCommand module allows us to set a pidfile for commands we
want to spawn. The caller constructs the string of pidfile path
and then uses virCommandSetPidFile() to tell the module to write
the pidfile once the command is ran. This usually works, but has
two flaws:

1) the child process does not hold the pidfile open & locked.
Therefore, the caller (or anybody else) can't use our fancy
virPidFileForceCleanupPath() function to kill the command
afterwards. Also, for everybody else on the system it's
needlessly harder to check if the pid from the pidfile is still
alive or not.

2) if the caller ever makes a mistake and passes the same pidfile
path for two different commands, the start of the second command
will overwrite the pidfile even though the first command might
still be running.

NOTE that this temporarily renders some command spawning
unusable, specifically those code patterns where both
virCommandSetPidFile() is used together with instructing spawned
command to acquire pidfile itself. Fortunately, there is only one
occurrence of such pattern and it is in
qemuProcessStartManagedPRDaemon(). This is fixed in next commit.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
5 years agoqemu_monitor_text.c: Use g_autofree
Seeteena Thoufeek [Tue, 24 Mar 2020 12:44:46 +0000 (18:14 +0530)]
qemu_monitor_text.c: Use g_autofree

Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agocommandtest: Fix test28 error detection
Michal Privoznik [Tue, 24 Mar 2020 12:48:58 +0000 (13:48 +0100)]
commandtest: Fix test28 error detection

As a part of c799d150d5e9dae I've introduced a test case that
tests whether passing error object between processes works. The
test spawns a child which reports a system error, parent process
then reads the error and compares with expected output. Problem
with this approach is that error message contains stringified
errno which is not portable. FreeBSD has generally different
messages than Linux. Therefore, use g_strerror() to do the errno
to string translation for us.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
5 years agostorage: Parse 'nvme' disk source properties from json:{} pseudo-uri
Peter Krempa [Mon, 23 Mar 2020 17:06:56 +0000 (18:06 +0100)]
storage: Parse 'nvme' disk source properties from json:{} pseudo-uri

Our code allows snapshots of NVMe based disks which means we create
overlay file with a 'json:{}' pseudo-uri refering to the NVME device.
Our parser code doesn't handle them though. Add the parser and test it
via the XML->json->XML round-trip and reference data.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuBlockGetBackingStoreString: Properly handle 'http/s' with cookies and others
Peter Krempa [Mon, 23 Mar 2020 15:36:22 +0000 (16:36 +0100)]
qemuBlockGetBackingStoreString: Properly handle 'http/s' with cookies and others

Format cookies into the backing store string without encryption as they
will not be visible on the command line when formatting a 'target' only
string. In cases when cookies or other options are used we must use the
JSON format rather than pure URI.

Add tests to validate the scenario.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: block: Extract formatting of cookie string
Peter Krempa [Mon, 23 Mar 2020 15:31:19 +0000 (16:31 +0100)]
qemu: block: Extract formatting of cookie string

Introduce qemuBlockStorageSourceGetCookieString which does the
concatenation so that we can reuse it later.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuBlockGetBackingStoreString: Add extra wrapping object to JSON strings
Peter Krempa [Mon, 23 Mar 2020 15:09:31 +0000 (16:09 +0100)]
qemuBlockGetBackingStoreString: Add extra wrapping object to JSON strings

QEMU requires an extra wrapper object where only the "file" member is
populated. This is basically a placeholder for establishing the format
layer. We did the same in qemuDiskSourceGetProps for the old-school
JSON usage with -drive but forgot to adopt this for -blockdev.

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

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agostorage: Implement backing store support for "fat:" prefix
Peter Krempa [Mon, 23 Mar 2020 15:04:19 +0000 (16:04 +0100)]
storage: Implement backing store support for "fat:" prefix

qemublocktest showed that we don't add the "fat:" prefix for directory
storage when formatting the backing store string. While it's unlikely to
be used it's simple enough to actually implement the support rather than
trying to forbid it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuBlockGetBackingStoreString: Remove 'ret' variable
Peter Krempa [Mon, 23 Mar 2020 14:29:56 +0000 (15:29 +0100)]
qemuBlockGetBackingStoreString: Remove 'ret' variable

We can return the appropriate string directly.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemublocktest: Test backing store strings
Peter Krempa [Mon, 23 Mar 2020 11:35:32 +0000 (12:35 +0100)]
qemublocktest: Test backing store strings

With -blockdev libvirt provides the string which is recorded  as
'backing store' property of an image to qemu. Add testing for
qemuBlockGetBackingStoreString which generates these strings as there's
logic which determines which format to use.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agotestQemuDiskXMLToProps: Store all per-image data in one structure
Peter Krempa [Mon, 23 Mar 2020 11:10:38 +0000 (12:10 +0100)]
testQemuDiskXMLToProps: Store all per-image data in one structure

We had two non-syncrhonized arrays holding the individual data. This was
a lazy way to do it when I was adding new tests recently. Since it's
hard to extend with new data to test refactor the storage of test data
to use a new struct where all per-image data are kept and can be
extended easily.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuBlockGetBackingStoreString: Add 'pretty' argument
Peter Krempa [Mon, 23 Mar 2020 10:48:10 +0000 (11:48 +0100)]
qemuBlockGetBackingStoreString: Add 'pretty' argument

Add support for pretty-printing of the JSON variant of the output for
consumption in tests. All current callers pass 'false'.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agotestQemuDiskXMLToPropsValidateFileSrcOnly: Move together with rest of xml->json code
Peter Krempa [Mon, 23 Mar 2020 10:50:57 +0000 (11:50 +0100)]
testQemuDiskXMLToPropsValidateFileSrcOnly: Move together with rest of xml->json code

The function was misplaced. Group it together with other helper
functions for testing disk XML to qemu JSON props conversion.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemublocktest: xml->json: Refactor cleanup in test case functions
Peter Krempa [Mon, 23 Mar 2020 10:24:39 +0000 (11:24 +0100)]
qemublocktest: xml->json: Refactor cleanup in test case functions

Use automatic variable clearing and remove the cleanup sections of
testQemuDiskXMLToProps, testQemuDiskXMLToPropsValidateSchema and
testQemuDiskXMLToPropsValidateFile.

testQemuDiskXMLToPropsValidateFileSrcOnly already uses new helpers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agovirDomainDiskSourceNVMeFormat: Format only valid 'managed' values
Peter Krempa [Mon, 23 Mar 2020 17:26:52 +0000 (18:26 +0100)]
virDomainDiskSourceNVMeFormat: Format only valid 'managed' values

VIR_TRISTATE_BOOL_ABSENT which maps to the 'default' string would not be
parsed back, so we shouldn't format it either.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemublocktest: xml->json: Add test for NVMe
Peter Krempa [Wed, 18 Mar 2020 17:49:10 +0000 (18:49 +0100)]
qemublocktest: xml->json: Add test for NVMe

Based on the configuration from the only qemuxml2argv test.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoconf: rename 'namespace' property of struct _virStorageSourceNVMeDef
Peter Krempa [Mon, 23 Mar 2020 09:57:49 +0000 (10:57 +0100)]
conf: rename 'namespace' property of struct _virStorageSourceNVMeDef

While 'namespace' is not a reserved word in C, it is in C++. Our
compilers are happy with it but syntax-hilighting in some editors
hilights is as a keyword. Rename it to prevent confusion.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoutil: virhostcpu: Fail when fetching CPU Stats for invalid cpu
Mauro S. M. Rodrigues [Fri, 21 Feb 2020 18:10:45 +0000 (15:10 -0300)]
util: virhostcpu: Fail when fetching CPU Stats for invalid cpu

virHostCPUGetStatsLinux walks through every cpu in /proc/stat until it
finds cpu%cpuNum that matches with the requested cpu.
If none is found it logs the error but it should return -1, instead of 0.
Otherwise virsh nodecpustats --cpu <invalid cpu number> and API bindings
don't fail properly, printing a blank line instead of an error message.

This patch also includes an additional test for virhostcputest to avoid
this regression to happen again in the future.

Fixes: 93af79fba3fd75a8df6b7ca608719dd97f9511a0
Reported-by: Satheesh Rajendran <satheera@in.ibm.com>
Signed-off-by: Mauro S. M. Rodrigues <maurosr@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
5 years agoqemu: virtiofs: shorten socket filename
Ján Tomko [Mon, 23 Mar 2020 15:48:57 +0000 (16:48 +0100)]
qemu: virtiofs: shorten socket filename

Use just 'fs' instead of 'virtiofsd'.

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

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: virtiofs: shorten pid filename
Ján Tomko [Mon, 23 Mar 2020 15:46:03 +0000 (16:46 +0100)]
qemu: virtiofs: shorten pid filename

There is no need to repeat the shortName, since it's
already present in the directory path.

Also use just 'fs' instead of 'virtiofsd'.

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

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Suggested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agonews.xml: document the new NVDIMM support for Pseries guests
Daniel Henrique Barboza [Mon, 23 Mar 2020 19:40:51 +0000 (16:40 -0300)]
news.xml: document the new NVDIMM support for Pseries guests

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoformatdomain.html.in: document NVDIMM 'label' requirement for pSeries
Daniel Henrique Barboza [Mon, 23 Mar 2020 19:40:50 +0000 (16:40 -0300)]
formatdomain.html.in: document NVDIMM 'label' requirement for pSeries

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoconf, qemu: enable NVDIMM support for ppc64
Daniel Henrique Barboza [Mon, 23 Mar 2020 19:40:49 +0000 (16:40 -0300)]
conf, qemu: enable NVDIMM support for ppc64

Using the 'uuid' element for ppc64 NVDIMM memory added in the
previous patch, use it in qemuBuildMemoryDeviceStr() to pass
it over to QEMU.

Another ppc64 restriction is the necessity of a mem->labelsize,
given than ppc64 only support label-area backed NVDIMMs.

Finally, we don't want ppc64 NVDIMMs to align up due to the
high risk of going beyond the end of file with a 256MiB
increment that the user didn't predict. Align it down
instead. If target size is less than the minimum of
256MiB + labelsize, error out since QEMU will error out
if we attempt to round it up to the minimum.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoformatdomain.html.in: document the new 'uuid' NVDIMM element
Daniel Henrique Barboza [Mon, 23 Mar 2020 19:40:48 +0000 (16:40 -0300)]
formatdomain.html.in: document the new 'uuid' NVDIMM element

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoconf: Introduce optional 'uuid' element for NVDIMM memory
Daniel Henrique Barboza [Mon, 23 Mar 2020 19:40:47 +0000 (16:40 -0300)]
conf: Introduce optional 'uuid' element for NVDIMM memory

ppc64 NVDIMM support was implemented in QEMU by commit [1].
The support is similar to what x86 already does, aside from
an extra 'uuid' element.

This patch introduces a new optional 'uuid' element for the
NVDIMM memory model. This element behaves like the 'uuid'
element of the domain definition - if absent, we'll create
a new one, otherwise use the one provided by the XML.
The 'uuid' element is exclusive to pseries guests and are
unavailable for other architectures.

Next patch will use this new element to add NVDIMM support
for ppc64.

[1] https://github.com/qemu/qemu/commit/ee3a71e36654317b14ede0290e87628f8b79f850

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: capabilities: update qemu-5.0.0 capabilities for ppc64
Daniel Henrique Barboza [Mon, 23 Mar 2020 19:40:46 +0000 (16:40 -0300)]
qemu: capabilities: update qemu-5.0.0 capabilities for ppc64

Update ppc64 capabilities to pick up the new NVDIMM capability
support for ppc64.

Since the ppc64 capabilities weren't updated for some time, the
bulk of the changes here are related to the blockdev support
(see commit c6a9e54ce3 for info) that we are picking up just
now.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agonews: Document removal of support for .ini style of comments
Michal Privoznik [Mon, 23 Mar 2020 14:49:59 +0000 (15:49 +0100)]
news: Document removal of support for .ini style of comments

In previous patches virKeyFile was replaced with its GLib
counterpart which created an incompatible change: comments can
now begin only with a number sign (#). While this won't probably
affect anyone, mention it in the release notes.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
5 years agoutil: remove virKeyFile
Rafael Fonseca [Mon, 23 Mar 2020 13:19:13 +0000 (14:19 +0100)]
util: remove virKeyFile

The functionality is now provided by glib's GKeyFile.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoutil: authconfig: use g_key_file_*
Rafael Fonseca [Mon, 23 Mar 2020 13:19:12 +0000 (14:19 +0100)]
util: authconfig: use g_key_file_*

Replace libvirt's virKeyFile by glib's GKeyFile.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoapi: disallow virDomainAgentSetResponseTimeout() on read-only connections
Jonathon Jongsma [Fri, 20 Mar 2020 14:43:13 +0000 (09:43 -0500)]
api: disallow virDomainAgentSetResponseTimeout() on read-only connections

This function changes the amount of time that libvirt waits for a
response from the guest agent for all guest agent commands. Since this
is a configuration change, it should not be allowed on read-only
connections.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: fix response timeout for agent guest-sync
Jonathon Jongsma [Fri, 20 Mar 2020 22:28:10 +0000 (17:28 -0500)]
qemu: fix response timeout for agent guest-sync

The agent 'guest-sync' command historically had a 5s response timeout
which was different from other agent commands, which waited forever.
When we added the ability to customize the response timeout for guest
agent commands, we intended to continue to use 5s for 'guest-sync' when
the user specified a response timeout greater than 5s, and use the
user-specified timeout if it was below 5s. Unfortunately, when
attempting to determine whether the user-specified timeout was less than
5s, we were comparing against an enum value of
VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT (which is -1) rather than against
the actual time value that it represented (5).

This change makes it so that 'guest-sync' now uses the user-specified
tiemout if it is less than 5s.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: Use g_autofree and g_autoptr in qemuAgentSetUserPassword
Seeteena Thoufeek [Mon, 23 Mar 2020 11:13:59 +0000 (16:43 +0530)]
qemu: Use g_autofree and g_autoptr in qemuAgentSetUserPassword

Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoconf: use virStringParseYesNo
Rafael Fonseca [Sun, 22 Mar 2020 16:39:50 +0000 (17:39 +0100)]
conf: use virStringParseYesNo

Use existing function built for this exact purpose.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agosecurity: Try harder to run transactions
Michal Privoznik [Wed, 18 Mar 2020 09:18:46 +0000 (10:18 +0100)]
security: Try harder to run transactions

When a QEMU process dies in the middle of a hotplug, then we fail
to restore the seclabels on the device. The problem is that if
the thread doing hotplug locks the domain object first and thus
blocks the thread that wants to do qemuProcessStop(), the
seclabel cleanup code will see vm->pid still set and mount
namespace used and therefore try to enter the namespace
represented by the PID. But the PID is gone really and thus
entering will fail and no restore is done. What we can do is to
try enter the namespace (if requested to do so) but if entering
fails, fall back to no NS mode.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1814481

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
5 years agovirprocess: Passthru error from virProcessRunInForkHelper
Michal Privoznik [Wed, 18 Mar 2020 11:59:08 +0000 (12:59 +0100)]
virprocess: Passthru error from virProcessRunInForkHelper

When running a function in a forked child, so far the only thing
we could report is exit status of the child and the error
message. However, it may be beneficial to the caller to know the
actual error that happened in the child.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
5 years agovirfile: Handle directories in virFileBindMountDevice()
Michal Privoznik [Fri, 4 Oct 2019 19:01:49 +0000 (21:01 +0200)]
virfile: Handle directories in virFileBindMountDevice()

The @src is not always a file. It may also be a directory (for
instance qemuDomainCreateDeviceRecursive() assumes that) - even
though it doesn't happen usually. Anyway, mount() can mount only
a dir onto a dir and a file onto a file.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
5 years agoqemuDomainBuildNamespace: Make @devPath const
Michal Privoznik [Fri, 4 Oct 2019 19:56:25 +0000 (21:56 +0200)]
qemuDomainBuildNamespace: Make @devPath const

The @devPath variable is not modifiable. It merely just points to
string containing path where private devtmpfs is being
constructed. Make it const so it doesn't look weird that it's not
freed.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
5 years agoqemuDomainBuildNamespace: Try harder to remove temp directories
Michal Privoznik [Fri, 4 Oct 2019 19:01:29 +0000 (21:01 +0200)]
qemuDomainBuildNamespace: Try harder to remove temp directories

If building namespace fails somewhere in the middle (that is some
files exists under devMountsSavePath[i]), then plain rmdir() is
not enough to remove dir. Umount the temp location and use
virFileDeleteTree() to remove the directory.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
5 years agoqemuDomainCreateDeviceRecursive: Report error if mkdir() fails
Michal Privoznik [Fri, 4 Oct 2019 18:59:10 +0000 (20:59 +0200)]
qemuDomainCreateDeviceRecursive: Report error if mkdir() fails

The virFileMakePathWithMode() which is our recursive version of
mkdir() fails, it simply just returns a negative value with errno
set. No error is reported (as compared to virFileTouch() for
instance).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
5 years agotests: virstoragetest: validate that array deflattening works for gluster
Peter Krempa [Thu, 14 Dec 2017 17:01:05 +0000 (18:01 +0100)]
tests: virstoragetest: validate that array deflattening works for gluster

Validate that we are able to parse back the dotted syntax arrays we were
generating in the pre-blockdev era.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agojsontest: Add test cases for deflattening of arrays
Peter Krempa [Wed, 18 Mar 2020 16:02:42 +0000 (17:02 +0100)]
jsontest: Add test cases for deflattening of arrays

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agovirjson: Deflatten arrays generated by the json->commandline generator
Peter Krempa [Thu, 14 Dec 2017 17:03:04 +0000 (18:03 +0100)]
virjson: Deflatten arrays generated by the json->commandline generator

For the few instances where we'd generate an array in dotted syntax we
should be able to parse it back. Add another step in deflattening of the
dotted syntax which reconstructs the arrays so that the backing store
parser can parse it.

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

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoutil: json: Extract deflattening of keys into a separate function
Peter Krempa [Thu, 19 Mar 2020 17:11:48 +0000 (18:11 +0100)]
util: json: Extract deflattening of keys into a separate function

Extract the code so that there's a clean separation once we'll want do
do other steps.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agovirJSONValueObjectDeflattenWorker: Refactor cleanup
Peter Krempa [Wed, 18 Mar 2020 15:38:16 +0000 (16:38 +0100)]
virJSONValueObjectDeflattenWorker: Refactor cleanup

Use automatic memory handling to remove the cleanup section.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agovirBitmapNewEmpty: Use g_new0 to allocate and remove error checking
Peter Krempa [Wed, 18 Mar 2020 14:51:14 +0000 (15:51 +0100)]
virBitmapNewEmpty: Use g_new0 to allocate and remove error checking

virBitmapNewEmpty can't fail now so we can make it obvious and fix all
callers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agovirStorageEncryptionSecretCopy: Properly copy internals
Peter Krempa [Thu, 19 Mar 2020 14:38:06 +0000 (15:38 +0100)]
virStorageEncryptionSecretCopy: Properly copy internals

virStorageEncryptionSecretPtr may have a string inside it, thus we must
copy the string too. Use virSecretLookupDefCopy to do that.

Caused by non-obvious code introduced in 756b46ddd24 and later 47e88b33b
which added a string that needed to be copied.

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

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agovirSecretLookupDefCopy: Remove return value
Peter Krempa [Thu, 19 Mar 2020 14:27:40 +0000 (15:27 +0100)]
virSecretLookupDefCopy: Remove return value

The function always returns succes so there's no need for a return
value.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: block: Split up formatting of JSON props for 'raw' and 'luks' drivers
Peter Krempa [Thu, 19 Mar 2020 16:23:33 +0000 (17:23 +0100)]
qemu: block: Split up formatting of JSON props for 'raw' and 'luks' drivers

qemuBlockStorageSourceGetFormatRawProps aggregated both formats but
since we now have props specific for either of those formats it's
unwanted to aggregate the code such way. Split out the 'luks' props
formatter into qemuBlockStorageSourceGetFormatLUKSProps.

The wrong separation demonstrates istself on formatting of the 'size'
and 'offset' attributes for the 'luks' driver which does not conform
to the qapi schema.

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

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuBlockStorageSourceNeedsStorageSliceLayer: Deal with 'luks' files
Peter Krempa [Thu, 19 Mar 2020 15:54:52 +0000 (16:54 +0100)]
qemuBlockStorageSourceNeedsStorageSliceLayer: Deal with 'luks' files

The 'luks' driver in qemu is as any other non-raw format driver and thus
doesn't support the properties for 'slice'. Since libvirt considers
luks files to be raw+encryption we need to special case them when
dealing with the slice.

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

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemu: block: Extract logic decision when to use a separate 'raw' layer for slice
Peter Krempa [Thu, 19 Mar 2020 15:43:49 +0000 (16:43 +0100)]
qemu: block: Extract logic decision when to use a separate 'raw' layer for slice

Introduce qemuBlockStorageSourceNeedsStorageSliceLayer which will hold
the decision logic and fix all places that open-code it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemuxml2argvdata/disk-slices: Add test case for 'luks' encryption
Peter Krempa [Thu, 19 Mar 2020 15:26:53 +0000 (16:26 +0100)]
qemuxml2argvdata/disk-slices: Add test case for 'luks' encryption

Since libvirt handles the luks encryption in a weird special way
(raw+encryption) we should really test that case with slices as well.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agoqemu: reset await_event in all error paths in qemuAgentCommand
Nikolay Shirokovskiy [Fri, 20 Mar 2020 07:16:23 +0000 (10:16 +0300)]
qemu: reset await_event in all error paths in qemuAgentCommand

A fixup to patch [1]. We need to reset await_event in all
error paths.

[1] 52532073d : qemu: remove redundant needReply argument of qemuAgentCommand

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu: Don't crash when getting targets for a multipath
Michal Privoznik [Thu, 19 Mar 2020 11:51:55 +0000 (12:51 +0100)]
qemu: Don't crash when getting targets for a multipath

In one of my previous commits I've introduced code that creates
all devices for given (possible) multipath target. But I've made
a mistake there - the code accesses 'next->path' without checking
if the disk source is local. Note that the 'next->path' is
NULL/doesn't make sense for VIR_STORAGE_TYPE_NVME.

Fixes: a30078cb832646177defd256e77c632905f1e6d0
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1814947

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agodocs: Use <h1> to make sure kbase.html has page title
Sebastian Mitterle [Thu, 19 Mar 2020 17:59:27 +0000 (18:59 +0100)]
docs: Use <h1> to make sure kbase.html has page title

Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agodocs: formatbackup: Fix link to knowledge base article
Sebastian Mitterle [Thu, 19 Mar 2020 17:59:20 +0000 (18:59 +0100)]
docs: formatbackup: Fix link to knowledge base article

Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agotests: switch away from HAVE_SOCKETPAIR
Pino Toscano [Thu, 19 Mar 2020 11:02:45 +0000 (12:02 +0100)]
tests: switch away from HAVE_SOCKETPAIR

Since the removal of gnulib, HAVE_SOCKETPAIR is no more defined, making
these two tests effectively skipped.

Use the same strategy used in other generic library bits, i.e. exclude
the socketpair usage on Windows.

Semi-related change in virnetdaemontest.c to make it build: since
virutil.h does not include unistd.h anymore, we need to include it.

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
5 years agovmx: make 'fileName' optional for CD-ROMs
Pino Toscano [Wed, 18 Mar 2020 09:46:55 +0000 (10:46 +0100)]
vmx: make 'fileName' optional for CD-ROMs

It seems like CD-ROMs may have no 'fileName' property specified in case
there is nothing configured as attachment for the drive. Hence, make
sure that virVMXParseDisk() do not consider it mandatory anymore,
considering it an empty block cdrom device. Sadly virVMXParseDisk() is
used also to parse disk and floppies, so make sure that a NULL fileName
is handled in cdrom- and floppy-related paths.

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

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
5 years agovmx: shortcut earlier few 'ignore' cases in virVMXParseDisk()
Pino Toscano [Wed, 18 Mar 2020 09:29:51 +0000 (10:29 +0100)]
vmx: shortcut earlier few 'ignore' cases in virVMXParseDisk()

Move earlier the checks for skipping a hard disk when parsing a CD-DROM,
and for skipping a CD-ROM when parsing a hard disk. This should have no
behaviour changes, and avoids to add repeated checks in following
commits.

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
5 years agoqemu: Suppress error reporting from qemuMonitorDelObject in cleanup paths
Peter Krempa [Wed, 18 Mar 2020 11:24:40 +0000 (12:24 +0100)]
qemu: Suppress error reporting from qemuMonitorDelObject in cleanup paths

Many calls of qemuMonitorDelObject don't actually check the return value
or report the error from the object deletion itself since they are on
cleanup paths. In some cases this can lead to reporting of spurious
errors e.g. when qemuMonitorDelObject is used to clean up a possibly
pre-existing objects.

Add a new argument for qemuMonitorDelObject which controls whether
the internals report errors from qemu and fix all callers accordingly.

Note that some of the cases on device unplug which check the error code
don't in fact propagate the error to the user, but in this case it is
important to add the log entry anyways for tracing that the device
deletion failed.

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

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuMonitorJSONCheckError: Allow suppressing of error reporting
Peter Krempa [Wed, 18 Mar 2020 10:08:58 +0000 (11:08 +0100)]
qemuMonitorJSONCheckError: Allow suppressing of error reporting

In some cases we'll need to check whether there was an error but avoid
reporting an actual libvirt error. Rename qemuMonitorJSONCheckError to
qemuMonitorJSONCheckErrorFull with a new flag to suppress the error
reporting and add a wrapper with the original name so that callers don't
need to be fixed.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuMonitorJSONCheckError: Use g_autofree
Peter Krempa [Wed, 18 Mar 2020 09:34:32 +0000 (10:34 +0100)]
qemuMonitorJSONCheckError: Use g_autofree

Eliminate cleanup code by using g_autofree.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuMonitorJSON(Add|Del)Object: Refactor cleanup
Peter Krempa [Wed, 18 Mar 2020 09:29:41 +0000 (10:29 +0100)]
qemuMonitorJSON(Add|Del)Object: Refactor cleanup

Use 'g_autoptr' and remove the cleanup label and ret variable.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuDomainChangeEjectableMedia: Don't always remove managed PR daemon
Peter Krempa [Wed, 18 Mar 2020 10:44:01 +0000 (11:44 +0100)]
qemuDomainChangeEjectableMedia: Don't always remove managed PR daemon

When changing media we'd attempt to remove the managed pr daemon even if
neither of the images involved in the media change used it. This caused
libvirtd to log a spurious error:

2020-03-18 01:41:19.832+0000: 643207: error : qemuMonitorJSONCheckError:412 : internal error: unable to execute QEMU command 'object-del': object 'pr-helper0' not found

With this patch we completely avoid calling the deletion code.

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

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemuDomainVcpuValidateConfig: Properly initialize 'firstcpu' variable
Peter Krempa [Tue, 17 Mar 2020 17:33:31 +0000 (18:33 +0100)]
qemuDomainVcpuValidateConfig: Properly initialize 'firstcpu' variable

The loop which checks whether the vcpus are in proper configuration for
the requested hot(un)plug skips the first modified vcpu. This means
that 'firstvcpu' which is used to print the error message in case the
configuration is not suitable would never point to the first modified
vcpu.

In cases such as:

  <vcpu placement='auto' current='5'>8</vcpu>
  <vcpus>
    <vcpu id='0' enabled='yes' hotpluggable='no'/>
    <vcpu id='1' enabled='yes' hotpluggable='no'/>
    <vcpu id='2' enabled='yes' hotpluggable='no'/>
    <vcpu id='3' enabled='yes' hotpluggable='no'/>
    <vcpu id='4' enabled='yes' hotpluggable='no'/>
    <vcpu id='5' enabled='no' hotpluggable='yes'/>
    <vcpu id='6' enabled='no' hotpluggable='yes'/>
    <vcpu id='7' enabled='no' hotpluggable='yes'/>
  </vcpus>

 # virsh setvcpu --config --disable  upstream 1
 error: invalid argument: vcpu '-1' can't be modified as it is followed by non-hotpluggable online vcpus

After this fix the proper vcpu is reported in the error message:

 # virsh setvcpu --config --disable  upstream 1
 error: invalid argument: vcpu '1' can't be modified as it is followed by non-hotpluggable online vcpu

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

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
5 years agoconf: Don't generate clashing machine names for embed driver
Michal Privoznik [Wed, 11 Mar 2020 16:56:12 +0000 (17:56 +0100)]
conf: Don't generate clashing machine names for embed driver

So far, when using the qemu:///embed driver, management
applications can't chose whether they want to register their
domains in machined or not. While having that option is certainly
desired, it will require more work. What we can do meanwhile is
to generate names that include part of hash of the root
directory. This is to ensure that if two applications using
different roots but the same domain name (and ID) start the
domain no clashing name for machined is generated.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
5 years agovirDomainGenerateMachineName: Use g_autofree for @username
Michal Privoznik [Wed, 18 Mar 2020 14:45:07 +0000 (15:45 +0100)]
virDomainGenerateMachineName: Use g_autofree for @username

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
5 years agoqemu_conf: Track embed root dir
Michal Privoznik [Wed, 11 Mar 2020 17:33:34 +0000 (18:33 +0100)]
qemu_conf: Track embed root dir

When initializing virQEMUDriverConfig structure we are given the
root directory for possible embed connection. Save it for future
use. While we could get it later from @uri member, it's not as
easy as dereferencing a pointer (virURIParse() +
virURIGetParam() + error reporting).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
5 years agocpu: Honor check='full' for host-passthrough CPUs
Jiri Denemark [Mon, 9 Mar 2020 13:14:04 +0000 (14:14 +0100)]
cpu: Honor check='full' for host-passthrough CPUs

The check attribute was completely ignored for host-passthrough CPUs
even if they explicitly requested some features to be enabled. For
example, a domain with the following CPU definition

  <cpu mode='host-passthrough' check='full'>
    <feature policy='require' name='svm'/>
  </cpu>

would happily start even when 'svm' cannot be enabled.

Let's call virCPUArchUpdateLive for host-passthrough CPUs with
VIR_CPU_CHECK_FULL to make sure the architecture specific code can
validate the provided virtual CPU against the desired definition.

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

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agocpu_x86: Prepare virCPUx86UpdateLive for easier extension
Jiri Denemark [Mon, 9 Mar 2020 13:12:04 +0000 (14:12 +0100)]
cpu_x86: Prepare virCPUx86UpdateLive for easier extension

Adding more checks into the existing if statements would turn them into
an unreadable mess.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
5 years agocpu: Change control flow in virCPUUpdateLive
Jiri Denemark [Mon, 9 Mar 2020 09:30:39 +0000 (10:30 +0100)]
cpu: Change control flow in virCPUUpdateLive

The updateLive CPU sub-driver function is supposed to be called only for
a subset of CPU definitions. Let's make it more obvious by turning a
negative test and return into a positive check.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>