Previously we only checked MAC address and PCI address (or CCW
address). This is not enough information in cases where PCI address
isn't provided and multiple interfaces have the same MAC address (for
example, a virtio + hostdev "teaming" pair - their MAC addresses are
always the same).
Resolves: https://bugzilla.redhat.com/1926190
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
TPM devices with model='tpm-tis' are only valid with x86 and aarch64
virt machines. Add a check to qemuValidateDomainDeviceDefTPM() to
ensure VIR_DOMAIN_TPM_MODEL_TIS is only used with these architectures.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Starting a VM with swtpm device fails with qemu-system-aarch64.
E.g. with TPM device config
<tpm model='tpm-tis'>
<backend type='emulator' version='2.0'/>
</tpm>
QEMU reports the following error
error: internal error: process exited while connecting to monitor:
2021-02-07T05:15:35.378927Z qemu-system-aarch64: -device
tpm-tis,tpmdev=tpm-tpm0,id=tpm0: 'tpm-tis' is not a valid device model name
Indeed the TPM device name is 'tpm-tis-device' [1][2] for aarch64,
versus the shorter 'tpm-tis' for x86. The devices are the same from
a functional POV, i.e. they both emulate a TPM device conforming to
the TIS specification. Account for the unfortunate name difference
when building the TPM device option in qemuBuildTPMDevStr(). Also
include a test case for 'tpm-tis-device'.
[1] https://qemu.readthedocs.io/en/latest/specs/tpm.html
[2] c294ac327c
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The function is a wrapper on top of glibs g_strsplit, so is covered by
glibs testing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
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>
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>
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>
Some callers don't need to know the actual lenght of the list but only
care whether the required element is present or the list is non-empty.
Don't calculate the list length in those cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'cells' can be pushed into the loop removing the need for manual
cleanup, the check whether 'line' is NULL inside of the loop is always
false since the loop checks it right before and 'line' variable is
unnecessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
The glib variant doesn't accept NULL list, but there's just one caller
where it wasn't checked explicitly, thus there's no need for our own
wrapper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory freeing and remove the 'cleanup' label. Also make
it a bit more obvious that nothing happens if the 'old' list wasn't
present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The caller doesn't care about the number of tokens so use the function
which doesn't return it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
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>
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>
The validation code looks whether certain paths are in the 'notRestored'
list. For the purpose of lookup it's better to use a hash table rather
than a string list.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
To allow later removal of 'virStringListAdd' add an arbitrary upper
limit on the number of args we care about and don't store more than
that until necessary later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We are already looping over the arguments to construct the list, so we
can add them to fwBuf right away rather than in an extra loop if we move
some of the 'fwBuf' parts earlier and merge the two loops.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Precalculate the lenght to avoid use of 'virStringListAdd' in a loop.
The code is also simplified by using APIs which don't return errors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
The conversion removes the use of virStringListAdd/virStringListRemove
which try to add dynamic properties to a string list which is really
inefficient.
Storing the dbus VMState ids in a GSList is pretty straightforward and
the slightly increased complexity of the code will be paid back by
removing the string list helpers later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
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>
Pre-allocate the list to the upper bound and fill it gradually. Since
the data is kept long-term and the list won't be populated much shrink
it to the actual size after parsing.
While using 'virStringListAdd' here wouldn't be as expensive as this
function is used just once, the removal will allow to remove
'virStringListAdd' altogether to discourage the antipattern it promotes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We already know the upper bound of items we might need so we can
allocate the array upfront and avoid the quadratic complexity of
'virStringListAdd'.
In this instance the returned data is kept only temporarily so a
potential unused space due to filtered-out entries doesn't impose a
long-term burden on memory.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'virHashGetItems' already returns the number of entries which will be
considered for addition to the list so we can allocate it to the upper
bound upfront rather than growing it in a loop. This avoids the
quadratic complexity of 'virStringListAdd'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'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>
'virStringListAdd' calculates the string list length on every invocation
so constructing a string list using it results in O(n^2) complexity.
Use a GSList which has cheap insertion and iteration and doesn't need
failure handling.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
Some code paths return -1 directly while others jump to 'cleanup' which
cleans the list of mounts. Since qemuDomainGetPreservedMounts now
returns a NULL-terminated list, convert devMountsPath to g_auto(GStrv)
and remove the cleanup altoghether.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'i' is used in both outer and inner loop. Since 'devMountsPath' is now a
NULL-terminated list, we can use a GStrv to iterate it;
Additionally rewrite the conditional of adding to the 'unlinkPaths'
array so that it's more clear what's happening.
Fixes: 5c86fbb72d
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Refactor the handling of internals so that NULL-terminated lists are
always returned.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In commit 88957116c9 I've adapted
libvirt to QEMU's deprecation of -mem-path and -mem-prealloc and
switched to memory-backend-* even for system memory. My claim was
that that's what QEMU does under the hood anyway. And indeed it
was: see QEMU commit 900c0ba373aada4c13d47d95330aa72ec4067ba5 and
look at function create_default_memdev().
However, then commit d96c4d5f193e0e45beec80a6277728b32875bddb was
merged into QEMU. While it was fixing a bug, it also changed the
create_default_memdev() function in which it started turning off
use of canonical path (by setting
"x-use-canonical-path-for-ramblock-id" attribute to false). This
wasn't documented until QEMU commit
8db0b20415c129cf5e577a593a4a0372d90b7cc9. The path affects
migration - the same path has to be used on the source and on the
destination. Therefore, if there is old guest started with '-m X'
it has "pc.ram" block which doesn't use canonical path and thus
when migrating to newer QEMU which uses memory-backend-* we have
to turn off the canonical path explicitly. Otherwise,
"/objects/pc.ram" path would be expected by QEMU which doesn't
match the source.
Ideally, we would need to set it only for some machine types
(4.0 and older) because newer machine types already do what we
are doing. However, we treat machine types as opaque strings and
therefore we don't want to parse nor inspect their versions. But
then again, newer machine types already do what we are doing in
this commit, so when old machine types are deprecated and removed
we can remove our hack and forget it ever happened.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912201
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This capability tracks whether memory-backend-file has
"x-use-canonical-path-for-ramblock-id" attribute. Introduced into
QEMU by commit fa0cb34d2210cc749b9a70db99bb41c56ad20831. As of
QEMU commit 8db0b20415c129cf5e577a593a4a0372d90b7cc9 the property
is considered stable by qemu despite the 'x-' prefix to preserve
compatibility with released qemu versions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The 'conflict' key in a virt_daemon_unit dictionary is not used when
generating systemd service and socket files. The comment associated
with the key claims the default is 'true', and a few build files
needlessly set it to 'true' when defining their virt_daemon_unit.
Remove the 'conflict' key and its use in the affect build files.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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>
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>
