In 600462834f we've tried to remove Author(s): lines
from comments at the beginning of our source files. Well, in some
files while we removed the "Author" line we did not remove the
actual list of authors.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Our code is not bug free. The refcounting I introduced will
almost certainly not work in some use cases. Provide a script
that will remove all the XATTRs set by libvirt so that it can
start cleanly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This test checks if security label remembering works correctly.
It uses qemuSecurity* APIs to do that. And some mocking (even
though it's not real mocking as we are used to from other tests
like virpcitest). So far, only DAC driver is tested.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We are setting label on kernel, initrd, dtb and slic_table files.
But we never restored it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It helps whe trying to match calls with virSecuritySELinuxSetAllLabel
if the order in which devices are set/restored is the same in
both functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When iterating over list of paths/disk sources to relabel it may
happen that the process fails at some point. In that case, for
the sake of keeping seclabel refcount (stored in XATTRs) in sync
with reality we have to perform rollback. However, if that fails
too the only thing we can do is warn user.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's important to keep XATTRs untouched (well, in the same state
they were in when entering the function). Otherwise our
refcounting would be messed up.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to what I did in DAC driver, this also requires the
same SELinux label to be used for shared paths. If a path is
already in use by a domain (or domains) then and the domain we
are starting now wants to access the path it has to have the same
SELinux label. This might look too restrictive as the new label
can still guarantee access to already running domains but in
reality it is very unlikely and usually an admin mistake.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It is going to be important to know if the current transaction we
are running is a restore operation or set label operation so that
we know whether to call virSecurityGetRememberedLabel() or
virSecuritySetRememberedLabel(). That is, whether we are in a
restore and therefore have to fetch the remembered label, or we
are in set operation and therefore have to store the original
label.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that we have seclabel remembering we can safely restore
labels for shared and RO disks. In fact we need to do that to
keep seclabel refcount stored in XATTRs in sync with reality.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This also requires the same DAC label to be used for shared
paths. If a path is already in use by a domain (or domains) then
and the domain we are starting now wants to access the path it
has to have the same DAC label. This might look too restrictive
as the new label can still guarantee access to already running
domains but in reality it is very unlikely and usually an admin
mistake.
This requirement also simplifies seclabel remembering, because we
can store only one seclabel and have a refcounter for how many
times the path is in use. If we were to allow different labels
and store them in some sort of array the algorithm to match
labels to domains would be needlessly complicated.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Because the implementation that will be used for label
remembering/recall is not atomic we have to give callers a chance
to enable or disable it. That is, enable it if and only if
metadata locking is enabled. Otherwise the feature MUST be turned
off.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We are setting label on kernel, initrd, dtb and slic_table files.
But we never restored it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It helps whe trying to match calls with virSecurityDACSetAllLabel
if the order in which devices are set/restored is the same in
both functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When iterating over list of paths/disk sources to relabel it may
happen that the process fails at some point. In that case, for
the sake of keeping seclabel refcount (stored in XATTRs) in sync
with reality we have to perform rollback. However, if that fails
too the only thing we can do is warn user.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's important to keep XATTRs untouched (well, in the same state
they were in when entering the function). Otherwise our
refcounting would be messed up.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This file implements wrappers over XATTR getter/setter. It
ensures the proper XATTR namespace is used.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The driver is unmaintained, untested and severely broken for
quite some time now. Since nobody even reported any issue with it
let us drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1624223
There are two ways to request memory preallocation on cmd line:
-mem-prealloc and .prealloc attribute for a memory-backend-file.
However, as it turns out it's not safe to use both at the same
time. If -mem-prealloc is used then qemu will fully allocate the
memory (this is done by actually touching every page that has
been allocated). Then, if .prealloc=yes is specified,
mbind(flags = MPOL_MF_STRICT | MPOL_MF_MOVE) is called which:
a) has to (possibly) move the memory to a different NUMA node,
b) can have no effect when hugepages are in play (thus ignoring user
request to place memory on desired NUMA nodes).
Prefer -mem-prealloc as it is more backward compatible
compared to switching to "-numa node,memdev= + -object
memory-backend-file".
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
So far we have two arguments that we are passing to
qemuBuildMemoryBackendProps() and that are taken from domain
private data: @qemuCaps and @autoNodeset. In the next commit I
will use one more item from there. Therefore, instead of having
it as yet another argument to the function, pass pointer to the
private data object.
There is one change in qemuDomainAttachMemory() where previously
@autoNodeset was NULL but now is priv->autoNodeset (which may be
set). This is safe to do as @autoNodeset is advisory only.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1656014
An RNG device can consists of more devices than RND device
itself. For instance, in case of EGD there is a chardev that
connects to EGD daemon and feeds the qemu with random data. When
doing RNG device removal we have to remove the associated chardev
as well.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The way that the code is currently written makes my eyes hurt.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are two functions called from syncNicRxFilterMultiMode:
virNetDevSetRcvAllMulti() and virNetDevSetRcvMulti(). Both of
them return 0 on success and -1 on error. However, currently
their return value is checked for != 0 which conflicts with our
assumptions on retvals: a positive value is still considered
success but with current check it would lead to failure.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If migration is cancelled or confirm phase fails the domain
should be kept on the source even if VIR_MIGRATE_UNDEFINE_SOURCE
was requested.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
There are some checks done when parsing a migration cookie. For
instance, one of the checks ensures that the domain is not being
migrated onto the same host. If that is the case, then we are in
big trouble because the @vm is the same domain object used by
source and it has some jobs sets and everything so recovering
from failed cookie parsing would be needlessly hard.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The function currently takes virDomainObjPtr because it's using
both: the domain definition and domain private data.
Unfortunately, this means that in prepare phase we can't parse
migration cookie before putting incoming domain def onto domain
objects list (addressed in the very next commit). Change the
arguments so that virDomainDef and private data are passed
instead of virDomainObjPtr.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
There are several functions called in the cleanup path. Some of
them do report error (e.g. qemuDomainRemoveInactiveJob()) which
may result in overwriting an error reported earlier with some
less useful message.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Trying to use virlockd to lock metadata turns out to be too big
gun. Since we will always spawn a separate process for relabeling
we are safe to use thread unsafe POSIX locks and take out
virtlockd completely out of the picture.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When metadata locking is enabled that means the security commit
processing will be run in a fork similar to how namespaces use fork()'s
for processing. This is done to ensure libvirt can properly and
synchronously modify the metadata to store the original owner data.
Since fork()'s (e.g. virFork) have been seen as a performance bottleneck
being able to disable them allows the admin to choose whether the
performance 'hit' is worth the extra 'security' of being able to
remember the original owner of a lock.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
For metadata locking we might need an extra fork() which given
latest attempts to do fewer fork()-s is suboptimal. Therefore,
there will be a qemu.conf knob to {en|dis}able this feature. But
since the feature is actually not metadata locking itself rather
than remembering of the original owner of the file this is named
as 'rememberOwner'. But patches for that feature are not even
posted yet so there is actually no qemu.conf entry in this patch
nor a way to enable this feature.
Even though this is effectively a dead code for now it is still
desired.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The TPM code currently accepts pointer to a domain definition.
This is okay for now, but in near future the security driver APIs
it calls will require domain object. Therefore, change the TPM
code to accept the domain object pointer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Both virProcessRunInMountNamespace() and virProcessRunInFork()
look very similar. De-duplicate the code and make
virProcessRunInMountNamespace() call virProcessRunInFork().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This new helper can be used to spawn a child process and run
passed callback from it. This will come handy esp. if the
callback is not thread safe.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In 4674fc6afd I've implemented transactions for selinux driver.
Well, now that I am working in this area I've noticed a subtle
bug: @ret is initialized to 0 instead of -1. Facepalm.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1524230
The qemuBuildVhostuserCommandLine builds command line for
vhostuser type interfaces. It is duplicating some code of the
function it is called from (qemuBuildInterfaceCommandLine)
because of the way it's called. If we merge it into the caller
not only we save a few lines but we also enable checks that we
would have to duplicate otherwise (e.g. QoS availability).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
When we have variables A, B, C then there are two ways to free
them. Either in the order they are declared or the reversed one.
Any other ordering is confusing. In this commit I'm reordering
calls to VIR_FREE in the reversed order.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1640465
Weirdly enough, there can be symlinks in the path we are trying
to fix. If it is the case our clever algorithm that finds matches
against mount table won't work. Canonicalize path at the
beginning then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The virFileInData() function should return to the caller if the
current position the passed file is in is a data section or a
hole (and also how long the current section is). At any rate,
upon return from this function (be it successful or not) the
original position in the file is restored. This may mess up with
errno which might have been set earlier. Save the errno into a
local variable so it can be restored for the caller's sake.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1632833
When doing a SCSI passthrough we don't put format= onto the
command line. This causes qemu to probe the format automatically
which ends up in a warning in the domain log and possible qemu
disabling writes to the first block (according to the warning
message).
Based-on-work-of: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The mock is built on Linux only. Therefore we should load it only
on Linux too. This fixes the FreeBSD build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
There are couple of things wrong with the current implementation.
The first one is that in the first loop the code tries to build a
list of fuse.glusterfs mount points. Well, since the strings are
allocated in a temporary buffer and are not duplicated this
results in wrong decision made later in the code.
The second problem is that the code does not take into account
subtree mounts. For instance, if there's a fuse.gluster mounted
at /some/path and another FS mounted at /some/path/subdir the
code would not recognize this subdir mount.
Reported-by: Han Han <hhan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
If the given path is already a mount point (e.g. a bind mount of
a file, or simply a direct mount point of a FS), then our code
fails to detect that because the first thing it does is cutting
off part after last slash '/'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Introduce some basic test cases for virFileIsSharedFS(). More
will be added later. In order to achieve desired result, mocks
for setmntent() and statfs() need to be invented because the
first thing that virFileIsSharedFS() does is calling the latter.
If it finds a FUSE mount it'll call the former.
The mock might look a bit complicated, but in fact it's quite
simple. The test sets LIBVIRT_MTAB env variable to hold the
absolute path to a file containing mount table. Then, statfs()
returns matching FS it finds, and setmntent() is there just to
replace /proc/mounts with the file the test wants to load.
Adding this test also exposed a bug we have - because we assume
the given path points to a file we cut off what we assume is a
file name to obtain directory path and only then we call
statfs(). This is buggy because the passed path could be already
a mount point.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Because of lacking virTestCounterReset() call, the old test cases
name was preserved.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
For non-Linux platforms we have
virHostValidateCGroupControllers() stub which only reports an
error. But we are not marking the ignored arguments the way we
should.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1632711
GlusterFS is typically safe when it comes to migration. It's a
network FS after all. However, it can be mounted via FUSE driver
they provide. If that is the case we fail to identify it and
think migration is not safe and require VIR_MIGRATE_UNSAFE flag.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Turns out, there are couple of bugs that prevent this feature
from being operational. Given how close to the release we are
disable the feature temporarily. Hopefully, it can be enabled
back after all the bugs are fixed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
It may happen that in the list of paths/disk sources to relabel
there is a disk source. If that is the case, the path is NULL. In
that case, we shouldn't try to lock the path. It's likely a
network disk anyway and therefore there is nothing to lock.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This shouldn't be needed per-se. Security manager shouldn't
disappear during transactions - it's immutable. However, it
doesn't hurt to grab a reference either - transaction code uses
it after all.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This array is allocated in virSecuritySELinuxContextListAppend()
but never freed. This commit is essentially the same as ca25026.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We're passing size_t but using format for unsigned long.
Introduced in latest vshTable rework patches.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1630164
The domiflist command is designed to show a brief information on
domain interfaces. One piece of information that is shows is
"Source" - source network, device, name, bridge. However, it's
ignoring vhostuser for which we can show the unix socket it's
associated with.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
In 2e97450425 we've mistakenly removed gettext macro for
translating static strings. This results in table header being
printed in English regardless of user locale.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Lock all the paths we want to relabel to mutually exclude other
libvirt daemons.
The only hitch here is that directories can't be locked.
Therefore, when relabeling a directory do not lock it (this
happens only when setting up some domain private paths anyway,
e.g. huge pages directory).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
So far the whole transaction handling is done
virSecuritySELinuxSetFileconHelper(). This needs to change for
the sake of security label remembering and locking. Otherwise we
would be locking a path when only appending it to transaction
list and not when actually relabelling it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Firstly, the following code pattern is harder to follow:
if (func() < 0) {
error();
} else {
/* success */
}
We should put 'goto cleanup' into the error branch and move the
else branch one level up.
Secondly, 'rc' should really be named 'ret' because it holds
return value of the function. Not some intermediate value.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This label is used in both successful and error paths. Therefore
it should be named 'cleanup' and not 'err'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Lock all the paths we want to relabel to mutually exclude other
libvirt daemons.
The only hitch here is that directories can't be locked.
Therefore, when relabeling a directory do not lock it (this
happens only when setting up some domain private paths anyway,
e.g. huge pages directory).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Firstly, the message that says we're setting uid:gid shouldn't be
called from virSecurityDACSetOwnershipInternal() because
virSecurityDACRestoreFileLabelInternal() is calling it too.
Secondly, there are places between us reporting label restore and
us actually doing it where we can quit. Don't say we're doing
something until we are actually about to do it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
So far the whole transaction handling is done
virSecurityDACSetOwnershipInternal(). This needs to change for
the sake of security label remembering and locking. Otherwise we
would be locking a path when only appending it to transaction
list and not when actually relabeling it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Two new APIs are added so that security driver can lock and
unlock paths it wishes to touch. These APIs are not for other
drivers to call but security drivers (DAC and SELinux). That is
the reason these APIs are not exposed through our
libvirt_private.syms file.
Three interesting things happen in this commit. The first is the
global @lockManagerMutex. Unfortunately, this has to exist so that
there is only one thread talking to virtlockd at a time. If there
were more threads and one of them closed the connection
prematurely, it would cause virtlockd killing libvirtd. Instead
of complicated code that would handle that, let's have a mutex
and keep the code simple.
The second interesting thing is keeping connection open between
lock and unlock API calls. This is achieved by duplicating client
FD and keeping it open until unlock is called. This trick is used
by regular disk content locking code when the FD is leaked to
qemu.
Finally, the third thing is polling implemented at client side.
Since virtlockd has only one thread that handles locking
requests, all it can do is either acquire lock or error out.
Therefore, the polling has to be implemented in client. The
polling is capped at 60 second timeout, which should be plenty
since the metadata lock is held only for a fraction of a second.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Now that we know what metadata lock manager user wishes to use we
can load it when initializing security driver. This is achieved
by adding new argument to virSecurityManagerNewDriver() and
subsequently to all functions that end up calling it.
The cfg.mk change is needed in order to allow lock_manager.h
inclusion in security driver without 'syntax-check' complaining.
This is safe thing to do as locking APIs will always exist (it's
only backend implementation that changes). However, instead of
allowing the include for all other drivers (like cpu, network,
and so on) allow it only for security driver. This will still
trigger the error if including from other drivers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This config option allows users to set and enable lock manager
for domain metadata. The lock manager is going to be used by
security drivers to serialize each other when changing a file
ownership or changing the SELinux label. The only supported lock
manager is 'lockd' for now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In some cases we might want to not load the lock driver config.
Alter virLockManagerPluginNew() and the lock drivers to cope with
this fact.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Soon there will be a virtlockd client that wants to either lock
all the resources or none (in order to avoid virtlockd killing
the client on connection close). Because on the RPC layer we can
only acquire one resource at a time, we have to perform a
rollback once we hit a resource that can't be acquired.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This is a new type of object that lock drivers can handle.
Currently, it is supported by lockd driver only.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The fact whether domain has or doesn't have RW disks is specific
to VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN and therefore should
reside in union specific to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We will want virtlockd to lock files on behalf of libvirtd and
not qemu process, because it is libvirtd that needs an exclusive
access not qemu. This requires new lock context.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This flag causes virtlockd to use different offset when locking
the file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
So far the virLockSpaceAcquireResource() locks the first byte in
the underlying file. But caller might want to lock other range.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Now that committing transactions using pid == -1 means that we're
not fork()-ing to run the transaction in a specific namespace, we
can utilize the transaction processing semantics in order to
start, run a or multiple commands, and then commit the
transaction without being concerned with other interactions or
transactions interrupting the processing. This will eventually
allow us to have a single place where all the paths can be
locked, followed by relabeling and unlocking again.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
It will be desirable to run transactions more often than we
currently do. Even if the domain we're relabeling the paths for
does not run in a namespace. If that's the case, there is no need
to fork() as we are already running in the right namespace. To
differentiate whether transaction code should fork() or not the
@pid argument now accepts -1 (which means do not fork).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In the future, the transactions are not going to be optional and
they will be run regardless of domain using namespace to collect
list of paths to be relabeled.
To make sure there won't be an API that goes behind transaction
code back update the comment that serves as decision manual
whether an API must be fully implemented or plain #define is
sufficient.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Even though the current use of the functions does not require full
implementation with transactions (none of the callers passes a path
somewhere under /dev), it doesn't hurt either. Moreover, in
future patches the paradigm is going to shift so that any API
that touches a file is required to use transactions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Even though the current use of the function does not require full
implementation with transactions (none of the callers pass a path
somewhere under /dev), it doesn't hurt either. Moreover, in
future patches the paradigm is going to shift so that any API
that touches a file is required to use transactions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In some cases we are checking if the mount namespace is enabled
at two places: one is at the beginning of exported function (e.g.
qemuDomainNamespaceSetupDisk()) and the other is at the beginning
of qemuDomainNamespaceMknodPaths() which is called from the
former function anyway. Then we have some other functions which
rely on the later check solely.
In order to compensate for possibly needless function call,
qemuDomainNamespaceMknodPaths() returns early if @npaths is zero.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The qemuSecurityDomainSetPathLabel() function reports perfect
error itself. Do not overwrite it to something less meaningful.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
These two functions (virSecurityDACSetOwnership and
virSecurityDACRestoreFileLabelInternal) do not really change
@src. Make it const.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This function is going call security manager APIs and therefore
it needs pointer to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This function is going call security manager APIs and therefore
it needs pointer to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
There is no need to check if @npaths is not zero. Let's
qemuDomainNamespaceUnlinkPaths() handle that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1622455
If a domain is configured to use <source type='file'/> under
<memoryBacking/> we have to honour that setting and produce
-mem-path on the command line. We are not doing so if domain has
no guest NUMA nodes nor hugepages.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This will help in future expansions of the code when it is be
harder to track if @newName and/or @newLockspace were already
allocated or not and thus whether it is safe to 'return' or we
need to 'goto error'. By using the 'cleanup' label those two
cases merge into a single one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If drvNew callback fails, nobody calls drvFree and thus private
data of the driver might leak.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Michal Privoznik