In general, we try to make virt-xml-validate tolerant of input
elements in any order when possible. However, as written, the
RNG grammar did not permit <source> unless there was an explicit
type= attribute (even though the C code manages just fine by
defaulting to type='file'). After making the attribute optional
on the 'file' branch, I noticed that the use of diskspec was now
redundant with the branch when no <source> was supplied.
View this patch with 'git diff -b' for a better picture of the
schema change.
* docs/schemas/domaincommon.rng (disk): Hoist 'diskspec' out of
choice, make type='file' default, and still preserve interleave.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-source-pool.xml:
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-discard.xml:
New files.
* tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml:
* tests/qemuxml2argvdata/qemuxml2argv-disk-drive-discard.xml:
Reorder XML.
* tests/qemuxml2xmltest.c (mymain): Cover new files.
Signed-off-by: Eric Blake <eblake@redhat.com>
Another field no longer needed, getting us one step closer to
merging virStorageFileMetadata and virStorageSource.
* src/util/virstoragefile.h (_virStorageFileMetadata): Drop
field.
* src/util/virstoragefile.c (virStorageFileGetMetadataInternal)
(virStorageFileGetMetadataFromFDInternal): Alter signature.
(virStorageFileFreeMetadata, virStorageFileGetMetadataFromBuf)
(virStorageFileGetMetadataFromFD): Adjust clients.
* tests/virstoragetest.c (_testFileData, testStorageChain)
(mymain): Simplify test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Thanks to the testsuite, I feel quite confident that this rewrite
is correct; it gives the same results for all cases except for one.
I can make the argument that _that_ case was a pre-existing bug:
when looking up relative names, the lookup is supposed to be
pegged to the directory that contains the parent qcow2 file. Thus,
this resolves the fixme first mentioned in commit 367cd69 (even
though I accidentally removed the fixme comment early in 74430fe).
* src/util/virstoragefile.c (virStorageFileChainLookup): Depend on
new rather than old fields.
* tests/virstoragetest.c (mymain): Adjust test to match fix.
Signed-off-by: Eric Blake <eblake@redhat.com>
The original chain lookup code had to pass in the starting name,
because it was not available in the chain. But now that we have
added fields to the struct, this parameter is redundant.
* src/util/virstoragefile.h (virStorageFileChainLookup): Alter
signature.
* src/util/virstoragefile.c (virStorageFileChainLookup): Adjust
handling of top of chain.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Adjust caller.
* tests/virstoragetest.c (testStorageLookup, mymain): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
I realized that we had no good test coverage of looking up a
name from within a backing chain, even though code like
block-commit is relying on it.
* tests/virstoragetest.c (testStorageLookup): New function.
(mymain): New tests.
Signed-off-by: Eric Blake <eblake@redhat.com>
Noticed while tweaking the RelaxNG grammar for <disk> elements.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-numad-static-vcpu-no-numatune.xml:
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-cdrom-empty.xml:
Drop unused files.
* tests/qemuargv2xmltest.c (testInfo, DO_TEST_FULL): Drop unused
field.
Signed-off-by: Eric Blake <eblake@redhat.com>
Drop another redundant field from virStorageFileMetadata.
* src/util/virstoragefile.h (_virStorageFileMetadata): Drop
field.
* src/util/virstoragefile.c
(virStorageFileGetMetadataFromFDInternal)
(virStorageFileGetMetadataFromFD)
(virStorageFileGetMetadataRecurse): Adjust callers.
* tests/virstoragetest.c (_testFileData, testStorageChain)
(mymain): Simplify test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Finally starting to prune away some of the old fields that have
been made redundant by the new fields, on my way towards directly
reusing virStorageSource.
* src/util/virstoragefile.h (_virStorageFileMetadata): Drop
field.
* src/util/virstoragefile.c (virStorageFileGetMetadataInternal)
(virStorageFileChainLookup): Adjust callers.
* tests/virstoragetest.c (_testFileData, testStorageChain)
(mymain): Simplify test.
Signed-off-by: Eric Blake <eblake@redhat.com>
So far, my work has been merely preserving the status quo of
backing file analysis. But this patch starts to tread in the
territory of making the backing chain code more powerful - we
will eventually support network storage containing non-raw
formats. Here, we expose metadata information about a network
backing store, even if that information is still hardcoded to
a raw format for now.
* src/util/virstoragefile.c (virStorageFileGetMetadataRecurse):
Also populate struct for non-file backing.
(virStorageFileGetMetadata, virStorageFileGetMetadatainternal):
Recognize non-file top image.
(virFindBackingFile): Add comment.
(virStorageFileChainGetBroken): Adjust comment, ensure output
is set.
* tests/virstoragetest.c (mymain): Update test to reflect it.
Signed-off-by: Eric Blake <eblake@redhat.com>
I'm tired of alternating between test failures due to bugs in
my refactoring work, vs. test failures due to leftovers in
the file system from the previous test. This patch has no
impact when the testsuite is successful, but doeesn't hurt either.
* tests/virstoragetest.c (testPrepImages): Clean up from prior
failed test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Every test that makes use of virmock.h (only virsystemdtest as of now)
needs to be linked with -export-dynamic to make sure the LD_PRELOADed
mock library can access its wrap_* symbols. Normally,
DRIVER_MODULE_LDFLAGS variable contains -export-dynamic but when
--without-driver-modules configure option is used, DRIVER_MODULE_LDFLAGS
is empty.
This patch turns on -export-dynamic for all tests unconditionally
regardless on --without-driver-modules. This fixes virsystemdtest and
all future users of virmock.h.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Currently, virCgroupGetPercpuStats is only used by the LXC driver,
filling out the CPUTIME stats. qemuDomainGetPercpuStats does this
and also filles out VCPUTIME stats.
Extend virCgroupGetPercpuStats to also report VCPUTIME stats if
nvcpupids is non-zero. In the LXC driver, we don't have cpupids.
In the QEMU driver, there is at least one cpupid for a running domain,
so the behavior shouldn't change for QEMU either.
Also rename getSumVcpuPercpuStats to virCgroupGetPercpuVcpuSum.
Validate that all the new fields are getting set to desired values.
* tests/virstoragetest.c (_testFileData, testStorageChain): Check
for more fields.
(mymain): Populate additional fields.
Signed-off-by: Eric Blake <eblake@redhat.com>
The testsuite is absolutely essential to feeling comfortable
about swapping the backing chain structure over to a new format.
This patch tests the path settings, and demonstrates that the
correct short name is being passed to the child.
* tests/virstoragetest.c (testStorageChain): Test path.
(mymain): Update expected data.
Signed-off-by: Eric Blake <eblake@redhat.com>
A later patch will be adding some new fields to
virStorageFileMetadata; to minimize confusion, renaming the
test fields now will make it more obvious which fields are
being tested later.
* tests/virstoragetest.c (_testFileData): Alter names.
(testStorageChain, mymain): Adjust clients.
Signed-off-by: Eric Blake <eblake@redhat.com>
Introduced in commit d1e55de3.
virstoragetest.c: In function ‘testStorageChain’:
virstoragetest.c:249:10: warning: declaration of ‘abs’ shadows a global
declaration [-Wshadow]
Another reduction in the number of structs I have to modify
when I start tracking new fields in virStorageFileMetadata.
* tests/virstoragetest.c (_testFileData): Add fields.
(testStorageChain): Select between fields based on flag.
(mymain): Record both absolute and relative expectations in one
struct.
Signed-off-by: Eric Blake <eblake@redhat.com>
As I add more tests, it's getting harder to follow the split between
a struct in one place and a test using the struct in another.
Interleaving the tests makes changes more localized, and also makes
debugging easier when a test goes wrong during my refactoring work.
* tests/virstoragetest.c (mymain): Modify structs as we go, rather
than up-front.
(testStorageChain): Make failure debugging easier.
Signed-off-by: Eric Blake <eblake@redhat.com>
Part of the upcoming refactoring will change how broken chains
are detected; it makes sense to test that this works. In
particular, test the just-fixed infinite loop detection bug.
Also, make sure that detection of directories is sane.
* tests/virstoragetest.c (testStorageChain): Enhance test.
(mymain): Add more tests.
(testCleanupImages, testPrepImages): Populate a directory.
Signed-off-by: Eric Blake <eblake@redhat.com>
Since it is an abbreviation, PCI should always be fully
capitalized or full lower case, never Pci.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
A number of test suites want to mock the DBus APIs. To avoid
re-inventing the wheel create a re-usable virmockdbus.la
library for LD_PRELOAD usage.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Introduce virmock.h which provides some macros to assist in
creation of LD_PRELOAD overrides. When these are used, the
LD_PRELOAD code simply has some stubs which forward to a
wrapper function inside the main test case. This means that
logic for the test no longer has to be split between the
virXXXtest.c and virXXXmock.c files. It will also make it
possible to provide some common reusable modules for mocking
code like DBus.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
to avoid
CCLD storagevolxml2argvtest
/usr/bin/ld: ../src/.libs/libvirt_driver_storage_impl.a(libvirt_driver_storage_impl_la-storage_backend.o): undefined reference to symbol 'xmlFreeDoc@@LIBXML2_2.4.30'
//usr/lib/x86_64-linux-gnu/libxml2.so.2: error adding symbols: DSO missing from command line
I almost wrote a hash value free function that just called
VIR_FREE, then realized I couldn't be the first person to
do that. Sure enough, it was worth factoring into a common
helper routine.
* src/util/virhash.h (virHashValueFree): New function.
* src/util/virhash.c (virHashValueFree): Implement it.
* src/util/virobject.h (virObjectFreeHashData): New function.
* src/libvirt_private.syms (virhash.h, virobject.h): Export them.
* src/nwfilter/nwfilter_learnipaddr.c (virNWFilterLearnInit): Use
common function.
* src/qemu/qemu_capabilities.c (virQEMUCapsCacheNew): Likewise.
* src/qemu/qemu_command.c (qemuDomainCCWAddressSetCreate):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorGetBlockInfo): Likewise.
* src/qemu/qemu_process.c (qemuProcessWaitForMonitor): Likewise.
* src/util/virclosecallbacks.c (virCloseCallbacksNew): Likewise.
* src/util/virkeyfile.c (virKeyFileParseGroup): Likewise.
* tests/qemumonitorjsontest.c
(testQemuMonitorJSONqemuMonitorJSONGetBlockInfo): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit d7c4e0036 assumed all SELinux tests depended upon
securityselinuxhelper need xattr support, but forgot to
move viridentitytest under WITH_ATTR.
Reported-by: Nehal J Wani <nehaljw.kkd1@gmail.com>
Signed-off-by: Jincheng Miao <jmiao@redhat.com>
Up until now the traffic control filters for the vNIC QoS were
matching only ip traffic. For egress traffic that was unnoticed
because the unmatched traffic would just go to the default htb class
and be shaped anyway. For ingress, though, since the policing of the
rate is done by the filter itself.
The problem is solved by changing protocol to all and making anything
match the filter.
Bug-Url: https://bugzilla.redhat.com/1084444
Signed-off-by: Antoni S. Puimedon <asegurap@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
I'm about to add fields to virStorageFileMetadata, which means
also adding fields to the testFileData struct in virstoragetest.
Alas, adding even one pointer on an x86_64 machine gave me a
dreaded compiler error:
virstoragetest.c:712:1: error: the frame size of 4208 bytes is larger than 4096 bytes [-Werror=frame-larger-than=]
After some experimentation, I realized that each test was creating
yet another testChainData (which contains testFileData) on the stack;
forcing the reuse of one of these structures instead of creating a
fresh one each time drastically reduces the size requirements. While
at it, I also got rid of a lot of intermediate structs, with some
macro magic that lets me directly build up the destination chains
inline.
For a bit more insight into what this patch does:
The old code uses an intermediate variable as a fixed-size array
of structs:
testFileData chain[] = { a, b };
data.files = chain;
In the new code, the use of VIR_FLATTEN_* allows the TEST_CHAIN()
macro to still take a single argument for each chain, but now of
the form '(a, b)', where it is turned into the var-args 'a, b'
multiple arguments understood by TEST_ONE_CHAIN(). Thus, the
new code avoids an intermediate variable, and directly provides
the list of pointers to be assigned into array elements:
data.files = { &a, &b };
* tests/virstoragetest.c (mymain): Rewrite TEST_ONE_CHAIN to
reuse the same struct for each test, and to take the data
inline rather than via intermediate variables.
(testChainData): Use bounded array of pointers instead of
unlimited array of struct.
(testStorageChain): Reflect struct change.
Signed-off-by: Eric Blake <eblake@redhat.com>
Writing this test with C99 initializers will make it easier to test
additions and deletions to struct members as I refactor the code.
* tests/virstoragetest.c (mymain): Rewrite initializers.
Signed-off-by: Eric Blake <eblake@redhat.com>
No need to spawn a child 'rm' process when we can do it ourselves.
* tests/virstoragetest.c (testCleanupImages): Use dedicated
helper.
Signed-off-by: Eric Blake <eblake@redhat.com>
A future patch will merge virStorageFileMetadata and virStorageSource,
but I found it easier to do if both structs use the same information
for tracking whether a source file needs encryption keys.
* src/util/virstoragefile.h (_virStorageFileMetadata): Prepare
full encryption struct instead of just a bool.
* src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
Use transfer semantics.
* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Likewise.
* src/util/virstoragefile.c (virStorageFileGetMetadataInternal):
Populate struct.
(virStorageFileFreeMetadata): Adjust clients.
* tests/virstoragetest.c (testStorageChain): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
One of the features of qcow2 is that a wrapper file can have
more capacity than its backing file from the guest's perspective;
what's more, sparse files make tracking allocation of both
the active and backing file worthwhile. As such, it makes
more sense to show allocation numbers for each file in a chain,
and not just the top-level file. This sets up the fields for
the tracking, although it does not modify XML to display any
new information.
* src/util/virstoragefile.h (_virStorageSource): Add fields.
* src/conf/storage_conf.h (_virStorageVolDef): Drop redundant
fields.
* src/storage/storage_backend.c (virStorageBackendCreateBlockFrom)
(createRawFile, virStorageBackendCreateQemuImgCmd)
(virStorageBackendCreateQcowCreate): Update clients.
* src/storage/storage_driver.c (storageVolDelete)
(storageVolCreateXML, storageVolCreateXMLFrom, storageVolResize)
(storageVolWipeInternal, storageVolGetInfo): Likewise.
* src/storage/storage_backend_fs.c (virStorageBackendProbeTarget)
(virStorageBackendFileSystemRefresh)
(virStorageBackendFileSystemVolResize)
(virStorageBackendFileSystemVolRefresh): Likewise.
* src/storage/storage_backend_logical.c
(virStorageBackendLogicalMakeVol)
(virStorageBackendLogicalCreateVol): Likewise.
* src/storage/storage_backend_scsi.c
(virStorageBackendSCSINewLun): Likewise.
* src/storage/storage_backend_mpath.c
(virStorageBackendMpathNewVol): Likewise.
* src/storage/storage_backend_rbd.c
(volStorageBackendRBDRefreshVolInfo)
(virStorageBackendRBDCreateImage): Likewise.
* src/storage/storage_backend_disk.c
(virStorageBackendDiskMakeDataVol)
(virStorageBackendDiskCreateVol): Likewise.
* src/storage/storage_backend_sheepdog.c
(virStorageBackendSheepdogBuildVol)
(virStorageBackendSheepdogParseVdiList): Likewise.
* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Likewise.
* src/conf/storage_conf.c (virStorageVolDefFormat)
(virStorageVolDefParseXML): Likewise.
* src/test/test_driver.c (testOpenVolumesForPool)
(testStorageVolCreateXML, testStorageVolCreateXMLFrom)
(testStorageVolDelete, testStorageVolGetInfo): Likewise.
* src/esx/esx_storage_backend_iscsi.c (esxStorageVolGetXMLDesc):
Likewise.
* src/esx/esx_storage_backend_vmfs.c (esxStorageVolGetXMLDesc)
(esxStorageVolCreateXML): Likewise.
* src/parallels/parallels_driver.c (parallelsAddHddByVolume):
Likewise.
* src/parallels/parallels_storage.c (parallelsDiskDescParseNode)
(parallelsStorageVolDefineXML, parallelsStorageVolCreateXMLFrom)
(parallelsStorageVolDefRemove, parallelsStorageVolGetInfo):
Likewise.
* src/vbox/vbox_tmpl.c (vboxStorageVolCreateXML)
(vboxStorageVolGetXMLDesc): Likewise.
* tests/storagebackendsheepdogtest.c (test_vdi_list_parser):
Likewise.
* src/phyp/phyp_driver.c (phypStorageVolCreateXML): Likewise.
While running virdbustest, it was found that valgrind pointed out
the following memory leaks:
==9996== 17 (8 direct, 9 indirect) bytes in 1 blocks are definitely lost in loss record 9 of 36
==9996== at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==9996== by 0x4A06B62: realloc (vg_replace_malloc.c:662)
==9996== by 0x4C6B587: virReallocN (viralloc.c:245)
==9996== by 0x4C6B6AE: virExpandN (viralloc.c:294)
==9996== by 0x4C82B54: virDBusMessageDecodeArgs (virdbus.c:907)
==9996== by 0x4C83463: virDBusMessageDecode (virdbus.c:1141)
==9996== by 0x402C45: testMessageArrayRef (virdbustest.c:273)
==9996== by 0x404E71: virtTestRun (testutils.c:201)
==9996== by 0x401C2D: mymain (virdbustest.c:479)
==9996== by 0x4055ED: virtTestMain (testutils.c:789)
==9996== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==9996==
==9996== 28 (16 direct, 12 indirect) bytes in 1 blocks are definitely lost in loss record 12 of 36
==9996== at 0x4A06BE0: realloc (vg_replace_malloc.c:662)
==9996== by 0x4C6B587: virReallocN (viralloc.c:245)
==9996== by 0x4C6B6AE: virExpandN (viralloc.c:294)
==9996== by 0x4C82B54: virDBusMessageDecodeArgs (virdbus.c:907)
==9996== by 0x4C83463: virDBusMessageDecode (virdbus.c:1141)
==9996== by 0x402C45: testMessageArrayRef (virdbustest.c:273)
==9996== by 0x404E71: virtTestRun (testutils.c:201)
==9996== by 0x401C2D: mymain (virdbustest.c:479)
==9996== by 0x4055ED: virtTestMain (testutils.c:789)
==9996== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==9996==
Signed-off-by: Eric Blake <eblake@redhat.com>
Windows doesn't allow : in filenames.
Commit 6fdece9a33 added files with a : in
their names. This broke git operations on Windows as git is not able to
create those files on clone or pull.
Replace : with - in the offending filenames and adapt the test case.
As the tested Linux specific code expects the files to exist with : in
their path use symlinks to provide the name that way.
While running qemucaps2xmltest, it was found that valgrind pointed out
the following memory leaks:
==27045== 160 (112 direct, 48 indirect) bytes in 1 blocks are definitely lost in loss record 51 of 65
==27045== at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==27045== by 0x4C6BACD: virAllocVar (viralloc.c:560)
==27045== by 0x4CAF095: virObjectNew (virobject.c:193)
==27045== by 0x421453: virQEMUCapsNew (qemu_capabilities.c:1805)
==27045== by 0x41F04F: testQemuCapsXML (qemucaps2xmltest.c:72)
==27045== by 0x41FFD1: virtTestRun (testutils.c:201)
==27045== by 0x41EE7A: mymain (qemucaps2xmltest.c:203)
==27045== by 0x42074D: virtTestMain (testutils.c:789)
==27045== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==27045==
==27045== 160 (112 direct, 48 indirect) bytes in 1 blocks are definitely lost in loss record 52 of 65
==27045== at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==27045== by 0x4C6BACD: virAllocVar (viralloc.c:560)
==27045== by 0x4CAF095: virObjectNew (virobject.c:193)
==27045== by 0x421453: virQEMUCapsNew (qemu_capabilities.c:1805)
==27045== by 0x41F04F: testQemuCapsXML (qemucaps2xmltest.c:72)
==27045== by 0x41FFD1: virtTestRun (testutils.c:201)
==27045== by 0x41EEA3: mymain (qemucaps2xmltest.c:204)
==27045== by 0x42074D: virtTestMain (testutils.c:789)
==27045== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
At this point unittest covers 4 basic cases:
- minimal working XML for bhyve
- same as above, but with virtio disk
- ACPI and APIC args test
- MAC address test
A patch submitted by Steven Malin last week pointed out a problem with
libvirt's DNS SRV record configuration:
https://www.redhat.com/archives/libvir-list/2014-March/msg00536.html
When searching for that message later, I found another series that had
been posted by Guannan Ren back in 2012 that somehow slipped between
the cracks:
https://www.redhat.com/archives/libvir-list/2012-July/msg00236.html
That patch was very much out of date, but also pointed out some real
problems.
This patch fixes all the noted problems by refactoring
virNetworkDNSSrvDefParseXML() and networkDnsmasqConfContents(), then
verifies those fixes by added several new records to the test case.
Problems fixed:
* both service and protocol now have an underscore ("_") prepended on
the commandline, as required by RFC2782.
<srv service='sip' protocol='udp' domain='example.com'
target='tests.example.com' port='5060' priority='10'
weight='150'/>
before: srv-host=sip.udp.example.com,tests.example.com,5060,10,150
after: srv-host=_sip._udp.example.com,tests.example.com,5060,10,150
* if "domain" wasn't specified in the <srv> element, the extra
trailing "." will no longer be added to the dnsmasq commandline.
<srv service='sip' protocol='udp' target='tests.example.com'
port='5060' priority='10' weight='150'/>
before: srv-host=sip.udp.,tests.example.com,5060,10,150
after: srv-host=_sip._udp,tests.example.com,5060,10,150
* when optional attributes aren't specified, the separating comma is
also now not placed on the dnsmasq commandline. If optional
attributes in the middle of the line are not specified, they are
replaced with a default value in the commandline (1 for port, 0 for
priority and weight).
<srv service='sip' protocol='udp' target='tests.example.com'
port='5060'/>
before: srv-host=sip.udp.,tests.example.com,5060,,
after: srv-host=_sip._udp,tests.example.com,5060
(actually the would have generated an error, because "optional"
attributes weren't really optional.)
* The allowed characters for both service and protocol are now limited
to alphanumerics, plus a few special characters that are found in
existing names in /etc/services and /etc/protocols. (One exception
is that both of these files contain names with an embedded ".", but
"." can't be used in these fields of an SRV record because it is
used as a field separator and there is no method to escape a "."
into a field.) (Previously only the strings "tcp" and "udp" were
allowed for protocol, but this restriction has been removed, since
RFC2782 specifically says that it isn't limited to those, and that
anyway it is case insensitive.)
* the "domain" attribute is no longer required in order to recognize
the port, priority, and weight attributes during parsing. Only
"target" is required for this.
* if "target" isn't specified, port, priority, and weight are not
allowed (since they are meaningless - an empty target means "this
service is *not available* for this domain").
* port, priority, and weight are now truly optional, as the comments
originally suggested, but which was not actually true.
In all other drivers we are doing so. Moreover, we don't want to parse
runtime information in attach (even if the attach is meant as live)
because we are generating the runtime info ourselves. We can't trust
users they supply sane values anyway.
==1140== 9 bytes in 1 blocks are definitely lost in loss record 72 of 1,151
==1140== at 0x4A06C2B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1140== by 0x623C758: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1)
==1140== by 0x50FD763: virXMLPropString (virxml.c:483)
==1140== by 0x510F8B7: virDomainDeviceInfoParseXML (domain_conf.c:3685)
==1140== by 0x511ACFD: virDomainChrDefParseXML (domain_conf.c:7535)
==1140== by 0x5121D13: virDomainDeviceDefParse (domain_conf.c:9918)
==1140== by 0x13AE6313: qemuDomainAttachDeviceFlags (qemu_driver.c:6926)
==1140== by 0x13AE65FA: qemuDomainAttachDevice (qemu_driver.c:7005)
==1140== by 0x51C77DA: virDomainAttachDevice (libvirt.c:10231)
==1140== by 0x127FDD: remoteDispatchDomainAttachDevice (remote_dispatch.h:2404)
==1140== by 0x127EC5: remoteDispatchDomainAttachDeviceHelper (remote_dispatch.h:2382)
==1140== by 0x5241F81: virNetServerProgramDispatchCall (virnetserverprogram.c:437)
When doing live attach, we are passing the inactive definition anyway
since we are passing the result of virDomainDeviceDefCopy() which does
inactive copy by default.
Moreover, we are doing the same mistake in qemuhotplugtest.
Just a side note - it makes perfect sense to parse the runtime info
like alias in qemuDomainDetachDevice and qemuDomainUpdateDeviceFlags()
as in some cases the only difference to distinguish two devices can be
just their alias.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The test is loosely inspired from qemucapabilitiestest
and qemuxml2xmltest.
Added a new test instead of extending an existing one because
the feature being tested don't really fits nicely in any
existing place.
Currently, <cputune><shares>0</shares></cputune> is treated
as if it were not specified.
Treat is as a valid value if it was explicitly specified
and write it to the cgroups.
Recent changes in the module seemed to have caused Coverity to reanalyze
certain parts of the code. Previously the code was modified via commit
id '11a11812' to resolve a different error (perhaps DEADCODE). Up through
commit id '7b3f1f8c' there were no issues.
The new error indicats the 'outbuf' was checked for NULL and then complains
because of the dereference. Adding checks for non-NULL prior to the deref
resulted in a DEADCODE message.
So, resolve using an sa_assert() to keep Coverity quiet especially since
it doesn't understand that outbuf will change as a result of a successful
virCommandRun() call.
This patch adds qemuMonitorGetDumpGuestMemoryCapability, which is used to check
whether the specified dump-guest-memory format is supported by qemu.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
It's finally time to start tracking disk backing chains in
<domain> XML. The first step is to start refactoring code
so that we have an object more convenient for representing
each host source resource in the context of a single guest
<disk>. Ultimately, I plan to move the new type into src/util
where it can be reused by virStorageFile, but to make the
transition easier to review, this patch just creates the
new type then fixes everything until it compiles again.
* src/conf/domain_conf.h (_virDomainDiskDef): Split...
(_virDomainDiskSourceDef): ...to new struct.
(virDomainDiskAuthClear): Use new type.
* src/conf/domain_conf.c (virDomainDiskDefFree): Split...
(virDomainDiskSourceDefClear): ...to new function.
(virDomainDiskGetType, virDomainDiskSetType)
(virDomainDiskGetSource, virDomainDiskSetSource)
(virDomainDiskGetDriver, virDomainDiskSetDriver)
(virDomainDiskGetFormat, virDomainDiskSetFormat)
(virDomainDiskAuthClear, virDomainDiskGetActualType)
(virDomainDiskDefParseXML, virDomainDiskSourceDefFormat)
(virDomainDiskDefFormat, virDomainDiskDefForeachPath)
(virDomainDiskDefGetSecurityLabelDef)
(virDomainDiskSourceIsBlockType): Adjust all users.
* src/lxc/lxc_controller.c (virLXCControllerSetupDisk):
Likewise.
* src/lxc/lxc_driver.c (lxcDomainAttachDeviceMknodHelper):
Likewise.
* src/qemu/qemu_command.c (qemuAddRBDHost, qemuParseRBDString)
(qemuParseDriveURIString, qemuParseGlusterString)
(qemuParseISCSIString, qemuParseNBDString)
(qemuDomainDiskGetSourceString, qemuBuildDriveStr)
(qemuBuildCommandLine, qemuParseCommandLineDisk)
(qemuParseCommandLine): Likewise.
* src/qemu/qemu_conf.c (qemuCheckSharedDevice)
(qemuAddISCSIPoolSourceHost, qemuTranslateDiskSourcePool):
Likewise.
* src/qemu/qemu_driver.c (qemuDomainUpdateDeviceConfig)
(qemuDomainPrepareDiskChainElement)
(qemuDomainSnapshotCreateInactiveExternal)
(qemuDomainSnapshotPrepareDiskExternalBackingInactive)
(qemuDomainSnapshotPrepareDiskInternal)
(qemuDomainSnapshotPrepare)
(qemuDomainSnapshotCreateSingleDiskActive)
(qemuDomainSnapshotUndoSingleDiskActive)
(qemuDomainBlockPivot, qemuDomainBlockJobImpl)
(qemuDomainBlockCopy, qemuDomainBlockCommit): Likewise.
* src/qemu/qemu_migration.c (qemuMigrationIsSafe): Likewise.
* src/qemu/qemu_process.c (qemuProcessGetVolumeQcowPassphrase)
(qemuProcessInitPasswords): Likewise.
* src/security/security_selinux.c
(virSecuritySELinuxSetSecurityFileLabel): Likewise.
* src/storage/storage_driver.c (virStorageFileInitFromDiskDef):
Likewise.
* tests/securityselinuxlabeltest.c (testSELinuxLoadDef):
Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Currently the DBus helper APIs require the values for an array
to be passed inline in the variadic argument list. This change
introduces support for passing arrays using a pointer to a plain
C array of the basic type. This is of particular benefit for
decoding messages when you don't know how many array elements
are being received.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The dbus_connection_send_with_reply_and_block method will
automatically call dbus_set_error_from_message for us. We
mistakenly thought we had todo it because of a flaw in the
systemd unit test mock impl. The latter should have directly
set the error object, instead of creating an error message
object.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virSocketAddrMask method did not initialize all fields
in the sockaddr_in6 struct. In paticular the 'sin6_scope_id'
field could contain random garbage, which would in turn
affect the result of any later virSocketAddrFormat calls.
This led to ip6tables rules in the FORWARD chain which
matched on random garbage sin6_scope_id. Fortunately these
were ACCEPT rules, so the impact was merely that desired
traffic was blocked, rather than undesired traffic allowed.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
While running qemuhotplugtest, it was found that valgrind pointed out
the following memory leak:
==7906== 5 bytes in 1 blocks are definitely lost in loss record 7 of 121
==7906== at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==7906== by 0x3E782A754D: xmlStrndup (in /usr/lib64/libxml2.so.2.7.6)
==7906== by 0x4CDAE03: virDomainDeviceInfoParseXML.isra.32 (domain_conf.c:3685)
==7906== by 0x4CE3BB9: virDomainNetDefParseXML (domain_conf.c:6707)
==7906== by 0x4CFBA08: virDomainDefParseXML (domain_conf.c:12235)
==7906== by 0x4CFBC1E: virDomainDefParseNode (domain_conf.c:13039)
==7906== by 0x4CFBD95: virDomainDefParse (domain_conf.c:12981)
==7906== by 0x41FEB4: testQemuHotplug (qemuhotplugtest.c:66)
==7906== by 0x420F41: virtTestRun (testutils.c:201)
==7906== by 0x41F287: mymain (qemuhotplugtest.c:422)
==7906== by 0x4216BD: virtTestMain (testutils.c:784)
==7906== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
...and 10 more.
Problem is, since 20745748 we do both, parse <alias/> elements from
XML files and call qemuAssignDeviceAliases(). While generating runtime
info for domain at runtime is just fine in the test, we can parse just
inactive XML and remove all <alias/>-es from the XML files.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
I forgot to delete the underscore in object_locking_SOURCES when
changing the name in one of previous cleanups.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
To allow for fault injection of the virCommand dry run,
add the ability to register a callback. The callback will
be passed the argv, env and stdin buffer and is expected
to return the exit status and optionally fill stdout and
stderr buffers.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
A earlier commit changed the global log buffer so that it only
records messages that are explicitly requested via the log
filters setting. This removes the performance burden, and
improves the signal/noise ratio for messages in the global
buffer. At the same time though, it is somewhat pointless, since
all the recorded log messages are already going to be sent to an
explicit log output like syslog, stderr or the journal. The
global log buffer is thus just duplicating this data on stderr
upon crash.
The log_buffer_size config parameter is left in the augeas
lens to prevent breakage for users on upgrade. It is however
completely ignored hereafter.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Any source file which calls the logging APIs now needs
to have a VIR_LOG_INIT("source.name") declaration at
the start of the file. This provides a static variable
of the virLogSource type.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
As part of the goal to get away from doing string matching on
filenames when deciding whether to emit a log message, turn
the virLogSource enum into a struct which contains a log
"name". There will eventually be one virLogSource instance
statically declared per source file. To minimise churn in this
commit though, a single global instance is used.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If there should be some sort of separator it is better to use comment
with the filename, copyright, description, license information and
authors.
Found by:
git grep -nH '^$' | grep '\.[ch]:1:'
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Commit a1cbe4b5 added a check for spaces around assignments and this
patch extends it to checks for spaces around '=='. One exception is
virAssertCmpInt where comma after '==' is acceptable (since it is a
macro and '==' is its argument).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
When ran, cil is throwing out some errors and warnings for obsolete
'or' unused variables and wrong module name (it should not contain a
hyphen; hence the rename).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Introducing keepalive similarly to Guannan around 2 years ago. Since
we want to introduce keepalive for every connection, it makes sense to
wrap the connecting function into new virsh one that can deal
keepalive as well.
Function vshConnect() is now used for connecting and keepalive added
in that function (if possible) helps preventing long waits e.g. while
nework goes down during migration.
This patch also adds the options for keepalive tuning into virsh and
fails connecting only when keepalives are explicitly requested and
cannot be set (whether it is due to missing support in connected
driver or remote server). If not explicitely requested, a debug
message is printed (hence the addition to virsh-optparse test).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1073506
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=822839
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
GNULIB provides APIs for calculating md5 and sha256 hashes,
but these APIs only return you raw byte arrays. Most users
in libvirt want the hash in printable string format. Add
some helper APIs in util/vircrypto.{c,h} for doing this.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
With the previous commit's securityselinuxhelper enhancements, the
SELinux security manager can be tested even without SELinux enabled on
the test system.
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
Add fake implementations of:
- is_selinux_enabled
- security_disable
- selinux_virtual_domain_context_path
- selinux_virtual_image_context_path
- selinux_lxc_contexts_path
- selabel_open
- selabel_close
- selabel_lookup_raw
The selabel_* functions back onto the real implementations if SELinux is
enabled on the test system, otherwise we just implement a fake selabel
handle which errors out on all labelling lookups.
With these changes in place, securityselinuxtest and
securityselinuxlabeltest don't need to skip all tests if SELinux isn't
available; they can exercise much of the security manager code.
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
If systemd is installed, but is not the init system,
systemd-machined fails with an unhelpful error message:
Launch helper exited with unknown return code 1
Currently we only check if the "machine1" service is
available (in ListActivatableNames).
Also check if "systemd1" service is registered with DBus
(ListNames).
This fixes https://bugs.gentoo.org/show_bug.cgi?id=493246#c22
Commit 631923e used a few macros from sys/wait.h without including
it. On Linux, they were also defined in stdlib.h, but on FreeBSD
the build failed:
../../tests/commandtest.c: In function 'test1':
warning: implicit declaration of function 'WIFEXITED'
warning: nested extern declaration of 'WIFEXITED' [-Wnested-externs]
The old semantics of virFork() violates the priciple of good
usability: it requires the caller to check the pid argument
after use, *even when virFork returned -1*, in order to properly
abort a child process that failed setup done immediately after
fork() - that is, the caller must call _exit() in the child.
While uses in virfile.c did this correctly, uses in 'virsh
lxc-enter-namespace' and 'virt-login-shell' would happily return
from the calling function in both the child and the parent,
leading to very confusing results. [Thankfully, I found the
problem by inspection, and can't actually trigger the double
return on error without an LD_PRELOAD library.]
It is much better if the semantics of virFork are impossible
to abuse. Looking at virFork(), the parent could only ever
return -1 with a non-negative pid if it misused pthread_sigmask,
but this never happens. Up until this patch series, the child
could return -1 with non-negative pid if it fails to set up
signals correctly, but we recently fixed that to make the child
call _exit() at that point instead of forcing the caller to do
it. Thus, the return value and contents of the pid argument are
now redundant (a -1 return now happens only for failure to fork,
a child 0 return only happens for a successful 0 pid, and a
parent 0 return only happens for a successful non-zero pid),
so we might as well return the pid directly rather than an
integer of whether it succeeded or failed; this is also good
from the interface design perspective as users are already
familiar with fork() semantics.
One last change in this patch: before returning the pid directly,
I found cases where using virProcessWait unconditionally on a
cleanup path of a virFork's -1 pid return would be nicer if there
were a way to avoid it overwriting an earlier message. While
such paths are a bit harder to come by with my change to a direct
pid return, I decided to keep the virProcessWait change in this
patch.
* src/util/vircommand.h (virFork): Change signature.
* src/util/vircommand.c (virFork): Guarantee that child will only
return on success, to simplify callers. Return pid rather than
status, now that the situations are always the same.
(virExec): Adjust caller, also avoid open-coding process death.
* src/util/virprocess.c (virProcessWait): Tweak semantics when pid
is -1.
(virProcessRunInMountNamespace): Adjust caller.
* src/util/virfile.c (virFileAccessibleAs, virFileOpenForked)
(virDirCreate): Likewise.
* tools/virt-login-shell.c (main): Likewise.
* tools/virsh-domain.c (cmdLxcEnterNamespace): Likewise.
* tests/commandtest.c (test23): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Auditing all callers of virCommandRun and virCommandWait that
passed a non-NULL pointer for exit status turned up some
interesting observations. Many callers were merely passing
a pointer to avoid the overall command dying, but without
caring what the exit status was - but these callers would
be better off treating a child death by signal as an abnormal
exit. Other callers were actually acting on the status, but
not all of them remembered to filter by WIFEXITED and convert
with WEXITSTATUS; depending on the platform, this can result
in a status being reported as 256 times too big. And among
those that correctly parse the output, it gets rather verbose.
Finally, there were the callers that explicitly checked that
the status was 0, and gave their own message, but with fewer
details than what virCommand gives for free.
So the best idea is to move the complexity out of callers and
into virCommand - by default, we return the actual exit status
already cleaned through WEXITSTATUS and treat signals as a
failed command; but the few callers that care can ask for raw
status and act on it themselves.
* src/util/vircommand.h (virCommandRawStatus): New prototype.
* src/libvirt_private.syms (util/command.h): Export it.
* docs/internals/command.html.in: Document it.
* src/util/vircommand.c (virCommandRawStatus): New function.
(virCommandWait): Adjust semantics.
* tests/commandtest.c (test1): Test it.
* daemon/remote.c (remoteDispatchAuthPolkit): Adjust callers.
* src/access/viraccessdriverpolkit.c (virAccessDriverPolkitCheck):
Likewise.
* src/fdstream.c (virFDStreamCloseInt): Likewise.
* src/lxc/lxc_process.c (virLXCProcessStart): Likewise.
* src/qemu/qemu_command.c (qemuCreateInBridgePortWithHelper):
Likewise.
* src/xen/xen_driver.c (xenUnifiedXendProbe): Simplify.
* tests/reconnect.c (mymain): Likewise.
* tests/statstest.c (mymain): Likewise.
* src/bhyve/bhyve_process.c (virBhyveProcessStart)
(virBhyveProcessStop): Don't overwrite virCommand error.
* src/libvirt.c (virConnectAuthGainPolkit): Likewise.
* src/openvz/openvz_driver.c (openvzDomainGetBarrierLimit)
(openvzDomainSetBarrierLimit): Likewise.
* src/util/virebtables.c (virEbTablesOnceInit): Likewise.
* src/util/viriptables.c (virIpTablesOnceInit): Likewise.
* src/util/virnetdevveth.c (virNetDevVethCreate): Fix debug
message.
* src/qemu/qemu_capabilities.c (virQEMUCapsInitQMP): Add comment.
* src/storage/storage_backend_iscsi.c
(virStorageBackendISCSINodeUpdate): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Right now, a caller waiting for a child process either requires
the child to have status 0, or must use WIFEXITED() and friends
itself. But in many cases, we want the middle ground of treating
fatal signals as an error, and directly accessing the normal exit
value without having to use WEXITSTATUS(), in order to easily
detect an expected non-zero exit status. This adds the middle
ground to the low-level virProcessWait; the next patch will add
it to virCommand.
* src/util/virprocess.h (virProcessWait): Alter signature.
* src/util/virprocess.c (virProcessWait): Add parameter.
(virProcessRunInMountNamespace): Adjust caller.
* src/util/vircommand.c (virCommandWait): Likewise.
* src/util/virfile.c (virFileAccessibleAs): Likewise.
* src/lxc/lxc_container.c (lxcContainerHasReboot)
(lxcContainerAvailable): Likewise.
* daemon/libvirtd.c (daemonForkIntoBackground): Likewise.
* tools/virt-login-shell.c (main): Likewise.
* tools/virsh-domain.c (cmdLxcEnterNamespace): Likewise.
* tests/testutils.c (virtTestCaptureProgramOutput): Likewise.
* tests/commandtest.c (test23): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Thanks to namespaces, we have a couple of places in the code
base that want to reflect a child exit status, including the
ability to detect death by a signal, back to a grandparent.
Best to make it a reusable function.
* src/util/virprocess.h (virProcessExitWithStatus): New prototype.
* src/libvirt_private.syms (util/virprocess.h): Export it.
* src/util/virprocess.c (virProcessExitWithStatus): New function.
* tests/commandtest.c (test23): Test it.
Signed-off-by: Eric Blake <eblake@redhat.com>
When a child fails without exec'ing, we want a well-known status;
best is to match what env(1), nice(1), su(1), and other wrapper
programs do. This patch adds enum values that later patches will
use, and sets up virFork as the first client of EXIT_CANCELED
for errors detected prior to even attempting exec, as well as
virExec to distinguish between a missing executable vs. a binary
that cannot be executed.
This is a slight semantic change in the unlikely case of a child
process failing to restore its signal mask - we now kill the
child with a known status instead of relying on the caller to
notice and do an appropriate _exit(). A subsequent patch will
make further cleanups based on an audit of all callers.
* src/internal.h (EXIT_CANCELED, EXIT_CANNOT_INVOKE)
(EXIT_ENOENT): New enum.
* src/util/vircommand.c (virFork): Document specific exit value if
child aborts early.
(virExec): Distinguish between various exec failures.
* tests/commandtest.c (test1): Enhance test.
(test22): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Running 'make -C tests check TESTS=qemuagenttest' left a directory
/tmp/libvirt_XXXXXX/ behind. The culprit was failure to cleanup
when short-circuiting an expensive test.
* tests/qemuagenttest.c (testQemuAgentTimeout): Free resources
when skipping expensive test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Cygwin supports <dlfcn.h> and even has limited LD_PRELOAD
capabilities; but because it does not use ELF binaries it
cannot support RTLD_NEXT lookups.
CC libvirportallocatormock_la-virportallocatortest.lo
virportallocatortest.c: In function 'init_syms':
virportallocatortest.c:47:24: error: 'RTLD_NEXT' undeclared (first use in this function)
realsocket = dlsym(RTLD_NEXT, "socket");
* tests/virportallocatortest.c: Also require RTLD_NEXT.
Signed-off-by: Eric Blake <eblake@redhat.com>
Running ./autobuild.sh complained during the mingw cross-compile:
CC libvirportallocatormock_la-virportallocatortest.lo
../../tests/virportallocatortest.c:32:20: fatal error: dlfcn.h: No such file or directory
# include <dlfcn.h>
^
compilation terminated. With that fixed, the next failure was:
CCLD qemuxml2argvmock.la
libtool: link: libtool library `qemuxml2argvmock.la' must begin with `lib'
libtool: link: Try `libtool --help --mode=link' for more information.
While we don't need to limit all LD_PRELOAD tests to just Linux, we
do need to limit them to platforms that actually support loading;
we also need to avoid building qemu tests when qemu is not enabled.
* tests/virportallocatortest.c: Make conditional on <dlfcn.h>.
* tests/Makefile.am (test_libraries): Only build qemu mock library
when building qemu tests.
Signed-off-by: Eric Blake <eblake@redhat.com>
This moves the call to virNetDevBandwidthFormat() in
virDomainNetDefFormat() to be called right after the call to
virNetDevVPortProfileFormat(), so that a single chunk of that function
can be placed inside an if that conditionally calls
virDomainActualNetDefContentsFormat() instead (next patch). The
re-ordering necessitates modifying a couple of test data files.
Add a virStringSearch method to virstring.{c,h} which performs
a regex match against a string and returns the matching substrings.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
