Fabiano Fidêncio [Fri, 20 Dec 2019 12:43:14 +0000 (13:43 +0100)]
util: Get rid of "no_memory" labels
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of
abort()".
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of the no_memory labels and simplify the
code around them.
Mind that virfirewall.c was not touched and still contains no_memory
labels. The reason those are left behind, at least for now, is because
the conversion seems to be slightly more complicated than the rest, as
some other places are relying on firewall->err being set to ENOMEM.
Fabiano Fidêncio [Fri, 20 Dec 2019 12:43:13 +0000 (13:43 +0100)]
rpc: Get rid of "no_memory" labels
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of
abort()".
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of the no_memory labels and simplify the
code around them.
Fabiano Fidêncio [Fri, 20 Dec 2019 12:43:12 +0000 (13:43 +0100)]
openvz: Get rid of "no_memory" labels
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of
abort()".
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of the no_memory labels and simplify the
code around them.
Fabiano Fidêncio [Fri, 20 Dec 2019 12:43:11 +0000 (13:43 +0100)]
conf: Get rid of "no_memory" labels
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of
abort()".
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of the no_memory labels and simplify the
code around them.
Cole Robinson [Wed, 18 Dec 2019 15:23:34 +0000 (10:23 -0500)]
Remove phyp driver
The phyp driver was added in 2009 and does not appear to have had any
real feature change since 2011. There's virtually no evidence online
of users actually using it. IMO it's time to kill it.
This was discussed a bit in April 2016:
https://www.redhat.com/archives/libvir-list/2016-April/msg01060.html
Final discussion is here:
https://www.redhat.com/archives/libvir-list/2019-December/msg01162.html
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Cole Robinson <crobinso@redhat.com>
As part of a goal to eliminate Perl from libvirt build tools,
rewrite the genaclperms.pl tool in Python.
This was a straight conversion, manually going line-by-line to
change the syntax from Perl to Python. Thus the overall structure
of the file and approach is the same.
Tested-by: Cole Robinson <crobinso@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
docs: rewrite hvsupport.html page generator in python
As part of a goal to eliminate Perl from libvirt build tools,
rewrite the hvsupport.pl tool in Python.
This was a straight conversion, manually going line-by-line to
change the syntax from Perl to Python. Thus the overall structure
of the file and approach is the same.
The new impl generates byte-for-byte identical output to the
old impl.
Tested-by: Cole Robinson <crobinso@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
As part of a goal to eliminate Perl from libvirt build tools,
rewrite the check-file-access.pl tool in Python.
This was a straight conversion, manually going line-by-line to
change the syntax from Perl to Python. Thus the overall structure
of the file and approach is the same.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
As part of a goal to eliminate Perl from libvirt build tools,
rewrite the group-qemu-caps.pl tool in Python.
This was a straight conversion, manually going line-by-line to
change the syntax from Perl to Python. Thus the overall structure
of the file and approach is the same.
Tested-by: Cole Robinson <crobinso@redhat.com> Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Pino Toscano [Thu, 14 Nov 2019 10:19:13 +0000 (11:19 +0100)]
esx: split virtualswitchToNetwork helper
Move the creation of a virNetworkPtr object from the
esxVI_HostVirtualSwitch object of a virtual switch out of
esxNetworkLookupByName in an own helper. This way it can be used also
in other functions.
Pino Toscano [Tue, 12 Nov 2019 17:16:48 +0000 (18:16 +0100)]
esx: split targetToStoragePool helper
Move the creation of a virStoragePtr object from the
esxVI_HostInternetScsiHbaStaticTarget object of a target out of
esxStoragePoolLookupByName in an own helper. This way it can be used
also in other functions.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pino Toscano [Mon, 11 Nov 2019 15:34:34 +0000 (16:34 +0100)]
esx: split datastoreToStoragePoolPtr helper
Move the creation of a virStoragePtr object from the esxVI_ObjectContent
object of a datastore out of esxStoragePoolLookupByName in an own
helper. This way it can be used also in other functions.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
John Ferlan [Tue, 17 Dec 2019 15:43:24 +0000 (10:43 -0500)]
virsh: Adjust logic checks in virshUpdateDiskXML
Make it clearer that what we're trying to do is find @source and
@target_node so that the unattentive or code analysis utility
doesn't believe 'source' and 'target' could be found in the same
node element.
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
John Ferlan [Mon, 16 Dec 2019 12:36:32 +0000 (07:36 -0500)]
vbox: Reset @ret after xmlFreeNode
In the error path, if we xmlFreeNode @ret, then the return ret;
a few lines later returns something that's already been free'd
and could be reused, so let's reinit it.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
John Ferlan [Sat, 14 Dec 2019 15:37:57 +0000 (10:37 -0500)]
conf: Fix ATTRIBUTE_NONNULL usages
Recent changes removed the virCapsPtr, but didn't adjust/remove the
corresponding ATTRIBUTE_NONNULL resulting in a build failure to build
in my Coverity environment.
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Fabiano Fidêncio [Wed, 18 Dec 2019 20:13:16 +0000 (21:13 +0100)]
util: Remove VIR_FILE_*_SEPARATOR*
None of those are used and we should prefer using the ones provided by
GLib, as G_DIR_SEPARATOR, G_DIR_SEPARATOR_S, G_SEARCHPATH_SEPARATOR, and
G_SEARCHPATH_SEPARATOR_S.
qemu: handle unassigned PCI hostdevs in command line
Previous patch made it possible for the QEMU driver to check if
a given PCI hostdev is unassigned, by checking if dev->info->type is
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED, meaning that this device
shouldn't be part of the actual guest launch.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Introducing new address type='unassigned' for PCI hostdevs
This patch introduces a new PCI hostdev address type called
'unassigned'. This new type gives users the option to add
PCI hostdevs to the domain XML in an 'unassigned' state, meaning
that the device exists in the domain, is managed by Libvirt
like any regular PCI hostdev, but the guest does not have
access to it.
This adds extra options for managing PCI device binding
inside Libvirt, for example, making all the managed PCI hostdevs
declared in the domain XML to be detached from the host and bind
to the chosen driver and, at the same time, allowing just a
subset of these devices to be usable by the guest.
Next patch will use this new address type in the QEMU driver to
avoid adding unassigned devices to the QEMU launch command line.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemu: command: move validation of vmcoreinfo to qemu_domain.c
Move the validation of vmcoreinfo from qemuBuildVMCoreInfoCommandLine()
to qemuDomainDefValidateFeatures(), allowing for validation
at domain define time.
qemuxml2xmltest.c was changed to account for this caps being
now validated at this earlier stage.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemu: command: move qemuBuildSmartcardCommandLine validation to qemu_domain.c
Move smartcard validation being done by qemuBuildSmartcardCommandLine()
to the existing qemuDomainSmartcardDefValidate() function. This
function is called by qemuDomainDeviceDefValidate(), allowing smartcard
validation in domain define time.
Tests were adapted to consider the new caps being needed in
this earlier stage.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemu: command: move qemuBuildGraphicsEGLHeadlessCommandLine validation to qemu_domain.c
Move EGL Headless validation from qemuBuildGraphicsEGLHeadlessCommandLine()
to qemuDomainDeviceDefValidateGraphics(). This function is called by
qemuDomainDefValidate(), validating the graphics parameters in domain
define time.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemu: command: move NVDIMM validation to qemu_domain.c
Move the NVDIMM validation from qemuBuildMachineCommandLine()
to a new function in qemu_domain.c, qemuDomainDeviceDefValidateMemory(),
which is called by qemuDomainDeviceDefValidate(). This allows
NVDIMM validation to occur in domain define time.
It also increments memory hotplug validation, which can be seen
by the failures in the hotplug tests in qemuxml2xmltest.c that
needed to be adjusted after the move.
Reviewed-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
conf: fix populating of fake NUMA in multi-node hosts
If the host OS doesn't have NUMA present, we fallback to
populating fake NUMA info and the code thus assumes only a
single NUMA node.
Unfortunately we also fallback to fake NUMA if numactl-devel
was not present, and in this case we can still have multiple
NUMA nodes. In this case we create all CPUs, but only the
CPUs in the first node have any data filled in, resulting in
capabilities like:
The topology at least now reflects what 'virsh nodeinfo' reports.
The main bug is that the CPU "id" values won't match what the Linux
host actually uses.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
qemu_command: tidy up qemuBuildHostdevCommandLine loop
The current 'for' loop with 5 consecutive 'ifs' inside
qemuBuildHostdevCommandLine can be a bit smarter:
- all 5 'ifs' fails if hostdev->mode is not equal to
VIR_DOMAIN_HOSTDEV_MODE_SUBSYS. This check can be moved to the
start of the loop, failing to the next element immediately
in case it fails;
- all 5 'ifs' checks for a specific subsys->type to build the proper
command line argument (virHostdevIsSCSIDevice and virHostdevIsMdevDevice
do that but within a helper). Problem is that the code will keep
checking for matches even if one was already found, and there is
no way a hostdev will fit more than one 'if' (i.e. a hostdev can't
have 2+ different types). This means that a SUBSYS_TYPE_USB will
create its command line argument in the first 'if', then all other
conditionals will surely fail but will end up being checked anyway.
All of this can be avoided by moving the hostdev->mode comparing
to the start of the loop and using a switch statement with
subsys->type to execute the proper code for a given hostdev
type.
Suggested-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Wed, 18 Dec 2019 05:39:09 +0000 (06:39 +0100)]
qemu: Reorder cleanup in qemuStateCleanup()
This function is supposed to clean up virQEMUDriver structure and
free individual members. However, it's doing that in random order
which makes it hard to track which members are being freed and
which are not. Do the free in reverse order than the structure
definition - assuming that the most important members (like
mutex) are declared first and freed last.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Wed, 18 Dec 2019 06:40:17 +0000 (07:40 +0100)]
virCapabilitiesHostNUMAUnref: Accept NULL
Fortunately, this is not causing any problems now because glib
does this check for us when calling this function via attribute
cleanup. But in a future commit we will explicitly call this
function over a struct member that might be NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Wed, 11 Dec 2019 09:17:30 +0000 (10:17 +0100)]
testutilsxen: Avoid double free of driver caps
In testXLInitDriver() a dummy driver structure is filled and it
is freed later in testXLFreeDriver(). However, it is sufficient
to unref just driver->config because that results in
libxlDriverConfigDispose() being called which unrefs
driver->config->caps. There is no need to unref it again in
testXLFreeDriver() - in fact it's undesired.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Wed, 11 Dec 2019 09:13:15 +0000 (10:13 +0100)]
domaincapstest: Don't leak cpu definitions
When generating domain capabilities, we need to fake host CPU to
get reproducible result. We do this by copying a pre-existent CPU
config and setting VIR_TEST_MOCK_FAKE_HOST_CPU env variable which
is then consumed by qemucpumock. However, we forget to free the
CPU copy afterwards.
2,196 (2,016 direct, 180 indirect) bytes in 18 blocks are definitely lost in loss record 291 of 297
at 0x4838B86: calloc (vg_replace_malloc.c:762)
by 0x57CB6A0: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6000.7)
by 0x4A0F72D: virCPUDefNew (cpu_conf.c:87)
by 0x4A0FAC7: virCPUDefCopyWithoutModel (cpu_conf.c:235)
by 0x4A0FBBE: virCPUDefCopy (cpu_conf.c:273)
by 0x10E3C0: testUtilsHostCpusGetDefForArch (testutilshostcpus.h:157)
by 0x10E3C0: fakeHostCPU (domaincapstest.c:61)
by 0x10E3C0: fillQemuCaps (domaincapstest.c:86)
by 0x10E3C0: test_virDomainCapsFormat (domaincapstest.c:234)
by 0x10F4BC: virTestRun (testutils.c:146)
by 0x10DE93: doTestQemuInternal (domaincapstest.c:301)
by 0x10E13D: doTestQemu (domaincapstest.c:332)
by 0x1124CF: testQemuCapsIterate (testutilsqemu.c:635)
by 0x10DCE3: mymain (domaincapstest.c:435)
by 0x10FD8B: virTestMain (testutils.c:916)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Peter Krempa [Tue, 17 Dec 2019 16:04:04 +0000 (17:04 +0100)]
util: storage: Don't treat files with missing backing store format as 'raw'
Assuming that the backing image format is raw is wrong when doing image
detection:
1) In -drive mode qemu will still probe the image format of the backing
image. This means it will try to open a backing file of the image
which will fail if a more advanced security model is in use.
2) In blockdev mode the image will be opened as raw actually which is
wrong since it might be qcow. Not opening the backing images will
also end up in the guest seeing corrupted data.
Rather than attempt to solve various corner cases when us assuming the
storage file being raw and actually being right forbid startup when the
guest image doesn't have the format specified in the metadata.
Peter Krempa [Tue, 17 Dec 2019 16:46:18 +0000 (17:46 +0100)]
tests: storage: Use strict version of virStorageFileGetMetadata
Pass in 'true' as '@report_broken' of virStorageFileGetMetadata to make
it fail in the tests. The most important code paths (when starting the
VM) expect this function to fail rather than silently return partial
data. Switch the test to exercise this more important code path.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Tue, 17 Dec 2019 15:24:00 +0000 (10:24 -0500)]
qemu: homogenize MAC address in live & config when hotplugging a netdev
Prior to commit 55ce6564634 (first in libvirt 4.6.0), the XML sent to
virDomainAttachDeviceFlags() was parsed only once, and the results of
that parse were inserted into both the live object of the running
domain and into the persistent config. Thus, if MAC address was
omitted from in XML for a network device (<interface>), both the live
and config object would have the same MAC address.
Commit 55ce6564634 changed the code to parse the incoming XML twice -
once for live and once for config. This does eliminate the problem of
PCI (/scsi/sata) address conflicts caused by allocating an address
based on existing devices in live object, but then inserting the
result into the config (which may already have a device using that
address), BUT it also means that when the MAC address of a network
device hasn't been specified in the XML, each copy will get a
different auto-generated MAC address.
This results in the MAC address of the device changing the next time
the domain is shutdown and restarted, which creates havoc with the
guest OS's network config.
There have been several discussions about this in the last > 1 year,
attempting to find the ideal solution to this problem that makes MAC
addresses consistent and accounts for all sorts of corner cases with
PCI/scsi/sata addresses. All of these discussions fizzled out because
every proposal was either too difficult to implement or failed to fix
some esoteric case someone thought up.
So, in the interest of solving the MAC address problem while not
making the "other address" situation any worse than before, this patch
simply adds a qemuDomainAttachDeviceLiveAndConfigHomogenize() function
that (for now) copies the MAC address from the config object to the
live object (if the original xml had <mac address='blah'/> then this
will be an effective NOP (as the macs already match)).
Any downstream libvirt containing upstream commit 55ce6564634 should have this patch as well.