With libpmem support compiled into qemu it will trigger the following
denials on every startup.
apparmor="DENIED" operation="open" name="/"
apparmor="DENIED" operation="open" name="/sys/bus/nd/devices/"
This is due to [1] that tries to auto-detect if the platform supports
auto flush for all region.
Once we know all the paths that are potentially needed if this feature
is really used we can add them conditionally in virt-aa-helper and labelling
calls in case </pmem> is enabled.
But until then the change here silences the denial warnings seen above.
[1]: https://github.com/pmem/pmdk/blob/master/src/libpmem2/auto_flush_linux.c#L131
Bug: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1871354
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>
Starting with 3b076391be
(v6.1.0-122-g3b076391be) we support http cookies. Since they may contain
somewhat sensitive information we should not format them into the XML
unless VIR_DOMAIN_DEF_FORMAT_SECURE is asserted.
Reported-by: Han Han <hhan@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
We always tried to install backing store for the image even if it didn't
make sense, e.g. for a full backup into a raw image. Additionally we
didn't record the backing file into the qcow2 metadata so the image
itself contained the diff of data but reading from it would be
incomplete as it depends on the backing image.
This patch fixes both issues by carefully installing the correct backing
file when appropriate and also recording it into the metadata when
creating the image.
https://bugzilla.redhat.com/show_bug.cgi?id=1813310
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is the last missing g_autofree conversion change in the module after
commit 1e2ae2e311 took care of the VIR_AUTOFREE conversion.
Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Before this patch we would simply rely on QEMU failing to attach the
device. Since we have a flag in the address set telling us which
controllers support hotplug, we can fail the operation sooner.
This also assures that when hotplugging with no provided PCI address,
that we skip any controllers with hotplug='off', and attempt to assign
the device to a controller that not only supports hotplug, but also
has it enabled.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The HOTPLUGGABLE flag is set for appropriates buses in a PCI address
set, and thnis patch updates virDomainPCIAddressFlagsCompatible() to
check the HOTPLUGGABLE flag when searching for a suitable bus/slot for
a device. No devices request HOTPLUGGABLE though (yet), so there is no
observable effect.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virDomainPCIAddressBusSetModel() is called for each PCI controller
when building an address set prior to assiging PCI addresses to
devices.
This patch adds a new argument, allowHotplug, to that function that
can be set to false if we know for certain that a particular
controller won't support hotplug
The most interesting case is in qemuDomainPCIAddressSetCreate(), where
the config of each existing controller is available while building the
address set, so we can appropriately set allowHotplug = false when the
user has "hotplug='off'" in the config of a controller that normally
would support hotplug. In all other cases, it is set to true or false
in accordance with the capability of the controller model.
So far we aren't doing anything with this bus flag in the address set.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Old behavior: If the address was manually provided by config, copy
device AUTOASSIGN flag into the bus flag, and then later on in the
function *always* check for a match of the flags (which will always
match if the address came from config, since we just copied it).
New behavior: Don't mess with the bus flags - just directly check if
the AUTOASSIGN flag matches in bus and dev, but only make the check if
the address didn't come from config (i.e. it was auto-assigned by
libvirt).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When the HOTPLUGGABLE flag was originally added, it was set for all
the PCI controllers that accepted hotplugged devices, and requested
for all devices that were auto-assigned to a controller. While we're
still autoassigning to the same list of controllers, those controllers
may or may not support hotplug, so let's use the flag that fits what
we're actually doing.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This new flag will be set for any controller that we decide can have
devices assigned to it automatically during PCI device assignment. In
the past PCI_CONNECT_TYPE_HOTPLUGGABLE was used for this purpose, but
that is overloading that flag, and no longer technically correct; what
we *really* want is to auto-assign devices to any pcie-root-port or
pcie-switch-downstream-port regardless of whether or not that
controller happens to have hotplug enabled.
This patch just adds the flag, but doesn't use it at all. Note that
the numbering of all the other flags was changed in order to insert
the new flag near the beginning of the list; that doesn't cause any
problem because the connect flags aren't stored anywhere between runs
of libvirtd.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If a pcie-root-port or pcie-downstream-port has hotplug='off' in its
<target> subelement, and if the qemu binary supports the hotplug=false
option, then it will be added to the commandline for the pcie
controller. This controller will then not allow any hotplug/unplug of
devices while the guest is running (and the hotplug capability won't
be advertised to the guest OS, so the guest OS also won't present
unplugging of PCI devices as an option).
<controller type='pci' model='pcie-root-port'>
<target hotplug='off'/>
</controller>
For any PCI controllers other than pcie-downstream-port and
pcie-root-port, of for qemu binaries that don't support the hotplug
commandline option, an error will be logged during validation.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
a <controller type='pci'...> element can now have a "hotplug"
attribute in the <target> subelement. This is intended to control
whether or not the slot(s) of the controller support
hotplugging/unplugging a device:
<controller type='pci' model='pcie-root-port'>
<target hotplug='off'/>
</controller>
The default value of hotplug is "on".
Since support for configuring such an option is hypervisor-dependent
(and will vary among different types of PCI controllers even on a
single hypervisor), no validation is done in this patch - that
validation will be done in the patch that wires support for the
setting into the hypervisor.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This caps flag is set when the qemu binary supports the option
"hotplug" for pcie-root-port, ioh3420 (Intel pcie-root-port) and
xio3130-downstream (Intel pcie-downstream-port). If it's available,
it's possible to disable hotplugging/unplugging devices on a
particular port by adding ",hotplug=off" to the qemu device
commandline. This option first appears in qemu-5.0.0.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add support in the domXML<->native config converter for max_event_channels.
The parser and formater functions for max_grant_frames were reworked to
also parse max_event_channels. In doing so the xenbus controller is added
earlier in the config parsing, requiring a small adjustment to one of the
existing tests. Include a new test for the event channel conversion.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Add support for setting event_channels in libxl domain config object and
include a test to check that it is properly converted from XML to libxl
domain config.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Event channels are like PV interrupts and in conjuction with grant frames
form a data transfer mechanism for PV drivers. They are also used for
inter-processor interrupts. Guests with a large number of vcpus and/or
many PV devices many need to increase the maximum default value of 1023.
For this reason the native Xen config format supports the
'max_event_channels' setting. See xl.cfg(5) man page for more details.
Similar to the existing maxGrantFrames option, add a new xenbus controller
option 'maxEventChannels', allowing to adjust the maximum value via libvirt.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The signatures of these two CPU model differ only in stepping as both
report family 6 and model 85. Skylake-Server uses stepping 4 or less and
Cascadelake-Server uses stepping 5..7.
https://bugzilla.redhat.com/show_bug.cgi?id=1761678
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
CPU models defined in the cpu_map can use signature/@stepping attribute
to match a limited set of stepping numbers. The value is a bitmap for
bits 0..15 each corresponding to a single stepping value. For example,
stepping='4-6,9' will match 4, 5, 6, and 9. Omitting the attribute is
equivalent to stepping='0-15'.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Thanks to glib allocation functions which abort on OOM the function
cannot ever return NULL.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The CPU models in our cpu_map define their signatures using separate
family and model numbers. Let's store the signatures in the same way in
our runtime representation of the cpu_map.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It can be used for separating family, model, and stepping numbers from a
single 32b integer as reported by CPUID.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function will be used for freeing virCPUx86Signatures structure
introduced later in this series.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Later in this series the function will work on a newly introduced
virCPUx86Signatures structure. Let's move it to the place where all
related functions will be added and rename the function as
virCPUx86SignaturesFormat for easier review of the virCPUx86Signatures
patch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Later in this series the function will work on a newly introduced
virCPUx86Signatures structure. Let's move it to the place were all
related functions will be added and rename the function as
virCPUx86SignaturesMatch for easier review of the virCPUx86Signatures
patch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Later in this series the function will work on a newly introduced
virCPUx86Signatures structure. Let's move it to the place were all
related functions will be added and rename the function as
virCPUx86SignaturesCopy for easier review of the virCPUx86Signatures
patch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The hint was introduced a long time ago when broken TSX implementation
was found in Haswell and Broadwell CPUs. Since then many more CPUs with
TSX were introduced and and disabled due to TAA vulnerability.
Thus the hint is not very useful and I think removing it is a better
choice then updating it to cover all current noTSX models.
This partially reverts:
commit 7f127ded65
cpu: Rework cpuCompare* APIs
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pass the packed option on the QEMU command line of the capability for
packed virtqueues is detected and the parameter is set explicitly.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Expose the virtio parameter for packed virtqueues as an optional libvirt
XML attribute to virtio-backed devices, e.g.:
<interface type='user'>
<mac address='00:11:22:33:44:55'/>
<model type='virtio'/>
<driver packed='on'/>
</interface>
If the attribute is omitted, the default value for this attribute is 'off' and
regular split virtqueues are used.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Add the capability for QEMU's packed virtqueues for virtio that supposedly have
better cache utilization and performance compared to the default split queues.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Unlike `waitpid`, `virProcessWait` only returns -1 (error) or 0
(success), so comparing that to `pid` will always be false and the
parent will report failure with:
error : main:851 : Failed to fork as daemon: No such file or directory
even though the grandchild process is succesfully running. Note that the
errno message is misleading: it was last set when trying to find a
restart state file.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reported-by: Marcin Krol <hawk@tld-linux.org>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Replace vm->def->disks[i] with dom_disk variable which is
initialized to the same disk.
Signed-off-by: Yi Li <yili@winhong.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
So far, libvirt generates the following path for automatic dumps:
$autoDumpPath/$id-$shortName-$timestamp
where $autoDumpPath is where libvirt stores dumps of guests (e.g.
/var/lib/libvirt/qemu/dump), $id is domain ID and $shortName is
shortened version of domain name. So for instance, the generated
path may look something like this:
/var/lib/libvirt/qemu/dump/1-QEMUGuest-2020-03-25-10:40:50
While in case of embed driver the following path would be
generated by default:
$root/lib/libvirt/qemu/dump/1-QEMUGuest-2020-03-25-10:40:50
which is not clashing with other embed drivers, we allow users to
override the default and have all embed drivers use the same
prefix. This can create clashing paths. Fortunately, we can reuse
the approach for machined name generation
(v6.1.0-178-gc9bd08ee35) and include part of hash of the root in
the generated path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
So far, libvirt generates the following path for memory:
$memoryBackingDir/$id-$shortName/ram-nodeN
where $memoryBackingDir is the path where QEMU mmaps() memory for
the guest (e.g. /var/lib/libvirt/qemu/ram), $id is domain ID
and $shortName is shortened version of domain name. So for
instance, the generated path may look something like this:
/var/lib/libvirt/qemu/ram/1-QEMUGuest/ram-node0
While in case of embed driver the following path would be
generated by default:
$root/lib/qemu/ram/1-QEMUGuest/ram-node0
which is not clashing with other embed drivers, we allow users to
override the default and have all embed drivers use the same
prefix. This can create clashing paths. Fortunately, we can reuse
the approach for machined name generation
(v6.1.0-178-gc9bd08ee35) and include part of hash of the root in
the generated path.
Note, the important change is in qemuGetMemoryBackingBasePath().
The rest is needed to pass driver around.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
So far, libvirt generates the following path for hugepages:
$mnt/libvirt/qemu/$id-$shortName
where $mnt is the mount point of hugetlbfs corresponding to
hugepages of desired size (e.g. /dev/hugepages), $id is domain ID
and $shortName is shortened version of domain name. So for
instance, the generated path may look something like this:
/dev/hugepages/libvirt/qemu/1-QEMUGuest
But this won't work with embed driver really, because if there
are two instances of embed driver, and they both want to start a
domain with the same name and with hugepages, both drivers will
generate the same path which is not desired. Fortunately, we can
reuse the approach for machined name generation
(v6.1.0-178-gc9bd08ee35) and include part of hash of the root in
the generated path.
Note, the important change is in qemuGetBaseHugepagePath(). The
rest is needed to pass driver around.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This reverts commit 06a19921b6.
What I haven't realized when writing this ^^ commit is that the
virQEMUDriver structure already stores the root directory path.
And since the pointer is immutable it can be accessed right from
the structure and thus there is no need to duplicate it in the
driver config.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The cfg->root is going away, therefore get the info right from
the driver structure.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The code that generates "qemu-embed-$hash" is going to be useful
in more places. Separate it out into a function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The virDomainGenerateMachineName() function doesn't belong in
src/conf/ really, because it has nothing to do with domain XML
parsing. It landed there because of lack of better place in the
past. But now that we have src/hypervisor/ the function should
live there. At the same time, the function name is changed to
match new location.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Initially introduced in v3.10.0-rc1~172.
When generating a path for memory-backend-file or -mem-path, qemu
driver will use the following pattern:
$memoryBackingDir/libvirt/qemu/$id-$shortName
where $memoryBackingDir defaults to /var/lib/libvirt/qemu/ram but
can be overridden in qemu.conf. Anyway, the "/libvirt/qemu/" part
looks redundant, because it's already contained in the default,
or creates unnecessary nesting if overridden in qemu.conf.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Introduced in v1.2.17-rc1~121, the assumption was that the
driver->privileged is immutable at the time but it might change
in the future. Well, it did not ever since. It is still immutable
variable. Drop the needless accessor then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Commit 54a401af47 split out DriverConfigInit from DriverConfigNew, but
then called it a bit late from libxlStateInitialize. The cfg is used in
libxlDriverConfigLoadFile and when uninitialized results in a crash.
Calling DriverConfigInit immediately after DriverConfigNew fixes the
crash.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This option prevents misbehaviours on guest if a qemu 9pfs export
contains multiple devices, due to the potential file ID collisions
this otherwise may cause.
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce new 'multidevs' option for filesystem.
<filesystem type='mount' accessmode='mapped' multidevs='remap'>
<source dir='/path'/>
<target dir='mount_tag'>
</filesystem>
This option prevents misbehaviours on guest if a qemu 9pfs export
contains multiple devices, due to the potential file ID collisions
this otherwise may cause.
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The QEMU 9pfs 'multidevs' option exists since QEMU 4.2. Probe QEMU's
command line set though to check whether this option is really
available, and if yes enable this new QEMU_CAPS_FSDEV_MULTIDEVS
capability on libvirt side.
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that all its helper functions are in qemu_validate.c, we can
move the function itself. The helpers can become static again since
they're all in the same file.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This will allow to move qemuDomainDeviceDefValidate() itself in
the next patch in a cleaner way.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Move the function and all its static helper functions.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This function will remain public due to its usage in qemublocktest.c
even after moving qemuDomainDeviceDefValidate(). The position of its
header in qemu_validate.h is no accident.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This function alone requires other 3 static functions to be
moved as well, thus let's move it in its own patch.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
qemuDomainChrDefValidate() has a lot of static helpers functions
that needed to be moved as well.
Other functions from qemuDomainDeviceDefValidate() that were
also moved:
- qemuValidateDomainSmartcardDef
- qemuValidateDomainRNGDef
- qemuValidateDomainRedirdevDef
- qemuValidateDomainWatchdogDef
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
The next big task is to move qemuDomainDeviceDefValidate() to
qemu_validation.c, which is a function that calls a lot of
other static helper functions. This patch starts it by moving
qemuDomainDeviceDefValidateAddress().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Move the static functions qemuDomainValidateDef() uses, as well as
qemuDomainValidateDef() itself to qemu_validate.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This patch introduces a new file to host domain validations from
the QEMU driver. And to get things started, let's move
qemuDomainDefValidateFeatures() to this new file.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is the only instance of g_autofree change applicable for
qemu_checkpoint.c
Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
When the comment in libvirtd.sasl was last updated with
commit fe772f24a6
Author: Cole Robinson <crobinso@redhat.com>
Date: Sat Oct 20 14:10:03 2012 -0400
daemon: Avoid 'Could not find keytab file' in syslog
it was noted that only old versions of kerberos would need the
environment variable to be set: that was more than seven years
ago, so it's safe to assume that none of our current target
platforms still requires that hack and setting the appropriate
key in the configuration file will be enough.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
libvirtd supports this feature, and virtqemud ultimately calls to
the same code so it does as well: advertise it in the sysconf file
for the latter, as is already the case for the former.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This follows the example set by libvirtd, and makes it easier for
the admin to tweak the timeout or disable it altogether.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
While not terribly useful in general, tweaking each daemon's
timeout (or disabling it off altogether) is a valid use case which
we can very easily support while being consistent with what already
happens for libvirtd. This is a first step in that direction.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We're going to add many more later, so start by adjusting the
existing ones to more closely follow the example set by libvirtd.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When using systemd we want to take advantage of socket activation
instead of keeping daemons running all the time, so we default to
shutting them down after two minutes of inactivity.
At the same time, we want it to be possible for the admin to opt
out of this behavior and disable timeouts entirely. A very natural
way to do so would be to specify a zero-length timeout, but that's
currently not accepted by the command line parser. Address that.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When redefining checkpoints from scratch we'd not set the 'current'
checkpoint if there wasn't any. This meant that the code wasn't ever
able to set a 'current' checkpoint as any other one looks up if the
parent of the redefined checkpoint is current.
Since the backup code then requires the current checkpoint to start the
lookup we'd not be able to perform a backup after restoring the
checkpoint hierarchy.
Reported-by: Eyal Shenitzky <eshenitz@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Skip the liveness and capability checks when redefining checkpoints as
we don't need qemu interactions to update the metadata.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Add a comment noting that job update can cause the pointer to be invalid
and thus should not be accessed after.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
No callers use it any more. Additionally if qemuBlockJobUpdate was
called with the last reference of the job e.g. in
qemuBlockJobRefreshJobs, the reading of the job state would happen from
freed memory.
Reported-by: Pavel Mores <pmores@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Upcoming patch will remove it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virStorageFileSupportsSecurityDriver ends up initializing the storage
file backend which after the recent changes to the daemon architecture
may end up dlopening of the backend modules.
Since this is required only for remote storage we can optimize the call
by moving the check whether the backend is supported to the branch which
deals with remote storage.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
Treat the shortcut for chowning local files as a stand-alone section
by returning success from it and refactor the rest so that the cleanup
section is inline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
The same timeout as libvirtd can't be used for virtlogd: even with
socket activation in place, any message produced by QEMU on its
standard output/error between when virtlogd quits due to the timeout
and when it's started again due to socket activation will get lost.
This reverts commit 02b6005063
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 2ace7a87a8 introduced a logic bug by an improperly
modified condition where we'd skip to the else branch when reusing of
external images was requested and blockdev is available.
The original intentions were to skip the backing store update with
blockdev.
Fix it by only asserting the boolean which was used to track whether we
support update of the backing store only when blockdev is not present
along with the appropriate rename.
https://bugzilla.redhat.com/show_bug.cgi?id=1820016
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When moving the formatting of this attributes from -drive
to -device, the QEMU_CAPS_USB_STORAGE_WERROR capability
was used, because usb-storage was the last device to gain
this capability.
However this lead to the assumption that QEMU binaries
without the usb-storage device do not support this,
leading to breakage on s390x with blockdev.
Fixes: bb4f3543bbhttps://bugzilla.redhat.com/show_bug.cgi?id=1819250
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Detect the werror property on SCSI and virtio disks.
But clear it if the QEMU supports usb-storage device without it
also supporting this option for usb-storage.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In previous commit:
commit e6afacb0fe
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Wed Feb 12 12:26:11 2020 +0000
qemu: start/stop an event loop thread for domains
A bogus comment was added claiming we didn't need to shutdown the
event thread in the qemuProcessStop method, because this would be
done in the monitor EOF callback. This was wrong because the EOF
callback only runs in the case of a QEMU crash or a guest initiated
clean shutdown & poweroff. In the case where the libvirt admin
calls virDomainDestroy, the EOF callback never fires because we
have already unregistered the event callbacks. We must thus always
attempt to stop the event thread in qemuProcessStop.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reported-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
For http/https URIs we need to preserve the query part as it may be
important to refer to the image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If the storage source has the query part set, format it in the output.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a new attribute for holding the query part for http(s) disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since we are refreshing the relative paths when doing the blockjobs we
no longer need to load them upfront when doing the snapshot.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Preservation of the relative relationship requires us to load the
backing store strings from the disk images. With blockdev we stopped
detecting the backing chain if it's specified in the XML so the relative
links were not loaded at that point. To preserve the functionality from
the pre-blockdev without accessing the backing chain unnecessarily
during VM startup we must refresh the relative links when relative
block commit or block pull is requested.
https://bugzilla.redhat.com/show_bug.cgi?id=1818655
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While it is impossible for VIR_ALLOC() to return an error, we
should be consistent with the rest of the code and not continue
initializing the virSecurityDeviceLabelDef structure.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Even with GLib it is still possible for virQEMUCapsNew() to
return NULL because it calls virQEMUCapsInitialize() which is a
wrapper over pthread_once() which may fail. At least, we still
check for its retval. If it so happens that the virQEMUCapsNew()
fails and returns NULL, we should not dereference it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Unfortunately, yajl_free() is not NOP on NULL. It really does
expect a valid pointer. Therefore, check whether the pointer we
want to pass to it is NULL or not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The daemons are not supported on Win32 and therefore were not compiled
in that platform. However, with the daemon code sharing, all the code in
utils *is* compiled and it failed because `waitpid`, `fork`, and
`setsid` are not available. So, as before, let's not build them on
Win32 and make the code more portable by using existing vir* wrappers.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Several daemons have similar code around general daemon startup code.
Let's move it into a file and share it among them.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The quotes are forbidden only inside the value, but the value itself may
be enclosed in quotes. Fix the RNG schema and validator and add a test
case.
https://bugzilla.redhat.com/show_bug.cgi?id=1804750
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 5540acb9a2 added a minimum size verification for the target
size of ppc64 NVDIMMs but forgot to remove a MAX() size check that
was being used in earlier reviews of that commit. The size
verification makes this check unneeded since we're making sure
that guestArea will always be at least equal to ppc64AlignSize.
Fixes: 5540acb9a2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is the only instance of g_autofree change applicable for
qemu_agent.c
Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This fixes a FreeBSD build error from
commit a11a0e6e84
Author: Rafael Fonseca <r4f4rfs@gmail.com>
Date: Tue Mar 24 17:14:30 2020 +0100
bhyve: move video default logic to driver
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Move the liveness check prior to the capability check. If the VM is
offline the capabilities are not initialized and thus we'd report the
wrong error.
https://bugzilla.redhat.com/show_bug.cgi?id=1812531
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The code attempting to clean up after a failed pull mode backup job
wrongly entered monitor but didn't clean up nor exit monitor due to a
logic bug. Fix the condition.
Introduced in a1521f84a5https://bugzilla.redhat.com/show_bug.cgi?id=1817327
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Move the liveness check prior to the capability check. If the VM is
offline the capabilities are not initialized and thus we'd report the
wrong error.
https://bugzilla.redhat.com/show_bug.cgi?id=1812531
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Host-model CPU definitions (and domain capabilities) will use the
original CPU models (without noTSX in their name) and explicitly disable
hle and rtm features. This way domains with host-model CPUs will be
migratable even to older versions of libvirt which do not support the
noTSX model variants.
The new models will be advertised in host capabilities and they may
be used explicitly with custom CPUs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
The element specifies whether a particular CPU model can be used when
creating a CPU definition from raw CPUID/MSR data. The @host attribute
determines whether the CPU model can be used (host='on') for creating
CPU definition for host capabilities. Usability of the model for domain
capabilities and host-model CPU definitions is controlled by the @guest
attribute.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
One of the mitigation methods for TAA[1] is to disable TSX
support on the host system. Linux added a mechanism to disable
TSX globally through the kernel command line, and many Linux
distributions now default to tsx=off. This makes existing CPU
models that have HLE and RTM enabled not usable anymore.
Add new versions of all CPU models that have the HLE and RTM
features enabled, that can be used when TSX is disabled in the
host system.
On systems disabling the features without those types defined
in cpu-maps users end up without modern CPU types in the list
of usable CPUs to use in the likes of virsh domcapabilities
or tools higher in the stack like virt-manager.
This adds:
-Cascadelake-Server-noTSX
-Icelake-Client-noTSX
-Icelake-Server-noTSX
-Skylake-Server-noTSX-IBRS
-Skylake-Client-noTSX-IBRS
Introduced in QEMU by commit v4.2.0-rc2-3-g9ab2237f19 (function)
and commit v4.2.0-rc2-4-g02fa60d101 (names)
References:
[1] TAA, TSX asynchronous Abort:
https://software.intel.com/security-software-guidance/insights/deep-dive-intel-transactional-synchronization-extensions-intel-tsx-asynchronous-aborthttps://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/tsx_async_abort.html
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1853200
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Message-Id: <20200310104806.2723-2-christian.ehrhardt@canonical.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The PMU feature is enabled by default in ppc64 guests and can't
be disabled via Libvirt or QEMU [1]. The current PMU feature
implementation does not allow PMU to enabled or disabled in the
ppc64 guest. Declaring the PMU feature will make the 'pmu'
property to be passed on to QEMU, but this property isn't
available for ppc64:
qemu-kvm: can't apply global host-powerpc64-cpu.pmu=on: Property '.pmu' not found
A similar error is thrown when trying to disable the PMU.
This patch standardizes the PMU handling for ppc64 guests by:
- throwing an error if the user attempts to set the feature to
'off', given that this feature can't be turned off at all;
- allowing the feature to be declared as 'on' in the domain XML.
This is done by skipping ppc64 guests when creating the command
line for this feature.
[1] https://www.redhat.com/archives/libvir-list/2020-March/msg00874.html
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Hyperv features are supported by both x86 and aarch64. The <hyperv/>
declaration in the XML by itself is benign to other architectures,
but any of its 14 current features will break QEMU with an error
like this (from ppc64):
qemu-kvm: Expected key=value format, found hv_relaxed
This is a more extreme case than the one for apic eoi because we
would need an extra 'switch' statement, with all current Hyperv
features in the body of qemuDomainDefValidateFeatures(), to
check if the user attempted to activate any of them. It's easier to
simply fail to launch with any 'hyperv' declaration in the XML for
every arch which is not x86 and aarch64.
A fair disclaimer about Windows and PowerPC: the last Windows version
that ran in the architecture is the hall of famer Windows NT 4.0,
launched in 1996 and with end of extended support for the Server
version in 2004 [1]. I am acknowledging that there might be Windows
NT 4.0 users running in PowerPC, but not enough people running it
under KVM/QEMU to justify Libvirt allowing 'hyperv' to exist in the
domain XML of ppc64 domains.
[1] https://en.wikipedia.org/wiki/Windows_NT_4.0
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The 'pvspinlock' feature is x86 only. The "<pvspinlock/>" declaration
will always have a value 'on' or 'off', and both will break QEMU when
launching non-x86 guests. This is the error message for
"<pvspinlock state='on'/>" when running a ppc64 guest:
qemu-kvm: Expected key=value format, found +kvm_pv_unhalt
A similar error message is thrown for "<pvspinlock state='off'/>".
This patch prevents non-x86 guests from launching with any
pvspinlock setting with a more informative error message:
error: unsupported configuration: The 'pvspinlock' feature is not
supported for architecture 'ppc64' or machine type 'pseries'
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The "<apic/>" feature, although it's available only for x86 guests,
can be declared in the domain XML of other archs without errors.
But setting its 'eoi' attribute will break QEMU. For "<apic eoi='on'/>",
in a ppc64 guest:
qemu-kvm: Expected key=value format, found +kvm_pv_eoi
A similar error happens with eoi='off'.
One can argue that it's better to simply forbid launching non-x86
guests with "<apic/>" declared in the XML - it is a feature that
the architecture doesn't support and this would make it clearer
about it. This is sensible, but there are non-x86 guests that are
running with "<apic/>" declared in the domain (and A LOT of guests
running with "<acpi/>" for that matter, probably reminiscent of x86
templates that were reused for other archs) that will stop working if we
go this route.
A more subtle approach is to detect if the 'eoi' element is being set
for non-x86 guests and warn the user about it with a better error
message than the one QEMU provides. This is the new error message
when any value is set for the 'eoi' element in a ppc64 XML:
error: unsupported configuration: The 'eoi' attribute of the 'apic'
feature is not supported for architecture 'ppc64' or machine type
'pseries'.
https://bugzilla.redhat.com/show_bug.cgi?id=1236440
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Don't report cases when the guest information is not requested
explicitly and not present either.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use qemuAgentCommandFull so that callers of qemuAgentGetFSInfo can
suppress error reports if the function is not supported by the guest
agent.
Since this patch removes the last use of
qemuAgentErrorCommandUnsupported the whole function is deleted as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use qemuAgentCommandFull so that callers of qemuAgentGetTimezone can
suppress error reports if the function is not supported by the guest
agent.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use qemuAgentCommandFull so that callers of qemuAgentGetOSInfo can
suppress error reports if the function is not supported by the guest
agent.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use qemuAgentCommandFull so that callers of qemuAgentGetUsers can
suppress error reports if the function is not supported by the guest
agent.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use qemuAgentCommandFull in qemuAgentGetHostname so that we can suppress
error reports if the caller will not require them. Callers for now
always require error reporting but will be fixed later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Return 0 on success to match the documentation. The callers only check
for negative values.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In some cases we don't want to log errors if an agent command is
unsupported. Wire it up into qemuAgentCheckError via qemuAgentCommandFull
and provide a thin wrapper (qemuAgentCommand) to prevent having to fix
all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'qemuDomainGetGuestInfoCheckSupport' despite its name was not checking
whether the info types are supported. Convert the function to return
integers and include the check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The logic has been moved to the individual drivers.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The logic setting a device default should be in the post parse function
of individual driver code.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The logic setting a device default should be in the post parse function
of individual driver code.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The logic setting a device default should be in the post parse function
of individual driver code.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The logic setting a device default should be in the post parse function
of individual driver code.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The logic setting a device default should be in the post parse function
of individual driver code.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The logic setting a device default should be in the post parse function
of individual driver code.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Unfortunately, advisory record locking lose the lock if any fd refering
to the file is closed. There doesn't seem to be a way to preserve the
lock atomically. We could eventually retake the lock if low pidfilefd
is required.
This fixes processes being leaked, as they are not killed in
virPidFileForceCleanupPath() if the lock can be taken. Here also, we may
consider this is not good enough, as a process may leak by simply
closing the pidfilefd.
Fixes commit d146105f1e ("virCommand:
Actually acquire pidfile instead of just writing it")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The WIP specification is hosted on slirp wiki at this point:
https://gitlab.freedesktop.org/slirp/libslirp/-/wikis/Slirp-Helper
We would need more feedback from various parties (including libvirt,
podman, and other developpers) before declaring a frozen version.
So for now, follow it, and feedback welcome!
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When the helper supports DBus, connect it to the bus and set its ID.
If the helper supports migration, register its ID to the list of
dbus-vmstate ID to migrate, and specify --dbus-incoming when
restoring the VM.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Helper processes may have their state migrated with QEMU data stream
thanks to the QEMU "dbus-vmstate".
libvirt maintains the list of helpers to be migrated. The
"dbus-vmstate" is added when required, and given the list of helper
Ids that must be migrated, on save & load sides.
See also:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus-vmstate.rst
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This avoids trying to start a dbus-daemon when its already running.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add a unit to start & stop a private dbus-daemon.
The daemon is meant to be started on demand, and associated with a
QEMU process. It should be stopped when the QEMU process is stopped.
The current policy is permissive like a session bus. Stricter
policies can be added later, following recommendations from:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus.rst
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This code was based on a per-helper instance and peer-to-peer
connections. The code that landed in qemu master for v5.0 is relying
on a single instance and DBus bus.
Instead of trying to adapt the existing dbus-vmstate code, let's
remove it and resubmit. That should make reviewing easier.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In the network driver code there's networkKillDaemon() which is
the same as virProcessKillPainfully(). Replace the former with
the later and drop what becomes unused function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Now, that we know that the virtiofsd will have the pidfile open
and locked we can use virPidFileForceCleanupPath() to kill it and
unlink the pidfile.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Now, that we know that the slirp helper will have the pidfile
open and locked we can use virPidFileForceCleanupPath() to kill
it and unlink the pidfile.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Now, that our virCommandSetPidFile() is more intelligent we don't
need to rely on the daemon to create and lock the pidfile and use
virCommandSetPidFile() at the same time.
NOTE that as advertised in the previous commit, this was
temporarily broken, because both virCommand and
qemuProcessStartManagedPRDaemon() would try to lock the pidfile.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Our virCommand module allows us to set a pidfile for commands we
want to spawn. The caller constructs the string of pidfile path
and then uses virCommandSetPidFile() to tell the module to write
the pidfile once the command is ran. This usually works, but has
two flaws:
1) the child process does not hold the pidfile open & locked.
Therefore, the caller (or anybody else) can't use our fancy
virPidFileForceCleanupPath() function to kill the command
afterwards. Also, for everybody else on the system it's
needlessly harder to check if the pid from the pidfile is still
alive or not.
2) if the caller ever makes a mistake and passes the same pidfile
path for two different commands, the start of the second command
will overwrite the pidfile even though the first command might
still be running.
NOTE that this temporarily renders some command spawning
unusable, specifically those code patterns where both
virCommandSetPidFile() is used together with instructing spawned
command to acquire pidfile itself. Fortunately, there is only one
occurrence of such pattern and it is in
qemuProcessStartManagedPRDaemon(). This is fixed in next commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Our code allows snapshots of NVMe based disks which means we create
overlay file with a 'json:{}' pseudo-uri refering to the NVME device.
Our parser code doesn't handle them though. Add the parser and test it
via the XML->json->XML round-trip and reference data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Format cookies into the backing store string without encryption as they
will not be visible on the command line when formatting a 'target' only
string. In cases when cookies or other options are used we must use the
JSON format rather than pure URI.
Add tests to validate the scenario.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce qemuBlockStorageSourceGetCookieString which does the
concatenation so that we can reuse it later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU requires an extra wrapper object where only the "file" member is
populated. This is basically a placeholder for establishing the format
layer. We did the same in qemuDiskSourceGetProps for the old-school
JSON usage with -drive but forgot to adopt this for -blockdev.
https://bugzilla.redhat.com/show_bug.cgi?id=1804617
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemublocktest showed that we don't add the "fat:" prefix for directory
storage when formatting the backing store string. While it's unlikely to
be used it's simple enough to actually implement the support rather than
trying to forbid it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add support for pretty-printing of the JSON variant of the output for
consumption in tests. All current callers pass 'false'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
VIR_TRISTATE_BOOL_ABSENT which maps to the 'default' string would not be
parsed back, so we shouldn't format it either.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
While 'namespace' is not a reserved word in C, it is in C++. Our
compilers are happy with it but syntax-hilighting in some editors
hilights is as a keyword. Rename it to prevent confusion.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virHostCPUGetStatsLinux walks through every cpu in /proc/stat until it
finds cpu%cpuNum that matches with the requested cpu.
If none is found it logs the error but it should return -1, instead of 0.
Otherwise virsh nodecpustats --cpu <invalid cpu number> and API bindings
don't fail properly, printing a blank line instead of an error message.
This patch also includes an additional test for virhostcputest to avoid
this regression to happen again in the future.
Fixes: 93af79fba3
Reported-by: Satheesh Rajendran <satheera@in.ibm.com>
Signed-off-by: Mauro S. M. Rodrigues <maurosr@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
There is no need to repeat the shortName, since it's
already present in the directory path.
Also use just 'fs' instead of 'virtiofsd'.
https://bugzilla.redhat.com/show_bug.cgi?id=1816577
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Suggested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Using the 'uuid' element for ppc64 NVDIMM memory added in the
previous patch, use it in qemuBuildMemoryDeviceStr() to pass
it over to QEMU.
Another ppc64 restriction is the necessity of a mem->labelsize,
given than ppc64 only support label-area backed NVDIMMs.
Finally, we don't want ppc64 NVDIMMs to align up due to the
high risk of going beyond the end of file with a 256MiB
increment that the user didn't predict. Align it down
instead. If target size is less than the minimum of
256MiB + labelsize, error out since QEMU will error out
if we attempt to round it up to the minimum.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
ppc64 NVDIMM support was implemented in QEMU by commit [1].
The support is similar to what x86 already does, aside from
an extra 'uuid' element.
This patch introduces a new optional 'uuid' element for the
NVDIMM memory model. This element behaves like the 'uuid'
element of the domain definition - if absent, we'll create
a new one, otherwise use the one provided by the XML.
The 'uuid' element is exclusive to pseries guests and are
unavailable for other architectures.
Next patch will use this new element to add NVDIMM support
for ppc64.
[1] ee3a71e366
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The functionality is now provided by glib's GKeyFile.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace libvirt's virKeyFile by glib's GKeyFile.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function changes the amount of time that libvirt waits for a
response from the guest agent for all guest agent commands. Since this
is a configuration change, it should not be allowed on read-only
connections.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The agent 'guest-sync' command historically had a 5s response timeout
which was different from other agent commands, which waited forever.
When we added the ability to customize the response timeout for guest
agent commands, we intended to continue to use 5s for 'guest-sync' when
the user specified a response timeout greater than 5s, and use the
user-specified timeout if it was below 5s. Unfortunately, when
attempting to determine whether the user-specified timeout was less than
5s, we were comparing against an enum value of
VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT (which is -1) rather than against
the actual time value that it represented (5).
This change makes it so that 'guest-sync' now uses the user-specified
tiemout if it is less than 5s.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use existing function built for this exact purpose.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When a QEMU process dies in the middle of a hotplug, then we fail
to restore the seclabels on the device. The problem is that if
the thread doing hotplug locks the domain object first and thus
blocks the thread that wants to do qemuProcessStop(), the
seclabel cleanup code will see vm->pid still set and mount
namespace used and therefore try to enter the namespace
represented by the PID. But the PID is gone really and thus
entering will fail and no restore is done. What we can do is to
try enter the namespace (if requested to do so) but if entering
fails, fall back to no NS mode.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1814481
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
When running a function in a forked child, so far the only thing
we could report is exit status of the child and the error
message. However, it may be beneficial to the caller to know the
actual error that happened in the child.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
The @src is not always a file. It may also be a directory (for
instance qemuDomainCreateDeviceRecursive() assumes that) - even
though it doesn't happen usually. Anyway, mount() can mount only
a dir onto a dir and a file onto a file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
The @devPath variable is not modifiable. It merely just points to
string containing path where private devtmpfs is being
constructed. Make it const so it doesn't look weird that it's not
freed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
If building namespace fails somewhere in the middle (that is some
files exists under devMountsSavePath[i]), then plain rmdir() is
not enough to remove dir. Umount the temp location and use
virFileDeleteTree() to remove the directory.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
The virFileMakePathWithMode() which is our recursive version of
mkdir() fails, it simply just returns a negative value with errno
set. No error is reported (as compared to virFileTouch() for
instance).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
For the few instances where we'd generate an array in dotted syntax we
should be able to parse it back. Add another step in deflattening of the
dotted syntax which reconstructs the arrays so that the backing store
parser can parse it.
https://bugzilla.redhat.com/show_bug.cgi?id=1466177
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Extract the code so that there's a clean separation once we'll want do
do other steps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory handling to remove the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virBitmapNewEmpty can't fail now so we can make it obvious and fix all
callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virStorageEncryptionSecretPtr may have a string inside it, thus we must
copy the string too. Use virSecretLookupDefCopy to do that.
Caused by non-obvious code introduced in 756b46ddd2 and later 47e88b33b
which added a string that needed to be copied.
https://bugzilla.redhat.com/show_bug.cgi?id=1814923
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function always returns succes so there's no need for a return
value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemuBlockStorageSourceGetFormatRawProps aggregated both formats but
since we now have props specific for either of those formats it's
unwanted to aggregate the code such way. Split out the 'luks' props
formatter into qemuBlockStorageSourceGetFormatLUKSProps.
The wrong separation demonstrates istself on formatting of the 'size'
and 'offset' attributes for the 'luks' driver which does not conform
to the qapi schema.
https://bugzilla.redhat.com/show_bug.cgi?id=1814975
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'luks' driver in qemu is as any other non-raw format driver and thus
doesn't support the properties for 'slice'. Since libvirt considers
luks files to be raw+encryption we need to special case them when
dealing with the slice.
https://bugzilla.redhat.com/show_bug.cgi?id=1814975
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce qemuBlockStorageSourceNeedsStorageSliceLayer which will hold
the decision logic and fix all places that open-code it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
A fixup to patch [1]. We need to reset await_event in all
error paths.
[1] 52532073d : qemu: remove redundant needReply argument of qemuAgentCommand
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In one of my previous commits I've introduced code that creates
all devices for given (possible) multipath target. But I've made
a mistake there - the code accesses 'next->path' without checking
if the disk source is local. Note that the 'next->path' is
NULL/doesn't make sense for VIR_STORAGE_TYPE_NVME.
Fixes: a30078cb83
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1814947
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It seems like CD-ROMs may have no 'fileName' property specified in case
there is nothing configured as attachment for the drive. Hence, make
sure that virVMXParseDisk() do not consider it mandatory anymore,
considering it an empty block cdrom device. Sadly virVMXParseDisk() is
used also to parse disk and floppies, so make sure that a NULL fileName
is handled in cdrom- and floppy-related paths.
https://bugzilla.redhat.com/show_bug.cgi?id=1808610
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Move earlier the checks for skipping a hard disk when parsing a CD-DROM,
and for skipping a CD-ROM when parsing a hard disk. This should have no
behaviour changes, and avoids to add repeated checks in following
commits.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Many calls of qemuMonitorDelObject don't actually check the return value
or report the error from the object deletion itself since they are on
cleanup paths. In some cases this can lead to reporting of spurious
errors e.g. when qemuMonitorDelObject is used to clean up a possibly
pre-existing objects.
Add a new argument for qemuMonitorDelObject which controls whether
the internals report errors from qemu and fix all callers accordingly.
Note that some of the cases on device unplug which check the error code
don't in fact propagate the error to the user, but in this case it is
important to add the log entry anyways for tracing that the device
deletion failed.
https://bugzilla.redhat.com/show_bug.cgi?id=1784040
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In some cases we'll need to check whether there was an error but avoid
reporting an actual libvirt error. Rename qemuMonitorJSONCheckError to
qemuMonitorJSONCheckErrorFull with a new flag to suppress the error
reporting and add a wrapper with the original name so that callers don't
need to be fixed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use 'g_autoptr' and remove the cleanup label and ret variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When changing media we'd attempt to remove the managed pr daemon even if
neither of the images involved in the media change used it. This caused
libvirtd to log a spurious error:
2020-03-18 01:41:19.832+0000: 643207: error : qemuMonitorJSONCheckError:412 : internal error: unable to execute QEMU command 'object-del': object 'pr-helper0' not found
With this patch we completely avoid calling the deletion code.
https://bugzilla.redhat.com/show_bug.cgi?id=1814486
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The loop which checks whether the vcpus are in proper configuration for
the requested hot(un)plug skips the first modified vcpu. This means
that 'firstvcpu' which is used to print the error message in case the
configuration is not suitable would never point to the first modified
vcpu.
In cases such as:
<vcpu placement='auto' current='5'>8</vcpu>
<vcpus>
<vcpu id='0' enabled='yes' hotpluggable='no'/>
<vcpu id='1' enabled='yes' hotpluggable='no'/>
<vcpu id='2' enabled='yes' hotpluggable='no'/>
<vcpu id='3' enabled='yes' hotpluggable='no'/>
<vcpu id='4' enabled='yes' hotpluggable='no'/>
<vcpu id='5' enabled='no' hotpluggable='yes'/>
<vcpu id='6' enabled='no' hotpluggable='yes'/>
<vcpu id='7' enabled='no' hotpluggable='yes'/>
</vcpus>
# virsh setvcpu --config --disable upstream 1
error: invalid argument: vcpu '-1' can't be modified as it is followed by non-hotpluggable online vcpus
After this fix the proper vcpu is reported in the error message:
# virsh setvcpu --config --disable upstream 1
error: invalid argument: vcpu '1' can't be modified as it is followed by non-hotpluggable online vcpu
https://bugzilla.redhat.com/show_bug.cgi?id=1611061
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
So far, when using the qemu:///embed driver, management
applications can't chose whether they want to register their
domains in machined or not. While having that option is certainly
desired, it will require more work. What we can do meanwhile is
to generate names that include part of hash of the root
directory. This is to ensure that if two applications using
different roots but the same domain name (and ID) start the
domain no clashing name for machined is generated.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
When initializing virQEMUDriverConfig structure we are given the
root directory for possible embed connection. Save it for future
use. While we could get it later from @uri member, it's not as
easy as dereferencing a pointer (virURIParse() +
virURIGetParam() + error reporting).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The check attribute was completely ignored for host-passthrough CPUs
even if they explicitly requested some features to be enabled. For
example, a domain with the following CPU definition
<cpu mode='host-passthrough' check='full'>
<feature policy='require' name='svm'/>
</cpu>
would happily start even when 'svm' cannot be enabled.
Let's call virCPUArchUpdateLive for host-passthrough CPUs with
VIR_CPU_CHECK_FULL to make sure the architecture specific code can
validate the provided virtual CPU against the desired definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1515677
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Adding more checks into the existing if statements would turn them into
an unreadable mess.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The updateLive CPU sub-driver function is supposed to be called only for
a subset of CPU definitions. Let's make it more obvious by turning a
negative test and return into a positive check.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit is related to RTC timer device too. HPET is being shared
from host device through `localtime` clock. This timer is available
creating a new timer using `hpet` name.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This commit share host Real Time Clock device (rtc) into LXC containers
to support hardware clock. This should be available setting up a `rtc`
timer under clock section. Since this option is not emulated, it should
be available only for `localtime` clock. This option should be readonly
due to security reasons.
Before:
root# hwclock --verbose
hwclock from util-linux 2.32.1
System Time: 1581877557.598365
Trying to open: /dev/rtc0
Trying to open: /dev/rtc
Trying to open: /dev/misc/rtc
No usable clock interface found.
hwclock: Cannot access the Hardware Clock via any known method.
Now:
root# hwclock
2020-02-16 18:23:55.374134+00:00
root# hwclock -w
hwclock: ioctl(RTC_SET_TIME) to /dev/rtc to set the time failed:
Permission denied
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function repeatedly checked the first element rather than iterating
through the array.
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tested-by: Daniel P. Berrangé <berrange@redhat.com>
Allocate space also for the terminating NULL.
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tested-by: Daniel P. Berrangé <berrange@redhat.com>
Some of the node device APIs are a little odd because they accept a
virNodeDevicePtr object but are still implemented by the virt drivers.
The first thing the virt drivers need to do is get the XML config
associated with the node device, and that means talking to the node
device driver.
This worked previously because with monolithic libvirtd, both the
virt driver and node device driver were in the same daemon and thus
a single virConnectPtr can talk to both drivers.
With the split daemon world though, the virNodeDevicePtr passed into
the APIs is associated with the QEMU driver virConnectPtr, which has
no ability to invoke APIs against the node device driver. We must thus
get a duplicate virNodeDevicePtr object which is associated with a
virConnectPtr for the node device driver.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The node device APIs are a little unusual because we don't use a
"remote_nonnull_node_device" object on the wire, instead we just
have a "remote_string" for the device name. This meant dispatcher
code generation needed special cases. In doing so we mistakenly
used the virNodeDeviceLookupByName() API which gets dispatched
into the driver, instead of get_nonnull_node_device() which
directly populates a virNodeDevicePtr object.
This wasn't a problem with monolithic libvirtd, as the
virNodeDeviceLookupByName() API call was trivially satisfied
by the registered driver, albeit with an extra (undesirable)
authentication check. With the split daemons, the call to
virNodeDeviceLookupByName() fails in virtqemud, because the
node device driver obviously doesn't exist in that daemon.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Despite their names, the following APIs:
virNodeDeviceDettach
virNodeDeviceDetachFlags
virNodeDeviceReAttach
virNodeDeviceReset
are all handled by the virt drivers, not the node device driver.
A bug in the RPC generator meant that these APIs were sent to
the nodedev driver for handling. This caused breakage with the
split daemons, since nothing was available to process them.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 7b79ee2f78 makes assumptions about die_id parsing in
the sysfs that aren't true for Power hosts. In both Power8
and Power9, running 5.6 and 4.18 kernel respectively,
'die_id' is set to -1:
$ cat /sys/devices/system/cpu/cpu0/topology/die_id
-1
This breaks virHostCPUGetDie() parsing because it is trying to
retrieve an unsigned integer, causing problems during VM start:
virFileReadValueUint:4128 : internal error: Invalid unsigned integer
value '-1' in file '/sys/devices/system/cpu/cpu0/topology/die_id'
This isn't necessarily a PowerPC only behavior. Linux kernel commit
0e344d8c70 added in the former Documentation/cputopology.txt, now
Documentation/admin-guide/cputopology.rst, that:
To be consistent on all architectures, include/linux/topology.h
provides default definitions for any of the above macros that are
not defined by include/asm-XXX/topology.h:
1) topology_physical_package_id: -1
2) topology_die_id: -1
(...)
This means that it might be expected that an architecture that
does not implement the die_id element will mark it as -1 in
sysfs.
It is not required to change die_id implementation from uInt to
Int because of that. Instead, let's change the parsing of the
die_id in virHostCPUGetDie() to read an integer value and, in
case it's -1, default it to zero like in case of file not found.
This is enough to solve the issue Power hosts are experiencing.
Fixes: 7b79ee2f78
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
During startup the udev node device driver impl uses a background thread
to populate the list of devices to avoid blocking the daemon startup
entirely. There is no synchronization to the public APIs, so it is
possible for an application to start calling APIs before the device
initialization is complete.
This was not a problem in the old approach where libvirtd was started
on boot, as initialization would easily complete before any APIs were
called.
With the use of socket activation, however, APIs are invoked from the
very moment the daemon starts. This is easily seen by doing a
'virsh -c nodedev:///system list'
the first time it runs it will only show one or two devices. The second
time it runs it will show all devices. The solution is to introduce a
flag and condition variable for APIs to synchronize against before
returning any data.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Don't rely on error check and assign hostname only when non-NULL.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If a block-commit fails we should at least re-enable the bitmaps so that
the operation can be re-tried.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Merge the bitmaps into base of the block commit after the job finishes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Active layer block commit makes the 'base' image the new top image of
the disk after it finishes. This means that all bitmap operations need
to be handled prior to this happening as we'd lose writes otherwise.
The ideal place is to handle it when pivoting to the new image as only
guest-writes would be happening after this point.
Use qemuBlockBitmapsHandleCommitFinish to calculate the merging
transaction.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
On start of the commit job, we need to disable any active bitmap in the
base. Use qemuBlockBitmapsHandleCommitStart to calculate which and call
the appropriate QMP APIs. We use blockdev-reopen to make the 'base'
writable to disable the bitmaps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Add an argument to qemuBlockJobDiskNewCommit to propagate the list of
disabled bitmaps into the job data structure.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
qemuBlockBitmapsHandleCommitStart prepares for disabling the bitmaps in
the 'base' of the commit job so that the bitmaps are not dirtied by the
commit job. This needs to be done prior to start of the commit job.
qemuBlockBitmapsHandleCommitFinish then calculates the necessary merges
that agregate all the bitmaps between the commited images and write them
into the base bitmap.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Starting a commit job will require disabling bitmaps in the base image
so that they are not dirtied by the commit job. We need to store a list
of the bitmaps so that we can later re-enable them.
Add a field and status XML handling code as well as a test.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
I'll be adding more fields to care about so splitting the code out will
be better long-term.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
I'll be adding more fields to care about so splitting the code out will
be better long-term.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Since capabilities are not present for inactive VMs we'd report that we
don't support '--delete' or committing while checkpoints exist rather
than the proper error.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The code deleting checkpoints needs the name of the parent checkpoint's
disk's bitmap but was using the disk alias instead. This would create
wrong bitmaps after deleting some checkpoints.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Qemu's bitmap APIs don't reopen the appropriate images read-write for
modification. It's libvirt's duty to reopen them via blockdev-reopen
if we wish to modify the bitmaps.
Use the new helpers to reopen the images for bitmap manipulation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Introduce a set of helpers to call blockdev-reopen in certain scenarios
Libvirt will use the QMP command to turn certain members of the backing
chain read-write for bitmap manipulation and we'll also want to use it
to replace/install the backing chain of a qcow2 format node.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
This capability will be asserted once qemu stabilizes 'blockdev-reopen'.
For now we just add the capability so that we can introduce some code
that will use the reopening call. This will show our willingness to
adopt use of reopen and help qemu developers stabilize it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The logic for querying hotpluggable CPUs needs to sort the list
of CPUs returned by QEMU. Unfortunately our sorting method failed
to use the die_id field, so CPUs were not correctly sorted.
This is seen when configuring a guest with partially populated
CPUs
<vcpu placement='static' current='1'>16</vcpu>
<cpu...>
<topology sockets='4' dies='2' cores='1' threads='2'/>
</cpu>
Then trying to start it would fail:
# virsh -c qemu:///system start demo
error: Failed to start domain demo
error: internal error: qemu didn't report thread id for vcpu '0'
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We currently don't model the 'ssh' protocol properties properly and
since it seems impossible for now (agent path passed via environment
variable). To allow libguestfs to work as it used in pre-blockdev era we
must carry the properties over to the command line. For this instance we
just store it internally and format it back.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
libguestfs abuses a quirk of qemu's parser to accept also other variants
of the 'sslverify' field which would be valid on the command line but
are not documented in the QMP schema.
If we encounter the 'off' string instead of an boolean handle it rather
than erroring out to continue support of pre-blockdev configurations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add support for parsing the recently added fields from backing file
pseudo-protocol strings.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pass the alias of the secret object holding the cookie data as
'cookie-secret' to qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Implement both commandline support and hotplug by adding the http cookie
handling to 'qemuBlockStorageSourceAttachData' handling functions for
it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU's curl driver requires the cookies concatenated and allows themi to
be passed in via a secret. Prepare the value for the secret and encrypt
it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The http cookies can have potentially sensitive values and thus should
not be leaked into the command line. This means that we'll need to
instantiate a 'secret' object in qemu to pass the value encrypted.
This patch adds infrastructure for storing of the alias in the status
XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow disabling of SSL certificate validation for HTTPS and FTPS drives
in qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ensure that the new fields are allowed only when -blockdev is used or
when they are in the detected part of the backing chain where qemu will
handle them internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Some disk backends support configuring the readahead buffer or timeout
for requests. Add the knobs to the XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add possibility to specify one or more cookies for http based disks.
This patch adds the config parser, storage and validation of the
cookies.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
To allow turning off verification of SSL cerificates add a new element
<ssl> to the disk source XML which will allow configuring the validation
process using the 'verify' attribute.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are two last callers of this function. Replace them by
qemuAliasForSecret and delete qemuDomainGetSecretAESAlias.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Originally there was only the secret for authentication so we didn't use
any suffix to tell it apart. With the introduction of encryption we
added a 'luks' suffix for the encryption secrets. Since encryption is
really generic and authentication is not the only secret modify the
aliases for the secrets to better describe what they are used for.
This is possible as we store the disk secrets in the status XML thus
only new machines will use the new secrets.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Replace qemuDomainGetSecretAESAlias by the new function so that we can
reuse qemuDomainSecretAESSetupFromSecret also for setting up other kinds
of objects.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently we don't have infrastructure to remember the secret aliases
for hostdevs. Since an upcoming patch is going to change aliases for
the disks, initialize the iscsi hostdevs separately so that we can keep
the alias. At the same time let's use qemuAliasForSecret instead of
qemuDomainGetSecretAESAlias when unplugging the iscsi hostdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In order to be able to change the function generating the alias and thus
also the aliases itself, we must hardcode the old format for the case of
upgrading form libvirt which didn't record them in the status XML yet.
Note that this code path is tested by
'tests/qemustatusxml2xmldata/disk-secinfo-upgrade-in.xml'
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The naming of the variables was tied to what they are used for not what
the alias represents. Since we'll need to use some of the aliases for
another type of secrets fix the name so that it makes sense.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuAliasForSecret is meant as a replacement qemuDomainGetSecretAESAlias
with saner API. The sub-type we are creating the alias for is passed in
as a string rather than the unflexible 'isLuks' boolean.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Replace it by a direct call to qemuDomainSecretAESSetupFromSecret.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Split out the lookup of the secret from the secret driver into
qemuDomainSecretAESSetupFromSecret so that we can also instantiate
secret objects in qemu with data from other sources.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rather than passing in an empty qemuDomainSecretInfoPtr allocate it
in this function and return it. This is done by absorbing the check from
qemuDomainSecretInfoNew and removing the internals of
qemuDomainSecretInfoNew.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use g_autofree for the ciphertext and init vector as they are not
secret and thus don't have to be cleared and use g_new0 to allocate the
iv for parity.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The comment mentioned that the function resets migration params, but
that is not true as of commit eb54cb473a
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use g_autofree instead of VIR_FREE and delete the comment mentioning
possible failure to allocate memory.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Using a double pointer prevents the function from being used as the
automatic cleanup function for the given type.
Remove the double pointer use by replacing the calls with
g_clear_pointer which ensures that the pointer is cleared.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The server needs to use CA certificate, CRL, server certificate/key to
complete the TLS handshake. If these files change, we needed to restart
libvirtd for them to take effect. This API can update the TLS context
*ONLINE* without restarting libvirtd.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com>
Signed-off-by: Wu Qingliang <wuqingliang4@huawei.com>
Prevent the handshake function from reading 'tlsCtxt' while
updating 'tlsCtxt'.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com>
Signed-off-by: Wu Qingliang <wuqingliang4@huawei.com>
Add an API to update server's tls context.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com>
Signed-off-by: Wu Qingliang <wuqingliang4@huawei.com>
Now that we use g_strerror exclusively, remove this unused
function.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
oVirt used a quirk in the pre-blockdev semantics of drive-mirror which
opened the backing chain of the mirror destination only once
'block-job-complete' was called.
Our introduction of blockdev made qemu open the backing chain images
right at the start of the job. This broke oVirt's usage of this API
because they copy the data into the backing chain during the time the
block copy job is running.
Re-introduce late open of the backing chain if qemu allows us to use
blockdev-snapshot on write-only nodes as it can be used to install the
backing chain even for an existing image now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The capability is based on qemu's support of using blockdev-snapshot to
install backing chain also for images which are in use by a block-copy
job.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
For a long time we've masked out VIR_DOMAIN_BLOCK_COPY_SHALLOW if
there's no backing chain for the copied disk to simplify the code.
One of the refactors of the block copy code caused that we no longer
update the 'flags' variable just the local copies. This was okay until
in ccd4228aff we started storing the job flags in the block job data.
Given that we modify how we call qemu we also should modify @flags so
that the correct value is recorded in the block job data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Move the check whether the job is already synchronised to the beginning
of the function so that we don't try to do some of the steps necessary
for pivoting prior to actually wanting to pivot.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The stub impl of virGetDeviceID just returns ENOSYS and does not
initialize the min/maj output parameters. This lead to a false
positive warning on mingw about possible use of uninitialized
variables.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
According to the linked BZ, machined expects either valid
hostname or valid FQDN (see systemd commit
v239-3092-gd65652f1f2). While in case of multiple dots, a
trailing one doesn't violate FQDN, it does violate the rule in
case of something simple, like "domain.". But it's safe to remove
it in both cases.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1808499
Fixes: 45464db8ba
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'nfs' variable was set to -1 or -2 on agent failure. Cleanup then tried
to free 'nfs' elements of the array which resulted into a crash.
Make 'nfs' size_t and assign it only on successful agent call.
https://bugzilla.redhat.com/show_bug.cgi?id=1812965
Broken by commit 599ae372d8
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The only caller doesn't check the value and also there are no real
errors to report anyways.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virQEMUCaps structure has usedQMP member which in the past
used to tell if qemu we are dealing with is capable of QMP. Well,
we don't support HMP anymore (minus a few HMP passthrough
commands, which are wrapped into QMP anyways) and the member is
not used really.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
needReply added in [1] looks redundant. Indeed it is set to false only
when mon->await_event is set too (the only exception qemuAgentFSTrim
which is mistaken).
However it fixes the issue when qemuAgentCommand exits on error path and
mon->await_event is not reset. Let's instead reset mon->await_event properly.
Also remove "Woken up by event" debug message as it can be misleading.
We can get it also if monitor is closed due to serial changed event
currently. Anyway both qemuAgentClose and qemuAgentNotifyEvent log
itself.
[1] qemu: make sure agent returns error when required data are missing
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Sync was introduced in [1] to check for ga presence. This
check is racy but in the era before serial events are available
there was not better solution I guess.
In case we have the events the sync function is different. It allows us
to flush stateless ga channel from remnants of previous communications.
But we need to do it only once. Until we get timeout on issued command
channel state is ok.
[1] qemu_agent: Issue guest-sync prior to every command
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If a disk has persistent reservations enabled, qemu-pr-helper
might open not only /dev/mapper/control but also individual
targets of the multipath device. We are already querying for them
in CGroups, but now we have to create them in the namespace too.
This was brought up in [1].
1: https://bugzilla.redhat.com/show_bug.cgi?id=1711045#c61
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Lin Ma <LMa@suse.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
This converts the QEMU agent APIs to use the per-VM
event loop, which involves switching from virEvent APIs
to GMainContext / GSource APIs.
A GSocket is used as a convenient way to create a GSource
for a socket, but is not yet used for actual I/O.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We are dealing with the QEMU agent, not the monitor.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This converts the QEMU monitor APIs to use the per-VM
event loop, which involves switching from virEvent APIs
to GMainContext / GSource APIs.
A GSocket is used as a convenient way to create a GSource
for a socket, but is not yet used for actual I/O.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In common with regular QEMU guests, the QMP probing
will need an event loop for handling monitor I/O
operations.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The event loop thread will be responsible for handling
any per-domain I/O operations, most notably the QEMU
monitor and agent sockets.
We start this event loop when launching QEMU, but stopping
the event loop is a little more complicated. The obvious
idea is to stop it in qemuProcessStop(), but if we do that
we risk loosing the final events from the QEMU monitor, as
they might not have been read by the event thread at the
time we tell the thread to stop.
The solution is to delay shutdown of the event thread until
we have seen EOF from the QEMU monitor, and thus we know
there are no further events to process.
Note that this assumes that we don't have events to process
from the QEMU agent.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We want a way to easily run a private GMainContext in a
thread, with correct synchronization between startup
and shutdown of the thread.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virbpf module wraps syscalls to BPF. However, if the kernel
headers used at the compile time don't have support for BPF the
module offers stubs which return a negative one to signal error
to the caller. But there is a slight discrepancy between real
functions and these stubs. While the former set errno and return
-1 the latter report an error (without setting the errno) and
return -1. This is not optimal because the caller might see stale
errno and overwrite the error message with a less accurate one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
In the virCgroupV2DevicesAvailable() function we try to determine
whether CGroups version 2 are available. We do this by opening
what we believe is the CGroup mount point and issuing a BPF call.
When the call fails, a debug message is printed. However, the BPF
call sets errno too. Include it in the debug message to help us
with debugging.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
When rewriting the virDomainDiskTranslateSourcePool() function in
v6.1.0-rc1~184 a typo was introduced. Previously, we allowed
startup policy only for those volumes which translated to
VIR_STORAGE_TYPE_FILE. But starting with the referenced commit,
the value we checked for was changed to VIR_STORAGE_VOL_FILE
which comes from a different enum and has a different value too.
This is wrong, because virStorageSourceGetActualType() returns a
value from the original enum.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1811728
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When preparing images for block jobs we modify their seclabels so
that QEMU can open them. However, as mentioned in the previous
commit, secdrivers base some it their decisions whether the image
they are working on is top of of the backing chain. Fortunately,
in places where we call secdrivers we know this and the
information can be passed to secdrivers.
The problem is the following: after the first blockcommit from
the base to one of the parents the XATTRs on the base image are
not cleared and therefore the second attempt to do another
blockcommit fails. This is caused by blockcommit code calling
qemuSecuritySetImageLabel() over the base image, possibly
multiple times (to ensure RW/RO access). A naive fix would be to
call the restore function. But this is not possible, because that
would deny QEMU the access to the base image. Fortunately, we
can use the fact that seclabels are remembered only for the top
of the backing chain and not for the rest of the backing chain.
And thanks to the previous commit we can tell secdrivers which
images are top of the backing chain.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1803551
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Our decision whether to remember seclabel for a disk image
depends on a few factors. If the image is readonly or shared or
not the chain top the remembering is suppressed for the image.
However, the virSecurityManagerSetImageLabel() is too low level
to determine whether passed @src is chain top or not. Even though
the function has the @parent argument it does not necessarily
reflect the chain top - it only points to the top level image in
the chain we want to relabel and not to the topmost image of the
whole chain. And this can't be derived from the passed domain
definition reliably neither - in some cases (like snapshots or
block copy) the @src is added to the definition only after the
operation succeeded. Therefore, introduce a flag which callers
can use to help us with the decision.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
If only IPv6 is configured on the host, getaddrinfo with AI_ADDRCONFIG
in hints would return EAI_ADDRFAMILY for nodenames that resolve to IPv4.
Also pass AI_V4MAPPED to accept IPv4-mapped addresses on IPv6-only
systems.
Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
[rewrote the commit message - jtomko]
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This is the same timeout of all other daemons, and just like them
virtlogd is socket-activated, so it will automatically be started
on demand whenever that's necessary.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
In the following recent change:
commit db72866310
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Tue Jan 14 10:40:52 2020 +0000
util: add API for reading password from the console
the fact that "bufptr" pointer may point to either heap or stack
allocated data was overlooked. As a result, when the strdup was
removed, we ended up returning a pointer to the local stack to
the caller. When the caller referenced this stack pointer they
got out garbage which fairly quickly resulted in a crash.
We need to copy the stack buffer into heap memory in the username
case.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Virtualization event types were added in 2.0.5:
https://github.com/linux-audit/audit-userspace/commit/3755e9ff
Even Ubuntu 14.04 (which we don't support) has 2.3.2.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
When spawning a thread via our virThread APIs we let pthread
spawn this helper thread which sets couple of thread local
variables (e.g. thread job name or thread worker name) and as of
v6.1.0-40-gc85256b31b it also sets pthread name (which is then
visible in `ps' output for instance). Only after these steps the
intended function is called. However, just before calling it we
free the buffer that holds the thread name which results in
invalid memory reads:
==47027== Invalid read of size 1
==47027== at 0x48389C2: strlen (vg_replace_strmem.c:459)
==47027== by 0x58BB3D6: __vfprintf_internal (vfprintf-internal.c:1645)
==47027== by 0x58CE6E0: __vasprintf_internal (vasprintf.c:57)
==47027== by 0x574BA28: g_vasprintf (in /usr/lib64/libglib-2.0.so.0.6000.7)
==47027== by 0x57240CC: g_strdup_vprintf (in /usr/lib64/libglib-2.0.so.0.6000.7)
==47027== by 0x48E0EFA: vir_g_strdup_vprintf (glibcompat.c:209)
==47027== by 0x493AA05: virLogVMessage (virlog.c:573)
==47027== by 0x493A8FE: virLogMessage (virlog.c:513)
==47027== by 0x4992FC7: virThreadJobClear (virthreadjob.c:121)
==47027== by 0x4992844: virThreadHelper (virthread.c:237)
==47027== by 0x5817496: start_thread (pthread_create.c:486)
==47027== by 0x59563CE: clone (clone.S:95)
The problem is that neither virThreadJobSetWorker() nor
virThreadJobSet() create a copy of passed name. They just set a
thread local variable to point to the buffer which is then
freed. Moving the free towards the end of the wrapper function
solves the issue.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The 'auths' struct in the test driver was not free()d. This was easy to
miss because the default XML doesn't include auth info.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
'template' might be used uninitialized.
Use g_autofree for everything and remove all the custom labels.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Convert the function to use g_autofree to silence -Wmaybe-uninitialized.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Our implementation masks GCC warnings of uninitialized use of the passed
argument. After changing this I got a load of following warnings:
src/conf/virnetworkportdef.c: In function 'virNetworkPortDefSaveStatus':
/usr/include/glib-2.0/glib/gmem.h:136:8: error: 'path' may be used uninitialized in this function [-Werror=maybe-uninitialized]
136 | if (_p) \
| ^
src/conf/virnetworkportdef.c:447:11: note: 'path' was declared here
447 | char *path;
| ^~~~
For the curious, g_clear_pointer is still safe for arguments with
side-effect. Here's the pre-processed output of trying to do a
VIR_FREE(*(test2++)):
do {
typedef char _GStaticAssertCompileTimeAssertion_1[(sizeof *(&(*(test2++))) == sizeof (gpointer)) ? 1 : -1] __attribute__((__unused__));
__typeof__((&(*(test2++)))) _pp = (&(*(test2++)));
__typeof__(*(&(*(test2++)))) _ptr = *_pp;
*_pp = ((void *)0);
if (_ptr)
(g_free) (_ptr);
} while (0) ;
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'path' could be accessed uninitialized. Fix it by using g_autofree which
also mandates initialization.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use 'g_autofree' to clean both 'path' and 'xml' which mandates
initialization and get rid of the 'cleanup' label and 'ret variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically threads are given a name based on the C function,
and this name is just used inside libvirt. With OS level thread
naming this name is now visible to debuggers, but also has to
fit in 15 characters on Linux, so function names are too long
in some cases.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Setting the thread name makes it easier to debug libvirtd
when many threads are running.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The qemuMonitorOpenFD method has not been used since it
was first introduced.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Libvirt has never configured the QEMU agent to support
running on a PTY implicitly. In theory an end user may
have written such an XML config, but this is reasonably
unlikely since when a bare <channel> is provided, libvirt
will auto-expand it to a UNIX socket backend.
With this change a user who has use the PTY backend will
have to switch to the UNIX backend if they wish to use
libvirt APIs for interacting with the agent. This will
not have guest ABI impact.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
qemuMonitorJSONMakeCommandInternal does the full command construction if
you pass in what would become the value of the 'arguments' key. Refactor
the open-coded implementation to use the helper and use modern cleanup
helpers at the same time.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Make it obvious that the function always returns a valid pointer and fix
all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
I've found that if my virtlogd is socket activated but the daemon
doesn't run yet, then the virt-qemu-run is killed right after it
tries to start the domain. The problem is that because the default
setting is to use virtlogd, the domain create code tries to
connect to virtlogd socket, which in turn tries to detect who is
connecting (virNetSocketGetUNIXIdentity()) and as a part of it,
it will try to open /proc/${PID_OF_SHIM}/stat which is denied by
SELinux:
type=AVC msg=audit(1582903501.927:323): avc: denied { search } for \
pid=1210 comm="virtlogd" name="1843" dev="proc" ino=37224 \
scontext=system_u:system_r:virtlogd_t:s0-s0:c0.c1023 \
tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=dir \
permissive=0
Virtlogd reacts by closing the connection which the shim sees as
SIGPIPE. Since the default response to the signal is Term, we
don't even get to reporting any error nor to removing the
temporary directory.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
When virt-qemu-run is ran without any root directory specified on
the command line, a temporary directory is made and used instead.
But since we are using g_dir_make_tmp() to create the directory
it is going to have 0700 mode. So even though we create the whole
directory structure under it and label everything, QEMU is very
likely to not have the access. This is because in this case there
is no qemu.conf and thus distro default UID:GID is used to run
QEMU (e.g. qemu:kvm on Fedora). Change the mode of the temporary
directory so that everybody has eXecute permission.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Libvirt tries to forbid migration onto the same host and it does
that by checking if local and remote hostnames are the same and
whether local and remote UUIDs are the same. Well, the latter
makes sense but the former doesn't really because libvirtd can be
running inside an UTS namespace and hostnames can appear the same
on both sides of migration. On the other hand, host UUIDs are
unique, so rely on them when trying to prevent migration onto the
same host.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1639596
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Gaurav Agrawal <agrawalgaurav@gnome.org>
[removed dead assignment]
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the 'flat' flag for 'query-named-block-nodes' if qemu supports
QEMU_CAPS_QMP_QUERY_NAMED_BLOCK_NODES_FLAT in qemuBlockGetNamedNodeData.
We don't need the data so plumb in whether qemu supports the
'flat' output.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modern qemu allows to skip the nested redundant data in the output of
query-named-block-nodes. Plumb in the support for the argument that
enables it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Replace qemuMonitorBlockGetNamedNodeData by qemuBlockGetNamedNodeData.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Detect the presence of the flag and make it available internally as
QEMU_CAPS_QMP_QUERY_NAMED_BLOCK_NODES_FLAT.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The monitor password callback was removed long time ago but the callback
type and variable were left around. Finish the cleanup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Format the 'vhost-user-fs' device on the QEMU command line.
This device provides shared file system access using the FUSE protocol
carried over virtio.
The actual file server is implemented in an external vhost-user-fs device
backend process.
https://bugzilla.redhat.com/show_bug.cgi?id=1694166
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Look into /usr/share/qemu/vhost-user to see whether we can find
a suitable virtiofsd binary, in case the user did not provide one
in the domain XML.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Wire up the code to put virtiofsd in the emulator cgroup on domain
startup.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Start virtiofsd for each <filesystem> device using it.
Pre-create the socket for communication with QEMU and pass it
to virtiofsd.
Note that virtiofsd needs to run as root.
https://bugzilla.redhat.com/show_bug.cgi?id=1694166
Introduced by QEMU commit a43efa34c7d7b628cbf1ec0fe60043e5c91043ea
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
This is not yet supported.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Add a 'virtiofsd_debug' option for tuning whether to run virtiofsd
in debug mode.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Add more elements for tuning the virtiofsd daemon
and the vhost-user-fs device:
<driver type='virtiofs' queue='1024' xattr='on'>
<binary path='/usr/libexec/virtiofsd'>
<cache mode='always'/>
<lock posix='off' flock='off'/>
</binary>
</driver>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Introduce a new 'virtiofs' driver type for filesystem.
<filesystem type='mount' accessmode='passthrough'>
<driver type='virtiofs'/>
<source dir='/path'/>
<target dir='mount_tag'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
</filesystem>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Introduced by QEMU commit 98fc1ada4cf70af0f1df1a2d7183cf786fc7da05
virtio: add vhost-user-fs base device
Released in QEMU v4.2.0.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Pass logManager to qemuExtDevicesStart for future usage.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
The default memlock limit is 64k which is not enough to start a single
VM. The requirements for one VM are 12k, 8k for eBPF map and 4k for eBPF
program, however, it fails to create eBPF map and program with 64k limit.
By testing I figured out that the minimal limit is 80k to start a single
VM with functional eBPF and if I add 12k I can start another one.
This leads into following calculation:
80k as memlock limit worked to start a VM with eBPF which means there
is 68k of lock memory that I was not able to figure out what was using
it. So to get a number for 4096 VMs:
68 + 12 * 4096 = 49220
If we round it up we will get 64M of memory lock limit to support 4096
VMs with default map size which can hold 64 entries for devices.
This should be good enough as a sane default and users can change it if
the need to.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1807090
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Whenever there is a guest CPU configured in domain XML, we will call
some CPU driver APIs to validate the CPU definition and check its
compatibility with the hypervisor. Thus domains with guest CPU
specification can only be started if the guest architecture is supported
by the CPU driver. But we would add a default CPU to any domain as long
as QEMU reports it causing failures to start any domain on affected
architectures.
https://bugzilla.redhat.com/show_bug.cgi?id=1805755
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Allow format probing to work around lazy clients which did not specify
their format in the overlay. Format probing will be allowed only, if we
are able to probe the image, the probing result was successful and the
probed image does not have any backing or data file.
This relaxes the restrictions which were imposed in commit 3615e8b39b
in cases when we know that the image probing will not result in security
issues or data corruption.
We perform the image format detection and in the case that we were able
to probe the format and the format does not specify a backing store (or
doesn't support backing store) we can use this format.
With pre-blockdev configurations this will restore the previous
behaviour for the images mentioned above as qemu would probe the format
anyways. It also improves error reporting compared to the old state as
we now report that the backing chain will be broken in case when there
is a backing file.
In blockdev configurations this ensures that libvirt will not cause data
corruption by ending the chain prematurely without notifying the user,
but still allows the old semantics when the users forgot to specify the
format.
Users thus don't have to re-invent when image format detection is safe
to do.
The price for this is that libvirt will need to keep the image format
detector still current and working or replace it by invocation of
qemu-img.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
While our code can detect ISO as a separate format, qemu does not use it
as such and just passes it through as raw. Add conversion for detected
parts of the backing chain so that the validation code does not reject
it right away.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Now that this file no longer transitively includes
domain_conf.h, it can be included here.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Make the header easier to read and let the compiler inline
what it wants.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is pulled in via domain_conf.h somehow, but it is directly used.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This file uses the virNetDevBandwidth*Floor helpers
without including the correct include,
relying on virnetworkportdef.h to include it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 17f430eb5c
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The ParseNode function takes arguments with types
from libxml.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are a lots of strings being handled inside some LXC functions.
They can be moved to g_autofree to avoid declaring a return value to get
proper code cleanups. This commit is changing functions from
lxc_{controller,cgroup,fuse} only.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Ever since commit c5a00350 the libxl parser invokes the emulator
to probe which device model to use.
Commit b90c4b5 introduced a workaround that used a stable path
which was very likely to result in the answer matching the default.
However the test is still affected by the host state and the binary
gets invoked if present.
Mock the libxlDomainGetEmulatorType function to stop wasting CPU
cycles every time a 'make check' is run on a system with xen installed.
For example xlconfigtest gets faster by 90 %
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: b90c4b5f50
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Take the parts affected by the host state out of DriverConfigNew
and put them into a separate function.
Adjust all the callers to call both functions.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
These hardcoded defaults do not need to be read from
the file. Move them out of libxlDriverConfigLoadFile.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The way that our file locking works is that we open() the file we
want to lock and then use fcntl(fd, F_SETLKW, ...) to lock it.
The problem is, we are doing all of these as root which doesn't
work if the file lives on root squashed NFS, because if it does
then the open() fails. The way to resolve this is to make this a
non fatal error and leave callers deal with this (i.e. disable
remembering) - implemented in the previous commit.
https://bugzilla.redhat.com/show_bug.cgi?id=1804672
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There are some cases where we want to remember the original owner
of a file but we fail to lock it for XATTR change (e.g. root
squashed NFS). If that is the case we error out and refuse to
start a domain. Well, we can do better if we disable remembering
for paths we haven't locked successfully.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
So far, in the lock state we are storing only the file
descriptors of the files we've locked. Therefore, when unlocking
them and something does wrong the only thing we can report is FD
number, which is not user friendly at all. But if we store paths
among with FDs we can do better error reporting.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virutil.h header defines a geteuid() macro for Windows platforms.
This fixes a few missed cases from:
commit b11e8cccdd
Author: Ján Tomko <jtomko@redhat.com>
Date: Sun Feb 16 23:09:15 2020 +0100
Remove virutil.h from all header files
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit fb01e1a44d missed including virutil.h, causing the following
compilation error
../../src/security/virt-aa-helper.c:1055:43: error: implicit declaration of
function 'virHostGetDRMRenderNode' [-Werror=implicit-function-declaration]
1055 | char *defaultRenderNode = virHostGetDRMRenderNode();
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
After the split of enum functions into virenum.h,
this function does not contain anything worth including
in another header file.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Include virutil.h in all files that use it,
instead of relying on it being pulled in somehow.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Include unistd.h in all files that use it, instead
of relying on it being pulled in via virutil.h
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
There is nothing in the vircgroup.h header file
requiring virutil.h.
Remove it and include unistd.h in the C files.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Historically, this file was a dump for most of our helper
functions and needed almost everywhere.
With the introduction of virfile.h and virstring.h,
and more importantly, virenum.h and the introduction
of GLib, that is no longer true.
Remove its include from C files that don't even use it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Prefer g_ascii_xdigit_value to virHexToBin.
Check the return value of the function and
remove the g_ascii_isxdigit calls, since
they're done anyway internally.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
When moving virclosecallbacks to src/hypervisor, I did not
adjust all the possible includes in Makefiles.
Use a path relative to src to fix the build.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 25c29ac2f5
This *is* a no-op, but there was a period of sickening dread while
auditing to be sure that no actual confusion between bus and slot had
occurred. I hope to avoid that by following the conventional order.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Reduce the complexity by isolating loop bodies in separate functions.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Pull the code for registering and unregistering a bhyve monitor object
into separate functions to improve code clarity.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
This makes lifecycle management a bit easier thanks to ref counting, and
it is closer to what the qemu driver does.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Just like virhostdev, this depends on domain_conf and
it's shared by multiple hypervisor drivers.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This module depends on domain_conf and is used directly by various
hypervisor drivers.
Move it to src/hypervisor.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently they live in util/virhostdev.
However the virhostdev module is wrongly placed
in util, which is below conf/ in our hierarchy.
Move the functions that are actually used in conf/
to conf/ and remove the include of virhostdev.h
from domain_conf.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Take the format of the backing store from the 'meta' object directly and
use g_steal_pointer to steal the path.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When we create the new virStorageSource from the definitions stored in
the parent we should also use the 'backingStoreRawFormat' field to
populate the format.
Callers which use virStorageSourceNewFromBacking are also fixed to stop
setting the format manually.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We store the backing file string in the structure so we should also
store the format so that callers can be simplified.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Both callers pass false. Since we frown upon format probing, remove the
unused possibility to do the probing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Configuring vhost-user-gpu like:
<video>
<driver name='vhostuser'/>
<model type='virtio' heads='1'/>
</video>
Triggers an apparmor denial like:
apparmor="DENIED" operation="exec" profile="libvirtd"
name="/usr/lib/qemu/vhost-user-gpu" pid=888257 comm="libvirtd"
requested_mask="x" denied_mask="x" fsuid=0 ouid=0
This helper is provided by qemu for vhost-user-gpu and thereby being
in the same path as qemu_bridge_helper. Due to that adding a rule allowing
to call uses the same path list.
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
The backend name is memory-backend-memfd but we've been checking
for memory-backend-memory.
Reported by GCC on rawhide:
../../../src/internal.h:75:22: error: 'strcmp' of a string of length 21 and
an array of size 21 evaluates to nonzero [-Werror=string-compare]
../../../src/qemu/qemu_command.c:3525:20: note: in expansion of macro 'STREQ'
3525 | } else if (STREQ(backendType, "memory-backend-memory") &&
| ^~~~~
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 24b74d187c
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If PrlVmDev_GetType(), PrlVmDev_GetIndex() or PrlVmCfg_GetBootDevCount()
fails, return false to indicate error. Returning -1 would be interpreted
as true when used in an if-statement.
Fixes: 8c9252aa6d
Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The order of arguments were not the same in the definition and
declaration. All callers use the same order as the definition, so there
is no bug, but change the function declaration to match the
implementation to avoid confusion.
Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
To more closely match the previous usage in virEventPollDispatchHandles,
where called the handle callback for any revents returned by poll.
This should fix the virtlogd error on subsequent domain startup:
error: can't connect to virtlogd: Cannot open log file:
'/var/log/libvirt/qemu/f28live.log': Device or resource busy
as well as virtlogd spinning caused by virLogHandlerDomainLogFileEvent
never being called on hangup.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: f8ab47cb44
Fixes: 946a25274c
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These were needed for virBhyveTapGetRealDeviceName
but were not deleted after the function was moved
to src/util.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: a1bd8d2546
Another vircgroup helper to avoid code repetition between
the LXC and QEMU driver.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
lxcDomainSetMemoryParameters() and qemuDomainSetMemoryParameters()
has duplicated chunks of code that can be put in a new
helper.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This new helper avoids more code repetition inside
lxcDomainSetBlkioParameters() and qemuDomainSetBlkioParameters().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After the introduction of virDomainDriverMergeBlkioDevice() in a
previous patch, it is now clear that lxcDomainSetBlkioParameters() and
qemuDomainSetBlkioParameters() uses the same loop to set cgroup
blkio parameter of a domain.
Avoid the repetition by adding a new helper called
virDomainCgroupSetupDomainBlkioParameters().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
lxcDomainParseBlkioDeviceStr() and qemuDomainParseBlkioDeviceStr()
are the same function. Avoid code repetition by putting the code
in a new helper.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
lxcDomainMergeBlkioDevice() and qemuDomainMergeBlkioDevice()
are the same functions. This duplicated code can't be put in
the existing domain_cgroup.c since it's not cgroup related.
This patch introduces a new src/hypervisor/domain_driver.c to
host this more generic code that can be shared between virt
drivers. This new file is then used to create a new helper
called virDomainDeivceMergeBlkioDevice() to eliminate the code
repetition mentioned above. Callers in LXC and QEMU files
were updated.
This change is a preliminary step for more code reduction of
cgroup related code inside lxcDomainSetBlkioParameters() and
qemuDomainSetBlkioParameters().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuSetupCgroupVcpuBW() and lxcSetVcpuBWLive() shares the
same code to set CPU CFS period and quota. This code can be
moved to a new virCgroupSetupCpuPeriodQuota() helper to
avoid code repetition.
A similar code is also executed in virLXCCgroupSetupCpuTune(),
but without the rollback on error. Use the new helper in this
function as well since the 'period' rollback, if not a
straight improvement for virLXCCgroupSetupCpuTune(), is
benign. And we end up cutting more code repetition.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code that calls virCgroupSetCpuShares() and virCgroupGetCpuShares()
is repeated in 4 different places. Let's put it in a new
virCgroupSetupCpuShares() to avoid code repetition.
There's a reason of why we execute a Get in the same value we
just executed Set, explained in detail by commit 97814d8ab3.
Let's add a gist of the reasoning behind it as a comment in
this new function as well.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code from qemuSetupCgroupCpusetCpus() and virLXCCgroupSetupCpusetTune()
can be centralized in a new helper called virCgroupSetupCpusetCpus().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virLXCCgroupSetupMemTune() and qemuSetupMemoryCgroup() shares
duplicated code that can be put in a new helper to avoid
code repetition.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There is duplicated code between virt drivers that needs to
be moved to avoid code repetition. In the case of duplicated
code between lxc_cgroup.c and qemu_cgroup.c a common place
would be utils/vircgroup.c. The problem is that this would
introduce /conf related definitions that shouldn't be imported
to vircgroup.c, which is supposed to be a place for utilitary
cgroups functions only. And syntax-check would forbid it anyway
due to cross-directory includes being used.
An alternative would be to overload domain_conf.c, which already
contains all the definitions required. But that file is already
crowded with XML handling code and we wouldn't do any favors to
it by putting more utilitary, non-XML parsing/formatting code
there.
In [1], Cole suggested a 'domain_cgroup' file to host common code
between lxc_cgroup and qemu_cgroup, and Daniel suggested a
'src/hypervisor' dir to host these type of files. This patch
introduces src/hypervisor/domain_cgroup.c and, to get started,
introduces a new virDomainCgroupSetupBlkio() function to host shared
code between virLXCCgroupSetupBlkioTune() and qemuSetupBlkioCgroup().
[1] https://www.redhat.com/archives/libvir-list/2019-December/msg00817.html
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Previous patch moved all duplicated code that were setting
and getting BlkioDevice parameters to vircgroup.c. We can
turn them into static and spare a few symbols in
libvirt_private.syms.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are code repetition of set() and get() blkio device
parameters across lxc and qemu files. Use the new vircgroup
helpers to trim the repetition a bit.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The current use of the functions that set and get
BlkioDevice attributes is doing a set(), followed by
a get() of the same parameter right after. This is done
because there is no guarantee that the kernel will accept
the desired value given by the set() call, thus we need to
execute a get() right after to get the actual value.
This patch adds helpers inside vircgroup.c to execute these
operations. Next patch will use these helpers to reduce
code repetition in LXC and QEMU files.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The inc.am Makfiles are included by src/Makefile.am.
Adjust the paths added to OPENRC_INIT_FILES_IN
accordingly.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: f4b1c020a2
These are missing files for OpenRC.
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The persistent alias name @persistent is allocated in
virDomainNetDefParseXML() but never freed.
==119642== 22 bytes in 2 blocks are definitely lost in loss record 178 of 671
==119642== at 0x483579F: malloc (vg_replace_malloc.c:309)
==119642== by 0x58F89F1: xmlStrndup (in /usr/lib64/libxml2.so.2.9.9)
==119642== by 0x4BA3B74: virXMLPropString (virxml.c:520)
==119642== by 0x4BDB0C5: virDomainNetDefParseXML (domain_conf.c:11876)
==119642== by 0x4BF9EF4: virDomainDefParseXML (domain_conf.c:21196)
==119642== by 0x4BFCD5B: virDomainDefParseNode (domain_conf.c:21943)
==119642== by 0x4BFCC36: virDomainDefParse (domain_conf.c:21901)
==119642== by 0x4BFCCCB: virDomainDefParseFile (domain_conf.c:21924)
==119642== by 0x114A9D: testCompareXMLToArgv (qemuxml2argvtest.c:452)
==119642== by 0x13894F: virTestRun (testutils.c:143)
==119642== by 0x11F46E: mymain (qemuxml2argvtest.c:1316)
==119642== by 0x13A60E: virTestMain (testutils.c:839
Fixes: fb0509d06a
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The privateData object is allocated in virDomainFSDefNew() but
never unref'd.
==119642== 480 bytes in 20 blocks are definitely lost in loss record 656 of 671
==119642== at 0x4837B86: calloc (vg_replace_malloc.c:762)
==119642== by 0x57806A0: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6000.7)
==119642== by 0x4AE7392: virAllocVar (viralloc.c:331)
==119642== by 0x4B64395: virObjectNew (virobject.c:241)
==119642== by 0x48F1464: qemuDomainFSPrivateNew (qemu_domain.c:1427)
==119642== by 0x4BBF004: virDomainFSDefNew (domain_conf.c:2307)
==119642== by 0x4BD859A: virDomainFSDefParseXML (domain_conf.c:11217)
==119642== by 0x4BF9DD1: virDomainDefParseXML (domain_conf.c:21179)
==119642== by 0x4BFCD5B: virDomainDefParseNode (domain_conf.c:21943)
==119642== by 0x4BFCC36: virDomainDefParse (domain_conf.c:21901)
==119642== by 0x4BFCCCB: virDomainDefParseFile (domain_conf.c:21924)
==119642== by 0x114A9D: testCompareXMLToArgv (qemuxml2argvtest.c:452)
Fixes: 5120577ed7
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
During the hypervisor-agnostic validation of network devices, verify
that the interface type is either "network" or "bridge", and that if
there is any <virtualport>, that it doesn't have any type associated
with it.
This needs to be done both for the parse-time validation and for
runtime validation (after a port has been acquired from any associated
network), because an interface with type='network' could have an
actual type at runtime of "hostdev" or "direct", neither of which
support isolated='true' (yet). Likewise, if an interface is
type='network', then at runtime a <virtualport> with a type that
doesn't support isolated='yes' (e.g. "openvswitch", "802.1Qbh" -
currently *none* of the available virtualport types support it)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This setting can be updating very easily on an already active
interface by just changing it in sysfs. If the bridge used for
connection is also changed, there is no need to separately update it,
because the new setting isf done as a part of connecting to the bridge
anyway.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This patch pushes the isolatedPort setting from the <interface> down
all the way to the callers of virNetDevBridgeAddPort(), and sets
BR_ISOLATED on the port (using virNetDevBridgePortSetIsolated()) after
the port has been successfully added to the bridge.
Signed-off-by: Laine Stump <laine@redhat.com>
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similar to the way that the <vlan>, <bandwidth>, and <virtualport>
elements and the trustGuestRxFilters attribute in a <network> (or in
the appropriate <portgroup> element of a <network> can be applied to a
port when it is allocated for a domain's network interface, this patch
checks for a configured value of <port isolated="yes|no"/> in
either the domain <interface> or in the network, setting isolatedPort
in the <networkport> to the first one it finds (the setting from the
domain's <interface> is preferred). This, in turn, is passed back to
the domain when a port is allocated, so that the domain will use that
setting.
(One difference from <vlan>, <bandwidth>, <virtualport>, and
trustGuestRxFilters, is that all of those can be set in a <portgroup>
so that they can be applied only to a subset of interfaces connected
to the network. This didn't really make sense for the isolated setting
due to the way that it's implemented in Linux - the BR_ISOLATED flag
will prevent traffic from passing between two ports that both have
BR_ISOLATED set, but traffic can still go between those ports and
other ports that *don't* have BR_ISOLATED. (It would be nice if all
traffic from a BR_ISOLATED port could be blocked except traffic going
to/from a designated egress port or ports, but instead the entire
feature is implemented as a single flag. Because of this, it's really
only useful if all the ports on a network are isolated, so setting it
for a subset has no practical utility.)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is a very simple thing to parse and format, but needs to be done
in 4 places, so two trivial utility functions have been made that can
be called from all the higher level parser/formatters:
<domain><interface>
<domain><interface><actual> (only in domain status)
<network>
<networkport>
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When this flag is set for an interface attached to a bridge, traffic
to/from the specified interface can only enter/exit the bridge via
another attached interface that *doesn't* have the BR_ISOLATED flag
set. This can be used to permit guests to communicate with the rest of
the network, but not with each other.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Not only was the original error code destroyed in the case of
encountering an error during recovery from a failed attach to the
bridge (and then *that* error was destroyed by logging a *second*
error about the failure to recover - virNetDevBridgeAddPort() already
logs an error, so the one about failing to recover was redundant), but
if the recovery was successful, the function would then return success
to the caller even though it had failed.
Fixes: 2711ac8716
(overwritten errors were introduced along with this functionality)
Fixes: 6bde0a1a37
(the wrong return value was introduced by a refactor)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Firstly, the check for disk I/O error can be moved into 'if
(!offline)' section a few lines below.
Secondly, checks for vmstate and slirp should be moved under the
same section because they reflect live state of a domain. For
offline migration no QEMU is involved and thus these restrictions
are not valid.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In two places where virPidFileForceCleanupPath() is called, we
try to unlink() the pidfile again. This is needless because
virPidFileForceCleanupPath() has done just that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virPidFileReadPath() function is supposed to return 0 on
success or a negative value on failure. But the negative value
has a special meaning - it's negated errno. Therefore, when
converting string to int we shouldn't return -1 which translates
to EPERM. Returning EINVAL looks closer to the truth.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's nothing to clean up. Make it obvious what is returned.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Extract the code that directly deals with storage. This allows further
simplification and clarification of virStorageFileGetMetadataRecurse.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Replacing virHashLookup by virHashHasEntry allows us to use NULL as the
payload of the hash table rather than putting a fake '1' pointer into
the table.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The path can be NULL e.g. for NBD disks. Use NULLSTR to prevent use of
NULL in %s.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Move the assignment to a place where we know that the backing store is
present rather than having to check in the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
We call virStorageFileSupportsBackingChainTraversal which already checks
that the 'storageFileRead' callback is non-NULL, which in turn means
that virStorageFileRead will not return -2.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Probing by file suffix was meant to be a last resort if probing by
contents fails or is not supported. For most formats we never specified
any suffix. There's a few formats implementing both magic bytes and
suffix and finally DMG which had only suffix probing. Since suffix
probing is nowhere reliable and only one format depends on in which has a
comment that qemu doesn't do the probing either drop the whole
infrastructure.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
In commit v5.9.0-400-gaf8e39921a I removed printing model's fallback and
vendor_id attributes when no model is specified. However, vendor_id
makes sense even without a specific CPU model (for host-model CPUs).
https://bugzilla.redhat.com/show_bug.cgi?id=1804549
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuMonitorGetIOThreads returns a NULL-terminated list even when 0
iothreads are present. The caller didn't perform cleanup if there were 0
iothreads leaking the array.
https://bugzilla.redhat.com/show_bug.cgi?id=1804548
Fixes: d1eac92784
Reported-by: Jing Yan <jiyan@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
To fix the actual bug, it was necessary to make networkPlugBandwidth() be
called also for 'bridge'-type networks implemented using macvtap's 'bridge'
mode (previously it was only called for those implemented on top of an
existing bridge).
However, it seems beneficial to call it for other network types as well, at
least because it removes an inconsistency in types of bandwidth configuration
changes permissible in inactive and active domain configs. It should also be
safe as the function pretty much amounts to NOP if no QoS is requested and the
new behaviour should not be any worse than before if it is.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Even if an interface of type 'network', setting 'floor' is only supported
if the network's forward type is nat, route, open or none.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
QoS 'floor' setting is documented to be only supported for interfaces of
type 'network'. Fail with an error message on attempt to set 'floor' on
an interface of any other type.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This compound condition will be useful in several places so it
makes sense to give it a name for better readability.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If the parsed 'raw' format JSON string has 'offset' or 'size' attributes
parse them as the format slice.
https://bugzilla.redhat.com/show_bug.cgi?id=1791788
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Implement support for the slice of type 'storage' which allows to set
the offset and size which modifies where qemu should look for the start
of the format container inside the image.
Since slicing is done using the 'raw' driver we need to add another
layer into the blockdev tree if there's any non-raw image format driver
used to access the data.
This patch adds the blockdev integration and setup of the image data so
that we can use the slices for any backing image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When creating overlay images e.g. for snapshots or when merging
snapshots we often specify the backing store string to use. Make the
formatter aware of backing chain entries which have a <slice>
configured so that we record it properly. Otherwise such images
would not work without the XML (when detecting the backing chain).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The storage slice will require a specific node name in cases when the
image format is not raw. Store and format them in the status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Specifically creating such images via libvirt during blockjobs would
be much more hassle than it's worth. Just forbid them for now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We support explicit storage slices only when using blockdev. Storage
slices expressed via the backing store string are left to qemu to
open correctly.
Reject storage slices configured via the XML for non-blockdev usage.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If we have a 'format' type slice for a raw driver we can directly format
the values.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce virStorageSourceSlice which will store the 'offset' and 'size'
of a virStorageSource and declare it as 'sliceStorage' and 'sliceFormat'
attributes of a virStorageSource.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If a domain has a NVMe disk it already has the access configured.
Trying to configure it again on a commit or some other operation
is wrong and condemned to failure.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Its behavior is controlled by a KVM-specific CPU feature.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Its use is limited to certain guest types, and it only supports
a subset of all possible tick policies.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This new timer model will be used to control the behavior of the
virtual timer for KVM ARM/virt guests.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We will use this capability to detect whether the QEMU binary
supports the kvm-no-adjvtime CPU feature.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make sure we are taking all possible virDomainTimerNameType values
into account. This will make upcoming changes easier.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Libvirt switched to using a UNIX socket for monitors in
2009 for version 0.7.0. It seems unlikely that there is
a running QEMU process that hasn't been restarted for
11 years while also taking a libvirt upgrade. Therefore
we can drop support for opening a PTY for the QEMU
monitor.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
On FreeBSD 12 the default ulimit settings allow for 100,000
open file descriptors. As a result spawning processes in
libvirt is abominably slow. Fortunately FreeBSD has long
since provided a good solution in the form of closefrom(),
which closes all FDs equal to or larger than the specified
parameter.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When emulating smartcard with host certificates, qemu needs to
be able to read the certificates files. Add necessary code to
add the smartcard certificates file path to the apparmor profile.
Passthrough support has been tested with spicevmc and remote-viewer.
v2:
- Fix CodingStyle
- Add support for 'host' case.
- Add a comment to mention that the passthrough case doesn't need
some configuration
- Use one rule with '{,*}' instead of two rules.
Signed-off-by: Arnaud Patard <apatard@hupstream.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Since we parse attributes for 'raw' which is a format driver and thus
has nested 'file' structure we must prevent that this isn't nested
arbitrarily.
Add a flag for the function which allows parsing of 'format' type
drivers only on the first pass.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are two possibilities:
1) json:{"file":{"driver":...}}
2) json:{"driver":...}
Our code didn't work properly with the second one as it was expecting
the 'file' wrapper. Conditionalize the removal to only the situation
when the top level doesn't have "driver".
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The parser was originally designed only for protocol parsers. Since
we already have 'raw' format driver in the list we'll need to be able
to parse it too. In later patches this will be used to prevent parsing
nested format drivers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Originally virStorageSourceParseBackingJSON didn't recurse, but when
the 'raw' driver support was added we need to parse it's information
which contains nested 'file' object.
Since the deflattening helper recurses already there's no need to call
it again. Move it one level up to the entry point.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are a few error messages which might want to report the original
backing store string. Pass it around rather than trying to re-generate
it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
My original implementation was completely broken because it attempted to
use object-add/del instead of blockdev-add/del.
https://bugzilla.redhat.com/show_bug.cgi?id=1798366
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Modify qemuMonitorBlockdevAdd so that it takes a double pointer for the
@props argument so that it's cleared inside the call. This allows
writing cleaner callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Use automatic variable freeing and get rid of the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Use automatic variable freeing and get rid of the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Now that we accept full backing chains on input nothing should prevent
users from also using disk type 'VOLUME' for specifying the backing
images.
Do the translation for the whole backing chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract all the code setting up one storage source from the rest which
sets up the whole disk. This will allow us to prepare the whole backing
chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Only 'def->src' was ever used in this function. Use the source directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Only 'def->src' was ever used in this function. Use the source directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Only 'def->src' was ever used in this function. Use the source directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virStringSplitCount instead of virStringSplit so that we can drop
the call to virStringListLength and use VIR_AUTOSTRINGLIST to declare
it and allow removal of the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Few switch cases returned failure but didn't report an error. For a
situation when the backingStore type='volume' was not translated the
following error would occur:
$ virsh start VM
error: Failed to start domain VM
error: An error occurred, but the cause is unknown
After this patch:
$ virsh start VM
error: Failed to start domain VM
error: internal error: storage source pool 'tmp' volume 'pull3.qcow2' is not translated
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We call APIs that reset the error in the rollback code.
Preserve the error from the original call that failed.
This turns the boringly cryptic:
error: Unable to set interface parameters
error: An error occurred, but the cause is unknown
to the unexpectedly anarchist:
error: internal error: Child process (/usr/sbin/tc filter add
dev vnet1 parent ffff: protocol all u32 match u32 0 0 police
rate 4294968kbps burst 4294968kb mtu 64kb drop flowid :1)
unexpected exit status 1: Illegal "rate"
Illegal "police"
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: f02e21cb33https://bugzilla.redhat.com/show_bug.cgi?id=1800505
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Otherwise an attempt to set an invalid value:
virsh domiftune rhel8.2 vnet0 --outbound 4294968
on an interface with no bandwidth set crashes.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: f02e21cb33https://bugzilla.redhat.com/show_bug.cgi?id=1800505
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This deletes all trace of gnulib from libvirt. We still
have the keycodemapdb submodule to deal with. The simple
solution taken was to update it when running autogen.sh.
Previously gnulib could auto-trigger refresh when running
'make' too. We could figure out a solution for this, but
with the pending meson rewrite it isn't worth worrying
about, given how infrequently keycodemapdb changes.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
It is no longer require since switching to the GLib based
event loop impl.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This sets the GLib event loop as the impl when calling
virEventRegisterDefaultImpl(). This remains a private
impl detail of libvirt, so applications must *NOT*
assume that a call to virEventRegisterDefaultImpl()
results in a GLib based event loop.
They should continue to use the libvirt-glib API
gvir_event_register() if they explicitly want to guarantee
a GLib event loop.
This follows the general principal that the libvirt public
API should not expose the fact that GLib is being used
internally.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The libvirt-glib project has provided a GMainContext based
event loop impl for applications. This imports it and sets
it up for use by libvirt as the primary event loop. This
remains a private impl detail of libvirt.
IOW, applications must *NOT* assume that a call to
"virEventRegisterDefaultImpl" results in a GLib based
event loop. They should continue to use the libvirt-glib
API gvir_event_register() if they explicitly want to
guarantee a GLib event loop.
This follows the general principle that the libvirt public
API should not expose the fact that GLib is being used
internally.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
To eliminate the dependancy on GNULIB's poll impl, we need
to change the RPC client code to use GMainLoop. We don't
really want to use GIOChannel, but it provides the most
convenient way to do socket event watches with Windows
portability. The other alternative would be to use GSocket
but that is a much more complex change affecting libvirt
more broadly.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We need to be able to create event loop watches using the
GSource API for sockets. GIOChannel is able todo this, but
we don't want to use the GIOChannel APIs for reading/writing,
and testing shows just using its GSource APIs is unreliable
on Windows.
This patch thus creates a standalone helper API for creating
a GSource for a socket file descriptor. This impl is derived
from code in QEMU's io/channel-watch.c file that was written
by myself & Paolo Bonzini & thus under Red Hat copyright.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Pvpanic device supports bit 1 as crashloaded event, it means that
guest actually panicked and run kexec to handle error by guest side.
Handle crashloaded as a lifecyle event in libvirt.
Test case:
Guest side:
before testing, we need make sure kdump is enabled,
1, build new pvpanic driver (with commit from upstream
e0b9a42735f2672ca2764cfbea6e55a81098d5ba
191941692a3d1b6a9614502b279be062926b70f5)
2, insmod new kmod
3, enable crash_kexec_post_notifiers,
# echo 1 > /sys/module/kernel/parameters/crash_kexec_post_notifiers
4, trigger kernel panic
# echo 1 > /proc/sys/kernel/sysrq
# echo c > /proc/sysrq-trigger
Host side:
1, build new qemu with pvpanic patches (with commit from upstream
600d7b47e8f5085919fd1d1157f25950ea8dbc11
7dc58deea79a343ac3adc5cadb97215086054c86)
2, build libvirt with this patch
3, handle lifecycle event and trigger guest side panic
# virsh event stretch --event lifecycle
event 'lifecycle' for domain stretch: Crashed Crashloaded
events received: 1
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Starting a KVM domain on s390 with old machine type (such as
s390-ccw-virtio-2.5) and without any guest CPU model configured fails
with
CPU models are not available: KVM doesn't support CPU models
QEMU error. This is cause by libvirt using host-model CPU as the default
CPU based on QEMU reporting "host" CPU model as being the default one
(see commit v5.9.0-402-g24d8202294: qemu: Use host-model CPU on s390 by
default). However, even though both QEMU and KVM support CPU models on
s390 and QEMU can give us the host-model CPU, we can't use it with old
machine types which only support -cpu host.
https://bugzilla.redhat.com/show_bug.cgi?id=1795651
Reported-by: Christian Ehrhardt <paelzer@gmail.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The usability of a specific CPU mode may depend on machine type, let's
prepare for this by passing it to virQEMUCapsIsCPUModeSupported.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After LXC version 3, some settings were changed to new names. Same as
network. LXC introduced network indexes and changed IPv{4,6} addresses
fields. Before, users should only pass `lxc.network.ipv4` to define an
IPv4 address. Now, on version 3, users need to pass
`lxc.net.X.ipv4.address` to specify the same thing. Same for IPv6.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
To configure network settings using config file, legacy LXC settings
require starting them with 'lxc.network.type' entry. If someone
accidentally starts with 'lxc.network.name', libvirt will crash with
segfault. This patch checks if this case is happening.
Sample invalid settings:
lxc.network.link = eth0
lxc.network.type = phys
lxc.network.name = eth1
lxc.network.ipv4 = 192.168.122.2/24
lxc.network.ipv4.gateway = 192.168.122.1
Now, libvirt only see error without segmentation fault.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Extend QEMU with tpm-spapr support. Assign a device address to the
vTPM device model.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Extend the QEMU capabilties with tpm-spapr support.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This patch adds support for the tpm-spapr device model for ppc64. The XML for
this type of TPM looks as follows:
<tpm model='tpm-spapr'>
<backend type='emulator'/>
</tpm>
Extend the documentation.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Introduce VIR_DOMAIN_TPM_MODEL_DEFAULT as a default model which we use
in case the user does not provide a model in the device XML. It has
the TIS's previous value of '0'. In the post parsing function
we change this default value to 'TIS' to have the same model as before.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This lets it generate the remote dispatch for StorageVolGetInfoFlags.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Technically, there is no memleak here, since the only
allocations are filled by virDomainDeviceInfoParseXML,
which cleans up after itself.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Simple g_autofree is not enough if we put allocated
data into the device structure.
Define the AUTOPTR_CLEANUP function and use it here.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Xu Yandong <xuyandong2@huawei.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virFilePrintf function was a wrapper for fprintf() to provide
Windows portability, since gnulib's fprintf() replacement was
license restricted. This is no longer needed now we have the
g_fprintf function available.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The printf/fprintf function impls provided on Windows do
not follow the normal syntax for format specifiers as
the UNIX hosts. Currently we use GNULIB to provide a
portability fix for this. GLib has also imported the
GNULIB impl for this same reason, and thus we can rely
on the g_printf / g_fprintf functions.
This identified a couple of places not explicitly
linking to glib.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
On macOS some definitions are in xlocale.h, instead of in
locale.h. GNULIB hides this difference by making the latter
include the former.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
MinGW lacks ENOMSG until version 6.0.0 (Fedora 31).
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
All our supported Linux distros now have this header.
It has never existed on FreeBSD / macOS / Mingw.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When we get rid of GNULIB, we need to check for -lpthread
support.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This addreses portability to Windows and standardizes
error reporting. This fixes a number of places which
failed to set O_CLOEXEC or failed to report errors.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This hides the differences between Windows and UNIX,
and adds standard error reporting.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Most code now uses the virProcess / virCommand APIs, so
the need for sys/wait.h is quite limited. Removing this
include removes the dependency on GNULIB providing a
dummy sys/wait.h for Windows.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Almost none of the virFDStream code will actually work
on WIN32 builds, nor is it used except for in the
virtualbox driver for screenshots. It is simpler to
wrap it all in a '#ifndef WIN32'.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Remove imports of poll.h which are redundant, and
conditionalize remaining usage that needs to compile
on Windows platforms.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Use qemuBlockBitmapsHandleBlockcopy to calculate bitmaps to copy over
for a block-copy job.
We copy them when pivoting to the new image as at that point we are
certain that we don't dirty any bitmap unnecessarily.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a function calculating which bitmaps to copy to the mirror during
a block-copy operation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a validator which checks that a bitmap spanning multiple backing
chain members doesn't look broken. The current rules are that no
intermediate birmaps are missing (unfortunately it's hard to know
whether the topmost or bottommost bitmap is missing) and none of the
components is inconsistent.
We can obviously improve it over time.
The validator is also tested against the existing bitmap data we have
for the backup merging test as well as some of the existing broken
bitmap synthetic test cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The flags may control important aspects of the block job which may
influence also the termination of the job. Store the 'flags' for all
the block job types.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a variable which will store the contents of the 'flags' variable as
passed in by the individual block jobs. Since the flags may influence
behaviour of the jobs it's important to preserve them to the
finalization steps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use g_new0 and skip checking of the return value of keyCopy callback
as both are bound to return a valid pointer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tweak the return value expectation comment so that it doesn't
necessarily require to allocate memory and refactor the implementations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the glib allocation function that never returns NULL and remove the
now dead-code checks from all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a helper that concatenates the second array into the first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Create a wrapper for qemuBlockGetNamedNodeData named
qemuBlockGetNamedNodeData. The purpose of the wrapper is to integrate
the monitor handling functionality and in the future possible
qemuCaps-based flags.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow qemu access to modify backing files in case when we want to delete
a checkpoint.
This patch adds tracking of which images need to be relabelled when
calculating the transaction, the code to relabel them and rollback.
To verify that stuff works we also output the list of images to relabel
into the test case output files in qemublocktest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow deleting of checkpoints when snapshots were created along. The
code tracks and modifies the checkpoint list so that backups can still
be taken with such a backing chain. This unfortunately requires to
rename few bitmaps (by copying and deleting them) in some cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The configure script allows users to specify different paths for
/etc/, /usr/sbin/, /var/run/ and /usr/libexec/. Instead of
assuming user will pass expected value, generate the apparmor
profiles using the actual values.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This requires stealing one cmd pointer before returning it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Mark eligible declarations as g_autofree and remove
the corresponding VIR_FREE calls.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Always trim the full specified suffix.
All of the callers outside of tests were passing either
strlen or the actual length of the string.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Replace all the cases that only supply the length
and do not care about matching a suffix, as well
as that one test case that does.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Just like the existing virBufferTrim, but only
does one thing at a time.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
As discussed on the developer list, parallel migration connections
are not compatible with tunneled migration
https://www.redhat.com/archives/libvir-list/2020-January/msg00463.html
Prohibit the concurrent use of parallel and tunneled migration options.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Domain definition is useless now inside network structure. This pointer
was required because new network definition was being added each time
that a new network type appeared. So, this should be processed into
old function `lxcNetworkParseDataType()`. Now, as it was moved to an
array, it can be handle together each interface pointer.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
LXC version 3 or higher introduced indexes for network interfaces.
Libvirt should be able to parse entries like `lxc.net.2.KEY`. This
commit adds functions to parse this type of field. That's why array
structures are so important this time.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Struct lxcNetworkParseData is being used as a single pointer which
iterates through LXC config lines. It means that it will be applied as a
network each time that a new type appears. After, the same struct is
used to populate a new network interface. This commit changes this logic
to multiple lxcNetworkParseData to move this strcuture to an array. It
makes more sense if we are using indexes to fill interface settings.
This is better to improve code clarity.
This commit still introduces *Legacy() functions to keep support of
network old style definitions.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now, that every use of virAtomic was replaced with its g_atomic
equivalent, let's remove the module.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of calling virAtomicIntAdd(&var, 1); we can call
g_atomic_int_add() directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The rewrite to use GLib's atomic ops functions changed the behavior
of virAtomicIntInc - before it returned the pre-increment value.
Most of the callers using its value were adjusted, but the one
in qemuDriverAllocateID was not. If libvirtd would reconnect to
a running domain during startup, the next started domain would get
the same ID:
$ virsh list
Id Name State
--------------------------
1 f28live running
1 f28live1 running
Use the g_atomic_add function directly (as recommended in viratomic.h)
and add 1 to the result.
This also restores the usual numbering from 1 instead of 0.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: 7b9645a7d1
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Starting on commit 1f43393283, qemuDomainFillDeviceIsolationGroup()
returns 0 in all circunstances. Let's turn it to 'void' make it
clearer that the function will not fail. This also spares a
check for < 0 return in qemu_hotplug.c. The
qemuDomainFillDeviceIsolationGroupIter() callback now returns
0 at all times - which is already happening anyway.
Refer to 1f43393283 commit message for more details on why
the function was changed to never return an error.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemuDomainChrDefDropDefaultPath() returns an int, but it's
always returning 0. Callers are checking for result < 0 to
run their cleanup code needlessly.
Turn the function to 'void' and adjust the callers.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Avoid some of the virObjectUnref() calls by using g_autoptr.
Aside from the 'cleanup' label in qemuDomainSetFakeReboot(),
all other now deprecated cleanup labels will be removed in
the next patch.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree to remove VIR_FREE() calls used for cleanups.
Labels that became deprecated will be removed in a later
patch.
In qemuDomainSetupDisk(), the 'dst' variable is not used at
all and could be removed.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'caps' variable in qemuDomainObjPrivateXMLParseAutomaticPlacement()
is set to auto clean via g_autoptr(), but a 'virObjectUnref(caps)' is
being executed in the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Previous patch used 'g_autofree' to eliminate instances of
VIR_FREE(), making some cleanup labels obsolete. This
patch removes them.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree in strings when possible to spare a VIR_FREE()
call. Unneeded 'cleanup' labels will be taken care of in the
next patch.
The 'str' string in virDomainVirtioSerialAddrReserve() was
never used by the logic, only being used in cleanup by
VIR_FREE(). Let's remove it.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
With -blockdev we must look up via the nodename rather than the 'drive'
alias which is not present any more.
This fixes the pre-creation of storage volumes on migration with
non-shared storage.
https://bugzilla.redhat.com/show_bug.cgi?id=1793263
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Swithc to the helper which doesn't require checking of the return value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The data is gathered only once so we can move the whole block which
fetches the data out of the loop and get rid of the logic which
prevents multiple calls.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Refactor the logic to skip the body of the function if there's nothing
to do.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are two calls to virHashNew which check the return value. It's not
necessary any more as virHashNew always returns a valid pointer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This module has last two direct checks whether the value returned by
virHashCreateFull is NULL. Remove them so that static analyzers don't
get the false idea that checking the value is necessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the glib helpers and remove the mention of returning NULL on failure
of virHashNew, virHashCreate and virHashCreateFull.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since a3ab6d42 "apparmor: convert libvirtd profile to a named profile"
the detection of the subelement for qemu_bridge_helper is wrong.
In combination with the older 123cc3e1 "apparmor: allow
/usr/lib/qemu/qemu-bridge-helper" it now detects qemu-bridge-helper no
more with its path, but instead as a proper subelement of the named profile
like: label=libvirtd//qemu_bridge_helper
In the same fashion the reverse rule in the qemu_bridge_helper
sub-profile still uses the path and not the named profile label.
Triggering denies like:
apparmor="DENIED" operation="file_inherit"
profile="libvirtd//qemu_bridge_helper" pid=5629 comm="qemu-bridge-hel"
family="unix" sock_type="stream" protocol=0 requested_mask="send receive"
denied_mask="send receive" addr=none peer_addr=none peer="libvirtd"
This patch fixes the unix socket rules for the communication between
libvirtd and qemu-bridge-helper to match that.
Fixes: a3ab6d42d8
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1655111
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Our nwfilter code doesn't set any timeout on the pcap packet buffer which
means that when DHCP snooping is enabled on a guest interface and
libvirt is trying to learn the IP address from guest's DHCP traffic, it
takes up to 4x longer to ping a guest successfully compared to a case
where nwfilter isn't enabled at all or libvirt uses the cached nwfilter
leases to populate the corresponding rules to ebtables.
With the pcap filter and rate limiting already in place, we should be
able to afford enabling the immediate packet delivery, FWIW immediate
mode was actually the default prior libpcap-1.5.0 (CentOS 6) regardless
of whether a buffer was requested.
The lack of any kind of timeout on the pcap buffer messed with the
libvirt TCK test suite which, even with a generous timeout in place,
timeouts every single time simply because it takes a while until
guest actually starts producing any kind of traffic to fill up
the buffer in place (apart from the DHCP traffic which happens fairly
early on).
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
At the beginning of each profile we have a comment that says when
the profile was last updated. In theory, it makes sense because
one can see immediately if they are using an outdated profile.
However, we don't do a good job in keeping the comments in sync
with reality and also sysadmins should rather use their package
manager to find out libvirt version which installed the profiles.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
There are two more paths that we are missing in the default
domain profile: /usr/share/edk2-ovmf/ and /usr/share/sgabios/.
These exist on my Gentoo box and contain UEFI and BIOS images
respectively.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Even though we construct a domain specific profile for each
domain we start (which should cover domain specific paths), there
is also another file that is included from the profile and which
contains domain agnostic paths (e.g. to cover libraries that qemu
links with). The paths in the file are split into blocks divided
by comments. Sort the paths in each block individually (ignoring
case sensitivity).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
With Credit2 being Xen default scheduler, it's definitely the case to
allow Credit2's scheduling parameters to be get and set via libvirt.
This is easy, as Credit and Credit2 have (at least as of now) the very
same parameters ('weight' and 'cap'). So we can just let credit2 pass
the scheduler-type check and the same code will work for both.
Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Aside from itinerant error (actually warning) messages due to an
unrecognized response from qemu, this isn't even necessary - the
migration proceeds successfully to completion anyway.
(I'm not sure where to see this status reported in the API though - do
we need to add an extra state, or recognition of a new event somewhere?)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Normally a PCI hostdev can't be migrated, so
qemuMigrationSrcIsAllowedHostdev() won't permit it. In the case of a a
hostdev network interface that has <teaming type='transient'/> set,
QEMU will automatically unplug the device prior to migration, and
re-plug a corresponding device on the destination. This patch modifies
qemuMigrationSrcIsAllowedHostdev() to allow domains with those devices
to be migrated.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The QEMU driver uses the <teaming type='persistent|transient'
persistent='blah'/> element to setup a "failover" pair of devices -
the persistent device must be a virtio emulated NIC, with the only
extra configuration being the addition of ",failover=on" to the device
commandline, and the transient device must be a hostdev NIC
(<interface type='hostdev'> or <interface type='network'> with a
network that is a pool of SRIOV VFs) where the extra configuration is
the addition of ",failover_pair_id=$aliasOfVirtio" to the device
commandline. These new options are supported in QEMU 4.2.0 and later.
Extra qemu-specific validation is added to ensure that the device
type/model is appropriate and that the qemu binary supports these
commandline options.
The result of this will be:
1) The virtio device presented to the guest will have an extra bit set
in its PCI capabilities indicating that it can be used as a failover
backup device. The virtio guest driver will need to be equipped to do
something with this information - this is included in the Linux
virtio-net driver in kernel 4.18 and above (and also backported to
some older distro kernels). Unfortunately there is no way for libvirt
to learn whether or not the guest driver supports failover - if it
doesn't then the extra PCI capability will be ignored and the guest OS
will just see two independent devices. (NB: the current virtio guest
driver also requires that the MAC addresses of the two NICs match in
order to pair them into a bond).
2) When a migration is requested, QEMu will automatically unplug the
transient/hostdev NIC from the guest on the source host before
starting migration, and automatically re-plug a similar device after
restarting the guest CPUs on the destination host. While the transient
NIC is unplugged, all network traffic will go through the
persistent/virtio device, but when the hostdev NIC is plugged in, it
will get all the traffic. This means that in normal circumstances the
guest gets the performance advantage of vfio-assigned "real hardware"
networking, but it can still be migrated with the only downside being
a performance penalty (due to using an emulated NIC) during the
migration.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The subelement <teaming> of <interface> devices is used to configure a
simple teaming association between two interfaces in a domain. Example:
<interface type='bridge'>
<source bridge='br0'/>
<model type='virtio'/>
<mac address='00:11:22:33:44:55'/>
<alias name='ua-backup0'/>
<teaming type='persistent'/>
</interface>
<interface type='hostdev'>
<source>
<address type='pci' bus='0x02' slot='0x10' function='0x4'/>
</source>
<mac address='00:11:22:33:44:55'/>
<teaming type='transient' persistent='ua-backup0'/>
</interface>
The interface with <teaming type='persistent'/> is assumed to always
be present, while the interface with type='transient' may be be
unplugged and later re-plugged; the persistent='blah' attribute (and
in the one currently available implementation, also the matching MAC
addresses) is what associates the two devices with each other. It is
up to the hypervisor and the guest network drivers to determine what
to do with this information.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Presence of the virtio-net-pci option called "failover" indicates
support in a qemu binary of a simplistic bonding of a virtio-net
device with another PCI device. This feature allows migration of
guests that have a network device assigned to a guest with VFIO, by
creating a network bond device in the guest consisting of the
VFIO-assigned device and a virtio-net-pci device, then temporarily
(and automatically) unplugging the VFIO net device prior to migration
(and hotplugging an equivalent device on the migration
destination). (The feature is called "failover" because the bond
device uses the vfio-pci netdev for normal guest networking, but
"fails over" to the virtio-net-pci netdev once the vfio-pci device is
unplugged for migration.)
Full functioning of the feature also requires support in the
virtio-net driver in the guest OS (since that is where the bond device
resides), but if the "failover" commandline option is present for the
virtio-net-pci device in qemu, at least the qemu part of the feature
is available, and libvirt can add the proper options to both the
virtio-net-pci and vfio-pci device commandlines to indicate qemu
should attempt doing the failover during migration.
This patch just adds the qemu capabilities flag "virtio-net.failover".
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
* chown: use is conditionally compiled
* configmake: functionality imported to Makefile.am
* getaddrinfo: we have no portability problems
with Windows impl
* getpass: simplified impl is imported
* mgetgroups: getgrouplist is used directly
* net_if: header includes are conditionalized
* netdb: header includes are conditionalized
* passfd: simplified impl is imported
* posix-shell: functionality was unused & removed
* sigaction: usage is conditionalized
* sigpipe: usage is conditionalized
* stat-time: struct stat is used directly
* strchrnul: usage is eliminated
* strtok_r: usage is not a portability problem
* sys_stat: usage is conditionalized
* uname: rewritten to use native Win32 function to
get host arch
* waitpid: usage is conditionalized
* wcwidth: rewritten using g_unichar APIs
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The EAI_ADDRFAMILY constant has been removed from FreeBSD
headers, supposedly because it is deprecated by new RFC
drafts.
Previously GNULIB was providing a replacement because
MinGW lacked it too. The replacement provided for MinGW
was thus being used on FreeBSD too, but with a completely
bogus integer value.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There are a large number of different header files that
are related to the sockets APIs. The virsocket.h header
includes all of the relevant headers for Windows and UNIX
in one convenient place. If virsocketaddr.h is already
included, then there's no need for virsocket.h
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
chown and some stat constants are not available on
the Windows platform.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The O_BINARY flag is not defined on all platforms so we must
conditionalize its use once we remove GNULIB.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The O_DIRECT flag is not available on all platforms, so we
must introduce a compat define the same way gnulib does.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The F_DUPFD_CLOEXEC functionality is not available on
some platformms. We must thus explicitly call the
virSetCloexec function once we remove GNULIB's equiv
fix for this.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Windows uses _O_NOINHERIT as the name for its O_CLOEXEC
equivalent. Define O_CLOEXEC to match this to fix
portability when we remove GNULIB.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The O_DIRECTORY flag causes open() to return an error
if the filename is a directory. There's no obvious
reason why resctrl needs to use this, while the rest of
libvirt code does not. Removing it avoids build issues
on platforms where O_DIRECTORY is not defined, once we
remove GNULIB.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The S_ISSOCK macro is not available on Windows platforms.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The net/if.h is not portable so we must check for its
existance and avoid using it when missing. Some use
of net/if.h was redundant and could be removed.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Many of the virProcess APIs are relying on GNULIB providing
POSIX API stubs. Even with these stubs the APIs don't do
anything useful once compiled. We can thus conditionalize
the code so that we don't compile anything at all.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Cygwin is not a supported build platform for libvirt and
has no testing coverage in our CI systems. Stop pretending
the code is usable and remove it so there is less to port
to Meson.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
A large part of the virCommand code is still built on
WIN32, despite the fact that the core fork() & execve()
functions are not available. So despite succesfully
building most of the code, at runtime the APIs are
none the less unusuable. With the elimination of GNULIB
many of the APIs being used in this code no longer have
portability wrappers/shims for Windows.
Rather than try to add portability wrappers, or do tests
for each individual function, it is clearer to conditionalize
nearly all of the code using #ifdef WIN32.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
g_mkdir() provides portability to Windows platforms.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The sys/uio.h header is only needed when building logging
code with journald support enabled. Conditionally include
it so that we avoid break on platforms which lack this
header.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The Windows platform does not have the signal handling
support we need, so it must be disabled in several parts
of the codebase.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is a simplified variant of gnulib's passfd module
without the portability code that we do not require.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The gnulib stat-time.h header provides wrapper functions
to hide the differences in 'struct stat' between various
platforms.
Linux and FreeBSD support the same names, except for
birthtime which Linux only provides in the new 'statx()'
syscall we're not using. macOS has completely different
naming. Since we only rely on this code in one place
we just use conditionals at time of need.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virProcess code relies on windows.h and is getting it
indirectly via some GNULIB header fixes. This dependancy
needs to be made explicit.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The mgetgroups function is a GNULIB custom wrapper around
getgrouplist(). This implements a simplified version of
that code directly.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The strchrnul function doesn't exist on Windows and rather
than attempt to implement it, it is simpler to just avoid
its usage, as any callers are easily adapted.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This imports a simpler version of GNULIB's getpass() function
impl for Windows. Note that GNULIB's impl was buggy as it
returned a static string on UNIX, and a heap allocated string
on Windows. This new impl always heap allocates.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Instead of relying on GNULIb's uname() impl, directly use the
Windows API for determining CPU architecture.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Files inside /dev/vfio/ can't be opened more than once, meaning
that any subsequent open calls will fail. This behavior was
introduced in kernel v3.11, commit 6d6768c61b39.
When using the VFIO driver, we open a FD to /dev/vfio/N and
pass it to QEMU. If any other call attempt for the same
/dev/vfio/N happens while QEMU is still using the file, we are
unable to open it and QEMU will report -EBUSY. This can happen
if we hotplug a PCI hostdev that belongs to the same IOMMU group
of an existing domain hostdev.
The problem and solution is similar to what we already dealt
with for TPM in commit 4e95cdcbb3. This patch changes both
DAC and SELinux drivers to disable 'remember' for VFIO hostdevs
in virSecurityDACSetHostdevLabelHelper() and
virSecurityDACSetHostdevLabel(), and 'recall'
in virSecurityDACRestoreHostdevLabel() and
virSecuritySELinuxRestoreHostdevSubsysLabel().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There is a case in which we do not want 'remember' to be
set to true in SetOwnership() calls inside the
HostdevLabelHelper() functions of both DAC and SELinux drivers.
Next patch will explain and handle that scenario.
For now, let's make virSecurityDACSetOwnership() and
virSecuritySELinuxSetHostdevLabelHelper() accept a 'remember'
flag, which will be used to set the 'remember' parameter
of their respective SetOwnership() calls. No functional
change is made.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently when disk is removed from iotune group (by setting
all tunables to zero) group name is leaved in config. Let's fix
it.
Given iotune defaults are taken from the destination group setting
tunables to zero may require different set of zero settings in API
call. Let's prohibit removing from group while specifying different
group name then current for the sanity sake.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For example if disk is not in the group and we want to move it
there then it makes sense to specify only the group name in API call.
Currently the destination group iotune settings will be overwritten
with the disk settings which I would say is not what one would expect.
Thus let's get defaults from the group we are moving to.
And if we are moving the brand new group then is makes sense to
copy the current disk iotune settings to the group.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virDomainSetBlockIoTune not simply sets the iotune params given in API
but use current settings for all the omitted params. Unfortunately
it uses current settings for active config when setting inactive
params. Let's fix it.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently it is possible to start a domain which have disks
in same iotune group and at the same time having different iotune
params. Both params set are passed to qemu in command line and the one
that is passed later down command line is get actually set.
Let's prohibit such configurations.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently upon successfull call to qemu's implementation of
virDomainSetBlockIoTune iotune settings are changed only for the
disk given in API if the disk is in iotune group while we need
to change the settings for all disks in the group.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, if only iotune group name is given for some disk and
no any params then later start of domain will fail. I guess it
will be convenient to allow such configuration if there is
another disk in the same iotune group with iotune params set. The
meaning is that the first disk have same iotunes and the latter.
Thus one can easily add a disk to iotune group - just add group
name parameter and no need to copy all the params.
Also let's expand iotunes params in the described case so we don't
need to refer to another disk to know iotunes and this will make
logic in many places simple.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
And introduce virDomainBlockIoTuneInfoHasAny.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some *ParseXML functions have comments stating what kind of device
they parse with an outdated list of parameters, with the exception
of virDomainFSDefParseXML which claims to parse a disk.
Remove them, assuming the function names are descriptive enough.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
All the callees return either 0 or -1 so there is no need
for propagating the value. And we bail on the first error.
Remove the variable to make the function simpler.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We have a helper variable to make the code more concise,
use it consistently.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that the cleanup section is empty, eliminate the cleanup
label as well as the 'ret' variable.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Use the g_auto macros wherever possible to eliminate the cleanup
section.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Use the virXMLFormatElement helper to format the driver element
to simplify adding further sub-elements.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The template still references libvirt-qemu-shim, which was at one
point the name used to refer to what we now know as virt-qemu-run.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
A recent commit added an error check for too-nested backing chains
followed by a return, even though errors above jump to cleanup.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: b168fa88b8
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Remove bogus G_GNUC_UNUSED attribute and add a missing space.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: d600667278
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The algorithm is used in two places to find the parent checkpoint object
which contains given disk and then uses data from the disk. Additionally
the code is written in a very non-obvious way. Factor out the lookup of
the disk into a function which also simplifies the callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The function has no users now and there's no need for it as the common
pattern is to look up the whole disk object anyways.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
If a disk is unplugged and then the user tries to delete a checkpoint
the code would try to use NULL node name as it was not checked.
Fix this by fetching the whole disk definition object and verifying it
was found.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Lookup the whole disk definition rather than just the node name.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Upcoming patches will also use the domain disk definition. Rename disk
to chkdisk for clarity.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Upcoming patches will also use the domain disk definition. Rename disk
to chkdisk for clarity.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
qemuCheckpointDiscard is a massive function that can be separated into
smaller bits. Extract the part that actually modifies the disk from the
metadata handling.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Test code will need to know whether the virQEMUCaps object contains any
machine types already. Add a helper and expose it via 'qemu_capspriv.h'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The previous approac of just purging the alias combined with the fact
that we filled in fake machine types in the test data meant that if a
test case used an alias machine type such as 'pc' or 'q35' it would not
properly resolve to the actual data returned by qemu.
This started to be a problem since the CPU driver now looks at the
default CPU reported with the machine type.
This patch replaces the original approach of just removing the alias by
replacing it with a copy of the machine type data which the type would
alias to. This means that we are using the real data while we don't
modify the test output after every qemu upgrade.
Additionally this change will allow us to drop adding the fake machine
types later.
The test fallout is from actually excercising the CPU driver with
actual data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Separate out the internals as they will become more complex soon.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Every supported qemu is able to return the list of machine types it
supports so we can start validating it against that list. The advantage
is a better error message, and the change will also prevent having stale
test data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Debian/Ubuntu linkers are more strict that other distros requiring glib
to be linked explicitly.
macOS needs -export-dynamic instead of -Wl,--export-dynamic
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Similarly to 510d154a0b we need to prevent
doing too deeply nested backing chains and reject them with a sane error
message.
Add a loop to go through the snapshots prior to attempting actually
creating them to prevent some possible inconsistent scenarios.
We don't need to do it when reusing backing chains as we'll be
re-detecting the backing chain in that case anyways.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Don't adopt the backing store data when reusing images provided by the
user. This will force a backing chain re-probe as users might have
passed in something unexpected in the overlay where our view of the
backing chain would not correspond.
This is done only for inactive snapshots as there we have way less
verification.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The previous "QEMU shim" proof of concept was taking an approach of only
caring about initial spawning of the QEMU process. It was then
registered with the libvirtd daemon who took over management of it. The
intent was that later libvirtd would be refactored so that the shim
retained control over the QEMU monitor and libvirt just forwarded APIs
to each shim as needed. This forwarding of APIs would require quite alot
of significant refactoring of libvirtd to achieve.
This impl thus takes a quite different approach, explicitly deciding to
keep the VMs completely separate from those seen & managed by libvirtd.
Instead it uses the new "qemu:///embed" URI scheme to embed the entire
QEMU driver in the shim, running with a custom root directory.
Once the driver is initialization, the shim starts a VM and then waits
to shutdown automatically when QEMU shuts down, or should kill QEMU if
it is terminated itself. This ought to use the AUTO_DESTROY feature but
that is not yet available in embedded mode, so we rely on installing a
few signal handlers to gracefully kill QEMU. This isn't reliable if
we crash of course, but you can restart with the same root dir.
Note this program does not expose any way to manage the QEMU process,
since there's no RPC interface enabled. It merely starts the VM and
cleans up when the guest shuts down at the end. This program is
installed to /usr/bin/virt-qemu-run enabling direct use by end users.
Most use cases will probably want to integrate the concept directly
into their respective application codebases. This standalone binary
serves as a nice demo though, and also provides a way to measure
performance of the startup process quite simply.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This enables support for running the secret driver embedded to the
calling application process using a URI:
secret:///embed?root=/some/path
When using the embedded mode with a root=/var/tmp/embed, the
driver will use the following paths:
configDir: /var/tmp/embed/etc/secrets
stateDir: /var/tmp/embed/run/secrets
These are identical whether the embedded driver is privileged
or unprivileged.
This compares with the system instance which uses
configDir: /etc/libvirt/secrets
stateDir: /var/lib/libvirt/secrets
When an embedded instance of the secret driver is open, any other
embedded drivers will automatically use the embedded secret driver.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This enables support for running QEMU embedded to the calling
application process using a URI:
qemu:///embed?root=/some/path
Note that it is important to keep the path reasonably short to
avoid risk of hitting the limit on UNIX socket path names
which is 108 characters.
When using the embedded mode with a root=/var/tmp/embed, the
driver will use the following paths:
logDir: /var/tmp/embed/log/qemu
swtpmLogDir: /var/tmp/embed/log/swtpm
configBaseDir: /var/tmp/embed/etc/qemu
stateDir: /var/tmp/embed/run/qemu
swtpmStateDir: /var/tmp/embed/run/swtpm
cacheDir: /var/tmp/embed/cache/qemu
libDir: /var/tmp/embed/lib/qemu
swtpmStorageDir: /var/tmp/embed/lib/swtpm
defaultTLSx509certdir: /var/tmp/embed/etc/pki/qemu
These are identical whether the embedded driver is privileged
or unprivileged.
This compares with the system instance which uses
logDir: /var/log/libvirt/qemu
swtpmLogDir: /var/log/swtpm/libvirt/qemu
configBaseDir: /etc/libvirt/qemu
stateDir: /run/libvirt/qemu
swtpmStateDir: /run/libvirt/qemu/swtpm
cacheDir: /var/cache/libvirt/qemu
libDir: /var/lib/libvirt/qemu
swtpmStorageDir: /var/lib/libvirt/swtpm
defaultTLSx509certdir: /etc/pki/qemu
At this time all features present in the QEMU driver are available when
running in embedded mode, availability matching whether the embedded
driver is privileged or unprivileged.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The driver URI scheme:
"$drivername:///embed?root=/some/path"
enables a new way to use the drivers by embedding them directly in the
calling process. To use this the process must have a thread running the
libvirt event loop. This URI will then cause libvirt to dynamically load
the driver module and call its global initialization function. This
syntax is applicable to any driver, but only those will have been
modified to support a custom root directory and embed URI path will
successfully open.
The application can now make normal libvirt API calls which are all
serviced in-process with no RPC layer involved.
It is required to specify an explicit root directory, and locks will be
acquired on this directory to avoid conflicting with another app that
might accidentally pick the same directory.
Use of '/' is not explicitly forbidden, but note that the file layout
used underneath the embedded driver root does not match the file
layout used by system/session mode drivers. So this cannot be used as
a backdoor to interact with, or fake, the system/session mode drivers.
Libvirt will create arbitrary files underneath this root directory. The
root directory can be kept untouched across connection open attempts if
the application needs persistence. The application is responsible for
purging everything underneath this root directory when finally no longer
required.
Even when a virt driver is used in embedded mode, it is still possible
for it to in turn use functionality that calls out to other secondary
drivers in libvirtd. For example an embedded instance of QEMU can open
the network, secret or storage drivers in the system libvirtd.
That said, the application would typically want to at least open an
embedded secret driver ("secret:///embed?root=/some/path"). Note that
multiple different embedded drivers can use the same root prefix and
co-operate just as they would inside a normal libvirtd daemon.
A key thing to note is that for this to work, the application that links
to libvirt *MUST* be built with -Wl,--export-dynamic to ensure that
symbols from libvirt.so are exported & thus available to the dynamically
loaded driver module. If libvirt.so itself was dynamically loaded then
RTLD_GLOBAL must be passed to dlopen().
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The intent here is to allow the virt drivers to be run directly embedded
in an arbitrary process without interfering with libvirtd. To achieve
this they need to store all their configuration & state in a separate
directory tree from the main system or session libvirtd instances.
This can be useful for doing testing of the virt drivers in "make check"
without interfering with the user's own libvirtd instances.
It can also be used for applications using KVM/QEMU as a piece of
infrastructure to build an service, rather than for general purpose
OS hosting. A long standing example is libguestfs, which would prefer
if its temporary VMs did show up in the main libvirtd VM list, because
this confuses apps such as OpenStack Nova. A more recent example would
be Kata which is using KVM as a technology to build containers.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
If a domain is configured to have an egl-headless display and a virtio
video device, virgl will be enabled automatically within the guest, even
if the video device is configured with accel3d='no'.
In this case we should explicitly pass 'virgl=off' to qemu.
See https://bugzilla.redhat.com/show_bug.cgi?id=1791236 for more
information.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since v4.2-rc0, QEMU introduced a builtin rng backend that uses
getrandom() syscall to generate random. Add it to libvirt with the
backend model 'builtin'.
https://bugzilla.redhat.com/show_bug.cgi?id=1785091
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'builtin' rng backend model can be used as following:
<rng model='virtio'>
<backend model='builtin'/>
</rng>
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For qemu object like rng-builtin, there are no properties after id
property. We should always set comma after object id. Otherwise it will
cause trailing comma on object:
-object rng-builtin,id=ID,
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It is used to check if qemu is capable of rng-builtin object.
This object is added since qemu-4.2.0-rc0, commit 6c4e9d48.
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since v5.6.0-48-g270583ed98 we try to cache domain capabilities,
i.e. store filled virDomainCaps in a hash table in virQEMUCaps
for future use. However, there's a race condition in the way it's
implemented. We use virQEMUCapsGetDomainCapsCache() to obtain the
pointer to the hash table, then we search the hash table for
cached data and if none is found the domcaps is constructed and
put into the table. Problem is that this is all done without any
locking, so if there are two threads trying to do the same, one
will succeed and the other will fail inserting the data into the
table.
Also, the API looks a bit fishy - obtaining pointer to the hash
table is dangerous.
The solution is to use a mutex that guards the whole operation
with the hash table. Then, the API can be changes to return
virDomainCapsPtr directly.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1791790
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When fixing [1] I've ran attached reproducer and had it spawn
1024 threads and query capabilities XML in each one of them. This
lead libvirtd to hit the RLIMIT_NOFILE limit which was kind of
expected. What wasn't expected was a subsequent segfault. It
happened because virCPUProbeHost failed and returned NULL. We've
taken the NULL and passed it to virCapabilitiesHostNUMARef()
which dereferenced it. Code inspection showed the same flas in
virQEMUDriverGetHostNUMACaps(), so I'm fixing both places.
1: https://bugzilla.redhat.com/show_bug.cgi?id=1791790
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virCapabilitiesGetNodeInfo() function has the usual return
value semantics for integeres: a negative value means an error,
zero or a positive value means success. However, the function
call done in virCPUProbeHost() doesn't check for the return value
accordingly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Don't use ERANGE as it doesn't make much sense in the error message.
Also point out that the reply from qemu was too large which is not
obvious from the original error:
error: No complete monitor response found in 10485760 bytes: Numerical result out of range
The new message will read:
error: internal error: QEMU monitor reply exceeds buffer size (10485760 bytes)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
libvirt treats 'luks' images as raw+encryption. The logic in
qemuBlockStorageSourceCreateFormat skipped the creation if the requested
image was raw but didn't take into account the encryption.
This manifested itself e.g. when attempting to do a virsh blockcopy with
the following XML:
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/tmp/enccpy'>
<encryption format='luks'>
<secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
</encryption>
</source>
</disk>
Where qemu would report the following error:
unable to execute QEMU command 'blockdev-add': Volume is not in LUKS format
rather than actually formatting the image first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
If we get a user reporting this error message being shown it's pretty
useless in terms of actually debugging it since we don't know which hash
and which key are actually subject to the error.
This patch adds a new hash table callback which formats the
user-readable version of the hash key and reports it in the new message
which will look like:
"Duplicate hash table key 'blah'"
That way we will at least have an anchor point where to start the
search.
There are two special implementations of keys which are numeric so we
add specific printer functions for them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the user-configured name of the bitmap when merging the appropriate
bitmaps for an incremental backup so that the user can see it as
configured. Additionally expose the default bitmap name if nothing is
configured.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Pass the exportname as configured when exporting the image via NBD and
fill it with the default if it's not configured.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
If users wish to use different name for exported disks or bitmaps
the new fields allow to do so. Additionally they also document the
current settings.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
When using blockdev configurations the 'device' argument of
'blockdev-commit' must correspond to the topmost node in the block node
graph. Libvirt didn't do this properly in case when 'copy_on_read'
option was enabled on the disk.
Use qemuDomainDiskGetTopNodename to fix it when calling block-commit.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
When using blockdev configurations the 'device' argument of
'blockdev-mirror' must correspond to the topmost node in the block node
graph. Libvirt didn't do this properly in case when 'copy_on_read'
option was enabled on the disk.
Use qemuDomainDiskGetTopNodename to fix it for the blockdev-mirror calls
in qemuDomainBlockCopy and the non-shared-storage migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
There are more places which require getting the topmost nodename to be
passed to qemu. Separate it out into a new function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
If a mirror job fails to start in -blockdev mode we'd not unplug the
backing files we added first because the code on the error path checked
the wrong value. 'rc' is used as status of the code which added the
images, but the state of the 'block(dev)-mirror' call is stored in 'ret'
at that point.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The virConnectGetDomainCapabilities API accepts either a binary path
to the emulator, or desired guest arch. If guest arch is not given,
then the host arch is assumed.
In the case where the binary is not given, the code tried to find the
emulator binary in the existing list of cached emulator capabilities.
This is not valid since we switched to lazy population of the cache in:
commit 3dd91af01f
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Mon Dec 2 13:04:26 2019 +0000
qemu: stop creating capabilities at driver startup
As a result of this change, if there are no persistent guests defined
using the requested guest architecture, virConnectGetDomainCapabilities
will fail to find an emulator binary.
The solution is to stop relying on the cached capabilities to find the
binary and instead use the same logic we use to pick default a binary
per arch when populating capabilities.
Tested-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The "ps2" bus is only available on certain machines like x86. On
machines like s390x, we should refuse to add a device to this bus
instead of silently ignoring it.
Looking at the QEMU sources, PS/2 is only available if the QEMU binary
has the "i8042" device, so let's check for that and only allow "ps2"
devices if this QEMU device is available, or if we're on x86 anyway
(so we don't have to fake the QEMU_CAPS_DEVICE_I8042 capability in
all the tests that use <input ... bus='ps2'/> in their xml data).
Reported-by: Sebastian Mitterle <smitterl@redhat.com>
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1763191
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
LXC driver is not able to retrieve IP addresses from domains. This
function was not implemented yet. It can be done using DHCP lease and
ARP table. Different from QEMU, LXC does not have an agent to fetch
this info, but other sources can be used.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU driver has two functions: qemuGetDHCPInterfaces() and
qemuARPGetInterfaces() that are being used inside only one single
function. They can be turned into generic functions that other drivers
can use. This commit move both from QEMU driver tree to domain conf
tree.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Simplify function logic by using g_autofree to free local variables so
that we can remove some goto statements that are used for cleanup.
Introduce a g_autoptr cleanup function for virNodeDeviceDef.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Since commit <60d9ad6f1e42618fce10baeb0f02c35e5ebd5b24> we require
GnuTLS and since commit <ac0d21c762351f58dd5d2dafa2014ed48a8b49f3>
we can actually drop the usage of WITH_GNUTLS.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The libxl driver already tries to call shutdown inhibit callback in the
right places, but only if it's set. That last part was missing,
resulting in premature shutdown when running libvirtd
--timeout=...
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The function virSecretGetSecretString calls into secret driver and is
used from other hypervisors drivers and as such makes more sense in
util.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Remove many imports of sys/ioctl.h which are redundant,
and conditionalize remaining usage that needs to compile
on Windows platforms.
The previous change to remove the "nonblocking" gnulib
module indirectly caused the loss of the "ioctl" gnulib
module that we did not explicitly list in bootstrap.conf
despite relying on.
Rather than re-introduce the "ioctl" module this patch
makes it redundant.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This fixes a build bug introduced by
commit fbf27730a3
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Mon Dec 16 11:16:51 2019 +0000
conf: add support for specifying CPU "dies" parameter
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When parsing legacy NBD backing file strings such as
'nbd:unix:/tmp/sock:exportname=/' we'd fail to set the transport to
VIR_STORAGE_NET_HOST_TRANS_UNIX. This started to be a problem once we
actually started to generate config of the backing store on the command
line with -blockdev as the JSON code would try to format it as TCP and
fail with:
internal error: argument key 'host' must not have null value
Set the type properly and add a test.
This bug was found by the libguestfs test suite in:
https://bugzilla.redhat.com/show_bug.cgi?id=1791614
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reported-by: Ming Xie <mxie@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
gmtime_r/localtime_r are mostly used in combination with
strftime to format timestamps in libvirt. This can all
be replaced with GDateTime resulting in simpler code
that is also more portable.
There is some boundary condition problem in parsing POSIX
timezone offsets in GLib which tickles our test suite.
The test suite is hacked to avoid the problem. The upsteam
GLib bug report is
https://gitlab.gnome.org/GNOME/glib/issues/1999
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The GNULIB termios module ensures termios.h exists (but
is none the less empty) when building for Windows. We
already exclude usage of the functions that would exist
in a real termios.h, so having an empty termios.h is
not especially useful.
It is simpler to just put all use of termios.h related
functions behind a "#ifndef WIN32" conditional.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
G_STATIC_ASSERT() is a drop-in functional equivalent of
the GNULIB verify() macro.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Libvirt's original atomic ops impls were largely copied
from GLib's code at the time. The only API difference
was that libvirt's virAtomicIntInc() would return a
value, but g_atomic_int_inc was void. We thus use
g_atomic_int_add(v, 1) instead, though this means
virAtomicIntInc() now returns the original value,
instead of the new value.
This rewrites libvirt's impl in terms of g_atomic_int*
as a short term conversion. The key motivation was to
quickly eliminate use of GNULIB's verify_expr() macro
which is not a direct match for G_STATIC_ASSERT_EXPR.
Long term all the callers should be updated to use
g_atomic_int* directly.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We don't need all the platforms gnulib deals with, so
this is a cut down version of GNULIB's physmem.c
code. This also allows us to integrate libvirt's
error reporting functions closer to the error cause.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Convert to use socket wrappers. Aside from the header file
include change, this requires changing close -> closesocket
since our portability isn't trying to replace the close
function.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Windows sockets take a SOCKET HANDLE object instead of a
file descriptor. Wrap them in the same way that gnulib
does so that they use C runtime file descriptors.
While we could in theory use GSocket, it is hard to get
the exact same semantics libvirt has for its current
socket usage. Wrapping the Winsock2 APIs is thus the
easiest approach in the short term.
In changing the socke wrappers we need to re-implement
the nonblocking function too, since the GNULIB impl
expects to be used with the GNULIB sockets wrappers.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
All UNIX platforms we care about have openpty() in the libutil
library. Use of pty.h must also be made conditional, excluding
Win32.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The GLib g_size_checked_mul() function is not quite the
same signature, and gives compiler warnings due to not
correctly casting from gsize to guint64/32. Implementing
a replacement for INT_MULTIPLY_OVERFLOW is easy enough
to do ourselves.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Introduce a vastly simpler VIR_INT64_STR_BUFLEN constant
which is large enough for all cases where we currently
use INT_BUFSIZE_BOUND. This eliminates most use of the
gnulib intprops.h header.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
RHEL7 has libcurl 7.29.0, which is the oldest of any
supported build platform. Thus we no longer need the
back compat for libcurl < 7.28.0.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Switch from old VIR_ allocation APIs to glib equivalents.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This function potentially grabs both a monitor job and an agent job at
the same time. This is problematic because it means that a malicious (or
just buggy) guest agent can cause a denial of service on the host. The
presence of this function makes it easy to do the wrong thing and hold
both jobs at the same time. All existing uses have already been removed
by previous commits.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In order to avoid holding an agent job and a normal job at the same
time, we want to avoid accessing the domain's definition while holding
the agent job. To achieve this, qemuAgentGetFSInfo() only returns the
raw information from the agent query to the caller. The caller can then
release the agent job and then proceed to look up the disk alias from
the vm definition. This necessitates moving a few helper functions to
qemu_driver.c and exposing the agent data structure (qemuAgentFSInfo) in
the header.
In addition, because the agent function no longer returns the looked-up
disk alias, we can't test the alias within qemuagenttest. Instead we
simply test that we parse and return the raw agent data correctly.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The qemuAgentDiskInfo structure is filled with information received from
the agent command response, except for the 'alias' field, which is
retrieved from the vm definition. Limit this structure only to data that
was received from the agent message.
This is another intermediate step in moving the responsibility for
searching the vmdef from qemu_agent.c to qemu_driver.c so that we can
avoid holding an agent job and a normal job at the same time.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In an effort to avoid holding both an agent and normal job at the same
time, we shouldn't access the vm definition from within qemu_agent.c
(i.e. while the agent job is being held). In preparation, we need to
store the full filesystem disk information in qemuAgentDiskInfo. In a
following commit, we can pass this information back to the caller and
the caller can search the vm definition to match the filsystem disk to
an alias.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function name doesn't give a good idea of what the function does.
Rename to qemuAgentGetFSInfoFillDisks() to make it more obvious than it
is filling in the disk information in the fsinfo struct.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Update the host CPU code to report the die_id in the NUMA topology
capabilities. On systems with multiple dies, this fixes the bug
where CPU cores can't be distinguished:
<cpus num='12'>
<cpu id='0' socket_id='0' core_id='0' siblings='0'/>
<cpu id='1' socket_id='0' core_id='1' siblings='1'/>
<cpu id='2' socket_id='0' core_id='0' siblings='2'/>
<cpu id='3' socket_id='0' core_id='1' siblings='3'/>
</cpus>
Notice how core_id is repeated within the scope of the same socket_id.
It now reports
<cpus num='12'>
<cpu id='0' socket_id='0' die_id='0' core_id='0' siblings='0'/>
<cpu id='1' socket_id='0' die_id='0' core_id='1' siblings='1'/>
<cpu id='2' socket_id='0' die_id='1' core_id='0' siblings='2'/>
<cpu id='3' socket_id='0' die_id='1' core_id='1' siblings='3'/>
</cpus>
So core_id is now unique within a (socket_id, die_id) pair.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
QEMU since 4.1.0 supports the "dies" parameter for -smp
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Recently CPU hardware vendors have started to support a new structure
inside the CPU package topology known as a "die". Thus the hierarchy
is now:
sockets > dies > cores > threads
This adds support for "dies" in the XML parser, with the value
defaulting to 1 if not specified for backwards compatibility.
For example a system with 64 logical CPUs might report
<topology sockets="4" dies="2" cores="4" threads="2"/>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When pause-before-switchover QEMU capability is enabled, we get STOP
event before MIGRATION event with postcopy-active state. To properly
handle post-copy migration and emit correct events commit
v4.10.0-rc1-4-geca9d21e6c added a hack to
qemuProcessHandleMigrationStatus which translates the paused state
reason to VIR_DOMAIN_PAUSED_POSTCOPY and emits
VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY event when migration state changes
to post-copy.
However, the code was effective on both sides of migration resulting in
a confusing VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY event on the destination
host, where entering post-copy mode is already properly advertised by
VIR_DOMAIN_EVENT_RESUMED_POSTCOPY event.
https://bugzilla.redhat.com/show_bug.cgi?id=1791458
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is only a theoretical leak, but in virChrdevAlloc() we
initialize a mutex and if creating a hash table fails,
then virChrdevFree() is called which because of incorrect check
doesn't deinit the mutex.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
When opening a console to a domain, we put a tuple of {path,
virStreamPtr} into a hash table that's private to the domain.
This is to ensure only one client at most has the console stream
open. Later, when the console is closed, the tuple is removed
from the hash table and freed. Except, @path won't be freed.
==234102== 60 bytes in 5 blocks are definitely lost in loss record 436 of 651
==234102== at 0x4836753: malloc (vg_replace_malloc.c:307)
==234102== by 0x5549110: g_malloc (in /usr/lib64/libglib-2.0.so.0.6000.6)
==234102== by 0x5562D1E: g_strdup (in /usr/lib64/libglib-2.0.so.0.6000.6)
==234102== by 0x4A5A917: virChrdevOpen (virchrdev.c:412)
==234102== by 0x17B64645: qemuDomainOpenConsole (qemu_driver.c:17309)
==234102== by 0x4BC8031: virDomainOpenConsole (libvirt-domain.c:9662)
==234102== by 0x13F854: remoteDispatchDomainOpenConsole (remote_daemon_dispatch_stubs.h:9211)
==234102== by 0x13F72F: remoteDispatchDomainOpenConsoleHelper (remote_daemon_dispatch_stubs.h:9178)
==234102== by 0x4AB0685: virNetServerProgramDispatchCall (virnetserverprogram.c:430)
==234102== by 0x4AB01F0: virNetServerProgramDispatch (virnetserverprogram.c:302)
==234102== by 0x4AB700B: virNetServerProcessMsg (virnetserver.c:136)
==234102== by 0x4AB70CB: virNetServerHandleJob (virnetserver.c:153)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
When resuming a domain from a save file, we read the domain XML
from the file, add it onto our internal list of domains, start
the qemu process, let it load the incoming migration stream and
resume its vCPUs afterwards. If anything goes wrong, the domain
object is removed from the list of domains and error is returned
to the caller. However, the qemu process might be left behind -
if resuming vCPUs fails (e.g. because qemu is unable to acquire
write lock on a disk) then due to a bug the qemu process is not
killed but the domain object is removed from the list.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1718707
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Since there is no guest agent in LXC world (yet), we can
implement _LEASE flag only.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
We have to keep the default - querying the agent if no flag is
set.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
There is a lots of possibilities to retrieve hostname information
from domain. Libvirt could use lease information from dnsmasq to
get current hostname too. QEMU supports QEMU-agent but it can use
lease source.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
As of systemd commit:
commit d65652f1f21a4b0c59711320f34266c635393c89
Author: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
CommitDate: 2018-12-10 09:56:56 +0100
Partially unify hostname_is_valid() and dns_name_is_valid()
Dashes are no longer allowed at the end of machine names.
Trim the trailing dashes from the generated name before passing
it to machined.
https://bugzilla.redhat.com/show_bug.cgi?id=1790409
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
A new helper for trimming combinations of specified characters from
the tail of the buffer.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This leaks the FD of BPF map which means it will not be freed.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
libvirt currently always reports that USB is available as a bus subsystem
type when running "virsh domcapabilities". However, this is not always
true, for example the qemu-system-s390x binary normally never has support
for USB. Thus we should only report that USB is available if there is
also a USB host controller available where we can attach USB devices.
Reported-by: Sebastian Mitterle <smitterl@redhat.com>
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1759849
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When trying to specify an input device on s390x without bus like this:
<input type='keyboard'/>
... then libvirt currently complains:
error: unsupported configuration: USB is disabled for this domain,
but USB devices are present in the domain XML
This is somewhat confusing since the user did not specify an USB
device here. Since USB is not available on s390x, we should default
to the "virtio" bus here instead.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1790189
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Historically there are two places where we format authentication and
encryption for a disk. The logich which formats it for backing files was
flawed though and didn't format it at all. This worked if the image
became a backing file through the means of a snapshot but not directly.
Force formatting of the source and encryption for any non-disk case to
fix the issue.
This caused problems in many places as we use the formatter to copy the
definition. Effectively any copy lost the secret definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1789310https://bugzilla.redhat.com/show_bug.cgi?id=1788898
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In v5.0.0-rc1~94 we switched from one huge switch() to an array
for translating error numbers into error messages. However, the
array is declared to have VIR_ERR_NUMBER_LAST items which makes
it impossible to spot this place by compile checking when adding
new error number.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Mention the knowledge base article which has tips how to fix the backing
chain to work with current libvirt.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Internal snapshots of a non-running domain do not carry any memory state
and restoring such a snapshot will not replace existing saved memory
state. This allows a scenario, where a user first suspends a domain into
managedsave, restores a non-running snapshot and then resumes the domain
from managedsave. After that, the guest system will run with its
previous memory state atop a different disk state. The most obvious
possible fallout from this is extensive file system corruption. Swap
content and RAID bitmaps might also be off.
This has been discussed[1] and fixed[2] from the end-user perspective for
virt-manager.
This patch marks the restore operation as risky at the libvirt level,
requiring the user to remove the saved memory state first or force the
operation.
[1] https://www.redhat.com/archives/virt-tools-list/2019-November/msg00011.html
[2] https://www.redhat.com/archives/virt-tools-list/2019-December/msg00049.html
Signed-off-by: Michael Weiser <michael.weiser@gmx.de>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit v5.10.0-290-g3a4787a301 refactored qemuDomainGetHostdevPath to
return a single path rather than an array of paths. When the function is
called on a missing device, it will now return NULL in @path rather than
a NULL array with zero items and the callers need to be adapted
properly.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
bhyveargv2xmlmock calls virBhyveCapsBuild which in turn
calls virCPUProbeHost, probing the real host CPU. This
causes a test failure if the host CPU happens to contain
the 'arch-capabilities' feature as it triggers a call
to virHostCPUGetMSR() which fails on FreeBSD.
Fortunately we already have convenient code for mocking
the host CPU probing.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In v5.10.0-508-gfbf3f3d86a, the 'error' label was removed from
bhyveParseBhyveCommandLine(), however the CONSUME_ARG() macro
still uses it. Fix the macro to return an error instead.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
src/qemu/qemu_domain_address.c:680:13: error: this statement may fall through [-Werror=implicit-fallthrough=]
switch ((virDomainFSModel) dev->data.fs->model) {
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: f363af7e35
Commit v5.10.0-522-g9000b2f298 was too aggressive and removed the
'error' label from prlsdkRemoveBootDevices() even though it's
used. Luckily, it's used only from one place and we have an
alternative for it that doesn't require the label.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The point of this function is to translate virDomainOsDefFirmware
enum to qemuFirmwareOSInterface enum. However, with my commit
v5.10.0-507-g8e1804f9f6 we are passing a variable type of
virDomainLoader enum. Make the function accept both enums and
make the enum members correspond to each other.
This fixes clang build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Split the formatting by fsdriver type to allow adding a new type.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Split the switch by fsdriver type to allow adding a new one.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Wire up the allocation and disposal of private data.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Add an object to hold the private data and call the
allocation function if it's present in xmlopt.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This will be needed in the future for allocating private data.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Remove the 'gluster' part and decouple the return from
the gluster_debug_level parsing to allow adding more options
to this section.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Inactive VM doesn't have qemuCaps set thus we'd never properly report
that VM backups are supported only for running VMs.
Move the capability check after the active check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This is not strictly needed, but it makes sure we initialize the
@bootTime global variable. Thing is, in order to validate XATTRs
and prune those set in some previous runs of the host, a
timestamp is recorded in XATTRs. The host boot time was unique
enough so it was chosen as the timestamp value. And to avoid
querying and parsing /proc/uptime every time, the query function
does that only once and stores the boot time in a global
variable. However, the only time the query function is called is
in a child process that does lock files and changes seclabels. So
effectively, we are doing exactly what we wanted to prevent from
happening.
The fix is simple, call the virHostBootTimeInit() function which
sets the global variable.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
The idea is to offer callers an init function that they can call
independently to ensure that the global variables get
initialized.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
After one of previous commits (v5.10.0-524-gce56408e5f) there is
a variable left unused in verify_xpath_context() which breaks the
build.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
