https://bugzilla.redhat.com/show_bug.cgi?id=1264008
The existing algorithm assumed that someone was making small, incremental
changes; however, it is possible to change iothreads from 0 (or relatively
small number) to some really large number and the algorithm would possibly
spin its wheels doing unnecessary searches.
So, optimize the algorithm using a bitmap to find available iothread_id's
starting at 1 that aren't already defined by a "<thread id='#'>" and
filling in the iothreadids array with those iothread_id values.
https://bugzilla.redhat.com/show_bug.cgi?id=1249981
When qemuDomainPinIOThread was added in commit id 'fb562614', a check
for the IOThread capability was not needed since a check for iothreadpids
covered the condition where the support for IOThreads was not present.
The iothreadpids array was only created if qemuProcessDetectIOThreadPIDs
was able to query the monitor for IOThreads. It would only do that if
the QEMU_CAPS_OBJECT_IOTHREAD capability was set.
However, when iothreadids were added in commit id '8d4614a5' and the
check for iothreadpids was replaced by a search through the iothreadids[]
array for the matching iothread_id that left open the possibility that
an iothreadids[] array was defined, but the entries essentially pointed
to elements with only the 'iothread_id' defined leaving the 'thread_id'
value of 0 and eventually the cpumap entry of NULL.
This was because, the original IOThreads commit id '72edaae7' only
checked if IOThreads were defined and if the emulator had the IOThreads
capability, then IOThread objects were added at startup. The "capability
failure" check was only done when a disk was assigned to an IOThread in
qemuCheckIOThreads. This was because the initial implementation had no way
to dynamically add IOThreads, but it was possible to dynamically add a
disk to the domain. So the decision was if the domain supported it, then
add the IOThread objects. Then if a disk with an IOThread defined was
added, it could check the capability and fail to add if not there. This
just meant the 'iothreads' value was essentially ignored.
Eventually commit id 'a27ed6e7' allowed for the dynamic addition and
deletion of IOThread objects. So it was no longer necessary to generate
IOThread objects to dynamically attach a disk to. However, the startup
and disk check code was not modified to reflect this.
This patch will move the capability failure check to when IOThread
objects are being added to the command line. Thus a domain that has
IOThreads defined will not be started if the emulator doesn't support
the capability. This means when qemuCheckIOThreads is called to add
a disk, it's no longer necessary to check the capability. Instead the
code can use the IOThreadFind call to indicate that the IOThread
doesn't exist.
Finally because it could be possible to have a domain running with the
iothreadids[] defined prior to this change if libvirtd is restarted each
having mostly empty elements, qemuProcessDetectIOThreadPIDs will check
if there are niothreadids when the QEMU_CAPS_OBJECT_IOTHREAD capability
check fails and remove the elements and array if it exists.
With these changes in place, it turns out the cputune-numatune test
was failing because the right bit wasn't set in the test. So used the
opportunity to fix that and create a test that would expect to fail
with some sort of iothreads defined and used, but not having the
correct capability.
Although theoretically both should be the same value, the niothreadids
should be used in favor of iothreads when performing comparisons. This
leaves the iothreads as a purely numeric value to be saved in the config
file. The one exception to the rule is virDomainIOThreadIDDefArrayInit
where the iothreadids are being generated from the iothreads count since
iothreadids were added after initial iothreads support.
Event implementations need to be registered before a connection to the
Hypervisor is opened, otherwise event handling can be impaired (e.g.
delayed messages). This fact is referenced in an e-mail [1], but should
also be noted in the documentation of the registration functions.
[1] https://www.redhat.com/archives/libvirt-users/2014-April/msg00011.html
Create a separate local API that will fill in the iothreadid array
entries that were not defined by <iothread id='#'> entries in the XML.
Signed-off-by: John Ferlan <jferlan@redhat.com>
After a successful creation of a directory, if some other call results
in returning a failure, let's remove the directory we created to
prevent another round trip or confusion in the caller. In particular, this
function can be called during a storage backend buildVol, so in order
to ensure that caller doesn't need to distinguish between failed create
or some other failure after create, just remove the directory we created.
Signed-off-by: John Ferlan <jferlan@redhat.com>
After a successful creation of a file, if some other call results
in returning a failure, let's unlink the file we created to prevent
another round trip or confusion in the caller. In particular, this
function can be called during a storage backend buildVol, so in order
to ensure that caller doesn't need to distinguish between failed create
or some other failure after create, just remove the volume we created.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1233003
Track when the logical volume was successfully created in order to
properly handle the call to virStorageBackendLogicalDeleteVol. It's
possible that the failure to create was because someone created an
LV in the pool outside of libvirt's knowledge. In this case, we don't
want to delete that LV. A subsequent or future refresh of the pool
will find the volume and cause an earlier failure
Signed-off-by: John Ferlan <jferlan@redhat.com>
Commit id '1b5685da' refactored the code to move buildvoldef inside
the buildVol conditional; however, the VIR_FREE of the memory was
left only when 'buildret' failed, thus we're leaking memory.
Signed-off-by: John Ferlan <jferlan@redhat.com>
As of commit id '155ca616' a 'refreshVol' is called after a buildVol
succeeds in storageVolCreateXML, thus a volStorageBackendSheepdogRefreshVolInfo
call in virStorageBackendSheepdogBuildVol is no longer necessary.
Additionally, the 'conn' parameter becomes unused.
Signed-off-by: John Ferlan <jferlan@redhat.com>
As of commit id '155ca616' a 'refreshVol' is called after the buildVol
succeeds in storageVolCreateXML, thus the volStorageBackendRBDRefreshVolInfo
call in virStorageBackendRBDBuildVol is no longer necessary.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Without this, building on cygwin fails with:
CC libvirt_admin_la-libvirt-admin.lo
libvirt-admin.c:25:21: fatal error: rpc/rpc.h: No such file or directory
#include <rpc/rpc.h>
^
Reported-by: Yaakov Selkowitz <yselkowi@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Our apibuild.py script does not cope with ATTRIBUTE_NONNULL:
Parse Error: parsing function type, ')' expected
Got token ('name', 'char')
Last token: ('name', 'char')
Token queue: [('op', '*'), ('name', 'dconnuri'), ('sep', ')')]
Line 3297 end:
Makefile:2441: recipe for target '../../docs/apibuild.py.stamp' failed
Let's drop it. Moreover, up until e17ae3ccc2 where it was
introduced we did not really care about NULL-ity of dconnuri. And
moreover the ATTRIBUTE_NONNULL merely checks for static calls
over NULL, it won't catch the dynamic ones, where a NULL is
passed by a variable at runtime.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1256999
After creating a copy of the 'authdef' in a pool -> disk translation,
unconditionally clear the 'authType' in the resulting disk auth def
structure since that's used for a storage pool and not a disk. This
ensures virStorageAuthDefFormat will properly format the <auth> XML
for a <disk> (e.g. it won't have a <auth type='%s'.../>).
Check dconnuri is not null or we will catch nullpointer later.
I hope this makes Coverity happy.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
virDomainMigrateUnmanagedParams is not a good candidate for this functionality
as it is used by migrate family functions too and its have its own checks that
are superset of extracted and we don't need to check twice.
Actually name of the function is slightly misleading as there is also a check
for consistensy of flags parameter alone. So it could be refactored further and
reused by all migrate functions but for now let it be a matter of a different
patchset.
It is *not* a pure refactoring patch as it introduces offline check for older
versions. Looks like it must be done that way and no one will be broken too.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Finally on this step we get what we were aimed for - toURI{1, 2} (and
migration{*} APIs too) now can work thru V3_PARAMS protocol. Execution path
goes thru unchanged virDomainMigrateUnmanaged adapter function which is called
by all target places.
Note that we keep the fact that direct migration never works
thru V3_PARAMS proto. We can't change this aspect without
further investigation.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Move virDomainMigrateUnmanagedProto* expected params list check into
function itself and use common virTypedParamsCheck for this purpose.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Extract parameter adaptation and checking which is protocol dependent into
designated functions. Leave only branching and common checks in
virDomainMigrateUnmanagedParams.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Let's put main functionality into params version of virDomainMigrateUnmanaged
as a preparation step for merging it with virDomainMigratePeer2PeerParams.
virDomainMigrateUnmanaged then does nothing more then just adapting arguments.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
p2p plain and direct function are good candidates for code reuse. Their main
function is same - to branch among different versions of migration protocol and
implementation of this function is also same. Also they have other common
functionality in lesser aspects. So let's merge them.
But as they have different signatures we have to get to convention on how to
pass direct migration 'uri' in 'dconnuri' and 'miguri'. Fortunately we alreay
have such convention in parameters passed to toURI2 function, just let's follow
it. 'uri' is passed in miguri and dconnuri is ignored.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
We use miguri name for this parameter in other places. So
make naming more consitent.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Direct migration should work if *perform3 is present but *perform
is not. This is situation when driver migration is implemented
after new version of driver function is introduced. We should not
be forced to support old version too as its parameter space is
subspace of newer one.
This is more structured code so it will be easier to add branch for _PARAMS
protocol here. It is not a pure refactoring strictly speaking as we remove
scenarios for broken cases when driver defines V3 feature and implements
perform function. So it is additionally a more solid code.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
'useParams' parameter usage is an example of control coupling. Most of the work
inside the function is done differently except for the uri check. Lets split
this function into two, one with extensible parameters set and one with hardcoded
parameter set.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
The internal representation of a JSON array counts the items in
size_t. However, for some reason, when asking for the count it's
reported as int. Firstly, we need the function to return a signed
type as it's returning -1 on an error. But, not every system has
integer the same size as size_t. Therefore, lets return ssize_t.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit 4e8032272f used $(builddir) in the header search
path to fix a build issue; however, $(builddir) is not defined
by old autoconf versions such as the one available in CentOS 5,
resulting in the following error:
cc1: error: /util: No such file or directory
make[3]: *** [libvirt_driver_la-fdstream.lo] Error 1
Since $(builddir) is defined to always be '.', just use that
value directly instead.
Since a9fe620372, we are generating virkeymaps.h at build
time; however, we are not including $(builddir)/util in the
header search path, so when doing a VPATH build the compiler
is unable to locate the file.
make[2]: Entering directory
`/home/jenkins/libvirt/systems/libvirt-fedora-20/build/src'
GEN util/virkeymaps.h
...
CC util/libvirt_util_la-virkeycode.lo
CC util/libvirt_util_la-virkeyfile.lo
CC util/libvirt_util_la-virlockspace.lo
CC util/libvirt_util_la-virlog.lo
../../src/util/virkeycode.c:27:24: fatal error: virkeymaps.h: No such file or directory
#include "virkeymaps.h"
^
compilation terminated.
Coverity notices that net->ifname is potentially referenced after a
VIR_FREE(). Since the net->ifname will eventually be free'd during
virDomainDefFree when calling virDomainNetDefFree, let's just that
processing take care the free.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Since the strtok_r call in libxlCapsInitGuests expects a non NULL first
parameter when the third parameter is NULL, we need to check that
the returned 'capabilities' from a libxl_get_version_info call is
not NULL and error out if so since the code expects it.
Signed-off-by: John Ferlan <jferlan@redhat.com>
So imagine you want to crate new security manager:
if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false, true)));
Hard to parse, right? What about this:
if (!(mgr = virSecurityManagerNew("selinux", "QEMU",
VIR_SECURITY_MANAGER_DEFAULT_CONFINED |
VIR_SECURITY_MANAGER_PRIVILEGED)));
Now that's better! This is what the commit does.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This gets rid of the partially enforced alignment and makes it less
likely for a bogus value to be introduced in the enumeration.
Capabilities are divided in five-element groups for better readability.
Use #define for QEMU_CAPS_NET_NAME and QEMU_CAPS_HOST_NET_ADD, both
of which are aliases for QEMU_CAPS_0_10.
qemuMigrationIsAllowed would disallow offline migration if the VM
contained host devices or memory modules. Since during offline migration
we don't transfer any state we can safely migrate VMs with such
configuration.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1265049
Use the migration @flags for checking various migration aspects rather
than picking them out as booleans. Document the new semantics in the
function header.
Now that qemuMigrationIsAllowed is always called with @vm, we can drop
the @def argument and simplify the control flow.
Additionally the comment is invalid so drop it.
Extract the hostdev check from qemuMigrationIsAllowed into a separate
function since that is the only part that needs to be done in the v2
migration protocol prepare phase on the destination. All other checks
were added when the v3 protocol existed so they don't need to be
extracted.
This change will allow to drop the @def argument for
qemuMigrationIsAllowed and further simplify the function.
In fact, it was never used as far as vz has no features supporting it.
That is why there will be no harm to anyone if we just remove this code to
prevent further misunderstanding and efforts to support dead code.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
At the time this code was added we had intentions to support libvirt interface
to manage vz networks. In fact, it was never implemented completely to work
correctly that makes me think that there will be no harm to anyone if we just
rip it off. Moreover, in vz7 we started to use libvirt bridge network driver to
manage networks.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Even though QEMU on the source host reports completed migration and thus
we move to the Finish phase, QEMU on the destination host may still be
processing migration data. Thus before we can start guest CPUs on the
destination, we have to wait for a completed migration event.
https://bugzilla.redhat.com/show_bug.cgi?id=1265902
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
With new QEMU which supports migration events,
qemuMigrationCheckJobStatus needs to explicitly query QEMU for migration
statistics once migration is completed to make sure the caller sees
up-to-date statistics with both old and new QEMU. However, some callers
are not interested in the statistics at all and once we start waiting
for a completed migration on the destination host too, checking the
statistics would even fail. Let's push the decision whether to update
the statistics or not to the caller.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The function already has two bool parameters and we will need to add a
new one. Let's switch to flags to make the callers readable.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The destination host gets detailed statistics about the current
migration form the source host via migration cookie and copies them to
the domain object so that they can be queried using
virDomainGetJobStats. However, we should only copy statistics to the
domain object when migration finished successfully.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Even if we are migrating a domain with VIR_MIGRATE_PAUSED flag set, we
should still update the total time of the migration. Updating downtime
doesn't hurt either, even though we don't actually start guest CPUs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
We are distributing virkeymaps.h and all the tools needed to rebuild
that file. On top of that, we are generating that file into the
$(srcdir) and that sometimes fails when trying to do make dist in VPATH
on rawhide fedora. And we don't clean the file when maintainer-clean
make target is requested. So let's not distribute the file and rather
let everyone rebuild it when needed and clean it when appropriate.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
profile_status function was not making any difference between error
cases and unconfined profiles. The problem with this approach is that
dominfo was throwing an error on unconfined domains.
Our docs state that subelements of <metadata> shall have a namespace
and the medatata APIs expect that too. To avoid inaccessible
<metadata> sub-elements, just remove those that don't conform to the
documentation.
Apart from adding the new condition this patch renames the function and
refactors the code flow to allow the changes.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1245525
https://bugzilla.redhat.com/show_bug.cgi?id=1247987
Calculation of the extended and logical partition values for the disk
pool is complex. As the bz points out an extended partition should have
it's allocation initialized to 0 (zero) and keep the capacity as the size
dictated by the extents read. Then for each logical partition found,
adjust the allocation of the extended partition.
Finally, previous logic tried to avoid recalculating things if a logical
partition was deleted; however, since we now have special logic to handle
the allocation of the extended partition, just make life easier by reading
the partition table again - rather than doing the reverse adjustment.
https://bugzilla.redhat.com/show_bug.cgi?id=1251461
When 'starting' up a disk pool, we need to make sure the label on the
device is valid; otherwise, the followup refreshPool will assume the
disk has been properly formatted for use. If we don't find the valid
label, then refuse the start and give a proper reason.
Let's check to ensure we can find the Partition Table in the label
and that libvirt actually recognizes that type; otherwise, when we
go to read the partitions during a refresh operation we may not be
reading what we expect.
This will expand upon the types of errors or reason that a build
would fail, so we can create more direct error messages.
Modify virStorageBackendDiskValidLabel to add a 'writelabel' parameter.
While initially for the purpose of determining whether the label should
be written during DiskBuild, a future use during DiskStart could determine
whether the pool should be started using the label found. Augment the
error messages also to give a hint as to what someone may need to do
or why the command failed.
Create a new function virStorageBackendDiskValidLabel to handle checking
whether there is a label on the device and whether it's valid or not.
While initially for the purpose of determining whether the label can be
overwritten during DiskBuild, a future use during DiskStart could determine
whether the pool should be started using the label found.
https://bugzilla.redhat.com/show_bug.cgi?id=1233003
Although perhaps bordering on a don't do that type scenario, if
someone creates a volume in a pool outside of libvirt, then uses that
same name to create a volume in the pool via libvirt, then the creation
will fail and in some cases cause the same name volume to be deleted.
This patch will refresh the pool just prior to checking whether the
named volume exists prior to creating the volume in the pool. While
it's still possible to have a timing window to create a file after the
check - at least we tried. At that point, someone is being malicious.
As it turns out the caller in this case expects a return < 0 for failure
and to get/use "errno" rather than using the negative of returned status.
Again different than the create path.
If someone "deleted" a file from the pool without using virsh vol-delete,
then the unlink/rmdir would return an error (-1) and set errno to ENOENT.
The caller checks errno for ENOENT when determining whether to throw an
error message indicating the failure. Without the change, the error
message is:
error: Failed to delete vol $vol
error: cannot unlink file '/$pathto/$vol': Success
This patch thus allows the fork path to follow the non-fork path
where unlink/rmdir return -1 and errno.
Unlike create options, if the file to be removed is already in the
pool, then the uid/gid will come from the pool. If it's the same as the
currently running process, then just do the unlink/rmdir directly
rather than going through the fork processing unnecessarily
qemu-kvm can be used to run ppc64 guests on ppc64le hosts and vice
versa, since the hardware is actually the same and the endianness
is chosen by the guest kernel.
Up until now, however, libvirt didn't allow the use of qemu-kvm
to run guests if their endianness didn't match the host's.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1267882
Commit 792f81a40e caused a regression in the libssh2 host key
verification code by changing the variable type of 'i' to unsigned.
Since one of the loops used -1 as a special value if the asking
callback was found the conversion made a subsequent test always fail.
The bug was stealth enough to pass review, compilers and coverity.
Refactor the condition to avoid problems.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1047861
Since we'd disallow migration of a guest that would have possibly
invalid config but still be able to work, relax the WWN check to be
performed only on new starts of the VM.
If a system has a large number of active or active interfaces, it can
be a big waste of time to retrieve and qualify all interfaces if the
caller only wanted one subset. Since netcf has a simple flag for this,
translate the libvirt flag into a netcf flag and let netcf pre-filter.
Getting the MAC address of an interface is actually fairly expensive,
and we've already gotten it and stored it into def, so just keep def
around a bit longer and retrieve it from there.
This reduces the time for "virsh iface-list --all" from 28 to 23
seconds when there are 400 interfaces.
The spec for virConnectListAllInterfaces says that if the pointer that
is supposed to hold the list of interfaces is NULL, the function
should just return the count of interfaces that matched the filter,
but the code never increments the count if the list pointer is NULL.
We are using memory-backing-file even when it's not needed, for example
if user requests hugepages for memory backing, but does not specify any
pagesize or memory node pinning. This causes migrations to fail when
migrating from older libvirt that did not do this. So similarly to
commit 7832fac847 which does it for
memory-backend-ram, this commit makes is more generic and
backend-agnostic, so the backend is not used if there is no specific
pagesize of hugepages requested, no nodeset the memory node should be
bound to, no memory access change required, and so on.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1266856
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
So since the introduction of the memory-backend-file object until now we
only added '-mem-path' for non-NUMA guests and we used the parameters of
the memory-backend-file object to specify the path to the hugetlbfs
mount. But hugepages can be also used without memory-backend-file
object, as it used to be before its introduction. Let's just get this
part of the code back and properly append the '-mem-path' for NUMA
guests as well, but only when the memory backend is not needed.
This parameter is already being applied when no numa is requested and
because we still use memory-object-file unconditionally for
hugepage-backed NUMA guests, this should not fire until later.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
That function is called qemuBuildMemPathStr() and will be used in
other places in the future. The change in the test suite is proper due
to the fact that -mem-prealloc makes only sense with -mem-path (from
qemu documentation -- html/qemu-doc.html).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Support for GICv3 has been recently introduced in qemu using gic-version
option for the 'virt' machine. The option can actually take values of
'2', '3' and 'host', however, since in libvirt this is a numeric
parameter, we limit it only to 2 and 3. Value of 2 is not added to the
command line in order to keep backward compatibility with older qemu
versions.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
Unfortunately qemu currently doesn't offer introspection for machine types,
so we have to rely on version number, similar to QEMU_CAPS_MACHINE_USB_OPT.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
Commit 307fb904 (Sep 10) added a 'privileged' variable when creating
the DAC driver:
@@ -153,6 +157,7 @@ virSecurityManagerNewDAC(const char *virtDriver,
bool defaultConfined,
bool requireConfined,
bool dynamicOwnership,
+ bool privileged,
virSecurityManagerDACChownCallback chownCallback)
But argument order is mixed up at the caller, swapping dynamicOwnership
and privileged values. This corrects the argument order
https://bugzilla.redhat.com/show_bug.cgi?id=1266628
Since commit e0139e3, we update the pool allocation with
the user-provided allocation values.
For qcow2, the allocation is ignored for volume building,
but we still subtracted it from pool's allocation.
This can result in interesting values if the user-provided
allocation is large enough:
Capacity: 104.71 GiB
Allocation: 109.13 GiB
Available: 16.00 EiB
We already do a VolRefresh on volume creation. Also refresh
the volume after creating and use the new value to update the pool.
https://bugzilla.redhat.com/show_bug.cgi?id=1163091
Commit id '7383b8cc' changed virDomainDef 'virtType' to an enum, that
caused a build failure on some archs due to comparing an unsigned value
to < 0. Adjust the fetch of 'type' to be into temporary 'int virtType'
and then assign that virtType to the def->virtType
Introduce VIR_DOMAIN_VIRT_NONE to give domaintype the default value of zero.
This is specially helpful in constructing better error messages
when we don't want to look up the default emulator by virtType.
The test data in vircapstest.c is also modified to reflect this change.
As of commit 6992994, we set graphics/@listen attribute according to the
first listen child element even if that element is of type='network'.
This was done for backward compatibility with applications which only
support the original listen attribute. However, by doing so we broke
migration to older libvirt which tried to check that the listen
attribute matches one of the listen child elements but which did not
take type='network' elements into account.
We are not concerned about compatibility with old applications when
formatting domain XML for migration for two reasons. The XML is consumed
only by libvirtd and the IP address associated with type='network'
listen address on the source host is just useless on the destination
host. Thus, we can safely avoid propagating the type='network' IP
address to graphics/@listen attribute when creating migratable XML.
https://bugzilla.redhat.com/show_bug.cgi?id=1265111
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This seemed to be more of a false positive as for some reason Coverity
was missing the "ret < 0" goto error condition and somehow believing that
event could be overwritten. At first I thought it was just the ret != 0
condition difference, but it wasn't.
In any case, make use of the recent change to qemuDomainEventQueue to
check event == NULL and just pass it as a parameter directly in the
error path. That avoids the error.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Coverity complains that return from virHookCall is not checked in
one place in qemuProcessStop. Since the comment notes that we cannot
stop the operation even it if fails, just added the ignore_value.
So while working on my previous patches, I've noticed that
virDomainRestore implementation in qemu and test drivers has the
same problem as I am fixing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
So far we have the following pattern occurring over and over
again:
if (!vm->persistent)
qemuDomainRemoveInactive(driver, vm);
It's safe to put the check into the function and save some LoC.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=871452
So, you want to create a domain from XML. The domain already
exists in libvirt's database of domains. It's okay, because name
and UUID matches. However, on domain startup, internal
representation of the domain is overwritten with your XML even
though we claim that the XML you've provided is a transient one.
The bug is to be found across nearly all the drivers.
Le sigh.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=871452
Okay, so we allow users to 'virsh create' an already existing
domain, providing completely different XML than the one stored in
Libvirt. Well, as long as name and UUID matches. However, in some
drivers the code that handles errors unconditionally removes the
domain that failed to start even though the domain might have
been persistent. Fortunately, the domain is removed just from the
internal list of domains and the config file is kept around.
Steps to reproduce:
1) virsh dumpxml $dom > /tmp/dom.xml
2) change XML so that it is still parse-able but won't boot, e.g.
change guest agent path to /foo/bar
3) virsh create /tmp/dom.xml
4) virsh dumpxml $dom
5) Observe "No such domain" error
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
I initially added this in order to keep the code more error-prone to
following additions, but it seems it's still frowned upon.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Now that virQEMUDriverCreateXMLConf is never called with NULL
(after 086f37e97a) we can safely drop useless check in
qemuDomainDeviceDefPostParse as we are guaranteed to be always
called with the driver initialized. Therefore checking if driver
is NULL makes no sense. Moreover, if we mix it with direct driver
dereference. And after that, we are sure that nor @cfg will be
NULL, therefore we can drop checks for that too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Qemu unfortunately doesn't update internal state right after migration
and so the actual balloon size as returned by 'query-balloon' are
invalid for a while after the CPUs are started after migration. If we'd
refresh our internal state at this point we would report invalid current
memory size until the next balloon event would arrive.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1242940
My original implementation was based on a qemu version that still did
not have all the checks in place. Using sizes that would align to odd
megabyte increments will produce the following error:
qemu-kvm: -device pc-dimm,node=0,memdev=memdimm0,id=dimm0: backend memory size must be multiple of 0x200000
qemu-kvm: -device pc-dimm,node=0,memdev=memdimm0,id=dimm0: Device 'pc-dimm' could not be initialized
Introduce an alignment retrieval function for memory devices and use it
to align the devices separately and modify a test case to verify it.
Even though we hit an error in client's IO loop, we still want to
process any pending data. So instead of reporting the error right away,
we can finish the current iteration and report the error once we're done
with it. Note that the error is stored in client->error by
virNetClientMarkClose so we don't need to worry about it being reset or
rewritten by any API we call in the meantime.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Whenever a connection was closed due to keepalive timeout, we would log
a warning but the interrupted API would return rather useless generic
error:
internal error: received hangup / error event on socket
Let's report a proper keepalive timeout error and make sure it is
propagated to all pending APIs. The error should be better now:
internal error: connection closed due to keepalive timeout
Based on an old patch from Martin Kletzander.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Build fail and error like this:
CC qemu/libvirt_driver_qemu_impl_la-qemu_command.lo
qemu/qemu_capabilities.c:46:27: fatal error: qemu_capspriv.h: No such file or directory
#include "qemu_capspriv.h"
Add qemu_capspriv.h to source.
Signed-off-by: Luyao Huang <lhuang@redhat.com>
For some machine types ppc64 machines now require that memory sizes are
aligned to 256MiB increments (due to the dynamically reconfigurable
memory). As now we treat existing configs reasonably in regards to
migration, we can round all the sizes unconditionally. The only drawback
will be that the memory size of a VM can potentially increase by
(256MiB - 1byte) * number_of_NUMA_nodes.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1249006
When we are starting a qemu process for an incomming migration or
snapshot reloading we should not modify the memory sizes in the domain
since we could potentially change the guest ABI that was tediously
checked before. Additionally the function now updates the initial memory
size according to the NUMA node size, which should not happen if we are
restoring state.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1252685
When implementing memory hotplug I've opted to recalculate the initial
memory size (contents of the <memory> element) as a sum of the sizes of
NUMA nodes when NUMA was enabled. This was based on an assumption that
qemu did not allow starting when the NUMA node size total didn't equal
to the initial memory size. Unfortunately the check was introduced to
qemu just lately.
This patch uses the new XML parser flag to decide whether it's safe to
update the memory size total from the NUMA cell sizes or not.
As an additional improvement we now report an error in case when the
size of hotplug memory would exceed the total memory size.
The rest of the changes assures that the function is called with correct
flags.
Add 'initial_memory' member to struct virDomainMemtune so that the
memory size can be pre-calculated once instead of inferring it always
again and again.
Separating of the fields will also allow finer granularity of decisions
in later patches where it will allow to keep the old initial memory
value in cases where we are handling incomming migration from older
versions that did not always update the size from NUMA as the code did
previously.
The change also requires modification of the qemu memory alignment
function since at the point where we are modifying the size of NUMA
nodes the total size needs to be recalculated too.
The refactoring done in this patch also fixes a crash in the hyperv
driver that did not properly initialize def->numa and thus
virDomainNumaGetMemorySize(def->numa) crashed.
In summary this patch should have no functional impact at this point.
The post parse func is growing rather large. Since later patches will
introduce more logic in the memory post parse code, split it into a
separate handler.
Add a new parser flag that will mark code paths that parse XML files
wich will not be used with existing VM state so that post parse
callbacks can possibly do ABI incompatible changes if needed.
The flag was used only for formatting the XML and once the parser and
formatter flags were split in 0ecd685109
it doesn't make sense any more to have it.
Extract the size determination into a separate function and reuse it
across the memory device alignment functions. Since later we will need
to decide the alignment size according to architecture let's pass def to
the functions.
Since test suite now correctly creates capabilities cache, the hack is not
needed any more.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
The main purpose of this patch is to introduce test mode to
virQEMUCapsCacheLookup(). This is done by adding a global variable, which
effectively overrides binary name. This variable is supposed to be set by
test suite.
The second addition is qemuTestCapsCacheInsert() function which allows the
test suite to actually populate the cache.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Invalid read of size 4
at 0x945CA30: __pthread_mutex_unlock_full (in /lib64/libpthread-2.20.so)
by 0x4F0404B: virMutexUnlock (virthread.c:94)
by 0x4F7161B: virStoragePoolObjUnlock (storage_conf.c:2603)
by 0x4FE0476: testStoragePoolUndefine (test_driver.c:4328)
by 0x4FCF086: virStoragePoolUndefine (libvirt-storage.c:656)
by 0x15A7F5: cmdPoolUndefine (virsh-pool.c:1721)
by 0x12F48D: vshCommandRun (vsh.c:1212)
by 0x132AA7: main (virsh.c:943)
Address 0xfda56a0 is 16 bytes inside a block of size 104 free'd
at 0x4C2BA6C: free (vg_replace_malloc.c:473)
by 0x4EA5C96: virFree (viralloc.c:582)
by 0x4F70B69: virStoragePoolObjFree (storage_conf.c:412)
by 0x4F7167B: virStoragePoolObjRemove (storage_conf.c:437)
by 0x4FE0468: testStoragePoolUndefine (test_driver.c:4323)
by 0x4FCF086: virStoragePoolUndefine (libvirt-storage.c:656)
by 0x15A7F5: cmdPoolUndefine (virsh-pool.c:1721)
by 0x12F48D: vshCommandRun (vsh.c:1212)
by 0x132AA7: main (virsh.c:943)
Similar to commit id '35847860', it's possible to attempt to create
a 'netfs' directory in an NFS root-squash environment which will cause
the 'vol-delete' command to fail. It's also possible error paths from
the 'vol-create' would result in an error to remove a created directory
if the permissions were incorrect (and disallowed root access).
Thus rename the virFileUnlink to be virFileRemove to match the C API
functionality, adjust the code to following using rmdir or unlink
depending on the path type, and then use/call it for the VIR_STORAGE_VOL_DIR
As far as not every call of prlsdkUUIDParse assume correct UUID
supplied, there is no use to complain about wrong format in it.
Otherwise our log is flooded with false error messages.
For instance, calling prlsdkUUIDParse from prlsdkEventsHandler
works as a filter and in case of uuid absence for event issuer,
we simply know that we shouldn't continue further processing.
Instead of error logging for all calls we should explicitly take
into accaunt where it is called from.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
So far this function was not kept in sync with changing
virDomainDiskDef. Fill in all the missing checks and reorganize
their order so it's easier to track which items are not being
checked for.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
I always felt like this function is qemu specific rather than
libvirt-wide. Other drivers may act differently on virDomainDef
change and in fact may require talking to underlying hypervisor
even if something else's than disk->src has changed. I know that
the function is still incomplete, but lets break that into two
commits that are easier to review. This one is pure code
movement.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Firstly, our coding guidelines suggest using 'cleanup' label
instead of 'end'. Then, @ret should be set to value representing
success as the last statement before the 'cleanup' label.
And while I am at this function, lets enumerate all the possible
enum items (virDomainDiskDevice) and avoid using 'default' in
switch(). Pooh. Also, nothing bad happens if we look up the disk
to change in the domain upfront. In fact, it's going to be
helpful later when we want to keep some old values for performing
a rollback.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This new private API should return true iff sources of two disks
differs in sense that qemu should be instructed to change the
disk backend. For instance, ejecting a CDROM is such case, or
pointing disk into a different ISO location, and so on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
While we currently only allow changing a media in a disk, this is
going to change in a while, so the function name would be
invalid. Moreover, the old name does not match the pattern laid
out by other update functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1159219
So, in 11e058ca58 I've tried to make UpdateDevice update
startupPolicy too. And it worked well until somebody came around
and pushed d0dc6c0369 which accidentally removed my
contribution. Redo my commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When persistently migrating a domain to a destination host where the
same domain already exists (i.e., it is persistent and shutdown at the
destination), we would happily throw away the original persistent
definition without properly freeing it. And when updating the definition
fails for some reason we don't properly revert to the original state
leaving the domain broken.
In addition to fixing these issues, the patch also makes sure the domain
definition parsed from a migration cookie is either used or freed.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
For quite a long time we don't need to postpone queueing events until
the end of the function since we no longer have the big driver lock.
Let's make the code of qemuMigrationFinish simpler by queuing events at
the time we generate them.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Every single call to qemuDomainEventQueue() uses the following pattern:
if (event)
qemuDomainEventQueue(driver, event);
Let's move the check for valid event to qemuDomainEventQueue and
simplify all callers.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Finish is the final state in v2 of our migration protocol. If something
fails, we have no option to abort the migration and resume the original
domain. Non fatal errors (such as failure to start guest CPUs or make
the domain persistent) has to be treated as success. Keeping the domain
running while reporting the failure was just asking for trouble.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Whenever something fails during incoming migration in Finish phase
before we started guest CPUs, we need to kill the domain in addition to
reporting the failure.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When we save status XML at the point during migration where we have
already started the domain on destination, we can't really go back and
abort migration. Thus the only thing we can do is to log a warning and
report success.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Offline migration is quite special because we don't really need to do
anything but make the domain persistent. Let's do it separately from
normal migration to avoid cluttering the code with
!(flags & VIR_MIGRATE_OFFLINE).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
After attach-device a <hostdev> with --config, new device doesn't
show up in dumpxml and in guest.
To fix that, set dev->data.hostdev = NULL after work so that the
pointer is not freed, since vmdef has the pointer and still need it.
Signed-off-by: Chunyan Liu <cyliu@suse.com>
Tool such as libguestfs need the datacenter path to get access to disk
images. The ESX driver knows the correct datacenter path, but this
information cannot be accessed using libvirt API yet. Also, it cannot
be deduced from the connection URI in a robust way.
Expose the datacenter path in the domain XML as <vmware:datacenterpath>
node similar to the way the <qemu:commandline> node works. The new node
is ignored while parsing the domain XML. In contrast to <qemu:commandline>
it is output only.
Commit id 'f1f68ca33' added code to remove the directory paths for
auto-generated sockets, but that code could be called before the
paths were created resulting in generating error messages from
virFileDeleteTree indicating that the file doesn't exist.
Rather than "enforce" all callers to make the non-NULL and existence
checks, modify the virFileDeleteTree API to silently ignore NULL on
input and non-existent directory trees.
When looking for a QEMU binary suitable for running ppc64le guests
we have to take into account the fact that we use the QEMU target
as key for the hash, so direct comparison is not good enough.
Factor out the logic from virQEMUCapsFindBinaryForArch() to a new
virQEMUCapsFindTarget() function and use that both when looking
for QEMU binaries available on the system and when looking up
QEMU capabilities later.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1260753
libxl/libxl_conf.c: In function 'libxlDriverConfigNew':
libxl/libxl_conf.c:1560:30: error: 'log_level' may be used uninitialized
in this function [-Werror=maybe-uninitialized]
Instead of a hardcoded DEBUG log level, use the overall
daemon log level specified in libvirtd.conf when opening
a log stream with libxl. libxl is very verbose when DEBUG
log level is set, resulting in huge log files that can
potentially fill a disk. Control of libxl verbosity should
be placed in the administrator's hands.
Fixes the following error when attempting to add a disk with bus='virtio'
to a machine which actually supports virtio-mmio (caught with ARM virt):
virtio disk cannot have an address of type 'virtio-mmio'
The problem has been likely introduced by
e8d5517254. Before that
qemuAssignDevicePCISlots() was never called for ARM "virt" machine.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1124841
If running in session mode it may happen that we fail to set
correct SELinux label, but the image may still be readable to
the qemu process. Take this into account.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We may want to do some decisions in drivers based on fact if we
are running as privileged user or not. Propagate this info there.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We have plenty of callbacks in the driver. Some of these
callbacks require more than one argument to be passed. For that
we currently have a data type (struct) per each callback. Well,
so far for only one - SELinuxSCSICallbackData. But lets turn it
into more general name so it can be reused in other callbacks too
instead of each one introducing a new, duplicate data type.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
