Either an error should be returned in all error paths in a
function or in none (leaving it up to caller). Well,
virPCIGetVirtualFunctionIndex() breaks this pattern. Fix it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Adding a storage volume into a pool is done by calling
virStoragePoolObjAddVol(). This function may fail if another
volume already exists with the same key/name/target. In some
cases the storage driver does check for duplicates before calling
the function. But in some cases (e.g. when refreshing an RBD pool
in virStorageBackendRBDRefreshPool()) it doesn't.
The problem here is that the function reports no error upon
failure and leaves it as an exercise for caller. Well, no caller
does that.
Therefore, make the function report an error. The advantage of
this approach is - the function can report more accurate error
message than any caller ever could.
NB¸ this stems from a discussion on the users list [1], and while
this does NOT solve the original issue, it fixes one of the
symptoms.
1: https://lists.libvirt.org/archives/list/users@lists.libvirt.org/message/BALVNCRQM4KBKGV4RQ7BMKSX7UIJKLQH/
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Since some hyperv features might be already enabled/disabled when
entering qemuProcessEnableDomainFeatures() only those which are
not set in domain XML (i.e. are VIR_TRISTATE_SWITCH_ABSENT)
should be modified. Furthermore, some features are not a simple
on/off switch, but a number or a string even. Well, that doesn't
matter really as the logic for setting them is the same: only set
their value iff they are not already set.
Resolves: https://issues.redhat.com/browse/RHEL-148219
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
As it turns out, some users of the hyperv "host-model" mode might
want to override the hypervisor defaults. For instance disable a
feature that's on by default, or vice versa. Currently, this is
not possible because as soon as our XML parser sees the
"host-model" mode it exits early and skips parsing of individual
features (for "custom" mode). Well, do not return early and parse
the rest.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virDomainInterfaceAddresses() API accepts @source argument,
but since this is hyperv, we can't really use _SRC_LEASE (we
didn't spawn any dnsmasq there), not _SRC_ARP. The only source
that's more or less usable is _SRC_AGENT. Okay, there's no QEMU
guest agent running, but hyperv has its own guest agent. In my
testing (with Linux guest) I had to install 'hyperv' package and
then enable 'hv_kvp_daemon.service'. After that,
Msvm_GuestNetworkAdapterConfiguration struct [1] contained guest
IP addresses.
There's one caveat though: the interface name
(virDomainInterface::name). We don't fetch that one even for
hypervDomainGetXMLDesc() case. And there's no <target dev=''/>
either nor device alias (v12.0.0-43-g4009126f17). So just put
InstanceID there for now, which is this long path, with some
UUIDs, e.g.:
Microsoft:5C58E5F2-946E-490F-B81D-6E2A7328640D\C85554E0-2B3B-487C-A557-D230BFF5F9E6\
But hey, at least it's unique.
1: https://learn.microsoft.com/en-us/windows/win32/hyperv_v2/msvm-guestnetworkadapterconfiguration
Resolves: https://issues.redhat.com/browse/RHEL-145306
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When constructing a domain definition, NICs are fetched from WMI
and their MAC addresses are then parsed. Move this code into a
separate function so that it can be reused later.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The aim of this helper is to convert subnet mask to prefix. For
instance for input "255.0.0.0" to return 8. Additionally, if the
input string is already a prefix (with optional leading slash
character) just return that number parsed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virDomainInterface type (struct _virDomainInterface) is
defined in our public header and even has a public free function
(virDomainInterfaceFree()). But in our code we will want to use
automatic memory freeing for it. Hence, make appropriate
declaration in datatypes.h.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The aim of cmdDomIfAddr() is to obtain IP addresses for given
domain and then print (ifName, MAC, type, IP Address) tuple.
Preferably in an aligned table. This is hard to do with printf
style of spacing ("%-NNs") since the interface name (ifName) can
vary a lot in length. Fortunately, we have vshTable which is
designed to handle this case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Just like with other features, check whether QEMU supports them
based on capabilities. Now, instead of inventing a new QEMU
capability, an existing one can be used:
QEMU_CAPS_VIRTIO_IOMMU_AW_BITS.
This is because the aw-bits and granule attributes were
introduced into QEMU in close succession (v9.0.0-rc0~9^2~7
v9.0.0-rc0~9^2~11), neither can be disabled at compile time and
backporting just one without the other makes almost no sense.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In PCI assignment scenario the virtio-iommu needs to know the
guest page size also known as granule. Expose it as an attribute
to the <driver/> element of a virtio-iommu.
This is possibly interesting only for aarch64 since it supports
virtio-iommu and also supports running guests with different page
size than the host.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Introduced in QEMU commit of v9.0.0-rc0~9^2~7 the virtio-iommu
device is also capable of using different addres width. The
corresponding attribute is also called 'aw-bits', just like in
case of intel-iommu. Wire up the missing pieces.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This capability tracks whether the virtio-iommu device has
aw-bits attribute.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
So far, virDomainParseMemory() returns either 0 or -1. While this
allows callers to distinguish a success case from an error it
doesn't allow them to differentiate the case when no value was
provided in the XML, thus nothing was parsed and nothing was
required. Therefore, make virDomainParseMemory() return 1 on
success, 0 in case nothing was parsed and nothing was required,
and -1 on failure.
Arguably, no caller needs this distinction currently, but that is
about to change.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When parsing a physical disk, the @hostResouce is escaped once
with the retval being stored into @hostEscaped. Then, it's
escaped again, but the retval is stored into the very same
variable, leading to a leak where intermediate value is lost.
256 bytes in 1 blocks are definitely lost in loss record 469 of 483
at 0x49543A0: realloc (vg_replace_malloc.c:1804)
by 0x516C251: g_realloc (in /usr/lib64/libglib-2.0.so.0.8400.4)
by 0x518BB7E: g_string_expand (in /usr/lib64/libglib-2.0.so.0.8400.4)
by 0x518BFF9: g_string_insert_len (in /usr/lib64/libglib-2.0.so.0.8400.4)
by 0x4A58B5F: g_string_append_len_inline (gstring.h:247)
by 0x4A58B5F: virBufferAdd (virbuffer.c:164)
by 0x4AFDA71: virStringReplace (virstring.c:708)
by 0x4DA4381: hypervDomainDefParsePhysicalDisk (hyperv_driver.c:1375)
by 0x4DA4A18: hypervDomainDefParseStorage (hyperv_driver.c:1487)
by 0x4DA9E31: hypervDomainGetXMLDesc (hyperv_driver.c:2761)
by 0x4DFB3E5: virDomainGetXMLDesc (libvirt-domain.c:2898)
by 0x406D39B: cmdDumpXML (virsh-domain.c:10787)
by 0x40B13B1: vshCommandRun (vsh.c:1383)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Driver capabilities are allocated at the beginning of mymain(),
but roughly in the middle the architecture is switched to aarch64
and capabilities are constructed again. Without freeing the old
ones.
704 (288 direct, 416 indirect) bytes in 1 blocks are definitely lost in loss record 328 of 332
at 0x4885098: calloc (vg_replace_malloc.c:1682)
by 0x4EE35CA: g_malloc0 (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x53314B8: g_type_create_instance (in /usr/local/lib/libgobject-2.0.so.0.8400.4)
by 0x531A263: ??? (in /usr/local/lib/libgobject-2.0.so.0.8400.4)
by 0x531975E: g_object_new (in /usr/local/lib/libgobject-2.0.so.0.8400.4)
by 0x4AA9AB6: virObjectNew (virobject.c:252)
by 0x4AF0BBA: virCapabilitiesNew (capabilities.c:87)
by 0x401797B: virBhyveCapsBuild (bhyve_capabilities.c:51)
by 0x4012F57: mymain (bhyvexml2xmltest.c:60)
by 0x4016872: virTestMain (testutils.c:913)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
The firmwareDir member of driver config is set at the beginning
of mymain(). But then, roughly in the middle of test cases it is
overwritten to fakefirmwareemptydir. But this means the old value
must be freed. Or reassigned back to its original variable which
is freed automatically.
16 bytes in 1 blocks are definitely lost in loss record 190 of 505
at 0x4883224: malloc (vg_replace_malloc.c:451)
by 0x4EE6562: g_malloc (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x4F0100F: g_strdup (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x4013E26: g_strdup_inline (gstrfuncs.h:321)
by 0x4013E26: mymain (bhyvexml2argvtest.c:151)
by 0x40189A2: virTestMain (testutils.c:913)
by 0x4013DE6: main (bhyvexml2argvtest.c:354)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
At the beginning of mymain() the virBhyveDriverConfigNew() is
called which inits driver config with some paths. These are
then overwritten to produce stable test output. Well, the old
ones should be freed first.
128 bytes in 1 blocks are definitely lost in loss record 453 of 508
at 0x4883224: malloc (vg_replace_malloc.c:451)
by 0x506BD16: vasprintf_l (in /lib/libc.so.7)
by 0x4F39073: g_vasprintf (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x4F01288: g_strdup_printf (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x401F75B: virBhyveDriverConfigNew (bhyve_conf.c:62)
by 0x4013FAA: mymain (bhyvexml2argvtest.c:164)
by 0x4018892: virTestMain (testutils.c:913)
by 0x4013DC6: main (bhyvexml2argvtest.c:352)
25 bytes in 1 blocks are definitely lost in loss record 206 of 508
at 0x4883224: malloc (vg_replace_malloc.c:451)
by 0x4EE6562: g_malloc (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x4F0100F: g_strdup (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x401F715: g_strdup_inline (gstrfuncs.h:321)
by 0x401F715: virBhyveDriverConfigNew (bhyve_conf.c:60)
by 0x4013FAA: mymain (bhyvexml2argvtest.c:164)
by 0x4018892: virTestMain (testutils.c:913)
by 0x4013DC6: main (bhyvexml2argvtest.c:352)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Driver capabilities are allocated at the beginning of mymain(),
but roughly in the middle the architecture is switched to aarch64
and capabilities are constructed again. Without freeing the old
ones.
1,583 (288 direct, 1,295 indirect) bytes in 1 blocks are definitely lost in loss record 520 of 536
at 0x4888098: calloc (vg_replace_malloc.c:1682)
by 0x4EE65CA: g_malloc0 (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x53344B8: g_type_create_instance (in /usr/local/lib/libgobject-2.0.so.0.8400.4)
by 0x531D263: ??? (in /usr/local/lib/libgobject-2.0.so.0.8400.4)
by 0x531C75E: g_object_new (in /usr/local/lib/libgobject-2.0.so.0.8400.4)
by 0x4AAC806: virObjectNew (virobject.c:252)
by 0x4AF366A: virCapabilitiesNew (capabilities.c:87)
by 0x401998B: virBhyveCapsBuild (bhyve_capabilities.c:51)
by 0x4013E93: mymain (bhyvexml2argvtest.c:155)
by 0x4018882: virTestMain (testutils.c:913)
by 0x4013DC6: main (bhyvexml2argvtest.c:351)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
The bhyvexml2argvmock is loaded by bhyvexml2argvtest which calls
virBhyveCapsBuild() which in turn calls virCPUProbeHost(). To
make our test environment stable, it shouldn't depend on actual
CPU and thus mocked implementation for virCPUProbeHost should be
offered. Surprisingly, this is done in bhyveargv2xmlmock but not
in bhyvexml2argvmock. Until now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Inside of bhyveBuildNetArgStr() there is @nic_model which is
allocated, appended into cmd line and then freed under cleanup
label. Firstly, There are few cases where instead of jumping onto
the label there's a return statement (this alone can lead to a
memory leak), but more importantly - the variable doesn't need
dynamically allocated string. It's the same story with @brname.
After making them both const strings, the return statements can
be used more freely (up until first possible allocation).
6 bytes in 1 blocks are definitely lost in loss record 4 of 508
at 0x4883224: malloc (vg_replace_malloc.c:451)
by 0x4EE6562: g_malloc (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x4F0100F: g_strdup (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x401BC02: g_strdup_inline (gstrfuncs.h:321)
by 0x401BC02: bhyveBuildNetArgStr (bhyve_command.c:64)
by 0x401B362: virBhyveProcessBuildBhyveCmd (bhyve_command.c:1033)
by 0x4015F15: testCompareXMLToArgvFiles (bhyvexml2argvtest.c:72)
by 0x4015BB9: testCompareXMLToArgvHelper (bhyvexml2argvtest.c:144)
by 0x4016598: virTestRun (testutils.c:143)
by 0x4015121: mymain (bhyvexml2argvtest.c:275)
by 0x4018892: virTestMain (testutils.c:913)
by 0x4013DC6: main (bhyvexml2argvtest.c:352)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
When building OS loader part of bhyve command line, there's @buf
declared and it is even correctly annotated with g_auto() to be
freed automatically. But then, the buffer contents is appended
onto the command line using virBufferContentAndReset() which
leads to a memleak because the buffer is reset. It's
virBufferCurrentContent() that should have been used instead.
128 bytes in 1 blocks are definitely lost in loss record 476 of 536
at 0x48882B1: realloc (vg_replace_malloc.c:1810)
by 0x4EE6622: g_realloc (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x4F048BC: g_string_new (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x4A59E1E: virBufferInitialize (virbuffer.c:121)
by 0x4A5A63C: virBufferVasprintf (virbuffer.c:321)
by 0x4A5A5DE: virBufferAsprintf (virbuffer.c:303)
by 0x401B22F: virBhyveProcessBuildBhyveCmd (bhyve_command.c:1021)
by 0x4015F05: testCompareXMLToArgvFiles (bhyvexml2argvtest.c:72)
by 0x4015BA9: testCompareXMLToArgvHelper (bhyvexml2argvtest.c:144)
by 0x4016588: virTestRun (testutils.c:143)
by 0x4015919: mymain (bhyvexml2argvtest.c:341)
by 0x4018882: virTestMain (testutils.c:913)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Inside of bhyveDomainAssignPCIAddresses() the @addr variable is
allocated and in a few cases stolen into domain private data. But
in all other cases the associated memory is never freed.
12,800 (3,200 direct, 9,600 indirect) bytes in 100 blocks are definitely lost in loss record 533 of 538
at 0x4888098: calloc (vg_replace_malloc.c:1682)
by 0x4EE67D9: g_malloc0_n (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x4AFD4AC: virDomainPCIAddressSetAlloc (domain_addr.c:1011)
by 0x4020F68: bhyveDomainPCIAddressSetCreate (bhyve_device.c:65)
by 0x40210BD: bhyveDomainAssignPCIAddresses (bhyve_device.c:219)
by 0x402180C: bhyveDomainAssignAddresses (bhyve_device.c:241)
by 0x4020083: bhyveDomainDefAssignAddresses (bhyve_domain.c:230)
by 0x4B71820: virDomainDefPostParse (domain_postparse.c:1503)
by 0x4B28282: virDomainDefParseNode (domain_conf.c:20565)
by 0x4B2810B: virDomainDefParse (domain_conf.c:20502)
by 0x4B281DF: virDomainDefParseFile (domain_conf.c:20549)
by 0x4015D6B: testCompareXMLToArgvFiles (bhyvexml2argvtest.c:47)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
The aim of bhyveParsePassthru() is to parse PCI address from
bhyve command line. The PCI address might be of a form
bus:slot:function or bus/slot/function. If the former isn't found
the latter is parsed (both using g_strsplit()). But after the
first call, g_strsplit() just returns a string list containing
but the whole input duplicated. Therefore, calling plain g_free()
is not enough, the array must be freed too.
6 bytes in 1 blocks are definitely lost in loss record 1 of 325
at 0x4863224: malloc (vg_replace_malloc.c:451)
by 0x4EC6562: g_malloc (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x4EE28D9: g_strsplit (in /usr/local/lib/libglib-2.0.so.0.8400.4)
by 0x4011297: bhyveParsePassthru (bhyve_parse_command.c:699)
by 0x4010082: bhyveParseBhyvePCIArg (bhyve_parse_command.c:800)
by 0x400EE14: bhyveParseBhyveCommandLine (bhyve_parse_command.c:862)
by 0x400DF9C: bhyveParseCommandLineString (bhyve_parse_command.c:1058)
by 0x4008CA0: testCompareXMLToArgvFiles (bhyveargv2xmltest.c:39)
by 0x4008B29: testCompareXMLToArgvHelper (bhyveargv2xmltest.c:105)
by 0x4009288: virTestRun (testutils.c:143)
by 0x40085AC: mymain (bhyveargv2xmltest.c:164)
by 0x400B582: virTestMain (testutils.c:913)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
In previous commit of v12.0.0-85-g2c66b6d72c we've tried to solve
a problem where xdrproc_t is a prototype of a function which
takes only two arguments instead of three. See original commit
for more info. The fix consists of a config time check and
setting XDRPROC_T_3ARGS accordingly (in meson-config.h). This
works for nearly all of our code, except rpcgen which is
intentionally independent of the rest of the code. Therefore, the
macro has to be set extra - by specifying it on the compiler cmd
line.
Fixes: 2c66b6d72c
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Our network has multiple means of forwarding the traffic and
'hostdev' is one of them. This mode means that the network is
configured to use a set of PCI devices which are then assigned to
individual domains to use (PCI device assignment). Now, as of
v12.0.0-61-gecb2e06bdf our test runners
(testCompareXMLToXMLFiles() and testCompareXMLToConfFiles()) call
networkValidateTests(). For aforementioned type of network this
means checking that the specified set of devices contains only
VFs (see v3.2.0-rc1~24 for more info). It is true that our
virpcimock is preloaded which mimics VFs, but our utils module
(virpci.c specifically) talks to sysfs to check various PCI
device attributes, including whether it's a VF.
This obviously works on Linux and doesn't work anywhere else.
Therefore, until our utils module is taught how to check PCI
attribs on other systems, make the "hostdev" test case expect
validation failure on non-Linux systems.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Now that networkxml2xmltest does both XML -> XML and XML -> conf
tests its name became misleading. Rename it to networkxmlconftest
and move its data into networkxmlconfdata/ dir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that networkxml2xmltest does XML->conf tests the
networkxml2conftest is redundant. Drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The networkxml2xmltest does basic parse -> format tests.
The networkxml2conftest does parse -> conf tests.
Now, majority of XMLs are the same. That is, output XMLs of
networkxml2xmltest and input XMls of networkxml2conftest. There
are only a few differences. This is actually great, because it
will allow either tests to do both test cases.
There are some (subtle) differences in individual test cases
though:
1) some test cases exist only in networkxml2conftest and not
networkxml2xmltest, or
2) some test cases in networkxml2conftest have more values, i.e.
extra elements, extra attributes. or
3) some test cases in networkxml2conftest have less values.
For cases from 1) they were just copied over. For cases from 2)
those extra elements/attributes were added, and for cases from 3)
those extra attributes were removed (to minimize changes to .conf
files in near future).
One caveat though: networkxml2xmlupdatetest uses input XMLs of
networkxml2xmltest too (hence changes under
networkxml2xmlupdateout/ dir). This means that the
"delete-srv-record-protocol" test started failing, because the
input network XML now has more <srv/> records than the test case
anticipated. But this is easy to fix - hence seemingly unrelated
change under networkxml2xmlupdatein/ dir.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Soon, the testRun() will run more than one test case. The input
network XML, however, stays the same. Instead of parsing it and
throwing away immediately, store it temporarily.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
So far, the testInfo struct contained immutable data (from its
lifetime point of view). But that is about to change. For
instance, it will hold parsed network definition (virNetworkDef)
and in order to avoid leaking dynamically allocated data
corresponding free function must be introduced (or clear
function, doesn't really matter). At this point, the structure
might as well be dynamically allocated entirely.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This effectively dissolves testCompareXMLToXMLHelper() into
testRun(). Motivation is that parts of data generated inside of
testCompareXMLToXMLHelper() is going to be reused from the caller
(testRun()).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is a beginning of something bigger. The idea is that one
DO_TEST_FULL() macro (and its friends) will run multiple test
cases (just like qemuxmlconftest does). But in order to do that
in a readable fashion, the macro should merely just expand to a
function call. The function will then call virTestRunLog(),
multiple times possibly.
This is the first step in that direction.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The aim of virNetworkXMLOption object is to provide some
immutable data to XML parser (e.g. various callbacks). Since the
object is immutable, it can be created once and then reused by
all test cases.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently, there are two calls to virTestCompareToFile() inside
of testCompareXMLToConfFiles(). If the first one fails the
control jumps directly onto the fail label and skips the second
one. This means that When regenerating test case output
(VIR_TEST_REGENERATE_OUTPUT) the test binary has to be called
twice to regenerate all the files. Suboptimal. Try harder to call
both compare helpers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Inside of testCompareXMLToConfFiles() the
networkDnsmasqConfContents() is called. This may also produce
contents of corresponding hosts file. This is then compared to
expected contents stored on disk as ${testname}.hostsfile. But
due to additional checks virTestCompareToFile() might not even be
called. Problem with that is when there's actual content but the
file doesn't exist the compare helper is not called and thus
VIR_TEST_REGENERATE_OUTPUT trick doesn't work. Let's call the
helper more often as it is perfectly capable of handling this
edge case. What it is not capable of handling is when the file
shouldn't exist at all. So handling of that case is kept.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Inside of testCompareXMLToConfFiles() the network definition is
parsed and if that succeeds a virNetworkObj is created by calling
virNetworkObjNew(). But if the latter fails, the control jumps
onto the fail label where only the object is freed but not
already parsed definition leading to a leak.
Swapping these two steps ensures that if either of them fails no
memleak occurs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our network driver calls networkValidate() right after a network
XML is parsed. This is similar to domain validation step when
parsing domain XML. But it's not that convoluted in network
driver. Regardless, any network related test should mimic real
life scenario as close as possible and thus
networkValidateTests() should be called right after domain XML is
parsed.
Now, networkValidate() might query sysfs wrt to PCI devices and
thus tests must start using virpcimock. The function will also
generate random MAC addresses, if needed, hence virrandommock.
With this change, passthrough-pf and passthrough-address-crash
test cases of networkxml2xmltest started failing but looking at
corresponding XMLs those test cases were designed to test just
XML parsing. They were never designed to showcase a "real"
network XML. So mark them as expected fail.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The aim of this internal API is to wrap networkValidate() and it
is meant to be called from our tests.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Soon, individual test cases of networkxml2xmltest will be subject
to networkValidate() call. This means, that input XMLs must be
valid (or marked as expected fail). Anyway, there are couple of
offenders:
1) 8021Qbh-net.xml setting vlan for <forward mode='private'/> is
unsupported,
2) hostdev.xml networkValidate() will check if hostdevs specified
for <forward mode='hostdev'/> are VFs. Use PCI addresses from
virpcimock.
3) openvswitch-net.xml for <forward mode='bridge'/> only
openvswitch type of virtualports is allowed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If network config does not require dnsmasq then none is spawned.
Having a test case that would still require generating dnsmasq
config is weird and can lead to spurious results. Just fail such
test case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This test case is spurious. If this was real life scenario then
no dnsmasq would be spawned and yet, the test tries to generate
dnsmasq config for it. Just drop the test case and move on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Whether a network needs dnsmasq or not is decided at the
beginning of networkStartDhcpDaemon(). Move that code into a
separate function so it can be reused later.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are two functions implemented in bridge_driver.c that are
used in tests (networkDnsmasqCreateXMLConf() and
networkDnsmasqConfContents()) but are declared in
bridge_driver.h. This goes against our current practice where
such APIs are declared in $name_priv.h.
Therefore, move those APIs to bridge_driver_priv.h
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of including network driver header files as:
#include "network/header.h"
pass network_inc_dir to the compiler and include files directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The aw_bits member of the virDomainIOMMUDef is of type unsigned
int. However, in a few places
(virDomainIOMMUDefCheckABIStability(), virDomainIOMMUDefFormat(),
qemuBuildIOMMUCommandLine()) incorrect printf modifier is used.
Fix those places.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virDomainInterfaceAddresses() API returns an array of
_virDomainInterface structs which then describe IP addresses
associated with given domain. The struct contains 'name' member
which is documented deliberately vaguely: "interface name". This
is because depending on the source of truth used (controlled by
'source' argument) the name can be wildly different from the one
in domain XML. Now, in case of source =
VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP, the host's ARP table is
parsed and matching interfaces are found by comparing MAC
addresses. If it's a match then the 'name' is set to net->ifname
(corresponds to /interface/target/@dev). But that is not always
set and sometimes may be NULL (e.g. for hostdevs, usernet). We
can't change the API (like we did for hwaddr in v1.2.14-rc1~105)
because this is already released. So the next best thing to do is
to put the interface alias in there.
To be on a safe side, do the same change to the
VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE case.
Resolves: https://issues.redhat.com/browse/RHEL-141496
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ever since of v1.2.14-rc1~105 the hwaddr member of
_virDomainInterface struct can be NULL. And this is documented
inside the struct. But then the public API documents it can never
be NULL which is obviously wrong. Fix the public API
documentation.
Fixes: 3640245db7
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since v10.8.0-rc1~133 a different error is reported from
virNetDevTapCreate() when the tap device already exists (and
<interface/> XML specifies managed='no'). But the change covers
only one scenario: if multiqueue was requested in <inteface/> XML
BUT pre-created tap device doesn't have multi_queue flag set. The
opposite scenario: if the device has the multi_queue flag set BUT
no multiqueue was specified in the XML. Until now.
Resolves: https://issues.redhat.com/browse/RHEL-118303
Fixes: f6fb097e11
Fixes: 465a38154f
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When connecting to a VMWare server, the hostname from URI is
resolved using esxUtil_ResolveHostname() which in turn calls
getaddrinfo(). But in the hints argument, we restrict the return
address to be IPv4 (AF_INET) which obviously fails if the address
to resolve is an IPv6 address. Set the hint to AF_UNSPEC which
allows both IPv4 and IPv6. While at it, also allow IPv4 addresses
mapped in IPv6 by setting AI_V4MAPPED flag.
Resolves: https://issues.redhat.com/browse/RHEL-138300
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
In shell, the following function doesn't echo '1' but '0':
func() {
local var=$(false)
echo $?
}
This is because '$?' does not refer to 'false' but 'local'. The
bash_builtins(1) manpage explains it well. And it also mentions
other commands behaving the same: export, declare and readonly.
Since it is really easy to miss this pattern, introduce a
syntax-check rule. Mind you, the following pattern (which passes
the rule) does check for the subshell exit code:
func() {
local var
var=$(false)
echo $?
}
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The guest_is_on() function is documented to check whether given
domain is running and set guest_running variable accordingly. It
does so by running virsh (transitively), then setting the
variable and only after that comparing $? variable. This is
obviously wrong, because after the guest_running variable
assignment the $? variable no longer holds the exit code of
virsh. Even worse, as explained in the previous commit, it never
held that value in the first place. Fix this by firstly setting
the global variable and only after that running virsh.
Fixes: 08071ec0f1
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/839
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In Bash, the following code does not do what you think it does:
func() {
local var=$(false)
echo $?
}
Here, '0' is echoed even though false is designed to exit with a
non-zero code. This is because in fact the last executed command
is 'local' not 'false' and thus '$?' contains zero as in "yeah,
the variable is successfully declared" [1]. In our libvirt-guest
shell script, there are a few places like this. Fix them.
1: bash_builtins(1)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Some functions inside of domain_logcontext.c call virLogManager
APIs. But that one is available only when remote driver is
enabled. Therefore, make domain_logcontext.c conditional, just
like log_manager.c is: if config.has('WITH_REMOTE').
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/842
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The aim of this rule is to capture the following pattern:
g_autofree const char *var = ...;
The pattern is problematic, because it frees a const pointer. If
written the old way, we'd get a compiler warning:
warning: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The 'forwardIf' variable inside of pfAddNatFirewallRules() is
declared as both g_autofree and const. This makes no sense.
Since the variable is g_strdup()-ed into right in the
declaration, it's not const. Drop that part of variable
declaration.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The 'resourceType' variable inside of hypervDomainAttachSerial()
is declared as both g_autofree and const. This makes no sense.
Since the variable is g_strdup_printf()-ed into a couple of lines
later, it's not const. Drop that part of variable declaration.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
There are some features/improvements/bug fixes I've either
contributed or reviewed/merged. Document them for upcoming
release.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Per our coding style, function shall be separated by either a
single line or two lines. But in esx_util.c functions are
separated by three or even four blank lines. Drop excessive
spacing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The aim of this helper function is to URI-encode given string
twice. There's a bug (fixed in next commit) in which we're unable
to fetch .vmx file for a domain if corresponding datastore
contains some special characters (like +). Cole Robinson
discovered that encoding datastore twice enables libvirt to work
around the issue [2]. Well, this function does exactly that.
It was tested with the following inputs and all worked
flawlessly: "datastore", "datastore2", "datastore2+",
"datastore3+-@", "data store2+".
1: https://issues.redhat.com/browse/RHEL-134127
2: https://issues.redhat.com/browse/RHEL-133729#comment-28604072
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
The original commit (v11.9.0-rc1~84) added a dependency checking
of VIR_DOMAIN_HYPERV_STIMER on VIR_DOMAIN_HYPERV_VPINDEX
(meaning, if stimer is on then vpindex must also be on). It
justified this by citing QEMU documentation:
Per QEMU documentation (docs/system/i386/hyperv.rst):
``hv-stimer``
Enables Hyper-V synthetic timers. <snip/>
Requires: ``hv-vpindex``, ``hv-synic``, ``hv-time``
While the documentation is almost correct (see previous commit
when it's incorrect), the code express no dependency on vpindex
(kvm_hyperv_properties[] array from target/i386/kvm/kvm.c):
[HYPERV_FEAT_STIMER] = {
.desc = "synthetic timers (hv-stimer)",
.flags = {
{.func = HV_CPUID_FEATURES, .reg = R_EAX,
.bits = HV_SYNTIMERS_AVAILABLE}
},
.dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_TIME)
},
If transitivity is taken into account then the documentation is
of course correct (minus that one aforementioned special case).
Well, there's no need for us to implement transitional checks.
VIR_DOMAIN_HYPERV_STIMER requires VIR_DOMAIN_HYPERV_SYNIC and
whether that requires VIR_DOMAIN_HYPERV_VPINDEX is another
question.
Just drop the transitive check.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/837
Resolves: https://issues.redhat.com/browse/RHEL-138689
Fixes: da261327ea
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Turns out, that synic hyperv enlightenment not always requires
vpindex. Some (older) machine types (e.g. pc-i440fx-3.0,
pc-q35-3.0, pc-i440fx-rhel7.6.0) can run with synic enabled and vpindex
disabled. This is because they did enable 'x-hv-synic-kvm-only'
CPU property, but starting from QEMU commit v3.1.0-rc0~44^2~9 the
property is disabled by default.
To avoid parsing machine type version, let's just drop this
dependency validation and rely on QEMU to report sensible error
message.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/837
Resolves: https://issues.redhat.com/browse/RHEL-138689
Fixes: 1822d030c3
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
In the <dns/> section of network configuration users can set up
forwarding of DNS requests to custom DNS servers. These are
specified using 'addr' attribute. But configuring port wasn't
possible, until now. New 'port' attribute is introduced, which
allows overriding the default DNS port for given address.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
A memory device can have <nodemask/> which specifies which host
NUMA nodes the memory should be allocated from (currently
supported for dimm, virtio-mem and sgx-epc models).
But when generating corresponding command line for the device,
the NUMA policy is taken from the guest NUMA node that
corresponds to the memory device (as defined by target/node) or
overall domain NUMA policy (as defined by <numatune/>).
This may lead to memory being allocated from unexpected NUMA
node. For instance, if the memory device has
<nodemask>0</nodemask> and domain has <numatune> <memory
mode='preferred' nodeset='1'/> </numatune> then the cmd line for
the memory device also has just "policy":"preferred".
Treat <nodemask/> as mode='strict'.
But I agree that this is kind of nonsense configuration. Why
would somebody want to prefer one NUMA node but then configure
memory device to allocate NUMA from the other?
Resolves: https://issues.redhat.com/browse/RHEL-114415
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's new commit in glibc [1] which makes memchr(), strchr(),
strrchr(), strpbrk() and strstr() reflect type of the input
string. If it's a constant string, then the return type of these
functions is also 'const char *'. But this change tickles
-Wincompatible-pointer-types-discards-qualifiers warning.
And indeed, there are some places where we use a 'char *' typed
variable to store the retval, or even misuse the fact 'char *' is
returned and modify const string.
To fix this, a couple of different approaches is used:
a) switch variable type to 'const char *',
b) switch argument to 'char *' (in a few places we have
strdup()-ed) the const string already,
c) strdup() the string and use b).
1: https://sourceware.org/git/?p=glibc.git;a=commit;h=cd748a63ab1a7ae846175c532a3daab341c62690
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Jaroslav Suchanek <jsuchane@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The iptablesPrivateChainCreate() function is given an array of
const strings. This constitutes a promise to the caller that the
data is not modified. But inside the data is modified anyway (to
cut out some parts of the data). Well, with a help from
g_strdup() the promise can be kept.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Jaroslav Suchanek <jsuchane@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The body of iptablesPrivateChainCreate() uses STRPREFIX() to
match strings starting with certain prefix. Then it uses pointer
arithmetic to skip the prefix. Well, that's exactly what
STRSKIP() is meant to do. Switch the body to use the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Jaroslav Suchanek <jsuchane@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The iptablesPrivateChainCreate() function gets a NULL terminated
array of strings (@lines argument), each item representing one
line of iptables output. Currently, the variable used to iterate
over the array is named 'tmp' which is not very descriptive.
Rename it to 'line'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Jaroslav Suchanek <jsuchane@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The aim of virSkipSpacesBackwards() is find the first space
trailing character in given string, For instance, if the input is
"Something whitespacey ", then the output should be pointing to
the very first space after "y".
Problem here is that the input string is constant, but the
returned pointer is non-constant. This is confusing, a caller
shouldn't be able to modify the string, since the input was a
constant string.
Therefore, make the function return a const pointer too.
Under the hood the function used virTrimSpaces() which under some
circumstances could modify the input string. A trick was used to
hide this fact away, but to be double sure rewrite the function's
body.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Jaroslav Suchanek <jsuchane@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When the network driver starts up it may inject some firewall
rules (e.g. for a network with NAT). So far, this scenario wasn't
covered in our test suite. The reason for adding this test is
twofold: the first, check we add correct rules, the second is to
cover iptablesPrivateChainCreate() as its implementation is soon
to be changed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Jaroslav Suchanek <jsuchane@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The signature and implementation of virSkipSpacesBackwards() is
soon about to change. Introduce a test case to make sure its
behaviour stays the same.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Jaroslav Suchanek <jsuchane@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Throughout years, virCommandSetDryRun() has gained more
functionality and arguments. But:
1) not all arguments are covered in documentation,
2) the example wouldn't even compile.
Expand the documentation to reflect current behaviour.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Jaroslav Suchanek <jsuchane@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are some features/improvements/bug fixes I've either
contributed or reviewed/merged. Document them for upcoming
release.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
When reworking the vmx2xmltest to call esxParseVMXFileName() from
the ESX driver I also made the test link with the driver
statically. But the function then calls some other functions
which are mocked in vmx2xmlmock. Now, on many systems this works
just fine as the dynamic linker finds the mocked functions first.
But on Fedora 41 and Fedora 42 the dynamic linker resolves the
symbols to those from statically linked library rendering our
mock ineffective.
Just don't link in the esx_lib.
Fixes: f82d30307d
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This test case demonstrates correctness of the previous fix.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The esxParseVMXFileName() function parses path to a disk image
trying to replace some "known" patterns (e.g. datastore paths).
A simple filename is treated as a path relative to .vmx file. But
disk images (and thus filenames) can be in a subdirectory,
relative to the .vmx file. For instance:
subfolder/disk.vmdk
Adapt our parser to this fact.
Resolves: https://issues.redhat.com/browse/RHEL-122751
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Having a custom file name parsing function in vmx2xml that's
different to the one used in production (esxParseVMXFileName())
might have served us well, but it also defeats the point of
having a unit test. More specifically, if there's a bug in
esxParseVMXFileName() then our unit test would not catch it.
But now that we have vmx2xmlmock the custom parsing function can
be dropped and the test can use the real one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
If we want vmx2xmltest to use actual file name parser that's used
in production (esxParseVMXFileName()) we need a mock to stop it
from doing any HTTP requests and also to return predictable data.
So far, the function can call three functions that do HTTP
requests: esxVI_LookupDatastoreList(),
esxVI_LookupDatastoreHostMount() and
esxVI_LookupDatastoreByName().
Mock all three of them. And since their implementation uses some
other symbols (like allocators or _AppendToList() helpers) we
need to expose these symbols too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This function is going to be mocked soon. Annotate and export it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This function is going to be mocked soon. Annotate and export it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This function is going to be mocked soon. Annotate and export it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
So far, our vmx2xmltest uses a custom .parseFileName callback.
And it kind of makes sense because the one that's used in
production (esxParseVMXFileName()) does some HTTP requests which
we don't want to do in our test suite. But this creates other
sorts of problems and the idea is to have the test ditch custom
parse callback and stick with the production one. But for now,
just expose it. With it, the esxVMX_Data struct is exposed too as
it is passed into the function (via 'opaque' argument).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The esxVI_DateTime_ConvertToCalendarTime() symbol is declared in
esx_vi_types.h header file. Reflect this in the corresponding
.syms file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Firstly, there's no need to list header files in
ch_driver_sources (we don't do that anywhere else, and meson is
smart enough to figure them out). And secondly, the list of
source file is not sorted which means new source files are added
in random order.
Thus, drop header files from the list and sort it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Assigning device should happen from ch_hotplug.c (just like it's
done for disks currently) not in ch_process.c. Move alias
assignment out of chProcessAddNetworkDevice(). And while at it,
mimic what's done with disks and have net hotplug handling done
from a function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Libvirt's philosophy is that for a running domain there are two
(in general distinct) definitions: live definition (reflects the
running state) and inactive definition (used to seed the live
definition when domain is being created). That's why we have
VIR_DOMAIN_AFFECT_LIVE and VIR_DOMAIN_AFFECT_CONFIG flags to APIs
that modify domain definitions.
Well, the CH driver doesn't do this distinction. Fix this by
making the domain definition transient when it's being created.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The chDomainGetXMLDesc() function claims to support
VIR_DOMAIN_XML_INACTIVE to obtain the persistent definition of a
running domain (in its call to virCheckFlags()) but in fact, it's
always passing vm->def to virDomainDefFormat().
So far, there's no harm done because CH driver never sets domain
def as transient. But that'll change.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The 'payload' variable inside of chProcessAddNetworkDevice() is
reused and thus the memory it points to just before its
repurpose is not freed. Avoid reusing g_autofree variables.
128 bytes in 1 blocks are definitely lost in loss record 1,828 of 2,026
at 0x491A120: realloc (vg_replace_malloc.c:1801)
by 0x4FEC251: g_realloc (in /usr/lib64/libglib-2.0.so.0.8400.4)
by 0x500BB7E: g_string_expand (in /usr/lib64/libglib-2.0.so.0.8400.4)
by 0x500BBF0: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.8400.4)
by 0x4A114C0: virBufferInitialize (virbuffer.c:121)
by 0x4A11890: virBufferAdd (virbuffer.c:160)
by 0x4A67344: virJSONValueToBuffer (virjson.c:1562)
by 0x4A673DB: virJSONValueToString (virjson.c:1599)
by 0xBC878AB: virCHMonitorBuildNetJson (ch_monitor.c:466)
by 0xBC8D4A9: chProcessAddNetworkDevice (ch_process.c:688)
by 0xBC8FCE2: chDomainAttachDeviceLive (ch_hotplug.c:78)
by 0xBC900CA: chDomainAttachDeviceLiveAndUpdateConfig (ch_hotplug.c:174)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Domain capabilities XML is formatted (mostly) using
FORMAT_PROLOGUE and FORMAT_EPILOGUE macros. These format opening
and closing stanzas for given element. The FORMAT_PROLOGUE macro
even tries to be clever and format element onto one line (if the
element isn't supported), but that's not enough. Fortunately, we
have virXMLFormatElement() which formats elements properly, so
let's switch macros into using that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In the virDomainCaps struct there are some pointers that might be
NULL (for instance 'sev', 'sgx', 'hyperv'). Teach FORMAT_PROLOGUE
macro to check for NULL argument so that format functions (like
virDomainCapsFeatureHypervFormat()) don't need to.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik