Commit Graph

39683 Commits

Author SHA1 Message Date
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
Pavel Hrdina
09c12ba248 qemu_dbus: use emulator cgroup for dbus-daemon
All other helper processes are moved to cgroup with QEMU emulator
thread as we keep the root VM cgroup without any processes. This
assumption is validated in qemuRestoreCgroupState() which is called
when libvirtd is restarted and reconnected to all running VMs.

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
b43acd8e82 qemu: remove dead code that setup cgroups for helper processes
In both cases priv->cgroup will always be NULL because it is called
before the QEMU process is started and cgroups are configured.

In qemuProcessLaunch() the call order is following:

    qemuExtDevicesStart()
    ...
    virCommandRun()
    ...
    qemuSetupCgroup()

where qemuDBusStart() is called from qemuExtDevicesStart() but we
cgroups are created in qemuSetupCgroup().

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
382071b8bf qemu_cgroup: remove unused @empty variable
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 21:26:32 +01:00
Tim Wiederhake
82bebba180 cpu_map: Unify apostrophe and quotation mark usage
Usage was mixed.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2020-11-03 17:10:26 +01:00
Matt Coleman
7795712ddc hyperv: call openwsman's ws_serializer_free_mem
This should have been included with the upgrade to openwsman 2.6.3.

Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 11:30:11 +01:00
Matt Coleman
3b52bcfa43 hyperv: minor formatting fix in hyperv_wmi.h
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 11:29:38 +01:00
Matt Coleman
d2ab8a530c hyperv: consistent names for SettingData functions
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 11:29:14 +01:00
Matt Coleman
87e062415f hyperv: move hypervGetProcSDByVSSDInstanceId to hyperv_wmi.c
This places it next to existing related functions.

Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 11:28:57 +01:00
Matt Coleman
add5cd8a3c hyperv: move hypervGetWmiClass to hyperv_wmi.h
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 11:28:25 +01:00
Matt Coleman
58ae6be649 hyperv: WMI class list function general cleanup
* use the same section comment in the header and code
* place the items in the same relative location within the .h and .c
* one parameter per line for multiline function definitions

Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 11:27:37 +01:00
Matt Coleman
db294a7eec hyperv: remove spaces after hypervObject* casts
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 11:26:15 +01:00
Matt Coleman
f5dadf9a71 hyperv: g_autofree username and password in hypervConnectOpen()
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-03 11:24:47 +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
e98f216719 conf: convert final DIR* to g_autoptr
This use of DIR* was re-using the same function-scope DIR* each time
through a for loop, and due to multiple error gotos in the loop, it
needed to have the scope of the DIR* reduced to just the loop at the
same time as switching to g_autoptr. That's what this patch does.

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
7f42bdf5c0 util: manually set dirp to NULL after closing in virCapabilitiesInitCache()
In all uses of VIR_DIR_CLOSE() except one, the DIR* is never
referenced after closing all the way until it goes out of
scope. virCapabilitiesInitCaches(), however, reuses the same DIR* over
and over in a loop, but due to having many error conditions that
result in a goto out of the loop, it's not well suited to reducing the
scope of the variable until we introduce a g_autoptr cleanup function
for DIR*.

In preparation for doing just that, we need to get rid of the side
effect of VIR_DIR_CLOSE() setting the DIR* to NULL, so in this one
case, let's manually set the DIR* to NULL. Then in an upcoming patch
we can safely remove the side effect from VIR_DIR_CLOSE().

This extra/ugly bit of code is only temporary: once we introduce the
g_autoptr cleanup function for DIR*, we will remove this manual
close/clear completely anyway.

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
21f659d952 storage: remove extraneous call to VIR_DIR_CLOSE()
VIR_DIR_CLOSE(dir) is called in the middle of
virStorageBackendRefreshLocal(), which is okay, but redundant - there
is no reference to dir between that call and the end of the function,
where VIR_DIR_CLOSE() is called again. Remove the extra call in the
middle to simplify the function and make the conversion to g_autoptr
trivial/mechanical.

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
59fc1e35eb tools: reduce scope of a DIR* in virHostValidateIOMMU()
This will make the trivial nature of a conversion to g_autoptr (in a
later patch) more obvious.

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
Michal Privoznik
fafeed8bed virDomainNetDefParseXML: Fix error message for unknown value of //mac/@type
In v6.6.0-rc1~124 we've introduced a new mechanism for MAC
addresses for ESX: ignore all checks (type='static') that libvirt
or ESX would do (and possibly fail) for specified MAC address.
Accepted values for the @type attribute are "generated" and
"static". But the error message mentions a different attribute.

Fixes 454e5961ab
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1892130
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-02 20:49:38 +01:00
Matt Coleman
b1a06feed7 hyperv: do not overwrite errors from hypervInvokeMethod()
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-02 18:44:21 +01:00
Matt Coleman
f1c406a9aa hyperv: reduce duplicate code for Msvm_ComputerSystem lookups
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-02 18:44:21 +01:00
Matt Coleman
855af506d3 hyperv: remove unneeded braces in hypervDomainGetInfo() and hypervDomainGetXMLDesc()
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-02 18:44:21 +01:00
Matt Coleman
a7fc186fd2 hyperv: remove duplicate function hypervGetMemSDByVSSDInstanceId()
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-02 18:44:21 +01:00
Matt Coleman
e08393e339 hyperv: remove duplicate function hypervGetVSSDFromUUID()
Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-02 18:44:21 +01:00