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>
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>
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>
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>
Remove one level of indentation by splitting the condition.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@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.
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>
The "max" model can be treated the same way as "host" model in general.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is a special CPU model similar to "-cpu host", so won't use our
normal CPU model detection logic.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The logic applied in the ppc64 case isn't quite correct, as the
interpretation of maximum mode depends on whether hardware virt
is used or not. This is information the CPU driver doesn't have.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The data reported is the same as for "host-passthrough"
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
For hardware virtualization this is functionally identical to the
existing host-passthrough mode so the same caveats apply.
For emulated guest this exposes the maximum featureset supported by
the emulator. Note that despite being emulated this is not guaranteed
to be migration safe, especially if different emulator software versions
are used on each host.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The @vhostuser member of virStorageSource structure is allocated
during parsing in virDomainDiskSourceVHostUserParse() but never
freed leading to a memleak. Since the member is an object it has
to be unrefed instead of g_free()-d.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
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>
A <machine/> element can have "deprecated" attribute that
corresponds to 'deprecated' member of _virQEMUCapsMachineType
struct. But the member is of boolean type. Therefore, the string
returned by virXMLPropString() must be freed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If parsing "maxCpus" attribute of <machine/> element fails an
error is printed but the corresponding string is not freed. While
it is very unlikely to happen (parsed XML is not user provided
and we are the ones generating it), it is possible. Instead of
freeing the variable in the error path explicitly, let's declare
it as g_autofree. And while I'm at it, let's bring it into the
loop where it's used.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The various virtproxyd socket files are generated with invalid syntax,
e.g. from virtproxyd.socket
[Unit]
Description=Libvirt proxy local socket
Before=virtproxyd.service
libvirtd.socket libvirtd-ro.socket libvirtd-admin.socket libvirtd-tcp.socket libvirtd-tls.socket
Note the missing 'Conflicts=' in the last line. Fix it by prepending
'Conflicts=' to libvirtd_socket_conflicts when adding virtproxyd
to virt_daemon_units.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
On platforms that lack both getauxval() and elf_aux_info(),
such as OpenBSD and macOS, host CPU detection can't work.
https://gitlab.com/libvirt/libvirt/-/issues/121
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
No need to fetch the same information twice.
As a side effect, this solves a bug where, on platforms where
elf_aux_info() is used instead of getauxval(), we would not
make sure the CPUID feature is available before attempting to
use it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This header is not present on several non-Linux targets that
nonetheless support aarch64.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
A few commits back I've introduced new 'virtio-pmem' <memory/>
device. Since it's virtio it goes onto PCI bus. Therefore, on
hotplug new PCI address is generated (or provided one is
reserved). However, if hotplug fails (for whatever reason) the
address needs to be released. This is different to 'dimm' type of
address because for that type we don't keep a map of used slots
rather generate one on each address assign request. The map is
then thrown away. But for PCI addresses we keep internal state
and thus has to keep it updated. Therefore, this new
qemuDomainReleaseMemoryDeviceSlot() function is NOP for those
models which use 'dimm' address type ('dimm' and 'nvdimm').
While I'm at it, let's release the address in case of hot unplug.
Not that is supported (any such attempt fails with the following
error:
"virtio based memory devices cannot be unplugged"
But if QEMU ever implements hot unplug then we don't have to
remember to fix our code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Currently, nmdm console device requires user to specify master and slave
path attributes (such as /dev/nmdm0A and /dev/nmdm0B respectively).
However, making user find a non-occupied device name might be not
convenient, especially for the remote connections.
Update the logic to make these attributes optional. In case if not
specified, use /dev/nmdm$UUID[AB], where $UUID is a domain's UUID.
With this schema it's unlikely nmdm device will clash with other domains
or even other non-bhyve nmdm devices.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All of these options are actually supported by vhostuser disk so
we should allow them to be usable.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Currently, requesting domain capabilities fails when the specified
emulator binary does not equal to "/usr/sbin/bhyve". As we're
not using user-specified emulator anyway, drop this check to avoid
showing errors for values like "bhyve" (without absolute path).
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Get rid of the 'need_release' variable. The code can be rewritten
so that it is not needed.
Signed-off-by: Yi Li <yili@winhong.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
