Our documentation states that the chardev logging file is truncated
unless append='on' is specified. QEMU also behaves the same way and
truncates the file unless we provide the argument. The new virlogd
implementation did not honor if the argument was missing and continued
to append to the file.
Truncate the file even when the 'append' attribute is not present to
behave the same with both implementations and adhere to the docs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1420205
This function is calling public APIs (virNodeDeviceLookupByName
etc.). That requires the driver lock to be unlocked and locked
again. If we, however, replace the public APIs calls with the
internal calls (that public APIs call anyway), we can drop the
lock/unlock exercise.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The 'nodes' is overwritten after the first usage and possibly leaked
if any code in the first set of parsing goes to error.
Found by Coverity.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Arguably though, function returning only on success is a very
interesting, although quite impractical concept. Also, the errno isn't
and shouldn't be preserved in this case, since the errno can be directly
fed to the virReportSystemError.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
After eca76884ea in case of error in qemuDomainSetPrivatePaths()
in pretended start we jump to stop. I've changed this during
review from 'cleanup' which turned out to be correct. Well, sort
of. We can't call qemuProcessStop() as it decrements
driver->nactive and we did not increment it. However, it calls
virDomainObjRemoveTransientDef() which is basically the only
function we need to call. So call that function and goto cleanup;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The API is useful for creating virCPUData in a hypervisor driver from
data we got by querying the hypervisor.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The API is useful for creating virCPUData in a hypervisor driver from
data we got by querying the hypervisor.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The API is useful for creating virCPUData in a hypervisor driver from
data we got by querying the hypervisor.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The CPU driver provides APIs to create and free virCPUDataPtr. Thus all
APIs exported from the driver should work with that rather than
requiring the caller to pass a pointer to an internal part of the
structure.
In other words
virCPUx86DataAddCPUID(cpudata, &cpuid)
is much better than the original
virCPUx86DataAddCPUID(&cpudata->data.x86, &cpuid)
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The new API is called virCPUDataFree. Individual CPU drivers are no
longer required to implement their own freeing function unless they need
to free architecture specific data from virCPUData.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Our documentation of the domain capabilities XML says that the fallback
attribute of a CPU model is used to indicate whether the CPU model was
detected by libvirt itself (fallback="allow") or by asking the
hypervisor (fallback="forbid"). We need to properly set
fallback="forbid" when CPU model comes from QEMU to match the
documentation.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This will eventually replace virQEMUBuildBufferEscapeComma, however
it's not possible right now. Some parts of the code that uses the
old function needs to be refactored.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The port is stored in graphics configuration and it will
also get released in qemuProcessStop in case of error.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1397440
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Previously the code called virStorageSourceUpdateBlockPhysicalSize which
did not do anything on empty drives since it worked only on block
devices. After the refactor in c5f6151390 it's called for all devices
and thus attempts to deref the NULL path of empty drives.
Add a check that skips the update of the physical size if the storage
source is empty.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1420718
Fix incorrect jump labels in error paths as the stop jump is only
needed if the driver has already changed the state. For example
'virAtomicIntInc(&driver->nactive)' will be 'reverted' in the
qemuProcessStop call.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When a domain needs an access to some device (be it a disk, RNG,
chardev, whatever), we have to allow it in the devices CGroup (if
it is available), because by default we disallow all the devices.
But some of the functions that are responsible for setting up
devices CGroup are lacking check whether there is any CGroup
available. Thus users might be unable to hotplug some devices:
virsh # attach-device fedora rng.xml
error: Failed to attach device from rng.xml
error: internal error: Controller 'devices' is not mounted
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In GCC 7 there is a new warning triggered when a switch
case has a conditional statement (eg if ... else...) and
some of the code paths fallthrough to the next switch
statement. e.g.
conf/domain_conf.c: In function 'virDomainChrEquals':
conf/domain_conf.c:14926:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
if (src->targetTypeAttr != tgt->targetTypeAttr)
^
conf/domain_conf.c:14928:5: note: here
case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
^~~~
conf/domain_conf.c: In function 'virDomainChrDefFormat':
conf/domain_conf.c:22143:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
if (def->targetTypeAttr) {
^
conf/domain_conf.c:22151:5: note: here
default:
^~~~~~~
GCC introduced a __attribute__((fallthrough)) to let you
indicate that this is intentionale behaviour rather than
a bug.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
One of the conditions in qemuDomainDeviceCalculatePCIConnectFlags
was missing a break that could result it in falling through to
an incorrect codepath.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The libxl code was checking that a 'char *' was != '\0', instead
of checking the first element in the string
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
GCC 7 gets upset by
if (!tmp && (size * count))
warning
util/viralloc.c: In function 'virReallocN':
util/viralloc.c:246:23: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context]
if (!tmp && (size * count)) {
~~~~~~^~~~~~~~
Keep it happy by adding != 0 to the right hand expression
so it realizes we really are wanting to treat the result
of the arithmetic expression as a boolean
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Current code for example can call unsubscribe if connection
succeeds but subscribing fails. This will probabaly lead
only to spurious error messages without any actual inconsistencies
but nevertheless.
qemuDomainAssignPCIAddresses() hardcoded the assumption
that the only way to support devices on a non-zero bus is
to add one or more pci-bridges; however, since we now
support a large selection of PCI controllers that can be
used instead, the assumption is no longer true.
Moreover, this check was always redundant, because the
only sensible time to check for the availability of
pci-bridge is when building the QEMU command line, and
such a check is of course already in place.
In fact, there were *two* such checks, but since one of
the two was relying on the incorrect assumption explained
above, and it was redundant anyway, it has been dropped.
When switching over the values in the virDomainControllerModelPCI
enumeration, make sure the proper cast is in place so that the
compiler can warn us when the coverage is not exaustive.
For the same reason, remove the 'default' case from one of the
existing switch statements.
When switching over the values in the virDomainControllerModelPCI
enumeration, make sure the proper cast is in place so that the
compiler can warn us when the coverage is not exaustive.
For the same reason, fold some unstructured checks (performed by
comparing directly against some values in the enumeration) inside
an existing switch statement.
The switch in virDomainPCIControllerModelToConnectType()
had some code that, while techically part of the
_PCIE_SWITCH_DOWNSTREAM_PORT case, was in fact dead due
to the early return.
Get rid of the dead code, and fix the inaccurate function
description while at it.
virCPUDef.arch is not required to be filled in for guest CPU
definitions. It doesn't make sense to artificially mandate it to be set
when cpuDecode is called especially when virCPUData.arch passed to
cpuDecode already contains the architecture.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The CPU model info formating code in virQEMUCapsFormatCache will get
more complicated soon. Separating the code in
virQEMUCapsFormatHostCPUModelInfo will make the result easier to read.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
All features the function is currently supposed to filter out are
specific to x86_64. We should avoid removing them on other
architectures. It seems to be quite unlikely other achitectures would
use the same names, but one can never be sure.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The 'raw' block driver in Qemu is not directly interesting from
libvirt's perspective, but it can be layered above some other block
drivers and this may be interesting for the user.
The patch adds support for the 'raw' block driver. The driver is treated
simply as a pass-through and child driver in JSON is queried to get the
necessary information.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Split virStorageSourceParseBackingJSON into two functions so that the
core can be reused by other functions. The new function called
virStorageSourceParseBackingJSONInternal accepts virJSONValuePtr.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Add a new storage driver registration function that will force the
backend code to fail if any of the storage backend modules can't be
loaded. This will make sure that they work and are present.
If driver modules are enabled turn storage driver backends into
dynamically loadable objects. This will allow greater modularity for
binary distributions, where heavyweight dependencies as rbd and gluster
can be avoided by selecting only a subset of drivers if the rest is not
necessary.
The storage modules are installed into 'LIBDIR/libvirt/storage-backend/'
and users can override the location by using
'LIBVIRT_STORAGE_BACKEND_DIR' environment variable.
rpm based distros will at this point install all the backends when
libvirt-daemon-driver-storage package is installed.
Our virSomeEnumTypeFromString() functions return either the value
of item from the enum or -1 on error. Usually however the value 0
means 'this value is not set in the domain XML, use some sensible
default'. Therefore, we don't accept corresponding string in
domain XML, for instance:
<memoryBacking>
<source mode="none"/>
<access mode="default"/>
<allocation mode="none"/>
</memoryBacking>
should be rejected as invalid XML.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Rather than the inlined VIR_FREE's, use a cleanup: label... Fixes an
issue introduced by 03346def where @name was free'd before usage in
a virAsprintf to format scsi_host_name.
The functions in virCommand() after fork() must be careful with regard
to accessing any mutexes that may have been locked by other threads in
the parent process. It is possible that another thread in the parent
process holds the lock for the virQEMUDriver while fork() is called.
This leads to a deadlock in the child process when
'virQEMUDriverGetConfig(driver)' is called and therefore the handshake
never completes between the child and the parent process. Ultimately
the virDomainObjectPtr will never be unlocked.
It gets much worse if the other thread of the parent process, that
holds the lock for the virQEMUDriver, tries to lock the already locked
virDomainObject. This leads to a completely unresponsive libvirtd.
It's possible to reproduce this case with calling 'virsh start XXX'
and 'virsh managedsave XXX' in a tight loop for multiple domains.
This commit fixes the deadlock in the same way as it is described in
commit 61b52d2e38.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Similarly to domainSetGuestVcpus this commit adds API which allows to
modify state of individual vcpus rather than just setting the count.
This allows to enable CPUs in specific guest NUMA nodes to achieve any
necessary configuration.
With that users could access files outside /dev/shm. That itself
isn't a security problem, but might cause some errors we want to
avoid. So let's forbid slashes as we do with domain and volume names
and also mention that in the schema.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1395496
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Add APIs that allow to dynamically register driver backends so that the
list of available drivers does not need to be known during compile time.
This will allow us to modularize the storage driver on runtime.
Pass the registration function name to virDriverLoadModule so that we
can later call specific functions if necessary (e.g. for testing
purposes). This gets rid of the rather ugly automatic name generator and
unifies the code to load/initialize the modules.
It's also clear which registration function gets called.
The niothreadids struct field is size_t, so must use %zu not %lu
with printf. While they're identical on some platforms, on others
they are different, causing warnings
conf/domain_conf.c: In function 'virDomainDefCheckABIStabilityFlags':
conf/domain_conf.c:19575:26: error: format '%lu' expects argument of type 'long unsigned int', but argument 7 has type 'size_t {aka unsigned int}' [-Werror=format=]
_("Target domain iothreads count %lu does not "
^
conf/domain_conf.c: In function 'virDomainDefFormatInternal':
conf/domain_conf.c:23915:46: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t {aka unsigned int}' [-Werror=format=]
virBufferAsprintf(buf, "<iothreads>%lu</iothreads>\n",
^
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This follows the same check for disk, because we cannot remove iothread
if it's used by disk or by controller. It could lead to crashing QEMU.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
If virDomainDelIOThread API was called with VIR_DOMAIN_AFFECT_LIVE
and VIR_DOMAIN_AFFECT_CONFIG and both XML were already a different
it could result in removing iothread from config XML even if there
was a disk using that iothread.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The situation covered by the removed code will not ever happen.
This code is called only while starting a new QEMU process where
the capabilities where already checked and while attaching to
existing QEMU process where we don't even detect the iothreads.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The utils code should stay separated from other code (except for very
well justified cases). Unfortunately commit 272769becc
made it trivial to break the separation (and not get slapped by the
syntax-check rule) by adding -I src/conf to the CFLAGS for utils.
Remove this shortcut and except the two offenders from the syntax check
so that the codebase can be kept separated.
The comment was actually wrong as
https://www.freedesktop.org/software/systemd/man/udev_new.html#
mentions that on failure NULL is returned. Also the same return value
is checked in src/interface/interface_backend_udev.c already.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
When enabling virgl, qemu opens /dev/dri/render*. So far, we are
not allowing that in devices CGroup nor creating the file in
domain's namespace and thus requiring users to set the paths in
qemu.conf. This, however, is suboptimal as it allows access to
ALL qemu processes even those which don't have virgl configured.
Now that we have a way to specify render node that qemu will use
we can be more cautious and enable just that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
So far, qemuDomainGetHostdevPath has no knowledge of the reasong
it is called and thus reports /dev/vfio/vfio for every VFIO
backed device. This is suboptimal, as we want it to:
a) report /dev/vfio/vfio on every addition or domain startup
b) report /dev/vfio/vfio only on last VFIO device being unplugged
If a domain is being stopped then namespace and CGroup die with
it so no need to worry about that. I mean, even when a domain
that's exiting has more than one VFIO devices assigned to it,
this function does not clean /dev/vfio/vfio in CGroup nor in the
namespace. But that doesn't matter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
So far, we are allowing /dev/vfio/vfio in the devices cgroup
unconditionally (and creating it in the namespace too). Even if
domain has no hostdev assignment configured. This is potential
security hole. Therefore, when starting the domain (or
hotplugging a hostdev) create & allow /dev/vfio/vfio too (if
needed).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Since these two functions are nearly identical (with
qemuSetupHostdevCgroup actually calling virCgroupAllowDevicePath)
we can have one function call the other and thus de-duplicate
some code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
There's no need for this function. Currently it is passed as a
callback to virSCSIVHostDeviceFileIterate(). However, SCSI host
devices have just one file path. Therefore we can mimic approach
used in qemuDomainGetHostdevPath() to get path and call
virCgroupAllowDevicePath() directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
There's no need for this function. Currently it is passed as a
callback to virSCSIDeviceFileIterate(). However, SCSI devices
have just one file path. Therefore we can mimic approach used in
qemuDomainGetHostdevPath() to get path and call
virCgroupAllowDevicePath() directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
There's no need for this function. Currently it is passed as a
callback to virUSBDeviceFileIterate(). However, USB devices have
just one file path. Therefore we can mimic approach used in
qemuDomainGetHostdevPath() to get path and call
virCgroupAllowDevicePath() directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Add a test that allows providing the parent fabric_wwn in the input XML
in order to create the vHBA.
This also fixes a mixed setting of the fabric_wwn field from the read
test driver XML strings.
Rework the code to perform the various searches by parent, parent_wwnn/
parent_wwpn, parent_fabric_wwn, or vport capable in order to return the
'parent_host' number that is vHBA capable.
The former virNodeDeviceGetParentHost is renamed to add the ByParent
on it fixes an issue where if no parent was supplied in the XML to
create the vHBA, then virNodeDeviceFindByName was called with a NULL
second parameter which had bad results.
The reworked code will make the various calls to fetch the NPIV host
by the passed parameter options or if none are provided find a vport
capable NPIV HBA to perform the create. If the call is from the delete
path, then this option won't be allowed.
Each of virNodeDeviceGetParentHostBy* functions is now static, so
remove them external definitions.
A secondary benefit of this is the test_driver now can make use of
the new API to add some new tests to test the various creation options.
While perhaps improbable, it could be possible that after finding our
object that another thread running essentially in parallel could attempt
to delete the same vHBA.
So rather than dropping the lock right after finding the object, keep
the lock around while we drop the object lock and work on deleting the
object. Once the delete occurs we can safely drop the driver lock again.
Cleanup some of the usage of cleanup instead out for the goto label.
Create a virscsihost.c and place the functions there. That removes the
last #ifdef __linux__ from virutil.c.
Take the opporunity to also change the function names and in one case
the parameters slightly
Use the new virNodeDeviceGetParentName instead. Modify the callers to
build the node device scsi_host# name string in order to call the new
function so that proper lookup occurs.
Rather than have them mixed in with the virutil apis, create a separate
virvhba.c module and move the vHBA related calls into there. Soon there
will be more added.
Also modify the names of the functions and some arguments to be more
indicative of what is really happening. Adjust the callers respectively.
While I was changing fchosttest, rather than the non-descriptive names
test1...test6, rename them to match what the test is doing.
Modify the code to react more like a real HBA -> vHBA creation.
Currently the code would just modify the input XML definition to
set the name to a wwpn and then modify the scsi_host capability
entry for the defintion to change the scsi_host# and unique_id
before adding that into the node device.
This patch does things a bit better. It finds and copies a known
existing vHBA (scsi_host11) in the node_device database and modifies
that definition to change the name to scsi_host12 and set the wwnn/
wwpn to what the input XML would expect before adding the def to the
node device object list.
Then rather than create a returned "dev" using the (poorly) mocked
name - perform the lookup using the new device name.
Rather than inline the dummy creation of a vHBA to add to the node
devices - create a helper to do that work.
Also just tidy up a couple of things while at it...
Predefine a second NPIV capable HBA as well as a vHBA using the first
NPIV capable HBA. This will allow for a mechanism to perform more
realistic create vHBA testing.
Alter "test-scsi-host-vport" to be "scsi_host1" to match the real
environment. This is the vport capable HBA - IOW the NPIV device.
Add more fields to scsi_host1 as well.
Alter the XML being used by the objecttest to create a vHBA in order
to match the scsi_host1 parent name and to use validateable wwnn/wwpn.
This will allow for realistic testing.
Build fails with:
conf/node_device_conf.c:825:62: error: comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-compare]
if ((data->drm.type = virNodeDevDRMTypeFromString(type)) < 0) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
conf/node_device_conf.c:1801:59: error: comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-compare]
if ((type = virNodeDevDevnodeTypeFromString(tmp)) < 0) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
2 errors generated.
Fix by using intermediate variable to store the result similarly
to how it's done for other FromString* calls.
After 7f1bdec5fa our nodedev driver is capable of
determining DRM devices (DRM stands for Direct Render Manager not
Digital rights management). There is still one bit missing
though: virConnectListAllNodeDevices() is capable of listing
either all devices or just those with specified capability. Well,
DRM capability is missing there.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add a new attribute 'rendernode' to <gl> spice element.
Give it to QEMU if qemu supports it (queued for 2.9).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add a new 'drm' capability for Direct Rendering Manager (DRM) devices,
providing device type information.
Teach the udev backend to populate those devices.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add new <devnode> top-level <device> element, that list the associated
/dev files. Distinguish the main /dev name from symlinks with a 'type'
attribute of value 'dev' or 'symlink'.
Update a test to check XML schema, and actually add it to the test list
since it was missing.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
When the 'parent' was added to the virNodeDevicePtr structure
by commit id 'e8a4ea75a' the 'parent' field was not properly filled
in when a virGetNodeDevice call was made within driver/config code.
Only the device name was ever filled in. Fetching the parent required
a second trip via virNodeDeviceGetParent into the node device lookup
code was required in order to retrieve the specific parent field (and
still the parent field was never filled in although it was free'd).
Since we have the data when we initially call virGetNodeDevice from
within driver/node_config code - let's just fill in the parent field
as well for anyone that wants it without requiring another trip into
the node_device lookup just to get the parent.
This will allow API's such as virConnectListAllNodeDevices,
virNodeDeviceLookupByName, and virNodeDeviceLookupSCSIHostByWWN
to retrieve both name and parent in the returned virNodeDevicePtr.
Signed-off-by: John Ferlan <jferlan@redhat.com>
As discussed here [0][1] Coverity reported two issues:
- On libxlDomainMigrationPrepareTunnel3 @@mig will be leaked on failures
after sucessfull call libxlDomainMigrationPrepareAny hence we free it.
Setting mig = NULL after @mig is assigned plus adding libxlMigrationCookieFree
on error paths addresses the issue. In case virThreadCreate fails,
unref of args frees the cookie on dispose function (libxlMigrationDstArgsDispose)
- On libxlMigrationStartTunnel @tc would be leaked.
Fixed by correctly saving the newly allocated @tc onto @tnl such that
libxlMigrationStopTunnel would free it up.
[0] https://www.redhat.com/archives/libvir-list/2017-February/msg00791.html
[1] https://www.redhat.com/archives/libvir-list/2017-February/msg00833.html
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Before 9c17d665fd (v1.3.2 - I know, right?) it was possible to
have the following interface configuration:
<interface type='ethernet'/>
<script path=''/>
</interface>
This resulted in -netdev tap,script=,.. Fortunately, qemu helped
us to get away with this as it just ignored the empty script
path. However, after the commit mentioned above it's libvirtd
who is executing the script. Unfortunately without special
case-ing empty script path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently disk names do not follow the
(regex) /^[fhv]d[a-z]+[0-9]*$/ completely
and hence one can assign disk names like
vd2 etc. This patch ensures that the
disk names follow the regex mentioned.
This patch also adds a testcase.
Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com>
Commit 4ab0c959 fixed a memory leak in libxlDriverGetDom0MaxmemConf
but introduced a potential double free of mem_tokens
*** Error in `/usr/sbin/libvirtd': double free or corruption (out):
0x00007fffc808cfd0 ***
Avoid double free by setting mem_tokens to NULL after calling
virStringListFree.
Tunnelled migration doesn't require any extra network connections
beside the libvirt daemon. It's capable of strong encryption and the
default option of openstack-nova.
This patch adds the tunnelled migration(Tunnel3params) support to
libxl. On the source side, the data flow is:
* libxlDoMigrateSend() -> pipe libxlTunnel3MigrationFunc() polls pipe
* out and then write to dest stream.
While on the destination side:
* Stream -> pipe -> 'recvfd of libxlDomainStartRestore'
The usage is the same as p2p migration, execpt adding one extra
'--tunnelled' to the libvirt p2p migration command.
Signed-off-by: Bob Liu <bob.liu@oracle.com>
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
The newly introduced function libxlDomainMigrationPrepareAny
will be shared between P2P and tunnelled variations.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
This function is returning a boolean therefore check for '< 0'
makes no sense. It should have been
'!qemuDomainNamespaceAvailable'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The bare fact that mnt namespace is available is not enough for
us to allow/enable qemu namespaces feature. There are other
requirements: we must copy all the ACL & SELinux labels otherwise
we might grant access that is administratively forbidden or vice
versa.
At the same time, the check for namespace prerequisites is moved
from domain startup time to qemu.conf parser as it doesn't make
much sense to allow users to start misconfigured libvirt just to
find out they can't start a single domain.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If the apparmor security driver is loaded/enabled and domain config
contains a <seclabel> element whose type attribute is not 'apparmor',
starting the domain fails when attempting to label resources such
as tap FDs.
Many of the apparmor driver entry points attempt to retrieve the
apparmor security label from the domain def, returning failure if
not found. Functions such as AppArmorSetFDLabel fail even though
domain config contains an explicit 'none' secuirty driver, e.g.
<seclabel type='none' model='none'/>
Change the entry points to succeed if the domain config <seclabel>
is not apparmor. This matches the behavior of the selinux driver.
Commit 2a8d40f4ec refactored qemuMonitorJSONGetCPUx86Data and replaced
virJSONValueObjectGet(reply, "return") with virJSONValueObjectGetArray.
While the former is guaranteed to always return non-NULL pointer the
latter may return NULL if the returned JSON object is not an array.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
mknod() is affected my the current umask, so we're not
guaranteed the newly-created device node will have the
right permissions.
Call chmod(), which is not affected by the current umask,
immediately afterwards to solve the issue.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1421036
To make sure bit 'b' fits into the bitmap, we need to allocate b+1
bits, since we number from 0.
Adjust the bitmap test to set a bit at a multiple of 16.
That way the test fails without this fix, because the VIR_REALLOC
call clears the newly added memory even if the original pointer
has not changed.
Move the range check introduced by commit 2650d5e into
virDomainUSBAddressFindPort. That way both virDomainUSBAddressRelease
and virDomainUSBAddressSetAddHub can benefit from it.
Reported-by: Michal Privoznik <mprivozn@redhat.com>
Due to a logic error, the autofilling of USB port when a bus is
specified:
<address type='usb' bus='0'/>
does not work for non-hub devices on domain startup.
Fix the logic in qemuDomainAssignUSBPortsIterator to also
assign ports for USB addresses that do not yet have one.
https://bugzilla.redhat.com/show_bug.cgi?id=1374128
We have to allocate first and if, and only if, it was successful we
can set the count. A segfault has occurred in
virNetServerServiceNewPostExecRestart() when VIR_ALLOC_N(svc->socks,
n) has failed, but svc->nsocsk = n was already set. Thus
virObejectUnref(svc) was called and therefore it was possible that
virNetServerServiceDispose was called => segmentation fault. For
safeness NULL pointer check were added in
virNetServerServiceDispose().
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Recently e1000 NIC support was added to bhyve; implement that in
the bhyve driver:
- Add capability check by analyzing output of the 'bhyve -s 0,e1000'
command
- Modify bhyveBuildNetArgStr() to support e1000 and also pass
virConnectPtr so it could call bhyveDriverGetCaps() to check if this
NIC is supported
- Modify command parsing code to add support for e1000 and adjust tests
- Add net-e1000 test
If either the "if (STRPREFIX(mem_tokens[j], "max:"))" is never entered
or the "if (virStrToLong_ull(mem_tokens[j] + 4, &p, 10, maxmem) < 0)" break
is hit, control goes back to the outer loop processing 'cmd_tokens' and
it's possible that the 'mem_tokens' would be overwritten.
Found by Coverity
Right now, we use simple string comparison both on the source paths
(mount's output vs pool's source) and the target (mount's mnt_dir vs
pool's target). The problem are symlinks and mount indeed returns
symlinks in its output, e.g. /dev/mappper/lvm_symlink. The same goes for
the pool's source/target, so in order to successfully compare these two
replace plain string comparison with virFileComparePaths which will
resolve all symlinks and canonicalize the paths prior to comparison.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1417203
Signed-off-by: Erik Skultety <eskultet@redhat.com>
So rather than comparing 2 paths (strings) as they are, which can very
easily lead to unnecessary errors (e.g. in storage driver) that the paths
are not the same when in fact they'd be e.g. just symlinks to the same
location, we should put our best effort into resolving any symlinks and
canonicalizing the path and only then compare the 2 paths for equality.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
When FS pool's source is already mounted on the target location instead
of just simply marking the pool as active, thus starting it we fail with
an error stating that the source is indeed already mounted on the target.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
On a system with 697 SCSI disks each configured with 8 paths the command
virsh nodedev-list fails with
error: Failed to list node devices
error: internal error: Too many node_devices '16816' for limit '16384'
Increasing the upper limit on lists of node devices from 16K to 64K.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Although currently this is documented in virsh man page
and virsh help, the expicit mention in the error message
is helful for tools using the API directly.
Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com>
==11846== 240 bytes in 1 blocks are definitely lost in loss record 81 of 107
==11846== at 0x4C2BC75: calloc (vg_replace_malloc.c:624)
==11846== by 0x18C74242: virAllocN (viralloc.c:191)
==11846== by 0x4A05E8: qemuMonitorCPUModelInfoCopy (qemu_monitor.c:3677)
==11846== by 0x446E3C: virQEMUCapsNewCopy (qemu_capabilities.c:2171)
==11846== by 0x437335: testQemuCapsCopy (qemucapabilitiestest.c:108)
==11846== by 0x437CD2: virTestRun (testutils.c:180)
==11846== by 0x437AD8: mymain (qemucapabilitiestest.c:176)
==11846== by 0x4397B6: virTestMain (testutils.c:992)
==11846== by 0x437B44: main (qemucapabilitiestest.c:188)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This commit removes the handcrafted code for
remoteDomainCreateWithFlags() and lets it auto generate.
A little bit of history repeating...
Commit 03d813bbcd removed the auto generation of
remoteDomainCreateWithFlags() because it was thought that the design
flaw in the remote protocol for virDomainCreate is also within the
remote protocol for virDomainCreateWithFlags. As the commit message of
ddaf15d7a3 mentions this is not the case therefore we
can auto generate the client part.
Even worse there was a typo in remoteDomainCreateWithFlags()
'remote_domain_create_with_flags_args ret;' but in fact it has to be
'remote_domain_create_with_flags_ret ret;'.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
After freeing the data structures we have to reset the counters to
zero. This fixes a segmentation fault when virNetDevIPInfoClear is
called twice (e.g. this is possible in virDomainNetDefParseXML() if
virDomainNetIPInfoParseXML(...) fails with ret < 0 (this leads to the
first call of 'virNetDevIPInfoClear(&def->guestIP)') and the resulting
call of virDomainNetDefFree(def) in the error path of
virDomainNetDefParseXML() (this leads to the second call of
virNetDevIPInfoClear(&def->guestIP), and finally to the segmentation
fault).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
If virDomainChrSourceDefNew(xmlopt) fails, it will lead to free()ing
the uninitialized pointer bus. The fix for this is to initialize bus
with NULL.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Check if virQEMUCapsNewCopy(...) has failed, thus a segmentation fault
in virQEMUCapsFilterByMachineType(...) will be avoided.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Using libvirt to do live migration over RDMA via IPv6 address failed.
For example:
rhel73_host1_guest1 qemu+ssh://[deba::2222]/system --verbose
root@deba::2222's password:
error: internal error: unable to execute QEMU command 'migrate': RDMA
ERROR: could not rdma_getaddrinfo address deba
As we can see, the IPv6 address used by rdma_getaddrinfo() has only
"deba" part because we didn't properly enclose the IPv6 address in []
and passed rdma:deba::2222:49152 as the migration URI in
qemuMonitorMigrateToHost.
Signed-off-by: David Dai <zdai@linux.vnet.ibm.com>
When the libxl driver is initialized, it creates a virDomainDef
object for dom0 and adds it to the list of domains. Total memory
for dom0 was being set from the max_memkb field of libxl_dominfo
struct retrieved from libxl, but this field can be set to
LIBXL_MEMKB_DEFAULT (~0ULL) if dom0 maximum memory has not been
explicitly set by the user.
This patch adds some simple parsing of the Xen commandline,
looking for a dom0_mem parameter that also specifies a 'max' value.
If not specified, dom0 maximum memory is effectively all physical
host memory.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
The libxl driver reports different values of maximum memory depending
on state of a domain. If inactive, maximum memory value is reported
correctly. When active, maximum memory is derived from max_pages value
returned by the XEN_SYSCTL_getdomaininfolist sysctl operation. But
max_pages can be changed by toolstacks and does not necessarily
represent the maximum memory a domain can use during its active
lifetime.
A better location for determining a domain's maximum memory is the
/local/domain/<id>/memory/static-max node in xenstore. This value
is set from the libxl_domain_build_info.max_memkb field when creating
the domain. Currently it cannot be changed nor can its value be
exceeded by a balloon operation. From libvirt's perspective, always
reporting maximum memory with virDomainDefGetMemoryTotal() will produce
the same results as reading the static-max node in xenstore.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
When a user does not explicitly set a <driver> in the disk config,
libvirt defers selection of a default to libxl. This approach works
fine when starting a domain with such configuration or attaching a
disk to a running domain. But when detaching such a disk, libxl
will fail with "unrecognized disk backend type: 0". libxl makes no
attempt to recalculate a default backend (driver) on detach and
simply fails when uninitialized.
This patch updates the libvirt disk config with the backend selected
by libxl when starting a domain or attaching a disk to a running
domain. Another benefit of this approach is that the live XML is
also updated with the backend driver selected by libxl.
When starting a domian, a libxl_domain_config object is created from
virDomainDef. Any virDomainDiskDef devices with a format of
VIR_STORAGE_FILE_NONE are mapped to LIBXL_DISK_FORMAT_RAW in the
corresponding libxl_disk_device, but the virDomainDiskDef format is
never updated to reflect the change.
A better place to set a default format for disk devices is the
device post-parse callback, ensuring the virDomainDiskDef object
reflects the default format.
This patchs allows to set the timeout value used for all
openvswitch calls. The default timeout value remains as
before at 5 seconds.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Provide the ability to specify a default timeout value for
successful completion of openvswitch calls in the libvirtd
configuration file.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This patch add support for file memory backing on numa topology.
The specified access mode in memoryBacking can be overriden
by specifying token memAccess in numa cell.
This part introduces new xml elements for file based
memorybacking support and their parsing.
(It allows vhost-user to be used without hugepages.)
New xml elements:
<memoryBacking>
<source type="file|anonymous"/>
<access mode="shared|private"/>
<allocation mode="immediate|ondemand"/>
</memoryBacking>
Add new parameter memory_backing_dir where files will be stored when memoryBacking
source is selected as file.
Value is stored inside char* memoryBackingDir
Rename to avoid duplicate code. Because virDomainMemoryAccess will be
used in memorybacking for setting default behaviour.
NOTE: The enum cannot be moved to qemu/domain_conf because of headers
dependency
Strings associated with virDomainHyperv values in domain_conf.c are used to
construct HyperV CPU features names to be compared with names defined in
cpu_x86_data.h and the names for HyperV "spinlocks" feature don't match.
This leads to a misleading warning:
"host doesn't support hyperv 'spinlocks' feature" even when it's supported.
Let's fix it and rename along with it VIR_CPU_x86_KVM_HV_SPINLOCK to
VIR_CPU_x86_KVM_HV_SPINLOCKS.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
virCPUDefStealModel is called with keepVendor == true which means the
cpu structure will keep its original vendor/vendor_id values. Thus it
makes no sense to copy them to the translated definition as they won't
be used there anyway. Except that the translated->vendor pointer might
get lost in x86Decode.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When we happen to lose a domain but still get a performance event
for it, we should also free the event handle.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Just like we need wrappers over other virSecurityManager APIs, we
need one for virSecurityManagerSetImageLabel and
virSecurityManagerRestoreImageLabel. Otherwise we might end up
relabelling device in wrong namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The typical pattern when calling libxl functions that populate a
structure is
libxl_foo foo;
libxl_foo_init(&foo);
libxl_get_foo(ctx, &foo);
...
libxl_foo_dispose(&foo);
Fix several instances of libxl_physinfo missing the init and
dispose calls.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
libxlGetAutoballoonConf is supposed to honor user-specified
autoballoon setting in libxl.conf. As written, the user-specified
setting could be overwritten by the subsequent logic to check
dom0_mem parameter. If user-specified setting is present and
correct, accept it. Only fallback to checking Xen dom0_mem
command line parameter if user-specfied setting is not present.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
If no graphics element is in XML xenFormatXLSpice will access
graphics without checking it has one in the first place, leading to a
segmentation fault.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Firstly, instead of checking for next->path the
virStorageSourceIsEmpty() function should be used which also
takes disk type into account.
Secondly, not every disk source passed has the correct type set
(due to our laziness). Therefore, instead of checking for
virStorageSourceIsBlockLocal() and also S_ISBLK() the former can
be refined to just virStorageSourceIsLocalStorage().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Again, one missed bit. This time without this commit there is no
/dev entry in the namespace of the qemu process when doing disk
snapshots or block-copy.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
These functions do not need to see the whole virDomainDiskDef.
Moreover, they are going to be called from places where we don't
have access to the full disk definition. Sticking with
virStorageSource is more than enough.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There is no need for this. None of the namespace helpers uses it.
Historically it was used when calling secdriver APIs, but we
don't to that anymore.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Again, one missed bit. This time without this commit there is no
/dev entry in the namespace of the qemu process when attaching
vhost SCSI device.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Since we have qemuSecurity wrappers over
virSecurityManagerSetHostdevLabel and
virSecurityManagerRestoreHostdevLabel we ought to use them
instead of calling secdriver APIs directly. Without those
wrappers the labelling won't be done in the correct namespace
and thus won't apply to the nodes seen by qemu itself.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
libvirt was able to set the host_mtu option when an MTU was explicitly
given in the interface config (with <mtu size='n'/>), set the MTU of a
libvirt network in the network config (with the same named
subelement), and would automatically set the MTU of any tap device to
the MTU of the network.
This patch ties that all together (for networks based on tap devices
and either Linux host bridges or OVS bridges) by learning the MTU of
the network (i.e. the bridge) during qemuInterfaceBridgeConnect(), and
returning that value so that it can then be passed to
qemuBuildNicDevStr(); qemuBuildNicDevStr() then sets host_mtu in the
interface's commandline options.
The result is that a higher MTU for all guests connecting to a
particular network will be plumbed top to bottom by simply changing
the MTU of the network (in libvirt's config for libvirt-managed
networks, or directly on the bridge device for simple host bridges or
OVS bridges managed outside of libvirt).
One question I have about this - it occurred to me that in the case of
migrating a guest from a host with an older libvirt to one with a
newer libvirt, the guest may have *not* had the host_mtu option on the
older machine, but *will* have it on the newer machine. I'm curious if
this could lead to incompatibilities between source and destination (I
guess it all depends on whether or not the setting of host_mtu has a
practical effect on a guest that is already running - Maxime?)
Likewise, we could run into problems when migrating from a newer
libvirt to older libvirt - The guest would have been told of the
higher MTU on the newer libvirt, then migrated to a host that didn't
understand <mtu size='blah'/>. (If this really is a problem, it would
be a problem with or without the current patch).
Example:
<network>
...
<mtu size='9000'/>
...
If mtu is unset, it's assumed that we want the default for whatever is
the underlying transport (usually this is 1500).
This setting isn't yet wired in, so it will have no effect.
This partially resolves: https://bugzilla.redhat.com/1224348
virNetDevTapCreateInBridgePort() has always set the new tap device to
the current MTU of the bridge it's being attached to. There is one
case where we will want to set the new tap device to a different
(usually larger) MTU - if that's done with the very first device added
to the bridge, the bridge's MTU will be set to the device's MTU. This
patch allows for that possibility by adding "int mtu" to the arg list
for virNetDevTapCreateInBridgePort(), but all callers are sending -1,
so it doesn't yet have any effect.
Since the requested MTU isn't necessarily what is used in the end (for
example, if there is no MTU requested, the tap device will be set to
the current MTU of the bridge), and the hypervisor may want to know
the actual MTU used, we also return the actual MTU to the caller (if
actualMTU is non-NULL).
In order for memory locking to work, the hard limit on memory
locking (and usage) has to be set appropriately by the user.
The documentation mentions the requirement already: with this
patch, it's going to be enforced by runtime checks as well,
by forbidding a non-compliant guest from being defined as well
as edited and started.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1316774
Like it usually happens, I fixed one thing and broke another:
in 803966c76 address allocation was fixed for SATA disks, but
broke that for virtio disks, because it dropped disk address
assignment completely. It's not needed for SATA disks anymore,
but still needed for the virtio ones.
Bring that back and add a couple of tests to make sure it won't
happen again.
Similarly to one of the previous commits, we need to deal
properly with symlinks in hotplug case too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Imagine you have a disk with the following source set up:
/dev/disk/by-uuid/$uuid (symlink to) -> /dev/sda
After cbc45525cb the transitive end of the symlink chain is
created (/dev/sda), but we need to create any item in chain too.
Others might rely on that.
In this case, /dev/disk/by-uuid/$uuid comes from domain XML thus
it is this path that secdriver tries to relabel. Not the resolved
one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
After previous commit this has become redundant step.
Also setting up devices in namespace and setting their label
later on are two different steps and should be not done at once.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The idea is to move all the seclabel setting to security driver.
Having the relabel code spread all over the place looks very
messy.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Because of the nature of security driver transactions, it is
impossible to use them properly. The thing is, transactions enter
the domain namespace and commit all the seclabel changes.
However, in RestoreAllLabel() this is impossible - the qemu
process, the only process running in the namespace, is gone. And
thus is the namespace. Therefore we shouldn't use the transactions
as there is no namespace to enter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The current ordering is as follows:
1) set label
2) create the device in namespace
3) allow device in the cgroup
While this might work for now, it will definitely not work if the
security driver would use transactions as in that case there
would be no device to relabel in the domain namespace as the
device is created in the second step.
Swap steps 1) and 2) to allow security driver to use more
transactions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We will need to traverse the symlinks one step at the time.
Therefore we need to see where a symlink is pointing to.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The comment to the function states that the errors from the child
process are reported. Well, the error buffer is filled with
possible error messages. But then it is thrown away. Among with
important error message from the child process.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
==11260== 1,006 bytes in 1 blocks are definitely lost in loss record 106 of 111
==11260== at 0x4C2AE5F: malloc (vg_replace_malloc.c:297)
==11260== by 0x4C2BDFF: realloc (vg_replace_malloc.c:693)
==11260== by 0x4EA430B: virReallocN (viralloc.c:245)
==11260== by 0x4EA7C52: virBufferGrow (virbuffer.c:130)
==11260== by 0x4EA7D28: virBufferAdd (virbuffer.c:165)
==11260== by 0x4EA8E10: virBufferStrcat (virbuffer.c:718)
==11260== by 0x42D263: xenFormatXLDiskSrcNet (xen_xl.c:960)
==11260== by 0x42D4EB: xenFormatXLDiskSrc (xen_xl.c:1015)
==11260== by 0x42D870: xenFormatXLDisk (xen_xl.c:1101)
==11260== by 0x42DA89: xenFormatXLDomainDisks (xen_xl.c:1148)
==11260== by 0x42EAF8: xenFormatXL (xen_xl.c:1558)
==11260== by 0x40E85F: testCompareParseXML (xlconfigtest.c:105)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Originally/discovered proposed by "Wang King <king.wang@huawei.com>"
When the virCloseCallbacksSet is first called, it increments the refcnt
on the domain object to ensure it doesn't get deleted before the callback
is called. The refcnt would be decremented in virCloseCallbacksUnset once
the entry is removed from the closeCallbacks has table.
When (mostly) normal shutdown occurs, the qemuProcessStop will end up
calling qemuProcessAutoDestroyRemove and will remove the callback from
the list and hash table normally and decrement the refcnt.
However, when qemuConnectClose calls virCloseCallbacksRun, it will scan
the (locked) closeCallbacks list for matching domain and callback function.
If an entry is found, it will be removed from the closeCallbacks list and
placed into a lookaside list to be processed when the closeCallbacks lock
is dropped. The callback function (e.g. qemuProcessAutoDestroy) is called
and will run qemuProcessStop. That code will fail to find the callback
in the list when qemuProcessAutoDestroyRemove is called and thus not decrement
the domain refcnt. Instead since the entry isn't found the code will just
return (mostly) harmlessly.
This patch will resolve the issue by taking another ref during the
search UUID process during virCloseCallackRun, decrementing the refcnt
taken by virCloseCallbacksSet, calling the callback routine and returning
overwriting the vm (since it could return NULL). Finally, it will call the
virDomainObjEndAPI to lower the refcnt and remove the lock taken during
the search UUID processing. This may cause the vm to be destroyed.
After deploying virtlogd by default we identified a number of
mistakes in the systemd unit file. virtlockd's relationship
to libvirtd is the same as virtlogd, so we must apply the
same unit file fixes to virtlockd
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
xen.git commit 57f8b13c changed several of the libxl memory
get/set functions to take 64 bit parameters. The libvirt
libxl driver still uses uint32_t variables for these various
parameters, which is particularly problematic for the
libxl_set_memory_target() function.
When dom0 autoballooning is enabled, libvirt (like xl) determines
the memory needed to start a domain and the memory available. If
memory available is less than memory needed, dom0 is ballooned
down by passing a negative value to libxl_set_memory_target()
'target_memkb' parameter. Prior to xen.git commit 57f8b13c,
'target_memkb' was an int32_t. Subtracting a larger uint32 from
a smaller uint32 and assigning it to int32 resulted in a negative
number. After commit 57f8b13c, the same subtraction is widened
to a int64, resulting in a large positive number. The simple
fix taken by this patch is to assign the difference of the
uint32 values to a temporary int32 variable, which is then
passed to 'target_memkb' parameter of libxl_set_memory_target().
Note that it is undesirable to change libvirt to use 64 bit
variables since it requires setting LIBXL_API_VERSION to 0x040800.
Currently libvirt supports LIBXL_API_VERSION >= 0x040400,
essentially Xen >= 4.4.
Now that we have a function for properly assigning the blockdeviotune
info, let's use it instead of dropping the group name on every
assignment. Otherwise it will not work with both --live and --config
options.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This is necessary to be able to get statistics for venet0 or
"host-routed" adapter, which has -1 index and thus, its statistics
is shown as "net.nic4294967295".
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Commit 815d98a started auto-adding one hub if there are more USB devices
than available USB ports.
This was a strange choice, since there might be even more devices.
Before USB address allocation was implemented in libvirt, QEMU
automatically added a new USB hub if the old one was full.
Adjust the logic to try adding as many hubs as will be needed
to plug in all the specified devices.
https://bugzilla.redhat.com/show_bug.cgi?id=1410188
As bhyve for a long time didn't have a notion of the explicit SATA
controller and created a controller for each drive, the bhyve driver
in libvirt acted in a similar way and didn't care about the SATA
controllers and assigned PCI addresses to drives directly, as
the generated command will look like this anyway:
2:0,ahci-hd,somedisk.img
This no longer makes sense because:
1. After commit c07d1c1c4f it's not possible to assign
PCI addresses to disks
2. Bhyve now supports multiple disk drives for a controller,
so it's going away from 1:1 controller:disk mapping, so
the controller object starts to make more sense now
So, this patch does the following:
- Assign PCI address to SATA controllers (previously we didn't do this)
- Assign disk addresses instead of PCI addresses for disks. Now, when
building a bhyve command, we take PCI address not from the disk
itself but from its controller
- Assign addresses at XML parsing time using the
assignAddressesCallback. This is done mainly for being able to
verify address allocation via xml2xml tests
- Adjust existing bhyvexml2{xml,argv} tests to chase the new
address allocation
This patch is largely based on work of Fabian Freyer.
Add virBhyveDriverCreateXMLConf, a simple wrapper around
virDomainXMLOptionNew that makes it easier to pass bhyveConnPtr
as a private data for parser. It will be used later for device
address allocation at parsing time.
Update consumers to use it instead of direct calls to
virDomainXMLOptionNew.
As we now have proper callbacks connected for the tests, update
test files accordingly to include the automatically generated
PCI root controller.
Introduce a BHYVE_CAP_AHCI32SLOT capability that shows
if 32 devices per SATA controller are supported, and
a bhyveProbeCapsAHCI32Slot function that probes it.
Because this is invalid xml for containers. This patch almost
reverts 7eda8369, but still skips converting vz sdk bootorder
for containers to libvirt bootorder because we use boot order
in containers for quite different purpurse.
==12618== 110 bytes in 10 blocks are definitely lost in loss record 269 of 295
==12618== at 0x4C2AE5F: malloc (vg_replace_malloc.c:297)
==12618== by 0x1CFC6DD7: vasprintf (vasprintf.c:73)
==12618== by 0x1912B2FC: virVasprintfInternal (virstring.c:551)
==12618== by 0x1912B411: virAsprintfInternal (virstring.c:572)
==12618== by 0x50B1FF: qemuAliasChardevFromDevAlias (qemu_alias.c:638)
==12618== by 0x518CCE: qemuBuildChrChardevStr (qemu_command.c:4973)
==12618== by 0x522DA0: qemuBuildShmemBackendChrStr (qemu_command.c:8674)
==12618== by 0x523209: qemuBuildShmemCommandLine (qemu_command.c:8789)
==12618== by 0x526135: qemuBuildCommandLine (qemu_command.c:9843)
==12618== by 0x48B4BA: qemuProcessCreatePretendCmd (qemu_process.c:5897)
==12618== by 0x4378C9: testCompareXMLToArgv (qemuxml2argvtest.c:498)
==12618== by 0x44D5A6: virTestRun (testutils.c:180)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
For example when both total_bytes_sec and total_bytes_sec_max are set,
but the former gets cleaned due to new call setting, let's say,
read_bytes_sec, we end up with this weird message for the command:
$ virsh blkdeviotune fedora vda --read-bytes-sec 3000
error: Unable to change block I/O throttle
error: unsupported configuration: value 'total_bytes_sec_max' cannot be set if 'total_bytes_sec' is not set
So let's make it more descriptive. This is how it looks after the change:
$ virsh blkdeviotune fedora vda --read-bytes-sec 3000
error: Unable to change block I/O throttle
error: unsupported configuration: cannot reset 'total_bytes_sec' when 'total_bytes_sec_max' is set
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1344897
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
We were setting it based on whether it was supported and that lead to
setting it to NULL, which our JSON code caught. However it ended up
producing the following results:
$ virsh blkdeviotune fedora vda --total-bytes-sec-max 2000
error: Unable to change block I/O throttle
error: internal error: argument key 'group' must not have null value
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The issue is that if this graphics definition is provided:
<graphics type='vnc' port='0'/>
it's parsed as:
<graphics type='vnc' autoport='no'>
<listen type='address'/>
</graphics>
but if the resulting XML is parsed again the output is:
<graphics type='vnc' port='-1' autoport='yes'>
<listen type='address'/>
</graphics>
and this should not happen. The XML have to always remain the same
after it was already parsed by libvirt.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1383039
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
