Add support to filter by 'ap_card' and 'ap_queue' capabilities.
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Each AP card device can support upto 256 AP queues. AP queues are
also detected by udev, so add support for libvirt nodedev driver.
https://www.kernel.org/doc/html/latest/s390/vfio-ap.html#ap-architectural-overview
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Introduce support for the Adjunct Processor (AP) crypto card device.
Udev already detects the device, so add support for libvirt nodedev
driver.
https://www.kernel.org/doc/html/latest/s390/vfio-ap.html#ap-architectural-overview
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Create a new function called virDomainDefIdMapValidate() and
use it to move these checks out of virDomainIdmapDefParseXML()
and virDomainDefParseXML().
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
virDomainControllerDefParseXML() does a lot of checks with
virDomainPCIControllerOpts parameters that can be moved to
virDomainControllerDefValidate, sharing the logic with other use
cases that does not rely on XML parsing.
'pseries-default-phb-numa-node' parse error was changed to reflect
the error that is being thrown by qemuValidateDomainDeviceDefController()
via deviceValidateCallback, that is executed before
virDomainControllerDefValidate().
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Next patch will add more validations to this function. Let's move
it to domain_validate.c beforehand.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Move this check to a new virDomainDefTunablesValidate(), which
is called by virDomainDefValidateInternal().
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This check is not tied to XML parsing and can be moved to
virDomainSmartcardDefValidate().
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Next patch will move a validation to virDomainSmartcardDefValidate(),
but this function can't be moved alone to domain_validate.c without
making virDomainChrSourceDefValidate(), from domain_conf.c, public.
Given that the idea is to eventually move all validations to domain_validate.c
anyways, let's move all ChrSource related validations in a single punch.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The function isn't doing XML validation of any sort. Rename it to
be compatible with its actual use.
While we're at it, change the VIR_ERR_XML_ERROR error being thrown
in the function to VIR_ERR_CONFIG_UNSUPPORTED.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The 'tray' check isn't a XML parse specific code and can be pushed
to the validate callback, in virDomainDiskDefValidate().
'vendor' and 'product' string sizes are already checked by the
domaincommon.rng schema, but can be of use in the validate callback
since not all scenarios will go through the XML parsing.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Next patch will add more validations to the function. Let's move
it beforehand to domain_validate.c.
virSecurityDeviceLabelDefValidateXML() is still used inside
domain_conf.c, so make it public for now until its current
caller (virDomainChrSourceDefValidate()) is also moved to
domain_validate.c.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
These checks are not related to XML parsing and can be moved to the
validate callback. Errors were changed from VIR_ERR_XML_ERROR to
VIR_ERR_CONFIG_UNSUPPORTED.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
We'll add more video validations into the function in the next
patch. Let's move it beforehand to domain_validate.c.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This check isn't exclusive to XML parsing. Let's move it to
virDomainDefVideoValidate() in domain_validate.c
We don't have a failure test for this scenario, so a new test called
'video-multiple-primaries' was added to test this failure case.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This patch creates a new function, virDomainDefBootValidate(), to host
the validation of boot menu timeout and rebootTimeout outside of parse
time. The checks in virDomainDefParseBootXML() were changed to throw
VIR_ERR_XML_ERROR in case of parse error of those values.
In an attempt to alleviate the amount of code being stacked inside
domain_conf.c, let's put this new function in a new domain_validate.c
file that will be used to place these validations.
Suggested-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Commit 926563dc3a which refactored the function call deleting the
snapshot's on disk state introduced a logic bug, which skips over the
deletion of libvirt metadata after the disk state deletion is done.
To fix it we must not return early.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/109
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
We already calculated the guest area, which is what is subject
to minimum size requirements, a few lines earlier.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The validation callback always fetched a fresh copy of 'qemuCaps' to use
for validation which is wrong in cases when the VM is already running,
such as device hotplug. The newly-fetched qemuCaps may contain flags
which weren't originally in use when starting the VM e.g. on a libvirtd
upgrade.
Since the post-parse/validation machinery has a per-run 'parseOpaque'
field filled with qemuCaps of the actual process we can reuse the caps
in cases when we get them.
The code still fetches a fresh copy if parseOpaque doesn't have a
per-run copy to preserve existing functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The validation callbacks always fetch latest qemuCaps so it won't ever
be NULL. Remove the tautological conditions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virDomainDefPostParse infrastructure has apart from the global opaque
data also per-run data, but this was not duplicated into the validation
callbacks.
This is important when drivers want to use correct run-state for the
validation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit f5e8715a8b added logic which adds some fake job info when qemu
didn't return anything but in such case the job type would not be set.
Since we already have the proper job type recorded in qemuBlockJobDataPtr
which the caller fetched, we can use this it and also remove the lookup
from the disk which was necessary prior to the conversion to
qemuBlockJobDataPtr.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Similarly to other disk-related stuff, the index is useful when you want
to refer to the image in APIs such as virDomainSetBlockThreshold.
For internal use we also need to parse it inside of the status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Nodename may be asociated to a disk backup job, add support to looking
up in that chain too. This is specifically useful for the
BLOCK_WRITE_THRESHOLD event which can be registered for any nodename.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Nodename may be asociated to a disk backup job, add support to looking
up in that chain too. This is specifically useful for the
BLOCK_WRITE_THRESHOLD event which can be registered for any nodename.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's a technical detail in qemu that QCOW2 is needed for a pull-mode
backup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'virStorageFileChainLookup' reports an error when the lookup of the
backing chain entry is unsuccessful. Since we possibly use it multiple
times when looking up backing for 'disk->mirror' the function can report
error which won't be actually reported.
Replace the call to virStorageFileChainLookup by lookup in the chain by
index.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use dummy variable to fill 'src' so that access to it doesn't need to be
conditionalized and use temporary variable for 'disk' rather than
dereferencing the array multiple times.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We previous added code for passing FDs which was explicitly derived from
gnulib's passfd code:
commit 17460825f3
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Fri Jan 17 11:57:17 2020 +0000
src: implement APIs for passing FDs over UNIX sockets
This is a simplified variant of gnulib's passfd module
without the portability code that we do not require.
while the license was unchanged, we mistakenly failed to copy the FSF
copyright header which is required by the license terms.
Reported-by: Bruno Haible <bruno@clisp.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Since Xen 4.2 libxl expects device_model_override="/path" instead of
device_model="/path". Adjust the code to parse this as <emulator>.
While libxl also recognizes device_model_version="", this knob is not
required for libvirt. A runtime detection exists in libvirt to select
either "qemu-xen" or "qemu-xen-traditional".
Since qemu-xen-traditional is marked as supported just for stubdoms
there is no need to handle it.
Test data files with 'device_model' were adjusted to use
'device_model_override' instead.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Attempting to create a lxc domain with <seclabel type='none'/> fails
virsh --connect lxc:/// create distro_nosec.xml
error: Failed to create domain from distro_nosec.xml
error: unsupported configuration: Security driver model '(null)' is not available
Commit 638ffa2228 adjusted the logic for setting a driver's default
security model.
The lxc driver does not set a default security driver model in the XML
parser config, causing seclabels of type='none' to have a null model.
The lxc driver's security manager is initialized in lxcStateInitialize()
by calling lxcSecurityInit(). Use the model of this manager as the
default in the XML parser config.
For the record, this is a regression caused by commit 638ffa2228, which
changed the logic for setting a driver's default security model. The
qemu driver was adjusted accordingly, but a similar change was missed
in the lxc driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If a feature is added (or removed) in a QEMU CPU model version, we
get to see the QEMU pretty name for the feature, not the name of
the macro.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In v6.0.0-rc1~439 (and friends) we tried to cache NUMA
capabilities because we assumed they are immutable. And to some
extent they are (NUMA hotplug is not a thing, is it). However,
our capabilities contain also some runtime info that can change,
e.g. hugepages pool allocation sizes or total amount of memory
per node (host side memory hotplug might change the value).
Because of the caching we might not be reporting the correct
runtime info in 'virsh capabilities'.
The NUMA caps are used in three places:
1) 'virsh capabilities'
2) domain startup, when parsing numad reply
3) parsing domain private data XML
In cases 2) and 3) we need NUMA caps to construct list of
physical CPUs that belong to NUMA nodes from numad reply. And
while this may seem static, it's not really because of possible
CPU hotplug on physical host.
There are two possible approaches:
1) build a validation mechanism that would invalidate the
cached NUMA caps, or
2) drop the caching and construct NUMA caps from scratch on
each use.
In this commit, the latter approach is implemented, because it's
easier.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1819058
Fixes: 1a1d848694
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
If the job has finished, but we didn't yet process the completion fake
that it's still incomplete so that apps which decided to poll
qemuDomainGetBlockJobInfo rather than use events can be sure that the
XML update was completed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Replace qemuMonitorGetBlockJobInfo by qemuMonitorGetAllBlockJobInfo and
hash table lookup. This basically open-codes qemuMonitorGetBlockJobInfo,
but it will be removed in next patch.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
If starting an container fails, the virLXCProcessStop() is
called. But since vm->def->id is not set until libvirt_lxc is
spawned (the domain's ID is PID of that process),
virLXCProcessStop() returns early as virDomainObjIsActive()
returns false. But doing so leaves behind resources reserved for
the containers during the startup process. Most notably, hostdevs
are not re-attached to the host, the domain's transient XML is
not removed, etc.
To resolve this, virLXCProcessCleanup() is called in this case.
However, it is modified to accept @flags which allows caller to
run only specific cleanups (depending how far in container
creation the failure occurred). There is plenty of cleanups which
don't need this guard because either they detect a NULL pointer
or try to release an unique resource.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In recent commit of bf8bd93df0 (and friends) we switched the way
we process queried command line arguments: from string lists to
virJSONValue stored in a hash table. To achieve this
qemuMonitorJSONGetCommandLineOptions() helper was introduced
which executes the "query-command-line-options" monitor command
and then calls virJSONValueArrayForeachSteal() to process the
output. The array process function is also given
qemuMonitorJSONGetCommandLineOptionsWorker() as the callback
which is called over each item of the returned array. This
callback then steals "parameters" attribute of each array iteam
storing it in the hash table, but it leaves behind "option"
attribute (because it's g_strdup()-ed). After all of this, the
callback returns 0 which is a signal to the array processing
function that the callback took ownership of the array item. But
this is not true. While it removed "parameters" it did not take
the rest ("option" for instance). And therefore, it leads to a
memory leak:
5,347 (1,656 direct, 3,691 indirect) bytes in 69 blocks are definitely lost in loss record 2,752 of 2,794
at 0x483BEC5: calloc (vg_replace_malloc.c:760)
by 0x4E25A10: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6400.5)
by 0x4943317: virJSONValueNewObject (virjson.c:569)
by 0x4945692: virJSONParserHandleStartMap (virjson.c:1768)
by 0x5825A86: yajl_do_parse (in /usr/lib64/libyajl.so.2.1.0)
by 0x4945BFA: virJSONValueFromString (virjson.c:1896)
by 0xAF5C115: qemuMonitorJSONIOProcessLine (qemu_monitor_json.c:224)
by 0xAF5C45E: qemuMonitorJSONIOProcess (qemu_monitor_json.c:279)
by 0xAF4BB6C: qemuMonitorIOProcess (qemu_monitor.c:342)
by 0xAF4C444: qemuMonitorIO (qemu_monitor.c:574)
by 0x4FEF846: socket_source_dispatch (in /usr/lib64/libgio-2.0.so.0.6400.5)
by 0x4E1F727: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.6400.5)
The callback must return 1 so that the array item is properly
freed.
Fixes: ebeff6cd57
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Since the function is now only used in qemu_domain.c, move it from
domain_conf.c and rename it.
This reverts the work done in commit ace5931553
(conf, qemu: move qemuDomainNVDimmAlignSizePseries to domain_conf.c).
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemuDomainAlignMemorySizes() has an operation order problem. We are
calculating 'initialmem' without aligning the memory modules first.
Since we're aligning the dimms afterwards this can create inconsistencies
in the end result. x86 has alignment of 1-2MiB and it's not severely
impacted by it, but pSeries works with 256MiB alignment and the difference
is noticeable.
This is the case of the existing 'memory-hotplug-ppc64-nonuma' test.
The test consists of a 2GiB (aligned value) guest with 2 ~520MiB dimms,
both unaligned. 'initialmem' is calculated by taking total_mem and
subtracting the dimms size (via virDomainDefGetMemoryInitial()), which
wil give us 2GiB - 520MiB - 520MiB, ending up with a little more than
an 1GiB of 'initialmem'. Note that this value is now unaligned, and
will be aligned up via VIR_ROUND_UP(), and we'll end up with 'initialmem'
of 1GiB + 256MiB. Given that the dimms are aligned later on, the end
result for QEMU is that the guest will have a 'mem' size of 1310720k,
plus the two 512 MiB dimms, exceeding in 256MiB the desired 2GiB
memory and currentMemory specified in the XML.
Existing guests can't be fixed without breaking ABI, but we have
code already in place to align pSeries NVDIMM modules for new guests.
Let's extend it to align all pSeries mem modules.
A new test, 'memory-hotplug-ppc64-nonuma-abi-update', a copy of the
existing 'memory-hotplug-ppc64-nonuma', was added to demonstrate the
result for new pSeries guests. For the same unaligned XML mentioned
above, after applying this patch:
- starting QEMU mem size without PARSE_ABI_UPDATE:
-m size=1310720k,slots=16,maxmem=4194304k \ (no changes)
- starting QEMU mem size with PARSE_ABI_UPDATE:
-m size=1048576k,slots=16,maxmem=4194304k \ (size fixed)
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
A previous patch removed the pSeries NVDIMM align that wasn't
being done properly. This patch reintroduces it in the right
fashion, making it reliant on VIR_DOMAIN_DEF_PARSE_ABI_UPDATE.
This makes it complying with the intended design defined by
commit c7d7ba85a6.
Since the PARSE_ABI_UPDATE is more restrictive than checking for
!migrate && !snapshot, like is being currently done with
qemuDomainAlignMemorySizes(), this means that we'll align the
pSeries NVDIMMs in two places - in post parse time for new
guests, and in qemuDomainAlignMemorySizes() for all guests
that aren't migrating or in a snapshot.
Another difference is that the logic is now in the QEMU driver
instead of domain_conf.c. This was necessary because all
considerations made about the PARSE_ABI_UPDATE flag were done
under QEMU. Given that no other driver supports ppc64 there is no
impact in this change.
A new test was added to exercise what we're doing. It consists
of a a copy of the existing 'memory-hotplug-nvdimm-ppc64' xml2xml
test, called with the PARSE_ABI_UPDATE flag. As intended, we're
not changing QEMU command line or any XML without the flag,
while the pseries NVDIMM memory is being aligned when the
flag is used.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The code to align ppc64 NVDIMMs on post parse was introduced in
commit d3f3c2c97f. That commit failed to realize that we
can't align memory unconditionally. As of commit c7d7ba85a6
("qemu: command: Align memory sizes only on fresh starts"),
all memory alignment should be executed only when we're not
migrating or in a snapshot.
This revert does not break any guests in the wild, given that
ppc64 NVDIMMs are still being aligned in qemuDomainAlignMemorySizes().
Next patch will introduce a mechanism where we can have post
parse NVDIMM alignment for pSeries without breaking the
intended design, as defined by c7d7ba85a6.
This reverts commit d3f3c2c97f.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The AppArmorSetMemoryLabel() is a callback that is called from
qemuSecuritySetMemoryLabel() which never passes NULL as @mem.
Therefore, there is no need to check whether @mem is NULL. Also,
no other driver does that and just dereference it immediately.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
After previous cleanup the @qemuCaps argument in
qemuDomainDefValidateMemoryHotplug() is unused and thus doesn't
need to be passed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
So far our memory modules could go only into DIMM slots. But with
virtio model this assumption is no longer true - virtio-pmem goes
onto PCI bus. But for formatting PCI address onto command line we
already have a function - qemuBuildDeviceAddressStr(). Therefore,
mode DIMM address generation into it so that we don't have to
special case address building later on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Han Han <hhan@redhat.com>
There is this function qemuDomainDefValidateMemoryHotplug() which
is called explicitly from hotplug path and the qemu's domain def
validator. This is not really necessary because we can move the
part that validates feature against qemuCaps into device
validator which is called implicitly (from qemu driver's POV).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Han Han <hhan@redhat.com>
The virDomainMemoryTargetDefFormat() uses good old style of
formatting child buffer (virBufferAdjustIndent()). When switched
to virXMLFormatElement() we can save a couple of lines
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Han Han <hhan@redhat.com>
The virDomainMemorySourceDefFormat() uses good old style of
formatting child buffer (virBufferAdjustIndent()). When switched
to virXMLFormatElement() we can save a couple of lines.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Han Han <hhan@redhat.com>
The virDomainMemoryModel structure has a @type member which is
really type of virDomainMemoryModel but we store it as int
because the virDomainMemoryModelTypeFromString() call stores its
retval right into it. Then, to have compiler do compile time
check for us, every switch() typecasts the @type. This is
needlessly verbose because the parses already has @val - a
variable to store temporary values. Switch @type in the struct to
virDomainMemoryModel and drop all typecasts.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Han Han <hhan@redhat.com>
Our code expects that a nvdimm has a path defined always. And the
parser does check for that. Well, not fully - only when parsing
<source/> (which is an optional element). So if the element is
not in the XML then the check is not performed and the assumption
is broken. Verify in the memory def validator that a path was
set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Han Han <hhan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Han Han <hhan@redhat.com>
The UUID is guest visible and thus shouldn't change if we want to
not break guest ABI.
Fixes: 08ed673901
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Han Han <hhan@redhat.com>
This macro checks whether given number is an integer power of
two. At the same time, I've identified two places where we check
for pow2 and I'm replacing them with the macro.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Han Han <hhan@redhat.com>
The only test we do when checking for UUID validity is that
whether all bytes are the same (invalid UUID) or not (valid
UUID). The algorithm we use is needlessly complicated.
Also, the checked UUID is not modified and hence the argument can
be of 'const' type.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Han Han <hhan@redhat.com>
The domain definition stored with a checkpoint isn't used currently
apart from matching disks when creating a new checkpoints.
As some users of the incremental backup API want to provide backups in
offline mode under their control (obviously while compying with our
documentation on how the on-disk state should be handled) and then want
to define the checkpoint for live use, supplying a <domain> sub-element
is overly complex and not actually needed by the code.
Relax the restriction when re-defining a checkpoint so that <domain> is
not necessary and add (alibistic) documentation saying that future
actions may not work if it's missing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Conditionalize code which assumes that the domain definition stored in
the checkpoint is present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Checking the definition ABI when redefining checkpoints doesn't make
much sense for the following reasons:
* the domain definition in the checkpoint is mostly unused (a relic
adopted from the snapshot code)
* can be very easily overridden by deleting the checkpoint metadata
before redefinition
Rather than complicating the logic when we'll be taking into account
that the domain definition may be missing, let's just remove the check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Fix the type for a variable holding flags to the usual 'unsigned int'
and change the name to be more appropriate to its use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
We can extract './domain' directly and let the parser deal with the
type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
See arch/x86/include/asm/intel-family.h in the Kernel:
#define INTEL_FAM6_ICELAKE_X 0x6A
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
LXC processes confined by apparmor are not permitted to receive signals
from libvirtd. Attempting to destroy such a process fails
virsh --connect lxc:/// destroy distro_apparmor
error: Failed to destroy domain distro_apparmor
error: Failed to kill process 29491: Permission denied
And from /var/log/audit/audit.log
type=AVC msg=audit(1606949706.142:6345): apparmor="DENIED"
operation="signal" profile="libvirt-314b7109-fdce-48dc-ad28-7c47958a27c1"
pid=29390 comm="libvirtd" requested_mask="receive" denied_mask="receive"
signal=term peer="libvirtd"
Similar to the libvirt-qemu abstraction, add a rule to the libvirt-lxc
abstraction allowing reception of signals from libvirtd.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
qemuBuildCommandLine() is calling qemuDomainAlignMemorySizes(),
which is an operation that changes live XML and domain and has
little to do with the command line build process.
Move it to qemuProcessPrepareDomain() where we're supposed to
make live XML and domain changes before launch. qemuProcessStart()
is setting VIR_QEMU_PROCESS_START_NEW if !migrate && !snapshot,
same conditions used in qemuBuildCommandLine() to call
qemuDomainAlignMemorySizes(), making this change seamless.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemuProcessCreatePretendCmdPrepare() is setting the
VIR_QEMU_PROCESS_START_NEW regardless of whether this is
a migration case or not. This behavior differs from what we're
doing in qemuProcessStart(), where the flag is set only
if !migrate && !snapshot.
Fix it by making the flag setting consistent with what we're
doing in qemuProcessStart().
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Let's pass along / fill @niothreads rather than trying to make dual
use as a return value and thread count.
This resolves a Coverity issue detected in qemuDomainGetIOThreadsMon
where if qemuDomainObjExitMonitor failed, then a -1 was returned and
overwrite @niothreads causing a memory leak.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In this previous commit:
commit 65491a2dfe
Author: Martin Kletzander <mkletzan@redhat.com>
Date: Thu Nov 12 13:58:53 2020 +0100
Do not disable incompatible-pointer-types-discards-qualifiers
We selectively rewrite G_DEFINE_TYPE to avoid warnings about
mismatched volatile/non-volatile pointers that appeared with
CLang when using GLib2 >= 2.67
We have now just hit the reverse problem, GCC >= 11 has started
warning about mismatched volatile/non-volatile pointers but only
with GLib2 < 2.67. The new GLib2 avoids the warning, as does
older GCC.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Existing practice with the filesystem fields reported for the
virDomainGetGuestInfo API is to use the singular form for
field names. Ensure the disk info follows this practice.
Fixes
commit 05a75ca2ce
Author: Marc-André Lureau <marcandre.lureau@redhat.com>
Date: Fri Nov 20 22:09:46 2020 +0400
domain: add disk informations to virDomainGetGuestInfo
commit 0cb2d9f05d
Author: Marc-André Lureau <marcandre.lureau@redhat.com>
Date: Fri Nov 20 22:09:47 2020 +0400
qemu_driver: report guest disk informations
commit 172b830435
Author: Marc-André Lureau <marcandre.lureau@redhat.com>
Date: Fri Nov 20 22:09:48 2020 +0400
virsh: add --disk informations to guestinfo command
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We don't need the index that virDomainDiskIndexByName returns.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We don't need the index that virDomainDiskIndexByName returns.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to d3c029bb10 where we've refactored
virDomainSnapshotAlignDisks, modify the extension algorithm to avoid use
of the 'idx' variable and sorting of the array.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a local variable holding the pointer instead of indexing the array
multiple times.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Clarify that the variable refers to the definition of the disk from the
checkpoint definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In most cases 'def' is used for the domain definition. Rename it to
chkdef to prevent confusion.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract the pointer and use a local variable throughout the function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use g_autoptr for virBitmap 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>
Commit f1b0890 introduced a potential crash due to incorrect operator
precedence when accessing an element from a pointer to an array.
Backtrace below:
#0 virNodeDeviceGetMdevTypesCaps (sysfspath=0x7fff801661e0 "/sys/devices/pci0000:00/0000:00:02.0", mdev_types=0x7fff801c9b40, nmdev_types=0x7fff801c9b48) at ../src/conf/node_device_conf.c:2676
#1 0x00007ffff7caf53d in virNodeDeviceGetPCIDynamicCaps (sysfsPath=0x7fff801661e0 "/sys/devices/pci0000:00/0000:00:02.0", pci_dev=0x7fff801c9ac8) at ../src/conf/node_device_conf.c:2705
#2 0x00007ffff7cae38f in virNodeDeviceUpdateCaps (def=0x7fff80168a10) at ../src/conf/node_device_conf.c:2342
#3 0x00007ffff7cb11c0 in virNodeDeviceObjMatch (obj=0x7fff84002e50, flags=0) at ../src/conf/virnodedeviceobj.c:850
#4 0x00007ffff7cb153d in virNodeDeviceObjListExportCallback (payload=0x7fff84002e50, name=0x7fff801cbc20 "pci_0000_00_02_0", opaque=0x7fffe2ffc6a0) at ../src/conf/virnodedeviceobj.c:909
#5 0x00007ffff7b69146 in virHashForEach (table=0x7fff9814b700 = {...}, iter=0x7ffff7cb149e <virNodeDeviceObjListExportCallback>, opaque=0x7fffe2ffc6a0) at ../src/util/virhash.c:394
#6 0x00007ffff7cb1694 in virNodeDeviceObjListExport (conn=0x7fff98013170, devs=0x7fff98154430, devices=0x7fffe2ffc798, filter=0x7ffff7cf47a1 <virConnectListAllNodeDevicesCheckACL>, flags=0)
at ../src/conf/virnodedeviceobj.c:943
#7 0x00007fffe00694b2 in nodeConnectListAllNodeDevices (conn=0x7fff98013170, devices=0x7fffe2ffc798, flags=0) at ../src/node_device/node_device_driver.c:228
#8 0x00007ffff7e703aa in virConnectListAllNodeDevices (conn=0x7fff98013170, devices=0x7fffe2ffc798, flags=0) at ../src/libvirt-nodedev.c:130
#9 0x000055555557f796 in remoteDispatchConnectListAllNodeDevices (server=0x555555627080, client=0x5555556bf050, msg=0x5555556c0000, rerr=0x7fffe2ffc8a0, args=0x7fffd4008470, ret=0x7fffd40084e0)
at src/remote/remote_daemon_dispatch_stubs.h:1613
#10 0x000055555557f6f9 in remoteDispatchConnectListAllNodeDevicesHelper (server=0x555555627080, client=0x5555556bf050, msg=0x5555556c0000, rerr=0x7fffe2ffc8a0, args=0x7fffd4008470, ret=0x7fffd40084e0)
at src/remote/remote_daemon_dispatch_stubs.h:1591
#11 0x00007ffff7ce9542 in virNetServerProgramDispatchCall (prog=0x555555690c10, server=0x555555627080, client=0x5555556bf050, msg=0x5555556c0000) at ../src/rpc/virnetserverprogram.c:428
#12 0x00007ffff7ce90bd in virNetServerProgramDispatch (prog=0x555555690c10, server=0x555555627080, client=0x5555556bf050, msg=0x5555556c0000) at ../src/rpc/virnetserverprogram.c:302
#13 0x00007ffff7cf042b in virNetServerProcessMsg (srv=0x555555627080, client=0x5555556bf050, prog=0x555555690c10, msg=0x5555556c0000) at ../src/rpc/virnetserver.c:137
#14 0x00007ffff7cf04eb in virNetServerHandleJob (jobOpaque=0x5555556b66b0, opaque=0x555555627080) at ../src/rpc/virnetserver.c:154
#15 0x00007ffff7bd912f in virThreadPoolWorker (opaque=0x55555562bc70) at ../src/util/virthreadpool.c:163
#16 0x00007ffff7bd8645 in virThreadHelper (data=0x55555562bc90) at ../src/util/virthread.c:233
#17 0x00007ffff6d90432 in start_thread () at /lib64/libpthread.so.0
#18 0x00007ffff75c5913 in clone () at /lib64/libc.so.6
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Currently it is simply ignored.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Currently it is simply ignored.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Commit c4f4e195 fixed a double free, but if the code returns before
we realloc the list and virFirmwareFreeList was called with cfg->nfirmwares
> 0 (e.g. during virQEMUDriverConfigDispose), then it would be rather
disastrous. So let's reinitialize that too to indicate the list is empty.
Coverity pointed out that using nvram[0] as a guard to reallocating the
list could lead to a possible NULL deref. While nvram[0] may always be
true in this case, if it wasn't then the subsequent for loop would fail.
Just reallocate always regardless - even if nfirmwares == 0 as
virFirmwareFreeList will free it for us anyway.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Initialize and free @magic since virJSONValueObjectAppendString
does not free it for us eventually.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Initialize and free @magic since virJSONValueObjectAppendString
does not free it for us eventually.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Since 032548c4 @cmd was never autofree'd. Perhaps as a result of
VIR_AUTOPTR type changes occurring at roughly the same time so the
copy pasta missed this.
Found by Coverity.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Now that no one uses VIR_AUTOSTRINGLIST it can be dropped.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Glib provides g_auto(GStrv) which is in-place replacement of our
VIR_AUTOSTRINGLIST.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If there is an error getting info from guest agent, then the
control on qemuDomainGetGuestInfo() jumps onto 'exitagent' label
and subsequently continues on 'endagentjob'. Both labels are hit
also in success case too. The control then continues by
attempting to match fetched info (e.g. disk addresses) with
domain def. But this is needless - the API will return error
regardless.
To return early from the function move both 'exitagent' and
'endagentjob' labels at the end of the function and jump straight
onto 'cleanup' afterwards. This allows us to set 'ret = 0' later
- only when we know we succeeded.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the function along with helpers for caching the reply and tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the new handler to fetch the required data and do the extraction
locally without conversion to string list.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add a new set hander for getting the data for
'query-command-line-options' which returns everything at once and lets
the caller extract the data. This way we don't need to cache the output
of the monitor command for repeated calls.
Note that we will have enough testing of this code path via
qemucapabilitiestest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'error' label is just returning -1, so let's 'return -1'
directly.
Use g_autoptr() with virDomainControllerDefPtr to remove the
need to call virDomainControllerDefFree() in the error path.
There is no need to VIR_FREE(nodes) explictly since 'nodes'
is using g_autofree.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Let's register AUTOPTR_CLEANUP_FUNC for virDomainControllerDefPtr
and modernize this function, removing the 'error' label using
g_autoptr().
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The 'error' label is just doing a 'return -1'.
There's also a couple of 'VIR_FREE(nodes)' calls that are happening
right before exiting on error, but 'nodes' is already set for
autocleanup. These calls can also be removed.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Register a AUTOPTR_CLEANUP_FUNC for virDomainSmartcardDef and use
g_autoptr() to eliminate the 'error' label.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Register an AUTOPTR_CLEANUP_FUNC for virDomainDiskDefPtr, then
use g_autoptr() in virDomainDiskDef and virStorageEncryption
pointers to get rid of the 'cleanup' and 'error' labels.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This will open an opportunity to modernize virDomainDiskDefParseXML()
in the next patch.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This will modernize virDomainVideoDefParseXML() and
virDomainDefAddImplicitVideo() by removing unneeded
cleanup labels.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The 'video' pointer is only being freed on error path, meaning
that we're leaking it after each loop restart.
There are more opportunities for auto cleanups of virDomainVideoDef
pointers, so let's register AUTOPTR_CLEANUP_FUNC for it to use
g_autoptr() later on.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Use g_autoptr() with the hash and remove the 'cleanup' label.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This spares us of 2 explicit VIR_FREE() calls.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Do not look up the index of the passed FD in places where
we already have it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
An alternative to qemuVirCommandGetFDSet that takes the index
into the passed FD set as an argument and does not try to look it up.
Use it as well ass virCommandPassFDIndex in qemuBuildChrChardevFileStr
and qemuBuildInterfaceCommandLine.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Just like virCommandPassFD, but it also returns an index of
the passed FD in the FD set.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We only jump to cleanup before allocating any lists.
Drop the dead code.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, ret is only used in the 'cleanup' section
and initialized right before the jump.
Switch to the customary initialization to -1 and only
leave in the 'ret = 0' statement on an empty list.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In a few commit back (v6.10.0-5-gb3dad96972) a new helper for
obtaining string arrays from a virJSONObject was introduced:
virJSONValueObjectGetStringArray(). I've identified three places
where it can be used instead of open coding it:
qemuAgentSSHGetAuthorizedKeys(),
qemuMonitorJSONGetStringListProperty() and
qemuMonitorJSONGetCPUDefinitions().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virJSONValueObjectGetStringArray() function is given a @key
which is supposed to be an array inside given @object. Well, if
it's not then an error state is returned (NULL), but no error
message is set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
cfg->firmwares still points to the original memory address after being
freed by virFirmwareFreeList(). As cfg get freed, it will be freed again
even if cfg->nfirmwares=0 which eventually lead to crash.
The patch fix it by setting cfg->firmwares to NULL explicitly after
virFirmwareFreeList() returns
Signed-off-by: Guoyi Tu<tu.guoyi@h3c.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The original logic is incorrect. We would delete the device entry
from eBPF map only if the newval would be same as current val in the
map. In case that the device was allowed only as read-only but later
we remove all permissions for that device it would remain in the table
with empty values.
The old code would still deny the device but it's not working as
intended. Instead we will update the value in advance. If the updated
value is 0 it means that we are removing all permissions so it should
be removed from the map, otherwise we will update the value in map.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1810356
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Kernel commit <d505b8af58912ae1e1a211fabc9995b19bd40828> added proper
check for cpu quota maximum limit to prevent internal overflow.
Even though this change is not present in all kernels it makes sense
to enforce the same limit in libvirt.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1750315
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
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>
Tested-by: Han Han <hhan@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
guest-get-disks is available since QEMU 5.2:
https://wiki.qemu.org/ChangeLog/5.2#Guest_agent
Note that the test response was manually edited based on a reply on my
bare-metal computer. It shows partial results due to pcieport driver not
being currently supported by QGA.
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>
Tested-by: Han Han <hhan@redhat.com>
There might be more potential users around, I haven't looked thoroughly.
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>
Tested-by: Han Han <hhan@redhat.com>
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>
Tested-by: Han Han <hhan@redhat.com>
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>
Tested-by: Han Han <hhan@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
To match the QGA schema name (we are introducing a qemuAgentDiskInfo
struct again for different purpose).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
I previously did a workaround for a glib event loop race
that causes crashes:
commit 0db4743645
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Tue Jul 28 16:52:47 2020 +0100
util: avoid crash due to race in glib event loop code
it turns out that the workaround has a significant performance
penalty on I/O intensive workloads. We thus need to avoid the
workaround if we know we have a new enough glib to avoid the
race condition.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
It was reported that the performance of tunnelled migration and
volume upload/download regressed in 6.9.0, when the virt-ssh-helper
is used for remote SSH tunnelling instead of netcat.
When seeing data available to read from stdin, or the socket,
the current code will allocate at most 1k of extra space in
the buffer it has.
After writing data to the socket, or stdout, if more than 1k
of extra space is in the buffer, it will reallocate to free
up that space.
This results in a huge number of mallocs when doing I/O, as
well as a huge number of syscalls since at most 1k of data
will be read/written at a time.
Also if writing blocks for some reason, it will continue to
read data with no memory bound which is bad.
This changes the code to use a 1 MB fixed size buffer in each
direction. If that buffer becomes full, it will update the
watches to stop reading more data. It will never reallocate
the buffer at runtime.
This increases the performance by orders of magnitude.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Even though it is technically possible, when running the migrations QEMU's
nbd-server-start errors out with:
"TLS is only supported with IPv4/IPv6"
We can always enable it when QEMU adds this feature, but for now it is safer to
show our error message rather than rely on QEMU to error out properly.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
When executing the hypervisor-cpu-baseline command and if there is
only a single CPU definition present in the XML file, then the
baseline handler will exit early and libvirt will print an unhelpful
message:
"error: An error occurred, but the cause is unknown"
This is due to no CPU definition ever being "baselined", since the
handler expects at least two CPU models.
Let's fix this by performing a CPU model expansion on the single CPU
definition and returning the result to the caller. This will also
ensure the CPU model's feature set is sane if any were provided in
the file.
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Check the provided CPU models against the CPU models
known by the hypervisor before baselining and print
an error if an unrecognized model is found.
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
When executing the hypervisor-cpu-baseline command and the
XML file contains a CPU definition without a model name, or
an invalid CPU definition, then the commands will fail and
return an error message from the QMP response.
Let's clean this up by checking for a valid definition and
presence of a model name.
This code is copied from virCPUBaseline.
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Hypervisor-cpu-baseline requires the cpu-model-expansion
capability when expanding CPU model features if the
--features flag is provided.
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
When libvirt added support for firewalld, we were unable to use
firewalld's higher level rules, because they weren't detailed enough
and could not be applied to the iptables FORWARD or OUTPUT chains
(only to the INPUT chain). Instead we changed our code so that rather
than running the iptables/ip6tables/ebtables binaries ourselves, we
would send these commands to firewalld as "passthrough commands", and
firewalld would run the appropriate program on our behalf.
This was done under the assumption that firewalld was somehow tracking
all these rules, and that this tracking was benefitting proper
operation of firewalld and the system in general.
Several years later this came up in a discussion on IRC, and we
learned from the firewalld developers that, in fact, adding iptables
and ebtables rules with firewalld's passthrough commands actually has
*no* advantage; firewalld doesn't keep track of these rules in any
way, and doesn't use them to tailor the construction of its own rules.
Meanwhile, users have been complaining for some time that whenever
firewalld is restarted on a system with libvirt virtual networks
and/or nwfilter rules active, the system logs would be flooded with
warning messages whining that [lots of different rules] could not be
deleted because they didn't exist. For example:
firewalld[3536040]: WARNING: COMMAND_FAILED:
'/usr/sbin/iptables -w10 -w --table filter --delete LIBVIRT_OUT
--out-interface virbr4 --protocol udp --destination-port 68
--jump ACCEPT' failed: iptables: Bad rule
(does a matching rule exist in that chain?).
(See https://bugzilla.redhat.com/1790837 for many more examples and a
discussion)
Note that these messages are created by iptables, but are logged by
firewalld - when an iptables/ebtables command fails, firewalld grabs
whatever is in stderr of the program, and spits it out to the system
log as a warning. We've requested that firewalld not do this (and
instead leave it up to the calling application to do the appropriate
logging), but this request has been respectfully denied.
But combining the two problems above ( 1) firewalld doesn't do
anything useful when you use it as a proxy to add/remove iptables
rules, 2) firewalld often insists on logging lots of
annoying/misleading/useless "error" messages when you use it as a
proxy to remove iptables rules that don't already exist), leads to a
solution - simply stop using firewalld to add and remove iptables
rules. Instead, exec iptables/ip6tables/ebtables directly in the same
way we do when firewalld isn't active.
We still need to keep track of whether or not firewalld is active, as
there are some things that must be done, e.g. we need to add some
actual firewalld rules in the firewalld "libvirt" zone, and we need to
take notice when firewalld restarts, so that we can reload all our
rules.
This patch doesn't remove the infrastructure that allows having
different firewall backends that perform their functions in different
ways, as that will very possibly come in handy in the future when we
want to have an nftables direct backend, and possibly a "pure"
firewalld backend (now that firewalld supports more complex rules, and
can add those rules to the FORWARD and OUTPUT chains). Instead, it
just changes the action when the selected backend is "firewalld" so
that it adds rules directly rather than through firewalld, while
leaving as much of the existing code intact as possible.
In order for tests to still pass, virfirewalltest also had to be
modified to behave in a different way (i.e. by capturing the generated
commandline as it does for the DIRECT backend, rather than capturing
dbus messages using a mocked dbus API).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
When it is starting up, firewalld will delete all existing iptables
rules and chains before adding its own rules. If libvirtd were to try
to directly add iptables rules during the time before firewalld has
finished initializing, firewalld would end up deleting the rules that
libvirtd has just added.
Currently this isn't a problem, since libvirtd only adds iptables
rules via the firewalld "passthrough command" API, and so firewalld is
able to properly serialize everything. However, we will soon be
changing libvirtd to add its iptables and ebtables rules by directly
calling iptables/ebtables rather than via firewalld, thus removing the
serialization of libvirtd adding rules vs. firewalld deleting rules.
This will especially apparent (if we don't fix it in advance, as this
patch does) when libvirtd is responding to the dbus NameOwnerChanged
event, which is used to learn when firewalld has been restarted. In
that case, dbus sends the event before firewalld has been able to
complete its initialization, so when libvirt responds to the event by
adding back its iptables rules (with direct calls to
/usr/bin/iptables), some of those rules are added before firewalld has
a chance to do its "remove everything" startup protocol. The usual
result of this is that libvirt will successfully add its private
chains (e.g. LIBVIRT_INP, etc), but then fail when it tries to add a
rule jumping to one of those chains (because in the interim, firewalld
has deleted the new chains).
The solution is for libvirt to preface it's direct calling to iptables
with a iptables command sent via firewalld's passthrough command
API. Since commands sent to firewalld are completed synchronously, and
since firewalld won't service them until it has completed its own
initialization, this will assure that by the time libvirt starts
calling iptables to add rules, that firewalld will not be following up
by deleting any of those rules.
To minimize the amount of extra overhead, we request the simplest
iptables command possible: "iptables -V" (and aside from logging a
debug message, we ignore the result, for good measure).
(This patch is being done *before* the patch that switches to calling
iptables directly, so that everything will function properly with any
fractional part of the series applied).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Even though *we* don't call ebtables/iptables/ip6tables (yet) when the
firewalld backend is selected, firewalld does, so these binaries need
to be there; let's check for them. (Also, the patch after this one is
going to start execing those binaries directly rather than via
firewalld).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
iptables and ip6tables have had a "-w" commandline option to grab a
systemwide lock that prevents two iptables invocations from modifying
the iptables chains since 2013 (upstream commit 93587a04 in
iptables-1.4.20). Similarly, ebtables has had a "--concurrent"
commandline option for the same purpose since 2011 (in the upstream
ebtables commit f9b4bcb93, which was present in ebtables-2.0.10.4).
Libvirt added code to conditionally use the commandline option for
iptables/ip6tables in upstream commit ba95426d6f (libvirt-1.2.0,
November 2013), and for ebtables in upstream commit dc33e6e4a5
(libvirt-1.2.11, November 2014) (the latter actually *re*-added the
locking for iptables/ip6tables, as it had accidentally been removed
during a refactor of firewall code in the interim).
I say "conditionally" because a check was made during firewall module
initialization that tried executing a test command with the
-w/--concurrent option, and only continued using it for actual
commands if that test command completed successfully. At the time the
code was added this was a reasonable thing to do, as it had been less
than a year since introduction of -w to iptables, so many distros
supported by libvirt were still using iptables (and possibly even
ebtables) versions too old to have the new commandline options.
It is now 2020, and as far as I can discern from repology.org (and
manually examining a RHEL7.9 system), every version of every distro
that is supported by libvirt now uses new enough versions of both
iptables and ebtables that they all have support for -w/--concurrent.
That means we can finally remove the conditional code and simply
always use them.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
All the unit tests that use iptables/ip6tables/ebtables have been
written to omit the locking/exclusive use primitive on the generated
commandlines. Even though none of the tests actually execute those
commands (and so it doesn't matter for purposes of the test whether or
not the commands support these options), it still made sense when some
systems had these locking options and some didn't.
We are now at a point where every supported Linux distro has supported
the locking options on these commands for quite a long time, and are
going to make their use non-optional. As a first step, this patch uses
the virFirewallSetLockOverride() function, which is called at the
beginning of all firewall-related tests, to set all the bools
controlling whether or not the locking options are used to true. This
means that all the test cases must be updated to include the proper
locking option in their commandlines.
The change to make actual execs of the commands unconditionally use
the locking option will be in an upcoming patch - this one affects
only the unit tests.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The feature is never enabled by default on KVM and QEMU dropped it from
the models long ago.
https://bugzilla.redhat.com/show_bug.cgi?id=1798004
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
For backward compatibility with older versions of libvirt CPU models in
our CPU map are mostly immutable. We only changed them in a few specific
cases after showing it was safe. Sometimes QEMU developers realize a
specific feature should not be part of a particular (or any) CPU model
because it can never be enabled automatically without further
configuration. But we couldn't follow them because doing so would break
migration to older libvirt.
If QEMU drops feature F from CPU model M because F could not be enabled
automatically anyway, asking for M would never enable F. Even with older
QEMU versions. Naively removing F from libvirt's definition of M would
seem to work nicely on a single host. Libvirt would consider M to be
compatible with hosts CPU that do not support F. However, trying to
migrate domains using M without explicitly enabling or disabling F could
fail, because older libvirt would think F was enabled (it is part of M
there), but QEMU reports it as disabled once started.
Thus we can remove such feature from a libvirt's CPU model, but we have
to make sure any CPU definition using the affected model will always
explicitly mention the state of the removed feature.
https://bugzilla.redhat.com/show_bug.cgi?id=1798004
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
The patch adds a new attribute for the 'feature' element in CPU model
specification to indicate that a given feature was removed from a CPU
model. In other words, older versions of libvirt would consider such
feature to be included in the CPU model.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
This is just a preparation for adding new functionality to
virCPUx86Update.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Until now, the function returned immediately when the guest CPU
definition did not use optional features or minimum match. Clearly,
there's nothing to be updated according to the host CPU in this case,
but the arch specific code may still want to do some compatibility
updates based on the model and features used in the guest CPU
definition.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
This new function adds a feature to a CPU definition only if it is not
present there yet.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Replace the 'update' bool parameter with an enum so that we can have
more than two possible values.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
The function is supposed to add a feature to a CPU definition, let's
name it virCPUDefAddFeatureInternal. The behavior in case the feature is
already present in the CPU def is configurable and we will soon add a
new option to not do anything in that case, which wouldn't really work
well with the current *Update* name.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
dumpxml can now serialize:
* floppy drives
* file-backed and device-backed disk drives
* images mounted to virtual CD/DVD drives
* IDE and SCSI controllers
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Co-authored-by: Sri Ramanujam <sramanujam@datto.com>
Signed-off-by: Matt Coleman <matt@datto.com>
Forgetting to use the VIR_MIGRATE_TLS flag with migration can lead to
leak of sensitive information. Add an administrative knob to force use
of the flag.
Note that without VIR_MIGRATE_PEER2PEER, the migration is driven by an
instance of the client library which doesn't necessarily run on either
of the hosts so the flag can't be used to assume VIR_MIGRATE_TLS even
if it wasn't provided by the user instead of rejecting if it's not.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/67
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu's internals were not prepared for switching to -blockdev for the
legacy storage migration. Add a proper error message since qemu is
unlikely to attempt fixing the old protocol.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/65
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move and aggregate all the logic which is switched based on whether the
migration is tunnelled or not before other checks. Further checks will
be added later.
While the code is being moved the error message is put on a single line
per new coding style.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our streams are not the best transport for migration data and we support
TLS for security now. It's unlikely that there will be enough motivation
to add a new migration protocol to tunnel NBD too.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commit dealing with snapshots we must rewrite the
metadata of the previously-'current' checkpoint when changing which
checkpoint is considered 'current'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Whether a snapshot definition is considered 'current' or active is
stored in the metadata XML libvirt writes when we create metadata.
This means that if we are changing the 'current' snapshot we must
re-write the metadata of the previously 'current' snapshot to update the
field to prevent having multiple active snapshots.
Unfortunately the snapshot creation code didn't do this properly, which
resulted in the following error:
error : qemuDomainSnapshotLoad:430 : internal error: Too many snapshots claiming to be current for domain snapshot-test
being printed if libvirtd was terminated and restarted.
Introduce qemuSnapshotSetCurrent which writes out the old snapshot's
metadata when updating the current snapshot.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In some cases such as when creating an internal inactive snapshot we
know that the domain definition in the snapshot is equivalent to the
current definition. Additionally we set up the current definition for
the snapshotting but not the one contained in the snapshot. Thus in some
cases the caller knows better which def to use.
Make qemuDomainSnapshotForEachQcow2 take the definition by the caller
and copy the logic for selecting the definition to callers where we
don't know for sure that the above claim applies.
This fixes internal inactive snapshots when <disk type='volume'> is used
as we translate the pool/vol combo only in the current def.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/97
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Don't try to manipulate snapshots on network or unresolved volume backed
storage.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'continue' the loop if the device is not a disk. Saving the level makes
one of the error messages fit on a single line.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 912c6b22fc added abort() when the
'val' parameter is NULL along with setting the error variable for the
command. We don't want to abort in this case, just set the error.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When the host is shutting down then we get PrepareForShutdown
signal on DBus to which we react by creating a thread which
runs virStateStop() and thus qemuStateStop(). But if scheduling
the thread is delayed just a but it may happen that we receive
SIGTERM (sent by systemd) to which we respond by quitting our
event loop and cleaning up everything (including drivers). And
only after that the thread gets to run only to find qemu_driver
being NULL.
What we can do is to delay exiting event loop and join the thread
that's executing virStateStop(). If the join doesn't happen in
given timeout (currently 30 seconds) then libvirtd shuts down
forcefully anyways (see virNetDaemonRun()).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1895359
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1739564
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The ESP SCSI controllers (NCR53C90, DC390, AM53C974) have the same
requirement as the LSI Logic controller for each disk to be set via
the scsi-id=NNN property, not the lun=NNN property.
Switching the code to use an enum will force authors to pay attention
to this difference when adding future SCSI controllers.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When introducing the API I've mistakenly used 'int' type for
@nkeys argument which does nothing more than tells the API how
many items there are in @keys array. Obviously, negative values
are not expected and therefore 'unsigned int' should have been
used.
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The NCR53C90 is the built-in SCSI controller on all sparc machine types,
but not sparc64. Note that it has the fixed alias "scsi", which differs
from our normal naming convention of "scsi0".
The DC390 and AM53C974 are PCI SCSI controllers that can be added to any
PCI machine.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Probing for the NCR53C90 controller is a little unusual. The
qom-list-types QMP command returns a list of all types known to
the QEMU binary. It does not distinguish devices which are user
creatable from those which are built-in.
Any QEMU target that supports PCI will have the DC390 / AM53C974
devices because they are PCI based. Due to code dependencies
in QEMU though, existence of these two devices will also pull in
the NCR53C90 device (called just 'esp' in QEMU). The NCR53C90 is
not user-creatable and can only be used when built-in to the
machine type.
This is only the case on sparc machines, and certain mips64 and
m68k machines. IOW, we don't rely on qom-list-types as a guide
for existence of NCR53C90, as it shouldn't really exist in most
QEMU binaries.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The NCR53C90 is the built-in SCSI controller on all sparc machine types,
and some mips and m68k machine types.
The DC390 and AM53C974 are PCI SCSI controllers that can be added to any
PCI machine.
These are only interesting for emulating obsolete hardware platforms.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The NCR53C90 ESP SCSI controller is only usable when built-in to the
machine type. This method will facilitate checking that restriction
across many places.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The sparc machines have little in common with sparc64 machines.
No sparc machine type includes a PCI bus, so we should not be adding one
to the XML. This further means that we should not be adding a memory
balloon device, nor USB controller as these are both PCI based.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We are generating a fresh UUID and storing it in the XML for the
default network, but this is unnecessary because the network
driver will automatically generate one if it's missing from the
XML; the fact that we only do this if the uuidgen command happens
to be available on the build machine is further proof that we can
safely skip this step.
This patch is best viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
While we generally expect libvirt objects to be defined using the
appropriate APIs, there are cases where it's reasonable for an
external entity, usually a package manager, to drop a valid
configuration file under /etc/libvirt and have libvirt take over
from there: notably, this is exactly how the default network is
handled.
For the most part, whether the configuration is saved back to disk
after being parsed by libvirt doesn't matter, because we'll end up
with the same values anyway, but an obvious exception to this is
data that gets randomly generated when not present, namely MAC
address and UUID.
Historically, both were handled by our build system, but commit
a47ae7c004 moved handling of the former inside libvirt proper;
this commit extends such behavior to the latter as well.
Proper error handling for the virNetworkSaveConfig() call, which
was missing until now, is introduced in the process.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
We jump to the error label if the 'if' condition is true.
Remove the explicit else to make it more obvious that 'hostname'
is filled on both branches of 'if (!uri_in)'.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In case no uri_in was supplied, we forgot to set the hostname
to the current hostname and formatted a useless uri_out.
src/util/glibcompat.h:57:26: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
57 | # define g_strdup_printf vir_g_strdup_printf
src/openvz/openvz_driver.c:2136:16: note: in expansion of macro ‘g_strdup_printf’
2136 | *uri_out = g_strdup_printf("ssh://%s", hostname);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Jaroslav Suchanek <jsuchane@redhat.com>
Fixes: e3c626a61d
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The way our domain capabilities work currently, is that we have
virDomainCapsEnum struct which contains 'unsigned int values'
member which serves as a bitmask. More complicated structs are
composed from this struct, giving us whole virDomainCaps
eventually.
Whenever we want to report that a certain value is supported, the
'1 << value' bit is set in the corresponding unsigned int member.
This works as long as the resulting value after bitshift does not
overflow unsigned int. There is a check inside
virDomainCapsEnumSet() which ensures exactly this, but no caller
really checks whether virDomainCapsEnumSet() succeeded. Also,
checking at runtime is a bit too late.
Fortunately, we know the largest value we want to store in each
member, because each enum of ours ends with _LAST member.
Therefore, we can check at build time whether an overflow can
occur.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Using virtCgroupNewSelf() is not correct with cgroups v2 because the
the virt-host-validate process is executed from from the same cgroup
context as the terminal and usually not all controllers are enabled
by default.
To do a proper check we need to use the root cgroup to see what
controllers are actually available. Libvirt or systemd ensures that
all controllers are available for VMs as well.
This still doesn't solve the devices controller with cgroups v2 where
there is no controller as it was replaced by eBPF. Currently libvirt
tries to query eBPF programs which usually works only for root as
regular users will get permission denied for that operation.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/94
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In QEMU 5.2, the guest agent learned to manipulate a user
~/.ssh/authorized_keys. Bind the JSON API to libvirt.
https://wiki.qemu.org/ChangeLog/5.2#Guest_agent
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Since both APIs accept/return an array of strings we can't have
client/server dispatch code generated. But implementation is
fairly trivial, although verbose.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When setting up a new guest or when a management software wants
to allow access to an existing guest the
virDomainSetUserPassword() API can be used, but that might be not
good enough if user want to ssh into the guest. Not only sshd has
to be configured to accept password authentication (which is
usually not the case for root), user have to type in their
password. Using SSH keys is more convenient. Therefore, two new
APIs are introduced:
virDomainAuthorizedSSHKeysGet() which lists authorized keys for
given user, and
virDomainAuthorizedSSHKeysSet() which modifies the authorized
keys file for given user (append, set or remove keys from the
file).
It's worth nothing that while authorized_keys file entries have
some structure (as defined by sshd(8)), expressing that structure
goes beyond libvirt's focus and thus "keys" are nothing but an
opaque string to libvirt.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This reverts commit b3710e9a2a.
That check is very valuable for our code, but it causes issue with glib >=
2.67.0 when building with clang.
The reason is a combination of two commits in glib, firstly fdda405b6b1b which
adds a g_atomic_pointer_{set,get} variants that enforce stricter type
checking (by removing an extra cast) for compilers that support __typeof__, and
commit dce24dc4492d which effectively enabled the new variant of glib's atomic
code for clang. This will not be necessary when glib's issue #600 [0] (8 years
old) is fixed. Thankfully, MR #1719 [1], which is supposed to deal with this
issue was opened 3 weeks ago, so there is a slight sliver of hope.
[0] https://gitlab.gnome.org/GNOME/glib/-/issues/600
[1] https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1719
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Similarly to previous commits, we can utilize domCaps to check if
graphics type is supported.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
QEMU supports egl-headless if QEMU_CAPS_EGL_HEADLESS capability
is present. There are some additional requirements but those are
checked for in qemuValidateDomainDeviceDefGraphics() and depend
on domain configuration and thus are not representable in domain
capabilities. Let's stick with plain qemuCaps check then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
In my recent commit of 5216304bfe I've moved RNG model check
from domain capabilities validator into qemu validator. During
that I had to basically duplicate RNG model to qemuCaps checks.
Problem with this approach is that after my commit qemu validator
and domCaps are disconnected and thus domCaps might report (in
general) different set of supported RNG models.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
In my recent commit of a33279daa8 I've moved video model check
from domain capabilities validator into qemu validator. During
that I had to basically duplicate video model to qemuCaps checks.
Problem with this approach is that after my commit qemu validator
and domCaps are disconnected and thus domCaps might report (in
general) different set of supported video models.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
This is a convenient macro for querying whether particular domain
caps enum value is set or not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
The helper methods for getting integer properties ignore a missing
property setting its value to zero. This lack of error reporting
resulted in missing the regression handling hotplug of USB devices
with the vendor and model IDs getting set to zero silently.
The few callers which relied on this silent defaulting have been fixed,
so now we can report fatal errors immediately.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The access of DKD_MEDIA_AVAILABLE for floppy disks, is mistakenly
protected by a check for ID_CDROM_MEDIA, introduced in:
commit 10427db779
Author: Ján Tomko <jtomko@redhat.com>
Date: Fri Jun 3 16:10:21 2016 +0200
Only return two values in udevGetUintProperty
Thus the check of DKD_MEDIA_AVAILABLE never run. In practice this didn't
matter since this property is set by the DeviceKit-Disks daemon which
was only around for 3 Fedora releases before being killed off around
F13. Thus we can just remove this legacy property.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The udevProcessStorage method relies on udevGetIntProperty ignoring
errors about non-existant properties and instead setting the value to
zero. In theory when seeing ID_CDROM=1, you might expect that devices
which are not CDs will get ID_CDROM=0, but that's not what happens in
practice. Instead the property simply won't get set at all.
IOW, the code does not need to care about the value of the property,
merely whether it exists or not.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
'kvm-spice' is a binary name used to call 'kvm' which actually is a wrapper
around qemu-system-x86_64 enabling kvm acceleration. This isn't in use
for quite a while anymore, but required to work for compatibility e.g.
when migrating in old guests.
For years this was a symlink kvm-spice->kvm and therefore covered
apparmor-wise by the existing entry:
/usr/bin/kvm rmix,
But due to a recent change [1] in qemu packaging this now is no symlink,
but a wrapper on its own and therefore needs an own entry that allows it
to be executed.
[1]: https://salsa.debian.org/qemu-team/qemu/-/commit/9944836d3
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Michal Privoznik <mprivozn redhat com>
QEMU version 4.2 introduced a performance feature under commit
d645e13287 ("kvm: i386: halt poll control MSR support").
This patch adds a new KVM feature 'poll-control' to set this performance
hint for KVM guests. The feature is off by default.
To enable this hint and have libvirt add "-cpu host,kvm-poll-control=on"
to the QEMU command line, the following XML code needs to be added to the
guest's domain description:
<features>
<kvm>
<poll-control state='on'/>
</kvm>
</features>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In v6.3.0-rc1~67 I've made a switch: instead of listening on udev
events the nodedev driver started listening for kernel events.
This was because when a device changes its name (e.g. NICs) we
will get "move" event with DEVPATH_OLD property set, which we can
then use to remove the old device and thus keep our internal list
up to date. The switch to "kernel" source was made because if the
old NICs naming (eth0, eth1, ...) is enabled (e.g. via
net.ifnames=0 on the kernel cmd line) then udev overwrites the
property with the new name making our internal list go out of
sync. Interestingly, when the od NICs naming is not enabled then
the DEVPATH_OLD contains the correct value.
But as it turns out, "kernel" source might be missing some other
important properties, e.g. USB vendor/product IDs. Therefore,
switch back to "udev" source and wish the best of luck to users
using the old NICs naming.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1897625
Fixes: 9a13704818
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Now that the domCaps cache is history, this code is no longer
used and thus can be removed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Currently, whenever a domain capabilities is needed (fortunately,
after cleanup done by previous commits it is now only in
virConnectGetDomainCapabilities()), the object is stored in a
cache. But there is no invalidation mechanism for the cache
(except the implicit one - the cache is part of qemuCaps and thus
share its lifetime, but that is not enough). Therefore, if
something changes - for instance new firmware files are
installed, or old are removed these changes are not reflected in
the virConnectGetDomainCapabilities() output.
Originally, the caching was there because domCaps were used
during device XML validation and they were used a lot from our
test suite. But this is no longer the case. And therefore, we
don't need the cache and can construct fresh domCaps on each
virConnectGetDomainCapabilities() call.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1807198
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Now that nothing uses virDomainCapsDeviceDefValidate() it can be
removed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The aim is to eliminate virDomainCapsDeviceDefValidate(). And in
order to do so, the domain video model has to be validated in
qemuValidateDomainDeviceDefVideo().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The aim is to eliminate virDomainCapsDeviceDefValidate(). And in
order to do so, the domain RNG model has to be validated in
qemuValidateDomainRNGDef().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This is a Coverity fix pointed out by John in IRC. This code
was introduced in 19d74fdf0e, when the TPM Proxy device for
for ppc64 was introduced.
This will leak in case we have 2 TPMs in the same domain, a
possible scenario with the protected Ultravisor execution in
PowerPC guests.
Fixes: 19d74fdf0e
Reported-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Introduced by commit <22494556542c676d1b9e7f1c1f2ea13ac17e1e3e> which
fixed a CVE.
If the @path passed to virDMSanitizepath() is not a DM name or not a
path to DM name this function could return incorrect sanitized path as
it would always be the first device under /dev/mapper/.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Commit <2f3b7a5555c4cf4127ff3f8e00746eafcc91432c> replaced VIR_STRDUP
by g_strdup which made the error: path mostly useless.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The issue was introduced together with the function itself by commit
<da1eba6bc8f58bfce34136710d1979a3a44adb17>. Calling
`virDomainObjGetPersistentDef` may return NULL which is later passed
to `virDomainDefFormat` where the `def` attribute is marked as NONNULL
and later in `virDomainDefFormatInternalSetRootName` it is actually
defererenced without any other check.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Leftover after commit <479a8c1fa1e0f58d3165c0446cd1abd72160256e>.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Libvirt's backup code has two modes:
1) push - where qemu actively writes the difference since the checkpoint
into the output file
2) pull - where we instruct qemu to expose a frozen disk state along
with a bitmap of blocks which changed since the checkpoint
For push mode qemu needs the temporary bitmap we use where we calculate
the actual changes to be present on the block node backing the disk.
For pull mode where we expose the bitmap via NBD qemu actually wants the
bitmap to be present for the exported block node which is the scratch
file.
Until now we've calculated the bitmap twice and installed it both to the
scratch file and to the disk node, but we don't need to since we know
when it's needed.
Pass in the 'pull' flag and decide where to install the bitmap according
to it and also when to register the bitmap name with the blockjob.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The NBD server used to export pull-mode backups doesn't have any other
form of client authentication on top of the TLS transport, so the only
way to authenticate clients is to verify their certificate.
Enable this option by defauilt when both 'backup_tls_x509_verify' and
'default_tls_x509_verify' were not configured.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1879477
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The migration stream connection and also the NBD server for non-shared
storage migration don't have any other form of client authentication on
top of the TLS transport, so the only way to authenticate clients is to
verify their certificate.
Enable this option by defauilt when both 'migrate_tls_x509_verify' and
'default_tls_x509_verify' were not configured.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1879477
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Chardevs don't have any other form of client authentication on top of
the TLS transport, so the only way to authenticate clients is to verify
their certificate.
Enable this option by defauilt when both 'chardev_tls_x509_verify' and
'default_tls_x509_verify' were not configured.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1879477
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If both "vnc_tls_x509_verify" and "default_tls_x509_verify" are missing
from the config file the client certificate validation is disabled. VNC
provides a layer of authentication so client certificate validation is
not strictly required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Store whether "default_tls_x509_verify" was provided and enhance the
SET_TLS_VERIFY_DEFAULT macro so that indiviual users can provide their
own default if "default_tls_x509_verify" config option was not provided.
For now we keep setting it to 'false'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Typecast the controller type variable to the appropriate type and add
the missing controller types for future extension.
Note that we currently allow only unplug of
VIR_DOMAIN_CONTROLLER_TYPE_SCSI thus the other controller types which
are not implemented return false now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Iterate through hostdevs only when the controller type is
VIR_DOMAIN_CONTROLLER_TYPE_SCSI.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The tests which match the disk bus to the controller type were backwards
in this function. This meant that any disk bus type (such as
VIR_DOMAIN_DISK_BUS_SATA) would not skip the controller index comparison
even if the removed controller was of a different type.
Switch the internals to a switch statement with selects the controller
type in the first place and a proper type so that new controller types
are added in the future.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870072
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While it's certainly good to log events like "failed to close fd"
and "tried to close invalid fd", which are likely to be the
consequence of some bug in libvirt, logging a message every single
time a file descriptor is closed successfully is perhaps excessive
and can lead to useful information being missed among the noise.
Log filters don't help in this situation, because filtering out all
of util.file is too big a hammer and would cause important messages
to be left out as well.
To give an idea of just how much noise this single debug statement
can cause, here's a real life example from a quite large libvirtd
log I had to look at recently:
$ grep virFile libvirt.log | wc -l
1307
$ grep virFile libvirt.log | grep -v 'Closed fd' | wc -l
343
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Before commit 24d8968c, virDirClose took a DIR**, and that was never
NULL, so its declaration included ATTRIBUTE_NONNULL(1). Since that
commit, virDirClose takes a DIR*, and it may be NULL (e.g. if the DIR*
is initialized to NULL and was never closed).
Even though virDirClose() is currently only called implicitly (as the
cleanup for a g_autoptr(DIR)), and (as I've just newly learned) the
autocleanup function g_autoptr will only be called if the pointer in
question is non-null (see the definition of
_GLIB_AUTOPTR_CLEAR_FUNC_NAME in
/usr/include/glib-2.0/glib/gmacros.h), it does still cause Coverity to
complain that it *could* be called with a NULL, and it's also possible
that in the future someone might add code that explicitly calls
virDirClose.
To eliminate the Coverity complaints, and protect against the
hypothetical future where someone both explicitly calls virDirClose()
with a potentially NULL value, *and* re-enables the nonnull directive
when not building with Coverity (disabled by commit eefb881) this
patch removes the ATTRIBUTE_NONNULL(1) from the declaration of
virDirClose().
Fixes: 24d8968cd0
Reported-by: John Ferlan <jferlan@redhat.com>
Details-Research-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Laine Stump <laine@redhat.com>
A bad merge while rebasing 74b2834333 caused the @event variable
to be defined twice, inside the 'cleanup' label, causing coverity
errors.
This code was originally moved outside of the label by commit
773c7c4361. Delete the unintended code in the 'cleanup'
label.
Fixes: 74b2834333
Reported-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
