Connecting a tap device to an Open vSwitch is done by adding a "port"
to the switch with the ovs-vsctl "add-port" command. The port will
have the same name as the tap device, but it is a separate entity, and
can survive beyond the destruction of the tap device (although under
normal circumstances the port will be deleted around the same time the
tap device is deleted).
This makes it possible for a port of a particular name to already
exist at the time libvirt calls ovs-vsctl to add that port. The
original commit of Open vSwitch support (commit df81004632, libvirt
0.9.10, Feb. 2012) used the "--may-exist" option to the add-port
command to indicate that a port of the desired name might already
exist, and that it was okay to simply re-use this port (rather than
failing with an error message).
Then in commit 33445ce844 (libvirt 1.2.7, April 2014) the command
was changed to use "--if-exists del-port blah" instead of
"--may-exist". The reason given was that there was a bug in OVS where
a stale port would be unusable even though it still existed; the
workaround was to forcibly delete any existing port prior to adding
the new port (of the same name). This is the ovs-vsctl command still
in use by libvirt today.
It recently came up in the discussion of a bug concerning guest packet
loss during OpenStack upgrades (https://bugzilla.redhat.com/1963164)
that the bug in OVS that necessitated the del-port workaround was
fixed quite a long time ago (August 2015):
e21c6643a0
thus rendering the workaround in libvirt unnecessary. The assertion in
that discussion is that this workaround is now the cause of the packet
loss being experienced during OpenStack upgrades. I'm not convinced
this is the case, but it does appear that there is no reason to carry
this workaround in libvirt any longer, so this patch reverts the code
back to the original behavior (using "--may-exist" instead of
"--if-exists del-port").
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
I've identified some places (mostly by looking for
virBufferUse()) that can use virXMLFormatElement() instead of
open coded version of it. I'm sure there are many more places
that could use the same treatment. Let's cure them some other
time.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The semicolon in question makes the pipeline fail over a style checker
complaint.
Introduced-in: 360b8eb2d2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virt-host-validate should print "Checking for device assignment IOMMU
support" for all architectures, not only for Intel / AMD.
This is the output without the patch:
```
[fidencio@dentola libvirt]$ virt-host-validate
QEMU: comprobando if device /dev/kvm exists : PASA
QEMU: comprobando if device /dev/kvm is accessible : PASA
QEMU: comprobando if device /dev/vhost-net exists : PASA
QEMU: comprobando if device /dev/net/tun exists : PASA
QEMU: comprobando for cgroup 'cpu' controller support : PASA
QEMU: comprobando for cgroup 'cpuacct' controller support : PASA
QEMU: comprobando for cgroup 'cpuset' controller support : PASA
QEMU: comprobando for cgroup 'memory' controller support : PASA
QEMU: comprobando for cgroup 'devices' controller support : ADVERTENCIA (Enable 'devices' in kernel Kconfig file or mount/enable cgroup controller in your system)
QEMU: comprobando for cgroup 'blkio' controller support : PASA
ADVERTENCIA (Unknown if this platform has IOMMU support)
QEMU: comprobando for secure guest support : ADVERTENCIA (Unknown if this platform has Secure Guest support)
```
This is the output with the patch:
```
[fidencio@dentola libvirt]$ ./build/tools/virt-host-validate
QEMU: Checking if device /dev/kvm exists : PASS
QEMU: Checking if device /dev/kvm is accessible : PASS
QEMU: Checking if device /dev/vhost-net exists : PASS
QEMU: Checking if device /dev/net/tun exists : PASS
QEMU: Checking for cgroup 'cpu' controller support : PASS
QEMU: Checking for cgroup 'cpuacct' controller support : PASS
QEMU: Checking for cgroup 'cpuset' controller support : PASS
QEMU: Checking for cgroup 'memory' controller support : PASS
QEMU: Checking for cgroup 'devices' controller support : WARN (Enable 'devices' in kernel Kconfig file or mount/enable cgroup controller in your system)
QEMU: Checking for cgroup 'blkio' controller support : PASS
QEMU: Checking for device assignment IOMMU support : WARN (Unknown if this platform has IOMMU support)
QEMU: Checking for secure guest support : WARN (Unknown if this platform has Secure Guest support)
```
Signed-off-by: Fabiano Fidêncio <fabiano@fidencio.org>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This was introduced in qemu commit
e11fd68996fb27c040552320f01a7d30a15a7cc1.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This member is unused (apart from only being set in
virCHDriverConfigNew()), and never freed really (leading to a
memleak).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If the chStateInitialize method fails, we call chStateCleanup
which free's all global state. It fails to set the global
'ch_driver' to NULL, however, so a later attempt to open the
cloud hypervisor driver will succeed and then crash attempting
to access freed memory.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Previous patches rendered 'return 0' at the end of the function a
dead code. Therefore, the code can be rearranged a bit and the
line can be dropped.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Fabiano Fidêncio <fabiano@fidencio.org>
Ideally, every virHostMsgFail() would be coupled with
VIR_HOST_VALIDATE_FAILURE() so that the failure is correctly
propagated to the caller. However, in
virHostValidateSecureGuests() we are either ignoring @level and
returning 0 directly (no error), or not returning at all, relying
on 'return 0' at the end of the function. Neither of these help
propagate failure correctly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Fabiano Fidêncio <fabiano@fidencio.org>
When validating secure guests support on s390(x) we may read
/proc/cmdline and look for "prot_virt" argument. Reading the
kernel command line is done via virFileReadValueString() which
may fail. In such case caller won't see any error message. But we
can produce the same warning/error as if "prot_virt" argument
wasn't found. Not only this lets users know about the problem,
it also terminates the "Checking for ...." line correctly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Fabiano Fidêncio <fabiano@fidencio.org>
As a part of its checks, virt-host-validate calls virCgroupNew()
to detect CGroup controllers which are then printed out. However,
virCgroupNew() can fail (with appropriate error message set).
Let's print an error onto stderr if that happens.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Fabiano Fidêncio <fabiano@fidencio.org>
Several libvirt functions are called from virt-host-validate.
Some of these functions do report an error on failure. But
reporting an error is coupled with freeing previous error (by
calling virResetError()). But we've never called
virErrorInitialize() and thus resetting error object frees some
random pointer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Fabiano Fidêncio <fabiano@fidencio.org>
The iscsi-direct storage pool backend works merely like this: a
connection is established to the target (usually done via
virStorageBackendISCSIDirectSetConnection()), intended action is
executed (e.g. reporting LUNs, volume wiping), and at the end the
connection is closed via virISCSIDirectDisconnect().
The problem is that virISCSIDirectDisconnect() reports its own
errors which may overwrite error that occurred during LUN
reporting, or volume wiping or whatever.
To fix this, use virErrorPreserveLast() + virErrorRestore()
combo, which either preserves previously reported error message,
or is NOP if there's no error reported.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1797879
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Xen only supports one firmware, making autoselection easy to implement.
In fact, <os firmware='efi'> is probably preferable in the Xen driver,
where libxl supports a firmware setting with accepted values such as
bios, ovmf, uefi (currently same semantics as ovmf), seabios, etc.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Xen+ovmf does not support secure boot. Fail domain def validation
if secure boot is enabled.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce libxlDomainDefValidate and move the existing validation
check from libxlDomainDefPostParse. Additional validation will be
introduced in subsequent patches.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The audit log contains the following denials from libvirtd
apparmor="DENIED" operation="capable" profile="libvirtd" pid=6012 comm="daemon-init" capability=17 capname="sys_rawio"
apparmor="DENIED" operation="capable" profile="libvirtd" pid=6012 comm="rpc-worker" capability=39 capname="bpf"
apparmor="DENIED" operation="capable" profile="libvirtd" pid=6012 comm="rpc-worker" capability=38 capname="perfmon"
Squelch the denials and allow the capabilities in the libvirtd
apparmor profile.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We are already compiling libvirt with -Wvla - so it does not make
too much sense to still allow people to use alloca() instead. Thus
put it on the list of things we want to warn about. Fortunately,
there is currently no warning with this flag, so the current
sources should be clean.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
The flag has a typo in it, it's "...-than=..." and not "...-then=...",
so this was in fact never used. Since we're also using -Wvla (without
size), we should already get warnings about any variable length arrays
anyway, so the additional "-Wvla-larger-than" does not make much sense
and thus we can simply drop this.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Currently `virt-host-validate` will fail whenever one of its calls fail,
regardless of virHostValidateLevel set.
This behaviour is not optimal and makes it not exactly reliable as a
command line tool as other tools or scripts using it would have to check
its output to figure out whether something really failed or if a warning
was mistakenly treated as failure.
With this change, the behaviour of whether to fail or not, is defined by
the caller of those functions, based on the virHostValidateLevel passed
to them.
https://gitlab.com/libvirt/libvirt/-/issues/175
Signed-off-by: Fabiano Fidêncio <fabiano@fidencio.org>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently we expose libvirt Go packages at
libvirt.org/libvirt-go
libvirt.org/libvirt-go-xml
These packages have not supported Go modules historically and when we
tried to introduce modules, we hit the problem that we're not using
semver for versioning.
The only way around this is to introduce new packages under a different
namespace, that will have the exact same code, but be tagged with a
different version numbering scheme.
This change proposes:
libvirt.org/go/libvirt
libvirt.org/go/libvirtxml
Note the hyphen is removed so that the import basename matches the
Go package name.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Disk serials are truncated arbitrarily and silently by qemu depending on
the device type and how they are configured. Since changing the current
state would lead to more regressions than we have now, document that the
truncation is arbitrary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Nowadays memfd is the most convenient memory backend for vhost-user
devices. Compared to file-backend memory and hugepages, there is no need
to worry about configuring the location of the shm directory or
allocating hugepages.
Cc: Michal Prívozník <mprivozn@redhat.com>
Cc: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Remove some dupicate text and replace in incorrect occurance of
monolithic with modular.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In fcdcf8f70c the remoteGetUNIXSocket() function was changed and
one new variable was introduced (among other things): @env_name.
However, for WIN32 case the variable changed name to @env_path
which builds mingw builds.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The virNetSocketNewConnectUNIX() function was changed in
48f66cfe3e. And its WIN32 version (which just reports an error)
was updated too, but this new argument @spawnDaemonPath was not
marked as unused.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We want to use those shared drivers provided by libvirt to avoid
implementing our own.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Wei Liu <liuwe@microsoft.com>
After previous patches, the @ret variable and the 'cleanup'
label are redundant. Remove them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There are two variables that can be freed automatically: @cmd
(which allows us to drop explicit virCommandFree() call at the
end of the function) and @help which was never freed (and thus
leaked).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The CH driver needs "cloud-hypervisor" binary. And if none was
found then the initialization of the driver fails as
chStateInitialize() returns VIR_DRV_STATE_INIT_ERROR. This in
turn means that whole daemon fails to initialize. Let's return
VIR_DRV_STATE_INIT_SKIPPED in this particular case, which
disables the CH drvier but lets the daemon run.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
After previous patches, there's not much value in
chExtractVersion(). Rename chExtractVersionInfo() to
chExtractVersion() and have it use virCHDriver directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The only caller, chExtractVersion() passes not NULL. Therefore,
it's redundant to check for NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If chExtractVersionInfo() fails, in some cases it reports error
and in some it doesn't. Fix those places and drop reporting error
from chExtractVersion() which would just overwrite more specific
error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The <audio> element is configuring exclusively a backend, not a device.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When the default driver mode requests the modular daemons, we still
defaulted to spawning libvirtd if the URI was NULL, because we don't
know which driver specific daemon to spawn. virtproxyd has logic
that can handle this as it is used for compatibility when accepting
incoming TCP connections with a NULL URI.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The "spawnDaemon" and "binary" parameters are co-dependant, with the
latter non-NULL, if-and-only-if the former is true. Getting rid of the
"spawnDaemon" parameter simplifies life for the callers and eliminates
an error checking scenario.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When deciding what socket to connect to, we build the daemon path
that we need to autostart. This path only needs to be populated
if we actually intend to use autostart.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The remoteGetUNIXSocket method currently just returns the daemon name
and the caller then converts this to a path. Except the SSH helper
didn't do this, so it was relying on later code expanding $PATH, and
this doesn't allow for build root overrides.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We have helper methods that return boolans for ro/user/autostart
properties. We then pack them into a flags parameter, and later
unpack them again. This makes the code consistently use flags
throughout.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This enum will shortly be used by the remote driver sockets helper
methods too.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
