]> git.ipfire.org Git - thirdparty/libvirt.git/log
thirdparty/libvirt.git
2 years agodocs: ACL: Mention the ACL object name along with the corresponding libvirt object...
Peter Krempa [Fri, 17 Feb 2023 15:48:35 +0000 (16:48 +0100)] 
docs: ACL: Mention the ACL object name along with the corresponding libvirt object name

It's not trivial to figure out the ACL object name from our
documentation. Add it above the table outlining existing permissions.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agodocs: Fix generated names for ACL objects
Peter Krempa [Mon, 20 Feb 2023 10:31:11 +0000 (11:31 +0100)] 
docs: Fix generated names for ACL objects

Both the object name and permission name in ACL use '-' instead of '_'
separator when referring to them in the docs or even when used inside of
polkit. Unfortunately the generators used for generating our docs don't
honour this in certain cases which would result in broken names in the
API docs (once they will be generated).

Rename both object and permission name to use dash and reflect that in
the anchor names in the documentation.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoTranslated using Weblate (Georgian)
Temuri Doghonadze [Sun, 19 Feb 2023 16:20:23 +0000 (17:20 +0100)] 
Translated using Weblate (Georgian)

Currently translated at 1.7% (185 of 10405 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/ka/

Co-authored-by: Temuri Doghonadze <temuri.doghonadze@gmail.com>
Signed-off-by: Temuri Doghonadze <temuri.doghonadze@gmail.com>
2 years agoselinux: Don't ignore ENOENT in Permissive mode
Michal Privoznik [Mon, 20 Sep 2021 11:02:37 +0000 (13:02 +0200)] 
selinux: Don't ignore ENOENT in Permissive mode

In selinux driver there's virSecuritySELinuxSetFileconImpl()
which is responsible for actual setting of SELinux label on given
file and handling possible failures. In fhe failure handling code
we decide whether failure is fatal or not. But there is a bug:
depending on SELinux mode (Permissive vs. Enforcing) the ENOENT
is either ignored or considered fatal. This not correct - ENOENT
must always be fatal for couple of reasons:

- In virSecurityStackTransactionCommit() the seclabels are set
  for individual secdrivers (e.g. SELinux first and then DAC),
  but if one secdriver succeeds and another one fails, then no
  rollback is performed for the successful one leaking remembered
  labels.

- QEMU would fail opening the file anyways (if neither of
  secdrivers reported error and thus cancelled domain startup)

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2004850
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agoselinux: Swap two blocks handling setfilecon_raw() failure
Michal Privoznik [Mon, 20 Sep 2021 10:21:04 +0000 (12:21 +0200)] 
selinux: Swap two blocks handling setfilecon_raw() failure

In virSecuritySELinuxSetFileconImpl() we have code that handles
setfilecon_raw() failure. The code consists of two blocks: one
for dealing with shared filesystem like NFS (errno is ENOTSUP or
EROFS) and the other block that's dealing with EPERM for
privileged daemon. Well, the order of these two blocks is a bit
confusing because the comment above them mentions the NFS case
but EPERM block follows. Swap these two blocks to make it less
confusing.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agoqemu_passt: Let passt write the PID file
Michal Privoznik [Thu, 16 Feb 2023 10:46:55 +0000 (11:46 +0100)] 
qemu_passt: Let passt write the PID file

The way we start passt currently is: we use
virCommandSetPidFile() to use our virCommand machinery to acquire
the PID file and leak opened FD into passt. Then, we use
virPidFile*() APIs to read the PID file (which is needed when
placing it into CGroups or killing it). But this does not fly
really because passt daemonizes itself. Thus the process we
started dies soon and thus the PID file is closed and unlocked.

We could work around this by passing '--foreground' argument, but
that weakens passt as it can't create new PID namespace (because
it doesn't fork()).

The solution is to let passt write the PID file, but since it
does not lock the file and closes it as soon as it is written, we
have to switch to those virPidFile APIs which don't expect PID
file to be locked.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2 years agoqemu_passt: Deduplicate passt killing code
Michal Privoznik [Thu, 16 Feb 2023 11:07:42 +0000 (12:07 +0100)] 
qemu_passt: Deduplicate passt killing code

There are two places where we kill passt:

1) qemuPasstStop() - called transitively from qemuProcessStop(),
2) qemuPasstStart() - after failed start.

Now, the code from 2) lack error preservation (so if there's
another error during cleanup we might overwrite the original
error). Therefore, move the internals of qemuPasstStop() into a
separate function and call it from both places.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2 years agoqemu_passt: Report passt's error on failed start
Michal Privoznik [Thu, 16 Feb 2023 11:19:26 +0000 (12:19 +0100)] 
qemu_passt: Report passt's error on failed start

When starting passt, it may write something onto its stderr
(convincing it to print even more is addressed later). Pass this
string we read to user.

Since we're not daemonizing passt anymore (see previous commit),
we can let virCommand module do all the heavy lifting and switch
to virCommandSetErrorBuffer() instead of reading error from an
FD.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2 years agoqemu_passt: Avoid double daemonizing passt
Michal Privoznik [Thu, 16 Feb 2023 11:00:58 +0000 (12:00 +0100)] 
qemu_passt: Avoid double daemonizing passt

When passt is started, it daemonizes itself by default. There's
no point in having our virCommand module daemonize it too.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2 years agodocs: ACL: Show which permissions are allowed for unauthenticated connections
Peter Krempa [Fri, 17 Feb 2023 15:31:20 +0000 (16:31 +0100)] 
docs: ACL: Show which permissions are allowed for unauthenticated connections

Certain APIs are allowed also without authentication but the ACL page
didn't outline which. Generate a new column with the information.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agolibvirt-nodedev: Allow read-only access to virNodeDeviceGetAutostart
Peter Krempa [Fri, 17 Feb 2023 15:07:25 +0000 (16:07 +0100)] 
libvirt-nodedev: Allow read-only access to virNodeDeviceGetAutostart

Fetching whether a node-device is marked for autostart can be allowed
from read-only connections similarly to other objects.

Fixes: c6607a25b93
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agoaccess: Allow 'node-device.read' permission for anonymous users
Peter Krempa [Fri, 17 Feb 2023 15:02:09 +0000 (16:02 +0100)] 
access: Allow 'node-device.read' permission for anonymous users

For all other objects we allow the 'read' permission for anonymous
users. In fact the idea is to allow all permissions users using the
readonly connection would have.

This impacts the following APIs (in terms of RPC procedure names):

  $ git grep -A 3 node_device:read | grep REMOTE
  src/remote/remote_protocol.x-    REMOTE_PROC_NODE_DEVICE_GET_XML_DESC = 114,
  src/remote/remote_protocol.x-    REMOTE_PROC_NODE_DEVICE_GET_PARENT = 115,
  src/remote/remote_protocol.x-    REMOTE_PROC_NODE_DEVICE_NUM_OF_CAPS = 116,
  src/remote/remote_protocol.x-    REMOTE_PROC_NODE_DEVICE_LIST_CAPS = 117,
  src/remote/remote_protocol.x-    REMOTE_PROC_NODE_DEVICE_GET_AUTOSTART = 433,
  src/remote/remote_protocol.x-    REMOTE_PROC_NODE_DEVICE_IS_PERSISTENT = 435,
  src/remote/remote_protocol.x-    REMOTE_PROC_NODE_DEVICE_IS_ACTIVE = 436,

Fixes: a93cd08f
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agoqemu_extdevice: Add a comment into qemuExtDevicesSetupCgroup()
Michal Privoznik [Wed, 15 Feb 2023 14:52:21 +0000 (15:52 +0100)] 
qemu_extdevice: Add a comment into qemuExtDevicesSetupCgroup()

The way setting up CGroups for external helpers work, is:
qemuExtDevicesHasDevice() is called first to determine whether
there is a helper process running, the CGroup controller is
created and then qemuExtDevicesSetupCgroup() is called to place
helpers into the CGroup. But when one reads just
qemuExtDevicesSetupCgroup() it's easy to miss this hidden logic.
Therefore, add a warning at the beginning of the function.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2 years agoqemu_passt: Report error when getting passt PID failed
Michal Privoznik [Mon, 13 Feb 2023 15:05:04 +0000 (16:05 +0100)] 
qemu_passt: Report error when getting passt PID failed

If qemuPasstGetPid() fails, or the passt's PID is -1 then
qemuPasstSetupCgroup() returns early without any error message
set. Report an appropriate error.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2 years agoqemu_extdevice: Make qemuExtDevicesHasDevice() check def->nets
Michal Privoznik [Mon, 13 Feb 2023 15:01:32 +0000 (16:01 +0100)] 
qemu_extdevice: Make qemuExtDevicesHasDevice() check def->nets

We can have external helper processes running for domain
<interface/> too (e.g. slirp or passt). But this is not reflected
in qemuExtDevicesHasDevice() which simply ignores these.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2 years agoRevert "qemu: allow passt to self-daemonize"
Michal Privoznik [Mon, 13 Feb 2023 09:25:51 +0000 (10:25 +0100)] 
Revert "qemu: allow passt to self-daemonize"

This reverts commit 0c4e716835eaf2a575bd063fde074c0fc7c4e4d4.

This patch was pushed by my mistake. Even though it got ACKed on
the list, I've raised couple of issues with it. They will be
fixed in next commits.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2 years agoscripts: check-html-refernces: Add checking for image file usage
Peter Krempa [Tue, 14 Feb 2023 13:38:40 +0000 (14:38 +0100)] 
scripts: check-html-refernces: Add checking for image file usage

Check both that a file is referenced from our pages and also that pages
reference existing images.

The mode for dumping external references now also dumps images.

'--ignore-image' can be used repeatedly to suppress errors for specific
images.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agodocs: logos: Turn 'README' into rST, generate an index and link to images
Peter Krempa [Tue, 14 Feb 2023 14:03:18 +0000 (15:03 +0100)] 
docs: logos: Turn 'README' into rST, generate an index and link to images

The logo directory wasn't really referenced from anywhere. Additionally
there wasn't any reasonable index for all the image files which we have.

Turn the README file into rST and display the images it references. Link
to the new index file from the docs page.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agokbase: eventloop: Fix path to referenced images
Peter Krempa [Tue, 14 Feb 2023 13:38:46 +0000 (14:38 +0100)] 
kbase: eventloop: Fix path to referenced images

The images are referenced from '../images/' but the document is two
layers deep thus '../../images' needs to be used

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agoscripts: check-html-references: Detect pages that are not linked to
Peter Krempa [Tue, 14 Feb 2023 12:14:25 +0000 (13:14 +0100)] 
scripts: check-html-references: Detect pages that are not linked to

Prevent sub-pages without a way to reach them.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agodocs: Add sub-page for all golang modules
Peter Krempa [Tue, 14 Feb 2023 12:51:02 +0000 (13:51 +0100)] 
docs: Add sub-page for all golang modules

Our documentation has pages for 4 go modules, 2 current and 2 obsolete
ones, but points only to one of them and directly to golang's docs page.

Add a sub-page where all 4 sub-pages for the modules are linked.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agodocs: manpages: Add missing manpages to index
Peter Krempa [Tue, 14 Feb 2023 12:19:09 +0000 (13:19 +0100)] 
docs: manpages: Add missing manpages to index

The manpages for 'virt-pki-query-dn', 'virt-qemu-qmp-proxy' and
'virt-ssh-helper.rst' were not referenced from the manpage index or any
other place.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agoscripts: check-html-references: Improve error messages and don't mess with relative...
Peter Krempa [Tue, 14 Feb 2023 11:35:23 +0000 (12:35 +0100)] 
scripts: check-html-references: Improve error messages and don't mess with relative paths

Now that we have the source file name as a custom attribute we can use
it to report which file actually needs to be edited to fix the error:

 ERROR: 'docs/uri.rst': broken link to: 'drvqemu.html#exaple'

rather than:

 broken link targets:
 docs/uri.html broken link: drvqemu.html#exaple

which pointed to file which does not exist in the source directory.

This also allows us to delete all the relative path handling needed to
report at least somewhat user-legible errors before.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agoscripts: check-html-references: Rename --prefix to --webroot and make it mandatory
Peter Krempa [Tue, 14 Feb 2023 11:05:30 +0000 (12:05 +0100)] 
scripts: check-html-references: Rename --prefix to --webroot and make it mandatory

Force users to pass the path to the root of the webpage the script
should check. The script lives in a different subdirectory so the
default of the current directory doesn't make much sense.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agodocs: XSL: Add source document name as custom data attribute for <html>
Peter Krempa [Tue, 14 Feb 2023 10:59:22 +0000 (11:59 +0100)] 
docs: XSL: Add source document name as custom data attribute for <html>

The html standard allows custom data attributes on any element in the
format of 'data-*' which are not interpreted. We can use it to embed the
name of the source document used to generate the page so that our
checker tools can use the friendly name.

https://html.spec.whatwg.org/multipage/dom.html#embedding-custom-non-visible-data-with-the-data-*-attributes

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agovirLogCleanerShutdown: Don't call g_regex_unref on NULL regex
Peter Krempa [Wed, 15 Feb 2023 10:02:10 +0000 (11:02 +0100)] 
virLogCleanerShutdown: Don't call g_regex_unref on NULL regex

Shutdown of virtlogd prints:

  (process:54742): GLib-CRITICAL **: 11:00:40.873: g_regex_unref: assertion 'regex != NULL' failed

Use g_clear_pointer instead which prevents it in the NULL case.

Fixes: 69eeef5dfbf
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agorpc: Don't warn about "max_client_requests" in single-threaded daemons
Peter Krempa [Wed, 15 Feb 2023 09:48:31 +0000 (10:48 +0100)] 
rpc: Don't warn about "max_client_requests" in single-threaded daemons

The warning about max_client_requests is hit inside virtlogd every time
a VM starts which spams the logs.

Emit the warning only when the client request limit is not 1 and add a
warning into the daemon config to not configure it too low instead.

Fixes: 031878c2364
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2145188
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agorpc: client: Don't check return value of virNetMessageNew
Peter Krempa [Wed, 15 Feb 2023 09:43:53 +0000 (10:43 +0100)] 
rpc: client: Don't check return value of virNetMessageNew

virNetServerClientDispatchRead checked the return value but it's not
necessary any more as it can't return NULL nowadays.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agotest: Introduce chxml2xmltest
Michal Privoznik [Fri, 10 Feb 2023 11:40:04 +0000 (12:40 +0100)] 
test: Introduce chxml2xmltest

Whilst reviewing a patch upstream (that ended up as
v9.0.0-200-g092176e5ec), I realized we don't have a single
xml2xml test for CH driver. Well, introduce the test with one
simple test case for now.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoqemu: blockjob: Handle 'pending' blockjob state only when we need it
Peter Krempa [Fri, 10 Feb 2023 16:16:43 +0000 (17:16 +0100)] 
qemu: blockjob: Handle 'pending' blockjob state only when we need it

The 'pending' state needs to be handled by the blockjob code only when
the snapshot code requests a block-commit without auto-finalization.

If we always handle it we fail to properly remove the blockjob data for
the 'blockdev-create' job as that also transitions trhough 'pending' but
we'd never update it once it reaches 'concluded' as the code already
thinks that the job has finished and is no longer watching it.

Introduce a 'processPending' property into block job data and set it
only when we know that we need to process 'pending'.

Fixes: 90d9bc9d74a5157167548b26c00b1a016655e295
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2168769
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2 years agoTranslated using Weblate (Polish)
Piotr Drąg [Mon, 13 Feb 2023 12:20:21 +0000 (13:20 +0100)] 
Translated using Weblate (Polish)

Currently translated at 22.0% (2292 of 10405 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/pl/

Co-authored-by: Piotr Drąg <piotrdrag@gmail.com>
Signed-off-by: Piotr Drąg <piotrdrag@gmail.com>
2 years agoconf: Allow conventional PCI devices to be marked as integrated
Andrea Bolognani [Thu, 9 Feb 2023 17:15:08 +0000 (18:15 +0100)] 
conf: Allow conventional PCI devices to be marked as integrated

Integrated PCI devices can be either PCIe (virtio-iommu) or
conventional PCI (pvpanic-pci). Right now libvirt will refuse
to assign an address on pcie.0 for the latter, but that's an
undesirable limitation that we can easily remove.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2 years agoch: Do not add stub console to ch VMs
Praveen K Paladugu [Thu, 9 Feb 2023 22:09:28 +0000 (22:09 +0000)] 
ch: Do not add stub console to ch VMs

virDomainDefAddConsoleCompat in post parsing step appends a stub console
of type VIR_DOMAIN_CHR_TYPE_NULL to ch VMs' Domain XML. Cloud-hypervisor's
deviceValidateCallback (chValidateDomainDeviceDef) checks that the type of
stub console is not of type VIR_DOMAIN_CHR_TYPE_PTY and throws an error.

This commit introduces NO_STUB_CONSOLE feature check to Domain features and
uses it to skip adding stub console to ch VMs.

Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2 years agoqemu_extdevice: Do cleanup host only for VIR_DOMAIN_TPM_TYPE_EMULATOR
Michal Privoznik [Fri, 10 Feb 2023 08:47:05 +0000 (09:47 +0100)] 
qemu_extdevice: Do cleanup host only for VIR_DOMAIN_TPM_TYPE_EMULATOR

We only set up host for VIR_DOMAIN_TPM_TYPE_EMULATOR and thus
similarly, we should do cleanup for the same type. This also
fixes a crasher, in which qemuTPMEmulatorCleanupHost() accesses
tpm->data.emulator.storagepath which is NULL for
VIR_DOMAIN_TPM_TYPE_EXTERNAL.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2168762
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agocpu_s390: Implement getVendorForModel for IBM Z
Thomas Huth [Fri, 25 Nov 2022 10:52:55 +0000 (11:52 +0100)] 
cpu_s390: Implement getVendorForModel for IBM Z

When running "virsh domcapabilities" on a s390x host, all the CPU
models show up with vendor='unknown' - which sounds kind of weird
since the vendor of these mainframe CPUs is well known: IBM.
All CPUs starting with either "z" or "gen" match a real mainframe
CPU by IBM, so let's return the string "IBM" for those now.
The only remaining ones are now the artifical "qemu" and "max"
models from QEMU itself, so it should be OK to get an "unknown"
vendor for those two.

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Boris Fiuczynski<fiuczy@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2 years agoqemu: allow passt to self-daemonize
Laine Stump [Wed, 8 Feb 2023 23:13:10 +0000 (18:13 -0500)] 
qemu: allow passt to self-daemonize

I initially had the passt process being started in an identical
fashion to the slirp-helper - libvirt was daemonizing the new process
and recording its pid in a pidfile. The problem with this is that,
since it is daemonized immediately, any startup error in passt happens
after the daemonization, and thus isn't seen by libvirt - libvirt
believes that the process has started successfully and continues on
its merry way. The result was that sometimes a guest would be started,
but there would be no passt process for qemu to use for network
traffic.

Instead, we should be starting passt in the same manner we start
dnsmasq - we just exec it as normal (along with a request that passt
create the pidfile, which is just another option on the passt
commandline) and wait for the child process to exit; passt then has a
chance to parse its commandline and complete all the setup prior to
daemonizing itself; if it encounters an error and exits with a non-0
code, libvirt will see the code and know about the failure. We can
then grab the output from stderr, log that so the "user" has some idea
of what went wrong, and then fail the guest startup.

Signed-off-by: Laine Stump <laine@redhat.com>
2 years agoqemuProcessRefreshDisks: Don't skip filling of disk information if tray state didn...
Peter Krempa [Thu, 9 Feb 2023 08:40:32 +0000 (09:40 +0100)] 
qemuProcessRefreshDisks: Don't skip filling of disk information if tray state didn't change

Commit 5ef2582646eb98 added emitting of even when refreshign disk state,
where it wanted to avoid sending the event if disk state didn't change.
This was achieved by using 'continue' in the loop filling the
information. Unfortunately this skips extraction of whether the device
has a tray which is propagated into internal structures, which in turn
broke cdrom media change as the code thought there's no tray for the
device.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2166411
Fixes: 5ef2582646eb98af208ce37355f82bdef39931fa
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
2 years agoremote_driver: Work around broken clang
Michal Privoznik [Thu, 9 Feb 2023 07:38:17 +0000 (08:38 +0100)] 
remote_driver: Work around broken clang

In recent commit of v9.0.0-191-gc71c159248 I've introduced
remoteConnectFormatURI() function and in the function @query
variable. Even though, the variable is used, clang-13 fails to
see it. Surprisingly, newer clang is not affected. Fortunately,
swapping the order in which variables are set makes clang happy
again.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2 years agoRevert ".gitignore: Ignore cscope and other *tags files"
Martin Kletzander [Mon, 6 Feb 2023 14:05:38 +0000 (15:05 +0100)] 
Revert ".gitignore: Ignore cscope and other *tags files"

This reverts commit f2d379e7cb802f922409c35e4831ee52a2162486.

Any tool-related ignores should go to user's global ignore file or the user's
local exclude file which is per-project.  See git-config(1) and gitignore(5) for
more details.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Not-Ignored-by: Ján Tomko <jtomko@redhat.com>
2 years agoremote: Pass 'mode' and 'socket' URI parameters to virt-ssh-helper
Michal Privoznik [Fri, 3 Feb 2023 14:22:18 +0000 (15:22 +0100)] 
remote: Pass 'mode' and 'socket' URI parameters to virt-ssh-helper

When handling virConnectOpen(), we parse given URI, specifically
all those parameters we know, like ?mode, ?socket, ?name, etc.
ignoring those we don't recognize yet. Then, we reconstruct the
URI back, but ignoring all parameters we've parsed. In other
words:

  qemu:///system?mode=legacy&foo=bar

becomes:

  qemu:///system?foo=bar

The reconstructed URI is then passed to the corresponding driver
(QEMU in our example) with intent of it parsing parameters
further (or just ignoring them). But for some transport modes,
where virt-ssh-helper is ran on the remote host (libssh, libssh2,
ssh) we need to pass ?mode and ?socket parameters, so that it can
do the right thing, e.g. for 'mode=legacy' start the monolithic
daemon, or for 'socket=' connect to the given socket.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/433
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2 years agoviruri: Introduce virURIParamsSetIgnore()
Michal Privoznik [Fri, 3 Feb 2023 14:10:23 +0000 (15:10 +0100)] 
viruri: Introduce virURIParamsSetIgnore()

The aim of this helper is to manipulate the .ignore value for
given list of parameters. For instance:

  virURIParamsSetIgnore(uri, false, {"mode", "socket", NULL});

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2 years agoremote_driver: Move URI re-generation into a function
Michal Privoznik [Fri, 3 Feb 2023 13:57:47 +0000 (14:57 +0100)] 
remote_driver: Move URI re-generation into a function

There's a piece of code in doRemoteOpen() that is going to be
called twice. Instead of duplicating the code, move it into a
function that will be called twice, later on.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2 years agovirt-ssh-helper: Accept ?socket= in connection URI
Michal Privoznik [Fri, 3 Feb 2023 09:52:06 +0000 (10:52 +0100)] 
virt-ssh-helper: Accept ?socket= in connection URI

Similarly to the previous commit, let's accept "socket" parameter
in the connection URI. This change will allow us to use
virt-ssh-helper instead of 'nc' in all cases (done in one of
future commits).

Please note, when the parameter is used it effectively disables
automatic daemon spawning and an error is reported. But this is
intentional - so that the helper behaves just like regular
virConnectOpen() with different transport than ssh, e.g. unix.

But this 'change' is acceptable - there's no way for users to
make our remote code pass the argument to virt-ssh-helper, yet.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2 years agovirt-ssh-helper: Accept ?mode= in connection URI
Michal Privoznik [Thu, 2 Feb 2023 15:51:58 +0000 (16:51 +0100)] 
virt-ssh-helper: Accept ?mode= in connection URI

When split daemons were introduced, we also made connection URI
accept new parameter: mode={auto,legacy,direct} so that a client
can force connecting to either old, monolithic daemon, or to
split daemon (see v5.7.0-rc1~257 for more info).

Now, the change was done to the remote driver, but not to
virt-ssh-helper. True, our remote driver code still does not pass
the 'mode' parameter, but that will be addressed in next commits.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2 years agodoRemoteOpen(): Rename 'failed' label to 'error'
Michal Privoznik [Fri, 3 Feb 2023 08:53:28 +0000 (09:53 +0100)] 
doRemoteOpen(): Rename 'failed' label to 'error'

Our own coding style suggest not inventing new names for labels
and stick with 'cleanup' (when the path is used in both,
successful and unsuccessful returns), or 'error' (when the code
below the label is used only upon error). Well, 'failed' label
falls into the latter category. Rename it then.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2 years agoDrop checks for virURIFormat() retval
Michal Privoznik [Fri, 3 Feb 2023 14:38:03 +0000 (15:38 +0100)] 
Drop checks for virURIFormat() retval

The virURIFormat() function either returns a string, or aborts
(on OOM). There's no way this function can return NULL (as of
v7.2.0-rc1~277). Therefore, it doesn't make sense to check its
retval against NULL.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2 years agoviruri: Search params case insensitively
Michal Privoznik [Fri, 3 Feb 2023 09:23:14 +0000 (10:23 +0100)] 
viruri: Search params case insensitively

Our URI handling code (doRemoteOpen() specifically), uses case
insensitive parsing of query part of URI. For instance:

  qemu:///system?socket=/some/path
  qemu:///system?SoCkEt=/some/path

are the same URI. Even though the latter is probably not used
anywhere, let's switch to STRCASEEQ() instead of STREQ() at two
places: virURIGetParam() and virURICheckUnixSocket().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2 years agoci: Test with latest Avocado again
Erik Skultety [Mon, 6 Feb 2023 15:36:08 +0000 (16:36 +0100)] 
ci: Test with latest Avocado again

Test with the following fix:
https://github.com/avocado-framework/avocado/pull/5567/commits

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agoqemu_namespace: Deal with nested mounts when umount()-ing /dev
Michal Privoznik [Tue, 7 Feb 2023 14:06:32 +0000 (15:06 +0100)] 
qemu_namespace: Deal with nested mounts when umount()-ing /dev

In one of recent commits (v9.0.0-rc1~106) I've made our QEMU
namespace code umount the original /dev. One of the reasons was
enhanced security, because previously we just mounted a tmpfs
over the original /dev. Thus a malicious QEMU could just
umount("/dev") and it would get to the original /dev with all
nodes.

Now, on some systems this introduced a regression:

   failed to umount devfs on /dev: Device or resource busy

But how this could be? We've moved all file systems mounted under
/dev to a temporary location. Or have we? As it turns out, not
quite. If there are two file systems mounted on the same target,
e.g. like this:

  mount -t tmpfs tmpfs /dev/shm/ && mount -t tmpfs tmpfs /dev/shm/

then only the top most (i.e. the last one) is moved. See
qemuDomainUnshareNamespace() for more info.

Now, we could enhance our code to deal with these "doubled" mount
points. Or, since it is the top most file system that is
accessible anyways (and this one is preserved), we can
umount("/dev") in a recursive fashion.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2167302
Fixes: 379c0ce4bfed8733dfbde557c359eecc5474ce38
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
2 years agoqemu_process: Produce better debug message wrt domain namespaces
Michal Privoznik [Tue, 7 Feb 2023 09:34:40 +0000 (10:34 +0100)] 
qemu_process: Produce better debug message wrt domain namespaces

When going through debug log of a domain startup process, one can
meet the following line:

  debug : qemuProcessLaunch:7668 : Building mount namespace

But this is in fact wrong. Firstly, domain namespaces are just
enabled in domain's privateData. Secondly, the debug message says
nothing about actual state of namespace - whether it was enabled
or not.

Therefore, move the debug printing into
qemuProcessEnableDomainNamespaces() and tweak it so that the
actual value is reflected.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
2 years agoqemu: Jump to cleanup label on umount failure
Jim Fehlig [Mon, 6 Feb 2023 17:40:12 +0000 (10:40 -0700)] 
qemu: Jump to cleanup label on umount failure

Similar to other error paths in qemuDomainUnshareNamespace(), jump to
the cleanup label on umount error instead of directly returning -1.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2 years agoqemuProcessLaunch: Tighten rules for external devices wrt incoming migration
Michal Privoznik [Fri, 27 Jan 2023 12:59:08 +0000 (13:59 +0100)] 
qemuProcessLaunch: Tighten rules for external devices wrt incoming migration

When starting a guest, helper processes are started first. But
they need a bit of special handling. Just consider a regular cold
boot and an incoming migration. For instance, in case of swtpm
with its state on a shared volume, we want to set label on the
state for the cold boot case, but don't want to touch the label
in case of incoming migration (because the source very
specifically did not restore it either).

Until now, these two cases were differentiated by testing
@incoming against NULL. And while that makes sense for other
aspects of domain startup, for external devices we need a bit
more, because a restore from a save file is also 'incoming
migration'.

Now, there is a difference between regular migration and restore
from a save file. In the former case we do not want to set
seclabels in the save state. BUT, in the latter case we do need
to set them, because the code that saves the machine restored
seclabels.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2161557
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoqemuExtTPMStop: Restore TPM state label more often
Michal Privoznik [Fri, 27 Jan 2023 09:46:55 +0000 (10:46 +0100)] 
qemuExtTPMStop: Restore TPM state label more often

When stopping swtpm we can restore the label either on just the
swtpm's domain specific logfile (/var/log/swtpm/libvirt/qemu/...),
or on the logfile and the state too (/var/lib/libvirt/swtpm/...).

The deciding factor is whether the guest is stopped because of
outgoing migration OR the state is on a shared filesystem.

But this is not correct condition, because for instance saving the
guest into a file (virsh save) is also an outgoing migration.
Alternatively, when the swtpm state is stored on a shared
filesystem, but the guest is destroyed (virsh destroy), i.e.
stopped because of different reason than migration, we want to
restore the seclabels.

The correct condition is: skip restoring the state on outgoing
migration AND shared filesystem.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2161557
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoqemuProcessStop: Fix detection of outgoing migration for external devices
Michal Privoznik [Fri, 27 Jan 2023 09:45:50 +0000 (10:45 +0100)] 
qemuProcessStop: Fix detection of outgoing migration for external devices

When cleaning up host in qemuProcessStop(), our external helper
processes (e.g. swtpm) want to know whether the domain is being
migrated out or not (so that they restore seclabels on a device
state that's on a shared storage).

This fact is reflected in the @outgoingMigration variable which
is set to true if asyncJob is anything but
VIR_ASYNC_JOB_MIGRATION_IN. Well, we have a specific job for
outgoing migration (VIR_ASYNC_JOB_MIGRATION_OUT) and thus we
should check for that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agovirhostdevtest: Decrease possibility of uninitialized @subsys
Michal Privoznik [Mon, 6 Feb 2023 15:03:44 +0000 (16:03 +0100)] 
virhostdevtest: Decrease possibility of uninitialized @subsys

With the current way the myInit() is written, it's fairly easy to
miss initialization of @subsys variable as the variable is
allocated firstly on the stack and then it's assigned to
hostdev[i] which was allocated using g_new0() (this it is
containing nothing but all zeroes).

Make the subsys point to the corresponding member in hostdev[i]
from the start. This way only the important bits are overwritten
and the rest stays initialized to zero.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agovirhostdevtest: Initialize hostdev @subsys
Michal Privoznik [Mon, 6 Feb 2023 14:55:31 +0000 (15:55 +0100)] 
virhostdevtest: Initialize hostdev @subsys

With recent work on storing original PCI stats in
_virDomainHostdevSubsysPCI struct, the virhostdevtest can across
a latent bug we had. Only some parts of the
virDomainHostdevSubsys structure are initialized. Incidentally,
subsys->u.pci.origstates is not one of them. This lead to
unexpected crashes at runtime.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agologging: use the log cleaner
Oleg Vasilev [Mon, 30 Jan 2023 15:00:02 +0000 (21:00 +0600)] 
logging: use the log cleaner

Actually use the log cleaner introduced by previous commit.

Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agologging: add log cleanup for obsolete domains
Oleg Vasilev [Mon, 30 Jan 2023 15:00:01 +0000 (21:00 +0600)] 
logging: add log cleanup for obsolete domains

Before, logs from deleted machines have been piling up, since there were
no garbage collection mechanism. Now, virtlogd can be configured to
periodically scan the log folder for orphan logs with no recent modifications
and delete it.

A single chain of recent and rotated logs is deleted in a single transaction.

Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agologging: add configuration for future log cleaner
Oleg Vasilev [Mon, 30 Jan 2023 15:00:00 +0000 (21:00 +0600)] 
logging: add configuration for future log cleaner

We want to specify the folder to clean and how much time can a log
chain live.

Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agologging: move virLogHandler to header
Oleg Vasilev [Mon, 30 Jan 2023 14:59:59 +0000 (20:59 +0600)] 
logging: move virLogHandler to header

Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agologging: refactor to store config inside log handler
Oleg Vasilev [Mon, 30 Jan 2023 14:59:58 +0000 (20:59 +0600)] 
logging: refactor to store config inside log handler

Signed-off-by: Oleg Vasilev <oleg.vasilev@virtuozzo.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agoconf: Use proper type for 'type' field of struct _virDomainDeviceDef
Peter Krempa [Fri, 2 Dec 2022 13:55:02 +0000 (14:55 +0100)] 
conf: Use proper type for 'type' field of struct _virDomainDeviceDef

Use virDomainDeviceType as type and update all switch statements which
didn't mention all possible values.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agovirDomainDeviceDefParse: Separate code for parsing type
Peter Krempa [Fri, 2 Dec 2022 13:46:42 +0000 (14:46 +0100)] 
virDomainDeviceDefParse: Separate code for parsing type

Move the code into a new function named virDomainDeviceDefParseType. The
separation will make it easier to change the type of the 'type' field in
side of virDomainDeviceDef.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoqemuDomainGetStatsVcpu: Refactor cleanup
Peter Krempa [Thu, 2 Feb 2023 15:46:19 +0000 (16:46 +0100)] 
qemuDomainGetStatsVcpu: Refactor cleanup

Automatically free 'cpuinfo' and remove the cleanup label and ret
variable.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2 years agoqemu: agent: Use virJSONValueObjectGetArray
Peter Krempa [Thu, 2 Feb 2023 15:43:30 +0000 (16:43 +0100)] 
qemu: agent: Use virJSONValueObjectGetArray

Replace virJSONValueObjectGet + virJSONValueIsArray by the single API
which returns only an array.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2 years agoqemu_monitor_json: Replace simplify fetching Array from JSON object
Peter Krempa [Tue, 6 Dec 2022 14:29:28 +0000 (15:29 +0100)] 
qemu_monitor_json: Replace simplify fetching Array from JSON object

Replace instances of virJSONValueObjectGet + virJSONValueIsArray by
virJSONValueObjectGetArray.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2 years agoqemuMonitorJSONQueryStats: Simplify logic to construct 'provider_list'
Peter Krempa [Wed, 9 Nov 2022 15:59:25 +0000 (16:59 +0100)] 
qemuMonitorJSONQueryStats: Simplify logic to construct 'provider_list'

Simplify construction of a single provider by using
virJSONValueObjectAdd and restructuring the code.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2 years agovirbitmap: Allow NULL bitmap in functions returning index of a set/clear bit
Peter Krempa [Wed, 9 Nov 2022 15:54:56 +0000 (16:54 +0100)] 
virbitmap: Allow NULL bitmap in functions returning index of a set/clear bit

virBitmapNextSetBit/virBitmapLastSetBit/virBitmapNextClearBit can be
used for iteration of a bitmap. Allow NULL bitmap so that iteration of a
bitmap can be simplified in certain cases.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2 years agoconf: Store 'origstates' of PCI hostdevs in a bitmap
Peter Krempa [Thu, 6 Oct 2022 11:17:00 +0000 (13:17 +0200)] 
conf: Store 'origstates' of PCI hostdevs in a bitmap

Refactor the code to use a bitmap with an enum.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agoqemustatusxml2xmltest: Add test data for testing '<origstates>' of PCI hostdev
Peter Krempa [Thu, 2 Feb 2023 14:54:09 +0000 (15:54 +0100)] 
qemustatusxml2xmltest: Add test data for testing '<origstates>' of PCI hostdev

The <origstates> XML element captures private data of a PCI device
needed to restore it after a VM is started. Unfortunately at the point
when it was added we didn't yet have the existing private data
infrastructure.

Since the element is parsed only in cases similar to the status XML we
need to test it there.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agovirBitmapIsBitSet: Allow NULL bitmap
Peter Krempa [Wed, 9 Nov 2022 15:54:56 +0000 (16:54 +0100)] 
virBitmapIsBitSet: Allow NULL bitmap

The virBitmapIsBitSet API is a permissive one which returns false when
the bit is not set or is out of range. We can do the same if the bitmap
is NULL to aid certain situations when this can happen, but we don't
want to add extra checks.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agovirNetworkDHCPHostDefParseXML: Use virXMLNodeGetSubelement to find 'lease'
Peter Krempa [Fri, 2 Dec 2022 10:02:58 +0000 (11:02 +0100)] 
virNetworkDHCPHostDefParseXML: Use virXMLNodeGetSubelement to find 'lease'

This also prevents a potential memleak when multiple elements would be
present.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agovirNetworkDHCPRangeDefParseXML: Use virXMLNodeGetSubelement to find 'lease'
Peter Krempa [Fri, 2 Dec 2022 10:02:58 +0000 (11:02 +0100)] 
virNetworkDHCPRangeDefParseXML: Use virXMLNodeGetSubelement to find 'lease'

This also prevents a potential memleak when multiple elements would be
present.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agovirNetDevVPortProfileParse: Use virXMLNodeGetSubelement to find '<parameters>'
Peter Krempa [Fri, 2 Dec 2022 10:01:06 +0000 (11:01 +0100)] 
virNetDevVPortProfileParse: Use virXMLNodeGetSubelement to find '<parameters>'

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agovirPCIDeviceAddressParseXML: Use virXMLNodeGetSubelement to find 'zpci'
Peter Krempa [Thu, 1 Dec 2022 10:02:05 +0000 (11:02 +0100)] 
virPCIDeviceAddressParseXML: Use virXMLNodeGetSubelement to find 'zpci'

Use the helper designed to find the subelement. A slight semantic
difference after this patch is that the first <zpci> element will be
considered instead of the last, but only one is expected in a valid XML.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agotools: Fix detection of remote libvirt access in virt-qemu-sev-validate
Jim Fehlig [Thu, 2 Feb 2023 18:04:20 +0000 (11:04 -0700)] 
tools: Fix detection of remote libvirt access in virt-qemu-sev-validate

The VM's firmware path is not extracted from the XML when invoking
virt-qemu-sev-validate in insecure mode and connecting to the local libvirt

virt-qemu-sev-validate --insecure --tk tek-tik.bin --domain test-sev-es
ERROR: Cannot access firmware path remotely

The test for remote access compares the return value from socket.gethostname()
to the return value from conn.getHostname(). The former doesn't always return
the fqdn, whereas the latter does. Use socket.getfqdn() instead.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years agodocs: Fix examples in virt-qemu-sev-validate man page
Jim Fehlig [Thu, 2 Feb 2023 18:00:18 +0000 (11:00 -0700)] 
docs: Fix examples in virt-qemu-sev-validate man page

Some of the examples refer to virt-dom-sev-validate. Replace them with
the proper name.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2 years ago.gitignore: Ignore cscope and other *tags files
Martin Kletzander [Thu, 2 Feb 2023 14:47:43 +0000 (15:47 +0100)] 
.gitignore: Ignore cscope and other *tags files

Commit f7114e61dbc2 cleaned up way too much and now that I have cscope
working again I noticed there are some files that ought to stay ignored.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agotools: use g_autofree more
Ján Tomko [Wed, 1 Feb 2023 15:08:22 +0000 (16:08 +0100)] 
tools: use g_autofree more

Remove some obvious uses of VIR_FREE in favor of automatic cleanup.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2 years agoschema: storage: Allow interleaving of 'cipher' and 'ivgen' elements
Peter Krempa [Thu, 13 Oct 2022 16:46:24 +0000 (18:46 +0200)] 
schema: storage: Allow interleaving of 'cipher' and 'ivgen' elements

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoschema: nodedev: Allow interleaving sub-elements of 'css' address type
Peter Krempa [Thu, 13 Oct 2022 16:29:55 +0000 (18:29 +0200)] 
schema: nodedev: Allow interleaving sub-elements of 'css' address type

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoschema: nodedev: Allow interleaving of sub-elements of 'device'
Peter Krempa [Thu, 13 Oct 2022 16:29:49 +0000 (18:29 +0200)] 
schema: nodedev: Allow interleaving of sub-elements of 'device'

Note that the schema doesn't allow us to represent the two branches of
optional <devnode type='dev'> and zero or more <devnode type='link'>
definitions, so I've merged them under the <zeroOrMore> case.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoschema: domain: Allow interleaving of 'inituser/initgroup' in 'osexe' definition
Peter Krempa [Thu, 13 Oct 2022 16:16:19 +0000 (18:16 +0200)] 
schema: domain: Allow interleaving of 'inituser/initgroup' in 'osexe' definition

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoschema: domain: Allow interleaving of elements in 'osxen' definition
Peter Krempa [Thu, 13 Oct 2022 15:40:18 +0000 (17:40 +0200)] 
schema: domain: Allow interleaving of elements in 'osxen' definition

The 'osxen' RNG type defines options for the <os> element in certain
modes. Allow interleaving of subelements recursively.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoschema: domain: Allow interleave of 'smartcard' subelements
Peter Krempa [Thu, 13 Oct 2022 14:22:22 +0000 (16:22 +0200)] 
schema: domain: Allow interleave of 'smartcard' subelements

Allow interleave of the top level sub-elements as well as the
subelements in the 'host-certificates' mode. Note that '<interleave>'
doesn't work properly if there's multiple definitions of the same
sub-element in the interleave so for this patch I chose to '<group>' the
'certificate' subelements. Another options would require us to stop
enforcing that there's exactly 3 of them.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoqemu: Don't remove macvtaps on failed start
Michal Privoznik [Tue, 31 Jan 2023 09:24:01 +0000 (10:24 +0100)] 
qemu: Don't remove macvtaps on failed start

If a domain is configured to create a macvtap/macvlan but the
target link already exists, startup fails (as expected) with:

  error: error creating macvtap interface test@eth0 (52:54:00:d9:0b:db): File exists

Okay, we could make that error message better, but that's not the
point. Since this error originated while generating cmd line
(the caller is qemuProcessStart(), transitively), the cleanup
after failed start is performed (qemuProcessStop()). Here,
virNetDevMacVLanDeleteWithVPortProfile() is called which removes
the macvtap interface we did not create (as it made us fail in
the first place).

Therefore, we need to track which macvtap/macvlan interface was
created successfully and remove only those.

You'll notice that only qemuProcessStop() has the new check. For
the (failed) hotplug case (qemuDomainAttachNetDevice()) this
function is already in place (the @iface_connected variable), or
not needed (qemuDomainRemoveNetDevice() - we're removing an
interface that was already attached to QEMU).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2166235
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoconf: Format and parse private data for virDomainNetDef
Michal Privoznik [Wed, 1 Feb 2023 08:02:19 +0000 (09:02 +0100)] 
conf: Format and parse private data for virDomainNetDef

The virDomainNetDef struct has privateData (which is currently
used by QEMU driver to store FDs opened during cmd line building
phase and pass them onto cmd line).

Soon, we will need to store additional information that needs to
survive daemon restart. Let's introduce machinery for parsing and
formatting privateData.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agovirnetdevmacvlan: Drop G_GNUC_WARN_UNUSED_RESULT annotation for virNetDevMacVLanDelet...
Michal Privoznik [Wed, 1 Feb 2023 12:29:37 +0000 (13:29 +0100)] 
virnetdevmacvlan: Drop G_GNUC_WARN_UNUSED_RESULT annotation for virNetDevMacVLanDeleteWithVPortProfile()

Every single caller of the
virNetDevMacVLanDeleteWithVPortProfile() function is calling it
wrapped inside of ignore_value() macro. This is because the
function is annotated as G_GNUC_WARN_UNUSED_RESULT. This makes no
sense. Drop the annotation and the macro envelope.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agodomain_conf: Rewrite virDomainChrSourceModeTypeFromString() using VIR_ENUM_IMPL()
Michal Privoznik [Wed, 1 Feb 2023 08:24:55 +0000 (09:24 +0100)] 
domain_conf: Rewrite virDomainChrSourceModeTypeFromString() using VIR_ENUM_IMPL()

In domain_conf.c there's virDomainChrSourceModeTypeFromString()
which is open coded. Let's rewrite it using VIR_ENUM_DECL() +
VIR_ENUM_IMPL() combo.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agodomain_conf: Move virDomainNetVhostuserMode enum declaration
Michal Privoznik [Wed, 1 Feb 2023 08:24:27 +0000 (09:24 +0100)] 
domain_conf: Move virDomainNetVhostuserMode enum declaration

While it's true that the virDomainNetVhostuserMode enum is used
solely in virDomainNetDefParseXML(), its placement just above the
function is rather unfortunate. Let's put it at the beginning of
the file with the rest of the enum declarations/implementations.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2 years agoqemu: fix a typo
Ján Tomko [Wed, 1 Feb 2023 12:12:20 +0000 (13:12 +0100)] 
qemu: fix a typo

s/usw/use/

Signed-off-by: Ján Tomko <jtomko@redhat.com>
2 years agoqemu: block: Properly handle FD-passed disk hot-(un-)plug
Peter Krempa [Tue, 31 Jan 2023 14:35:05 +0000 (15:35 +0100)] 
qemu: block: Properly handle FD-passed disk hot-(un-)plug

The hotplug code paths need to be able to pass the FDs to the monitor to
ensure that hotplug works.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agoqemu: domain: Store fdset ID for disks passed to qemu via FD
Peter Krempa [Tue, 31 Jan 2023 14:30:51 +0000 (15:30 +0100)] 
qemu: domain: Store fdset ID for disks passed to qemu via FD

To ensure that we can hot-unplug the disk including the associated fdset
we need to store the fdset ID in the status XML.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agoqemu: fd: Add helpers allowing storing FD set data in status XML
Peter Krempa [Tue, 31 Jan 2023 14:25:57 +0000 (15:25 +0100)] 
qemu: fd: Add helpers allowing storing FD set data in status XML

Rollback of FD sets passed to qemu is also needed after possible restart
of libvirtd when we need to serialize the data into status XML. For this
purpose we need to access the fdset ID once it was passed to qemu and
potentially re-create a 'qemuFDPass' struct in passed state.

Introduce 'qemuFDPassNewPassed' and 'qemuFDPassIsPassed'.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agoqemuFDPassTransferCommand: Mark that FD was passed
Peter Krempa [Tue, 31 Jan 2023 16:26:43 +0000 (17:26 +0100)] 
qemuFDPassTransferCommand: Mark that FD was passed

Until now the code didn't expect that we'd want to rollback/detach a FD
passed on the commandline, but whith disk backend FD passing this can
happen.

Properly mark the 'qemuFDPass' object as passed to qemu even when it was
done on the commandline.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agoqemu: command: Handle FD passing commandline via qemuBuildBlockStorageSourceAttachDat...
Peter Krempa [Tue, 31 Jan 2023 13:37:40 +0000 (14:37 +0100)] 
qemu: command: Handle FD passing commandline via qemuBuildBlockStorageSourceAttachDataCommandline

Copy the pointer to qemuFDPass into struct qemuBlockStorageSourceAttachData
so that it can be used from qemuBuildBlockStorageSourceAttachDataCommandline
rather than looping again in qemuBuildDiskSourceCommandLineFDs.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agoqemuStorageSourcePrivateDataFormat: Rename 'tmp' to 'objectsChildBuf'
Peter Krempa [Tue, 31 Jan 2023 14:19:58 +0000 (15:19 +0100)] 
qemuStorageSourcePrivateDataFormat: Rename 'tmp' to 'objectsChildBuf'

Be consistent with other children buffer variable naming scheme.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agoqemu_fd: Remove declaration for 'qemuFDPassNewDirect'
Peter Krempa [Tue, 31 Jan 2023 14:23:54 +0000 (15:23 +0100)] 
qemu_fd: Remove declaration for 'qemuFDPassNewDirect'

The function doesn't exist any more.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agostorageBackendCreateQemuImgSecretPath: Refactor cleanup
Peter Krempa [Thu, 8 Dec 2022 15:39:50 +0000 (16:39 +0100)] 
storageBackendCreateQemuImgSecretPath: Refactor cleanup

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2 years agolibxlMakeNetworkDiskSrc: Refactor cleanup
Peter Krempa [Thu, 8 Dec 2022 11:24:30 +0000 (12:24 +0100)] 
libxlMakeNetworkDiskSrc: Refactor cleanup

Automatically unref the 'conn' object and remove the 'cleanup' section
and 'ret' variable.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>