Not every platform is guaranteed to have dlopen/dlsym, so we should
conditionalize its use. Suprisingly it is actually present for Win32
via the mingw-dlfcn add on, but we should still conditionalize it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
These functions were made exportable back in 3aa3e072 when I was
splitting network code into parsing and list management parts.
Since then the split is finished now and these two functions do
not need to be exported anymore.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This member allows us to store a pointer to some private data.
However, the comment says it's used in both domain driver and
network driver. Well, it is not. It's just one pointer and domain
driver uses it directly. Network driver has a global driver
variable. Update the comment to not confuse others.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Since the introduction of shmem, there was a split of preparation code
from the formatting code from qemuBuildCommandLine() into
qemuProcessPrepareDomain(). Let's fix shmem in this regard, so that
we can slowly get to a cleaner codebase.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
So there are couple of issues here. Firstly, we never unref the
@pendingReply and thus it leaks.
==13279== 144 (72 direct, 72 indirect) bytes in 1 blocks are definitely lost in loss record 1,095 of 1,259
==13279== at 0x4C2E080: calloc (vg_replace_malloc.c:711)
==13279== by 0x781FA97: _dbus_pending_call_new_unlocked (in /usr/lib64/libdbus-1.so.3.14.11)
==13279== by 0x7812A4C: dbus_connection_send_with_reply (in /usr/lib64/libdbus-1.so.3.14.11)
==13279== by 0x56BEDF3: virNetDaemonCallInhibit (virnetdaemon.c:514)
==13279== by 0x56BEF18: virNetDaemonAddShutdownInhibition (virnetdaemon.c:536)
==13279== by 0x12473B: daemonInhibitCallback (libvirtd.c:742)
==13279== by 0x1249BD: daemonRunStateInit (libvirtd.c:823)
==13279== by 0x554FBCF: virThreadHelper (virthread.c:206)
==13279== by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)
==13279== by 0x928DE3C: clone (in /lib64/libc-2.23.so)
Secondly, while we send the message, we are suspended ('cos we're
talking to a UNIX socket). However, until we are resumed back
again the reply might have came therefore subsequent
dbus_pending_call_set_notify() has no effect and in fact the
virNetDaemonGotInhibitReply() callback is never called. Thirdly,
the dbus_connection_send_with_reply() has really stupid policy
for return values. To cite the man page:
Returns
FALSE if no memory, TRUE otherwise.
Yes, that's right. If anything goes wrong and it's not case of
OOM then TRUE is returned, i.e. you're trying to pass FDs and
it's not supported, or you're not connected, or anything else.
Therefore, checking for return value of
dbus_connection_send_with_reply() is not enoguh. We also have to
check if @pendingReply is not NULL before proceeding any further.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We are given a string in @machinename, we never allocate it, just
merely use it for reading. We should not free it otherwise it
leads to double free:
==32191== Thread 17:
==32191== Invalid free() / delete / delete[] / realloc()
==32191== at 0x4C2D1A0: free (vg_replace_malloc.c:530)
==32191== by 0x54BBB84: virFree (viralloc.c:582)
==32191== by 0x2BC04499: qemuProcessStop (qemu_process.c:6313)
==32191== by 0x2BC500FF: processMonitorEOFEvent (qemu_driver.c:4724)
==32191== by 0x2BC502FC: qemuProcessEventHandler (qemu_driver.c:4769)
==32191== by 0x5550640: virThreadPoolWorker (virthreadpool.c:167)
==32191== by 0x554FBCF: virThreadHelper (virthread.c:206)
==32191== by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)
==32191== by 0x928DE3C: clone (in /lib64/libc-2.23.so)
==32191== Address 0x31893d70 is 0 bytes inside a block of size 1,100 free'd
==32191== at 0x4C2D1A0: free (vg_replace_malloc.c:530)
==32191== by 0x54BBB84: virFree (viralloc.c:582)
==32191== by 0x54C1936: virCgroupValidateMachineGroup (vircgroup.c:343)
==32191== by 0x54C4B29: virCgroupNewDetectMachine (vircgroup.c:1550)
==32191== by 0x2BBDDA29: qemuConnectCgroup (qemu_cgroup.c:972)
==32191== by 0x2BC05DA7: qemuProcessReconnect (qemu_process.c:6822)
==32191== by 0x554FBCF: virThreadHelper (virthread.c:206)
==32191== by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)
==32191== by 0x928DE3C: clone (in /lib64/libc-2.23.so)
==32191== Block was alloc'd at
==32191== at 0x4C2BE80: malloc (vg_replace_malloc.c:298)
==32191== by 0x4C2E35F: realloc (vg_replace_malloc.c:785)
==32191== by 0x54BB492: virReallocN (viralloc.c:245)
==32191== by 0x54BEDF2: virBufferGrow (virbuffer.c:150)
==32191== by 0x54BF3B9: virBufferVasprintf (virbuffer.c:408)
==32191== by 0x54BF324: virBufferAsprintf (virbuffer.c:381)
==32191== by 0x55BB271: virDomainGenerateMachineName (domain_conf.c:27078)
==32191== by 0x2BBD5B8F: qemuDomainGetMachineName (qemu_domain.c:9595)
==32191== by 0x2BBDD9B4: qemuConnectCgroup (qemu_cgroup.c:966)
==32191== by 0x2BC05DA7: qemuProcessReconnect (qemu_process.c:6822)
==32191== by 0x554FBCF: virThreadHelper (virthread.c:206)
==32191== by 0x8F913D3: start_thread (in /lib64/libpthread-2.23.so)
Moreover, make the @machinename 'const char *' to mark it
explicitly that we are not changing the passed string.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit @4cb719b2dc moved the driver locks around since these have become
unnecessary at spots where the code handles now self-lockable object
list, but missed the possible double unlock if udevEnumerateDevices
fails, because at that point the driver lock had been already dropped.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
In commit 5e515b542d I've attempted to fix the inability to access
storage from the apparmor helper program by linking with the storage
driver. By linking with the .so the linker complains that it's not
portable. Fix this by loading the module dynamically as we are supposed
to do.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Driver modules proved to be reliable for a long time. Since support for
not building modules complicates the code and makefiles drop it.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
If a domain name contains a '=' and the unix socket path is
auto-generated or socket path provided by user contains '=' QEMU
is unable to properly parse the command line. In order to make it
work we need to use the new command line syntax for VNC if it's
available, otherwise we can use the old syntax.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1352529
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Remove the complex and unreliable code which inferred the node name
hierarchy only from data returned by 'query-named-block-nodes'. It turns
out that query-blockstats contain the full hierarchy of nodes as
perceived by qemu so the inference code is not necessary.
In query blockstats, the 'parent' object corresponds to the storage
behind a storage volume and 'backing' corresponds to the lower level of
backing chain. Since all have node names this data can be really easily
used to detect node names.
In addition to the code refactoring the one remaining test case needed
to be fixed along.
Reviewed-by: Eric Blake <eblake@redhat.com>
The same operation will become useful in other places so rename the
function to be more generic and move it to the top so that it can be
reused earlier in the file.
Reviewed-by: Eric Blake <eblake@redhat.com>
Allow getting the raw data from query-blockstats, so that we can use it
to detect the backing chain later on.
Reviewed-by: Eric Blake <eblake@redhat.com>
Disallow providing the wwnn/wwpn of the HBA in the adapter XML:
<adapter type='fc_host' [parent='scsi_hostN'] wwnn='HBA_wwnn'
wwpn='HBA_wwpn'/>
This should be considered a configuration error since a vHBA
would not be created. In order to use the HBA as the backing the
following XML should be used:
<adapter type='scsi_host' name='scsi_hostN'/>
So add a check prior to the checkParent call to validate that
the provided wwnn/wwpn resolves to a vHBA and not an HBA.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Since commit 2e6ecba1bc, the pointer to the qemu driver is saved in
domain object's private data and hence does not have to be passed as
yet another parameter if domain object is already one of them.
This is a first (example) patch of this kind of clean up, others will
hopefully follow.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The switch contains considerable amount of changes:
virQEMUCapsRememberCached() is removed because this is now handled
by virFileCacheSave().
virQEMUCapsInitCached() is removed because this is now handled by
virFileCacheLoad().
virQEMUCapsNewForBinary() is split into two functions,
virQEMUCapsNewData() which creates new data if there is nothing
cached and virQEMUCapsLoadFile() which loads the cached data.
This is now handled by virFileCacheNewData().
virQEMUCapsCacheValidate() is removed because this is now handled by
virFileCacheValidate().
virQEMUCapsCacheFree() is removed because it's no longer required.
Add virCapsPtr into virQEMUCapsCachePriv because for each call of
virFileCacheLookup*() we need to use current virCapsPtr.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This is a preparation for following patches where we switch to
virFileCache for QEMU capabilities cache
The host arch will always remain the same but virCaps may change. Now
the host arch is stored while creating new qemu capabilities cache.
It removes the need to pass virCaps into virQEMUCapsCache*() functions.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This will store private data that will be used by following patches
when switching to virFileCache.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The new virFileCache will nicely handle the caching logic for any data
that we would like to cache. For each type of data we will just need
to implement few handlers that will take care of creating, validating,
loading and saving the cached data.
The cached data must be an instance of virObject.
Currently we cache QEMU capabilities which will start using
virFileCache.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
It's possible to have more than one unnamed virtio-serial unix channel.
We need to generate a unique name for each channel. Currently, we use
".../unknown.sock" for all of them. Better practice would be to specify
an explicit target path name; however, in the absence of that, we need
uniqueness in the names we generate internally.
Before the changes we'd get /var/lib/libvirt/qemu/channel/target/unknown.sock
for each instance of
<channel type='unix'>
<source mode='bind'/>
<target type='virtio'/>
</channel>
Now, we get vioser-00-00-01.sock, vioser-00-00-02.sock, etc.
Signed-off-by: Scott Garfinkle <seg@us.ibm.com>
It is more related to a domain as we might use it even when there is
no systemd and it does not use any dbus/systemd functions. In order
not to use code from conf/ in util/ pass machineName in cgroups code
as a parameter. That also fixes a leak of machineName in the lxc
driver and cleans up and de-duplicates some code.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This way the function can work as a central point of clean-up code and
we don't have to duplicate code. And it works similarly to the qemu
driver.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Rather than rely on virSecretObjEndAPI to make the final virObjectUnref
after the call to virSecretObjListRemove, be more explicit by calling
virObjectUnref and setting @obj to NULL for secretUndefine and in
the error path of secretDefineXML. Calling EndAPI will end up calling
Unlock on an already unlocked object which has indeteriminate results
(usually an ignored error).
The virSecretObjEndAPI will both Unref and Unlock the object; however,
the virSecretObjListRemove would have already Unlock'd the object so
calling Unlock again is incorrect. Once the virSecretObjListRemove
is called all that's left is to Unref our interest since that's the
corrollary to the virSecretObjListAdd which returned our ref interest
plus references for each hash table in which the object resides. In math
terms, after an Add there's 2 refs on the object (1 for the object and
1 for the list). After calling Remove there's just 1 ref on the object.
For the Add callers, calling EndAPI removes the ref for the object and
unlocks it, but since it's in a list the other 1 remains.
Signed-off-by: John Ferlan <jferlan@redhat.com>
If the virSecretLoadValue fails, the code jumped to cleanup without
setting @ret = obj, thus calling virSecretObjListRemove which only
accounts for the object reference related to adding the object to
the list during virSecretObjListAdd, but does not account for the
reference to the object itself as the return of @ret would be NULL
so the caller wouldn't call virSecretObjEndAPI on the object recently
added thus reducing the refcnt to zero.
This patch will perform the ObjListRemove in the failure path of
virSecretLoadValue and Unref @obj in order to perform clean up
and return @obj as NULL. The @def will be freed as part of the
virObjectUnref.
Since the virSecretObjListAdd technically consumes @def on success,
the secretDefineXML should set @def = NULL immediately and process
the remaining calls using a new @objDef variable. We can use use
VIR_STEAL_PTR since we know the Add function just stores @def in
obj->def.
Because we steal @def into @objDef, if we jump to restore_backup:
and @backup is set, then we need to ensure the @def would be
free'd properly, so we'll steal it back from @objDef. For the other
condition this fixes a double free of @def if the code had jumped to
@backup == NULL thus calling virSecretObjListRemove without setting
@def = NULL. In this case, the subsequent call to DefFree would
succeed and free @def; however, the call to EndAPI would also
call DefFree because the Unref done would be the last one for
the @obj meaning the obj->def would be used to call DefFree,
but it's already been free'd because @def wasn't managed right
within this error path.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than assign to a local variable, let's just assign directly to the
object using the error path for cleanup.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This reverts commit 328bd24443.
As it turns out, this is not portable and very Linux & glibc
specific. Worse, this may lead to not starving writers on Linux
but everywhere else. Revert this and if the starvation occurs
resolve it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
The original name didn't hint at the fact that PHBs are
a pSeries-specific concept.
Suggested-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Recent commits made it so that pci-root controllers for
pSeries guests are automatically assigned the
spapr-pci-host-bridge model name; however, that prevents
guests to migrate to older versions of libvirt which don't
know about that model name at all, which at the moment is
all of them :)
To avoid the issue, just strip the model name from PHBs
when formatting the migratable XML; guests that use more
than one PHB are not going to be migratable anyway.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1458708
If the parent provided for the storage pool adapter is not vHBA
capable, then issue a configuration error even though the provided
wwnn/wwpn were found.
It is a configuration error to provide a mismatched parent to
the wwnn/wwpn. The @parent is optional and is used as a means to
perform duplicate pool source checks.
https://bugzilla.redhat.com/show_bug.cgi?id=1472277
Commit id '106930aaa' altered the order of checking for an existing
vHBA (e.g something created via nodedev-create functionality outside
of the storage pool logic) which inadvertantly broke the code to
decide whether to alter/force the fchost->managed field to be 'yes'
because the storage pool will be managing the created vHBA in order
to ensure when the storage pool is destroyed that the vHBA is also
destroyed.
This patch moves the check (and checkParent helper) for an existing
vHBA back into the createVport in storage_backend_scsi. It also
adjusts the checkParent logic to more closely follow the intentions
prior to commit id '79ab0935'. The changes made by commit id '08c0ea16f'
are only necessary to run the virStoragePoolFCRefreshThread when
a vHBA was really created because there's a timing lag such that
the refreshPool call made after a startPool from storagePoolCreate*
wouldn't necessarily find LUNs, but the thread would. For an already
existing vHBA, using the thread is unnecessary since the vHBA already
exists and the lag to configure the LUNs wouldn't exist.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Since virnodedeviceobj now has a self-lockable hash table, there's no
need to lock the table from the driver for processing. Thus remove the
locks from the driver for NodeDeviceObjList mgmt.
This includes the test driver as well.
Rather than use a forward linked list of elements, it'll be much more
efficient to use a hash table to reference the elements by unique name
and to perform hash searches.
This patch does all the heavy lifting of converting the list object to
use a self locking list that contains the hash table. Each of the FindBy
functions that do not involve finding the object by it's key (name) is
converted to use virHashSearch in order to find the specific object.
When searching for the key (name), it's possible to use virHashLookup.
For any of the list perusal functions that are required to evaluate
each object, the virHashForEach function is used.
Alter the node device deletion logic to make use of the parent field
from the obj->def rather than call virNodeDeviceObjListGetParentHost.
As it turns out the saved @def won't have parent_wwnn/wwpn or
parent_fabric_wwn, so the only logical path would be to call
virNodeDeviceObjListGetParentHostByParent which we can accomplish
directly via virNodeDeviceObjListFindByName.
There is no reason why two threads trying to look up two domains
should mutually exclude each other. Utilize new
virObjectRWLockable that was just introduced.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Up until now we only had virObjectLockable which uses mutexes for
mutually excluding each other in critical section. Well, this is
not enough. Future work will require RW locks so we might as well
have virObjectRWLockable which is introduced here.
Moreover, polymorphism is introduced to our code for the first
time. Yay! More specifically, virObjectLock will grab a write
lock, virObjectLockRead will grab a read lock then (what a
surprise right?). This has great advantage that an object can be
made derived from virObjectRWLockable in a single line and still
continue functioning properly (mutexes can be viewed as grabbing
write locks only). Then just those critical sections that can
grab a read lock need fixing. Therefore the resulting change is
going to be way smaller.
In order to avoid writer starvation, the object initializes RW
lock that prefers writers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We already have virRWLockInit. But this uses pthread defaults
which prefer reader to initialize the RW lock. This may lead to
writer starvation. Therefore we need to have the counterpart that
prefers writers. Now, according to the
pthread_rwlockattr_setkind_np() man page setting
PTHREAD_RWLOCK_PREFER_WRITER_NP attribute is no-op. Therefore we
need to use PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
attribute. So much for good enum value names.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
It was observed while adding new property that there should be a space
before closing a curly brace in intel-iommu object property definition.
Fixing it as a separate patch.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Currently, @port is type of string. Well, that's overkill and
waste of memory. Port is always an integer. Use it as such.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Split out parsing of one host into a separate function and add a new
function to loop through all the host XML nodes.
This change removes multiple levels of nesting due to the old XML
parsing approach used.
So the way we format this huge virQEMUCaps enum is we group the
values in groups of five. And then at the beginning of each group
we have a small comment that says what's the number of the first
item in the group. Well, the last commit of 11b2ebf3e1 does not
follow this formatting.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Alter the virStoragePoolObjNumOfVolumes, virStoragePoolObjVolumeGetNames,
and virStoragePoolObjVolumeListExport APIs to take a virStoragePoolObjPtr
instead of the &obj->volumes and obj->def.
Signed-off-by: John Ferlan <jferlan@redhat.com>
A virStoragePoolObjPtr will be an 'obj'.
A virStoragePoolPtr will be a 'pool'.
A virStorageVolPtr will be a 'vol'.
A virStorageVolDefPtr will be a 'voldef'.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than have repetitive code - create/use a couple of helpers:
testStoragePoolObjFindActiveByName
testStoragePoolObjFindInactiveByName
This will also allow for the reduction of some cleanup path logic.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rework some of the test driver API's to remove the need to return
failure when testStoragePoolObjFindByName returns NULL rather than
going to cleanup. This removes the need for check for "if (obj)" and in
some instances the need to for a cleanup label and a local ret variable.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Add a path for UEFI VMs for AArch32 VMs, based on the path Debian is using.
libvirt is the de facto canonical location for defining where distros
should place these firmware images, so let's define this path here to try
and minimize distro fragmentation.
The call to qemuBuildDeviceAddressStr() happens no matter
what, so we can move it to the outer possible scope inside
the function.
We can also move the call to virBufferAsprintf() after all
the checks have been performed, where it makes more sense.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
The virDomainDeviceInfo struct is defined in device_conf,
so generic functions that operate on it should also be
defined there rather than in domain_conf.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
This patch addresses the same aspects on PPC the bug 1103314 addressed
on x86.
PCI expander bus creates multiple primary PCI busses, where each of these
busses can be assigned a specific NUMA affinity, which, on x86 is
advertised through ACPI on a per-bus basis.
For SPAPR, a PHB's NUMA affinities are assigned on a per-PHB basis, and
there is no mechanism for advertising NUMA affinities to a guest on a
per-bus basis. So, even if qemu-ppc manages to get some sort of multi-bus
topology working using PXB, there is no way to expose the affinities
of these busses to the guest. It can only be exposed on a per-PHB/per-domain
basis.
So patch enables NUMA node tag in pci-root controller on PPC.
The way to set the NUMA node is through the numa_node option of
spapr-pci-host-bridge device. However for the implicit PHB, the only way
to set the numa_node is from the -global option. The -global option applies
to all the PHBs unless explicitly specified with the option on the
respective PHB of CLI. The default PHB has the emulated devices only, so
the patch prevents setting the NUMA node for the default PHB.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The patch adds a capability for spapr-pci-host-bridge.numa_node.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Instead of going through two completely different code paths,
one of which repeats the same hardcoded bit of information
three times in rapid succession, depending on whether or not
a firmware list has been provided at configure time, just
provide a reasonable default value and remove the extra code.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
'numad' may return a nodeset which contains NUMA nodes without memory
for certain configurations. Since cgroups code will not be happy using
nodes without memory we need to store only numa nodes with memory in
autoNodeset.
On the other hand autoCpuset should contain cpus also for nodes which
do not have any memory.
A new function virNetDevOpenvswitchUpdateVlan has been created to instruct
OVS of the changes. qemuDomainChangeNet has been modified to handle the
update of the VLAN configuration for a running guest and rely on
virNetDevOpenvswitchUpdateVlan to do the actual update if needed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Preparation for switching to virFileCache where there are two callbacks,
one to get a new data and second one to load a cached data.
This also removes virQEMUCapsReset which is no longer required.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
It's not required and following patches will change the code.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Cleanups the code a little bit and reduces amount of arguments passed
throughout the functions.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
While searching for an element using a function it may be
desirable to know the element key for future operation.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
At present shared disks can be migrated with either readonly or cache=none. But
cache=directsync should be safe for migration, because both cache=directsync and cache=none
don't use the host page cache, and cache=direct write through qemu block layer cache.
Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Reviewed-by: Wang Yechao <wang.yechao255@zte.com.cn>
Use virStorageSource accessors to check the file and call
virStorageFileAccess before even attempting to stat the target. This
will be helpful once we try to add network destinations for block copy,
since there will be no need to stat them.
When copying to a block device, the block device will already exist. To
allow users using a block device without any preparation, they need to
use the block copy without VIR_DOMAIN_BLOCK_COPY_REUSE_EXT.
This means that if the target is an existing block device we don't need
to prepare it, but we can't reject it as being existing.
To avoid breaking this feature, explicitly assume that existing block
devices will be reused even without that flag explicitly specified,
while skipping attempts to create it.
qemuMonitorDriveMirror still needs to honor the flag as specified by the
user, since qemu overwrites the metadata otherwise.
The refactor to split up storage driver into modules broke the apparmor
helper program, since that did not initialize the storage driver
properly and thus detection of the backing chain could not work.
Register the storage driver backends explicitly. Unfortunately it's now
necessary to link with the full storage driver to satisfy dependencies
of the loadable modules.
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
The purpose of this function is to tell if the current position
in given FD is in data section or a hole and how much bytes there
is remaining until the end of the section. This is achieved by
couple of lseeks(). The most important part is that we reposition
the FD back, so that the position is unchanged from the caller
POV. And until now the final lseek() back to the original
position was done with no check for errors. And I was convinced
that that's okay since nothing can go wrong. However, review
feedback from a related series persuaded me, that it's better to
be safe than sorry. Therefore, lets check if the final lseek()
succeeded and if it doesn't report an error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
All the pieces are now in place, so we can finally start
using isolation groups to achieve our initial goal, which is
separating hostdevs from emulated PCI devices while keeping
hostdevs that belong to the same host IOMMU group together.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1280542
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
These rules will make it possible for libvirt to
automatically assign PCI addresses in a way that
respects any isolation constraints devices might
have.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Isolation groups will eventually allow us to make sure certain
devices, eg. PCI hostdevs, are assigned to guest PCI buses in
a way that guarantees improved isolation, error detection and
recovery for machine types and hypervisors that support it,
eg. pSeries guest on QEMU.
This patch merely defines storage for the new information
we're going to need later on and makes sure it is passed from
the hypervisor driver (QEMU / bhyve) down to the generic PCI
address allocation code.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Now that we have a bit more control, let's convert our object into
a lockable object and let that magic handle the create and lock/unlock.
This also involves creating a virNodeDeviceEndAPI in order to handle
the object cleanup for API's that use the Add or Find API's in order
to get a locked/reffed object. The EndAPI will unlock and unref the
object returning NULL to indicate to the caller to not use the obj.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Move the structures to withing virnodedeviceobj.c
Move the typedefs from node_device_conf to virnodedeviceobj.h
Signed-off-by: John Ferlan <jferlan@redhat.com>
In an overall effort to privatize access to virNodeDeviceObj and
virNodeDeviceObjList into the virnodedeviceobj module, move the
object list parsing from node_device_driver and replace with a
call to a virnodedeviceobj helper. This follows other similar
APIs/helpers which peruse the object list looking for some specific
data in order to get/return an @device (virNodeDevice) object to
the caller.
Signed-off-by: John Ferlan <jferlan@redhat.com>
We're about to move the call to nodeDeviceSysfsGetSCSIHostCaps from
node_device_driver into virnodedeviceobj, so move the guts of the code
from the driver specific node_device_linux_sysfs into its own API
since virnodedeviceobj cannot callback into the driver.
Nothing in the code deals with sysfs anyway, as that's hidden by the
various virSCSIHost* and virVHBA* utility function calls.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Create local @obj and @def for the API's rather than referencing the
devs->objs[i][->def->]. It'll make future patches easier to read.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Ensure that any function that walks the node device object list is prefixed
by virNodeDeviceObjList.
Also, modify the @filter param name for virNodeDeviceObjListExport to
be @aclfilter.
Signed-off-by: John Ferlan <jferlan@redhat.com>
In preparation to make things private, make the ->devs be pointers to a
virNodeDeviceObjList and then manage everything inside virnodedeviceobj
Signed-off-by: John Ferlan <jferlan@redhat.com>
Create an allocator for the virNodeDeviceObjPtr - include setting up
the mutex, saving the virNodeDeviceDefPtr, and locking the return object.
Signed-off-by: John Ferlan <jferlan@redhat.com>
- In testDestroyVport rather than use a cleanup label, just return -1
immediately since nothing else is needed.
- In testStoragePoolDestroy, if !privpool, then just return -1 since
nothing else will happen anyway.
- Rather than "goto cleanup;" on failure to virNodeDeviceObjFindByName
an @obj, just return directly. This then allows the cleanup: label code
to not have to check "if (obj)" before calling virNodeDeviceObjUnlock.
This also simplifies some exit logic...
- In testNodeDeviceObjFindByName use an error: label to handle the failure
and don't do the ncaps++ within the VIR_STRDUP() source target index.
Only increment ncaps after success. Easier on eyes at error label too.
- In testNodeDeviceDestroy use "cleanup" rather than "out" for the goto
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than passing the object to be removed by reference, pass by value
and then let the caller decide whether or not the object should be free'd
and how to handle the logic afterwards. This includes free'ing the object
and/or setting the local variable to NULL to prevent subsequent unexpected
usage (via something like virNodeDeviceObjRemove in testNodeDeviceDestroy).
For now this function will just handle the remove of the object from the
list for which it was placed during virNodeDeviceObjAssignDef.
This essentially reverts logic from commit id '61148074' that free'd the
device entry on list, set *dev = NULL and returned. Thus fixing a bug in
node_device_hal.c/dev_refresh() which would never call dev_create(udi)
since @dev would have been set to NULL.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Recent refactors made it so that the function may use uninitialized
pointer, but it actually wanted to use a different variable and value
at all.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This reverts commit b3e71a8830.
As it turns out this ends up very badly as the @def could be Free'd
even though it's owned by @obj as a result of the AssignDef.
New API will be virNWFilterInstantiateFilterInternal as it's called from
the virNWFilterInstantiateFilter and virNWFilterUpdateInstantiateFilter.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rename to virNWFilterDoInstantiate to better describe the action.
Also fix the @vmuuid parameter to not have the ATTRIBUTE_UNUSED since it
is used in the call to virNWFilterDHCPSnoopReq.
Signed-off-by: John Ferlan <jferlan@redhat.com>
When looking for slots suitable for a PCI device, libvirt
might need to add an extra PCI controller: for pSeries guests,
we want that extra controller to be a PHB (pci-root) rather
than a PCI bridge.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
PCI bus has to be numbered sequentially, and no index can be
missing, so libvirt will fill in the blanks automatically for
the user.
Up until now, it has done so using either pci-bridge, for machine
types based on legacy PCI, or pcie-root-port, for machine types
based on PCI Express. Neither choice is good for pSeries guests,
where PHBs (pci-root) should be used instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Now that the multi-phb support series is in, work on the TODO at
qemuDomainGetMemLockLimitBytes() to arrive at the correct memlock limit
value.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Additional PHBs (pci-root controllers) will be created for
the guest using the spapr-pci-host-bridge QEMU device, if
available; the implicit default PHB, while present in the
guest configuration, will be skipped.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1431193
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Usually, a controller with alias 'x' will create a bus with the
same name; however, the bus created by a PHBs with alias 'x' will
be named 'x.0' instead, so we need to account for that.
As an exception to the exception, the implicit PHB that's added
automatically to every pSeries guest creates the 'pci.0' bus.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
This new capability can be used to detect whether a QEMU
binary supports the spapr-pci-host-bridge controller.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
pSeries guests will soon need the new information; luckily,
we can figure it out automatically most of the time, so
users won't have to worry about it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Adding it to the virDomainControllerPCIModelName enumeration
is enough for existing code to handle it, so parsing and
formatting will work without further tweaking.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
pSeries guests will soon be allowed to have multiple
PHBs (pci-root controllers), meaning the current check
on the controller index no longer applies to them.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
pSeries guests will soon be allowed to have multiple
PHBs (pci-root controllers), which of course means that
all but one of them will have a non-zero index; hence,
we'll need to relax the current check.
However, right now the check is performed in the conf
module, which is generic rather than tied to the QEMU
driver, and where we don't have information such as the
guest machine type available.
To make this change of behavior possible down the line,
we need to move the check from the XML parser to the
drivers. Luckily, only QEMU and bhyve are using PCI
controllers, so this doesn't result in much duplication.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Moving the check and rewriting it this way doesn't alter
the current behavior, but will allow us to special-case
pci-root down the line.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
We will soon need to be able to return a NULL pointer
without the caller considering that an error: to make
it possible, change the return type to int and use
an out parameter for the string instead.
Add some documentation for the function as well.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
The current algorithm for slot allocation tries to be clever
and avoid looking at buses / slots more than once unless it's
necessary. Unfortunately that makes the code more complex,
and it will cause problem later on in some situations unless
even more complex code is added.
Since the performance gains are going to be pretty modest
anyway, we can just get rid of the extra complexity and use a
completely straighforward implementation instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
We use hostdev->info frequently enough that having
a shorter name for it makes the code more readable.
We will also be adding even more uses later on.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Move @function after @flags to match other functions in the
same module like virDomainPCIAddressReserveNextAddr().
Also move virDomainPCIAddressReserveNextAddr() closer to
virDomainPCIAddressReserveAddr() in the header file.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
This function was private to the QEMU driver and was,
accordingly, called qemuDomainPCIBusFullyReserved().
However the function is really not QEMU-specific at
all, so it makes sense to move it closer to the
virDomainPCIAddressBus struct it operates on.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
The virDomainDeviceInfoIsSet() function no longer exists.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Modify code to have two spaces between functions, follow function more
recent function formatting w/r/t args per line and function return type
and name on separate lines.
Signed-off-by: John Ferlan <jferlan@redhat.com>
New name is qemuBlockStorageSourceGetGlusterProps and also hardcode the
protocol name rather than calling the ToString function, since this
function can't be made universal.
New name is qemuBlockStorageSourceBuildHostsJSONSocketAddress since it
formats the JSON object in accordance with qemu's SocketAddress type.
Since the new naming in qemu uses 'inet' instead of 'tcp' add a
compatibility layer for gluster which uses the old name.
Rename it to qemuBlockStorageSourceGetBackendProps and refactor it to
return the JSON object instead of filling a pointer since now it's
always expected to return data.
Add logic which will call qemuGetDriveSourceProps only in cases where we
need the JSON representation. This will allow qemuGetDriveSourceProps to
generate the JSON representation for all possible disk sources.
The command line generators for the protocols above hardcoded a default
port number. Since we now always assign it when parsing the source
definition, this ad-hoc code is not required any more.
Fill them in right away rather than having to figure out at runtime
whether they are necessary or not.
virStorageSourceNetworkDefaultPort does not need to be exported any
more.
Since we're storing a virUUIDFormat'd string in our Hash Table, let's
modify the Lookup API to receive a formatted string as well.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This reverts commit e4b980c853.
When a binary links against a .a archive (as opposed to a shared library),
any symbols which are marked as 'weak' get silently dropped. As a result
when the binary later runs, those 'weak' functions have an address of
0x0 and thus crash when run.
This happened with virtlogd and virtlockd because they don't link to
libvirt.so, but instead just libvirt_util.a and libvirt_rpc.a. The
virRandomBits symbols was weak and so left out of the virtlogd &
virtlockd binaries, despite being required by virHashTable functions.
Various other binaries like libvirt_lxc, libvirt_iohelper, etc also
link directly to .a files instead of libvirt.so, so are potentially
at risk of dropping symbols leading to a later runtime crash.
This is normal linker behaviour because a weak symbol is not treated
as undefined, so nothing forces it to be pulled in from the .a You
have to force the linker to pull in weak symbols using -u$SYMNAME
which is not a practical approach.
This risk is silent bad linkage that affects runtime behaviour is
not acceptable for a fix that was merely trying to fix the test
suite. So stop using __weak__ again.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When libvirt starts a new QEMU domain, it replaces host-model CPUs with
the appropriate custom CPU definition. However, when reconnecting to a
domain started by older libvirt (< 2.3), the domain would still have a
host-model CPU in its active definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1463957
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
qemuProcessReconnect will need to call additional functions which were
originally defined further in qemu_process.c.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Separated from qemuProcessUpdateAndVerifyCPU to handle updating of an
active guest CPU definition according to live data from QEMU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In addition to updating a guest CPU definition the function verifies
that all required features are provided to the guest. Let's make it
obvious by calling it qemuProcessUpdateAndVerifyCPU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Separated from qemuProcessUpdateLiveGuestCPU. The function makes sure
a guest CPU provides all features required by a domain definition.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Separated from qemuProcessUpdateLiveGuestCPU. Its purpose is to fetch
guest CPU data from a running QEMU process. The data can later be used
to verify and update the active guest CPU definition.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
CPU features unknown to a hypervisor will not be present in dataDisabled
even though the features won't naturally be enabled because.
Thus any features we asked for which are not in dataEnabled should be
considered disabled.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When checking ABI stability between two domain definitions, we first
make migratable copies of them. However, we also asked for the guest CPU
to be updated, even though the updated CPU is supposed to be already
included in the original definitions. Moreover, if we do this on the
destination host during migration, we're potentially updating the
definition with according to an incompatible host CPU.
While updating the CPU when checking ABI stability doesn't make any
sense, it actually just worked because updating the CPU doesn't do
anything for custom CPUs (only host-model CPUs are affected) and we
updated both definitions in the same way.
Less then a year ago commit v2.3.0-rc1~42 stopped updating the CPU in
the definition we got internally and only the user supplied definition
was updated. However, the same commit started updating host-model CPUs
to custom CPUs which are not affected by the request to update the CPU.
So it still seemed to work right, unless a user upgraded libvirt 2.2.0
to a newer version while there were some domains with host-model CPUs
running on the host. Such domains couldn't be migrated with a user
supplied XML since libvirt would complain:
Target CPU mode custom does not match source host-model
The fix is pretty straightforward, we just need to stop updating the CPU
when checking ABI stability.
https://bugzilla.redhat.com/show_bug.cgi?id=1463957
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Currently the scan of the /proc/mounts file used to find cgroup mount
points doesn't take into account that mount points may hidden by other
mount points. For, example in certain Kubernetes environments the
/proc/mounts contains the following lines:
cgroup /sys/fs/cgroup/net_prio,net_cls cgroup ...
tmpfs /sys/fs/cgroup tmpfs ...
cgroup /sys/fs/cgroup/net_cls,net_prio cgroup ...
In this particular environment the first mount point is hidden by the
second one. The correct mount point is the third one, but libvirt will
never process it because it only checks the first mount point for each
controller (net_cls in this case). So libvirt will try to use the first
mount point, which doesn't actually exist, and the complete detection
process will fail.
To avoid that issue this patch changes the virCgroupDetectMountsFromFile
function so that when there are duplicates it takes the information from
the last line in /proc/mounts. This requires removing the previous
explicit condition to skip duplicates, and adding code to free the
memory used by the processing of duplicated lines.
Related-To: https://bugzilla.redhat.com/1468214
Related-To: https://github.com/kubevirt/libvirt/issues/4
Signed-off-by: Juan Hernandez <jhernand@redhat.com>
After 426dc5eb2 qemuCaps and virDomainDefPtr are unused here,
remove it from the call stack
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Obviously, old gcc-s ale sad when a variable shares the name with
a function. And we do have such variable (added in 4d8a914be0):
@mount. Rename it to @mountpoint so that compiler's happy again.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The way we create devices under /dev is highly linux specific.
For instance we do mknod(), mount(), umount(), etc. Some
platforms are even missing some of these functions. Then again,
as declared in qemuDomainNamespaceAvailable(): namespaces are
linux only. Therefore, to avoid obfuscating the code by trying to
make it compile on weird platforms, just provide a non-linux stub
for qemuDomainAttachDeviceMknodRecursive(). At the same time,
qemuDomainAttachDeviceMknodHelper() which actually calls the
non-existent functions is moved under ifdef __linux__ block since
its only caller is in that block too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1467826
Commit id 'b9b1aa639' was supposed to add logic to set the allocation
for sparse files when wr_highest_offset was zero; however, an unconditional
setting was done just prior. For block devices, this means allocation is
always returning 0 since 'actual-size' will be zero.
Remove the unconditional setting and add the note about it being possible
to still be zero for block devices. As soon as the guest starts writing to
the volume, the allocation value will then be obtainable from qemu via
the wr_highest_offset.
The Win32 platform will fail to link if you use weak symbols
because it is incompatible with exporting symbols in a DLL:
Cannot export virRandomGenerateWWN: symbol wrong type (2 vs 3)
We only need weak symbols for our test suite to do LD_PRELOAD
and this doesn't work on Win32, so we can just drop the hack
for Win32
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If we exceed a fixed limit in RPC code we get a horrible message
like this, if the parameter type is a 'string', because we forgot
to initialize the error message type field:
$ virsh snapshot-list ostack1
error: too many remote undefineds: 1329 > 1024
It would also be useful to know which RPC call and field was
exceeded. So this patch makes us report:
$ virsh snapshot-list ostack1
error: too many remote undefineds: 1329 > 1024,
in parameter 'names' for 'virDomainSnapshotListNames'
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
On domain startup, bind host or bind service can be omitted
and we will format a working command line.
Extend this to hotplug as well and specify the service to QEMU
even if the host is missing.
https://bugzilla.redhat.com/show_bug.cgi?id=1452441
Currently all mockable functions are annotated with the 'noinline'
attribute. This is insufficient to guarantee that a function can
be reliably mocked with an LD_PRELOAD. The C language spec allows
the compiler to assume there is only a single implementation of
each function. It can thus do things like propagating constant
return values into the caller at compile time, or creating
multiple specialized copies of the function body each optimized
for a different caller. To prevent these optimizations we must
also set the 'noclone' and 'weak' attributes.
This fixes the test suite when libvirt.so is built with CLang
with optimization enabled.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The TODO macro expands to an fprintf() call and is used in several
places in the Xen driver. Anything that wishes to print such debug
messages should use the logging macros. In this case though, all the
places in the Xen driver should have been raising a formal libvirt
error instead. Add proper error handling and delete the TODO macro
to prevent future misuse.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The HOST_NAME_MAX, INET_ADDRSTRLEN and VIR_LOOPBACK_IPV4_ADDR
constants are only used by a handful of files, so are better
kept in virsocketaddr.h or the source file that uses them.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We only ever test libvirt with GCC or CLang which provides a
GCC compatible compilation environment. Between them, these
compilers cover every important operating system platform,
even Windows.
Mandate their use to make it explicit that we don't care about
compilers like Microsoft VCC or other UNIX vendor C compilers.
GCC 4.4 was picked as the baseline, since RHEL-6 ships 4.4.7
and that lets us remove a large set of checks. There is a slight
issue that CLang reports itself as GCC 4.2, so we must also check
if __clang__ is defined. We could check a particular CLang version
too, but that would require someone to figure out a suitable min
version which is fun because OS-X reports totally different CLang
version numbers from CLang builds on Linux/BSD
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Back in this commit:
commit b436a8ae5c
Author: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Date: Thu Jun 9 00:50:35 2016 +0000
gnulib: add getopt module
config-post.h was modified to define __GNUC_PREREQ, but the
original definition was never removed from internal.h, and
that is now dead code since config.h is always the first file
included.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently, the only type of chardev that we create the backend
for in the namespace is type='dev'. This is not enough, other
backends might have files under /dev too. For instance channels
might have a unix socket under /dev (well, bind mounted under
/dev from a different place).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1462060
Just like in the previous commit, when attaching a file based
device which has its source living under /dev (that is not a
device rather than a regular file), calling mknod() is no help.
We need to:
1) bind mount device to some temporary location
2) enter the namespace
3) move the mount point to desired place
4) umount it in the parent namespace from the temporary location
At the same time, the check in qemuDomainNamespaceSetupDisk makes
no longer sense. Therefore remove it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1462060
When building a qemu namespace we might be dealing with bare
regular files. Files that live under /dev. For instance
/dev/my_awesome_disk:
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/dev/my_awesome_disk'/>
<target dev='vdc' bus='virtio'/>
</disk>
# qemu-img create -f qcow2 /dev/my_awesome_disk 10M
So far we were mknod()-ing them which is
obviously wrong. We need to touch the file and bind mount it to
the original:
1) touch /var/run/libvirt/qemu/fedora.dev/my_awesome_disk
2) mount --bind /dev/my_awesome_disk /var/run/libvirt/qemu/fedora.dev/my_awesome_disk
Later, when the new /dev is built and replaces original /dev the
file is going to live at expected location.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Currently, we silently assume that file we are creating in the
namespace is either a link or a device (character or block one).
This is not always the case. Therefore instead of doing something
wrong, claim about unsupported file type.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Currently, we silently assume that file we are creating in the
namespace is either a link or a device (character or block one).
This is not always the case. Therefore instead of doing something
wrong, claim about unsupported file type.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This function is going to be used on other places, so
instead of copying code we can just call the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1459592
In 290a00e41d I've tried to fix the process of building a
qemu namespace when dealing with file mount points. What I
haven't realized then is that we might be dealing not with just
regular files but also special files (like sockets). Indeed, try
the following:
1) socat unix-listen:/tmp/soket stdio
2) touch /dev/socket
3) mount --bind /tmp/socket /dev/socket
4) virsh start anyDomain
Problem with my previous approach is that I wasn't creating the
temporary location (where mount points under /dev are moved) for
anything but directories and regular files.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
It comes very handy to have source path for chardevs. We already
have such function: virDomainAuditChardevPath() but it's static
and has name not suitable for exposing. Moreover, while exposing
it change its name slightly to virDomainChrSourceDefGetPath.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If a value of the first level object contains more objects needing
deflattening which would be wrapped in an actual object the function
would not recurse into them.
By this simple addition we can fully deflatten the objects.
As it turns out sometimes users pass in an arbitrarily nested structure
e.g. for the qemu backing chains JSON pseudo protocol. This new
implementation deflattens now a single object fully even with nested
keys.
Additionally it's not necessary now to stick with the "file." prefix for
the properties.
Currently the function would deflatten the object by dropping the 'file'
prefix from the attributes. This does not really scale well or adhere to
the documentation.
Until we refactor the worker to properly deflatten everything we at
least simulate it by adding the "file" wrapper object back.
Users may want to run the init command of a container as a special
user / group. This is achieved by adding <inituser> and <initgroup>
elements. Note that the user can either provide a name or an ID to
specify the user / group to be used.
This commit also fixes a side effect of being able to run the command
as a non-root user: the user needs rights on the tty to allow shell
job control.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Some containers may want the application to run in a special directory.
Add <initdir> element in the domain configuration to handle this case
and use it in the lxc driver.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
virCommand is a version of virExec that doesn't fork, however it is
just calling execve and doesn't honors setting uid/gid and pwd.
This commit extrac those pieces from virExec() to a virExecCommon()
function that is called from both virExec() and virCommandExec().
When running an application container, setting environment variables
could be important.
The newly introduced <initenv> tag in domain configuration will allow
setting environment variables to the init program.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
All of these four functions (virStreamRecvAll, virStreamSendAll,
virStreamSparseRecvAll, virStreamSparseSendAll) take one or more
callback functions that handle various aspects of streams.
However, if any of them fails no error is reported therefore
caller does not know what went wrong.
At the same time, we silently presumed callbacks to set errno on
failure. With this change we should document it explicitly as the
error is not properly reported.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If one these four functions fail (virStreamRecvAll,
virStreamSendAll, virStreamSparseRecvAll, virStreamSparseSendAll)
the stream is aborted by calling virStreamAbort(). This is a
public API; therefore, the first thing it does is error reset. At
that point any error that caused us to abort stream in the first
place is gone.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Our documentation to the virStreamRecvAll, virStreamSendAll,
virStreamSparseRecvAll, and virStreamSparseSendAll functions
indicates that if these functions fail, then virStreamAbort is
called. But that is not necessarily true. For instance all of
these functions allocate a buffer to work with. If the allocation
fails, no virStreamAbort() is called despite -1 being returned.
It's the same story with argument sanity checks and a lot of
other checks.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Problem with our error reporting is that the error object is a
thread local variable. That means if there's an error reported
within the I/O thread it gets logged and everything, but later
when the event loop aborts the stream it doesn't see the original
error. So we are left with some generic error. We can do better
if we copy the error message between the threads.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When the I/O thread quits (e.g. due to an I/O error, lseek()
error, whatever), any subsequent virFDStream API should return
error too. Moreover, when invoking stream event callback, we must
set the VIR_STREAM_EVENT_ERROR flag so that the callback knows
something bad happened.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This is only used in qemu_command.c, so move it, and clarify that
it's really about identifying if the serial config is a platform
device or not.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Some qemu arch/machine types have built in platform devices that
are always implicitly available. For platform serial devices, the
current code assumes that only old style -serial config can be
used for these devices.
Apparently though since -chardev was introduced, we can use -chardev
in these cases, like this:
-chardev pty,id=foo
-serial chardev:foo
Since -chardev enables all sorts of modern features, use this method
for platform devices.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Every qemu version we support has QEMU_CAPS_CHARDEV, so stop
explicitly tracking it and blacklist it like we've done for many
other feature flags.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
AFAIK there aren't any cases where we will/should hit the old code
path for our supported qemu versions, so drop the old code.
Massive test suite churn follows
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
AFAIK there aren't any cases where we should fail these checks with
supported qemu versions, so just drop them.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
AFAIK there aren't any qemu arch/machine types with platform parallel
devices that would require old style -parallel config, so we shouldn't
ever need this nowadays.
Remove a now redundant test
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Rather than try to whitelist all device configs that can't use
-chardev, blacklist the only one that really can't, which is the
default serial/console target type=isa case.
ISA specifically isn't a valid config for arm/aarch64, but we've
always implicitly treated it to mean 'default platform device'.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
vcpu properties gathered from query-hotpluggable cpus need to be passed
back to qemu. As qemu did not use the node-id property until now and
libvirt forgot to pass it back properly (it was parsed but not passed
around) we did not honor this.
This patch adds node-id to the structures where it was missing and
passes it around as necessary.
The test data was generated with a VM with following config:
<numa>
<cell id='0' cpus='0,2,4,6' memory='512000' unit='KiB'/>
<cell id='1' cpus='1,3,5,7' memory='512000' unit='KiB'/>
</numa>
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1452053
Introduces support for virDomainSetMemory. This also serves an an
example for how to use the new method invocation API with a more
complicated method, this time including an EPR and embedded param.
This commit adds support for virDomainSendKey. It also serves as an
example of how to use the new method invocation APIs with a single
"simple" type parameter.
Update the generator to generate basic property type information for
each CIM object representation. Right now, it generates arrays of
hypervCimType structs:
struct _hypervCimType {
const char *name;
const char *type;
bool isArray;
};
This commit introduces functionality for creating and working with
invoke parameters. This commit does not include any code for serializing
and actually performing the method invocations; it merely defines the
functions and API for using invocation parameters in driver code.
HYPERV_DEFAULT_PARAM_COUNT was chosen because almost no method
invocations have more than 4 parameters.
Functions added:
* hypervInitInvokeParamsList
* hypervFreeInvokeParams
* hypervAddSimpleParam
* hypervAddEprParam
* hypervCreateEmbeddedParam
* hypervSetEmbeddedProperty
* hypervAddEmbeddedParam
* hypervFreeEmbeddedParam
The log category for virnetdaemon.c was mistakenly set
to rpc.netserver. Some useful info about the inhibitor
file descriptor was also never logged.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The DBus conditional was renamed way back:
commit da77f04ed5
Author: Daniel P. Berrange <berrange@redhat.com>
Date: Thu Sep 20 15:05:39 2012 +0100
Convert HAVE_DBUS to WITH_DBUS
but the shutdown inhibit code was not updated. Thus libvirt
was never inhibiting shutdown by a logged in user when VMs
are running.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
It's obvious that unsigned long long is 64 bit and also our web page
generator would misplace the comment after the return value due to the
way it's parsing them.
vcpu 0 must be always enabled and non-hotpluggable, thus you can't
modify it using the vcpu hotplug APIs. Disallow it so that users can't
create invalid configurations.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1459785
The default conf files, for example libvirtd.conf, virtlockd.conf, and
virtlogd.conf, should be located under the directory "/etc/libvirt" when
root as root, rather than "/etc". When run as non-root, the configuration
files should be located under "$XDG_CONFIG_HOME/libvirt/", rather than
"XDG_CONFIG_HOME".
Signed-off-by: Lily Zhu <lizhu@redhat.com>
Signed-off-by: Erik Skultety <eskultet@redhat.com>
When one has a non-blocking stream and aborts or finishes it without
removing the callback, any event loop invocation will trigger that
callback, but it cannot be removed any more. We cannot remove the
callback automatically from virStream{Abort,Finish} functions due to
forward-compatibility. So let's at least document this behaviour,
because it is not easy to find out the reason for.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Add support for vgaconf driver configuration. In domain xml it looks like
this:
<video>
<driver vgaconf='io|on|off'>
<model .../>
</video>
It was added with bhyve gop video in mind to allow users control how the
video device is exposed to the guest, specifically, how VGA I/O is
handled.
One can refer to the bhyve manual page to get more detailed description
of the possible VGA configuration options:
https://www.freebsd.org/cgi/man.cgi?query=bhyve&manpath=FreeBSD+12-current
The relevant part could be found using the 'vgaconf' keyword.
Also, add some tests for this new feature.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Commit 54fa1b44af added virDomainDeviceInfo::loadparm
and updated virDomainDeviceInfoClear() accordingly, but
omitted the necessary virDomainDeviceInfoCopy() changes.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
While qemuProcessIncomingDefNew takes an fd argument and stores it in
qemuProcessIncomingDef structure, the caller is still responsible for
closing the file descriptor.
Introduced by commit v1.2.21-140-ge7c6f4575.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Since qemu commit 3ef6c40ad0b it can fail if trying to hotplug a
disk that is not qcow2 despite us saying it is. We need to error
out in that case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If a remote call fails during event registration (more than likely from
a network failure or remote libvirtd restart timed just right), then when
calling the virObjectEventStateDeregisterID we don't want to call the
registered @freecb function because that breaks our contract that we
would only call it after succesfully returning. If the @freecb routine
were called, it could result in a double free from properly coded
applications that free their opaque data on failure to register, as seen
in the following details:
Program terminated with signal 6, Aborted.
#0 0x00007fc45cba15d7 in raise
#1 0x00007fc45cba2cc8 in abort
#2 0x00007fc45cbe12f7 in __libc_message
#3 0x00007fc45cbe86d3 in _int_free
#4 0x00007fc45d8d292c in PyDict_Fini
#5 0x00007fc45d94f46a in Py_Finalize
#6 0x00007fc45d960735 in Py_Main
#7 0x00007fc45cb8daf5 in __libc_start_main
#8 0x0000000000400721 in _start
The double dereference of 'pyobj_cbData' is triggered in the following way:
(1) libvirt_virConnectDomainEventRegisterAny is invoked.
(2) the event is successfully added to the event callback list
(virDomainEventStateRegisterClient in
remoteConnectDomainEventRegisterAny returns 1 which means ok).
(3) when function remoteConnectDomainEventRegisterAny is hit,
network connection disconnected coincidently (or libvirtd is
restarted) in the context of function 'call' then the connection
is lost and the function 'call' failed, the branch
virObjectEventStateDeregisterID is therefore taken.
(4) 'pyobj_conn' is dereferenced the 1st time in
libvirt_virConnectDomainEventFreeFunc.
(5) 'pyobj_cbData' (refered to pyobj_conn) is dereferenced the
2nd time in libvirt_virConnectDomainEventRegisterAny.
(6) the double free error is triggered.
Resolve this by adding a @doFreeCb boolean in order to avoid calling the
freeCb in virObjectEventStateDeregisterID for any remote call failure in
a remoteConnect*EventRegister* API. For remoteConnect*EventDeregister* calls,
the passed value would be true indicating they should run the freecb if it
exists; whereas, it's false for the remote call failure path.
Patch based on the investigation and initial patch posted by
fangying <fangying1@huawei.com>.
The function to check if -chardev is supported by QEMU was written a
long time ago, where adding chardevs did not make sense on the fixed ARM
platforms. Since then, we now have a general purpose virt platform,
which should support plugging in any device over PCIe which is supported
in a similar fashion on x86.
Signed-off-by: Christoffer Dall <cdall@linaro.org>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1371892
As it turns out the volume create, build, and refresh path was not peeking
at the meta data, so immediately after a create operation the value displayed
for capacity was still incorrect. However, if a pool refresh was done the
correct value was fetched as a result of a meta data peek.
The reason is it seems historically if the file type is RAW then peeking
at the file just took the physical value for the capacity. However, since
we know if it's an encrypted file, then peeking at the meta data will be
required in order to get a true capacity value.
So check for encryption in the source and if present, use the meta data
in order to fill in the capacity value and set the payload_offset.
Similarly to commit 5da28cc306 this check
actually does not make sense since duplicate WWNs are used e.g. when
multipathing disks.
This reverts commit 780fe4e4ba.
https://bugzilla.redhat.com/show_bug.cgi?id=1461270
When fetching stats for a vhost-user type of interface, we run
couple of ovs-vsctl commands and parse their output. However, not
all stats exist at all times, for instance "rx_dropped" or
"tx_errors" can be missing. Thing is, we ask for a bulk of
statistics and if one of them is missing an error is reported
instead of returning the rest. Since we ignore errors, we fail to
set statistics. Fix this by asking for each piece alone.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit 5c54d29aae forgot to do that when moving the only function
using it and it broke the build on some platforms.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Our commit e13e8808f9 was way too generic. Currently, virtlogd is
used only for chardevs type of file and nothing else. True, we
must not relabel the path in this case, but we have to in all
other cases. For instance, if you want to have a physical console
attached to your guest:
<console type='dev'>
<source path='/dev/ttyS0'/>
<target type='virtio' port='1'/>
</console>
Starting such domain fails because qemu doesn't have access to
/dev/ttyS0 because we haven't relabelled the path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This commit fixes a locale problem with locales that use comma as a mantissa
separator. Example: 12.34 en_US = 12,34 pt_BR. Since strtod() is a non-safe
function, virStrToDouble() will have problems to parse double numbers from
kernel settings and other double numbers from static files (XMLs, JSONs, etc).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1457634
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1457481
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
The function virDoubleToStr() is defined in virutil.* and virStrToDouble() is
defined in virstring.*. Joining both functions into the same file makes more
sense.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Even though we got both the original CPU (used for starting a domain)
and the updated version (the CPU really provided by QEMU) during
incoming migration, restore, or snapshot revert, we still need to update
the CPU according to the data we got from the freshly started QEMU.
Otherwise we don't know whether the CPU we got from QEMU matches the one
before migration. We just need to keep the original CPU in
priv->origCPU.
Messed up by me in v3.4.0-58-g8e34f4781.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This function is called unconditionally from qemuProcessStop to
make sure we leave no dangling dirs behind. However, whenever the
directory we want to rmdir() is not there (e.g. because it hasn't
been created in the first place because domain doesn't use
hugepages at all), we produce a warning like this:
2017-06-20 15:58:23.615+0000: 32638: warning :
qemuProcessBuildDestroyHugepagesPath:3363 : Unable to remove
hugepage path: /dev/hugepages/libvirt/qemu/1-instance-00000001
(errno=2)
Fix this by not producing the warning on ENOENT.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Similarly to how we specify the groups of 5 capabilities in the header
file move the labels to separate line also for the VIR_ENUM_IMPL part.
This simplifies rebase conflict resolution in the capability file since
only lines have to be shuffled around, but they don't need to be edited.
Commit 7456c4f5f introduced a regression by not reloading the backing
chain of a disk after snapshot. The regression was caused as
src->relPath was not set and thus the block commit code could not
determine the relative path.
This patch adds code that will load the backing store string if
VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT and store it in the correct place
when a snapshot is successfully completed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1461303
It is necessary for some parts of the code to refresh just data
based on the based on the backing store string. Add a convenience
function that will retrieve this data.
Changing labelling of the images does not need to happen after setting
the labeling and lock manager access. This saves the cleanup of the
labeling if the relative path can't be determined.
Check for the LOADPARM capabilility and potentially add a loadparm=x to
the "-machine" string for the QEMU command line.
Also add xml2argv test cases for loadparm.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Add new capability for the "-machine loadparm" QEMU option.
Add the capabilities replies/xml for s390x for QEMU 2.9.50.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com>
Update the per device boot schema to add an optional loadparm parameter.
eg: <boot order='1' loadparm='2'/>
Extend the virDomainDeviceInfo to support loadparm option.
Modify the appropriate functions to parse loadparm from boot device xml.
Add the xml2xml test to validate the field.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Starting from qemu 2.9, more granular options are supported. Add parser
for the relevant bits.
With this patch libvirt is able to parse the host and target IQN of from
the JSON pseudo-protocol specification.
This corresponds to BlockdevOptionsIscsi in qemu qapi.
'SocketAddress' structure was changed to contain 'inet' instead of
'tcp' since qemu commit c5f1ae3ae7b. Existing entries have a backward
compatibility layer.
Libvirt will parse 'inet' and 'tcp' as equivalents.
It was added in commit 6c2e4c3856
so that Coverity would not complain about passing -1 to
qemuDomainDetachThisHostDevice(), but the function in question
has changed since and so the annotation doesn't apply anymore.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The same json strucutre is used for NBD and sheepdog volumes for
specifying of the host. Rename the function and fix up error messages to
be more universal.
When added in multiple previous commits, it was used only with -device
qxl(-vga), but for some QEMUs (< 1.6) we need to add this
functionality when using -vga qxl as well.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1283207
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
In the case that virtlogd is used as stdio handler we pass to QEMU
only FD to a PIPE connected to virtlogd instead of the file itself.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1430988
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Improve the code to decide whether to use virtlogd or not by checking
the same variable that is updated in qemuProcessPrepareDomain().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In QEMU driver we can use virtlogd as stdio handler for source backend
of char devices if current QEMU is new enough and it's enabled in
qemu.conf. We should store this information while starting a guest
because the config option may change while the guest is running.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1431112
Imagine a FS mounted on /dev/blah/blah2. Our process of creating
suffix for temporary location where all the mounted filesystems
are moved is very simplistic. We want:
/var/run/libvirt/qemu/$domName.$suffix\
were $suffix is just the mount point path stripped of the "/dev/"
prefix. For instance:
/var/run/libvirt/qemu/fedora.mqueue for /dev/mqueue
/var/run/libvirt/qemu/fedora.pts for /dev/pts
and so on. Now if we plug /dev/blah/blah2 into the example we see
some misbehaviour:
/var/run/libvirt/qemu/fedora.blah/blah2
Well, misbehaviour if /dev/blah/blah2 is a file, because in that
case we call virFileTouch() instead of virFileMakePath().
The solution is to replace all the slashes in the suffix with say
dots. That way we don't have to care about nested directories.
IOW, the result we want for given example is:
/var/run/libvirt/qemu/fedora.blah.blah2
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1431112
There can be nested mount points. For instance /dev/shm/blah can
be a mount point and /dev/shm too. It doesn't make much sense to
return the former path because callers preserve the latter (and
with that the former too). Therefore prune nested mount points.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1431112
After 290a00e41d we know how to deal with file mount points.
However, when cleaning up the temporary location for preserved
mount points we are still calling rmdir(). This won't fly for
files. We need to call unlink(). Now, since we don't really care
if the cleanup succeeded or not (it's the best effort anyway), we
can call both rmdir() and unlink() without need for
differentiation between files and directories.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
On some platforms the number of bits in the cbm_mask might not be
divisible by 4 (and not even by 2), so we need to properly count the
bits. Similar file, min_cbm_bits, is properly parsed and used, but if
the number is greater than one, we lose the information about
granularity when reporting the data in capabilities. For that matter
always report granularity, but if it is not the same as the minimum,
add that information in there as well.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The split firmware and variables files introduced by
https://bugs.debian.org/764918 are in a different directory for
some reason. Let the virtual machine read both.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Change the settings from qemuDomainUpdateDeviceLive() as otherwise the
call would succeed even though nothing has changed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1414627
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Most places which want to check ABI stability for an active domain need
to call this API rather than the original
qemuDomainDefCheckABIStability. The only exception is in snapshots where
we need to decide what to do depending on the saved image data.
https://bugzilla.redhat.com/show_bug.cgi?id=1460952
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When making ABI stability checks for an active domain, we need to make
sure we use the same migratable definition which virDomainGetXMLDesc
with the MIGRATABLE flag provides, otherwise the ABI check will fail.
This is implemented in the new qemuDomainCheckABIStability which takes a
domain object and generates the right migratable definition from it.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This patch separates the actual ABI checks from getting migratable defs
in qemuDomainDefCheckABIStability so that we can create another wrapper
which will use different methods to get the migratable defs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The main goal of this function is to enable reusing the parsing code
from qemuDomainDefCopy.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1214369
My fix 671d18594f was incomplete. If domain doesn't have
hugepages enabled, because of missing condition we would still be
putting hugepages path onto qemu cmd line. Clean up the
conditions so that it's more visible next time.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
With glibc >= 2.25.90 writev() is only available if you explicitly
include sys/uio.h. This matches the documented requirements, but
older glibc and other *NIX pulled in writev indirectly so the bug
wasn't noticed previously.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
With the current logic, we only free @tlsalias as part of the error
label and would have to free it explicitly earlier in the code. Convert
the error label to cleanup, so that we have only one sink, where we
handle all frees. Since JSON object append operation consumes pointers,
make sure @backend is cleared before we hit the cleanup label.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1214369
Consider the following XML:
<memoryBacking>
<hugepages>
<page size='2048' unit='KiB' nodeset='1'/>
</hugepages>
<source type='file'/>
<access mode='shared'/>
</memoryBacking>
<numa>
<cell id='0' cpus='0-3' memory='512000' unit='KiB'/>
<cell id='1' cpus='4-7' memory='512000' unit='KiB'/>
</numa>
The following cmd line is generated:
-object
memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,
share=yes,size=524288000 -numa node,nodeid=0,cpus=0-3,memdev=ram-node0
-object
memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,
share=yes,size=524288000 -numa node,nodeid=1,cpus=4-7,memdev=ram-node1
This is obviously wrong as for node 1 hugepages should have been
used. The hugepages configuration is more specific than <source
type='file'/>.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1455819
It may happen that a domain is started without any huge pages.
However, user might try to attach a DIMM module later. DIMM
backed by huge pages (why would somebody want to mix regular and
huge pages is beyond me). Therefore we have to create the dir if
we haven't done so far.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1455819
Currently, the per-domain path for huge pages mmap() for qemu is
created iff domain has memoryBacking and hugepages in it
configured. However, this alone is not enough because there can
be a DIMM module with hugepages configured too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Commit v3.4.0-44-gac793bd71 fixed a memory leak, but failed to return
the special -3 value. Thus an attempt to start a domain with corrupted
managed save file would removed the corrupted file and report
"An error occurred, but the cause is unknown" instead of starting the
domain from scratch.
https://bugzilla.redhat.com/show_bug.cgi?id=1460962
Use ATTRIBUTE_FALLTHROUGH, introduced by commit
5d84f5961b, instead of comments to
indicate that the fall through is an intentional behavior.
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>
Add a comment for mon->watch to make clear what's the purpose of this
value.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
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>
The virDomainUSBAddressEnsure returns 0 or -1, so commit id 'de325472'
checking for 1 like qemuDomainAttachChrDeviceAssignAddr was wrong.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Commit 824272cb28 attempted to fix escaping of characters in unix
socket path but it was wrong. We need to escape only ',', there is
no escape character for '='.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1447618
Currently, any attempt to change MTU on an interface that is
plugged to a running domain is silently ignored. We should either
do what's asked or error out. Well, we can update the host side
of the interface, but we cannot change 'host_mtu' attribute for
the virtio-net device. Therefore we have to error out.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
https://bugzilla.redhat.com/show_bug.cgi?id=1408701
While implementing MTU (572eda12ad and friends), I've forgotten
to actually set MTU on the host NIC in case of hotplug. We
correctly tell qemu on the monitor what the MTU should be, but we
are not actually setting it on the host NIC.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
https://bugzilla.redhat.com/show_bug.cgi?id=1459091
We try to get the last element of the passed path by calling
strrch(path, '/'). However, the pointer that strrchr() returns
points at the slash, We want string that starts right after that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1459091
Currently, we are querying for vhostuser interface name in post
parse callback. At that time interface might not yet exist.
However, it has to exist when starting domain. Therefore it makes
more sense to query its name at that point. This partially
reverts 57b5e27.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
While reworking client side of streams, I had to postpone payload
decoding so that stream holes and stream data can be
distinguished in virNetClientStreamRecvPacket. That's merely what
18944b7aea does. However, I accidentally removed one important
bit: when server sends us an empty STREAM packet (with no
payload) - meaning end of stream - st->incomingEOF flag needs to
be set. It used to be before I touched the code. After I removed
it, virNetClientStreamRecvPacket will try to fetch more data from
the stream, but it will never come.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
In 9cb891141c we've introduced some logic to clearing suggested
macvtap/macvlan ifnames. The logic consists of comparing ifname
string with strings that libvirt would generate. However, due to
a typo only VIR_NET_GENERATED_MACVTAP_PREFIX was compared. Twice.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When adding the aliased serial stub console, the structure wasn't
properly allocated (VIR_ALLOC instead of virDomainChrDefNew) which then
resulted in SIGSEGV in virDomainChrSourceIsEqual during a serial device
coldplug.
https://bugzilla.redhat.com/show_bug.cgi?id=1434278
Signed-off-by: Erik Skultety <eskultet@redhat.com>
If QEMU is new enough and we have the live updated CPU definition in
either save or migration cookie, we can use it to enforce ABI. The
original guest CPU from domain XML will be stored in private data.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Since the domain XML saved in a snapshot or saved image uses the
original guest CPU definition but we still want to enforce ABI when
restoring the domain if libvirt and QEMU are new enough, we save the
live updated CPU definition in a save cookie.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Since the domain XML send during migration uses the original guest CPU
definition but we still want the destination to enforce ABI if it is new
enough, we send the live updated CPU definition in a migration cookie.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When persistent migration of a transient domain is requested but no
custom XML is passed to the migration API we would just let the
destination daemon make a persistent definition from the live definition
itself. This is not a problem now, but once the destination daemon
starts replacing the original CPU definition with the one from migration
cookie before starting a domain, it would need to add more ugly hacks to
reverse the operation. Let's just always send the persistent definition
in the cookie to make things a bit cleaner.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The destination host may not be able to start a domain using the live
updated CPU definition because either libvirt or QEMU may not be new
enough. Thus we need to send the original guest CPU definition.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When starting a domain we update the guest CPU definition to match what
QEMU actually provided (since it is allowed to add or removed some
features unless check='full' is specified). Let's store the original CPU
in domain private data so that we can use it to provide a backward
compatible domain XML.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The following patches will add an actual content in the cookie and use
the data when restoring a domain.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This patch implements a new save cookie object and callbacks for qemu
driver. The actual useful content will be added in the object later.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
virDomainXMLOption gains driver specific callbacks for parsing and
formatting save cookies.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The code will be used by snapshots and domain save/restore code to store
additional data for a saved running domain. It is analogous to migration
cookies, but simple and one way only.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
