Commit Graph

40571 Commits

Author SHA1 Message Date
Daniel P. Berrangé
032394856b nodedev: dont rely on ignoring errors on missing properties
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>
2020-11-17 16:54:57 +00:00
Daniel P. Berrangé
f4b4bfdf41 nodedev: improve debugging logs from udev device/event processing
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-11-17 16:54:45 +00:00
Christian Ehrhardt
1441ce83fe
apparmor: allow kvm-spice compat wrapper
'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>
2020-11-17 15:56:43 +01:00
Tim Wiederhake
ea21b8b87b tests: Add tests for kvm-poll-control feature
Update the KVM feature tests for QEMU's kvm-poll-control performance
hint.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-17 14:40:54 +01:00
Tim Wiederhake
3fc4412c6f qemu: support kvm-poll-control performance hint
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>
2020-11-17 14:40:46 +01:00
Michal Privoznik
7e67a136da node_device: Use "udev" monitor source
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>
2020-11-17 14:28:47 +01:00
Daniel P. Berrangé
7e09a6f1fd rpm: enable wireshark build for RHEL-8 and later
wireshark plugin was disabled in RHEL because RHEL-7 was too old, but we
forgot to enable it in RHEL-8 where it builds fine.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-11-17 10:19:05 +00:00
Daniel P. Berrangé
8d3c427dea rpm: remove version checks for wireshark
We only turn on with_wireshark if we already know the distro is
guaranteed to have new enough packages. The versioned dep is thus not
required.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-11-17 10:19:03 +00:00
Daniel P. Berrangé
45d714ce2e meson: bump min wireshark to 2.6.0
If using the declared min version of wireshark, 2.4.0, libvirt plugin
fails to build. This min version isn't present in any supported distros
and thus not tested by CI.

We don't support wireshark on RHEL-7 since it has 1.x.x series. The next
oldest version present in supported distros is 2.6.2 on RHEL-8.

Thus we should bump the min version to 2.6.0. This also lets us assume
that the "plugindir" variable exists in pkg-config.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-11-17 10:19:00 +00:00
Daniel P. Berrangé
7b42f33278 meson: assume pkg-config support for yajl
Per the platform support rules, we no longer need to consider SLES 12 as
a target, and so can now assume pkg-config support in yajl.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-11-17 10:18:48 +00:00
Michal Privoznik
19c4c6f8fd qemu: Remove virQEMUDomainCapsCache code
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>
2020-11-16 18:27:00 +01:00
Michal Privoznik
7db61843b0 qemu: Don't cache domCaps in virQEMUDriverGetDomainCapabilities()
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>
2020-11-16 18:26:50 +01:00
Michal Privoznik
4b487e1052 conf: Drop virDomainCapsDeviceDefValidate()
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>
2020-11-16 18:26:49 +01:00
Michal Privoznik
a33279daa8 qemu: Validate video model
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>
2020-11-16 18:26:46 +01:00
Michal Privoznik
5216304bfe qemu: Validate RNG model
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>
2020-11-16 18:26:41 +01:00
Daniel Henrique Barboza
904e59f43a qemu_tpm.c: fix 'shortName' leak
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>
2020-11-16 13:55:06 -03:00
Pavel Hrdina
e8e90a35df testutils: call va_end before return
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2020-11-16 17:25:41 +01:00
Pavel Hrdina
b04908319b vboxGetDriverConnection: unlock vbox_driver_lock before return
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2020-11-16 17:25:41 +01:00
Pavel Hrdina
f711fa9ad0 virdevmapper: fix stat comparison in virDMSanitizepath
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>
2020-11-16 17:25:41 +01:00
Pavel Hrdina
caaf792eed remoteDomainGetFSInfo: remove unreachable cleanup code
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2020-11-16 17:25:41 +01:00
Pavel Hrdina
5ca76b9fbf remoteDomainGetIOThreadInfo: remove unreachable cleanup code
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2020-11-16 17:25:41 +01:00
Pavel Hrdina
82bda55e2f qemuProcessHandleGraphics: no need to check for NULL
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2020-11-16 17:25:41 +01:00
Pavel Hrdina
0e7549fe47 interface_backend_udev: refactor udevListInterfacesByStatus
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>
2020-11-16 17:14:17 +01:00
Pavel Hrdina
2b58ce9155 hyperv_wmi: remove unreachable cleanup code
In the cleanup section @data will always be NULL.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2020-11-16 17:14:04 +01:00
Pavel Hrdina
b96174d9f2 domain_conf: fix NULL dereference on error in virDomainObjCopyPersistentDef
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>
2020-11-16 17:13:42 +01:00
Pavel Hrdina
ba6385c952 domain_conf: remove unused rc variable
Leftover after commit <479a8c1fa1e0f58d3165c0446cd1abd72160256e>.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2020-11-16 17:12:09 +01:00
Peter Krempa
0316c28a45 qemu: backup: Install bitmap for incremental backup to appropriate node only
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>
2020-11-16 14:12:38 +01:00
Peter Krempa
6f30d7003a NEWS: Mention change of default for TLS certificate verification
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-16 14:11:56 +01:00
Peter Krempa
0200fe42a0 qemu: conf: Enable 'backup_tls_x509_verify' by default
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>
2020-11-16 14:11:56 +01:00
Peter Krempa
930583149c qemu: conf: Enable 'migrate_tls_x509_verify' by default
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>
2020-11-16 14:11:56 +01:00
Peter Krempa
019f962c86 qemu: conf: Enable 'chardev_tls_x509_verify' by default
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>
2020-11-16 14:11:56 +01:00
Peter Krempa
940ef34443 qemu: conf: Clarify default of "vnc_tls_x509_verify"
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>
2020-11-16 14:11:56 +01:00
Peter Krempa
9ba2a06e47 qemu: conf: Allow individual control of default value for *_tls_x509_verify
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>
2020-11-16 14:11:56 +01:00
Peter Krempa
6a1bb797a7 qemuDomainControllerIsBusy: Fully populate switch statement
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>
2020-11-16 14:05:06 +01:00
Peter Krempa
279ba2d1cc qemuDomainDiskControllerIsBusy: Optimize checking for SCSI hostdevs
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>
2020-11-16 14:05:06 +01:00
Peter Krempa
022f4d431b qemuDomainDiskControllerIsBusy: Fix logic of matching disk bus to controller type
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>
2020-11-16 14:05:06 +01:00
Peter Krempa
a6d5a5712f qemuDomain(Disk)ControllerIsBusy: Fix function header format
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-11-16 14:05:05 +01:00
Peter Krempa
9ce41c8896 docs: kbase: Reorder some articles in the 'Usage' section
Historically we've added them in chronological order, but certain
articles are more likely to be needed and thus are easier to find when
placed earlier.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-16 14:04:04 +01:00
Peter Krempa
f6b0a311aa docs: css: Modify appearance of the kbase directory page
Re-style the knowledge base to look like the 'docs.html' page.

We still have room to add one more column.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-16 14:04:04 +01:00
Peter Krempa
9fd2e78b96 docs: xsl: Unify stylability of main container element
page.xsl was adding '<div id="content">' wrapper for the content picked
up from the <body> element from the original input file. Optionally
class="$DOCNAME" was added for some documents taken from <body>.

Since docs generated from RST by docutils have a '<div class='document'
id='$DOCNAME>' we actually don't need an extra wrapper for them.

Additionally if we standardize on one of them we can use the same styles
for both. I've picked the latter because it makes more sense to use the
document name as 'id'.

This patch:
1) Modifies the XSL trasformation to add the wrapper only if it's not
   present.

2) Modifies the XSL transformation to use 'id' for document name and
   class='document' for the wrapper element.

3) Changes docs.html/index.html/hvsupport.html to use 'id' instead of
   'class' for document name.

4) Modifies the main stylesheet to keep styling the elements properly

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-16 14:04:04 +01:00
Peter Krempa
5f0d7fc636 docs: kbase: Split articles into sections
Split the existing list of kbase articles into a 'Usage' category and
into 'Internals/Debugging'. This will later represent the two columns on
the web page.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-16 14:04:03 +01:00
Peter Krempa
24cd919aaa docs: kbase: Remove extra container from index page
The container was used to apply CSS classes to the content, so the looks
are degraded. The idea is to have a similar layout to the 'docs.html'
page with multiple columns, which will be added later.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-16 14:04:03 +01:00
Peter Krempa
7462301955 docs: kbase: Move index page to docs/kbase
Move docs/kbase.rst to docs/kbase/index.rst so that the directory itself
shows our index page rather than the autogenerated list of files by the
webserver.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-16 14:04:03 +01:00
Peter Krempa
95f059838e docs: css: Add a gray box around table of contents of RST based docs
Emphasise the table of contents visually.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-16 14:04:03 +01:00
Peter Krempa
f988d4320d docs: xslt: Use 'Link' rather than 'Permalink' in header links
The anchors are based on the article or chapter headers, thus they are
not 100% permanent. Especially with pages generated from RST.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-16 14:04:03 +01:00
Peter Krempa
501ff523e7 docs: Fix title of 'docs' page
XSLT transformation generates the page title from the topmost <h1>
element which is not present in 'docs.html.in'. Add it and hide it in
the CSS.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-16 14:04:03 +01:00
Andrea Bolognani
a35426bb64 news: Fix vertical and horizontal spacing
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2020-11-16 12:42:53 +01:00
Jianan Gao
99e5c011b3 news: Document apparmor improvements in v6.7.0
Add news about apparmor improvements.

Signed-off-by: Jianan Gao <jgao@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2020-11-16 12:37:38 +01:00
Andrea Bolognani
57515a4c36 util: Make virFileClose() quiet on success
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>
2020-11-16 09:18:03 +01:00
Laine Stump
7754933983 util: remove ATTRIBUTE_NONNULL from virDirClose declaration
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>
2020-11-13 14:58:48 -05:00