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>
There are two variables that can be freed automatically: @cmd
(which allows us to drop explicit virCommandFree() call at the
end of the function) and @help which was never freed (and thus
leaked).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The CH driver needs "cloud-hypervisor" binary. And if none was
found then the initialization of the driver fails as
chStateInitialize() returns VIR_DRV_STATE_INIT_ERROR. This in
turn means that whole daemon fails to initialize. Let's return
VIR_DRV_STATE_INIT_SKIPPED in this particular case, which
disables the CH drvier but lets the daemon run.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
After previous patches, there's not much value in
chExtractVersion(). Rename chExtractVersionInfo() to
chExtractVersion() and have it use virCHDriver directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The only caller, chExtractVersion() passes not NULL. Therefore,
it's redundant to check for NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If chExtractVersionInfo() fails, in some cases it reports error
and in some it doesn't. Fix those places and drop reporting error
from chExtractVersion() which would just overwrite more specific
error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When the default driver mode requests the modular daemons, we still
defaulted to spawning libvirtd if the URI was NULL, because we don't
know which driver specific daemon to spawn. virtproxyd has logic
that can handle this as it is used for compatibility when accepting
incoming TCP connections with a NULL URI.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The "spawnDaemon" and "binary" parameters are co-dependant, with the
latter non-NULL, if-and-only-if the former is true. Getting rid of the
"spawnDaemon" parameter simplifies life for the callers and eliminates
an error checking scenario.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When deciding what socket to connect to, we build the daemon path
that we need to autostart. This path only needs to be populated
if we actually intend to use autostart.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The remoteGetUNIXSocket method currently just returns the daemon name
and the caller then converts this to a path. Except the SSH helper
didn't do this, so it was relying on later code expanding $PATH, and
this doesn't allow for build root overrides.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We have helper methods that return boolans for ro/user/autostart
properties. We then pack them into a flags parameter, and later
unpack them again. This makes the code consistently use flags
throughout.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This enum will shortly be used by the remote driver sockets helper
methods too.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The g_build_filename() would decide which separator
to use instead of hardcoding in g_strdup_printf().
Related issue: https://gitlab.com/libvirt/libvirt/-/issues/12
Signed-off-by: Luke Yue <lukedyue@gmail.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Cloud-Hypervisor is a KVM virtualization using hypervisor. It
functions similarly to qemu and the libvirt Cloud-Hypervisor driver
uses a very similar structure to the libvirt driver.
The biggest difference from the libvirt perspective is that the
"monitor" socket is seperated into two sockets one that commands are
issued to and one that events are notified from. The current
implementation only uses the command socket (running over a REST API
with json encoded data) with future changes to add support for the
event socket (to better handle shutdowns from inside the VM).
This patch adds support for the following initial VM actions using the
Cloud-Hypervsior API:
* vm.create
* vm.delete
* vm.boot
* vm.shutdown
* vm.reboot
* vm.pause
* vm.resume
To use the Cloud-Hypervisor driver, the v15.0 release of
Cloud-Hypervisor is required to be installed.
Some additional notes:
* The curl handle is persistent but not useful to detect ch process
shutdown/crash (a future patch will address this shortcoming)
* On a 64-bit host Cloud-Hypervisor needs to support PVH and so can
emulate 32-bit mode but it isn't fully tested (a 64-bit kernel and
32-bit userspace is fine, a 32-bit kernel isn't validated)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: William Douglas <william.douglas@intel.com>
When processing node devices, the udevProcessStorage() will be
called if the device is some form of storage. In here, ID_TYPE
attribute is queried and depending on its value one of more
specialized helper functions is called. For instance, for
ID_TYPE=="cd" the udevProcessCDROM() is called, for
ID_TYPE=="disk" the udevProcessDisk() is called, and so on.
But there's a problem with ID_TYPE and its values. Coming from
udev, we are not guaranteed that ID_TYPE will contain "cd" for
CDROM devices. In fact, there's a rule installed by sg3_utils
that will overwrite ID_TYPE to "cd/dvd" leaving us with an
unhandled type. Fortunately, this was fixed in their upstream,
but there are still versions out there, on OS platforms that we
aim to support that contain the problematic rule. Therefore, we
should accept both strings.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1848875
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Let's use a different variable for storing retvals of helper
functions. This way the usual function pattern can be restored.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This function can't fail really as it's returning 0 no matter
what. This is probably a residue from old days when we cared
about propagating OOM errors. Now we just abort. Make its return
type void then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This function can't fail really as it's returning 0 no matter
what. This is probably a residue from old days when we cared
about propagating OOM errors. Now we just abort. Make its return
type void then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
libxl objects are supposed to be initialized and disposed. Adjust
libxlMakeNic to use an already initialized object owned by the caller.
Adjust libxlMakeNicList to initialize the list of objects, before they
are filled by libxlMakeNic. The libxl_domain_config object passed to
libxlMakeNicList is owned by the caller and will be disposed with
libxl_domain_config_dispose, which also disposes embedded objects such
as libxl_device_nic.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Olaf Hering <olaf@aepfle.de>
Before the mentioned commit we always parsed the whole disk definition
for qemuDomainBlockCopy API but we only used the @src part. Based on
that assumption the code was changed to parse only the disk <source>
element.
Unfortunately that is not correct as we need to parse some parts of
<driver> element as well.
Fixes: 0202467c4b
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Attribute `type` and sub-element `metadata_cache` are internally stored
in the `virStorageSource` structure. Sometimes we only care about the
disk source bits so we need a dedicated helper for that.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
libacl is Linux-only, so we don't need to explicitly check for
either the target platform or header availability, and we can
simply rely on cc.find_library() instead. The corresponding
preprocessor define is renamed to more accurately reflect the
nature of the check.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
QEMU_DOMAIN_DISK_PRIVATE(disk)->transientOverlayCreated flag
gets true unexpectedly on qemuProcessSetupDisksTransientSnapshot() when
the disk has <transient shareBacking='yes'> option.
The flag should be enabled on qemuDomainAttachDiskGeneric() after the
overlay setup is completed.
Skip enabling transientOverlayCreated for the disk here.
Fixes: 75871da0ec
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We supported autostart of node devices via an xml element, but this
is not consistent with other libvirt objects which use an explicit API
for setting autostart status. So revert this and implement it as an
official API in a future commit.
The initial support was refactored after merging, so this commit reverts
both of those previous commits.
Revert "virNodeDevCapMdevParseXML: Use virXMLPropEnum() for ./start/@type"
This reverts commit 9d4cd1d1cd.
Revert "nodedev: support auto-start property for mdevs"
This reverts commit 42a5585499.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
While we couldn't historically connect to the remote session daemon
automatically, we do allow the user to set an explicit socket path
to enable the connections to work. This ability was accidentally
lost in
commit f8ec7c842d
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Wed Jul 8 17:03:38 2020 +0100
rpc: use new virt-ssh-helper binary for remote tunnelling
We need to force use of 'netcat' when a 'socket' path is given in
the URI parameters.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the defaults for the proxy/mode settings are set before
parsing URI parameters. A following commit will introduce a dependancy
on the URI parsing for the defaults, so they need to move.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In one of my recent commits I've done some renaming. But whilst
doing so I also mistakenly replaced 'goto cleanup' with 'return
-1' in virCapabilitiesHostNUMAInitReal() which was incorrect.
Fixes: fe25224fda
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The remoteGetUNIXSocketHelper method always returns a non-NULL string.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
virFileFindResource needs to be given the absolute build path otherwise
its results will vary according to the CWD, leading to spurious failures
in dev testing.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When <transient shareBacking='yes'> is set to a disk and the overlay
disk already exists because of something abnormal, libvirt is terminated
by Segmentation fault.
# virsh start Test0
error: Disconnected from qemu:///system due to end of file
error: Failed to start domain 'Test0'
error: End of file while reading data: Input/output error
Add NULL check for snapdiskdef so that the rollback can work correctly.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Fixes: 2e94002d2a
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
