Commit Graph

5182 Commits

Author SHA1 Message Date
Daniel Henrique Barboza
ac81176614 virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListDel()
This change will allow us to remove PCI devices from a list
without the need of a PCI Device object, which will be need
in the next patch.

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
de80a10738 virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListSteal()
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
f1370f9ca6 virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListFind()
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
d7d1479fc0 virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListFindIndex()
We're going to need a way to remove a PCI Device from a list without having
a valid virPCIDevicePtr, because the device is missing from the host. This
means that virPCIDevicesListDel() must operate with a PCI Device address
instead.

Turns out that virPCIDevicesListDel() and its related functions only use
the virPCIDeviceAddressPtr of the virPCIDevicePtr, so this change is
simple to do and will not cause hassle in all other callers. Let's
start adapting virPCIDeviceListFindIndex() and crawl our way up to
virPCIDevicesListDel().

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Daniel Henrique Barboza
3acc65e1b0 virpci: introduce virPCIDeviceExists()
We're going to add logic to handle the case where a previously
existing PCI device does not longer exist in the host.

The logic was copied from virPCIDeviceNew(), which verifies if a
PCI device exists in the host, returning NULL and throwing an
error if it doesn't. The NULL is used for other errors as well
(product/vendor id read errors, dev id overflow), meaning that we
can't re-use virPCIDeviceNew() for the purpose of detecting
if the device exists.

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-03-01 12:25:33 -03:00
Kristina Hanicova
155151a3d0 Use g_steal_pointer where possible
Via coccinelle (not the handbag!)
spatches used:
@ rule1 @
identifier a, b;
symbol NULL;
@@

- b = a;
  ... when != a
- a = NULL;
+ b = g_steal_pointer(&a);

@@

- *b = a;
  ... when != a
- a = NULL;
+ *b = g_steal_pointer(&a);

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2021-03-01 15:54:42 +01:00
Peter Krempa
8298a5bd69 virJSONParserInsertValue: Take double pointer for @value
The function calls virJSONValueObjectAppend/virJSONValueArrayAppend, so
by taking a double pointer we can drop the pointer clearing from
callers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:38 +01:00
Peter Krempa
6b12e220b0 virJSONValueNewNumber: Take ownership of passed string
Avoid pointless copies of temporary strings when constructing number
JSON objects.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:38 +01:00
Peter Krempa
395ecd7a8c virJSONParserHandle*: Refactor memory cleanup and drop NULL checks
virJSONValueNew* won't return error nowadays so NULL checks are not
necessary. The memory can be cleared via g_autoptr.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:38 +01:00
Peter Krempa
6431b20c3e virJSONValueArrayAppend: Clear pointer when taking ownership of passed value
The parent array takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
d4e369a4c3 virMACMapHashDumper: Refactor array addition
Use automatic memory freeing and don't check return value of
virJSONValueNewString as it can't fail.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
5fc3892891 virJSONValueObjectAppend: Clear pointer when taking ownership of passed value
The parent object takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
e4b26c48cb virJSONValueObjectAddVArgs: Use autofree for the temporary bitmap
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
94ab321ffc virJSONValueNewArrayFromBitmap: Refactor cleanup
Use g_autoptr for the JSON value objects and remove the cleanup label
and inline freeing of objects.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
7d3a33b26b virJSONValue(Array|Object)Append*: Simplify handling of appended object
Use g_autofree for the pointer of the added object and remove the NULL
checks for values returned by virJSONValueNew* (except
virJSONValueNewNumberDouble) since they can't fail nowadays.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
3e411cbc5f virJSONValueCopy: Don't use virJSONValue(Object|Array)Append
We know the exact number of keys or array members for the copied objects
so we can pre-allocate the arrays rather than inserting into them in a
loop incurring realloc copy penalty.

Also virJSONValueCopy now can't fail since all of the functions
allocating the different cases use just g_new/g_strdup internally so we
can remove the NULL checks from the recursive calls.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:37 +01:00
Peter Krempa
b116e715a8 virJSONValueObjectInsert: Clear @value on successful insertion
The function takes ownership of @value on success so the proper
semantics will be to clear out the @value pointer. Convert @value to a
double pointer to do this.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Peter Krempa
9a1651f64d virLockSpacePreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-20 13:26:36 +01:00
Bruno Haible
66611bc0df util: Fix file descriptor passing on 64-bit FreeBSD and NetBSD.
* src/util/virsocket.c (virSocketRecvFD): Set msg.msg_controllen as documented
in the man pages.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-17 10:15:33 +00:00
Daniel P. Berrangé
2931839966 qemu: remove support for generating yes|no boolean options
All callers are now using the on|off syntax, so yes|no is a unreachable
code path.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-16 14:03:13 +00:00
Daniel P. Berrangé
0d981fcd97 qemu: use on|off instead of yes|no for -drive boolean properties
QEMU has long accepted many different values for boolean properties, but
set accepted has been different depending on which QEMU parser you hit.

The on|off values were supported by all QEMU parsers. The yes|no, y|n,
true|false values were only partially supported:

  https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01012.html

Thus we should standardize on on|off everywhere since that is most
widely supported in QEMU.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-16 14:03:13 +00:00
Daniel P. Berrangé
8851d87556 qemu: use on|off instead of yes|no for -object boolean properties
QEMU has long accepted many different values for boolean properties, but
set accepted has been different depending on which QEMU parser you hit.

The on|off values were supported by all QEMU parsers. The yes|no, y|n,
true|false values were only partially supported:

  https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01012.html

Thus we should standardize on on|off everywhere since that is most
widely supported in QEMU.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-16 14:03:13 +00:00
Pavel Hrdina
6a1f5e8a4f vircgroup: correctly free nested virCgroupPtr
Fixes: 184245f53b

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-15 19:21:35 +01:00
Laine Stump
04e90f72a7 util: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-12 12:10:38 -05:00
Peter Krempa
cafde24a9a util: virstring: Remove virStringListJoin
The glib alternative is now used everywhere.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
480fecaa21 Replace virStringListJoin by g_strjoinv
Our implementation was inspired by glib anyways. The difference is only
the order of arguments.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
dc7ac81d37 virStringSplitCount: Reimplement using g_strsplit and g_strv_length
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
50cc5d7035 util: virstring: Remove virStringSplit
Callers were replaced by g_strsplit.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
10157731f4 Replace virStringSplit with g_strsplit
Our implementation was heavily inspired by the glib version so it's a
drop-in replacement.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
51f8baee8d util: virstring: Remove virStringListLength
glib provides g_strv_length.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
dcd547aec1 Replace virStringListLength by g_strv_length
The glib implementation doesn't tolerate NULL but in most cases we check
before anyways. The rest of the callers adds a NULL check.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
1114cf5e7e virPolkitCheckAuth: Avoid virStringListLength in loop condition
Don't re-calculate the string list length on every iteration. Convert
the loop to NULL-terminated iteration.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:34 +01:00
Peter Krempa
4661ea3578 util: virstring: Remove virStringListHasString
All callers were converted to the glib alternative. Providing our own
just to have NULL tolerance doesn't make sense.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
e6c9c44e9a virStringListGetFirstWithPrefix: Remove unused helper
This is a uncommon and trivial operation, so having an utility function
for it is pointless.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
7cc3418915 virCgroupGetValueForBlkDev: Rewrite lookup of returned string
Lookup the string with prefix locally so that we can remove the helper
which isn't universal at all.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
2b3e390674 util: virstring: Remove virStringListAdd and virStringListRemove
virStringListAdd hides the fact that a O(n) count of elements is
performed every time it's called which makes it inefficient.

Stop supporting such semantics and remove the helpers. Users have a
choice of using GSList or an array with a counter variable rather than
repeated lookups.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
00dfd9c97d util: macmap: Convert to use GSList for storing macs instead of string lists
Since adding and removing is the main use case for the macmap module,
convert the code to a more efficient data structure.

The refactor also optimizes the loading from file where previously we'd
do a hash lookup + list lenght calculation for every entry.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:33 +01:00
Peter Krempa
b297714793 virResctrlMonitorGetStats: Don't use 'virStringListAdd'
The iner loop copies the 'resources' array multiple times using
'virStringListAdd' which has O(n^2) complexity.

Pre-calculate the length so we can allocate the array upfront and just
copy the strings in the loop.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
34088ea47f virResctrlInfoGetMonitorPrefix: Don't use 'virStringListAdd' to construct list
Pre-allocate a buffer for the upper limit and shrink it afterwards to
avoid use of 'virStringListAdd' in a loop.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
97fd333fde virHookCall: Don't use 'virStringListAdd' to construct list in loop
'virStringListAdd' calculates the string list length on every invocation
so constructing a string list using it results in O(n^2) complexity.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
0ec601bc48 util: Add helpers for auto-freeing GSList filled with strings
glib's 'g_autoslist()' doesn't support lists of 'char *' strings. Add a
type alias 'virGSListString' so that we can register an 'autoptr'
function for it for simple usage of GSList with strings.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
32def543d1 util: macmap: Remove unused cleanup labels and 'ret' variables
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:32 +01:00
Peter Krempa
a2e64fc6af util: virmacmap: Use g_autofree for virJSONValue
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-11 17:05:31 +01:00
Pavel Hrdina
184245f53b vircgroup: introduce nested cgroup to properly work with systemd
When running on host with systemd we register VMs with machined.
In this case systemd creates the root VM cgroup for us. This has some
implications where one of them is that systemd owns all files inside
the root VM cgroup and we should not touch them.

We already use DBus calls for some of the APIs but for the remaining
ones we will continue accessing the files directly. Systemd doesn't
support threaded cgroups so we need to do this.

The reason why we don't use DBus for most of the APIs is that we already
have a code that works with files and we would have to check if systemd
supports each API.

This change introduces new topology on systemd hosts:

$ROOT
  |
  +- machine.slice
     |
     +- machine-qemu\x2d1\x2dvm1.scope
        |
        +- libvirt
           |
           +- emulator
           +- vcpu0
           +- vcpu0

compared to the previous topology:

$ROOT
  |
  +- machine.slice
     |
     +- machine-qemu\x2d1\x2dvm1.scope
        |
        +- emulator
        +- vcpu0
        +- vcpu0

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:12 +01:00
Pavel Hrdina
badc2bcc73 vircgroup: introduce virCgroupV1Exists and virCgroupV2Exists
This will check if the cgroup actually exists on the system.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:12 +01:00
Pavel Hrdina
382fa15cde vircgroupv2: move task into cgroup before enabling controllers
When we create a new child cgroup and the parent cgroup has any process
attached to it enabling controllers for the child cgroup fails with
error. We need to move the process into the child cgroup first before
enabling any controllers.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:12 +01:00
Pavel Hrdina
5f56dd7c83 vircgroupv1: refactor virCgroupV1DetectPlacement
Remove one level of indentation by splitting the condition.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:12 +01:00
Pavel Hrdina
9c1693eff4 vircgroup: use DBus call to systemd for some APIs
When running on host with systemd we register VMs with machined.
In this case systemd creates the root VM cgroup for us. This has some
implications where one of them is that systemd owns all files inside
the root VM cgroup and we should not touch them.

If we change any value in file that systemd knows about it will be
changed to what systemd thinks it should be when executing
`systemctl daemon-reload`.

These are the APIs that we need to call using systemd because they set
limits that are proportional to sibling cgroups.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:12 +01:00
Pavel Hrdina
d3fb774b1e virsystemd: introduce virSystemdGetMachineUnitByPID
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:11 +01:00
Pavel Hrdina
385704d5a4 virsystemd: introduce virSystemdGetMachineByPID
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:11 +01:00
Pavel Hrdina
a51147d906 virsystemd: export virSystemdHasMachined
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-10 13:37:11 +01:00
Michal Privoznik
d3b2d5158a lib: Substitute some STREQLEN with STRPREFIX
There are few cases where STREQLEN() is called like this:

  STREQLEN(var, string, strlen(string))

which is the same as STRPREFIX(var, string). Use STRPREFIX()
because it is more obvious what the check is doing.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
2021-02-10 11:51:59 +01:00
Yalei Li
b29c86ae10 util: Remove '\n' from vhostuser ifname
When deleting the vhostuserclient interface, OVS prompts that the interface does not exist,
Through the XML file, I found that the "target dev" has a '\n', results in an XML parsing error.

XML file:

<target dev='vm-20ac9c030a47
'/>

That is because 'ovs-vsctl' returns a newline result, always come with a '\n',
and the vircommandrun function puts it in ifname.

So virNetDevOpenvswitchGetVhostuserIfname should remove '\n' from ifname.

Signed-off-by: Yalei Li <liyl43@chinatelecom.cn>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-05 09:57:14 +01:00
Laine Stump
2ca7234d7d util: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-05 00:22:09 -05:00
Laine Stump
8626fb378c util: rename two *Free() functions while changing VIR_FREE to g_free
dhcpHostFree() and addnHostFree() don't follow the normal pattern of
*Free functions in the rest of libvirt code - they are actually more
similar to the *Dispose() functions, in that they free all subordinate
objects, but not the object pointed to by the argument
itself. However, the arguments aren't virObjects, so it wouldn't be
proper to name them *Dispose() either.

They *currently* behave similar to a *Clear() function, in that they
free all the subordinate objects and nullify the pointers of those
objects. HOWEVER, we don't actually need or want that behavior - the
two functions in question are only called as part of a higher level
*Free() function, and the pointers are not referenced in any way
between the time they are freed and when the parent object is freed.

So, since the current name isn't correct, nor is *Dispose(), and we
want to change the behavior in such a way that *Clear() also wouldn't
be correct, lets name the functions *FreeContent(), which is an
accurate description of what the functions do, and what we *want* them
to do.

And since it's such a small patch, we can go ahead and change that
behavior - replacing the VIR_FREEs with g_free.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-05 00:20:45 -05:00
Laine Stump
238d96b8f1 util: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-02-05 00:20:43 -05:00
Michal Privoznik
3426bc5882 vircgroup: Don't leak @parent in virCgroupEnableMissingControllers()
A memory leak was identified in
virCgroupEnableMissingControllers():

==11680==    at 0x483EAE5: calloc (vg_replace_malloc.c:760)
==11680==    by 0x4E51780: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6701.0)
==11680==    by 0x4908618: virCgroupNew (vircgroup.c:701)
==11680==    by 0x49096F4: virCgroupEnableMissingControllers (vircgroup.c:1146)
==11680==    by 0x4909B17: virCgroupNewMachineSystemd (vircgroup.c:1228)
==11680==    by 0x4909E94: virCgroupNewMachine (vircgroup.c:1313)
==11680==    by 0x1694FDBC: qemuInitCgroup (qemu_cgroup.c:946)
==11680==    by 0x1695046B: qemuSetupCgroup (qemu_cgroup.c:1083)
==11680==    by 0x16A60126: qemuProcessLaunch (qemu_process.c:7077)
==11680==    by 0x16A61504: qemuProcessStart (qemu_process.c:7384)
==11680==    by 0x169B84C2: qemuDomainObjStart (qemu_driver.c:6590)
==11680==    by 0x169B8776: qemuDomainCreateWithFlags (qemu_driver.c:6641)

What happens is that new virCgroup is created and stored into
@parent. Then, if @tokens is not empty the for() loop is entered
into where another virCgroup is created and @parent is replaced
with this new virCgroup. But nothing freed the old @parent.

Fixes: 77291414c7
Reported-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2021-02-04 09:08:16 +01:00
Peter Krempa
bfdfa64010 viralloc: Remove VIR_ALLOC_VAR
The use case VIR_ALLOC_VAR deals with is very unlikely. We had just 2
legitimate uses, which were reimplemented locally using g_malloc0 and
sizeof instead as they used a static number of members of the trailing
array.

Remove VIR_ALLOC_VAR since in most cases the direct implementation is
shorter and clearer and there are no users of it currently.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-03 16:09:25 +01:00
Peter Krempa
52ca0a6229 virNetDevGetEthtoolGFeatures: Avoid use of VIR_ALLOC_VAR
In this case we need a 'struct ethtool_gfeatures' followed by two
'struct ethtool_get_features_block' so there's no risk of overflow.

Use g_malloc0 and sizeof() to allocate the memory instead of
VIR_ALLOC_VAR.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-03 16:09:25 +01:00
Peter Krempa
ed97683897 util: alloc: Remove VIR_DISPOSE_STRING
Users were replaced with virSecureEraseString with explicit freeing of
the memory.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-03 13:07:14 +01:00
Peter Krempa
2025001609 util: alloc: Remove VIR_AUTODISPOSE_STR
There are no users any more. The replacement is to use g_auto and
virSecureEraseString explicitly.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-03 13:07:14 +01:00
Peter Krempa
39089a6faf util: virsecureerase: Introduce virSecureEraseString
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-03 13:07:13 +01:00
Peter Krempa
bacf612607 util: viralloc: Remove VIR_DISPOSE(_N)
The macros are unused now and callers who care about clearing the memory
they use should use memset() appropriately.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-03 13:07:13 +01:00
Peter Krempa
91858434b4 virCryptoEncryptDataAESgnutls: Use virSecureErase instead of memset
Clear the key and IV structs using virSecureErase.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-03 13:07:13 +01:00
Peter Krempa
288d051494 virCryptoEncryptDataAESgnutls: Use virSecureErase instead of VIR_DISPOSE_N
Clear out the value using virSecureErase and free it with g_free so
that VIR_DISPOSE_N can be phased out.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-03 13:07:13 +01:00
Peter Krempa
43696418af util: Introduce virsecureerase module
The module will provide functions for disposing secrets stored in
memory.

Note that for now it's implemented using memset, which is not really
secure.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-03 13:07:12 +01:00
Peter Krempa
d115019b6a util: virstring: Remove unused prototypes for virStr(n)dup
The headers weren't removed after use of VIR_STRDUP was removed.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2021-02-02 15:10:47 +01:00
Tim Wiederhake
8b1755024b vircommand: Simplify virCommandAddArg
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-02-02 15:00:55 +01:00
Tim Wiederhake
2cdbfbe7ac virhostuptime: Fix rounding in uptime calculation
"f + 0.5" does not round correctly for values very close to
".5" for every integer multiple, e.g. "0.499999975".

Found by clang-tidy's "bugprone-incorrect-roundings" check.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-02-02 15:00:55 +01:00
Tim Wiederhake
1e2e8ac88f Replace bzero() with memset()
This was found by clang-tidy's
"clang-analyzer-security.insecureAPI.bzero" check.

bzero is marked as deprecated ("LEGACY") in POSIX.1-2001 and
removed in POSIX.1-2008.

Besides its deprecation, bzero can be unsafe to use under certain
circumstances, e.g. when used to zero-out memory containing secrects.
These calls can be optimized away by the compiler, if it concludes no
further access happens to the memory, thus leaving the secrets still
in memory. Hence its classification as "insecureAPI".

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-02-02 15:00:55 +01:00
Tim Wiederhake
8b8d6a24f9 virfile: Remove redundant #ifndef
This section is guarded by "#ifndef WIN32" in line 2109--2808.

Found by clang-tidy's "readability-redundant-preprocessor" check.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-02-02 15:00:55 +01:00
Roman Bogorodskiy
31d1835428 virfile: workaround for when posix_fallocate() is not supported by FS
posix_fallocate() might be not supported by a filesystem, for example,
it's not supported by ZFS. In that case it fails with
return code 22 (EINVAL), and thus safezero_posix_fallocate() returns -1.

As safezero_posix_fallocate() is the first function tried by safezero()
and it tries other functions only when it returns -2, it fails
immediately without falling back to other methods, such as
safezero_slow().

Fix that by returning -2 if posix_fallocate() returns EINVAL, to give
safezero() a chance to try other functions.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-02-01 20:15:22 +04:00
Daniel Henrique Barboza
b0264e9404 virpci.c: simplify virPCIDeviceNew() signature
The current virPCIDeviceNew() signature, receiving 4 uints in sequence
(domain, bus, slot, function), is not neat.

We already have a way to represent a PCI address in virPCIDeviceAddress
that is used in the code. Aside from the test files, most of
virPCIDeviceNew() callers have access to a virPCIDeviceAddress reference,
but then we need to retrieve the 4 required uints (addr.domain, addr.bus,
addr.slot, addr.function) to satisfy virPCIDeviceNew(). The result is
that we have extra verbosity/boilerplate to retrieve an information that
is already available in virPCIDeviceAddress.

A better way is presented by virNVMEDeviceNew(), where the caller just
supplies a virPCIDeviceAddress pointer and the function handles the
details internally.

This patch changes virPCIDeviceNew() to receive a virPCIDeviceAddress
pointer instead of 4 uints.

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-01-29 17:52:10 -03:00
Daniel Henrique Barboza
03f9c17805 virpci, domain_audit: use virPCIDeviceAddressAsString()
There is no need to open code the PCI address string format
when we have a function that does exactly that.

Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2021-01-29 17:46:34 -03:00
Peter Krempa
225c568378 util: Remove unused 'virStorageFileParseChainIndex'
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2021-01-27 07:49:58 +01:00
Peter Krempa
04489d9fca util: virstoragefile: Move virStorageIs[File|Relative] to storage_source
There are no other files using it. Move it and make the functions
static.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
2021-01-27 07:49:57 +01:00
Dmytro Linkin
5b1c525b1f util: Add phys_port_name support on virPCIGetNetName
virPCIGetNetName is used to get the name of the netdev associated with
a particular PCI device. This is used when we have a VF name, but need
the PF name in order to send a netlink command (e.g. in order to
get/set the MAC address of the VF).

In simple cases there is a single netdev associated with any PCI
device, so it is easy to figure out the PF netdev for a VF - just look
for the PCI device that has the VF listed in its "virtfns" directory;
the only name in the "net" subdirectory of that PCI device's sysfs
directory is the PF netdev that is upstream of the VF in question.

In some cases there can be more than one netdev in a PCI device's net
directory though. In the past, the only case of this was for SR-IOV
NICs that could have multiple PF's per PCI device. In this case, all
PF netdevs associated with a PCI address would be listed in the "net"
subdirectory of the PCI device's directory in sysfs. At the same time,
all VF netdevs and all PF netdevs have a phys_port_id in their sysfs,
so the way to learn the correct PF netdev for a particular VF netdev
is to search through the list of devices in the net subdirectory of
the PF's PCI device, looking for the one netdev with a "phys_port_id"
matching that of the VF netdev.

But starting in kernel 5.8, the NVIDIA Mellanox driver began linking
the VFs' representor netdevs to the PF PCI address [1], and so the VF
representor netdevs would also show up in the net
subdirectory. However, all of the devices that do so also only have a
single PF netdev for any given PCI address.

This means that the net directory of the PCI device can still hold
multiple net devices, but only one of them will be the PF netdev (the
others are VF representors):

$ ls '/sys/bus/pci/devices/0000:82:00.0/net'
ens1f0  eth0  eth1

In this case the way to find the PF device is to look at the
"phys_port_name" attribute of each netdev in sysfs. All PF devices
have a phys_port_name matching a particular regex

  (p[0-9]+$)|(p[0-9]+s[0-9]+$)

Since there can only be one PF in the entire list of devices, once we
match that regex, we've found the PF netdev.

[1] - https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/
      commit/?id=123f0f53dd64b67e34142485fe866a8a581f12f1

Co-Authored-by: Moshe Levi <moshele@nvidia.com>
Signed-off-by: Dmytro Linkin <dlinkin@nvidia.com>
Reviewed-by: Adrian Chiris <adrianc@nvidia.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-01-25 20:28:18 -05:00
Moshe Levi
97ebb98245 util: add virNetDevGetPhysPortName
This commit add virNetDevGetPhysPortName to read netdevice
phys_port_name from sysfs. It also refactor the code so
virNetDevGetPhysPortName and virNetDevGetPhysPortID will use
same method to read the netdevice sysfs.

Signed-off-by: Moshe Levi <moshele@nvidia.com>
Reviewed-by: Laine Stump <laine@redhat.com>
2021-01-25 20:27:38 -05:00
Pavel Hrdina
5ac39c4ab0 util: move virStorageEncryption code into conf
The code handles XML bits and internal definition and should be
in conf directory.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-22 11:10:27 +01:00
Pavel Hrdina
3e54766414 util: move virStorageSource code into conf
The code handles XML bits and internal definition and should be
in conf directory.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-22 11:10:27 +01:00
Pavel Hrdina
2cdd833eae util: move virStorageFileProbe code into storage_file
Same as virStorageFileBackend, it doesn't belong into util directory.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-22 11:10:27 +01:00
Pavel Hrdina
65abeb058f util: move virStorageFileBackend code into storage_file
It's used only by storage file code so it doesn't make sense to have
it in util directory.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-22 11:10:27 +01:00
Pavel Hrdina
01f7ade912 util: extract virStorageFile code into storage_source
Up until now we had a runtime code and XML related code in the same
source file inside util directory.

This patch takes the runtime part and extracts it into the new
storage_file directory.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-22 11:10:27 +01:00
Pavel Hrdina
296032bfb2 util: extract storage file probe code into virtstoragefileprobe.c
This code is not directly relevant to virStorageSource so move it to
separate file.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-22 11:10:27 +01:00
Pavel Hrdina
eaa0b3288e util: move virStorageSourceFindByNodeName into qemu_domain
It's only relevant for QEMU driver.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-22 11:10:27 +01:00
Pavel Hrdina
3e210d204c virstoragefile: change virStorageSource->drv to void pointer
This will allow following patches to move virStorageSource into conf
directory and virStorageDriverData into a new storage_file directory.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-22 11:10:27 +01:00
Pavel Hrdina
7b4e3bab5b virstoragefile: properly include virstoragefile.h header
It was indirectly included by virstoragefilebackend.h.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-22 11:10:27 +01:00
Matt Coleman
65e1b4fd26 hyperv: ambiguous VM names will throw an error
Since Hyper-V allows multiple VMs to be created with the same name,
some commands produce unpredictable results due to
hypervDomainLookupByName's WMI query selecting the wrong domain.

For example, this prevents `virsh dumpxml` from outputting XML for the
wrong domain.

Signed-off-by: Matt Coleman <matt@datto.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-01-21 10:18:21 +01:00
Shi Lei
037ea5d10c netlink: Introduce a helper function to simplify netlink functions
Extract common code as helper function virNetlinkTalk, then simplify
the functions virNetlink[DumpLink|NewLink|DelLink|GetNeighbor].

Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-01-15 11:45:41 +01:00
Shi Lei
871eba4d99 netlink: Introduce macro NETLINK_MSG_APPEND to wrap nlmsg_append
Introduce a macro NETLINK_MSG_APPEND to wrap nlmsg_append and
simplify code. Remove those labels 'buffer_too_small', since they
are now useless.

Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-01-15 11:45:41 +01:00
Shi Lei
121fdeacdf netlink: Minor changes for macros NETLINK_MSG_[NEST_START|NEST_END|PUT]
Move macros NETLINK_MSG_[NEST_START|NEST_END|PUT] from .h into .c;
within these macros, replace 'goto' with reporting error and returning;
simplify virNetlinkDumpLink and virNetlinkDelLink by using NETLINK_MSG_PUT.

Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-01-15 11:45:41 +01:00
Shi Lei
8133400234 netlink: Remove invalid flags(NLM_F_CREATE and NLM_F_EXCL) for RTM_DELLINK
NLM_F_CREATE and NLM_F_EXCL are invalid for RTM_DELLINK,
so remove them.

Signed-off-by: Shi Lei <shi_lei@massclouds.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2021-01-15 11:45:41 +01:00
Michal Privoznik
6f06ae15d0 openvswitch: Check if OVS_VSCTL exists when getting interface name
So far we assumed that any vhostuser interface is plugged into an
OVS bridge and thus 'ovs-vsctl' exists. But this is not always
true. In testing scenarios it is possible to create a vhostuser
interface with this tool dpdk-testpmd (part of dpdk RPM) which
creates/connects to UNIX socket needed for vhostuser. Of course,
since there is no OVS then there is no interface name in which
case virNetDevOpenvswitchGetVhostuserIfname() should return 0.

The rest of APIs that assume OVS are not 'fixed' because we still
want them to fail (e.g. getting statistics, plugging interface
into an OVS bridge, unplugging it from an OVS bridge, ...).

The only API that is fixed is
virNetDevOpenvswitchGetVhostuserIfname() because it is called
explicitly when starting a guest (and callers are okay if no name
was found).

The other way to fix this bug seems to be to simply require
'ovs-vsctl' on spec file level, but that is too heavy gun given
that vhostuser is used by a small set of our users (assumption
made on requirements for vhostuser). Also, this way would drag in
yet another dependency for all users (even those who want minimal
libvirt).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1913156
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-01-11 16:06:17 +01:00
Laine Stump
a4be2e35db util: Skip over any extra verbiage preceding version in dnsmasq version string
dnsmasq usually prints out a version string like this:

 Dnsmasq version 2.82 [...]

but a user reported that the build of dnsmasq included with pihole has
a version string like this:

 Dnsmasq version pi-hole-2.81 [...]

We parse the dnsmasq version number to figure out if the dnsmasq
binary supports certain features. Since we expect the version number
(and it must be only numbers!) to start on the first non-space after
the string "Dnsmasq version", we fail to parse this format of the
version string.

Rather than spending a bunch of time trying to get pihole to change
that, we can just make our parsing more permissive - after searching
for "Dnsmasq version", we'll skip ahead to the first decimal digit,
rather than just the first non-space.

(NB: The features we're checking for purely by looking at version
number have been in all releases of dnsmasq since at least 2012, so we
could actually just remove the reading of the version number
completely. However it's possible (although *highly* unlikely)
that some new feature would be added to dnsmasq in the future and we
would need to add that code back.)

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/29
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-01-08 10:15:31 -05:00
Laine Stump
0e89a7b4e0 util: new function virSkipToDigit()
This function skips over the beginning of a string until it reaches a
decimal digit (0-9) or the NULL at the end of the string. The original
pointer is modified in place (similar to virSkipSpaces()).

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-01-08 10:15:04 -05:00
Peter Krempa
154df5840d conf: Introduce <metadata_cache> subelement of <disk><driver>
In certain specific cases it might be beneficial to be able to control
the metadata caching of storage image format drivers of a hypervisor.

Introduce XML machinery to set the maximum size of the metadata cache
which will be used by qemu's qcow2 driver.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2021-01-08 15:27:00 +01:00
Ryan Gahagan
0f1f3f1228 util: virstoragefile: Add 'json:' pseudo-protocol parser for 'nfs' protocol
Enable parsing of backing store strings containing the native 'nfs'
protocol specification.

Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-08 15:09:26 +01:00
Ryan Gahagan
4b2f083c34 util: Add fields for VIR_STORAGE_NET_PROTOCOL_NFS to virStorageSource
'nfs_user'/'nfs_group' represents the XML configuration.

'nfs_uid'/'nfs_gid' is internal store when libvirt looks up the user's
uid/gid in the system.

Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-08 15:03:52 +01:00
Ryan Gahagan
6cfb4e2fe9 conf: Add VIR_STORAGE_NET_PROTOCOL_NFS disk protocol type
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2021-01-08 15:03:16 +01:00
Erik Skultety
0d49a565e5 Fix MinGW pipeline after 49cb59778a
Broken build job: https://gitlab.com/libvirt/libvirt/-/jobs/951162206

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2021-01-08 12:17:13 +01:00