Commit Graph

4945 Commits

Author SHA1 Message Date
Peter Krempa
5ca84b6cae util: hash: Add deprecation notices for functions which have g_hash_table replacements
For functions which have reasonable replacement, let's encourage usage
of g_hash_table_ alternatives.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
2020-11-06 10:40:56 +01:00
Peter Krempa
62a01d84a3 util: hash: Retire 'virHashTable' in favor of 'GHashTable'
Don't hide our use of GHashTable behind our typedef. This will also
promote the use of glibs hash function directly.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
2020-11-06 10:40:51 +01:00
Peter Krempa
de41e74bbc util: hash: Reimplement virHashTable using GHashTable
Glib's hash table provides basically the same functionality as our hash
table.

In most cases the only thing that remains in the virHash* wrappers is
NULL-checks of '@table' argument as glib's hash functions don't tolerate
NULL.

In case of iterators, we adapt the existing API of iterators to glibs to
prevent having rewrite all callers at this point.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
2020-11-06 10:31:57 +01:00
Peter Krempa
85d5b8bd9a util: hash: Don't use 'const' with virHashTablePtr
We didn't use it rigorously and some helpers even cast it away. Remove
const from all hash utility functions.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
2020-11-06 10:31:57 +01:00
Peter Krempa
247460ab41 util: hash: Use virHashForEachSafe in places which might delete the element
Convert all calls to virHashForEach where it's not obvious that the
callback is _not_ deleting the current element from the hash to
virHashForEachSafe which will be deemed safe to do such operation.

Now that no iterator used with virHashForEach deletes current element we
can document that virHashForEach must not touch the hash table in any
way.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
2020-11-06 10:31:57 +01:00
Peter Krempa
80f3af5fd8 util: hash: Add delete-safe hash iterator
'virHashForEach' historically allowed deletion of the current element as
'virHashRemoveSet' didn't exist. To prevent us from having to deeply
analyse all iterators add virHashForEachSafe which first gets a list of
elements and iterates them outside of the hash table.

This will allow replace the internals of the hash table with other
implementation which don't allow such operation.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
2020-11-06 10:31:57 +01:00
Peter Krempa
947d2db31b Use virHashForEachSorted in tested code
The simplest way to write tests is to check the output against expected
output, but we must ensure that the output is stable. We can use
virHashForEachSorted as a hash iterator to ensure stable ordering.

This patch fixes 3 instances of hash iteration which is tested in
various parts, including test output changes in appropriate places.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
2020-11-06 10:31:57 +01:00
Peter Krempa
280a6d8330 util: hash: Introduce virHashForEachSorted
Iterate the hash elements sorted by key. This is useful to provide a
stable ordering such as in cases when the output is checked in tests.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
2020-11-06 10:31:57 +01:00
Peter Krempa
4eb8e9ae8b util: hash: Rewrite sorting of elements in virHashGetItems
All but one of the callers either use the list in arbitrary order or
sorted by key. Rewrite the function so that it supports sorting by key
natively and make it return the element count. This in turn allows to
rewrite the only caller to sort by value internally.

This allows to remove multiple sorting functions which were sorting by
key and the function will be also later reused for some hash operations
internally.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
2020-11-06 10:31:57 +01:00
Pavel Hrdina
8f0f6ff082 vircgrouppriv: fix ATTRIBUTE_NONNULL for virCgroupNewDomainPartition
Commit <99d2c6519ad18651b5959fa0a3366bcb2c1e44f3> removed parameter
from the function but did not modified ATTRIBUTE_NONNULL.

Reported-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
2020-11-05 23:15:16 +01:00
Boris Fiuczynski
da5cf518ad util: refactor mdev_types methods return code usage
Remove mix of array length and error code in the return code.

Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-11-04 19:14:07 +01:00
Boris Fiuczynski
65c1f47760 util: refactor mdev_types method from PCI to mdev
Extract virPCIGetMdevTypes from PCI as virMediatedDeviceGetMdevTypes
into mdev for later reuse.

Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-11-04 19:11:49 +01:00
Pavel Hrdina
457877eae4 vircgroup: drop condition for absolute path from copyPlacement callbacks
Now that every caller to copyPlacement doesn't pass absolute path there
is no need to have a condition to handle that case.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
6f0aa96f41 vircgroup: refactor virCgroupNewPartition
The old code passed an absolute path to virCgroupNewFromParent() which
is not necessary. The code can take the current placement of parent
cgroup and append a relative path.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
14674ad436 vircgroup: move parentPath declaration
It's used only inside the if condition.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
77291414c7 vircgroup: refactor virCgroupEnableMissingControllers
Use virStringSplit() to get the list of directories needed to be
created. This improves readability of the code and stops passing
absolute path to virCgroupNewFromParent().

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
99d2c6519a vircgroup: drop @create from virCgroupNewDomainPartition
All callers pass true.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
085590fee4 vircgroup: introduce virCgroupSetPlacement
Currently this task is done by virCgroupCopyPlacement when the @path
starts with "/".

virCgroupNew is always called with @path starting with "/" and there is
no parent to copy path from. To make it obvious what the code is doing
introduce new helper.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
ca7b305631 vircgroup: drop @pid argument from virCgroupNew
Now it is always -1.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
c16da281e4 vircgroup: no need to use PID in virCgroupEnableMissingControllers
This function is relevant only with cgroups v1 where it creates
hierarchy for controllers that are not managed by systemd. PID is used
to detect a placement of current process but in this situation we are
building the hierarchy for already known placement.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
13958a8c5b vircgroup: expand virCgroupDetect into virCgroupNew
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
95dc2fabe3 vircgroup: virCgroupNew is now always called with absolute path
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
2eb83e270d vircgroup: drop @parent from virCgroupNew
Now it is always NULL.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
bcfa563707 vircgroup: introduce virCgroupNewParent
The current code uses virCgroupNew() as a single point of entry and
calls into virCgroupDetect() as well. Both have logic for several paths
which is difficult to figure out.

Extract the actually used code path from the two functions to make
it obvious what's happening in this case.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
234769b0d5 vircgroup: extract virCgroupNewDetect from virCgroupNew
The current code uses virCgroupNew() as a single point of entry and
calls into virCgroupDetect() as well. Both have logic for several paths
which is difficult to figure out.

Extract the actually used code path from the two functions to make
it obvious what's happening in this case.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
f8ca962589 vircgroup: introduce virCgroupDetectControllers helper
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
20da059e18 vircgroup: introduce virCgroupValidatePlacement helper
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
30f3516053 vircgroup: introduce virCgroupCopyPlacement helper
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
069f0994ab vircgroup: introduce virCgroupCopyMounts helper
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
a4353381f1 vircgroup: introduce virCgroupSetBackends helper
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
c88b3712ca vircgroup: remove useless cgroup->path variable
It is only used for debug and error purposes which can be easily
replaced by @placement.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
9d312af357 vircgroupv2: detect controllers enabled in parent cgroup
With cgroups v2 working with controllers is a bit more complicated then
with cgroups v1 where the controller had to be mounted.

There are two files, cgroups.controllers and cgroup.subtree_control.
The file cgroup.controllers lists all controllers enabled in the current
cgroup and cgroups.subtree_control, as the name suggest, controls which
controllers are enabled for a subtree of cgroups.

Now the issue here is that the current code doesn't make any difference
if the @parent variable is NULL or not because ../cgroup.subtree_control
will list the same controllers as ./cgroup.controllers.

The whole point of the @parent variable is when we are building the
cgroup topology ourselves without systemd help we need to detect which
controllers are enabled in the parent cgroup in order to enable them for
the current cgroup as well and for that we need to check
cgroup.controllers of the parent group.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
902c6644a8 vircgroupv2: properly detect placement of running VM
When libvirtd starts a VM it internally stores a path to the main
cgroup. When we restart libvirtd we should get to the same state.

When we start a VM on host with systemd the cgroup is created for us and
the process is already placed into that cgroup and we detect the path
created by systemd using /proc/$PID/cgroup. After that we create
sub-cgroups and move all threads there.

Once libvirtd is restarted we again detect the cgroup path using
/proc/$PID/cgroup, but in this case we will get a different path because
the main thread was moved to a "emulator" cgroup.

Instead of ignoring the "emulator" directory when validating cgroups
remove it completely when detecting cgroup otherwise cgroups will not
work properly when libvirtd is restarted.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Pavel Hrdina
e85cfb095a vircgroupv2: properly detect empty tasks
With cgroups v2 the file cgroup.procs will never be empty if threading
is enabled as it will always have ID of all processes even if all
threads of the processes are moved to sub-cgroups. If that happens the
file cgroup.threads will be empty.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Laine Stump
85c8c29214 remove unnecessary cleanup labels and unused return variables
After converting all DIR* to g_autoptr(DIR), many cleanup: labels
ended up just having "return ret", and every place that set ret would
just immediately goto cleanup. Remove the cleanup label and its
return, and just return the set value immediately, thus eliminating
the need for the return variable itself.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-11-02 22:01:36 -05:00
Laine Stump
77401d549c util: refactor function to simplify and remove label
Once the DIR* in virPCIGetName() was made g_autoptr, the cleanup:
label just had a "return ret;", but the rest of the function was more
compilcated than it needed to be, doing funky things with the value of
ret inside multi-level conditionals and a while loop that might exit
early via a break with ret == 0 or exit early via a goto cleanup with
ret == -1.

It really didn't need to be nearly as complicated. After doing the
trivial replacements of "goto cleanup" with appropriate direct
returns, it became obvious that:

1) the outermost level of the nested conditional at the end of the
   function ("if (ret < 0)") was now redundant, since ret is now
   *always* < 0 by that point (otherwise the function has returned).

2) by switching the sense of the next level of the conditional (making
   it "if (!physPortID)", the "else" (which is now just "return 0;"
   becomes the "if", and the new "else" no longer needs to be inside
   the conditional.

3) the value of firstEntryName can be moved into *netname with
   g_steal_pointer()

Once that is all done, ret is no longer used and can be removed.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-11-02 22:01:36 -05:00
Laine Stump
d4f071d39b util: remove unused VIR_DIR_CLOSE() macro
Since every single use of DIR* was converted to use g_autoptr, this
function is not currently needed. Even if someone comes up with a
usage for a non-g_autoptr DIR* in the future, they can just use
virDirClose(), since there is no longer a semantic difference between
the two (VIR_DIR_CLOSE() previously had an extra & on the pointer so
that it could be transparently passed as a DIR** to virDirClose(), but
that was removed several commits back.)

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-11-02 22:01:36 -05:00
Laine Stump
c0ae4919e3 change DIR* int g_autoptr(DIR) where appropriate
All of these conversions are trivial - VIR_DIR_CLOSE() (aka
virDirClose()) is called only once on the DIR*, and it happens just
before going out of scope.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-11-02 22:01:36 -05:00
Laine Stump
a61472aad8 util: declare g_autoptr cleanup function to auto-close DIR*
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-11-02 22:01:36 -05:00
Laine Stump
24d8968cd0 util: change virDirClose to take a DIR* instead of DIR**.
In order to make a usable g_autoptr(DIR), we need to have a close
function that is a NOP when the pointer is NULL, but takes a simple
DIR*. But virDirClose() (candidate to be the g_autoptr cleanup
function) currently takes a DIR**, not DIR*. It does this so that it
can clear the pointer, thus making it safe to call virDirClose on the
same DIR multiple times.

In the past the clearing of the DIR* was essential in a few places,
but those few places have now been changed, so we can modify
virDirClose() to take a DIR*, and remove the side effect of clearing
the DIR*. This will make it directly usable as the g_autoptr cleanup,
and will mean that this:

   {
   DIR *dirp = NULL;
   blah blah ...
   VIR_DIR_CLOSE(dirp)
   }

is functionally identical to

   {
   g_autoptr(DIR) dirp = NULL;
   blah blah ...
   }

which will make conversion to using g_autoptr mechanical and simple to review.

(Note that virDirClose() will still check for NULL before attempting
to close, so that it can always be safely called, as long as the DIR*
was initialized to NULL (another prerequisite of becoming a g_autoptr
cleanup function)

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-11-02 22:01:36 -05:00
Laine Stump
098f03c29e util: reduce scope of a DIR * in virCgroupV1SetOwner()
DIR *dh is being re-used each time through the for loop of this
function, so it must be closed and then re-opened, which means we
can't convert it to g_autoptr. By moving the definition of dh inside
the for loop, we make it possible to trivially convert to g_autoptr
(which will happen in a subsequent patch)

NB: VIR_DIR_CLOSE() is already called at the bottom of the for loop,
so removing the VIR_DIR_CLOSE() at the end of the function is *not*
creating a leak of a DIR*!

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-11-02 22:01:36 -05:00
Laine Stump
c40b673182 consistently use VIR_DIR_CLOSE() instead of virDirClose()
This will make it easier to review upcoming patches that use g_autoptr
to auto-close all DIRs.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-11-02 22:01:36 -05:00
Peter Krempa
e9c1b5c92e util: virhash: Standardize on 'opaque' for opaque data
Rename 'data' argument which is used for opaque data.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-11-02 14:15:49 +01:00
Martin Kletzander
1f807631f4 util: Avoid double free in virProcessSetAffinity
The cpu mask was free()'d immediately on any error and at the end of the
function, where it was expected that it would either error out and return or
goto another allocation if the code was to fail.  However since commit
9514e24984 the error path did not return in one new case which caused
double-free in such situation.  In order to make the code more straightforward
just free the mask after it's been used even before checking the return code of
the call.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1819801

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2020-10-27 16:37:43 +01:00
Peter Krempa
4505f11d65 virHashRemoveAll: Don't return number of removed items
Nobody uses the return value.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2020-10-22 15:02:46 +02:00
Peter Krempa
7c1a4bc775 util: virhash: Remove key handling callbacks
Since we use virHashTable for string-keyed values only, we can remove
all the callbacks which allowed universal keys.

Code which wishes to use non-string keys should use glib's GHashTable.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2020-10-22 15:02:46 +02:00
Peter Krempa
d6d4c08daf util: hash: Change type of hash table name/key to 'char'
All users of virHashTable pass strings as the name/key of the entry.
Make this an official requirement by turning the variables to 'const
char *'.

For any other case it's better to use glib's GHashTable.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2020-10-22 15:02:46 +02:00
Peter Krempa
a2c699856a util: hash: Remove virHashCreateFull
The only place we call it is in virHashNew. Move the code to virHashNew
and remove virHashCreateFull.

Code wishing to use non-strings as hash table keys will be better off
using glib's GHashTable directly.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2020-10-22 15:02:46 +02:00
Peter Krempa
8824fc8474 util: hash: Remove virHashValueFree
Use 'g_free' directly.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2020-10-22 15:02:46 +02:00
Peter Krempa
b82dfe3ba7 Replace all instances of 'virHashCreate' with 'virHashNew'
It doesn't make much sense to configure the bucket count in the hash
table for each case specifically. Replace all calls of virHashCreate
with virHashNew which has a pre-set size and remove virHashCreate
completely.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2020-10-22 15:02:46 +02:00