Andrea Bolognani [Wed, 10 Apr 2019 13:08:46 +0000 (15:08 +0200)]
tests: Fix MinGW build for domaincapstest
Commit 5b9819eedc71 started using the virFileWrapper APIs in
the test program, and correctly called them only in the section
of code guarded by WITH_QEMU; however, a single call to the
virFileWrapperClearPrefixes() function ended up in the
hypervisor-agnostic section, causing a build failure on MinGW.
Move the call to the QEMU-only section; while at it, also drop
the virFileWrapperRemovePrefix() calls, which are entirely
redundant since we'd drop all prefixes immediately afterwards
anyway.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Acked-by: Michal Privoznik <mprivozn@redhat.com>
domain capabilities: Expose firmware auto selection feature
If a management application wants to use firmware auto selection
feature it can't currently know if the libvirtd it's talking to
support is or not. Moreover, it doesn't know which values that
are accepted for the @firmware attribute of <os/> when parsing
will allow successful start of the domain later, i.e. if the mgmt
application wants to use 'bios' whether there exists a FW
descriptor in the system that describes bios.
This commit then adds 'firmware' enum to <os/> element in
<domainCapabilities/> XML like this:
We can see both 'bios' and 'efi' listed which means that there
are descriptors for both found in the system (matched with the
machine type and architecture reported in the domain capabilities
earlier and not shown here).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Acked-by: Laszlo Ersek <lersek@redhat.com>
The point of this API is to fetch all FW descriptors, parse them
and return list of supported interfaces and SMM feature for given
combination of machine type and guest architecture.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Acked-by: Laszlo Ersek <lersek@redhat.com>
Getting rid of unistd.h from our headers will require more work than
just fixing the broken mingw build. Revert it until I have a more
complete proposal.
Peter Krempa [Mon, 1 Apr 2019 13:14:30 +0000 (15:14 +0200)]
util: Move the VIR_AUTO(CLEAN|PTR) helper macros into a separate header
Keeping them with viralloc.h forcibly pulls in the other stuff from
viralloc.h into other header files. This in turn creates a mess
as more and more headers pull in the 'viral' header file.
If we want to make 'viralloc.h' omnipresent we should pick a different
approach.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Han Han [Tue, 9 Apr 2019 10:01:25 +0000 (18:01 +0800)]
vmx: Define VMX_CONFIG_FORMAT_ARGV
Define VMX_CONFIG_FORMAT_ARGV to replace the hardcoded 'vmware-vmx'
string used by the domxml-X-native APIs. This follows the pattern used
by other drivers.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Han Han <hhan@redhat.com>
Peter Krempa [Tue, 2 Apr 2019 15:35:42 +0000 (17:35 +0200)]
rpc: Refactor cleanup paths in virNetLibsshAuthenticatePassword
Now that the memory disposal is handled automatically we can simplify
the cleanup paths. In this case it's not as simple as sometimes the
value of the called function is returned.
While at it fix the initialization value of the returned variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Julio Faracco [Mon, 8 Apr 2019 20:32:14 +0000 (17:32 -0300)]
util: Fix uninitalized variable to avoid garbage
This commit fixes an unitialized variable to avoid garbage value
when virNetDevBridgeGet method returns error. When, that method fails
before initialize 'val' variable, it can cause problems related to
that.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Eric Blake [Mon, 8 Apr 2019 16:45:47 +0000 (11:45 -0500)]
snapshot: Fix use-after-free during snapshot delete
Commit b647d2195 introduced a use-after-free situation when the caller
is trying to delete a snapshot and its children: if the callback
function deletes the parent, it is no longer safe to query the parent
to learn which children also need to be deleted (where we previously
saved deleting the parent for last). To fix the problem, while still
maintaining support for topological visits of callback functions, we
have to stash off any information needed for later traversal prior to
using a callback function (virDomainMomentForEachChild already does
this, it is only virDomainMomentActOnDescendant that was running into
problems).
Sadly, the testsuite did not cover the problem at the time. Worse,
even though I later added commit 280a2b41e to catch problems like
this, and even though that test is indeed sufficient to detect the
problem when run under valgrind or suitable MALLOC_PERTURB_ settings,
I'm guilty of not running the test in such an environment. Thus,
v5.2.0 has a regression that could have been prevented had we used the
testsuite to its full power. On the bright side, deleting snapshots
requires ACL domain:snapshot, which is arguably as powerful as
domain:write, so I don't think this use-after-free forms a security
hole.
At some point, it would be nice to convert virDomainMomentObj into a
virObject, at which point, the solution is even simpler: add
virObjectRef/Unref around the callback. But as that will require
auditing even more places in the code, I went with the simplest patch
for the regression fix.
Fixes: b647d2195 Reported-by: Roman Bogorodskiy <bogorodskiy@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com> Tested-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
The microcode version checks are used to invalidate cached CPU data we
get from QEMU. To minimize /proc/cpuinfo parsing the microcode version
was only read when libvirtd started and cached for the daemon's
lifetime. However, the CPU microcode can change anytime (updating the
microcode package can automatically upload it to the CPU) and we need to
stop caching it to avoid using stale CPU model data.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The example currently assumes that a NULL URI will open Xen and thus
also assumes that a domain with ID 0 exists. Change it to require the
URI and a domain name as command line arguments.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
libvirt.org/search.php drops into some kind of screen which I guess
is supposed to show a search bar with options, but presently for me
renders as nothing but the following text:
Search the documentation on Libvirt.org
The search service indexes the libvirt APIs and documentation as well as the libvir-list@redhat.com mailing-list archives. To use it simply provide a set of keywords:
The main page search bar now redirects to google, this page is broken,
I say we just remove it and move on.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Cole Robinson <crobinso@redhat.com>
The website search is perpetually broken, has had XSS issues in the
past, and I suspect when it's working it's not as fast or capable as
a simple google site:libvirt.org search
Replace the <form> implementation with one that sends the user to
google.com with 'site:libvirt.org' appended to the search string
Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Cole Robinson <crobinso@redhat.com>
Eric Blake [Wed, 27 Mar 2019 18:42:45 +0000 (13:42 -0500)]
tests: Avoid writing into $HOME during virsh-snapshot
In a constrained CI environment, where it is intentional that attempts
to write outside the current directory will fail, virsh-snapshot was
failing:
@@ -1,2 +1,3 @@
error: invalid argument: parent s3 for snapshot s2 not found
error: marker
+error: Failed to create '/home/travis/.cache/libvirt/virsh': Permission denied
FAIL virsh-snapshot (exit status: 1)
But we've already solved the problem in virsh-uriprecedence: tell
virsh to use XDG locations pointing to somewhere we can write rather
than its default of falling back to $HOME with the test being at risk
of breaking due to the user's environment and/or unacceptably altering
the user's normal cache. Hoist that solution into test-lib.sh, so
that all scripts can use it as needed. While at it, fix a latent typo
where XDG_RUNTIME_HOME was set to a literal relative directory name
"XDG_CACHE_HOME" (the typo did not affect virsh-uriprecedence, but
could matter to other clients).
Fixes: 280a2b41 Fixes: 398de147 Reported-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
docs: stop advertizing FTP or HTTP for downloads of libvirt
On the modern internet it is not credible to continue to advertize
software downloads over unencrypted connections. Even if users could
theoretically use GPG to verify the signatures, not all our downloads
are signed and few people know how to correctly verify signatures.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Andrea Bolognani [Thu, 21 Mar 2019 15:35:27 +0000 (16:35 +0100)]
tests: Refresh capabilities for QEMU 4.0.0 on RISC-V
There are a few differences, but the one we're interested in is
that PCIe Root Ports are finally available: as a result of this,
our riscv64-virt-headless guest will switch from virtio-mmio to
virtio-pci.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com>
Andrea Bolognani [Thu, 21 Mar 2019 15:50:18 +0000 (16:50 +0100)]
qemu: Require PCIe Root Port for PCI by default on ARM virt
Our PCIe topology depends on the availability of PCIe Root Ports,
so if none of the suitable devices (pcie-root-port, ioh3420) is
compiled into QEMU we should fall back to virtio-mmio rather than
trying to use PCI addresses only to fail immediately afterwards
when we realize we can't use the necessary controllers.
Note that this additional check is basically moot for ARM virt
guests, because PCIe Root Ports were enabled in QEMU builds for
the architecture well before guest OS support had been widely
available; however, the opposite is true for RISC-V, and tweaking
the code this way will allow us to share it between architectures.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com>
If the console was disconnected due to a connection problem or a problem on the
server side it is convinient to provide the cause to the user. If the error
come from the API then the error is saved in a virsh global variable. However,
since success is returned from virshRunConsole after we reach the waiting stage,
then the error is never reported. Let's track the error in the event loop.
Next after failure we do a cleanup and this cleanup can overwrite
root cause. Thus let's save root cause immediately and then set it to
virsh error after all cleanup is done.
Since we'll be sending the error to the consumer, each failure path from
the event handlers needs to be augmented to provide what error generated
the failure.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
tools: console: check if console was shutdown in callbacks
On error in main thread virConsoleShutdown is called which
deletes fd watches/stream callback and yet callbacks can
be called after. Thus we can incorrectly allocate
terminalToStream.data memory and get memory leak for example.
Let's check if console was shutdown in the very beginning of
callbacks.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Stream/fd callbacks accessing console object are called from the
event loop thread and the console object is also accessed from
the main thread so we are better add locking to handlers.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
tools: console: cleanup console on errors in main thread
We only check now for virObjectWait failures in virshRunConsole but
we'd better check and for other failures too. And we need to shutdown
console on error in the main thread.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
We need to turn console into virObject object because stream/fd callbacks
can be called from the event loop thread after freeing console
in main thread. It is convinient to turn into virLockableObject as
we have mutex in console object.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
qemu: Don't duplicate suspend events and state changes
Since the STOP event handler can use the pausedReason as sent to
qemuProcessStopCPUs, we no longer need to send duplicate suspended
lifecycle events because we know what caused the stop along with extra
details. This processing allows us to also remove the duplicated state
change from qemuProcessStopCPUs.
Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
qemu: Pass stop reason from qemuProcessStopCPUs to stop handler
Similar to commit [1] which saves and passes the running reason to
the RESUME event handler, during qemuProcessStopCPUs let's save and pass
the pause reason in the domain private data so that the STOP event
handler can use it.
Michal Privoznik [Tue, 19 Mar 2019 13:39:20 +0000 (14:39 +0100)]
virNWFilterBindingObjListAddLocked: Produce better error message than 'Duplicate key'
If there are two concurrent threads, one of which is removing an
nwfilter from the list and the other is trying to add it back they
may serialize in the following order:
1) obj->removing is set and @obj is unlocked.
2) The tread that's trying to add the nwfilter onto the list locks
the list and tries to find, if the nwfilter already exists.
3) Our lookup functions say it doesn't, so the thread proceeds to
virHashAddEntry() which fails with 'Duplicate key' error.
This is obviously not helpful error message at all.
The problem lies in our lookup function
(virNWFilterBindingObjListFindByPortDevLocked()) which return
NULL even if the object is still on the list. They do this so
that the object is not mistakenly looked up by some API. The fix
consists of moving 'removing' check one level up and thus
allowing virNWFilterBindingObjListAddLocked() to produce
meaningful error message.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com>
Michal Privoznik [Tue, 19 Mar 2019 12:26:21 +0000 (13:26 +0100)]
virDomainObjListAddLocked: Produce better error message than 'Duplicate key'
If there are two concurrent threads, one of which is removing a
domain from the list and the other is trying to add it back they
may serialize in the following order:
1) vm->removing is set and @vm is unlocked.
2) The tread that's trying to add the domain onto the list locks
the list and tries to find, if the domain already exists.
3) Our lookup functions say it doesn't, so the thread proceeds to
virHashAddEntry() which fails with 'Duplicate key' error.
This is obviously not helpful error message at all.
The problem lies in our lookup functions
(virDomainObjListFindByUUIDLocked() and
virDomainObjListFindByNameLocked()) which return NULL even if the
object is still on the list. They do this so that the object is
not mistakenly looked up by some driver. The fix consists of
moving 'removing' check one level up and thus allowing
virDomainObjListAddLocked() to produce meaningful error message.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com>
Up until memfd introduction (in 24b74d187ca) we did not need to
know @pagesize because qemuGetDomainHupageMemPath() could deal
with it being zero (value of zero means use the default hugetlbfs
mount). But since for memfd we are not passing a path to
hugetlbfs mount rather the page size value we need to know its
value upfront.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Fri, 29 Mar 2019 15:58:23 +0000 (16:58 +0100)]
qemuxml2xmltest: Add memfd tests
Somehow, these were not tested. Use symlinks to point expected
output back to the input. This way we can also fix some
discrepancies in the input XMLs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Fri, 29 Mar 2019 15:30:04 +0000 (16:30 +0100)]
qemuxml2argvdata: Drop useless spaces at the beginning of lines
There are three test XMLs that have useless spaces at the
beginning of each line. I intend to add these to qemuxml2xmltest
and make xmlout a symlink to the original XML. In order to do
that the XMLs must look better than they do now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ján Tomko [Fri, 1 Jun 2018 11:10:11 +0000 (13:10 +0200)]
virjsontest: switch DO_TEST_PARSE_FILE to use output files
Also switch the expected output of DO_TEST_PARSE_FILE to be
in a file, now that we demonstrated the input files match
the expected string representation.
Ján Tomko [Wed, 3 Apr 2019 00:29:03 +0000 (02:29 +0200)]
configure.ac: add foreign to AM_INIT_AUTOMAKE
We do not care about the portability warnings implied by the implicit
'gnu' option. Switch to 'foreign' to opt out of checking the files
present in the top directory to let us drop ChangeLog completely.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Ján Tomko [Wed, 3 Apr 2019 00:21:28 +0000 (02:21 +0200)]
configure.ac: drop -Wno-obsolete from AM_INIT_AUTOMAKE
Even Ubuntu 16.04 has automake 1.11.
Now that we no longer cater to automake 1.9, drop the comment
as well as the -Wno-obsolete option, since it does not seem to generate
any warnings anymore.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Ján Tomko [Tue, 2 Apr 2019 21:59:38 +0000 (23:59 +0200)]
virJSONValueToString: bail out early on error
Now that we do not need to cater to YAJL 1, move the check for the
return value of yajl_gen_alloc earlier, so that we can assume it
was successful in later code.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
qemu: fix domain unlock/unref in qemuMigrationSrcPerform
qemuMigrationSrcPerform callers expect it to call virDomainObjEndAPI
in any case so on error paths we miss the virDomainObjEndAPI call.
To fix this let's make qemuMigrationSrcPerform callers responsible
for the virDomainObjEndAPI call.
ACKed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
examples: fix 64-bit integer formatting on Windows
The Windows printf functions don't support %llu/%lld for printing 64-bit
integers. For most of libvirt this doesn't matter as we rely on gnulib
which provides a replacement printf that is sane.
The example code is designed to compile against the normal OS headers,
with no use of gnulib and thus has to use the platform specific printf.
To deal with this we must use the macros PRI* macros from inttypes.h
to get the platform specific format string.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
cfg.mk: block use of d_type from dirent by default
The use of d_type is non-portable and leads to surprises when the OS
does not fill in any value except DT_UNKNOWN. Blacklist its usage
except in files which inherantly don't require portability (cgroups).
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The d_type field cannot be assumed to be filled. Some filesystems, such
as older XFS, will simply report DT_UNKNOWN.
Even if the d_type is filled in, the use of it in the SELinux functions
is dubious. If labelling all files in a directory there's no reason to
skip things which are not regular files. We merely need to skip "." and
"..", which is done by virDirRead() already.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
qemu: don't rely on the non-portable d_type field in dirent
d_type is a non-portable extension to the struct dirent and even if it
exists, its value may be DT_UNKNOWN if the filesystem doesn't support
it. This is common with older versions of XFS which have ftype=0
feature.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Quite a few of the tests have a need to mock the stat() / lstat()
functions and they are taking somewhat different & inconsistent
approaches none of which are actually fully correct. This is shown
by fact that 'make check' fails on 32-bit hosts. Investigation
revealed that the code was calling into the native C library impl,
not getting intercepted by our mocks.
The POSIX stat() function might resolve to any number of different
symbols in the C library.
The may be an additional stat64() function exposed by the headers
too.
On 64-bit hosts the stat & stat64 functions are identical, always
refering to the 64-bit ABI.
On 32-bit hosts they refer to the 32-bit & 64-bit ABIs respectively.
Libvirt uses _FILE_OFFSET_BITS=64 on 32-bit hosts, which causes the
C library to transparently rewrite stat() calls to be stat64() calls.
Libvirt will never see the 32-bit ABI from the traditional stat()
call. We cannot assume this rewriting is done using a macro. It might
be, but on GLibC it is done with a magic __asm__ statement to apply
the rewrite at link time instead of at preprocessing.
In GLibC there may be two additional functions exposed by the headers,
__xstat() and __xstat64(). When these exist, stat() and stat64() are
transparently rewritten to call __xstat() and __xstat64() respectively.
The former symbols will not actally exist in the library at all, only
the header. The leading "__" indicates the symbols are a private impl
detail of the C library that applications should not care about.
Unfortunately, because we are trying to mock replace the C library,
we need to know about this internal impl detail.
With all this in mind the list of functions we have to mock will depend
on several factors
- If _FILE_OFFSET_BITS is set, then we are on a 32-bit host, and we
only need to mock stat64 and __xstat64. The other stat / __xstat
functions exist, but we'll never call them so they can be ignored
for mocking.
- If _FILE_OFFSET_BITS is not set, then we are on a 64-bit host and
we should mock stat, stat64, __xstat & __xstat64. Either may be
called by app code.
- If __xstat & __xstat64 exist, then stat & stat64 will not exist
as symbols in the library, so the latter should not be mocked.
The same all applies to lstat()
These rules are complex enough that we don't want to duplicate them
across every mock file, so this centralizes all the logic in a helper
file virmockstathelper.c that should be #included when needed. The
code merely need to provide a filename rewriting callback called
virMockStatRedirect(). Optionally VIR_MOCK_STAT_HOOK can be defined
as a macro if further processing is needed inline.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Tue, 26 Mar 2019 17:33:01 +0000 (18:33 +0100)]
qemu: monitor: Avoid unnecessary copies of command string
Use virJSONValueToBuffer so that we can append the command terminator
string without copying of the string again. Also avoid a 'strlen' as we
can query the buffer use size.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Laine Stump <laine@laine.org>
Peter Krempa [Tue, 26 Mar 2019 17:36:30 +0000 (18:36 +0100)]
qemu: monitor: Remove few debug statements
The internal qemu machinery already logs the sent message via the PROBE
point in qemuMonitorSend and the monitor receive function. Those are way
better as they are easy grepable. Remove the additional ones from the
monitor code which just duplicate the sent data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Laine Stump <laine@laine.org>