We have reference to connection object in virConnectCloseCallbackData
object thus we have to refcount it. Obviously we have problems
in dispose and call functions. Let's fix it.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Make register and unregister functions return void because
we can check the state of callback object beforehand via
virConnectCloseCallbackDataGetCallback. This can be done
without race conditions if we use higher level locks for registering
and unregistering. The fact they return void simplifies
task of consistent registering/unregistering.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Henning Schild [Fri, 26 Feb 2016 15:34:24 +0000 (16:34 +0100)]
qemu_cgroup: use virCgroupAddTask instead of virCgroupMoveTask
qemuProcessSetupEmulator runs at a point in time where there is only
the qemu main thread. Use virCgroupAddTask to put just that one task
into the emulator cgroup. That patch makes virCgroupMoveTask and
virCgroupAddTaskStrController obsolete.
Henning Schild [Fri, 26 Feb 2016 15:34:23 +0000 (16:34 +0100)]
qemu_cgroup: put qemu right into emulator sub-cgroup
Move qemuProcessSetupEmulator up under qemuSetupCgroup. That way
we move the one main thread right into the emulator cgroup, instead
of moving multiple threads later on. And we do not actually want any
threads running in the parent cgroups (cpu cpuacct cpuset).
Pavel Hrdina [Tue, 23 Feb 2016 16:04:19 +0000 (17:04 +0100)]
qemu: introduce vram64 attribute for QXL video device
This attribute is used to extend secondary PCI bar and expose it to the
guest as 64bit memory. It works like this: attribute vram is there to
set size of secondary PCI bar and guest sees it as 32bit memory,
attribute vram64 can extend this secondary PCI bar. If both attributes
are used, guest sees two memory bars, both address the same memory, with
the difference that the 32bit bar can address only the first part of the
whole memory.
Pavel Hrdina [Tue, 23 Feb 2016 12:00:24 +0000 (13:00 +0100)]
domain_conf: always set primary video device as primary
We always place primary video device at first place, to make it easier
to create a qemu command or format an xml, but we should also set the
primary boolean for primary video device to 'true'.
When the login session doesn't have an ssh -X type display agent in
order for libvirtd to run the polkit session authentication, attempts
to run 'virsh -c qemu:///system list' from an unauthorized user (or one
that isn't part of the libvirt /etc/group) will fail with the following
error from libvirtd:
error: authentication unavailable: no polkit agent available to
authenticate action 'org.libvirt.unix.manage'
In order to handle the local authentication, we will use the new
virPolkitAgentCreate API in order to create a text based authentication
agent for our non readonly session to authenticate with.
The new code will execute in a loop allowing 5 failures to authenticate
before failing out.
With this patch in place, the following occurs:
$ virsh -c qemu:///system list
==== AUTHENTICATING FOR org.libvirt.unix.manage ===
System policy prevents management of local virtualized systems
Authenticating as: Some User (SUser)
Password:
==== AUTHENTICATION COMPLETE ===
Id Name State
----------------------------------------------------
1 somedomain running
John Ferlan [Tue, 9 Feb 2016 15:09:44 +0000 (10:09 -0500)]
util: Introduce API's for Polkit text authentication
Introduce virPolkitAgentCreate and virPolkitAgentDestroy
virPolkitAgentCreate will run the polkit pkttyagent image as an asynchronous
command in order to handle the local agent authentication via stdin/stdout.
The code makes use of the pkttyagent --notify-fd mechanism to let it know
when the agent is successfully registered.
virPolkitAgentDestroy will close the command effectively reaping our
child process
John Ferlan [Thu, 14 Jan 2016 19:34:28 +0000 (14:34 -0500)]
polkit: Adjust message when authentication agent isn't found
When there isn't a ssh -X type session running and a user has not
been added to the libvirt group, attempts to run 'virsh -c qemu:///system'
commands from an otherwise unprivileged user will fail with rather
generic or opaque error message:
"error: authentication failed: no agent is available to authenticate"
This patch will adjust the error code and message to help reflect the
situation that the problem is the requested mechanism is UNAVAILABLE and
a slightly more descriptive error. The result on a failure then becomes:
"error: authentication unavailable: no polkit agent available to
authenticate action 'org.libvirt.unix.manage'"
A bit more history on this - at one time a failure generated the
following type message when running the 'pkcheck' as a subprocess:
"error: authentication failed: polkit\56retains_authorization_after_challenge=1
Authorization requires authentication but no agent is available."
but, a patch was generated to adjust the error message to help provide
more details about what failed. This was pushed as commit id '96a108c99'.
That patch prepended a "polkit: " to the output. It really didn't solve
the problem, but gave a hint.
After some time it was deemed using DBus API calls directly was a
better way to go (since pkcheck calls them anyway). So, commit id
'1b854c76' (more or less) copied the code from remoteDispatchAuthPolkit
and adjusted it. Then commit id 'c7542573' adjusted the remote.c
code to call the new API (virPolkitCheckAuth). Finally, commit id
'308c0c5a' altered the code to call DBus APIs directly. In doing
so, it reverted the failing error message to the generic message
that would have been received from DBus anyway.
John Ferlan [Wed, 24 Feb 2016 15:52:30 +0000 (10:52 -0500)]
secret: Introduce secretAssignDef
This new API will allocate the secret, assign the def pointer, and
insert the secret onto the passed list. Whether that's the temporary
list in loadSecrets which gets loaded into the driver list or driver
list during secretDefineXML.
John Ferlan [Wed, 24 Feb 2016 15:57:28 +0000 (10:57 -0500)]
secret: Introduce listUnlinkSecret
Add a temporary helper to search for a specific secret by address
on the list and remove it if it's found. The following patch will
introduce a common allocation and listInsert helper. That means
error paths of the routines calling would need a way to remove the
secret off the list.
John Ferlan [Wed, 24 Feb 2016 01:14:04 +0000 (20:14 -0500)]
secret: Create a 'base64File' in virSecretObj
This patch removes need for secretBase64Path and secretComputePath. Similar
to the configFile, create an entry for base64File, which will be generated
as the driver->configDir, the UUID value, plus the ".base" suffix. Rather
than generating on the fly, store this in the virSecretObj.
The buildup of the pathname done in loadSecrets where the failure to build
is ignored which is no different than the failure to generate the name
in secretLoadValue which would have been ignored in the failure path
after secretLoad.
This also removes the need for secretComputPath and secretBase64Path.
John Ferlan [Wed, 24 Feb 2016 00:04:32 +0000 (19:04 -0500)]
secret: Create a 'configFile' in virSecretObj
This patch removes the need for secretXMLPath. Instead save 'path' during
loadSecret as 'configFile'. The secretXMLPath is nothing more than an
open coded virFileBuildPath. All that code did was concantenate the
driver->configDir, the UUID of the secret, and the ".xml" suffix to form
the configFile name which we now will generate and save instead.
John Ferlan [Tue, 23 Feb 2016 23:51:21 +0000 (18:51 -0500)]
secret: Adjust logic to build file path in secretLoad
The 'secretLoad' was essentially open coding virFileBuildPath.
Adjust the logic to have the caller build the path and pass it. The net
sum of ignoring the virFileBuildPath failure is the same as before where
the failure to virAsprintf the path would have been ignored anyway in
the secretLoad error path.
John Ferlan [Tue, 23 Feb 2016 18:44:12 +0000 (13:44 -0500)]
secret: Various formatting cleanups
Rather than having it interspersed with other changes, do it once.
Remove a couple ^L, 1 argument per line for functions, less than 80 chars
per line, use of spacing between logical groups of code, use of one line
if statements when doing fetch followed by comparison, use direct return
when no cleanup to be done.
Peter Krempa [Wed, 24 Feb 2016 13:40:51 +0000 (14:40 +0100)]
qemu: Allow setting pinning of emulator/iohtread with automatic placement
We honour the placement bitmaps when starting up, so there's no point in
having this check. Additionally the check was buggy since it checked
vm->def all the time even if the user requested to modify the persistent
definition which had different configuration.
Add Spice graphics gl attribute. qemu 2.6 should have -spice gl=on argument to
enable opengl rendering context (patches on the ML). This is necessary to
actually enable virgl rendering.
Add a qemuxml2argv test for virtio-gpu + spice with virgl.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Per-domain directories were introduced in order to be able to
completely separate security labels for each domain (commit f1f68ca33433825ce0deed2d96f1990200bc6618). However when the domain
name is long (let's say a ridiculous 110 characters), we cannot
connect to the monitor socket because on length of UNIX socket address
is limited. In order to get around this, let's shorten it in similar
fashion and in order to avoid conflicts, throw in an ID there as well.
Also save that into the status XML and load the old status XMLs
properly (to clean up after older domains). That way we can change it
in the future.
The shortening can be seen in qemuxml2argv tests, for example in the
hugepages-pages2 case.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
John Ferlan [Thu, 25 Feb 2016 20:47:56 +0000 (15:47 -0500)]
storage: Fix error path in storagePoolDefineXML
Found by inspection - after calling virStoragePoolObjAssignDef the
pool is part of the driver->pools.objs list and the failure path
for the virStoragePoolObjSaveDef will use virStoragePoolObjRemove
to remove the pool from the objs list which will unlock and free
the pool pointer (as pools->objs[i] during the loop). Since the call
doesn't clear the pool address from the callee, we need to set it
to NULL; otherwise, the virStoragePoolObjUnlock in the cleanup: code
will fail miserably.
John Ferlan [Thu, 25 Feb 2016 20:24:27 +0000 (15:24 -0500)]
storage: Fix error path in virStoragePoolObjLoad
While reviewing how storage driver used ObjListPtr's for reference
in some recent secret driver patches to use the same mechanism, I came
across an instance where the wrong API was called for error paths after
successfully allocating the storage pool pointer and inserting into
the driver pool list.
The path is after virStoragePoolObjAssignDef succeeds - the 'def' passed
in is assigned to pool->def (or newDef) so it shouldn't be the only thing
deleted. The pool is now part of driver->pools.objs, so it would need to
be removed (as happens in the storagePoolCreateXML error paths).
Rather than calling virStoragePoolDefFree to free the def which is now
assigned to the pool, call virStoragePoolObjRemove to ensure the pool
element is removed from the driver list and that anything stored in pool
is properly handled by virStoragePoolObjFree including the call to
virStoragePoolDefFree for the pool->{def|newDef} element.
This fix for https://bugzilla.redhat.com/show_bug.cgi?id=1267256
unconditionally required a machine type for all machine types
even though qemu is the only emulator using them.
Revert it to fix persistent configs for drivers with no machine type:
https://www.redhat.com/archives/libvir-list/2016-February/msg01228.html
Jim Fehlig [Wed, 24 Feb 2016 22:40:00 +0000 (15:40 -0700)]
libxl: unref objects in error paths
libxlMakeNic opens a virConnect object and takes a reference on a
virNetwork object, but doesn't drop the references on all error
paths. Rework the function to follow the standard libvirt pattern
of using a local 'ret' variable to hold the function return value,
performing all cleanup and returning 'ret' at a 'cleanup' label.
John Ferlan [Fri, 19 Feb 2016 19:03:16 +0000 (14:03 -0500)]
storage: No need to check ret after VIR_APPEND_ELEMENT
Generates a false positive for Coverity, but it turns out there's no need
to check ret == -1 since if VIR_APPEND_ELEMENT is successful, the local
vol pointer is cleared anyway.
Andrea Bolognani [Wed, 24 Feb 2016 17:06:52 +0000 (18:06 +0100)]
hostdev: Remove temporary variable when checking for VF
The virHostdevIsVirtualFunction() was called exactly twice, and in
both cases the return value was saved to a temporary variable before
being checked. This would be okay if it improved readability, but in
this case is pretty pointless.
Get rid of the temporary variable and check the return value
directly; while at it, change the check from '<= 0' to '!= 1' to
align it with the way other similar *IsVirtualFunction() functions
are used thorough the code.
Andrea Bolognani [Thu, 25 Feb 2016 11:03:35 +0000 (12:03 +0100)]
netdev: Use virNetDevIsVirtualFunction() properly
virNetDevIsVirtualFunction() returns 1 if the interface is a
virtual function, 0 if it isn't and -1 on error. This means that,
despite the name suggesting otherwise, using it as a predicate is
not correct.
Fix two callers that were doing so adding an explicit check on
the return value.
There's been a report on the upstream list [1] describing we
access /sys/devices/system/cpu/present directly on the host from
within our test suite. This may end up in unpredictable results
as no all linux systems are required to have that file. Mock
access to the file.
Eric Blake [Wed, 24 Feb 2016 22:02:44 +0000 (15:02 -0700)]
build: accomodate selinux 2.5 header API change
Yet again, selinux has been adding const-correctness; this change
is ABI-compatible, but breaks API, which affects us when we try to
override things in our testsuite:
../../tests/securityselinuxhelper.c:307:24: error: conflicting types for 'selabel_open'
struct selabel_handle *selabel_open(unsigned int backend,
^~~~~~~~~~~~
In file included from ../../tests/securityselinuxhelper.c:32:0:
/usr/include/selinux/label.h:73:24: note: previous declaration of 'selabel_open' was here
The problem is a new 'const' prior to the second parameter.
Fix it the same way we did in commit 292d3f2d: check for the new
const at configure time.
Joao Martins [Wed, 24 Feb 2016 13:25:46 +0000 (13:25 +0000)]
libxl: implement virDomainInterfaceStats
Introduce support for domainInterfaceStats API call for querying
network interface statistics. Consequently it also enables the use of
`virsh domifstat <dom> <interface name>` command plus seeing the
interfaces names instead of "-" when doing `virsh domiflist <dom>`.
After successful guest creation we fill the network interfaces names
based on domain, device id and append suffix if it's emulated in the
following form: vif<domid>.<devid>[-emu]. We extract the network
interfaces info from the libxl_domain_config object in
libxlDomainCreateIfaceNames() to generate ifname. On domain cleanup we
also clear ifname, in case it was set by libvirt (i.e. being prefixed
with "vif"). We also skip these two steps in case the name of the
interface was manually inserted by the administrator. Since the
introduction of netprefix (commit a040ba9), ifnames with a registered
prefix will be freed on virDomain{Obj,Def}Format*, thus eliminating
the migration issues observed with the reverted commit d2e5538 whereas
source and destination would have the same ifname.
For getting the interface statistics we resort to virNetInterfaceStats
and let libvirt handle the platform specific nits. Note that the
latter is not yet supported in FreeBSD.
Eric Blake [Tue, 23 Feb 2016 23:35:19 +0000 (16:35 -0700)]
rbd: fix 32-bit build
%zu is not always synonymous with uint64_t; on 32-bit machines,
size_t is only 32 bits. Prefer "%lld"/'unsigned long long' when
the variable is under our control, and "%"PRIu64 when we are
stuck with 'uint64_t' from RBD.
Fixes errors such as:
../../src/storage/storage_backend_rbd.c: In function 'virStorageBackendRBDVolWipe':
../../src/storage/storage_backend_rbd.c:1281:15: error: format '%zu' expects argument of type 'size_t', but argument 8 has type 'uint64_t {aka long long unsigned int}' [-Werror=format=]
VIR_DEBUG("Need to wipe %zu bytes from RBD image %s/%s",
^
../../src/util/virlog.h:90:73: note: in definition of macro 'VIR_DEBUG_INT'
virLogMessage(src, VIR_LOG_DEBUG, filename, linenr, funcname, NULL, __VA_ARGS__)
^
../../src/storage/storage_backend_rbd.c:1281:5: note: in expansion of macro 'VIR_DEBUG'
VIR_DEBUG("Need to wipe %zu bytes from RBD image %s/%s",
^
Michal Privoznik [Tue, 23 Feb 2016 08:38:26 +0000 (09:38 +0100)]
qemuBuildCommandLine: Change the condition for -nographics
There's this check when building command line that whenever
domain has no graphics card configured we put -nographics onto
qemu command line. The check is 'if (!def->graphics)'. This
makes coverity think that def->graphics can be NULL, which is
true. But later in the code every access to def->graphics is
guarded by check for def->ngraphics, so no crash occurs. But this
is something that coverity fails to deduct.
In order to shut coverity up lets change the condition to
'if (!def->ngraphics)'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Tue, 23 Feb 2016 08:34:49 +0000 (09:34 +0100)]
xen: Check return value of virStringReplace
After 6604a3dd9f8 in which new helper function has been
introduced, the code calls virStringReplace and dereference the
result immediately. The string function can, however, return NULL
so this would SIGSEGV right away. Check for the return value of
the string function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Tue, 23 Feb 2016 08:27:34 +0000 (09:27 +0100)]
vbox: Avoid signed and unsigned comparison
After 457ff97fa there are two defects in our code. In both of
them we use a signed variable to hold up a number of snapshots
that domain has. We use a helper function to count the number.
However, the helper function may fail in which case it returns
a negative one and control jumps to cleanup label where an
unsigned variable is used to iterate over array of snapshots. The
loop condition thus compare signed and unsigned variables which
in this specific case ends up badly for us.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Sat, 13 Feb 2016 10:19:44 +0000 (11:19 +0100)]
docs: Try harder to uninstall
Imagine you have partially installed libvirt, or maybe you're
just running 'make uninstall' from a different version than 'make
install' has been ran. One way or another, we are doing plain
'rm' instead of 'rm -f' and thus not trying hard enough when
uninstalling. In the rest of our code we stick with -f switch. Do
that for docs too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Jim Fehlig [Thu, 11 Feb 2016 03:53:36 +0000 (20:53 -0700)]
libxl: add support for rbd qdisk
xl/libxl already supports qemu's network-based block backends
such as nbd and rbd. libvirt has supported configuring such
<disk>s for long time too. This patch adds support for rbd
disks in the libxl driver by generating a rbd device URL from
the virDomainDiskDef object. The URL is passed to libxl via the
pdev_path field of libxl_device_disk struct. libxl then passes
the URL to qemu for cosumption by the rbd backend.
Jim Fehlig [Thu, 11 Feb 2016 00:39:18 +0000 (17:39 -0700)]
xenconfig: support xl<->xml conversion of rbd disk devices
The target= setting in xl disk configuration can be used to encode
meta info that is meaningful to a backend. Leverage this fact to
support qdisk network disk types such as rbd. E.g. <disk> config
such as
can be converted to the following xl config (and vice versa)
disk = [ "format=raw,vdev=hdb,access=rw,backendtype=qdisk,
target=rbd:pool/image:auth_supported=none:mon_host=mon1.example.org\\:6321\\;mon2.example.org\\:6322\\;mon3.example.org\\:6322"
]
Note that in xl disk config, a literal backslash in target= must
be escaped with a backslash. Conversion of <auth> config is not
handled in this patch, but can be done in a follow-up patch.
Change the xl disk formatter to produce this syntax, which allows
target= to contain meta info needed to setup a network-based
disksrc (e.g. rbd, nbd, iscsi). For details on xl disk config
format, see $xen-src/docs/misc/xl-disk-configuration.txt
Update the disk config in the tests to use the formal syntax.
But add tests to ensure disks specified with the positional
parameter syntax are correctly converted to <disk> XML.
Michal Privoznik [Mon, 22 Feb 2016 12:33:20 +0000 (13:33 +0100)]
cmdNetworkUpdate: Prefer VSH_EXCLUSIVE_OPTIONS over if-else tree
We have macros that check and reject mutually exclusive
parameters to our commands. Use those instead of if-else tree.
At the same time, the variable @current becomes useless therefore
it is dropped.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Thu, 18 Feb 2016 17:27:13 +0000 (18:27 +0100)]
qemu_process: mark auto-generated spice ports as reserved
In case you will specify graphics like this:
<graphics type='spice' port='-1'/>
or
<graphics type='spice' port='-1' tlsPort='6000'/>
libvirt will automatically add autoport='no'. This leads to an issue
that in qemuProcessStop() we don't release that port because we are
releasing both port if autoport=yes or only port marked as reserved.
If autoport=no but we request to generate port via '-1' we need to mark
that port as reserved in order to release it.
Michal Privoznik [Mon, 22 Feb 2016 06:59:25 +0000 (07:59 +0100)]
virDomainDefFormatInternal: Drop useless check
There's a check if a domain definition has any graphics card and
if so, we iterate over each one of them. This makes no sense,
because even if it has none we can still iterate over.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When getting a list of servers registered for a daemon, it's
returned as a dynamically allocated array filled in with pointers
to constant strings. Because the array is dynamic, it should be
freed when no longer needed (but not the strings!). Even the
function that creates the array suggests that.
==19446== 48 bytes in 3 blocks are definitely lost in loss record 821 of 1,034
==19446== at 0x4C2C28E: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==19446== by 0x54BAFC8: virReallocN (viralloc.c:245)
==19446== by 0x54BB0BE: virExpandN (viralloc.c:294)
==19446== by 0x54BB391: virInsertElementsN (viralloc.c:436)
==19446== by 0x164E3D: virNetDaemonGetServerNames (virnetdaemon.c:217)
==19446== by 0x15616F: adminDaemonListServers (admin_server.c:52)
==19446== by 0x155B8C: adminDispatchConnectListServers (admin.c:151)
==19446== by 0x155FD8: adminDispatchConnectListServersHelper (admin_dispatch.h:101)
==19446== by 0x568E862: virNetServerProgramDispatchCall (virnetserverprogram.c:437)
==19446== by 0x568E3C3: virNetServerProgramDispatch (virnetserverprogram.c:307)
==19446== by 0x5687B5B: virNetServerProcessMsg (virnetserver.c:135)
==19446== by 0x5687C1B: virNetServerHandleJob (virnetserver.c:156)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Thu, 18 Feb 2016 15:13:42 +0000 (16:13 +0100)]
gic: Introduce VIR_GIC_VERSION_DEFAULT alias
GIC v2 is the default, but checking against that specific version when
we want to know whether the default has been selected is potentially
error prone; using an alias instead makes it safer.
Andrea Bolognani [Thu, 18 Feb 2016 14:44:20 +0000 (15:44 +0100)]
docs: List possible GIC versions
Recent changes to the handling of GIC version, specifically commit 2a7b11eafb67, have clearly defined what values are acceptable for the
version attribute of the <gic> element. Update the documentation
accordingly.
Peter Krempa [Fri, 19 Feb 2016 15:04:15 +0000 (16:04 +0100)]
qemu: iothreadpin: Always set affinity when pinning iothread
Similarly to VM startup always set the legacy affinity. Additionally we
don't need to report an explicit error since virProcessSetAffinity
reports them themselves.
Peter Krempa [Fri, 19 Feb 2016 15:04:15 +0000 (16:04 +0100)]
qemu: emulatorpin: Always set affinity when pinning emulator thread
Similarly to VM startup always set the legacy affinity. Additionally we
don't need to report an explicit error since virProcessSetAffinity
reports them themselves.
Cole Robinson [Wed, 17 Feb 2016 16:53:07 +0000 (11:53 -0500)]
qemu: parse: drop redundant AddImplicitControllers
PostParse handles it for us now.
This causes some test suite churn; qemu's custom PostParse could is
now invoked before the generic AddImplicitControllers, so PCI
controllers end up sequentially in the XML before the generically
added IDE controllers. So it's just some XML reordering
Cole Robinson [Fri, 8 Jan 2016 03:49:57 +0000 (22:49 -0500)]
domain: add implicit controllers from post parse
Seems like the natural fit, since we are already adding other XML bits
in the PostParse routine.
Previously AddImplicitControllers was only called at the end of XML
parsing, meaning code that builds a DomainDef by hand had to manually
call it. Now those PostParse callers get it for free.
There's some test churn here; xen xm and sexpr test suite bits weren't
calling this before, but now they are, so you'll see new IDE controllers.
I don't think this will cause problems in practice, since the code already
needs to handle these implicit controllers like in the case when a user
defines their own XML.
Jiri Denemark [Tue, 16 Feb 2016 09:49:26 +0000 (10:49 +0100)]
Check for active domain in virDomainObjWait
virDomainObjWait is designed to be called in a loop. Make sure we break
the loop in case the domain dies to avoid waiting for an event which
will never happen.
Jiri Denemark [Thu, 11 Feb 2016 10:20:28 +0000 (11:20 +0100)]
qemu: Avoid calling qemuProcessStop without a job
Calling qemuProcessStop without a job opens a way to race conditions
with qemuDomainObjExitMonitor called in another thread. A real world
example of such a race condition:
- migration thread (A) calls qemuMigrationWaitForSpice
- another thread (B) starts processing qemuDomainAbortJob API
- thread B signals thread A via qemuDomainObjAbortAsyncJob
- thread B enters monitor (qemuDomainObjEnterMonitor)
- thread B calls qemuMonitorSend
- thread A awakens and calls qemuProcessStop
- thread A calls qemuMonitorClose and sets priv->mon to NULL
- thread B calls qemuDomainObjExitMonitor with priv->mon == NULL
=> monitor stays ref'ed and locked
Depending on how lucky we are, the race may result in a memory leak or
it can even deadlock libvirtd's event loop if it tries to lock the
monitor to process an event received before qemuMonitorClose was called.
Jiri Denemark [Thu, 11 Feb 2016 14:32:48 +0000 (15:32 +0100)]
qemu: Process monitor EOF in a job
Stopping a domain without a job risks a race condition with another
thread which started a job a which does not expect anyone else to be
messing around with the same domain object.
Jiri Denemark [Thu, 11 Feb 2016 14:13:09 +0000 (15:13 +0100)]
qemu: Introduce qemuProcessBeginStopJob
When destroying a domain we need to make sure we will be able to start a
job no matter what other operations are running or even stuck in a job.
This is done by killing the domain before starting the destroy job.
Let's introduce qemuProcessBeginStopJob which combines killing a domain
and starting a job in a single API which can be called everywhere we
need a job to stop a domain.
Jiri Denemark [Thu, 28 Jan 2016 12:48:17 +0000 (13:48 +0100)]
qemu: End nested jobs properly
Ending a nested job is no different from ending any other (non-async)
job, after all the code in qemuDomainBeginJobInternal does not handle
them differently either. Thus we should call qemuDomainObjEndJob to stop
nested jobs.
Peter Krempa [Fri, 12 Feb 2016 07:24:07 +0000 (08:24 +0100)]
qemu: qemuDomainGetStatsVcpu: Fix output for possible sparse vCPU settings
qemuDomainHelperGetVcpus would correctly return an array of
virVcpuInfoPtr structs for online vcpus even for sparse topologies, but
the loop that fills the returned typed parameters would number the vcpus
incorrectly. Fortunately sparse topologies aren't supported yet.
Peter Krempa [Fri, 12 Feb 2016 05:15:47 +0000 (06:15 +0100)]
virsh: cmdVcpuPin: Simplify handling of API flags
Rather than setting flags to -1 if none were specified, move the logic
to use the old API to the place where we need to decide. It simplifies
the logic a bit.
Andrea Bolognani [Fri, 19 Feb 2016 12:25:38 +0000 (13:25 +0100)]
test: qemuxml2argv: Drop QEMU_CAPS_DEVICE uses
Since commit 51045df01b3c, the QEMU_CAPS_DEVICE capability is enabled
automatically and shouldn't be passed as an argument to DO_TEST();
however, commit 998a936c4c1a accidentally introduced few such uses.
Erik Skultety [Thu, 18 Feb 2016 13:39:18 +0000 (14:39 +0100)]
admin: Fix memory leak in remoteAdminConnectClose
When virt-admin is run with valgrind, this kind of output can be obtained:
HEAP SUMMARY:
in use at exit: 134,589 bytes in 1,031 blocks
total heap usage: 2,667 allocs, 1,636 frees, 496,755 bytes allocated
88 bytes in 1 blocks are definitely lost in loss record 82 of 128
at 0x4C2A9C7: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x52F6D1F: virAllocVar (viralloc.c:560)
by 0x5350268: virObjectNew (virobject.c:193)
by 0x53503E0: virObjectLockableNew (virobject.c:219)
by 0x4E3BBCB: virAdmConnectNew (datatypes.c:832)
by 0x4E38495: virAdmConnectOpen (libvirt-admin.c:209)
by 0x10C541: vshAdmConnect (virt-admin.c:107)
by 0x10C7B2: vshAdmReconnect (virt-admin.c:163)
by 0x10CC7C: cmdConnect (virt-admin.c:298)
by 0x110838: vshCommandRun (vsh.c:1224)
by 0x10DFD8: main (virt-admin.c:862)
LEAK SUMMARY:
definitely lost: 88 bytes in 1 blocks
indirectly lost: 0 bytes in 0 blocks
possibly lost: 0 bytes in 0 blocks
still reachable: 134,501 bytes in 1,030 blocks
suppressed: 0 bytes in 0 blocks
This is because virNetClientSetCloseCallback was being reinitialized
incorrectly. By resetting the callbacks in a proper way, the leak is fixed.
Matthias Bolte [Mon, 15 Feb 2016 20:17:49 +0000 (21:17 +0100)]
esx: Avoid using vSphere SessionIsActive function
A login session with the vSphere API might expire after some idle time.
The esxVI_EnsureSession function uses the SessionIsActive function to
check if the current session has expired and a relogin needs to be done.
But the SessionIsActive function needs the Sessions.ValidateSession
privilege that is considered as an admin level privilege.
Only vCenter actually provides the SessionIsActive function. This results
in requiring an admin level privilege even for read-only operations on
a vCenter server.
ESX and VMware Server don't provide the SessionIsActive function and
the code already works around that. Use the same workaround for vCenter
again.