A small fix for the possiblitiy of jumping to an error path before
registering for domain events, preventing receiving important ones
like shutdown and death.
libxlDomainRestoreFlags acquires the driver lock while reading the
domain config from the save file and adding it to
libxlDriverPrivatePtr->domains. But virDomainObjList provides
self-locking APIs, so remove the needless driver locking.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
If available, let libxl handle reaping any children it creates by
specifying libxl_sigchld_owner_libxl_always_selective_reap. This
feature was added to improve subprocess handling in libxl when used
in an application that does not install a SIGCHLD handler like
libvirt
http://lists.xen.org/archives/html/xen-devel/2014-01/msg01555.html
Prior to this patch, it is possible to hit asserts in libxl when
reaping subprocesses, particularly during simultaneous operations
on multiple domains. With this patch, and the corresponding changes
to libxl, I no longer see the asserts. Note that the libxl changes
will be included in Xen 4.4.0. Previous Xen versions will be
susceptible to hitting the asserts even with this patch applied to
the libvirt libxl driver.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Handling the domain shutdown event within the event handler seems
a bit unfair to libxl's event machinery. Domain "shutdown" could
take considerable time. E.g. if the shutdown reason is reboot,
the domain must be reaped and then started again.
Spawn a shutdown handler thread to do this work, allowing libxl's
event machinery to go about its business.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Due to some misunderstanding of requirements libxl places on timer
handling, I introduced the half-brained idea of maintaining a list
of timeouts that the driver could force to expire before freeing a
libxlDomainObjPrivate (and hence libxl_ctx). But testing all
the latest versions of Xen supported by the libxl driver (4.2.3,
4.3.1, 4.4.0 RC3), I see that libxl will handle this just fine and
there is no need to force expiration behind libxl's back. Indeed it
may be harmful to do so.
This patch removes the timer list, allowing libxl to handle cleanup
of its timer registrations.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
When libxl registers an FD with the libxl driver, the refcnt of the
associated libxlDomainObjPrivate object is incremented. The refcnt
is decremented when libxl deregisters the FD. But some FDs are only
deregistered when their libxl ctx is freed, which unfortunately is
done in the libxlDomainObjPrivate dispose function. With references
held by the FDs, libxlDomainObjPrivate is never disposed.
I added the ref/unref in FD registration/deregistration when adding
the same in timer registration/deregistration. For timers, this
is a simple approach to ensuring the libxlDomainObjPrivate is not
disposed prior to their expirtation, which libxl guarantees will
occur. It is not needed for FDs, and only causes
libxlDomainObjPrivate to leak.
This patch removes the reference on libxlDomainObjPrivate for FD
registrations, but retains them for timer registrations. Tested on
the latest releases of Xen supported by the libxl driver: 4.2.3,
4.3.1, and 4.4.0 RC3.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1058839
Commit f9f56340 for CVE-2014-0028 almost had the right idea - we
need to check the ACL rules to filter which events to send. But
it overlooked one thing: the event dispatch queue is running in
the main loop thread, and therefore does not normally have a
current virIdentityPtr. But filter checks can be based on current
identity, so when libvirtd.conf contains access_drivers=["polkit"],
we ended up rejecting access for EVERY event due to failure to
look up the current identity, even if it should have been allowed.
Furthermore, even for events that are triggered by API calls, it
is important to remember that the point of events is that they can
be copied across multiple connections, which may have separate
identities and permissions. So even if events were dispatched
from a context where we have an identity, we must change to the
correct identity of the connection that will be receiving the
event, rather than basing a decision on the context that triggered
the event, when deciding whether to filter an event to a
particular connection.
If there were an easy way to get from virConnectPtr to the
appropriate virIdentityPtr, then object_event.c could adjust the
identity prior to checking whether to dispatch an event. But
setting up that back-reference is a bit invasive. Instead, it
is easier to delay the filtering check until lower down the
stack, at the point where we have direct access to the RPC
client object that owns an identity. As such, this patch ends
up reverting a large portion of the framework of commit f9f56340.
We also have to teach 'make check' to special-case the fact that
the event registration filtering is done at the point of dispatch,
rather than the point of registration. Note that even though we
don't actually use virConnectDomainEventRegisterCheckACL (because
the RegisterAny variant is sufficient), we still generate the
function for the purposes of documenting that the filtering
takes place.
Also note that I did not entirely delete the notion of a filter
from object_event.c; I still plan on using that for my upcoming
patch series for qemu monitor events in libvirt-qemu.so. In
other words, while this patch changes ACL filtering to live in
remote.c and therefore we have no current client of the filtering
in object_event.c, the notion of filtering in object_event.c is
still useful down the road.
* src/check-aclrules.pl: Exempt event registration from having to
pass checkACL filter down call stack.
* daemon/remote.c (remoteRelayDomainEventCheckACL)
(remoteRelayNetworkEventCheckACL): New functions.
(remoteRelay*Event*): Use new functions.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Drop unused parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/domain_event.c (virDomainEventFilter): Delete unused
function.
* src/conf/network_event.c (virNetworkEventFilter): Likewise.
* src/libxl/libxl_driver.c: Adjust caller.
* src/lxc/lxc_driver.c: Likewise.
* src/network/bridge_driver.c: Likewise.
* src/qemu/qemu_driver.c: Likewise.
* src/remote/remote_driver.c: Likewise.
* src/test/test_driver.c: Likewise.
* src/uml/uml_driver.c: Likewise.
* src/vbox/vbox_tmpl.c: Likewise.
* src/xen/xen_driver.c: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Coverity complains about default: label in libxl_driver.c not be able
to be reached. It's by design for the code and since it's not necessary
in the code nor does it elicit any compiler/make check warnings - just
remove it rather than adding a coverity[dead_error_begin] tag.
While I'm at it, lxc_driver.c and nodeinfo.c have the same design, so I
removed the default labels and the existing coverity tags.
I noticed that we allow virDomainGetVcpusFlags even for read-only
connections, but that with a flag, it can require guest agent
interaction. It is feasible that a malicious guest could
intentionally abuse the replies it sends over the guest agent
connection to possibly trigger a bug in libvirt's JSON parser,
or withhold an answer so as to prevent the use of the agent
in a later command such as a shutdown request. Although we
don't know of any such exploits now (and therefore don't mind
posting this patch publicly without trying to get a CVE assigned),
it is better to err on the side of caution and explicitly require
full access to any domain where the API requires guest interaction
to operate correctly.
I audited all commands that are marked as conditionally using a
guest agent. Note that at least virDomainFSTrim is documented
as needing a guest agent, but that such use is unconditional
depending on the hypervisor (so the existing domain:fs_trim ACL
should be sufficient there, rather than also requirng domain:write).
But when designing future APIs, such as the plans for obtaining
a domain's IP addresses, we should copy the approach of this patch
in making interaction with the guest be specified via a flag, and
use that flag to also require stricter access checks.
* src/libvirt.c (virDomainGetVcpusFlags): Forbid guest interaction
on read-only connection.
(virDomainShutdownFlags, virDomainReboot): Improve docs on agent
interaction.
* src/remote/remote_protocol.x
(REMOTE_PROC_DOMAIN_SNAPSHOT_CREATE_XML)
(REMOTE_PROC_DOMAIN_SET_VCPUS_FLAGS)
(REMOTE_PROC_DOMAIN_GET_VCPUS_FLAGS, REMOTE_PROC_DOMAIN_REBOOT)
(REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS): Require domain:write for any
conditional use of a guest agent.
* src/xen/xen_driver.c: Fix clients.
* src/libxl/libxl_driver.c: Likewise.
* src/uml/uml_driver.c: Likewise.
* src/qemu/qemu_driver.c: Likewise.
* src/lxc/lxc_driver.c: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
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>
This basically reverts commit ba64b97134
"libxl: Allow libxl to set NIC devid". However assigning devid's
before calling libxlMakeNic does not work as that is calling
libxl_device_nic_init which sets it back to -1.
Right now auto-assignment only works in the hotplug case. But even if
that would be fixed at some point (if that is possible at all), this
would add a weird dependency between Xen and libvirt versions.
The change here should accept any auto-assignment that makes it into
libxl_device_nic_init. My understanding is that a caller always is
allowed to make the devid choice itself. And assuming libxlMakeNicList
is only used on domain creation, a sequential numbering should be ok.
Signed-off-by: Stefan Bader <stefan.bader@canonical.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>
The libvirt_internal.h header was included by the internal.h header.
This made it painful to add new stuff to the header file that would
require some more specific types. Remove inclusion by internal.h and add
it to appropriate places manually.
As pointed out by the Xen folks [1], HVM nics should always be set
to type LIBXL_NIC_TYPE_VIF_IOEMU unless the user explicity requests
LIBXL_NIC_TYPE_VIF via model='netfront'. The current logic in
libxlMakeNic() only sets the nictype to LIBXL_NIC_TYPE_VIF_IOEMU if
a model is specified that is not 'netfront', which breaks PXE booting
configurations where no model is specified (i.e. use the hypervisor
default).
Reported-by: Stefan Bader <stefan.bader@canonical.com>
[1] https://www.redhat.com/archives/libvir-list/2013-December/msg01156.html
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>
there is a segfault in libxl logging in libxl_ctx_free when domain
create fail. because the log output handler vmessage is freed by
xtl_logger_destroy before libxl_ctx_free in virDomainObjListRemove.
move xtl_logger_destroy after libxl_ctx_free could fix this bug.
Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.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>
Report the error in virPortAllocatorAcquire instead
of doing it in every caller.
The error contains the port range name instead of the intended
use for the port, e.g.:
Unable to find an unused port in range 'display' (65534-65535)
instead of:
Unable to find an unused port for SPICE
This also adds error reporting when the QEMU driver could not
find an unused port for VNC, VNC WebSockets or NBD migration.
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
Currently we were storing domain feature flags in a bit field as the
they were either enabled or disabled. New features such as paravirtual
spinlocks however can be tri-state as the default option may depend on
hypervisor version.
To allow storing tri-state feature state in the same place instead of
having to declare dedicated variables for each feature this patch
refactors the bit field to an array.
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>
'const fooPtr' is the same as 'foo * const' (the pointer won't
change, but it's contents can). But in general, if an interface
is trying to be const-correct, it should be using 'const foo *'
(the pointer is to data that can't be changed).
Fix up offenders in src/conf/domain_conf, and their fallout.
Several things to note: virObjectLock() requires a non-const
argument; if this were C++, we could treat the locking field
as 'mutable' and allow locking an otherwise 'const' object, but
that is a more invasive change, so I instead dropped attempts
to be const-correct on domain lookup. virXMLPropString and
friends require a non-const xmlNodePtr - this is because libxml2
is not a const-correct library. We could make the src/util/virxml
wrappers cast away const, but I figured it was easier to not
try to mark xmlNodePtr as const. Finally, virDomainDeviceDefCopy
was a rather hard conversion - it calls virDomainDeviceDefPostParse,
which in turn in the xen driver was actually modifying the domain
outside of the current device being visited. We should not be
adding a device on the first per-device callback, but waiting until
after all per-device callbacks are complete.
* src/conf/domain_conf.h (virDomainObjListFindByID)
(virDomainObjListFindByUUID, virDomainObjListFindByName)
(virDomainObjAssignDef, virDomainObjListAdd): Drop attempt at
const.
(virDomainDeviceDefCopy): Use intended type.
(virDomainDeviceDefParse, virDomainDeviceDefPostParseCallback)
(virDomainVideoDefaultType, virDomainVideoDefaultRAM)
(virDomainChrGetDomainPtrs): Make const-correct.
* src/conf/domain_conf.c (virDomainObjListFindByID)
(virDomainObjListFindByUUID, virDomainObjListFindByName)
(virDomainDeviceDefCopy, virDomainObjListAdd)
(virDomainObjAssignDef, virDomainHostdevSubsysUsbDefParseXML)
(virDomainHostdevSubsysPciOrigStatesDefParseXML)
(virDomainHostdevSubsysPciDefParseXML)
(virDomainHostdevSubsysScsiDefParseXML)
(virDomainControllerModelTypeFromString)
(virDomainTPMDefParseXML, virDomainTimerDefParseXML)
(virDomainSoundCodecDefParseXML, virDomainSoundDefParseXML)
(virDomainWatchdogDefParseXML, virDomainRNGDefParseXML)
(virDomainMemballoonDefParseXML, virDomainNVRAMDefParseXML)
(virSysinfoParseXML, virDomainVideoAccelDefParseXML)
(virDomainVideoDefParseXML, virDomainHostdevDefParseXML)
(virDomainRedirdevDefParseXML)
(virDomainRedirFilterUsbDevDefParseXML)
(virDomainRedirFilterDefParseXML, virDomainIdMapEntrySort)
(virDomainIdmapDefParseXML, virDomainVcpuPinDefParseXML)
(virDiskNameToBusDeviceIndex, virDomainDeviceDefCopy)
(virDomainVideoDefaultType, virDomainHostdevAssignAddress)
(virDomainDeviceDefPostParseInternal, virDomainDeviceDefPostParse)
(virDomainChrGetDomainPtrs, virDomainControllerSCSINextUnit)
(virDomainSCSIDriveAddressIsUsed)
(virDomainDriveAddressIsUsedByDisk)
(virDomainDriveAddressIsUsedByHostdev): Fix fallout.
* src/openvz/openvz_driver.c (openvzDomainDeviceDefPostParse):
Likewise.
* src/libxl/libxl_domain.c (libxlDomainDeviceDefPostParse):
Likewise.
* src/qemu/qemu_domain.c (qemuDomainDeviceDefPostParse)
(qemuDomainDefaultNetModel): Likewise.
* src/lxc/lxc_domain.c (virLXCDomainDeviceDefPostParse):
Likewise.
* src/uml/uml_driver.c (umlDomainDeviceDefPostParse): Likewise.
* src/xen/xen_driver.c (xenDomainDeviceDefPostParse): Split...
(xenDomainDefPostParse): ...since per-device callback is not the
time to be adding a device.
Signed-off-by: Eric Blake <eblake@redhat.com>
The regular expression used to determine guest capabilities
was compiled in libxlCapsInitHost() but used in libxlCapsInitGuests().
Move compilation to libxlCapsInitGuests() where it is used, and free
the compiled regex after use. Ensure not to free the regex if
compilation fails.
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.
libxlMakeDomCreateInfo() 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.
If we reached cleanup: prior to allocating cpus, it was possible that
'nr_nodes' had a value, but cpus was NULL leading to a possible NULL
deref. Add a 'cpus' as an end condition to for loop
From: Dario Faggioli <dario.faggioli@citrix.com>
Starting from Xen 4.2, libxl has all the bits and pieces in place
for retrieving an adequate amount of information about the host
NUMA topology. It is therefore possible, after a bit of shuffling,
to arrange those information in the way libvirt wants to present
them to the outside world.
Therefore, with this patch, the <topology> section of the host
capabilities is properly populated, when running on Xen, so that
we can figure out whether or not we're running on a NUMA host,
and what its characteristics are.
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// capabilities
<capabilities>
<host>
<cpu>
....
<topology>
<cells num='2'>
<cell id='0'>
<memory unit='KiB'>6291456</memory>
<cpus num='8'>
<cpu id='0' socket_id='1' core_id='0' siblings='0-1'/>
<cpu id='1' socket_id='1' core_id='0' siblings='0-1'/>
<cpu id='2' socket_id='1' core_id='1' siblings='2-3'/>
<cpu id='3' socket_id='1' core_id='1' siblings='2-3'/>
<cpu id='4' socket_id='1' core_id='9' siblings='4-5'/>
<cpu id='5' socket_id='1' core_id='9' siblings='4-5'/>
<cpu id='6' socket_id='1' core_id='10' siblings='6-7'/>
<cpu id='7' socket_id='1' core_id='10' siblings='6-7'/>
</cpus>
</cell>
<cell id='1'>
<memory unit='KiB'>6881280</memory>
<cpus num='8'>
<cpu id='8' socket_id='0' core_id='0' siblings='8-9'/>
<cpu id='9' socket_id='0' core_id='0' siblings='8-9'/>
<cpu id='10' socket_id='0' core_id='1' siblings='10-11'/>
<cpu id='11' socket_id='0' core_id='1' siblings='10-11'/>
<cpu id='12' socket_id='0' core_id='9' siblings='12-13'/>
<cpu id='13' socket_id='0' core_id='9' siblings='12-13'/>
<cpu id='14' socket_id='0' core_id='10' siblings='14-15'/>
<cpu id='15' socket_id='0' core_id='10' siblings='14-15'/>
</cpus>
</cell>
</cells>
</topology>
</host>
....
Cleanup the libxl capabilities code to be a bit more extensible,
splitting out the creation of host and guest capabilities. This
should make it easier to implement additional capabilities in the
future, such as NUMA topology reporting.
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>
libxl supports the LIBXL_DISK_BACKEND_QDISK disk backend, where qemu
is used to provide the disk backend. This patch simply maps the
existing <driver name='qemu'/> to LIBXL_DISK_BACKEND_QDISK.
Specifying an unsupported disk format with the tap driver resulted in
a less than helpful error message
error: Failed to start domain test-hvm
error: internal error libxenlight does not support disk driver qed
Change the message to state that the qed format is not supported by
the tap driver, e.g.
error: Failed to start domain test-hvm
error: internal error libxenlight does not support disk format qed
with disk driver tap
While at it, check for unsupported formats in the other driver
backends.
libxl contains logic to determine an appropriate devid for new devices
that do not specify one in their configuration. For all device types
except NICs, the libxl driver allows libxl to determine devid. Do the
same for NICs.
Do not leave uninitialized variables, not all parameters are set in
libxlMake*.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
libxl uses some xenstore entries for hints in memory management
(especially when starting new domain). This includes dom0 memory limit
and Xen free memory margin, based on current system state. Entries are
created at first function usage, so force such call at daemon startup,
which most likely will be before any domain startup.
Also prevent automatic memory management if dom0_mem= option passed to
xen hypervisor - it is known to be incompatible with autoballoon.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
When creating a timer/event handler reference counting is used. So it could
be possible (in theory) that libxlDomainObjPrivateFree is called with
reference counting >1. The problem is that libxlDomainObjPrivateFree leave
the object in an invalid state with ctx freed (but still having dandling
pointer). This can lead timer/event handler to core.
This patch implements a dispose method for libxlDomainObjPrivate, and moves
freeing the libxl ctx to the dispose method, ensuring the ctx is valid while
the object's reference count is > 0.
Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
Actually only those interface types are handled correctly so reject
others instead of ignoring settings (i.e. treating as bridge/ethernet
anyway).
Also allow <script/> in 'ethernet' (which should be the only
script-allowing type). Keep <script/> allowed in bridge to be compatible
with legacy 'xen' driver.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
The legacy xen toolstack will set pygrub as the bootloader if not
specified. For compatibility, do the same in the libxl driver
iff not using direct kernel boot.
Currently, the libxl driver reports a connection type of "xenlight".
To be compatible with the legacy Xen driver, it should return "Xen".
Note: I noticed this while testing the libxl driver on OpenStack.
After switching my Xen compute nodes to use the libxl stack, I
could no longer launch instances on those nodes since
hypervisor_type was reported as "xenlight" instead of "xen".
libxl expects the event handler to free the event passed to it. From
libxl_event.h:
event becomes owned by the application and must be freed, either
by event_occurs or later
Xen 4.3 fixes a mistake in the libxl event handler signature where the
event owned by the application was defined as const. Detect this and
define the libvirt libxl event handler signature appropriately.
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=851411https://bugzilla.redhat.com/show_bug.cgi?id=955500
The first problem was that virFileOpenAs was returning fd (-1) in one
of the error cases rather than ret (-errno), so the caller thought
that the error was EPERM rather than ENOENT.
The second problem was that some log messages in the general purpose
qemuOpenFile() function would always say "Failed to create" even if
the caller hadn't included O_CREAT (i.e. they were trying to open an
existing file).
This fixes virFileOpenAs to jump down to the error return (which
returns ret instead of fd) in the previously mentioned incorrect
failure case of virFileOpenAs(), removes all error logging from
virFileOpenAs() (since the callers report it), and modifies
qemuOpenFile to appropriately use "open" or "create" in its log
messages.
NB: I seriously considered removing logging from all callers of
virFileOpenAs(), but there is at least one case where the caller
doesn't want virFileOpenAs() to log any errors, because it's just
going to try again (qemuOpenFile()). We can't simply make a silent
variation of virFileOpenAs() though, because qemuOpenFile() can't make
the decision about whether or not it wants to retry until after
virFileOpenAs() has already returned an error code.
Likewise, I also considered changing virFileOpenAs() to return -1 with
errno set on return, and may still do that, but only as a separate
patch, as it obscures the intent of this patch too much.
The LXC, QEMU, and LibXL drivers have all merged their handling of
the attach/update/modify device APIs into one large
'xxxxDomainModifyDeviceFlags'
which then does a 'switch()' based on the actual API being invoked.
While this saves some lines of code, it is not really all that
significant in the context of the driver API impls as a whole.
This merger of the handling of different APIs creates pain when
wanting to automated analysis of the code and do things which
are specific to individual APIs. The slight duplication of code
from unmerged the API impls, is preferrable to allow for easier
automated analysis.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the virGetHostname() API has a bogus virConnectPtr
parameter. This is because virtualization drivers directly
reference this API in their virDriverPtr tables, tieing its
API design to the public virConnectGetHostname API design.
This also causes problems for access control checks since
these must only be done for invocations from the public
API, not internal invocation.
Remove the bogus virConnectPtr parameter, and make each
hypervisor driver provide a dedicated function for the
driver API impl. This will allow access control checks
to be easily inserted later.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The source code base needs to be adapted as well. Some files
include virutil.h just for the string related functions (here,
the include is substituted to match the new file), some include
virutil.h without any need (here, the include is removed), and
some require both.
libxlBuildDomainConfig() was disposing the libxl_domain_config object
on error, only to have it disposed again by libxlBuildDomainConfig()'s
caller, which resulted in a segfault. Leave disposing of the config
object to it's owner.
this patch fix the wrong sequence for fd and timeout register. the sequence
was right in dfa1e1dd for fd register, but it changed in e0622ca2.
in this patch, set priv, xl_priv in info and increase info->priv ref count
before virEventAddHandle. if do this after virEventAddHandle, the fd
callback or fd deregister maybe got the empty priv, xl_priv or wrong ref
count.
after apply this patch, test more than 100 rounds passed compare to fail
within 3 rounds without this patch. each round includes define -> start ->
destroy -> create -> suspend -> resume -> reboot -> shutdown -> save ->
resotre -> dump -> destroy -> create -> setmem -> setvcpus -> destroy.
Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
Ensure that all drivers implementing public APIs use a
naming convention for their implementation that matches
the public API name.
eg for the public API virDomainCreate make sure QEMU
uses qemuDomainCreate and not qemuDomainStart
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Ensure that the driver struct field names match the public
API names. For an API virXXXX we must have a driver struct
field xXXXX. ie strip the leading 'vir' and lowercase any
leading uppercase letters.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
