The way that virSecurityDACRecallLabel is currently written is
that if XATTRs are not supported for given path to the caller
this is not different than if the path is still in use. The value
of 1 is returned which makes secdrivers skip label restore.
This is clearly a bug as we are not restoring labels on say NFS
even though previously we were.

Strictly speaking, changes to virSecurityDACRememberLabel are not
needed, but they are done anyway so that getter and setter behave
in the same fashion.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

It's better to have the function report errors, because none of
the callers does.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

It's better to have the function report errors, because none of
the callers does.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The way that security drivers use XATTR is kind of verbose. If
error reporting was left for caller then the caller would end up
even more verbose.

There are two places where we do not want to report error if
virFileGetXAttr fails. Therefore virFileGetXAttrQuiet is
introduced as an alternative that doesn't report errors.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Just like it's DAC counterpart is doing,
virSecuritySELinuxRestoreAllLabel() could print @migrated in the
debug message.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Firstly, there's no reason to enumerate all XATTRs since they
differ only in the prefix and we can construct them in a loop.

Secondly, and more importantly, the script was still looking for
just one prefix "trusted.libvirt.security" even on FreeBSD.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Having to enumerate all capabilities that we want domain to have
is too verbose and prevents us from adding more tests. Have the
domain always have the latest x86_64 capabilities. This means
that we have to drop two arm tests, but on the other hand, I'm
introducing 50 new cases. I've listed 50 biggest .args files and
added those:

  libvirt.git $ ls -Sr $(find tests/qemuxml2argvdata/ \
  -type f -iname "*.x86_64-latest.args") | tail -n 50

Except for two:
1) disk-backing-chains-noindex - this XML has some disks with
backing chain. And since set is done on the whole backing chain
and restore only on the top layer this would lead to instant test
failure. Don't worry, secdrivers will be fixed shortly too and
the test case will be added.

2) hostdev-mdev-display-spice-egl-headless - for this XML
secdriver tries to find IOMMU group that mdev lives in. Since we
are not mocking sysfs access this test case would fail.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

This simplifies the code a bit and removes the need for cleanup
label in one case. In the other case the label is kept because
it's going to be used later.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The @securityManager variable in testDomain() is unused. Drop it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Problem with current approach is that if
qemuSecuritySetAllLabel() fails, then the @chown_paths and
@xattr_paths hash tables are not freed and preserve values
already stored there into the next test case.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

I don't really know what happened when I was writing the original
code, but even if error was to be set the corresponding boolean
was set to false meaning no error.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

One of the functions of this mock is that it spoofs chown() and
stat() calls. But it is doing so in a clever way: it stores the
new owner on chown() and reports it on subsequent stat(). This is
done by using a 32bit unsigned integer where one half is used to
store uid the other is for gid. Later, when stat() is called the
integer is fetched and split into halves again. Well, my bit
operation skills are poor and the code I've written does not do
that properly.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

This test is beautiful. It checks if we haven't messed up
refcounting on security labels (well, XATTRs where the original
owner is stored). It does this by setting up tracking of XATTR
setting/removing into a hash table, then calling
qemuSecuritySetAllLabel() followed by immediate
qemuSecurityRestoreAllLabel() at which point, the hash table must
be empty. The test so beautifully written that no matter
what you do it won't fail. The reason is that all seclabel work
is done in a child process. Therefore, the hash table in the
parent is never changed and thus always empty.

There are two reasons for forking (only one of them makes sense
here though):

1) namespaces - when chown()-ing a file we have to fork() and
make the child enter desired namespace,
2) locking - because of exclusive access to XATTRs we lock the
files we chown() and this is done in a fork (see 20786092 for
more info).

While we want to fork in real world, we don't want that in a test
suite. Override virProcessRunInFork() then.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

* docs/news.xml: updated for the release
Signed-off-by: NDaniel Veillard <veillard@redhat.com>

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

Commit 5a148ce8 altered the virNetServerNew to remove a parameter
but neglected to update the ATTRIBUTE_NONNULL's which causes a build
failure for when checking is enabled such as when lv_cv_static_analysis
is enabled.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

While the script ultimately needs to be executable, the
source file really shouldn't be.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

We have been grouping network-port and nwfilter-binding permissions
under virNetworkPtr and virNWFilterPtr respectively.

Add the two missing classes that were matched because they contain
a substring of others.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

Introduced in commit 4a6ee535.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

It has been dropped in 215d9393, but not all of
the documentation was updated accordingly.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

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

Because of a systemd delegation policy [1] we should not write to any
cgroups files owned by systemd which in case of cgroups v2 includes
'cgroups.subtree_control'.

systemd will enable controllers automatically for us to have them
available for VM cgroups.

[1] <https://github.com/systemd/systemd/blob/master/docs/CGROUP_DELEGATION.md>
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

This reverts commit 7bca1c9b.

As it turns out it's not a good idea on systemd hosts.  The root
cgroup can have all controllers enabled but they don't have to be
enabled for sub-cgroups.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

This reverts commit 226094fb.

A deprecation is a warning to something that use of a feature is
being discouraged. By definition it is not an error condition to
continue to use a deprecated feature.

A VIR_ERR_DEPRECATED constant thus makes no conceptual sense. For
features which are entirely absent we already document that the
VIR_ERR_NO_SUPPORT code will be used. There is no need to distinguish
between a feature which never existed and a feature which previously
existed and was since removed.
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

This reverts commit 3026f6d9.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

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

The public API entry points will report VIR_ERR_NO_SUPPORT to the
caller when a driver does not provide an implementation of a particular
method.

When deleting methods, leaving the driver API entry point explicitly
set to NULL with an version range comment, allows the hvsupport.html
page to document when the AP was removed.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Since c2573527 a reference of 'cfg' would be leaked if the function
does not need to process anything. Fix it by using VIR_AUTOUNREF.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

when a disk without PR perform attach or detach operation,
need not call qemuHotplugRemoveManagedPR, otherwise, it will
print err log about PR, let us fix it.
Signed-off-by: NJie Wang <wangjie88@huawei.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The sys/sysctl.h header is only needed on BSD platforms to get
the sysctlbyname() function declaration. On Linux we talk to
procfs instead to change sysctls.

Unfortunately a legacy sys/sysctl.h header does exist on Linux
and including it has recently started triggering a deprecation
warning from glibc.

Protect its inclusion with a HAVE_SYSCTLBYNAME check instead
so that it only gets used on platforms where we need that
function declaration.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

When detecting available controllers on host we can be limited by list
of controllers from qemu.conf file.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

Currently CPU controller cannot be enabled if there is any real-time
task running and is assigned to non-root cgroup which is the case on
several distributions with graphical environment.

Instead of erroring out treat it as the controller is not available.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

In order to skip controllers that we are not able to activate we need
to return different return value so the caller can decide what to do.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

It might happen that we are not able to enable CPU controller so we
can enable it for thread sub-cgroups only if it's available in parent
cgroup.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The assumption that CPU controller would be always enabled is wrong, we
should use any available controller to create a new sub-cgroup.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

This affects only cgroups v2 where enabled controllers are not based on
available mount points but on the list provided in cgroup.controllers
file.  However, moving it will fill in placement as well, so it needs
to be freed together with mount point if we don't need that controller.

Before this patch we were assuming that all controllers available in
root cgroup where available in all other sub-cgroups which was wrong.

In order to fix it we need to move the cgroup controllers detection
after cgroup placement was prepared in order to build correct path for
cgroup.controllers file.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>

In cgroups v2 we don't have to detect available controllers every single
time if we are creating a new cgroup based on parent cgroup.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Our code would skip adding the default type in this cases, but since we
know that the only reasonable option here is 'fat' we can add it while
starting the VM.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The storage volume may in fact convert into a directory when starting
the VM so that it may be actually possible to use it.

This is a regression caused by c9b27af3 as moving the check to
validation time without adjustment causes problems as the volumes are
not translated yet.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>