Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends. We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.
Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration. But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access. The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global. Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.
Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.
* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Ever since their introduction (commit 1509b80 in v0.5.0 for
virConnectDomainEventRegister, commit 4445723 in v0.8.0 for
virConnectDomainEventDeregisterAny), the event deregistration
functions have been documented as returning 0 on success;
likewise for older registration (only the newer RegisterAny
must return a non-zero callbackID). And now that we are
adding virConnectNetworkEventDeregisterAny for v1.2.1, it
should have the same semantics.
Fortunately, all of the stateful drivers have been obeying
the docs and returning 0, thanks to the way the remote_driver
tracks things (in fact, the RPC wire protocol is unable to
send a return value for DomainEventRegisterAny, at least not
without adding a new RPC number). Well, except for vbox,
which was always failing deregistration, due to failure to
set the return value to anything besides its initial -1.
But for local drivers, such as test:///default, we've been
returning non-zero numbers; worse, the non-zero numbers have
differed over time. For example, in Fedora 12 (libvirt 0.8.2),
calling Register twice would return 0 and 1 [the callbackID
generated under the hood]; while in Fedora 20 (libvirt 1.1.3),
it returns 1 and 2 [the number of callbacks registered for
that event type]. Since we have changed the behavior over
time, and since it differs by local vs. remote, we can safely
argue that no one could have been reasonably relying on any
particular behavior, so we might as well obey the docs, as well
as prepare callers that might deal with older clients to not be
surprised if the docs are not strictly followed.
For consistency, this patch fixes the code for all drivers,
even though it only makes an impact for vbox and for local
drivers. By fixing all drivers, future copy and paste from
a remote driver to a local driver is less likely to
reintroduce the bug.
Finally, update the testsuite to gain some coverage of the
issue for local drivers, including the first test of old-style
domain event registration via function pointer instead of
event id.
* src/libvirt.c (virConnectDomainEventRegister)
(virConnectDomainEventDeregister)
(virConnectDomainEventDeregisterAny): Clarify docs.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventDeregister)
(libxlConnectDomainEventDeregisterAny): Match documentation.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventDeregister)
(lxcConnectDomainEventDeregisterAny): Likewise.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventDeregister)
(testConnectDomainEventDeregisterAny)
(testConnectNetworkEventDeregisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventDeregister)
(umlConnectDomainEventDeregisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister)
(vboxConnectDomainEventDeregister)
(vboxConnectDomainEventDeregisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventDeregister)
(xenUnifiedConnectDomainEventDeregisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventDeregisterAny): Likewise.
* tests/objecteventtest.c (testDomainCreateXMLOld): New test.
(mymain): Run it.
(testDomainCreateXML): Check return values.
Signed-off-by: Eric Blake <eblake@redhat.com>
By actually removing the <vcpupin> element (from within the
<cputune> section) from the XML, rather than jus update it with
a fully set vcpu affinity mask.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
And use it to implement libxlDomainPinVcpu(), similarly to what
happens in the QEMU driver. This way, it is possible to both
query and change the vcpu affinity of a persistent but not
running domain.
In face, before this patch, we have:
# virsh list --all
Id Name State
----------------------------------------------------
5 debian_32 running
- fedora20_64 shut off
# virsh vcpupin fedora20_64 0 2-4 --current
error: this function is not supported by the connection driver: virDomainPinVcpuFlags
After (same situation as above):
# virsh vcpupin fedora20_64 0 2-4 --current
# virsh vcpupin fedora20_64 0
VCPU: CPU Affinity
----------------------------------
0: 2-4
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
So that it is possible to query vcpu related information of
a persistent but not running domain, like it is for the QEMU
driver.
In fact, before this patch, we have:
# virsh list --all
Id Name State
----------------------------------------------------
5 debian_32 running
- fedora20_64 shut off
# virsh vcpuinfo fedora20_64
error: this function is not supported by the connection driver: virDomainGetVcpuPinInfo
After (same situation as above, i.e., fedora20_64 not running):
# virsh vcpuinfo fedora20_64
VCPU: 0
CPU: N/A
State: N/A
CPU time N/A
CPU Affinity: yyyyyyyy
VCPU: 1
CPU: N/A
State: N/A
CPU time N/A
CPU Affinity: yyyyyyyy
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
by, in libxlDomainGetNumaParameters(), calling libxl_bitmap_init() as soon as
possible, which avoids getting to 'cleanup:', where libxl_bitmap_dispose()
happens, without having initialized the nodemap, and hence crashing after some
invalid free()-s:
# ./daemon/libvirtd -v
*** Error in `/home/xen/libvirt.git/daemon/.libs/lt-libvirtd': munmap_chunk(): invalid pointer: 0x00007fdd42592666 ***
======= Backtrace: =========
/lib64/libc.so.6(+0x7bbe7)[0x7fdd3f767be7]
/lib64/libxenlight.so.4.3(libxl_bitmap_dispose+0xd)[0x7fdd2c88c045]
/home/xen/libvirt.git/daemon/.libs/../../src/.libs/libvirt_driver_libxl.so(+0x12d26)[0x7fdd2caccd26]
/home/xen/libvirt.git/src/.libs/libvirt.so.0(virDomainGetNumaParameters+0x15c)[0x7fdd4247898c]
/home/xen/libvirt.git/daemon/.libs/lt-libvirtd(+0x1d9a2)[0x7fdd42ecc9a2]
/home/xen/libvirt.git/src/.libs/libvirt.so.0(virNetServerProgramDispatch+0x3da)[0x7fdd424e9eaa]
/home/xen/libvirt.git/src/.libs/libvirt.so.0(+0x1a6f38)[0x7fdd424e3f38]
/home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa81e5)[0x7fdd423e51e5]
/home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa783e)[0x7fdd423e483e]
/lib64/libpthread.so.0(+0x7c53)[0x7fdd3febbc53]
/lib64/libc.so.6(clone+0x6d)[0x7fdd3f7e1dbd]
Signed-off-by: Dario Faggili <dario.faggioli@citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Starting from commit 2e82c18c in Xen (will be included in Xen 4.4)
both libxl_get_max_cpus() and libxl_get_max_nodes() start returning
a proper libxl error code, in case of failure. This patch fixes
this in the libxl driver.
Note that, although it is now basically impossible for them to return
0, that would, theoretically, still be wrong. Also, checking that the
returned value is '<= 0' makes the code correct for both Xen 4.4 and
Xen 4.3 (and 4.2), and that is why we go for it (rather than
just '< 0').
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Most of our code base uses space after comma but not before;
fix the remaining uses before adding a syntax check.
* src/libxl/libxl_driver.c: Consistently use commas.
* src/xen/xend_internal.c: Likewise.
* src/xen/xs_internal.c: Likewise.
* src/xenapi/xenapi_driver.c: Likewise.
* src/xenapi/xenapi_utils.c: Likewise.
* src/xenxs/xen_sxpr.c: Likewise.
* src/xenxs/xen_xm.c: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
ifdef LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS hides a multi-line body
for a brace-less else. Add braces to ensure proper logic is applied.
Without this fix, new domains cannot be started. Both
libxl_domain_create_new and libxl_domain_create_restore are called when
starting a new domain leading to this error:
libxl: error: libxl.c:324:libxl__domain_rename: domain with name "guest" already exists.
libxl: error: libxl_create.c:800:initiate_domain_create: cannot make domain: -6
Rather than casting the virBitmap pointer to uint8_t* and then using
the structure contents as a byte array, use the virBitmap API to determine
the bitmap size and test each bit.
Signed-off-by: Jeremy Fitzhardinge <jeremy@goop.org>
in recently xen commit: 7051d5c8, there is a api changes in
libxl_domain_create_restore.
Author: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Thu Oct 10 12:23:10 2013 +0100
tools/migrate: Fix regression when migrating from older version of Xen
use the macro LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS in libxl.h
in order to make libvirt could compile with old and new xen.
the params checkpointed_stream is useful if libvirt libxl driver
support migration. for new, set it as zero.
Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
According to the following valgrind output, there seems to be a
invalid limit for the iterator (captured on Fedora 19):
==3945== Invalid read of size 1
==3945== at 0x1E1FA410: libxlVmStart (libxl_driver.c:475)
==3945== by 0x1E1FAD9A: libxlDomainCreateWithFlags (libxl_driver.c:2633)
==3945== by 0x5187D46: virDomainCreate (libvirt.c:9439)
==3945== by 0x13BAA6: remoteDispatchDomainCreateHelper (remote_dispatch.h:2910)
==3945== by 0x51DE5B9: virNetServerProgramDispatch (virnetserverprogram.c:435)
==3945== by 0x51D93E7: virNetServerHandleJob (virnetserver.c:165)
==3945== by 0x50F5BF4: virThreadPoolWorker (virthreadpool.c:144)
==3945== by 0x50F5670: virThreadHelper (virthreadpthread.c:161)
==3945== by 0x8046C52: start_thread (pthread_create.c:308)
==3945== by 0x8758E1C: clone (clone.S:113)
==3945== Address 0x23424d81 is 0 bytes after a block of size 1 alloc'd
==3945== at 0x4A08121: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3945== by 0x50B1F8C: virAllocN (viralloc.c:189)
==3945== by 0x1E1FA3CA: libxlVmStart (libxl_driver.c:468)
==3945== by 0x1E1FAD9A: libxlDomainCreateWithFlags (libxl_driver.c:2633)
==3945== by 0x5187D46: virDomainCreate (libvirt.c:9439)
==3945== by 0x13BAA6: remoteDispatchDomainCreateHelper (remote_dispatch.h:2910)
==3945== by 0x51DE5B9: virNetServerProgramDispatch (virnetserverprogram.c:435)
==3945== by 0x51D93E7: virNetServerHandleJob (virnetserver.c:165)
==3945== by 0x50F5BF4: virThreadPoolWorker (virthreadpool.c:144)
==3945== by 0x50F5670: virThreadHelper (virthreadpthread.c:161)
==3945== by 0x8046C52: start_thread (pthread_create.c:308)
==3945== by 0x8758E1C: clone (clone.S:113)
==3945==
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1013045
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
No need to check if privileged when reading hostsysinfo, since
that check was already done in libxlDriverShouldLoad(). The
libxl driver fails to load if not privileged.
John Ferlan reported the following Coverity warning:
In libxlDomainCoreDump() Coverity has noted a FORWARD_NULL reference:
2004 if ((flags & VIR_DUMP_CRASH) && !vm->persistent) {
2005 virDomainObjListRemove(driver->domains, vm);
(20) Event assign_zero: Assigning: "vm" = "NULL".
Also see events: [var_deref_model]
2006 vm = NULL;
2007 }
2008
2009 ret = 0;
2010
2011 cleanup_unpause:
(21) Event var_deref_model: Passing null pointer "vm" to function
"virDomainObjIsActive(virDomainObjPtr)", which dereferences it. [details]
Also see events: [assign_zero]
2012 if (virDomainObjIsActive(vm) && paused) {
2013 if (libxl_domain_unpause(priv->ctx, dom->id) != 0) {
2014 virReportError(VIR_ERR_INTERNAL_ERROR,
Removing the vm from domain obj list and setting it to NULL can be
done in the previous 'if (flags & VIR_DUMP_CRASH)' conditional. Fix
the Coverity warning by ensuring vm is not NULL before testing if it
is still active.
Now that most fields of libxlDriverPrivate struct are immutable
or self-locking, there is no need to acquire the driver lock in
much of the libxl driver.
The libxlDriverPrivate struct contains an variety of data with
varying access needs. Similar to the QEMU and LXC drivers,
move all the static config data into a dedicated libxlDriverConfig
object. The only locking requirement is to hold the driver lock
while obtaining an instance of libxlDriverConfig. Once a reference
is held on the config object, it can be used completely lockless
since it is immutable.
libxlDomainGetInfo() uses the driver-wide libxl ctx when
it would be more appropriate to use the per-domain ctx
associated with the domain. Switch to using the per-domain
libxl ctx.
libxl version info is static data as far as the libxl driver
is concerned, so retrieve this info when the driver is initialized
and stash it in the libxlDriverPrivate object. Subsequently use
the stashed info instead of repeatedly calling libxl_get_version_info().
Detect early on in libxl driver initialization if the driver
should be loaded at all, avoiding needless initialization steps
that only have to be undone later. While at it, move the
detection to a helper function to improve readability.
After detecting that the driver should be loaded, subsequent
failures such as initializing the log stream, allocating libxl
ctx, etc. should be treated as failure to initialize the driver.
Create libxl_domain.[ch] and move all functions operating on
libxlDomainObjPrivate to these files. This will be useful for
future patches that e.g. add job support for libxlDomainObjPrivate.
More fallout from commit d72ef888. When reconnecting to running
domains, the libxl_ctx in libxlDomainObjPrivate was used before
initializing it, causing a segfault in libxl and consequently
crashing libvirtd.
Initialize the libxlDomainObjPrivate libxl_ctx in libxlReconnectDomain,
and while at it use this ctx in libxlReconnectDomain instead of the
driver-wide ctx.
There is a potential leak of a newly created libxlDomainObjPrivate
when subsequent allocation of the object's chrdev field fails.
Unref the object on such an error so that it is properly disposed.
Commit d72ef888 introduced a bug in the libxl driver that will
segfault libvirtd if libxl reports an error message, e.g. when
attempting to initialize the driver on a non-Xen system. I
assumed it was valid to pass a NULL logger to libxl_ctx_alloc(),
but that is not the case since any errors associated with the ctx
that are emitted by libxl will dereference the logger and crash
libvirtd.
Errors associated with the libxl driver-wide ctx could be useful
for debugging anyway, so create a 'libxl-driver.log' to capture
these errors.
introduced by cs 4b9eec50fe ("libxl: implement per
NUMA node free memory reporting"). What was wrong was that
libxl_get_numainfo() put in nr_nodes the actual number of
host NUMA nodes, not the highest node ID (like libnuma's
numa_max_node() does instead).
While at it, turn the failure of libxl_get_numainfo() from
a simple warning to a proper error, as requested during the
review of another patch of the original series.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Daniel P. Berrange <berrange@redhat.com>
Currently, only one log file is created by the libxl driver, with
all output from libxl for all domains going to this one file.
Create a per-domain log file based on domain name, making sifting
through the logs a bit easier. This required deferring libxl_ctx
allocation until starting the domain, which is fine since the
ctx is not used when the domain is inactive.
Tested-by: Dario Faggioli <dario.faggioli@citrix.com>
this patch introduce the console api in libxl driver for both pv and
hvm guest. and import and update the libxlMakeChrdevStr function
which was deleted in commit dfa1e1dd.
Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
The avail_vcpu bitmap has to be allocated before it can be used (using
the maximum allowed value for that). Then for each available VCPU the
bit in the mask has to be set (libxl_bitmap_set takes a bit position
as an argument, not the number of bits to set).
Without this, I would always only get one VCPU for guests created
through libvirt/libxl.
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Although, having it depending on Xen >= 4.3 (by using the proper
libxl feature flag).
Xen currently implements a NUMA placement policy which is basically
the same as the 'interleaved' policy of `numactl', although it can
be applied on a subset of the available nodes. We therefore hardcode
"interleave" as 'numa_mode', and we use the newly introduced libxl
interface to figure out what nodes a domain spans ('numa_nodeset').
With this change, it is now possible to query the NUMA node
affinity of a running domain:
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// list
Id Name State
----------------------------------------------------
23 F18_x64 running
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// numatune 23
numa_mode : interleave
numa_nodeset : 1
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
domainGetNumaParameters has a string typed parameter, hence it
is necessary for the libxl driver to support this.
This change implements the connectSupportsFeature hook for the
libxl driver, advertising that VIR_DRV_FEATURE_TYPED_PARAM_STRING
is supported.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Eric Blake <eblake@redhat.com>
Convert the type of loop iterators named 'i', 'j', k',
'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or
'unsigned int', also santizing 'ii', 'jj', 'kk' to use
the normal 'i', 'j', 'k' naming
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Use virDomainObjListRemoveLocked instead of virDomainObjListRemove, as
driver->domains is already taken by virDomainObjListForEach.
Above deadlock can be triggered when libvirtd is started after some
domain have been started by hand (in which case driver will not find
libvirt-xml domain config).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
By providing the implementation of nodeGetCellsFreeMemory for
the driver. This is all just a matter of properly formatting, in
a way that libvirt like, what Xen provides via libxl_get_numainfo().
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// freecell --all
0: 25004 KiB
1: 105848 KiB
--------------------
Total: 130852 KiB
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
