Julio Faracco [Wed, 5 Sep 2018 04:20:56 +0000 (01:20 -0300)]
qemu: unlink the error report from VIR_STRDUP.
The function to retrieve the file system info using QEMU-GA is using
some conditionals to retrieve the info. This is wrong because the error
of some conditionals will be raised if VIR_STRDUP return errors and not
if some problem occurred with JSON.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
This commit implements the function qemuAgentGetHostname() that uses
the QEMU guest agent command 'guest-get-host-name' to retrieve the
guest hostname of virtual machine running the QEMU-GA.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
tests: Add simple headless guests using latest caps
The new tests use DO_TEST_CAPS_ARCH_LATEST() with an input
XML describing a very simple headless guest and cover most
architectures and machine types we care about.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Thu, 30 Aug 2018 16:03:58 +0000 (18:03 +0200)]
tests: Fix use of virtio-serial for aarch64/virt
virtio-serial is an alias for virtio-serial-pci, which
should not have been used for a PCIe-less aarch64/virt
guest but it ended up being used anyway because the
virtio-mmio capability was missing and the algorithm
is buggy.
Fix the test case so that we can fix the algorithm next.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's a single user for it which takes an existing
virPCIDeviceAddress, passes its various bits to the
function which in turn constructs a virPCIDevice and
then copies the string representation for the caller
to use: we can use virPCIDeviceAddressAsString()
instead and avoid creating the virPCIDevice in the
first place. Since the function ends up having no
users after the change, we can just drop it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Wed, 22 Aug 2018 07:56:07 +0000 (09:56 +0200)]
virLockManagerLockDaemonAddResource: Switch to cleanup label rather than error
This will help in future expansions of the code when it is be
harder to track if @newName and/or @newLockspace were already
allocated or not and thus whether it is safe to 'return' or we
need to 'goto error'. By using the 'cleanup' label those two
cases merge into a single one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Michal Privoznik [Wed, 22 Aug 2018 09:59:45 +0000 (11:59 +0200)]
virLockManagerSanlockAddResource: Do not ignore unknown resource types
Currently, there are only two types of resource. So effectively
this is a dead code. However, that assumption can change and we
shouldn't just silently ignore the error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Michal Privoznik [Tue, 21 Aug 2018 12:06:44 +0000 (14:06 +0200)]
lock_driver_lockd: Don't leak lockspace dirs
On daemon deinit only fileLockSpaceDir is freed. The other two
(scsiLockSpaceDir and lvmLockSpaceDir) are missing even though
they are allocated in virLockManagerLockDaemonLoadConfig().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Michal Privoznik [Sun, 26 Aug 2018 06:48:47 +0000 (08:48 +0200)]
lock_daemon: Fix some memleaks
28 bytes in 1 blocks are definitely lost in loss record 26 of 66
at 0x4C2CF0F: malloc (vg_replace_malloc.c:299)
by 0x7A02719: strdup (strdup.c:42)
by 0x197DC1: virStrdup (virstring.c:961)
by 0x12B478: virLockDaemonConfigFilePath (lock_daemon_config.c:44)
by 0x12A759: main (lock_daemon.c:1270)
62 (32 direct, 30 indirect) bytes in 1 blocks are definitely lost in loss record 41 of 66
at 0x4C2EF26: calloc (vg_replace_malloc.c:711)
by 0x151B61: virAlloc (viralloc.c:144)
by 0x12B56C: virLockDaemonConfigNew (lock_daemon_config.c:71)
by 0x12A491: main (lock_daemon.c:1262)
13 bytes in 1 blocks are definitely lost in loss record 21 of 70
at 0x4C2CF0F: malloc (vg_replace_malloc.c:299)
by 0x7A02719: strdup (strdup.c:42)
by 0x197E3F: virStrdup (virstring.c:961)
by 0x12C86B: virLockSpaceProtocolDispatchRegister (lock_daemon_dispatch.c:291)
by 0x12BB73: virLockSpaceProtocolDispatchRegisterHelper (lock_daemon_dispatch_stubs.h:152)
by 0x1336AA: virNetServerProgramDispatchCall (virnetserverprogram.c:437)
by 0x13320D: virNetServerProgramDispatch (virnetserverprogram.c:304)
by 0x139E3E: virNetServerProcessMsg (virnetserver.c:144)
by 0x13A1A2: virNetServerDispatchNewMessage (virnetserver.c:230)
by 0x1350F5: virNetServerClientDispatchMessage (virnetserverclient.c:343)
by 0x137680: virNetServerClientDispatchEvent (virnetserverclient.c:1498)
by 0x147704: virNetSocketEventHandle (virnetsocket.c:2140)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
tests: rewrite socket to do something sensible and reliable
The current socket test is rather crazy in that it sets up a server
listening for sockets and then runs a client connect call, relying on
the fact that the kernel will accept this despite the application
not having called accept() yet. It then closes the client socket and
calls accept() on the server. On Linux accept() will always see that
the client has gone and so skip the rest of the code. On FreeBSD,
however, the accept sometimes succeeds, causing us to then go into
code that attempts to read and write to the client which will fail
aborting the test. The accept() never succeeds on FreeBSD guests
with a single CPU, but as you add more CPUs, accept() becomes more and
more likely to succeed, giving a 100% failure rate for the test when
using 8 CPUs.
This completely rewrites the test so that it is avoids this designed in
race condition. We simply spawn a background thread to act as the
client, which will read a byte from the server and write it back again.
The main thread can now properly listen and accept the client in a
synchronous manner avoiding any races.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
tests: skip some unicode tests if expected output won't match
The expected output strings from the vshtabletest.c are created on a
modern Linux host where unicode printing support is very good. On older
Linux platforms, or non-Linux platforms, some unicode characters will
not be considered printable. While the vsh table alignment code will
stil do the right thing with escaping & aligning in this case, the
result will not match the test's expected output.
Since we know the code is working correctly, do a check with iswprint()
to validate the platform's quality and skip the test if it fails. This
fixes the test on FreeBSD platforms.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Andrea Bolognani [Wed, 29 Aug 2018 16:19:15 +0000 (18:19 +0200)]
conf: Move virDomainDeviceAddressIsValid() to device_conf
The function is called on a virDomainDeviceInfo, so it
should be declared along with it.
Moving this function requires moving and making public
virDomainDeviceCCWAddressIsValid() as well, but that's
perfectly fine since the same reasoning above also
applies to it, due to virDomainDeviceCCWAddress being
(correctly) declared in device_conf.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Tue, 28 Aug 2018 14:04:13 +0000 (16:04 +0200)]
tests: Properly reset mocked CPU model
When switching the host architecture to something for which we do not
have any host CPU model defined, the mocked
virQEMUCapsProbeHostCPUForEmulator would just return the previous CPU
model resulting in strange combinations, such as "core2duo" host CPU
model in QEMU capabilities for "AArch64" architecture. It currently
doesn't break any test case, but we should fix it anyway to avoid future
surprises which would be quite hard to debug.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
When users want to update a path to a CDROM they tend to
construct a very minimal XML and feed the API with it. This is
not a good practice as it breaks the assumptions the API is built
on. Most notably, leaving an element out should be treated as a
request for removal of the corresponding setting. Just like
leaving out <bandwidth/> clears out any QoS previously set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Michal Privoznik [Sun, 19 Aug 2018 06:26:04 +0000 (08:26 +0200)]
storage_driver: Release pool object lock for some long running jobs
As advertised in previous commit, there are three APIs that might
run for quite some time (because they read/write data from/to a
volume) and these three are: downloadVol, uploadVol, wipeVol.
Release pool object lock and reacquire it later to allow more
concurrency.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Michal Privoznik [Sat, 18 Aug 2018 16:42:07 +0000 (18:42 +0200)]
storage_driver: Mark volume as 'in use' for some operations
There are few operations in the storage driver that read/write
data onto volumes. Such operations can take very long time to
finish. During that time the storage pool object is locked which
has bad performance impacts (other threads can't fetch its XML
for instance). This commit prepares the storage driver for
releasing the lock during those operations (downloadVol,
uploadVol, wipeVol).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Michal Privoznik [Fri, 17 Aug 2018 13:07:01 +0000 (15:07 +0200)]
virstorageobj: Check for source duplicates from virStoragePoolObjAssignDef
Just like a few commits earlier, checking for pool source
duplicates and unlocking pools list afterwards is a buggy
pattern. The check must go into virStoragePoolObjAssignDef.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Michal Privoznik [Fri, 17 Aug 2018 13:14:31 +0000 (15:14 +0200)]
virStoragePoolObjSourceFindDuplicate: Drop @conn argument
The @conn argument is needed only to do some source matching in
case of iSCSI source. Anyway, it's used just for node device
driver and as such can be replaced with virGetConnectNodeDev().
At the same time, the @conn struct member is dropped from
_virStoragePoolObjFindDuplicateData.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Michal Privoznik [Fri, 17 Aug 2018 12:50:59 +0000 (14:50 +0200)]
virstorageobj: Move virStoragePoolObjSourceFindDuplicate and friends up
This function is going to be made static in used in
virStoragePoolObjAssignDef(). Therefore move it and all the
static functions it calls a few lines up.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Michal Privoznik [Fri, 17 Aug 2018 10:30:08 +0000 (12:30 +0200)]
virstorageobj: Check for duplicates from virStoragePoolObjAssignDef
Even though we do some checking it is not as thorough as it
should be. We already have virStoragePoolObjIsDuplicate but the
way we use it is a typical TOCTOU. Imagine two threads trying to
define two pools with the same name but different UUIDs. With the
current code neither of them finds a duplicate and thus proceed
to virStoragePoolObjAssignDef where only names are compared.
Therefore both threads succeed which is obviously wrong.
We should check for duplicates where we care for them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Changing MTU on a running guest is not possible and trying to do
so made us face many problems. That's why we forbid it in 5f44d7e357f61f7. However, there is still one possible path where
users can sneak in change: migration XML.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When introducing this check back in 4ad54a417a1 my mindset was
that if an element is missing in update XML then user is
requesting for removal of the corresponding setting. For
instance, if <bandwidth/> is not present in update XML any QoS
previously set on <interface/> is cleared out. Well this
assumption is correct but only to some extent.
Turns out, we have some users who when updating path to ISO
image construct very minimalistic disk XML and pass it to device
update API. Such XML is lacking a lot of information, and alias
is one of them. This triggers error in
virDomainDefCompatibleDevice() because we think that user is
requesting to remove the alias. Well, they are not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Peter Krempa [Thu, 30 Aug 2018 08:30:09 +0000 (10:30 +0200)]
qemu: Don't overwrite stats in qemuDomainBlocksStatsGather
The size/capacity stats gathered in qemuDomainBlocksStatsGather when
using -blockdev would be overwritten by assigning/copying the transfered
data statistics at the end. Fix it by moving the assignment prior to
fetching the capacity data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Farhan Ali [Thu, 30 Aug 2018 17:07:34 +0000 (13:07 -0400)]
qemu: mdev: Use vfio-pci 'display' property only with vfio-pci mdevs
S390 is aware of both vfio-pci and vfio-ccw devices, so
on S390 the capability QEMU_CAPS_VFIO_PCI_DISPLAY will be
available. Add an extra check to make sure we only set the
display to off for vfio-pci mediated devices. Otherwise we
add display for vfio-ccw device and this breaks vfio-ccw
device qemu command line.
Fixes: d54e45b6e conf: Introduce new <hostdev> attribute 'display' Signed-off-by: Farhan Ali <alifm@linux.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Luyao Huang [Mon, 20 Aug 2018 09:48:33 +0000 (17:48 +0800)]
qemu: Validate memory access during validate domain config
Commit 6534b3c4 tried to raise an error when there is no numa
nodes by setting access='shared' in the domain config, but added
a helper called from qemuDomainDeviceDefValidate instead of a
helper called from qemuDomainDefValidate for XML:
Since there are no memory devices in the test XML, there would
be no validation failure, but the test added was still failing.
Investigating that it turns out that unnecessary XML elements
were causing the failure (no need for <video>, <graphics>,
<pm>, usb controller model "piix3-uhci", disk attribute for
"discard='unmap'", <serial>, <console>, <channel> and a
memballoon model). Removing all those before moving the method
caused the test to succeed.
So this patch moves the validation to the right place and
removes all the unnecessary XML pieces that were causing
a false validation failure.
Simon Kobyda [Wed, 29 Aug 2018 07:48:10 +0000 (09:48 +0200)]
vsh-table: Fix broken build on centos and rhel
The reason of broken build was that centos and rhel use older version of
glibc. These versions of glibc on these platforms cannot work with newer
unicodes, thus causing functions iswprint() and wcwidth() return
unexpected values causing the vshtabletest to fail. Therefore, let's
replace the new unicode characters causing issues with some older ones
to fix the test suite, as the issue would still persist during runtime.
Signed-off-by: Simon Kobyda <skobyda@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Marc Hartmayer [Mon, 27 Aug 2018 13:20:13 +0000 (15:20 +0200)]
virDomainObjListAddLocked: fix double free
If @vm has flagged as "to be removed" virDomainObjListFindByNameLocked
returns NULL (although the definition actually exists). Therefore, the
possibility exits that "virHashAddEntry" will raise the error
"Duplicate key" => virDomainObjListAddObjLocked fails =>
virDomainObjEndAPI(&vm) is called and this leads to a freeing of @def
since @def is already assigned to vm->def. But actually this leads to
a double free since the common usage pattern is that the caller of
virDomainObjListAdd(Locked) is responsible for freeing @def in case of
an error.
Let's fix this by setting vm->def to NULL in case of an error.
Backtrace:
➤ bt
#0 virFree (ptrptr=0x7575757575757575)
#1 0x000003ffb5b25b3e in virDomainResourceDefFree
#2 0x000003ffb5b37c34 in virDomainDefFree
#3 0x000003ff9123f734 in qemuDomainDefineXMLFlags
#4 0x000003ff9123f7f4 in qemuDomainDefineXML
#5 0x000003ffb5cd2c84 in virDomainDefineXML
#6 0x000000011745aa82 in remoteDispatchDomainDefineXML
...
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Andrea Bolognani [Tue, 28 Aug 2018 15:40:27 +0000 (17:40 +0200)]
qemu: Add more defaults for RISC-V virt guests
We would have used virtio for networking anyway, but it's
better to be explicit; for graphics, none of the existing
models work right now but virtio is the only one which
has a non-PCI variant, so it's as good a default as any
Spotted-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Mon, 27 Aug 2018 08:25:17 +0000 (10:25 +0200)]
qemu: Introduce 16550A serial console model
None of the existing models is suitable for use with
RISC-V virt guests, and we don't want information about
the serial console to be missing from the XML.
The name is based on comments in qemu/hw/riscv/virt.c:
RISC-V machine with 16550a UART and VirtIO MMIO
and in qemu/hw/char/serial.c:
QEMU 16550A UART emulation
along with the output of dmesg in the guest:
Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled 10000000.uart: ttyS0 at MMIO 0x10000000 (irq = 13,
base_baud= 230400) is a 16550A
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 16 Aug 2018 14:19:22 +0000 (16:19 +0200)]
util: json: Allow converting a virTristate(Bool|Switch) into JSON
Add a new modifier letter for virJSONValueObjectAddVArgs which will add
a boolean value with our tristate semantics. The value is omitted when
the _ABSENT value is used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
The x86 and ppc impls both duplicate some logic when parsing CPU
features. Change the callback signature so that this duplication can be
pushed up a level to common code.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Two pieces of code accidentally jumped to the wrong label when they
failed causing incorrect cleanup, returning a partially initialized
CPU model struct.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Ján Tomko [Tue, 28 Aug 2018 10:53:25 +0000 (12:53 +0200)]
qemu: initialize variables in qemuParseCommandLine
Commit 6700062 introduced a jump to error which skipped the
initialization of def:
qemu/qemu_parse_command.c:1870:9: error: variable 'def' is
used uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
if (!(qemuCaps = virQEMUCapsCacheLookup(capsCache, progargv[0])))
Initialize def to fix this warning and qemuCaps, to prevent
a future error like this.
qemu: fix default machine for argv -> xml convertor
Historically the argv -> xml convertor wanted the same default machine
as we'd set when parsing xml. The latter has now changed, however, to
use a default defined by libvirt. The former needs fixing to again
honour the default QEMU machine.
This exposed a bug in handling for the aarch64 target, as QEMU does not
define any default machine. Thus we should not having been accepting
argv without a -machine provided.
Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
qemu: rename method for getting preferred machine type
The virQEMUCapsGetDefaultMachine() method doesn't get QEMU's default
machine any more, instead it gets the historical default that libvirt
prefers for each arch. Rename it, so that the old name can be used for
getting QEMU's default.
Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
qemu: record the QEMU default machine in capabilities
We don't honour the QEMU default machine type anymore, always using the
libvirt chosen default instead. The QEMU argv parser, however, will need
to know the exacty QEMU default, so we must record that info.
Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Mon, 27 Aug 2018 16:06:20 +0000 (18:06 +0200)]
tests: qemuxml2argv: Remove 'args' for tests only used in xml2xmltest
'metadata' and 'leases' are features internal to libvirt and thus don't
influence the generated QEMU command line. As they are not tested we
don't need the output files.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Peter Krempa [Mon, 27 Aug 2018 15:59:26 +0000 (17:59 +0200)]
tests: qemuxml2argv: Make use of 'vram64' QXL device tests
The test files were unused, but we don't have any other test for this
feature. Make use of the existing files by removing disks and using
DO_TEST_CAPS_LATEST to execute them. The legacy output files will be
dropped.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Katerina Koukiou [Fri, 24 Aug 2018 10:28:41 +0000 (12:28 +0200)]
qemu: qemuDomainChangeNet: validity checks should be done before XML autocompletion
This patch ensures that changes in attributes of interfaces will emit
errors except if they are missing from the XML.
Previously we were falsely reporting successful updates, because some
changed attributes got overwritten before the validity checks.
QEMU is probed through monitor fd to check capabilities during libvirtd init.
The monitor fd is closed after probing by virQEMUCapsInitQMPCommandFree
that calls virQEMUCapsInitQMPCommandAbort that calls qemuMonitorClose,
the latter one notifies the event loop via an interrupt handle in
qemuMonitorUnregister and after then closes monitor fd.
There could be a case when interrupt is sent after eventLoop is unlocked
but before virEventPollRunOnce blocks in poll, shortly before file
descriptor is closed by qemuMonitorClose. Then poll receives closed monitor
fd in fdset and returns EBADF.
EBADF is not mentioned as a valid errno on macOS poll man-page but such
behaviour can appear release-to-release, according to cpython:
https://github.com/python/cpython/blob/master/Modules/selectmodule.c#L1161
The change also fixes the issue in qemucapabilitiestest. It returns
Bad file descriptor message 25 times without the fix.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 23 Aug 2018 08:21:03 +0000 (10:21 +0200)]
docs: api_extension: Remove links to the stale example patches
The patches used as an example for the api_extension manual don't hold
up to the current standards any more. Carefully remove links and
mentions of the patches from the docs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
projects / libvirt.git / log