Remove the code handling old Xen's hwcap words,
as well as the comment describing it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Two users of virEventGLibAddSocketWatch care about the GSource
it returns.
The other three free it by assigning it to an autofreed variable.
Mark them with G_GNUC_UNUSED to make this obvious to the reader
and the compiler.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In the unlikely case that we were unable to set the new
identity, we would unref the old one even though it still
could be in the thread-local storage.
Fixes: c6825d8813
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Unused as of:
commit effeee5c2f
qemu: driver: Use 'qemuDomainSaveStatus' for saving status XML
This function extracts the config from the vm object, so the caller
no longer needs to do it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We need to validate the XML against schema if option '--validate'
was passed to the virsh command. This patch also includes
propagation of flags into the virNWFilterBindingDefParse().
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>
We need to validate the XML against schema if option '--validate'
was passed to the virsh command. This patch also includes
propagation of flags into the virNetworkPortDefParse().
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>
The g_strdup_printf() function can't fail really. There's no need
to check for its return value.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In the previous commit I accidentally changed the mode of
qemu_driver.c file. Restore the original mode.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In process of iothread hotplug, qemuDomainHotplugAddIOThread() calls
qemuProcessSetupIOThread(). When qemuProcessSetupIOThread() returned
a failure, only the cgroup directory 'iothread' was cleaned up within
the function. Right after that qemuDomainHotplugAddIOThread() would
return failure directly without rolling back the livedef and iothread
process that created previously.
Further, when 'virsh schedinfo domain --live' requires schedinfo of
such machine, the interface will always return a failure print as
follows: 'Failed to create v1 controller cpu for group: No such file
or directory'. The reason is qemuGetIOThreadsBWLive() using member
vm->def->iothreadids[0]->iothread_id to findout the corresponding
cgroup dircetory. In case mentioned previously, iothreadids[0] was not
been cleaned up while whose cgroup directroy has already been removed.
This patch rolls back the livedef and iothread process after
qemuProcessSetupIOThread() returned a failure. Of course we are not
limited to this function, we also perform the same rolling back after
any exception proecss in qemuDomainHotplugAddIOThread().
Signed-off-by: Lei Yang <yanglei209@huawei.com>
Signed-off-by: Wang Xin <wangxinxin.wang@huawei.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Setup of a disk with <transient shareBacking='yes'/> option issues a
reset of qemu. In cases when QEMU didn't yet support the 'set-action'
QMP libvirt would in certain cases setup the commandline without
'-no-shutdown' which caused qemu to exit during startup. Forbid this
specific scenario.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Split out the logic which was used to determine whether qemu should
allow the guest OS to reboot for QEMU versions which don't support the
'set-action' QMP command.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The QEMU driver uses both virtlogd and virtlockd, while the Xen driver
uses virtlockd. The libvirtd.service unit contains deps on the socket
units for these services, but these deps were missed in the modular
daemons. As a result the virtlockd/virtlogd sockets are not started
when the virtqemud/virtxend daemons are started.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Attaching a newly created vhostuser port to a VM fails due to an
apparmor denial
internal error: unable to execute QEMU command 'chardev-add': Failed
to bind socket to /run/openvswitch/vhu838c4d29-c9: Permission denied
In the case of a net device type VIR_DOMAIN_NET_TYPE_VHOSTUSER, the
underlying chardev is not labeled in qemuDomainAttachNetDevice prior
to calling qemuMonitorAttachCharDev.
A simple fix would be to call qemuSecuritySetChardevLabel using the
embedded virDomainChrSourceDef in the virDomainNetDef vhostuser data,
but this incurs the risk of incorrectly restoring the label. E.g.
consider the DAC driver behavior with a vhostuser net device, which
uses a socket for the chardev backend. The DAC driver uses XATTRS to
store original labelling information, but XATTRS are not compatible
with sockets. Without the original labelling information, the socket
labels will be restored with root ownership, preventing other
less-privileged processes from connecting to the socket.
This patch avoids overloading chardev labelling with vhostuser net
devices by introducing virSecurityManager{Set,Restore}NetdevLabel,
which is currently only implemented for the apparmor driver. The
new APIs are then used to set and restore labels for the vhostuser
net devices.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that resource structure can have appid as well we need to adapt code
that creates default resource partition if not provided by user.
Otherwise starting a VM with appid defined would fail with following
error:
error: unsupported configuration: Resource partition '(null)' must start with '/'
Fixes: 38b5f4faab
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Clang on Rawhide started to complain that @tmp variable in
virSCSIDeviceListDel() is set but not used. This is obviously a
false positive because the variable is used to free device stolen
from the list. Anyway, we can do without the variable so in this
specific case let's fix our code to appease Clang.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virPCIDeviceIsBehindSwitchLackingACS() function checks
whether given PCI device is not behind a switch that lacks ACS.
It does so by starting at given device and traversing up, one
parent at time towards the root. The parent device is obtained
via virPCIDeviceGetParent() which allocates new virPCIDevice
structure. For freeing the structure we use g_autoptr() and a
temporary variable @tmp. However, Clang fails to understand our
clever algorithm and complains that the variable is set but never
used. This is obviously a false positive, but using a small trick
we can shut Clang up.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When the VM is inactive the 'virStorageSource' struct doesn't have the
necessary data pointing to the actual storage. This is a problem for
inactive snapshot operations on VMs which use disk type='volume'.
Add the translation steps for reversion and deletion of snapshots.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1977155
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/202
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In cases when we are adding a <transient/> disk with sharing backend
(and thus hotplugging it) we need to re-initialize ACPI tables so that
the VM boots from the correct device.
This has a side-effect of emitting the RESET event and forwarding it to
the clients which is not correct.
Fix this by ignoring RESET events during startup of the VM.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When qemu supports 'set-action' command we can update what happens on
reboot. Additionally we can fully relax the checks as we now properly
update the lifecycle actions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We don't use the value of the flag when the new handling is in place so
we don't have to initialize it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The '-no-shutdown' flag prevents qemu from terminating if a shutdown was
requested. Libvirt will handle the termination of the qemu process
anyways and using this consistently will allow greater flexibility for
the virDomainSetLifecycleAction API as well as will allow using
the 'system-reset' QMP command during startup to reinitiate devices
exported to the firmware.
This efectively partially reverts 0e034efaf9
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rather than using '-no-reboot' use the QMP command to update the
lifecycle action of 'on_reboot'.
This will be identical to how we set the behaviour during lifetime and
also avoids problems with use of the 'system-reset' QMP command during
bringup of the VM (used to update the firmware table of disks when disks
were hotplugged as part of startup).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The RESET event is delivered by qemu only when the guest OS is actually
allowed to reboot ('-no-reboot' or equivalent is not used) and due to
the nature of async handling of the events VM is actually already
executing guest code after the reboot, until our code gets to killing
it.
In general it should have been impossible to reach a state where the
reboot action is 'destroy' but we didn't use '-no-reboot' but due to
various bugs it was.
Due to the fact that this was not a desired operation and additionally
guest code already is executing I think the best option is not to kill
the VM any more (possible data loss?) and rely for the proper fix where
we use the new 'set-action' QMP command to enable an equivalent
behaviour to '-no-reboot' during runtime.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Without the ability to tell qemu to change the behaviour on reboot of
the guest it's fundamentally unsafe to change the action as the guest
would be able to execute instructions after the reboot before libvirt
terminates it due to the async nature of QMP events.
Stricten the code for now until we implement support for 'set-action'
QMP command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Directly use 'priv->allowReboot' as we now document what the behaiour is
to avoid another lookup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The original idea was to ensure that the destination has the same
original state of the '-no-reboot' flag to ensure identical behaviour of
the 'vidDomainModifyLifecycleAction' API.
With newer qemu's we'll be able to modify the behaviour using the
monitor so old daemons won't be able to keep up anyways.
Remove this feature as it's not very useful and will be replaced by a
proper solution.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Save further readers the headache of determining what it actually does
and note that it's not used with qemu version supporting the
'set-action' command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If current qemu supports 'set-action' use it instead of the single-use
command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'set-action' QMP command allows modifying the behaviour when the
guest resets.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We simply terminate qemu instead of issuing a reset as the semantics of
the setting dictate.
Fix it by handling it identically to 'fake reboot'.
We need to forbid the combination of 'onReboot' -> 'destroy' and
'onPoweroff' -> reboot though as the handling would be hairy and it
honetly makes no sense.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The qemu driver didn't ever implement any meaningful handling for the
'preserve' action.
Forbid the flag in the qemu def validator and update the documentation
to be factual.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some actions are not supported by qemu. Use the recently added
'qemuValidateLifecycleAction' helper to ensure that the API does the
same validation as we do on startup in the validation callbacks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The qemu driver didn't ever implement any meaningful handling for the
'rename-restart' action.
At this point the following handling would take place:
'on_reboot' set to 'rename-restart' is ignored on guest-initiated
reboots, the guest simply reboots.
For on_poweroff set to 'rename-restart' the following happens:
guest initiated shutdown -> 'destroy'
libvirt initiated shutdown -> 'reboot'
In addition when 'on_reboot' is 'destroy' in addition to 'on_poweroff'
being 'rename-restart' the guest is able to execute instructions after
issuing a reset before libvirt terminates it. This will be addressed
separately later.
Forbid the flag in the qemu def validator and update the documentation
to be factual.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We've got multiple random open-coded versions. Switch to the helper
function which doesn't report errors as they'd be mostly wrong as the
operation was indeed successful.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The public API wrapper range-checks the arguments. Save the next reader
the hassle of looking it up.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We need to validate the XML against schema if option '--validate'
was passed to the virsh command. This patch also includes
propagation of flags into the virStoragePoolDefParse() function.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
They require the caller to provide the maximum number
of array elements upfront, leading to either incomplete
results or violations of the zero-one-infinity rule.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Q35 machine types 2.3 and older had an integrated floppy controller.
Support for these machine types was removed by QEMU commit
commit 86165b499edf8b03bb2d0e926d116c2f12a95bfe
q35: Remove old machine versions
git describe: v2.5.0-1530-g86165b499e contains: v2.6.0-rc0~76^2~4
In libvirt, we have bumped the minimum QEMU version to 2.11:
commit b4cbdbe90b
qemu: Formally deprecate support for qemu < 2.11
git describe: v7.3.0-13-gb4cbdbe90b contains: v7.4.0-rc1~300
Since this QEMU version only supports Q35 machine versions 2.4+,
remove the code dealing with older ones.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Simon Rowe <simon.rowe@nutanix.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This patch also includes propagation of flags into the
virNetworkDefParse().
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
I have added new driver functions which define network with given
flags. I have also replaced definitions of the functions without
flags with function calls to the new ones.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
I need to propagate flags for the next commit.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This new API allows to define network with given flags.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Where easily possible, declare variables with g_auto to reduce
the amount of calls in cleanup sections.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
New clang has a false-positive about value of 'olddisks' being unused
after being set. This is clearly wrong because we want to use
'g_autofree' to clear it later.
While I'm against modifying good code for the sake of bad static
analysis in this case it's not obvious that we depend on the lifetime of
'olddisks' being needed until the end of the function as we store
pointers into it into the hash table and later copy them out.
Rewrite the code by assigning to 'olddisks' earlier and then using
'olddisks' in the loop, so it's clear where the lifetime of the objects
ends, and this should also silence the warning.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virNetworkDef was not freed if the function failed in the first
two ifs, causing a possible memory leak.
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>
When using 'virsh freepages' or 'virsh allocpages' then
virHostMemGetFreePages() or virHostMemAllocPages() is called,
respectively. But the following may happen: libvirt was built
without numactl support and thus a fake NUMA node was constructed
for capabilities, which means that startCell is going to be 0.
But we can't blindly pass startCell = 0 to virNumaGetPageInfo()
nor virNumaSetPagePoolSize() because they would operate over node
specific path (/sys/devices/system/node/nodeX) rather than NUMA
agnostic path (/sys/kernel/mm/hugepages/) and we are not
guaranteed that the former exists (kernel might have been built
without NUMA support).
Resolves:https://bugzilla.redhat.com/show_bug.cgi?id=1978574
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In all three cases (LXC, QEMU and VBox drivers) the caller has
access to host capabilities and thus know the maximum NUMA node.
This means, that virHostMemAllocPages() doesn't have to query
it. Querying may fail if libvirt was compiled without numactl
support.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In all three cases (LXC, QEMU and VBox drivers) the caller has
access to host capabilities and thus know the maximum NUMA node.
This means, that virHostMemGetFreePages() doesn't have to query
it. Querying may fail if libvirt was compiled without numactl
support.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This is just a small helper that will be used later.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
A logic bug in the code creating overlays on existing images resulted
into wrongly using "luks" instead of "qcow2" for the backing format if
the backing image is an luks-encrypted qcow2. The special format munging
is needed only for raw luks images.
In practice the impact is not as critical as to use encrypted images in
the backing chain the user must fully describe the backing chain
including backing images to provide encryption keys, which overrides the
metadata recorded in the qcow2 header.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need to validate the XML against schema if option '--validate'
was passed to the virsh command. This patch also includes
propagation of flags into the virSecretDefParse() function.
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>
We need to validate the XML against schema if option 'validate'
was passed to the 'iface-define' virsh command. For that we need
to allow validation flag and propagate flags to parse function.
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>
We need to know if validation flag is present in order to
validate given XML against schema in virXMLParse().
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>
This patch also includes propagation of flags into the
virNWFilterDefParse().
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
I have added a new driver function which allows to define
nwfilter with given flags. I have also replaced definition of
nwfilterDefineXML() with function call to the new function.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This new API function allows to define nwfilter with given flags.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When setting O_CLOEXEC flag on received FD fails the FD is closed
using VIR_FORCE_CLOSE(). But the call is wrapped in errno save
which is not necessary because VIR_FORCE_CLOSE() preserves errno
value.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use %s to print NULLSTR(duri).
Reported-by: Peng Liang <liangpeng10@huawei.com>
Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Warn these error instead of return when removing qos or queues. This will
avoid residual qos clearance on multiple interfaces.
Signed-off-by: zhangjl02 <zhangjl02@inspur.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Separate virNetDevOpenvswitchInterfaceClearQos into two steps. When setting
qos, we can set only rx or tx and the other one should be cleared.
Signed-off-by: zhangjl02 <zhangjl02@inspur.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add vmuuid notes on virNetDevOpenvswitchInterfaceSetQos,
and change vmid to vmuuid.
Signed-off-by: Jinsheng Zhang <zhangjl02@inspur.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
All supported QEMU versions have this option so there's no need for us
to base it on the capability.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All QEMU versions we support have these and it's very unlikely that they
will be removed. Remove the capability checks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'set-numa-node' is the command which can set the equivalent parameters
to '-numa' in preconfig mode, so we can use it as witness to see that
-numa is supported.
To ensure that the old detection method is removed once we'll be bumping
qemu support add a comment with the appropriate version check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All supported QEMU versions have all the fields so we can remove the
booleans controlling which fields are used on the monitor.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use qemuMonitorJSONCheckError instead of handcrafted error reporting.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Switch to automatic memory freeing and remove the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
They are no longer used as we now assume that all tuning caps are
present and in case some will be removed we'll need to use different
probing methods.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All currently supported qemu versions support all throttling
capabilities. It is unlikely that any of the fields will be removed in
the future and if it will we will need to do specific probing which is
possible via the 'throttle' object which is the replacement for the
legacy way to configure throttling.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The configurability of the number of dies in a CPU can be inferred from
the presence of the 'die-id' field in 'query-hotpluggable-cpus'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Probing QEMU_CAPS_DRIVE_DISCARD and QEMU_CAPS_DRIVE_DETECT_ZEROES can be
replaced by looking into the QMP schema rather than looking at -drive
which isn't in use any more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make it more obvious that we care about passing FDs on the commandline
before startup of qemu, which is used to avoid startup monitor polling.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU_CAPS_CHARDEV_RECONNECT, QEMU_CAPS_CHARDEV_LOGFILE and
QEMU_CAPS_CHARDEV_FILE_APPEND can be probed from the appropriate fields
in 'chardev-add' probed via the QMP schema instead of the command line
parameters.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a comment that will attempt to discourage adding new capabilities
based on 'query-command-line-options'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a cross reference of the enum value name with the string
representation. This allows a quick cross-reference of the values
without having to open the header and implementation files separately.
To achieve this the checker code at first obtains a list of the
flags and cross-references them when checking the grouping in
syntax-check, thus we are guaranteed to stay in sync.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Further commits will be refactoring and minimizing capabilities being
parsed from 'query-command-line-options'. Group the struct driving the
detection by argument name so it's easier to spot options belonging
together.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add g_autofree to functions changed in previous commits doing
g_auto cleanup for libxml2-related variables, where it could
lead to removal of a label.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Use g_auto where possible, reducing variable scope where applicable.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Move 'ctxt' and 'doc' inside the loop and mark them with g_auto.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Based on kernel commit messages the interface is
/sys/class/fc/fc_udev_device/appid_store
where we need to write the following string "$INODE:$APPID".
$INODE is the VM root cgroup inode in hexadecimal and $APPID is user
provided string that will be attached to each FC frame for the VM
within the cgroup identified by inode and has limit 128 bytes.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Prepare the function for additional sub-elements where all of the
sub-elements are optional.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
There is no need to error out for empty <partition></partition> element
as we can just simply ignore it. This allows to simplify the function
and prepare it for new sub-elements of <resource>.
It makes the <partition> element optional so we need to reflect the
change in schema as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
For new feature Fibre Channel VMID we will need to get inode of the
VM root cgroup as it is used in the new kernel API together with VMID.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
commit 2e668a61d5ae4("Fix error handling when adding MCS labels") uses
the 'pctx' in virReportError after it has been freed. Fix it.
Fixes: 2e668a61d5
Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Libvirt assumes that a SCSI bus can fit up to 8 devices
(including controller itself), except for so called wide bus
which can accommodate up to 16 devices (again, including
controller). This plays important role when computing 'drive'
address in virDomainDiskDefAssignAddress(). So far, the only
driver that enables wide SCSI bus is VMX. But with newer
releases, ESX is capable of "super wide" bus (64 devices).
We can blindly bump the limit in our code because then we would
compute address that's invalid for older ESX versions that we
still want to support.
Unfortunately, I haven't found a better place where to store this
than virDomainDef.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After previous patch it can no longer happen that @def will be
NULL and *def won't be.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The way we parse VMX configuration is rather unfortunate,
especially when it comes to disks. We allocate an array that can
handle all possible disks but leave the array counter (ndisks) at
zero and increase it only after successful parsing. But, we never
size the array down to release unneeded chunks of memory.
We can do better: we can use VIR_APPEND_ELEMENT() to allocate
array as needed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
At the beginning of vmx.c we have a comment that maps
virtualHW.version field onto ESX version. However, it wasn't
updated in a while. Fill it in using the following kbase article:
https://kb.vmware.com/s/article/1003746
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If the QEMU driver is configured to use the old "file" stdio
handler (meaning virtlogd is out of the picture) and a chardev
has a log file configured we rely on QEMU being able to create
the file itself. This may not be always possible (e.g. if the
logfile is set to a directory that QEMU process can't reach).
In such case we should create the file and just pass its FD to
QEMU.
We could do that unconditionally and just either pass FD from
virtlogd or the one we opened, because we bumped QEMU version
and are now requiring new enough QEMU. However, I'm keeping the
old style where logfile is appended on the cmd line for the tests
sake.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1989457
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
Again, we don't need full driver, just its config.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
The function doesn't really need domain object, but domain
definition from which it takes seclabels.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
The function doesn't write to domain definition really so make
@def argument as const. This allows us to call it from functions
where the domain definition is already const.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE exists since Xen 4.5.0
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
virXMLParse() now allows validating xml against schema directly,
eliminating the need to do it individually in each function.
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>
xmlDocPtr is no longer needed, because validation against schema
was moved to another function.
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>
virXMLParse() now allows to validate xml against schema directly,
eliminating the need to do it individually.
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>
We need this in order to validate XML against schema at one
place, rather than have the same code for validation in different
functions.
I will add '--validate' option to more virsh commands soon and
this makes it easier as virXMLParse() is called in every one I
plan to change.
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>
We didn't always save status xml after generating new taint message
which resulted in it being deleted in case of a libvirtd restart.
Some taint messages were preserved thanks to saving status xml
separately at the end of the calling functions. With this, every taint
message is saved, regardless of the calling functions.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1965589
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is the preferred way to do it, but there were a few
instances in which some of the path components had embedded
slashes instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Turn 'mounts' into a proper GStrv after sorting so that automatic
cleanup can be used and shuffle around the cleanup steps so that jumps
can be avoided in favor of direct return of error code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'devMountsPath' and 'devMountsSavePath' are NULL terminated.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'devMountsPath' can be converted to an auto-cleared stringlist and thus
asking for the number of entries is not necessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The only caller is passing a NULL terminated string list as
'devMountsPath' thus we don't need to get the count of elements.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The value of 'next' is copied into 'item.file' so we can move the update
to the 'next' pointer earlier and move the VIR_APPEND_ELEMENT call to
where we figure out that we need to append the value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We always process the full list so there's no value in storing the count
separately.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We always process the full list so there's no value in storing the count
separately.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We always process the full list so there's no value in storing the count
separately.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Previously they were stored in two separate arrays. This way it's
obvious when referencing the same one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Both virDomainAuthorizedSSHKeysGet and virDomainGetMessages return a
NULL-terminated string-list, so we can use g_auto(GStrv) to clear the
used memory on failures.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the conversion from virPCIVirtualFunctionList which encapsulates
the list of virtual functions to two disjunct arrays.
This greatly simplifies the fetching of the parameters as well as
cleanup in the caller.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'virNetDevGetVirtualFunctions' calls 'virPCIGetVirtualFunctions' and
then re-iterates the returned list to fetch the interface names for the
returned virtual functions.
If we move the fetching of the interface name into
virPCIGetVirtualFunctions we can simplify the code and remove a bunch of
impossible error states.
To accomplish this the function is renamed to
'virPCIGetVirtualFunctionsFull' while keeping a wrapper with original
name and if the physical port ID is passed the interface name is fetched
too without the need to re-convert the address into a sysfs link.
For now 'virNetDevGetVirtualFunctions' still converts the returned data
into two lists.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce a struct for holding the list of VFs returned by
virPCIGetVirtualFunctions so that we can employ automatic memory
clearing and also allow querying more information at once.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since UUID is not guaranteed to be unique by mdevctl, we may have more
than one nodedev with the same UUID. Therefore, we need to disambiguate
when looking up mdevs by specifying the UUID and parent address, which
mdevctl guarantees to be a unique combination.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Unfortunately, mdevctl supports defining more than one mdev with the
same UUID as long as they have different parent devices. (Only one of
these devices can be active at any given time).
This means that we can't use the UUID alone as a way to uniquely
identify mdev node devices. Append the parent address to ensure
uniqueness. For example:
Before: mdev_88a6b868_46bd_4015_8e5b_26107f82da38
After: mdev_88a6b868_46bd_4015_8e5b_26107f82da38_0000_00_02_0
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1979440
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This can be used similarly to other postparse callbacks in libvirt --
filling in additional information that can be determined by using the
information provided in the XML. In this case, we determine the address
of the parent device and cache it in the mdev caps so that we can use it
for generating a unique name and interacting with mdevctl.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
At the moment, this is only for mediated devices. When a new mediated
device is created or defined, the xml is expected specify the nodedev
name of an existing device as its parent. We were not previously
validating this and were simply accepting any string here.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
mdevctl can report multiple defined devices with the same UUID
but different parents, including parents that don't actually exist on
the host machine. Libvirt sets the parent to the 'computer' device for
all of the mdevs that have nonexistent parents. Because of this, it's
possible that there are multiple devices with the same UUID and the same
'computer' device as their parent, so the combination of uuid and parent
nodedev name is not guaranteed to be a unique name.
We need to ensure that each nodedev has a unique name. If we can't use
the UUID as a unique nodedev name, and we can't use the combination of
UUID and nodedev parent name, we need to find another solution. By
caching and using the parent name reported by mdevctl in combination
with the UUID, we can achieve a unique name. mdevctl guarantees that its
uuid/parent combination is unique.
This value will be used to set the mdev nodedev name in a following commit.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 51fbbfdce8 attempted to get the proper nodedev name for the
parent of an defined mdev by traversing the filesystem and looking for a
device that had the appropriate sysfs path. This works, but it would be
cleaner to to avoid mucking around in the filesystem and instead just
just examine the list of devices we have in memory.
We already had a function nodeDeviceFindAddressByName() which constructs
an address for parent device in a format that can be used with mdevctl.
So if we refactor this function into a a function that simply formats an
address for an arbitrary virNodeDeviceObj*, then we can use this
function as a predicate for our new virNodeDeviceObjListFind() function
from the previous commit. This will search our list of devices for one
whose address matches the address we get from mdevctl.
One nice benefit of this approach is that our test cases will now
display xml output with the proper parent name for mdevs (assuming that
we've added the appropriate mock parent devices to the test driver).
Previously they just displayed 'computer' for the parent because the
alternative would have required specially constructing a mock filesystem
environment with a sysfs that mapped to the appropriate parent.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is a generic function that you can provide your own predicate
function to search for a particular device. It will be used in an
upcoming commit.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We currently query the host MSRs to determine if TSC scaling is
supported. This works OK when running privileged and can open
the /dev/cpu/0/msr. When unprivileged we fallback to querying
MSRs from /dev/kvm. This is incorrect because /dev/kvm only
reports accurate info for MSRs that are valid to use from inside
a guest. The TSC scaling support MSR is not, thus we always end
up reporting lack of TSC scaling when unprivileged.
The solution to this is easy, because KVM can directly report
whether TSC scaling is available, which matches what QEMU will
do at startup.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/188
Reported-by: Roman Mohr <rmohr@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This reverts commit 05bd8db60b.
It is true that the remote driver client now contains logic for probing
the driver to connect to when using modular daemons. This logic, however,
only runs when the remote driver is NOT running inside a daemon since we
don't want it activated inside libvirtd. Since the same remote driver
build is used in all daemons, we can't rely on it in virtproxyd either.
Thus we need to keep the virtproxyd probing logic
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The function was inconsistently using 'return -1' and 'goto cleanup;'
unify it by removing the cleanup label and 'ret' variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It's used only inside the loop filling the extents, move it there and
restructure the code so that 'extent.path' doesn't have to be freed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'list' will always be NULL when reaching 'virObjectListFreeCount' thus
we can remove the call as well as the 'ret' variable which was only ever
equal to 'nclients' at the point when we returned the value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'bootHotplug' can be auto-freed when terminating the function and moving
the declaration of 'vcpuprops' to the loop which uses it along with
automatic freeing allows us to simplify cleanup in certain cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory freeing for 'tmpvars' and move the allocation of
tmpvars earlier so that we are guaranteed that 'obj' will always be
appended to 'inst->filters' and thus don't need cleanup for it.
By moving the reset of 'inst' to the block when virNWFilterDefToInst
fails we can get rid of the rest of the cleanup section and remove the
'ret' variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Construct the 'ruleinst->vars' hash table separately in a temporary
variable so that 'ruleinst' can be allocated on success. This allows us
to get rid of the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'cb' is always NULL when 'virObjectEventCallbackListAddID' is called.
Remove the call.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'item' is always NULLed-out by VIR_APPEND_ELEMENT and 'ret' variable is
always 0 when used so both can be removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'hub' doesn't need to be freed any more because it's always consumed and
NULLed-out by VIR_APPEND element. This also makes the 'ret' variable
obsolete.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'video' will only ever be NULL after the 'cleanup' label thus there's no
need to use 'virDomainVideoDefFree'. In fact we can fully remove the
cleanup section and 'ret' variable by returning directly from failure
points.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
VIR_APPEND_ELEMENT doesn't report any errors now so we can remove
VIR_APPEND_ELEMENT_QUIET and replace all uses by VIR_APPEND_ELEMENT
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use virAppendElement instead of virInsertElementsN to implement
VIR_APPEND_ELEMENT which allows us to remove error handling as the
only relevant errors were removed when switching to aborting memory
allocation functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For now it was an alias to VIR_APPEND_ELEMENT. Use virAppendElement
directly until VIR_APPEND_ELEMENT is refactored too and we'll be able to
get rid of VIR_APPEND_ELEMENT_QUIET completely.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use virAppendElement instead of virInsertElementsN to implement
VIR_APPEND_ELEMENT_COPY which allows us to remove error handling as the
only relevant errors were removed when switching to aborting memory
allocation functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
VIR_APPEND_ELEMENT_INPLACE and VIR_APPEND_ELEMENT_COPY_INPLACE already
ignore the return value from 'virInsertElementsN' which allows a trivial
conversion to virAppendElement without the need for 'ignore_value'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The new wrapper calls virInsertElementInternal with the appropriate
arguments without any checks which are unnecessary for appension. This
allows to have no return value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Split out the code doing the movement of the elements and insertion from
the range checks. This will help in adding an error-free version for
appension.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The idea of @add was that the insersion/appension macros would allow
adding more than one element but this feature was never implemented.
'add' is nowadays used as a dummy variable consuming the result of the
VIR_TYPEMATCH compile time check.
Make it obvious that we don't use 'add' by renaming it to
'typematchDummy', marking it as unused and replacing all occurences
where the value was used by literal '1'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Send TERM/KILL to virtiofsd and its child processes too
and do not exit until they are all dead.
https://bugzilla.redhat.com/show_bug.cgi?id=1940276
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Add a version of virPidFileForceCleanupPath that takes
a 'group' bool argument and propagate it all the way
down to virProcessKillPainfullyDelay.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Starting with QEMU 6.0, this controller is enabled by default
on aarch64.
https://bugzilla.redhat.com/show_bug.cgi?id=1967187
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When we try to migrate vm, we check if it contains only devices
that are able to migrate. If a hostdev device is not able to
migrate we raise an error with <hostdev/>, but it can actually be
<interface/>, so we need to check if hostdev device was created
by us from interface and show the right error message.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1942315
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In order to test the virDomainGetMessages for test driver, we need to
check some taints or deprecations, so introduce testDomainObjCheckTaint
for checking taints.
As we introduced testDomainObjCheckTaint for test driver, the `dominfo`
command in virshtest will now print tainting messages, so add them for
test.
Signed-off-by: Luke Yue <lukedyue@gmail.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The test driver and qemu driver could share the same code in
virDomainGetMessages(), so extract it to a function.
Signed-off-by: Luke Yue <lukedyue@gmail.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
It was added by commit c2121602 and later removed by 5a4c2374a
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When loop in function virNVMeDeviceListCreateReAttachList() there may be
reused index @i, this patch fix this by using a new @j.
Signed-off-by: Jia Zhou <zhou.jia2@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We need to pass the 'trim' requests through the copy-on-read filter so
if a user configures a discard policy on the disk the requests get
through to the appropriate format layer in the blockdev tree.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1986509
Reported-by: Richard W.M. Jones <rjones@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
This function add halt polling time interface in domstats. So that
we can use command 'virsh domstats VM' to get the data if system
support.
Signed-off-by: Yang Fei <yangfei85@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add helper function virHostCPUGetHaltPollTime to obtain halt polling
time. If the kernel support halt polling time statistic, and mount
debugfs. This function will take effect on KVM VMs.
Signed-off-by: Yang Fei <yangfei85@huawei.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use function virFileReadValueUllongQuiet to read unsigned long
long value without error report.
Signed-off-by: Yang Fei <yangfei85@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Base it on the presence of the "blockdev-reopen" QMP command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Export 'qemuBlockReopenFormatMon' and use it in a new test case wich
will validate the arguments against the QMP schema.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This function was added prior 'blockdev-reopen' being stable and qemu
changed the arguments to actually contain an array of block node
definitions to reopen.
In our case we are just changing between read-only and read-write modes
and thus we can keep operating on the nodes one-by-one.
Modify the code to add the wrapper array.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This will simplify testing of the blockdev-reopen code once it's
enabled.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Similar to what was done for qemu_firmware.c in 61d95a1073, don't
report an error for unknown vhost-user features, just log it and
correctly continue on
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Additional cleanup paths add the possibility of not freeing earlier
stuff. Add an AUTOPTR handler for qemuDomainObjPrivate and use it in
qemuDomainObjPrivateAlloc
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
The freeing function will be needed to undo failures in allocation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
'obj->classIdMap' is a bitmap with size of '16', thus the first 3 bits
are guaranteed to be available. Use 'virBitmapSetBit' instead of
'virBitmapSetBitExpand' since we don't need any expansion and ignore
errors as they are impossible.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Use a temporary auto-freed local variable to hold the hash table so that
the cleanup section can be removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Allocate the hash tables first so tat the 'data' struct can be directly
initialized removing the need for a memset and two additional
assignments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Declare the function with G_GNUC_WARN_UNUSED_RESULT as we always want to
use the returned value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
In one of my previous patches I've tried to postpone dropping
CAP_SETPCAP until the very end because it's needed for
capng_apply(). What I did not realize back then was that we might
not have the capability to begin with. Because of unknown reasons
capng_apply() pollutes logs only for CAPNG_SELECT_BOUNDS and not
for CAPNG_SELECT_CAPS.
Reproducer is really simple: run libvirtd as a regular user.
During its initialization, libvirtd will spawn some binaries
(dnsmasq, qemu-*, etc.) and while doing so it will try to drop
capabilities.
Anyway, let's call capng_apply(CAPNG_SELECT_BOUNDS) only if we
have the CAP_SETPCAP (which is tracked in need_setpcap variable).
Fixes: 438b50dda8
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1924218
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
After all capabilities were set (except for CAP_SETGID,
CAP_SETUID and CAP_SETPCAP) and after UID:GID was changed we drop
the last aforementioned capabilities (we couldn't drop them
before because we needed UID:GID and capabilities change).
Therefore, there's final capng_apply() call. However, it is
wrapped in one layer of parenthesis more than needed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Libvirt is using the G_GNUC_FALLTHROUGH macro provided by glib since
version 2.60. Since we need to support older glib, we also have some
compatibility code to define it if missing.
We set the GLIB_VERSION_MAX_ALLOWED macro to ensure we get warnings
when we use an API that dates from a glib version newer than our
minimum benchmark. Historically this didn't get enforced for (most)
macros, but GLib 2.69 has addressed that gap.
This causes our usage of G_GNUC_FALLTHROUGH to trigger warnings.
GLib is right to warn, because it does not know that we have added
our own fallback for older versions.
The only way to squelch this warning though, is to fully undefine
the GLib provided G_GNUC_FALLTHROUGH and use our own in its place.
We'll be able to remove all this compat burden when we finally
update the min glib version to be >= 2.60
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
If lvcreate found an existing signature when trying to create a
new logical volume (E.g. left after some deleted volume), the
action failed due to inability to answer interactive question to
wiping it (lvcreate assumed 'no' was the answer). With added
option --yes to the command line, the answer to any interactive
question is assumed to be yes. Therefore, lvcreate wipes the
signature and the new volume is created successfully.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1940413
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
`cur` is guaranteed to be of type `XML_ELEMENT_NODE` by using
`xmlFirstElementChild()` and `xmlNextElementSibling()`.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
"xmlNextElementSibling()" skips attribute nodes, making the explicit
check for the type of `cur` redundant. This prepares for the removal
of this check in the next commit.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add support for customizable grabToggle key combinations with
<input type='evdev'>.
Signed-off-by: Justin Gatzen <justin.gatzen@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The submission of the event to the helper thread has a verbose cleanup
path which was duplicated in all the event handlers. Simplify it by
extracting the code into a helper named 'qemuProcessEventSubmit' and
reuse it where appropriate.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
The removed error messages are impossible as the enum values are
converted via VIR_ENUM helpers and guarded by compiler checks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
It is also impossible for @info to be non-NULL in the cleanup section so
the cleanup can be completely removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the callback prototype and fix the callback registered in the
process code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The callers in the monitor code invoking the callbacks after events are
received don't actually check the return value from the callbacks and
there isn't really anything we could do on failure.
Remove the return value from the intermediary functions so we can later
remove them from the callback prototypes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemu process code doesn't register a callback for it so we don't
need to be handling it at all.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'virStoragePoolObjListSearch' returns a locked and refed object, thus we
must release it on ACL permission failure.
Fixes: 7aa0e8c0cb
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1984318
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit e9b534905f introduced an error when parsing an empty list
returned from mdevctl.
This occurs e.g. if nodedev-undefine is used to undefine the last
defined mdev which causes the following error messages
libvirtd[33143]: internal error: Unexpected format for mdevctl response
libvirtd[33143]: internal error: failed to query mdevs from mdevctl:
libvirtd[33143]: mdevctl failed to updated mediated devices
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
If the function is called with maxlen equal to `INT_MAX`, adding
one will trigger a signed integer overflow.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Since commit d399a728f4 placed the restore in the right scope the
restore can get removed in virDomainSEVDefParseXML.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We just found <qemu:commandline> is ignored in our xml. Further debug
shows that ctxt's node pointer isn't restored in virDomainSecDefParseXML(),
which leads to parsing of remaining elements failed.
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When SEV is not supported but specified in the domain XML by a user it
should not result in an internal error (VIR_ERR_INTERNAL_ERROR)
therefore switching to XML error (VIR_ERR_CONFIG_UNSUPPORTED).
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Use the common id 'lsec0' for all launchSecurity types in the QEMU
command line construction.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Adding availability of s390-pv in domain capabilities and adjust tests.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Add launch security type 's390-pv' as well as some tests.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Adding virDomainSecDef for general launch security data
and moving virDomainSEVDef as an element for SEV data.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Make use of virDomainLaunchSecurity enum.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When doing a peer-to-peer migration it may happen that the
connection to the destination disappears. If that happens,
there's no point in trying to unregister the close callback
because the connection is closed already. It results only in
polluting logs with this message:
error : virNetSocketReadWire:1814 : End of file while reading data: : Input/output error
and the reason for that is unregistering a connection callback
results in RPC (among other things).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1918211
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This appears to be a copy-paste mistake from the check directly above.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuMigrationSrcRunPrepareBlockDirtyBitmaps receives the flags parameter
from qemuMigrationSrcRun, where flags are based on the main API enum
values. Similar to commit f58349c9c6, use the main API enum instead of
internal driver enum when checking flags in
qemuMigrationSrcRunPrepareBlockDirtyBitmaps.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signaling the condition before vm->def->id is reset to -1 is dangerous:
in case a waiting thread wakes up, it does not see anything interesting
(the domain is still marked as running) and just enters virDomainObjWait
where it waits forever because the condition will never be signalled
again.
Originally it was impossible to get into such situation because the vm
object was locked all the time between signaling the condition and
resetting vm->def->id, but after commit 860a999802 released in 6.8.0,
qemuDomainObjStopWorker called in qemuProcessStop between
virDomainObjBroadcast and setting vm->def->id to -1 unlocks the vm
object giving other threads a chance to wake up and possibly hang.
In real world, this can be easily reproduced by killing, destroying, or
just shutting down (from the guest OS) a domain while it is being
migrated somewhere else. The migration job would never finish.
So let's make sure we delay signaling the domain condition to the point
when a woken up thread can detect the domain is not active anymore.
https://bugzilla.redhat.com/show_bug.cgi?id=1949869
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
On libvirtd startup, the list of priority worker threads is uninitialized
(`pool->prioWorkers` is NULL), and then "expanded" to zero (`prioWorkers`)
entries.
This causes `virThreadPoolExpand` to call `VIR_EXPAND_N` on a null pointer
and an increment of zero. The zero increment triggers `virReallocN` to not
actually allocate any memory and leave the pointer NULL, which, eventually,
causes `memset(NULL, 0, 0)` to be called in `virExpandN`.
`memset` is declared `__attribute__ ((__nonnull__ 1))`, which triggers the
following warning when libvirt is compiled with address sanitizing enabled:
$ meson -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address,undefined
build && ninja -C build
$ ./build/run build/src/libvirtd
src/util/viralloc.c:82:5: runtime error: null pointer passed as
argument 1, which is declared to never be null
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This simplyfies the code a bit and removes one "goto", one "VIR_FREE",
and one "VIR_INSERT_ELEMENT_COPY".
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
As test driver won't have real background job running, in order to get
all possible states, the time is used here to decide which state to be
returned. The default time will get `ok` as return value.
Note that using `virsh domtime fc4 200` won't take effect for the test
driver, to get other states, you have to enter virsh interactive
terminal and set time.
Signed-off-by: Luke Yue <lukedyue@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If the queried QMP command doesn't exist qemuMonitorGetTPMModels returns
0 but sets the string list to NULL which isn't accepted by
g_strv_contains.
Fixes: a5bc5f0ecf
Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Inactive mdevs were simply formatting their parent name as the value
received from mdevctl rather than looking up the libvirt nodedev name of
the parent device. This resulted in a parent value of e.g.
'0000:5b:00.0' instead of 'pci_0000_5b_00_0'. This prevented defining a
new mdev device from the output of nodedev-dumpxml.
Unfortunately, it's not simple to fix this comprehensively due to the
fact that mdevctl supports defining (inactive) mdevs for parent devices
that do not actually exist on the host (yet). So for those persistent
mdev definitions that do not have a valid parent in the device list, the
parent device will be set to the root "computer" device.
Unfortunately, because the value of the 'parent' field now depends on
the configuration of the host, the mdevctl parsing test will output
'computer' for all test devices. Fixing this would require a more
extensive mock test environment.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1979761
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Having multiple addresses having same hostname is a common config either
to have IPv4 and IPv6 address for the same hostname or even for DNS
round robin. The validation in the network update code didn't allow
adding such entries despite the fact that it is possible to define a
network with them.
Don't check hostname duplicity when adding a DNS entry.
The update of the test case adds another entry for the 'pudding'
hostname which is added in one of the networkxml2xmlupdate test cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We need to report via domcapabilities if specifying shared memory
is supported without hugepages or numa config in order to find
out if domain has suitable setup to make virtiofs work.
The solution is to report source types of memory backing to
determine if memfd is a valid option.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This was bothering someone as the debug message looked like there was an issue
despite it being just a debug message. Change it to what is actually happening
and why the name is being skipped.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
If the attempt to attach a device failed, we erased the
unattached device from the namespace. This resulted in erasing an
already attached device in case of a duplicate. We need to check
for existing file in the namespace in order to determine erasing
it in case of a failure.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1780508
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
A new apparmor profile initially derived from the libvirtd profile.
All rules were prefixed with the 'audit' qualifier to verify they
are actually used by virtxend. It turns out that several, beyond
the obvious ones, can be dropped in the resulting virtxend profile.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
A new apparmor profile derived from the libvirtd profile, with non-QEMU
related rules removed. Adopt the libvirt-qemu abstraction to work with
the new profile.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
This is a followup for commit e906c4d02b
("apparmor: Allow /usr/libexec for libxl-save-helper and pygrub"):
In recent rpm versions --libexecdir changed from /usr/lib64 to
/usr/libexec. A plain rpmbuild %configure in xen.git will install all
files, including the private copies of qemu, into /usr/libexec/xen/bin.
Expand the existing pattern to cover also this libexecdir variant.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We have an example in virDirRead() documentation on how to use
the function. In there, the directory structure is plain DIR, but
that won't work anymore. Switch over to g_autoptr(DIR) which is
what we use now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Allow swtpm (0.7.0 or later) to fsync on the directory where it writes
its state files into so that "the entry in the directory containing the
file has also reached disk" (fsync(2)).
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
`virHashNew` cannot return NULL, the check is not needed.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The same pattern of retrieving the domXML, running the hook script, and
checking for error is used throughout the libxl driver. Remove some
repetitive code by adding a helper function to perform these tasks.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce libxlDomainStartPerform as part of decomposing libxlDomainStart.
Perform all operations that are part of starting a domain. On error the
domain is destroyed from libxl's perspective, but the operations perfomed
in libxlDomainStartPrepare must be unwound by libxlDomainStart.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce libxlDomainStartPrepare as part of decomposing libxlDomainStart.
Perform all prepratory operations such as hostdevs, network devs, etc.
Also ensure all such operations are properly unwound on error.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move network device cleanup code from libxlDomainCleanup to a helper
function for use in a subsequent patch.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
the logic to check for existence of a managed save image and use it to
start the VM can be moved to libxlDomainStartNew. libxlDomainStart has
become unwieldy and this is a small step to make it more manageable.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'storageMigration' flag is supposed to be true if storage migration
is requested, which is based on VIR_MIGRATE_NON_SHARED_DISK or
VIR_MIGRATE_NON_SHARED_INC flags. The assignment to the variable used
QEMU_MONITOR_MIGRATE_NON_SHARED_INC (0x04) instead of
VIR_MIGRATE_NON_SHARED_INC (0x80), caused libvirtd to skip the actual
copy of data.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1978526
Fixes: da69f4b208
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Libvirt started emitting two threshold events, once with index and once
withouth when the index isn't registered. Document this caveat.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Remember whether the user passed an explicit index when registering the
event so that we can avoid the top level event when it isn't needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When users register the threshold event for the top level image with an
explicit index (e.g. vda[3]) they are clearly expecting the index in the
event.
This flag will help avoiding emission of the second event without the
index when the client clearly requested one with the index.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When qos is set or delete, we have to check if the port is an ovs managed
port. If true, call the virNetDevOpenvswitchInterfaceSetQos function when qos
is set, and call the virNetDevOpenvswitchInterfaceClearQos function when
the interface is to be destroyed.
Signed-off-by: Jinsheng Zhang <zhangjl02@inspur.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Return 0 directly if the port is ovs managed. When the ovs port is set
noqueue, qos config on this port will not work.
Signed-off-by: Jinsheng Zhang <zhangjl02@inspur.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce qos setting and cleaning method. Use ovs command to set qos
parameters on specific interface of qemu virtual machine.
When an ovs port is created, we add 'ifname' to external-ids. When setting
qos on an ovs port, query its qos and queue. If found, change qos on queried
queue and qos, otherwise create new queue and qos. When cleaning qos, query
and clean queues and qos in ovs table record by 'ifname' and 'vmid'.
Signed-off-by: Jinsheng Zhang <zhangjl02@inspur.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tell whether a port definition is an ovs managed virtual port
Signed-off-by: Jinsheng Zhang <zhangjl02@inspur.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When seeing a guest with a sound device, and no audio backend, we
automatically add an audio backend XML element based on the historical
QEMU driver behaviour. Unfortunately when we live migrate back to an
old libvirt, it may not understand the audio driver type we configured.
We thus need to strip the default audio backend when migrating.
Fixes https://gitlab.com/libvirt/libvirt/-/issues/179
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There might be misunderstanding [1] when libvirt permits domain
redefinition and if it's a valid case at all.
1. b973d7c4b4/plugins/modules/virt.py (L533)
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It all started as a simple bug: trying to move domain memory
between NUMA nodes (e.g. via virsh numatune) did not work. I've
traced the problem to qemuProcessHook() because that's where we
decide whether to rely on CGroups or use numactl APIs to satisfy
<numatune/>. The problem was that virCgroupControllerAvailable()
was telling us that cpuset controller is unavailable. This is
CGroupsV2, and pretty weird because CGroupsV2 definitely do
support cpuset controller and I had them mounted in a standard
way. What I found out (with Pavel's help) was that
virCgroupNewSelf() was looking into the following path to detect
supported controllers:
/sys/fs/cgroup/system.slice/cgroup.controllers
However, if there's no other VM running then the system.slice
only has 'memory' and 'pids' controllers. Therefore, we saw
'cpuset' as not available. The fix is to look at the top most
path, which has the full set of controllers:
/sys/fs/cgroup/cgroup.controllers
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1976690
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When the qemu or libxl driver is configured to use lockd and
file_lockspace_dir is set, virtlockd emits an error when libvirtd
is retarted
May 25 15:44:31 virt81 virtlockd[7723]: Requested operation is not
valid: Lockspace for path /data/libvirtd/lockspace already exists
There is really no need to fail when the lockspace already exists,
paricularly since the user is expected to create the lockspace
specified in file_lockspace_dir. Failure to do so will prevent
starting any domains
virsh start test
error: Failed to start domain 'test'
error: Unable to open/create resource /data/libvirtd/lockspace/de22c4bf931e7c48b49e8ca64b477d44e78a51543e534df488b05ccd08ec5caa: No such file or directory
Also, virLockManagerLockDaemonSetupLockspace already has logic to ignore
the error. Since callers are not interested in the error, change
virtlockd to not report or return an error when the specified lockspace
already exists.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
If guest is configured to use memfd then the function that build
memory-backend-* part of command line will put
memory-backend-memfd, always. Even for NVDIMMs. This is not
correct, because NVDIMMs need a backing path (usually to a real
host NVDIMM device). Therefore, regardless of memfd being
requested, we have to stick with memory-backend-file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
When constructing guest name for machined we have to be very
cautious as machined expects a name that's basically a valid URI.
Therefore, if there's a dot it has to be followed by a letter or
a number. And if there's a sequence of two or more dashes they
should be joined into a single dash. These rules are implemented
in virDomainMachineNameAppendValid(). There's the @skip variable
which is supposed to track whether it is safe to append a dot or
a dash into name. However, the variable is set to false (meaning
it is safe to append a dot or a dash) even if the current
character we are processing is not in the set of allowed
characters (and thus skipped over).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1948433
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
In 'virResctrlAllocUpdateMask', mask is updated only if 'previous mask' is NULL.
By default, the bitmask for a cache resource for a VM is initialized with
'default-resctrl-group' bitmask. So the 'previous mask' would not be NULL and
mask won't get updated if cachetune is configured for a VM. This causes libvirt
to use same bitmask as 'default-resctrl-group' bitmask for a cache resource for
a VM. This patch fixes the issue.
Fixes: d8a354954a
Signed-off-by: Vinayak Kale <vkale@nvidia.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Bounding set capabilities were introduced in kernel commit of
v2.6.25-rc1~912. I guess it is safe to assume that all Linux
hosts we ran on have at least that version or newer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When trying to destroy a node device that is not active, we end up with
a confusing error message:
# nodedev-destroy mdev_88a6b868_46bd_4015_8e5b_26107f82da38
error: Failed to destroy node device 'mdev_88a6b868_46bd_4015_8e5b_26107f82da38'
error: failed to access '/sys/bus/mdev/devices/88a6b868-46bd-4015-8e5b-26107f82da38/iommu_group': No such file or directory
With this patch, the error is more clear:
# nodedev-destroy mdev_88a6b868_46bd_4015_8e5b_26107f82da38
error: Failed to destroy node device 'mdev_88a6b868_46bd_4015_8e5b_26107f82da38'
error: Requested operation is not valid: Device 'mdev_88a6b868_46bd_4015_8e5b_26107f82da38' is not active
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Currently, we have three different types of mdevctl errors:
1. the command cannot be constructed ecause of unsatisfied
preconditions
2. the command cannot be executed due to some error
3. the command is executed, but returns an error status
These different failures are handled differently. Some cases set an
error and return and error status, and some return a error message but
do not set an error.
This means that the caller has to check both whether the return value is
negative and whether the errmsg parameter is non-NULL before deciding
whether to report the error or not. The situation is further complicated
by the fact that there are occasional instances where mdevctl exits with
an error status but does not print an error message. This results in
errmsg being an empty string "" (i.e. non-NULL).
Simplify the situation by ensuring that virReportError() is called for
all error conditions rather than returning an error message back to the
calling function.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
This macro will be utilized in the following patch. Since mdevctl
commands can fail with or without an error message, this macro makes it
easy to print a fallback error in the case that the error message is not
set.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
In commit 68580a51, I removed the checks for NULL cmd variables because
virCommandRun() already handles the case where it is called with a NULL
cmd. Unfortunately, it handles this case by raising a generic error
which is both unhelpful and overwrites our existing error message. So
for example, when I attempt to create a mediated device with an invalid
parent, I get the following output:
virsh # nodedev-create mdev-test.xml
error: Failed to create node device from mdev-test.xml
error: internal error: invalid use of command API
With this patch, I now get a useful error message again:
virsh # nodedev-create mdev-test.xml
error: Failed to create node device from mdev-test.xml
error: internal error: unable to find parent device 'pci_0000_00_03_0'
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
At the point where the error message is emitted, the field def->name is
still set to "new device", so the error message becomes:
Unable to start mediated device 'new device': ...
Since the name doesn't contain anything useful, just omit it from the
error message altogether.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Due to a rather unfortunate misunderstanding, we were parsing the list
of defined devices from mdevctl incorrectly. Since my primary
development machine only has a single device capable of mdevs, I
apparently neglected to test multiple parent devices and made some
assumptions based on reading the mdevctl code. These assumptions turned
out to be incorrect, so the parsing failed when devices from more than
one parent device were returned.
The details: mdevctl returns an array of objects representing the
defined devices. But instead of an array of multiple objects (with each
object representing a parent device), the array always contains only a
single object. That object has a separate property for each parent
device.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It is possible to define/edit(in shut off state) a domain XML with
same hostdev device repeated more than once, as shown below. This
behavior is not expected. So, this patch fixes it.
vser1:
<domain type='kvm'>
[...]
<devices>
[...]
<hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
<source>
<address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
</source>
<address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0005'/>
</hostdev>
<hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
<source>
<address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
</source>
<address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0006'/>
</hostdev>
[...]
</devices>
</domain>
$ virsh define vser1
Domain 'vser1' defined from vser1
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We already reject TPM 1.2 in a number of scenarios; let's add
ARM virt guests to the list.
https://bugzilla.redhat.com/show_bug.cgi?id=1970310
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Liu Yiding <liuyd.fnst@fujitsu.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The TPM 2.0 specification predates ARM virtualization, and so
implementing TPM 1.2 support on ARM was not considered a useful
endeavor.
This is technically a breaking change, but TPM support on ARM was
only introduced fairly recently (libvirt 7.1.0) and the previous
default resulted in non working TPM devices; anyone who has a
working configuration is not going to be affected.
https://bugzilla.redhat.com/show_bug.cgi?id=1970310
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Liu Yiding <liuyd.fnst@fujitsu.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
A process can access a file if the set of MCS categories
for the file is equal-to *or* a subset-of, the set of
MCS categories for the process.
If there are two VMs:
a) svirt_t:s0:c117
b) svirt_t:s0:c117,c720
Then VM (b) is able to access files labelled for VM (a).
IOW, we must discard case where the categories are equal
because that is a subset of many other valid category pairs.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/153
CVE-2021-3631
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There are few cases where we execute a virCommand with all caps
cleared (virCommandClearCaps()). For instance
dnsmasqCapsRefreshInternal() does just that. This means, that
after fork() and before exec() the virSetUIDGIDWithCaps() is
called. But since the caller did not want to change anything,
just drop capabilities, these are the values of arguments:
virSetUIDGIDWithCaps (uid=-1, gid=-1, groups=0x0, ngroups=0,
capBits=0, clearExistingCaps=true)
This means that indeed all capabilities will be dropped,
including CAP_SETPCAP. But this capability controls whether
capabilities can be set, IOW whether capng_apply() succeeds.
There are two calls of capng_apply() in the function. The
CAP_SETPCAP is dropped after the first call and thus the other
call (capng_apply(CAPNG_SELECT_BOUNDS);) fails.
The solution is to keep the capability for as long as needed
(just like CAP_SETGID and CAP_SETUID) and drop it only at the
very end (just like CAP_SETGID and CAP_SETUID).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1949388
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
I noticed the following denial when running confined VMs with the QEMU
driver
type=AVC msg=audit(1623865089.263:865): apparmor="DENIED" operation="open" \
profile="virt-aa-helper" name="/etc/ssl/openssl.cnf" pid=12503 \
comm="virt-aa-helper" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
Allow reading the file by including the openssl abstraction in the
virt-aa-helper profile.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
I noticed the following denial messages from apparmor in audit.log when
starting confined VMs via the QEMU driver
type=AVC msg=audit(1623864006.370:837): apparmor="DENIED" operation="open" \
profile="virt-aa-helper" name="/etc/libnl/classid" pid=11265 \
comm="virt-aa-helper" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
type=AVC msg=audit(1623864006.582:849): apparmor="DENIED" operation="open" \
profile="libvirt-0ca2720d-6cff-48bb-86c2-61ab9a79b6e9" \
name="/etc/libnl/classid" pid=11270 comm="qemu-system-x86" \
requested_mask="r" denied_mask="r" fsuid=107 ouid=0
It is possible for site admins to assign names to classids in this file,
which are then used by all libnl tools, possibly those used by libvirt.
To be on the safe side, allow read access to the file in the virt-aa-helper
profile and the libvirt-qemu abstraction.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
The host key fingerprint for SSH servers is used in a scenario where
cryptographic strength is important. We should thus be defaulting to
use of SHA256 where available. We only need SHA1 for Ubuntu 18.04
which does not have libssh >= 0.8.1
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Cleanup to follow. This removes the last re-use of `nodes` in this function,
eliminating two VIR_FREEs.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
`feature` is always one of the values listed in the switch,
ensured by `virDomainKVMTypeFromString` above.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be inlined and
simplified. This also removes the re-use of `nodes`, elimininating
two VIR_FREEs.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Vast majority of device types is not supported by the Cloud-Hypervisor
driver. Simplify the error reporting by using
virDomainDeviceTypeToString.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Now that the minimum supported Xen version has bumped to 4.9, all
uses of LIBXL_HAVE_* that are included in Xen 4.9 can be removed
from the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When removing check for return value of VIR_EXPAND_N this place was
incorrectly modified causing failure to start a VM with cputune
memorytune configured with useless error message:
error: Failed to start domain 'vm1'
error: An error occurred, but the cause is unknown
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1973094
Fixes: 7d2fd6ef01
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virISCSIDirectScanTargets now returns a GStrv, so we can use automatic
cleanup for it and get rid of the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Count the elements in advance rather than using VIR_APPEND_ELEMENT and
ensure that there's a NULL terminator for the string list so it's GStrv
compatible.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Using an allocated version together with copying the
host/initiator/device portions into it allows us to switch to automatic
clearing rather than open-coding it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of trying to match devices passed in based on the monitor
detecting the number of devices that were used in the domain
definition, use the deviceValidateCallback to evaluate if
unsupported devices are used.
This allows the compiler to detect when new device types are added
that need to be checked.
Signed-off-by: William Douglas <william.douglas@intel.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Originally qemuDomainAttachNetDevice() would wait until the cleanup at
the very end of the function to add newly hotplugged interfaces to the
domain's nets list. commit 7b8bec4560 modified it to add the new
interface to the nets list earlier (but not all the way at the
beginning of the function either, because there are some operations
(PCI address assignment in particular) that need the new device to not
yet be visible in the domaindef).
But hostdev interfaces short-circuit past most of the body of
qemuDomainAttachNetDevice() (since none of it applies to hostdev
interfaces). In the past that was okay, but since the line that adds
the new interface to the domaindef's nets list is in that "most of the
body", after that commit hotplugged hostdev interfaces are no longer
being properly added to the domaindef nets list, so they don't show up
in the status XML or the virsh domiflist output.
It really *is* important to add interfaces to the nets list earlier,
so we can't revert commit 7b8bec4560, and we also can't move the
insert to common code *earlier* in the function, so instead this patch
duplicates the VIR_APPEND_ELEMENT_COPY() just before the code path for
hostdev interfaces jumps to cleanup.
Resolves: https://bugzilla.redhat.com/1972468
Fixes: 7b8bec4560
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that the remote driver itself can probe for listening sockets /
running daemons, virtproxyd doesn't need to probe URIs itself. Instead
it can just delegate to the remote driver.
Tested-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
With the traditional libvirtd, the virConnectOpen call will probe active
drivers server side to find which one to use when the URI is NULL/empty.
With the modular daemons though, the remote client does not know which
daemon to connect in the first place, so we can't rely on virConnectOpen
probing. Currently the virtproxyd daemon has code to probe for a
possible driver by looking at which sockets are listening or which
binaries are installed. The remote client can thus connect to virtproxyd
which in turn can connect to a real hypervisor driver.
The virtproxyd probing code though isn't something that needs to live in
virtproxyd. By moving it into the remote client we can get probing
client side in all scenarios and avoid the extra trip via virtproxyd in
the common case.
Tested-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When virtproxyd gets a NULL URI, it needs to implement probing logic
similar to that found in virConnectOpen. The latter can't be used
directly since it relied on directly calling into the internal drivers
in libvirtd. virtproxyd approximates this behaviour by looking to see
what modular daemon sockets exist, or what daemon binaries are installed.
This same logic is also going to be needed when the regular libvirt
remote client switches to prefer modular daemons by default, as we
don't want to continue spawning libvirtd going forward.
Tested-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The libxl driver supports xen:///system URLs and the daemon socket
uses 'virtxend' as the socket prefix.
Reported-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When writing the memory snapshot into an existing file don't remove it
if the snapshot fails later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'file' is too generic to know what's going on. Rename it to
'memorysnapshotfile'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the snapshot disk type from the definition now that we validate that
it matches.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code executed later when creating a snapshot makes all decisions
based on the configured type rather than the actual type of the existing
file, while the check whether the file exists is based solely on the
on-disk type.
Since a block device is allowed to exist even when not reusing existing
files in contrast to regular files this creates a potential for a block
device to squeak past the check but then be influenced by other code
executed later. Specifically this is a problem when creating a snapshot
with the following XML:
<domainsnapshot>
<disks>
<disk name='vdb' type='file'>
<source file='/dev/sdb'/>
</disk>
</disks>
</domainsnapshot>
If the snapshot creation fails, '/dev/sdb' will be removed because it's
considered to be a regular file by the cleanup code.
Add a check that will force that the configured type matches the on-disk
state.
Additional supporting reason is that qemu stopped to accept block
devices with the 'file' backend, thus the above configuration will not
work any more. This allows us to fail sooner.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1972145
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In case when the snapshot target is of VIR_STORAGE_TYPE_BLOCK type and
doesn't exist libvirt won't be able to create it. Reject such a config
sooner.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Separate the 'else if' branches into nested conditions so that it's more
obvious when we'll be adding additional checks later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virQEMUCapsFillDomainDeviceGraphicsCaps fills data needed both for
validation of the graphics type and also for correct display in the
(dom)capablities XML.
Signal the support for egl-headless only when qemu has the capability.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
egl-headless graphics can be compiled out in qemu so we need to be able
to know whether the given qemu version support it.
Base the capability on the presence of the 'egl-headless' member in
'query-display-options' or imply it if 'query-display-options' is not
supported as we implied it before for all versions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
The DAC security driver has an option to register a callback that
is called instead of chown(). So far QEMU is the only user of
this feature and it's used to set labels on non-local disks (like
gluster), where exists notion of owners but regular chown() can't
be used.
However, this callback (if set) is called always, even for local
disks. And thus the QEMU's implementation duplicated parts of the
DAC driver to deal with chown().
If the DAC driver would call the callback only for non-local
disks then the QEMU's callback can be shorter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
I must admit, I have no idea why we build such POSIX dependent
code as DAC driver for something such not POSIX as WIN32. Anyway,
the code which is supposed to set error is not doing that. The
proper way is to mimic what chown() does:
On error, -1 is returned, and errno is set to indicate the error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As shown in the previous commit, @path can be NULL. However, in
that case @src->path is also NULL. Therefore, trying to "fix"
@path to be not NULL is not going to succeed. The real value of
NULLSTR() is in providing a non-NULL string for error reporting.
Well, that can be done in the error reporting without overwriting
argument.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virSecurityDACSetOwnershipInternal() function accepts two
arguments (among others): @path and @src. The idea being that in
some cases @path is NULL and @src is not and then @path is filled
from @src->path. However, this is done in both callers already
(because of seclabel remembering/recall). Therefore, this code in
virSecurityDACSetOwnershipInternal() is dead, effectively.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virSecurityDACSetOwnershipInternal() has two callers and in
both the private data (@priv) is obtained via
virSecurityManagerGetPrivateData(). But in case of DAC driver the
private data can never be NULL. This is because the private data
is allocated in virSecurityManagerNewDriver() according to
.privateDataLen attribute of secdriver. In case of DAC driver the
attribute is set to sizeof(virSecurityDACData).
NB, no other function within DAC driver checks for !priv.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce a function that frees individual items on the chown
list and declare and use g_autoptr() for it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When adding support for externally launched virtiofsd,
I was too liberal and did not require a target.
But the target is required, because it's passed to the
QEMU device, not to virtiofsd.
https://bugzilla.redhat.com/show_bug.cgi?id=1969232
Fixes: 12967c3e13
Fixes: 56dcdec1ac
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The NVRAM label is set in qemuSecuritySetAllLabel(). There's no
need to set its label upfront. In fact, setting it twice creates
an imbalance because it's unset only once which mangles seclabel
remembering. However, plain removal of the
qemuSecurityDomainSetPathLabel() undoes the fix for the original
bug (when dynamic ownership is off then the NVRAM is not created
with cfg->user and cfg->group but as root:root). Therefore, we
have to switch to virFileOpenAs() and pass cfg->user and
cfg->group and VIR_FILE_OPEN_FORCE_OWNER flag. There's no need to
pass VIR_FILE_OPEN_FORCE_MODE because the file will be created
with the proper mode.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1969347
Fixes: bcdaa91a27
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
126db34a81 had previously switched various
flows over to this from VIR_ERR_OPERATION_FAILED.
This change simply does the same for qemuDomainDetachPrepDisk,
qemuDomainDetachPrepInput and qemuDomainDetachPrepVsock to allow
management apps to centralise their error handling on just
VIR_ERR_DEVICE_MISSING for missing devices during a detach.
Signed-off-by: Lee Yarwood <lyarwood@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
If given file is not found in $PATH then g_find_program_in_path()
returns NULL. However, g_canonicalize_filename() does not accept
NULL as input.
Fixes: 65c2901906
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Use automatic memory freeing for the string list so that we can remove
the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'rbd_image_spec_t' struct has two string members 'id' and
'name'. We only stole the 'name' members thus the 'id's as well as the
whole list would be leaked on success.
Restructure the code so that we copy out the image names and call
rbd_image_spec_list_cleanup on success rather than on error.
The error path is then handled by using g_autofree for 'images'.
Since we no longer have a error path after allocating the returned
string list we can completely remove its cleanup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The only caller doesn't care about the number of elements in the string
list so we don't have to calculate it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's just one caller who cares (testQemuMonitorJSONGetTPMModels). Fix
it and remove the counting of elements.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This refactors multiple aspects of the function:
1) Use automatic memory freeing
2) Remove need to check element count in the returned arrays
3) Fixes questionable code linebreaks
4) Removes reuse of variables
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All callers pass in NULL-terminated string lists. Remove the 'nvalues'
argument and fix all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All the capability getters which return a string list do in fact return
a NULL-terminated list so we can use g_auto(GStrv) to free it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory clearing to simplify the control flow.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory clearing to simplify the control flow.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory clearing and remove the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use 'g_autoptr' for the two temporary JSON objects and remove the
cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virQEMUCapsFillDomainDeviceGraphicsCaps fills data needed both for
validation of the graphics type and also for correct display in the
(dom)capablities XML.
Signal the support for SDL only when qemu has the capability.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
SDL graphics can be compiled out in qemu so we need to be able to know
whether the given qemu version support it.
Base the capability on the presence of the 'sdl' member in
'query-display-options' or imply it if 'query-display-options' is not
supported as we implied it before for all versions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
The command allows to query various display-related options. The absence
of the command will be used to imply certain video-related capabilities
before we would be able to detect them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
IOThreads are supported with all 3 currently supported buses which can
have virtio devices (PCI, CCW, MMIO) , so there's no need for this check.
Additionally this check was buggy in the current location as on e.g.
hotplug cases the address may not yet be assigned for the disk and thus
a bogus error would be printed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1970277
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For validation of explicitly configured addresses we already ported the
same style of checks to qemuValidateDomainDeviceDefAddress and implicit
address assignment should do the right thing in the first place, thus
the function is redundant and can be removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Base the check on the logic from qemuDomainCheckCCWS390AddressSupport,
which will be removed later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We don't support any qemu which would support the 'virtio-s390'
addressing, thus we can drop all code related to it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modify the code in the last two instances in the code to behave as if
the flag is not asserted.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU_CAPS_VIRTIO_S390 can never be asserted any more, add an explicit
check that will reject the 'virtio-s390' address type and remove the
code which would auto-fill them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The devices no longer exist in qemu since the 2.6 release. Drop the
probing of the device properties and fix the data for
qemucapabilitiestest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU commit 7b3fdbd9a826791bd98e649cf44c0a6129a44179 released in 2.6
dropped the legacy s390 virtio machine and it's devices. Remove our
probing based on the devices.
The probing of properties of the appropriate devices will be removed
subsequently.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuDomainDefAddDefaultDevices skipped adding the memballoon for the
's390-virtio' machine type, but since it was removed in qemu 2.6 we can
remove the hack now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virCommandToString returns an allocated buffer, so using it directly as
argument of virBufferAdd which doesn't consume the string causes it to
be leaked. Switch to virBufferToStringBuf since we are already using a
buffer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The new version allows passing a virBuffer to format the string into.
This will be helpful in solving a memory lean in wrong usage of
virCommandToString and also in tests where we need to add a newline
after the command in certain cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virQEMUCapsProbeQMPMachineProps currently skips any not supported
machine type which includes `none` as well.
In order to start probing that machine type we need to add an exception
to not skip it when probing QEMU capabilities.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In libvirt we already use `query-command-line-options` QMP command but
that is useless as it doesn't provide correct data for `-machine`
option. So we need a new and better way to get that data.
We already use `qom-list-properties` to get options for specific machine
types so we can reuse it to get options for special `none` machine type
as a generic arch independent machine type.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Links between NUMA nodes can have different latencies and
bandwidths. This info is newly defined in ACPI 6.2 under
Heterogeneous Memory Attribute Table (HMAT) table. Linux kernel
learned how to report these values under sysfs and thus we can
expose them in our capabilities XML. The sysfs interface is
documented in kernel's Documentation/admin-guide/mm/numaperf.rst.
Long story short, two nodes can be in initiator-target
relationship. A node can be initiator if it has a CPU or a device
that's capable of initiating memory transfer. Therefore a node
that has just memory can only be target. An initiator-target link
can then have any combination of {bandwidth, latency} - {access,
read, write} attribute (6 in total). However, the standard says
access is applicable iff read and write values are the same.
Therefore, we really have just four combinations of attributes:
bandwidth-read, bandwidth-write, latency-read, latency-write.
This is the combination that kernel reports anyway.
Then, under /sys/system/devices/node/nodeX/acccessN/initiators we
find values for those 4 attributes and also symlinks named
"nodeN" which then represent initiators to nodeX. For instance:
/sys/system/node/node1/access1/initiators/node0 -> ../../node0
/sys/system/node/node1/access1/initiators/read_bandwidth
/sys/system/node/node1/access1/initiators/read_latency
/sys/system/node/node1/access1/initiators/write_bandwidth
/sys/system/node/node1/access1/initiators/write_latency
This means that node0 is initiator and node1 is target and values
of the interconnect can be read.
In theory, there can be separate links to memory side caches too
(e.g. one link from node X to node Y's main memory, another from
node X to node Y's L1 cache, another one to L2 cache and so on).
But sysfs does not express this relationship just yet.
The "accessN" means either "access0" or "access1". The difference
is that while the former expresses the best interconnect between
two nodes including CPUS and I/O devices (such as GPUs and NICs),
the latter includes only CPUs and thus is what we need.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1786309
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Expose virNumaInterconnect XML formatter so that it can be
re-used by other parts of the code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
There's nothing domain specific about NUMA interconnects. Rename
the virDomainNumaInterconnect* structures and enums to
virNumaInterconnect*.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Memory on a NUMA node can have a side caches. Configuring these
for a domain was implemented in v6.6.0-rc1~249 and friends.
However, up until now mgmt applications did not really know what
values to pass because we were not exposing caches of the host.
With recent enough kernel these are exposed under sysfs and with
a bit of parsing we can extend our capabilities XML. The sysfs
structure is documented in kernel's
Documentation/admin-guide/mm/numaperf.rst and basically maps in
1:1 fashion to our virNumaCache structure.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Expose virNumaCache XML formatter so that it can be re-used by
other parts of the code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
There's nothing domain specific about NUMA memory caches. Rename the
virDomainCache* structures and enums to virNumaCache*.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The way we format <cpu/> element for capabilities is not ideal,
because if there are no CPUs, i.e. no child elements, we still
output opening and closing element. To solve this,
virXMLFormatElement() could be used but that would introduce more
variables into the loop. Therefore, move the formatter into a
separate function and use virXMLFormatElement().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When using firmware auto-selection and user enables AMD SEV-ES we need
to pick correct firmware that actually supports it. This can be detected
by having `amd-sev-es` in the firmware JSON description.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In a few places we take 1 and shift it left repeatedly. So much
that it won't longer fit into signed integer. The problem is that
this is undefined behaviour. Switching to 1U makes us stay within
boundaries.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
In a few places it may happen that the array we want to sort is
still NULL (e.g. because there were no leases found, no paths for
secdriver to lock or no cache banks). However, passing NULL to
qsort() is undefined and even though glibc plays nicely we
shouldn't rely on undefined behaviour.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
"virt-aa-helper" links, amongst others, against "datatypes.o" and
"libvirt.so". The latter links against "libvirt_driver.a" which in turn
also links against "datatypes.o", leading to a One-Definition-Rule
violoation for "virConnectClass" et al. in "datatypes.c".
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now we have everything prepared so that @model doesn't have to be
rewritten. The correct model can be chosen right from the
beginning.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We want to call qemuBuildVirtioDevStr() from
qemuBuildDeviceVideoStr() but only for some models (currently
"virtio-gpu" and "vhost-user-gpu"), not all of them. Move this
logic into qemuDeviceVideoGetModel() because this logic will be
refined.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This may look like a step backwards, but it isn't. The point is
that in near future the chosen model will depend on more than
just video type.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This may look like a step backwards, but it isn't. The point is
that in near future the chosen model will depend on more than
just video type.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There is the same check written twice (whether given video card
is primary one and whether it supports VGA mode). Write it just
once and store it in a boolean variable.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The code that decides video card model is going to be reworked
and expanded. Separate it out into a function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This function doesn't modify passed video definition. Make the
argument const.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
QEMU 6.1 will replace the virgl property of virtio-vga device to
virtio-vga-gl device. Adapt to that update.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/167
Signed-off-by: Han Han <hhan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU 6.1 will add virtio-gpu-gl-pci device to replace the virgl property
of virtio-gpu-pci device. Adapt to that change.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1967356
Signed-off-by: Han Han <hhan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The devices virtio-gpu-gl-pci and virtio-vga-gl, aimed to replace the
virgl property, are valid for 3d accerlation as well.
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This flag will be used for the device virtio-gpu-gl-pci which is introduced
since QEMU 6.1.
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
"avx-vvni" was introduced to qemu in commit
c1826ea6a052084f2e6a0bae9dd5932a727df039, adding it Cooperlake.
This feature is currently not used by any libvirt CPU models, but its
addition silences a warning from sync_qemu_i386.py:
```
warning: Unknown feature 'CPUID_7_1_EAX_AVX_VNNI'
warning: Feature unknown to libvirt: CPUID_7_1_EAX_AVX_VNNI
```
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Connecting a tap device to an Open vSwitch is done by adding a "port"
to the switch with the ovs-vsctl "add-port" command. The port will
have the same name as the tap device, but it is a separate entity, and
can survive beyond the destruction of the tap device (although under
normal circumstances the port will be deleted around the same time the
tap device is deleted).
This makes it possible for a port of a particular name to already
exist at the time libvirt calls ovs-vsctl to add that port. The
original commit of Open vSwitch support (commit df81004632, libvirt
0.9.10, Feb. 2012) used the "--may-exist" option to the add-port
command to indicate that a port of the desired name might already
exist, and that it was okay to simply re-use this port (rather than
failing with an error message).
Then in commit 33445ce844 (libvirt 1.2.7, April 2014) the command
was changed to use "--if-exists del-port blah" instead of
"--may-exist". The reason given was that there was a bug in OVS where
a stale port would be unusable even though it still existed; the
workaround was to forcibly delete any existing port prior to adding
the new port (of the same name). This is the ovs-vsctl command still
in use by libvirt today.
It recently came up in the discussion of a bug concerning guest packet
loss during OpenStack upgrades (https://bugzilla.redhat.com/1963164)
that the bug in OVS that necessitated the del-port workaround was
fixed quite a long time ago (August 2015):
e21c6643a0
thus rendering the workaround in libvirt unnecessary. The assertion in
that discussion is that this workaround is now the cause of the packet
loss being experienced during OpenStack upgrades. I'm not convinced
this is the case, but it does appear that there is no reason to carry
this workaround in libvirt any longer, so this patch reverts the code
back to the original behavior (using "--may-exist" instead of
"--if-exists del-port").
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
I've identified some places (mostly by looking for
virBufferUse()) that can use virXMLFormatElement() instead of
open coded version of it. I'm sure there are many more places
that could use the same treatment. Let's cure them some other
time.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The semicolon in question makes the pipeline fail over a style checker
complaint.
Introduced-in: 360b8eb2d2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This was introduced in qemu commit
e11fd68996fb27c040552320f01a7d30a15a7cc1.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This member is unused (apart from only being set in
virCHDriverConfigNew()), and never freed really (leading to a
memleak).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If the chStateInitialize method fails, we call chStateCleanup
which free's all global state. It fails to set the global
'ch_driver' to NULL, however, so a later attempt to open the
cloud hypervisor driver will succeed and then crash attempting
to access freed memory.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The iscsi-direct storage pool backend works merely like this: a
connection is established to the target (usually done via
virStorageBackendISCSIDirectSetConnection()), intended action is
executed (e.g. reporting LUNs, volume wiping), and at the end the
connection is closed via virISCSIDirectDisconnect().
The problem is that virISCSIDirectDisconnect() reports its own
errors which may overwrite error that occurred during LUN
reporting, or volume wiping or whatever.
To fix this, use virErrorPreserveLast() + virErrorRestore()
combo, which either preserves previously reported error message,
or is NOP if there's no error reported.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1797879
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Xen only supports one firmware, making autoselection easy to implement.
In fact, <os firmware='efi'> is probably preferable in the Xen driver,
where libxl supports a firmware setting with accepted values such as
bios, ovmf, uefi (currently same semantics as ovmf), seabios, etc.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Xen+ovmf does not support secure boot. Fail domain def validation
if secure boot is enabled.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce libxlDomainDefValidate and move the existing validation
check from libxlDomainDefPostParse. Additional validation will be
introduced in subsequent patches.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The audit log contains the following denials from libvirtd
apparmor="DENIED" operation="capable" profile="libvirtd" pid=6012 comm="daemon-init" capability=17 capname="sys_rawio"
apparmor="DENIED" operation="capable" profile="libvirtd" pid=6012 comm="rpc-worker" capability=39 capname="bpf"
apparmor="DENIED" operation="capable" profile="libvirtd" pid=6012 comm="rpc-worker" capability=38 capname="perfmon"
Squelch the denials and allow the capabilities in the libvirtd
apparmor profile.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In fcdcf8f70c the remoteGetUNIXSocket() function was changed and
one new variable was introduced (among other things): @env_name.
However, for WIN32 case the variable changed name to @env_path
which builds mingw builds.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The virNetSocketNewConnectUNIX() function was changed in
48f66cfe3e. And its WIN32 version (which just reports an error)
was updated too, but this new argument @spawnDaemonPath was not
marked as unused.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We want to use those shared drivers provided by libvirt to avoid
implementing our own.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Wei Liu <liuwe@microsoft.com>
After previous patches, the @ret variable and the 'cleanup'
label are redundant. Remove them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
