We have a check for backslash alignment that checks for two blanks preceding a
backslash. However there can be alignment done using a tabulator and in some
cases one might be enough. There are none currently, but I found out that was
the case before. at some point. so let's check for \t preceding the final
backslash as well.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Peter Krempa [Wed, 1 Nov 2017 15:31:55 +0000 (16:31 +0100)]
qemu: command: Split out geometry frontend attribute formatting from -drive
Historically we've formatted a lot of the attributes of a disk (disk
geometry, etc) with -drive. Since we use -device now, they should be
formatted there. Extract them to a separate function for keeping
compatibility with SDcards which still use only -drive.
Start this by moving the geometry into a separate function.
Peter Krempa [Wed, 22 Nov 2017 15:42:53 +0000 (16:42 +0100)]
qemu: domain: Don't call namespace setup for storage already accessed by vm
When doing block commit we need to allow write for members of the
backing chain so that we can commit the data into them.
qemuDomainDiskChainElementPrepare was used for this which since commit 786d8d91b4 calls qemuDomainNamespaceSetupDisk which has very adverse
side-effects, namely it relabels the nodes to the same label it has in
the main namespace. This was messing up permissions for the commit
operation since its touching various parts of a single backing chain.
Since we are are actually not introducing new images at that point add a
flag for qemuDomainDiskChainElementPrepare which will refrain from
calling to the namespace setup function.
Calls from qemuDomainSnapshotCreateSingleDiskActive and
qemuDomainBlockCopyCommon do introduce new members all calls from
qemuDomainBlockCommit do not, so the calls are anotated accordingly.
Since we don't pack symlinks we cannot have recursive loops in them. Since we
need one directory to be in tests/vircaps2xmldata/linux-caches/, instead of
creating a symlink, just move the files in that directory and adjust tests.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Thu, 16 Nov 2017 13:35:12 +0000 (14:35 +0100)]
tools: Enable warnings for more binaries/libs
Because WARN_CFLAGS and COVERAGE_CFLAGS are not set globally, we
rely on each binary built to include WARN_CFLAGS/COVERAGE_CFLAGS.
But it is easy to forget those - e.g. libvirt_shell.la. However,
don't enable WARN_FLAGS (i.e. don't include AM_CFLAGS) for
wireshark plugin - parts of that code are generated and trigger
some warnings.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Michal Privoznik [Thu, 16 Nov 2017 12:38:27 +0000 (13:38 +0100)]
vsh: Make self-test more robust
There are couple of limitations when it comes to option types and
flags for the options. For instance, VSH_OT_STRING cannot have
VSH_OFLAG_REQ set (commit c7543a728). For some reason this is
checked in vshCmddefHelp() but not in vshCmddefCheckInternals().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Just like in 9324f67a572f9b32 we need to put default sata alias
(which is hardcoded to "ide", obvious, right?) onto the command
line instead of the one provided by user.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The compiler can warn us if we add a value to the
virDomainChrSerialTargetType enumeration but forget to handle
it properly in the code. Let's take advantage of that.
This commit is best viewed with 'git diff -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
John Ferlan [Mon, 6 Nov 2017 20:20:55 +0000 (15:20 -0500)]
libvirtd: Fix order of cleanup processing
Current cleanup processing is ad-hoc at best - it's led to a couple of
strange and hard to diagnose timing problems and crashes.
So rather than perform cleanup in a somewhat random order, let's
perform cleanup in the exact opposite order of startup.
NB: It is possible that virNetlinkEventServerStart fails and we jump
to cleanup before driversInitialized has been set. That could leave
things inconsistent; however, resolution of that possibility is perhaps
more trouble than it's worth to handle.
John Ferlan [Mon, 6 Nov 2017 21:55:15 +0000 (16:55 -0500)]
libvirtd: Alter order of virNetDaemonNew
Let's be sure we can get a Daemon object before the server object.
This is a more "orderly" way to do things since the svr object would
be added to the dmn object afterwards.
Starting from qemu 2.11, the `-device vmcoreinfo` will create a fw_cfg
entry for a guest to store dump details, necessary to process kernel
dump with KASLR enabled and providing additional kernel details.
In essence, it is similar to -fw_cfg name=etc/vmcoreinfo,file=X but in
this case it is not backed by a file, but collected by QEMU itself.
Since the device is a singleton and shouldn't use additional hardware
resources, it is presented as a <feature> element in the libvirt
domain XML.
The device is arm/x86 only for now (targets that support fw_cfg+dma).
Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=1395248
tests: Add resctrl-skx-twocaches test case to vircaps2xmltest
This doesn't add very much value for now, but future test for virresctrl will
take information from vircaps2xmldata (since it is dependent on the same info
then why duplicate it) and this particular use case helps us cover bit more of
the code regarding proper formatting and handling errors. And one more test for
vircaps2xmltest doesn't hurt either.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
This patch modifies some not yet used test data so that the adding a test using
this data is a clean patch and not an addition of huge file with some
adjustments in small files that will be hidden in the middle of that commit.
These changes include:
- Add system dir in vircaps2xmldata/linux-caches
Back when data for systems with resctrl support were added they had the
/sys/fs/system directory put into a system/ subdir of the test and
/sys/fs/resctrl in a resctrl/ subdir of that test. However, if we also want a
negative test for the resctrl (requesting allocation on a system that does not
support resctrl), we need one a test case with any sensible (with cache info)
system/ subdir and no resctrl/ one. Easiest way is to add a
system -> . symlink into existing test case.
- Change default group schemata for linux-resctrl and linux-resctrl-cdp
That way we can fit some allocation in.
- Remove one cache from resctrl-skx's schemata and make some room for
allocations
That system already has only one cache, so that file was wrong anyway. We
have a version with 2 caches already (linux-resctrl-skx-twocaches), so this
will also add variety to future tests.
- Add some empty allocation for resctrl-skx
Just to have slightly more coverage and variety. We can be sure nothing bad
happens if such allocation exists in case we have that in the tests.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Because the cache banks are initialized based on the order in which their
respective directories exist on the filesystem, they can appear in different
order. This is here mainly for tests because the cache directory might have
different order of children nodes and tests would fail otherwise. It should not
be the case with sysfs, but one can never be sure. And this does not take
almost any extra time, mainly because it gets initialized once per driver.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Sometimes the size of the bitmap matters and it might not be guessed correctly
when parsing from some type of input. For example virBitmapNewData() has Byte
granularity, virBitmapNewString() has nibble granularity and so on.
virBitmapParseUnlimited() can be tricked into creating huge bitmap that's not
needed (e.g.: "0-2,^99999999"). This function provides a way to shrink the
bitmap. It is not supposed to free any memory.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Our bitmaps can be represented as data (raw bytes for which we have
virBitmapNewData() and virBitmapToData()), human representation (list
of numbers in a string for which we have virBitmapParse() and
virBitmapFormat()) and hexadecimal string (for which we have only
virBitmapToString()). So let's add the missing complement for the
last one so that we can parse hexadecimal strings.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
util: Don't output too many zeros from virBitmapToString
Truncate the output so that it is only as big as is needed to fit all
the bits, not all the units from the map. This will be needed in the
future in order to properly format bitmaps for kernel's sysfs files.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
util: Rename virBitmapDataToString to virBitmapDataFormat
It is literally only a wrapper around virBitmapNewData() and
virBitmapFormat(), only the naming was wrong since it was introduced.
And because we have virBitmap*String functions where the meaning of
the 'String' is constant, this might confuse someone.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
This follows the virBitmapToData() function and, similarly to
virBitmapNewData(), we'll be able to have virBitmapNewString() later
on without name confusion.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
We can't output better memory sizes if we want to be compatible with libvirt
older than the one which introduced /memory/unit, but for new things we can just
output nicer capacity to the user if available. And this function enables that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Only output initialized capabilities with VIR_TEST_DEBUG > 1
Currenty virTestInit() outputs all capabilities that it created when running
with VIR_TEST_DEBUG=1. Since this is quite a lot of output for every call of
this function (and it is not needed until debugging a really deep-down issue)
let's just output the info when VIR_TEST_DEBUG is strictly greater than 1.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Since update to glibc-2.26 removed the /usr/include/rpc/rpc.h we used until now,
it showed us a problem with not using XDR_CFLAGS properly. On linux that
variable has usually -I/usr/include/tirpc because we already probe for it
properly, we just don't use it everywhere we need. It is needed by wireshark
dissector as well as testutilsqemu.c (through includes) so the build fails with:
wireshark/src/packet-libvirt.c:33:10: fatal error: rpc/xdr.h: No such file or directory
#include <rpc/xdr.h>
^~~~~~~~~~~
and
In file included from ../src/logging/log_manager.h:29:0,
from ../src/qemu/qemu_domain.h:40,
from testutilsqemu.c:11:
../src/logging/log_protocol.h:9:10: fatal error: rpc/rpc.h: No such file or directory
#include <rpc/rpc.h>
^~~~~~~~~~~
Since lot of tests use testutilsqemu.c it is easier to add XDR_CFLAGS to
AM_CFLAGS than adding it to all $binary_CFLAGS. It's just for tests and we
already have bunch of CFLAGS there anyway.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
docs: add a page describing support guarantees for libvirt features
While we have collective knowledge about the support status of various
parts of libvirt, this has never been formally documented, leaving our
users to guess.
Note, this document makes one change to our previous policy. It explicitly
declares the RPC protocol of libvirtd as being a supported interface. THis
accepts the reality that we can a) never change it without breaking compat
with old libvirt.so, b) there are both rust + go impls that are written
against the RPC protocol already.
Reviewed-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
John Ferlan [Mon, 6 Nov 2017 20:22:07 +0000 (15:22 -0500)]
storage: Resolve storage driver crash
Resolve a storage driver crash as a result of a long running
storageVolCreateXML when the virStorageVolPoolRefreshThread is
run as a result of when a storageVolUpload completed and ran the
virStoragePoolObjClearVols without checking if the creation
code was currently processing a buildVol after incrementing
the driver->asyncjob count.
The refreshThread will now check the pool asyncjob count before
attempting to pursue the pool refresh. Adjust the documentation
to describe the condition.
Crash from valgrind is as follows (with a bit of editing):
==21309== Invalid read of size 8
==21309== at 0x153E47AF: storageBackendUpdateVolTargetInfo
==21309== by 0x153E4C30: virStorageBackendUpdateVolInfo
==21309== by 0x153E52DE: virStorageBackendVolRefreshLocal
==21309== by 0x153DE29E: storageVolCreateXML
==21309== by 0x562035B: virStorageVolCreateXML
==21309== by 0x147366: remoteDispatchStorageVolCreateXML
...
==21309== Address 0x2590a720 is 64 bytes inside a block of size 336 free'd
==21309== at 0x4C2F2BB: free
==21309== by 0x54CB9FA: virFree
==21309== by 0x55BC800: virStorageVolDefFree
==21309== by 0x55BF1D8: virStoragePoolObjClearVols
==21309== by 0x153D967E: virStorageVolPoolRefreshThread
...
==21309== Block was alloc'd at
==21309== at 0x4C300A5: calloc
==21309== by 0x54CB483: virAlloc
==21309== by 0x55BDC1F: virStorageVolDefParseXML
==21309== by 0x55BDC1F: virStorageVolDefParseNode
==21309== by 0x55BE5A4: virStorageVolDefParse
==21309== by 0x153DDFF1: storageVolCreateXML
==21309== by 0x562035B: virStorageVolCreateXML
==21309== by 0x147366: remoteDispatchStorageVolCreateXML
...
Julio Faracco [Thu, 16 Nov 2017 03:01:54 +0000 (01:01 -0200)]
virsh: fixing wrong datatype of 'set-lifcycle-action' command options.
The 'set-lifcycle-action' is throwing a weird error after executing it with
the '--help' option. The command output is showing the options 'type' and
'action' are as optional, but they aren't. Both are required.
virsh # set-lifecycle-action --help
...
SYNOPSIS
set-lifecycle-action <domain> [--type <string>] [--action <string>] ...
...
OPTIONS
[--domain] <string> domain name, id or uuid
error: internal error: bad options in command: 'set-lifecycle-action'
After applying this patch, both arguments are required now.
Erik Skultety [Tue, 14 Nov 2017 13:32:16 +0000 (14:32 +0100)]
daemon: virtlogd: Drop the server shortcut ref pointer
We put the server into a hash table as we do with the other daemons,
there is no compelling reason why it should have another pointer
dedicated just to the server. Besides, the locking daemon doesn't have
it and virtlogd is essentially a copy paste of virtlockd.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
qemu: Enable configuration of HPT resizing for pSeries guests
Most of the time it's okay to leave this up to negotiation between
the guest and the host, but in some situations it can be useful to
manually decide the behavior, especially to enforce its availability.
Jiri Denemark [Mon, 13 Nov 2017 21:22:07 +0000 (22:22 +0100)]
vircapstest: Avoid (im)possible strcmp call with NULL argument
Some compilers may get confused and decide we are calling strcmp with
NULL argument from test_virCapsDomainDataLookupLXC. Although this does
not really happen since the call is guarded with
(data->machinetype != expect_machinetype), using STRNEQ_NULLABLE is
easier to understand, less fragile, and doing so makes sure strcmp is
never called with NULL argument.
Jim Fehlig [Fri, 10 Nov 2017 22:01:00 +0000 (15:01 -0700)]
news: add entries for specifying distance between vNUMA cells
Add two new entries under new features for 3.10.0. One
advertising support for specifying distance between vNUMA cells
and another advertising Xen's support for vNUMA configuration.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Peter Krempa [Thu, 9 Nov 2017 09:29:53 +0000 (10:29 +0100)]
qemu: block: Don't leak server JSON object from protocol generators
If creation of the main JSON object containing the storage portion of a
virStorageSource would fail but we'd allocate the server structure we'd
leak it. Found by coverity.
Peter Krempa [Thu, 9 Nov 2017 09:13:26 +0000 (10:13 +0100)]
qemu: block: Break out early on invalid storage sources
Return NULL right away in qemuBlockStorageSourceGetBackendProps when an
invalid storage source is presented so that virJSONValueObjectAdd isn't
called with a NULL argument.
Peter Krempa [Wed, 8 Nov 2017 14:42:51 +0000 (15:42 +0100)]
conf: Properly parse <backingStore/>
The terminator would not be parsed properly since the XPath selector was
looking for an populated element, and also the code did not bother
assigning the terminating virStorageSourcePtr to the backingStore
property of the parent.
Some tests would catch it if there wasn't bigger fallout from the change
to backing store termination in a693fdba0111. Fix them properly now.
Wim ten Have [Thu, 2 Nov 2017 15:47:22 +0000 (16:47 +0100)]
libxl: vnuma support
This patch generates a NUMA distance-aware libxl description from the
information extracted from a NUMA distance-aware libvirt XML file.
By default, if no NUMA node distance information is supplied in the
libvirt XML file, this patch uses the distances 10 for local and 20
for remote nodes/sockets.
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
If there is no XML <distances> description amongst the <cell> data the
conversion schema from xml to native will generate 10 for local and 20
for all remote instances.
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
A <cell> defines a NUMA node. <distances> describes the NUMA distance
from the <cell> to the other NUMA nodes (the <sibling>s). For example,
in above XML description, the distance between NUMA node0 <cell id='0'
...> and NUMA node2 <sibling id='2' ...> is 31.
Valid distance values are '10 <= value <= 255'. A distance value of 10
represents the distance to the node itself. A distance value of 20
represents the default value for remote nodes but other values are
possible depending on the physical topology of the system.
When distances are not fully described, any missing sibling distance
values will default to 10 for local nodes and 20 for remote nodes.
If distance is given for A -> B, then we default B -> A to the same
value instead of 20.
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
When testing user aliases it was discovered that for 440fx
machine type which has default IDE bus builtin, domain cannot
start if IDE controller has the user provided alias. This is
because for 440fx we don't put the IDE controller onto the
command line (since it is builtin) and therefore any device that
is plugged onto the bus must use the default alias.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
numa: avoid failure in nodememstats on non-NUMA systems
libvirt reports a fake NUMA topology in virConnectGetCapabilities
even if built without numactl support. The fake NUMA topology consists
of a single cell representing the host's cpu and memory resources.
Currently this is the case for ARM and s390[x] RPM builds.
A client iterating over NUMA cells obtained via virConnectGetCapabilities
and invoking virNodeGetMemoryStats on them will see an internal failure
"NUMA isn't available on this host" from virNumaGetMaxNode. An example
for such a client is VDSM.
Since the intention seems to be that libvirt always reports at least
a single cell it is necessary to return "fake" node memory statistics
matching the previously reported fake cell in case NUMA isn't supported
on the system.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Dawid Zamirski [Tue, 7 Nov 2017 22:36:34 +0000 (17:36 -0500)]
vbox: Add support for 5.2.x
Simply add the 5.2 SDK header to the existing unified framework. No
other special handling is needed as there's no API break between
existing 5.1 and the just added 5.2.
Commit 3cc2a9e0 fixed a similar problem when parsing content of a
file but missed parsing in-memory content. But AFAICT, the better
fix is to properly set the end of the content when initializing the
virConfParserCtxt in virConfParse().
This commit reverts the part of 3cc2a9e0 that appends a newline to
files missing it, and fixes setting the end of content when
initializing virConfParserCtxt. A test is also added to check
parsing in-memory content missing an ending newline.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
qemu-ns: Detect /dev/* mount point duplicates even better
In 4f1570720218302 I've tried to make duplicates detection for
nested /dev mount better. However, I've missed the obvious case
when there are two same mount points. For instance if:
# mount --bind /dev/blah /dev/blah
# mount --bind /dev/blah /dev/blah
Yeah, very unlikely (in qemu driver world) but possible.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Tue, 7 Nov 2017 15:20:23 +0000 (16:20 +0100)]
util: storage: Fix parsing of IPv6 portal address for iSCSI
Split on the last colon and avoid parsing port if the split remainder
contains the closing square bracket, so that IPv6 addresses are
interpreted correctly.
qemu: Use predictable file names for memory-backend-file
In some cases management application needs to allocate memory for
qemu upfront and then just let qemu use that. Since we don't want
to expose path for memory-backend-file anywhere in the domain
XML, we can generate predictable paths. In this case:
$memoryBackingDir/libvirt/qemu/$shortName/$alias
where $shortName is result of virDomainDefGetShortName().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
When removing path where huge pages are call virFileDeleteTree
instead of plain rmdir(). The reason is that in the near future
there's going to be more in the path than just files - some
subdirs. Therefore plain rmdir() is not going to be enough.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
qemu: Set alias for memory cell in qemuBuildMemoryCellBackendStr
Very soon qemuBuildMemoryBackendStr() is going to use memory cell
aliases. Therefore set one. At the same time, move it a bit
further - if virAsprintf() fails, there's no point in setting
rest of the members.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Dawid Zamirski [Tue, 7 Nov 2017 21:36:47 +0000 (16:36 -0500)]
docs: Update vbox driver documentation.
* libvirt no longer supports vbox <= 3.x
* update XML definition sample to show how to attach disks to VBOX's SAS
controller and how to change IDE controller model.
* update XML to show how to create RDP display with autoport.