Remove all the universal code since the 'else' part formats commandline
only for the SD card based disk. Note that we can use virDiskNameToIndex
without the check as we already validate that 'disk->dst' contains a
properly formatted string in the validation code.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

For 'SD' disks and floppies in the pre-blockdev era we don't format
-device. Extract the logic so that it's more clear and add comments.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The function effectively boils down to whether the disk is 'SD'. Since
we'll need to make more decisions based on the fact whether the disk is
on the SD bus, rename the function.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Remove the function and passing of 'def' through the callers.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Previously we've validated it in qemuCheckDiskConfig which was directly
called from the command line generator. Move the checks to the validator
where they belong.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Move the code from qemuCheckDiskConfigBlkdeviotune in
src/qemu/qemu_commandline.c to
qemuValidateDomainDeviceDefDiskBlkdeviotune.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Agregate validation of frontend properties in a new function called
qemuValidateDomainDeviceDefDiskFrontend.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Switch to DO_TEST_CAPS_LATEST for all of them and also add pre-blockdev
case for 'disk-discard' as we had it before.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Move the tests to DO_TEST_CAPS_LATEST. Since switch to blockdev stopped
us formatting the tunning parameters on the command line let's also add
version cases for qemu-4.1 data which doesn't enable blockdev.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

When firewalld is stopped, it removes *all* iptables rules and chains,
including those added by libvirt. Since restarting firewalld means
stopping and then starting it, any time it is restarted, libvirt needs
to recreate all the private iptables chains it uses, along with all
the rules it adds.

We already have code in place to call networkReloadFirewallRules() any
time we're notified of a firewalld start, and
networkReloadFirewallRules() will call
networkPreReloadFirewallRules(), which calls
networkSetupPrivateChains(); unfortunately that last call is called
using virOnce(), meaning that it will only be called the first time
through networkPreReloadFirewallRules() after libvirtd starts - so of
course when firewalld is later restarted, the call to
networkSetupPrivateChains() is skipped.

The neat and tidy way to fix this would be if there was a standard way
to reset a pthread_once_t object so that the next time virOnce was
called, it would think the function hadn't been called, and call it
again. Unfortunately, there isn't any official way of doing that (we
*could* just fill it with 0 and hope for the best, but that doesn't
seem very safe.

So instead, this patch just adds a static variable called
chainInitDone, which is set to true after networkSetupPrivateChains()
is called for the first time, and then during calls to
networkPreReloadFirewallRules(), if chainInitDone is set, we call
networkSetupPrivateChains() directly instead of via virOnce().

It may seem unsafe to directly call a function that is meant to be
called only once, but I think in this case we're safe - there's
nothing in the function that is inherently "once only" - it doesn't
initialize anything that can't safely be re-initialized (as long as
two threads don't try to do it at the same time), and it only happens
when responding to a dbus message that firewalld has been started (and
I don't think it's possible for us to be processing two of those at
once), and even then only if the initial call to the function has
already been completed (so we're safe if we receive a firewalld
restart call at a time when we haven't yet called it, or even if
another thread is already in the process of executing it. The only
problematic bit I can think of is if another thread is in the process
of adding an iptable rule at the time we're executing this function,
but 1) none of those threads will be trying to add chains, and 2) if
there was a concurrency problem with other threads adding iptables
rules while firewalld was being restarted, it would still be a problem
even without this change.

This is yet another patch that fixes an occurrence of this error:

COMMAND_FAILED: '/usr/sbin/iptables -w10 -w --table filter --insert LIBVIRT_INP --in-interface virbr0 --protocol tcp --destination-port 67 --jump ACCEPT' failed: iptables: No chain/target/match by that name.

In particular, this resolves: https://bugzilla.redhat.com/1813830Signed-off-by: NLaine Stump <laine@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

networkSetupPrivateChains() is currently called only once per run of
libvirtd, so it can assume that errInitV4 and errInitV6 are empty/null
when it is called. In preparation for potentially calling this
function multiple times during one run, this patch moves the reset of
errInitV[46] to the top of the function, to assure no memory is
leaked.
Signed-off-by: NLaine Stump <laine@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

As suggested in the linked bug, libvirt should firstly check
whether the major number of the device is device mapper major.
Because if it isn't subsequent DM_DEVICE_DEPS task may not only
fail, but also yield different results. In the bugzilla this is
demonstrated by creating a devmapper target named 'loop0' and
then creating loop target /dev/loop0. When the latter is then
passed to a domain, our virDevMapperGetTargetsImpl() function
blindly asks devmapper to provide target dependencies for
/dev/loop0 and because of the way devmapper APIs work, it will
'sanitize' the input by using the last component only which is
'loop0' and thus return different results than expected.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1823976Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

This format is much easier to tweak and update.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

It's been more than six months since we adopted GLib and we've
been pretty aggressive at replacing our homegrown APIs with more
standard ones, so by now most of the symbols mentioned in this
document haven't been around for quite a long time already.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

With the move to GitLab CI one of the things we miss from Jenkins is a
single page dashboard showing CI status across all projects. This is a
very simple replacement that uses badges for CI pipeline status.

A CSS tweak is needed because RST->HTML adds redundant <p> tags inside
table cells which causes excessive vertical whitespace to appear.
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

We have a framework to register cleanup callbacks that are run
when a domain is shut down. The idea is to run callbacks in
reverse order than they were registered. However, looking at the
code this is not the case. Fortunately, this framework is used to
register a single callback and a single callback only -
qemuMigrationDstPrepareCleanup() - therefore there was no problem
just yet.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Commit a13b2905 missed an adjustment to a test that is only run when
building against xen <= 4.9, where LIBXL_HAVE_BUILDINFO_NESTED_HVM is
not defined. Adjust fullvirt-cpuid-legacy-nest test similar to the others.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

When no video device is specified in config we should set both
hvm.nographic to 1 and hvm.vga.kind to NONE.

Without hvm.vga.kind=LIBXL_VGA_INTERFACE_TYPE_NONE both -nographic and
-device 'cirrus-vga' are on qemu cmdline.
Signed-off-by: NArtur Puzio <contact@puzio.waw.pl>
Reviewed-by: NJim Fehlig <jfehlig@suse.com>

Turns out that it's not enough to pass the qemu:///embed root to
virQEMUDriverConfigNew(), you also have to make sure the same
string is copied into the virQEMUDriver structure yourself, and
not doing so in our case resulted in the cleanup never happening
and in distcheck failing because of that.

On the other hand, actually setting config->embeddedRoot would
result in different paths being generated for each test run, which
would obviously break qemuxml2argvtest, so that's not an option
either.

This reverts commit d98cc196.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

Now that the QEMU driver natively supports storing all its runtime
data inside an arbitrary directory, we can avoid having multiple
copies of the same logic in the test suite.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

To attempt to catch unit tests which accidentally create files in $HOME,
or $XDG_RUNTIME_DIR, poison these env vars by pointing them to
directories which don't exist. This should give easier to debug test
failures. For example:

$ VIR_TEST_DEBUG=1 ./qemuhotplugtest
Could not initialize HostdevManager - operation failed: Failed to create state dir '/bad-test-used-env-xdg-runtime-dir/libvirt/hostdevmgr'
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Fix win32 keycode for VK_OEM_102
Reviewed-by: NLaine Stump <laine@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

We need to prevent accidental deletion of release tags and maint
branches.

We need to ensure that shared CI runners are enabled on all repos.
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

We need this for all tests that use virHostdevManager, because
during creation of this object for unprivileged connections
like those used in the test suite we would end up writing inside
the user's home directory.

That's bad manners in general, but when running the test suite
inside a purposefully constrained environment such as the one
exposed by pbuilder, it turns into an outright test failure:

  Could not initialize HostdevManager - operation failed: Failed
  to create state dir '/nonexistent/.cache/libvirt/hostdevmgr'
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The churn in the output files is caused primarily by the fact that
blockdev support has been introduced.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The churn in the output files is caused primarily by the fact that
replies were generated on a POWER9 machine, which is good because
we didn't have coverage of that before.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

So that we don't have to chase frequent Fedora releases, move the
non-build related jobs onto the long life CentOS 8 distro.
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Currently, qemucapsprobe fails when libvirt is not already installed
on the system:

  $ ./tests/qemucapsprobe /path/to/qemu-system-ppc64 >/dev/null
  I/O warning : failed to load external entity "/usr/share/libvirt/cpu_map/index.xml"
  2020-05-06 09:49:59.136+0000: 269822: info : libvirt version: 6.4.0
  2020-05-06 09:49:59.136+0000: 269822: info : hostname: [...]
  2020-05-06 09:49:59.136+0000: 269822: warning : virQEMUCapsLogProbeFailure:5127 :
  Failed to probe capabilities for /path/to/qemu-system-ppc64: XML error: failed to
  parse xml document '/usr/share/libvirt/cpu_map/index.xml'

It would be great if the tool could work entirely out of the build
directory, and this patch achieves just that.
Suggested-by: NPeter Krempa <pkrempa@redhat.com>
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

We never supported host-model CPUs on ARM and we don't want to support
them even once patches for direct detection of host CPU are merged. And
since using host CPU definition for host-model CPUs exists only for
backward compatibility, we should not use it for any host-model support
added in the future. Such enhancement should exclusively use the result
of query-cpu-model-expansion. Until proper host-model support is
implemented for ARM (if ever), we need to make sure the detected host
CPU is not accidentally used for host-model CPUs.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Start the new capability file for the new development cycle of QEMU.

Note that compared to previous version this was generated on an AMD cpu.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

When a system has enabled the iptables/ip6tables services rather than
firewalld, there is no explicit ordering of the start of those
services vs. libvirtd. This creates a problem when libvirtd.service is
started before ip[6]tables, as the latter, when it finally is started,
will remove all of the iptables rules that had previously been added
by libvirt, including the custom chains where libvirt's rules are
kept. This results in an error message similar to the following when a
user subsequently tries to start a new libvirt network:

 "Error while activating network: Call to virNetworkCreate failed:
 internal error: Failed to apply firewall rules
 /usr/sbin/ip6tables -w --table filter --insert LIBVIRT_FWO \
   --in-interface virbr2 --jump REJECT:
 ip6tables: No chain/target/match by that name."

(Prior to logging this error, it also would have caused failure to
forward (or block) traffic in some cases, e.g. for guests on a NATed
network, since libvirt's rules to forward/block had all been deleted
and libvirt didn't know about it, so it couldn't fix the problem)

When this happens, the problem can be remedied by simply restarting
libvirtd.service (which has the side-effect of reloading all
libvirt-generated firewall rules)

Instead, we can just explicitly stating in the libvirtd.service file
that libvirtd.service should start after ip6tables.service and
ip6tables.service, eliminating the race condition that leads to the
error.

There is also nothing (that I can see) in the systemd .service files
to guarantee that firewalld.service will be started (if enabled) prior
to libvirtd.service. The same error scenario given above would occur
if libvirtd.service started before firewalld.service.  Even before
that, though libvirtd would have detected that firewalld.service was
disabled, and then turn off all firewalld support. So, for example,
firewalld's libvirt zone wouldn't be used, and most likely traffic
from guests would therefore be blocked (all with no external
indication of the source of the problem other than a debug-level log
when libvirtd was started saying that firewalld wasn't in use); also
libvirtd wouldn't notice when firewalld reloaded its rules (which also
simultaneously deletes all of libvirt's rules).

I'm not aware of any reports that have been traced back to
libvirtd.service starting before firewalld.service, but have seen that
error reported multiple times, and also don't see an existing
dependency that would guarantee firewalld.service starts before
libvirtd.service, so it's possible it's been happening and we just
haven't gotten to the bottom of it.

This patch adds an After= line to the libvirtd.service file for each
of iptables.service, ip6tables.service, and firewalld.servicee, which
should guarantee that libvirtd.service isn't started until systemd has
started whichever of the others is enabled.

This race was diagnosed, and patch proposed, by Jason Montleon in
https://bugzilla.redhat.com/1723698 . At the time (April 2019) danpb
agreed with him that this change to libvirtd.service was a reasonable
thing to do, but I guess everyone thought someone else was going to
post a patch, so in the end nobody did.
Signed-off-by: NLaine Stump <laine@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

To make it simpler to answer questions of "Why doesn't this thing work
for me?"
Signed-off-by: NLaine Stump <laine@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

In formatdomain, using 'libxl' and 'xen' is redundant since they now
both refer to the same driver. 'xen' predates 'libxl' and unambiguously
identifies the Xen hypervisor, so drop the use of 'libxl'.

In aclpolkit, the connection URI was erroneously identified as 'libxl'
and the name 'xenlight'. Change the URI to 'xen' and driver name to 'Xen'.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The libxl driver has suffered an identity crisis since its introduction.
It took on the name 'libxl' since at the time libvirt already contained
a 'xen' driver for the old Xen toolstack implementation. 'libxl' is short
for libxenlight, which is often called xenlight. Unfortunately all forms
of the name are used in the libxl driver.

The only remaining use of the 'xenlight' form is when interacting with
the host device manager, which is difficult to change since it would
cause problems when upgrading the driver.

Rename the #define to make it clear the 'xenlight' form is internal and
add a comment describing why the name exists and that its use should be
discouraged.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The libxl driver declares its name as 'Xen' through the public
virConnectGetType() API. In the virHypervisorDriver table the name is
set to 'xenlight'. To add more confusion, the name is set to 'LIBXL'
in the virStateDriver. For consistency, use the same name in the driver
tables as reported in the public virConnectGetType() API.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The virConnectGetType() returns "Xen" for libxl, not "LIBXL".

This prevents users opening a connection to the libxl driver when using
the modular daemons.
Reviewed-by: NJim Fehlig <jfehlig@suse.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

This partially reverts fe65e9c8.

In the referenced commit I removed @ret from
virHostValidateBhyve() thinking it wasn't used when in fact it is
- it's usage is hidden under MODULE_STATUS_WARN(). Reintroduce
the variable back.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>