Limiting ourselves to qemu without QEMU_CAPS_DEVICE capability, we
used '-serial none' only if there was no serial device defined in the
domain XML. This means that if we want to have a possibility of the
device being defined in XML, but not used in the command-line
(e.g. when it's pointless), we'll fail to attach '-serial none' to the
command-line (when skipping the device's command-line building and the
device being the only one).
Since there is no such device, this patch doesn't actually do
anything, but enables easier future additions in this manner.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Add a new character device backend called 'spiceport' that uses
spice's channel for communications and apart from spicevmc can be used
as a backend for any character device from libvirt's point of view.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This new RBD format supports snapshotting and cloning. By having
libvirt create images in format 2 end-users of the created images
can benefit from the new RBD format.
Older versions of libvirt can work with this new RBD format as long
as librbd supports format 2. RBD format is supported by librbd since
version 0.56 (Ceph Bobtail).
Signed-off-by: Wido den Hollander <wido@widodh.nl>
When restarting sheepdog pool, all volumes are missing.
This patch add automatically all volume from the added pool.
Adding last Daniel P. Berrange's syntaxes correction.
Adding vol on separeted function 'inspired' from parallels_storage :
parallelsAddDiskVolume
In order to make a client-only build successful on RHEL4 (yes, you
read that correctly!), commit 3ed2e54 modified src/util/virnetdev.c so
that the functional version of virNetDevGetVLanID() was only compiled
if GET_VLAN_VID_CMD was defined. However, it is *never* defined, but
is only an enum value, so the proper version was no longer compiled
even on platforms that support it. This resulted in the vlan tag not
being properly set for guest traffic on VEPA mode guest macvtap
interfaces that were bound to a vlan interface (that's the only place
that libvirt currently uses virNetDevGetVLanID)
Since there is no way to compile conditionally based on the presence
of an enum value, this patch modifies configure.ac to check for said
enum value with AC_CHECK_DECLS(), which #defines
HAVE_DECL_GET_VLAN_VID_CMD to 1 if it's successful compiling a test
program that uses GET_VLAN_VID_CMD (and still #defines it, but to 0,
if it's not successful). We can then make the compilation of
virNetDevGetVLanID() conditional on the value of
HAVE_DECL_GET_VLAN_VID_CMD.
Reset line numbering on each input file in check-aclrules.pl. Otherwise
it reports wrong line numbers in its error messages.
Signed-off-by: Yuri Myasoedov <ymyasoedov@yandex.ru>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
In the network status XML we may have the <floor/> element with the
'sum' attribute. The attribute represents sum of all 'floor'-s of
computed over each interface connected to the network (this is needed to
guarantee certain bandwidth for certain domain). The sum is therefore a
number. However, if the number was mangled (e.g. by an user's
interference to network status file), we've just ignored it without
refusing to parse such file. This was all due to 'goto error' missing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The code took into account only the global permissions. The domains now
support per-vm DAC labels and per-image DAC labels. Use the most
specific label available.
The lack of debug printings might be frustrating in the future.
Moreover, this function doesn't follow the usual pattern we have in the
rest of the code:
int ret = -1;
/* do some work */
ret = 0;
cleanup:
/* some cleanup work */
return ret;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add a new <timer> for the HyperV reference time counter enlightenment
and the iTSC reference page for Windows guests.
This feature provides a paravirtual approach to track timer events for
the guest (similar to kvmclock) with the option to use real hardware
clock on systems with a iTSC with compensation across various hosts.
According to the documentation various timer options are only supported
by certain timer types. Add a post parse check to verify that the user
didn't specify invalid options.
Also fix the qemu command line parsing function to set correct default
values for the kvmclock timer so that it passes the new check.
According to the documentation describing various tunables for domain
timers not all the fields are supported by all the driver types. Express
these in the RNG:
- rtc, platform: Only these support the "track" attribute.
- tsc: only one to support "frequency" and "mode" attributes
- hpet, pit: tickpolicy/catchup attribute/element
- kvmclock: no extra attributes are supported
Additionally the attributes of the <catchup> element for
tickpolicy='catchup' are optional according to the parsing code. Express
this in the XML and fix a spurious space added while formatting the
<catchup> element and add tests for it.
Coverity complains about "USE_AFTER_FREE" due to how virPCIDeviceSetStubDriver
"could" return either -1, 0, or 1 from the VIR_STRDUP() and then possibly makes
a call to virPCIDeviceDetach().
The only way this could happen is if NULL were passed as the "driver" name
and virStrdup() returned 0. Since the calling functions check < 0 on the
initial function call, the 0 possibility causes Coverity to complain.
To fix this - enforce that the second parameter is not NULL using
ATTRIBUTE_NONNULL(2) for the function prototype, then in virPCIDeviceDetach
add an sa_assert(dev->stubDriver). This will result in Coverity not complaining
any more.
Couple of codepaths shared the same code which can be moved out to a
function and on one of such places, qemuMigrationConfirmPhase(), the
domain was resumed even if it wasn't running before the migration
started.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1057407
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
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>
This commit allows to attach/detach a <filesystem> device in qemu. For
this purpose I'm introducing two new functions: virDomainFSInsert() and
virDomainFSRemove() and adding necessary code in the qemu driver. It
compares filesystems based on their "destination" folder. So if two
filesystems share the same destination, they are considered equal and
the qemu driver would reject the insertion.
Signed-off-by: Matthieu Coudron <mattator@gmail.com>
If virDomainMemoryStats was run on a domain with virtio balloon driver
running on an old qemu which supports QMP but does not support qom-list
QMP command, libvirtd would crash. The reason is we did not check if
qemuMonitorJSONGetObjectListPaths failed and moreover we even stored its
result in an unsigned integer type.
When attempting a blockcommit from the top layer, the base argument
passed is NULL. This will be dereferenced when attempting a commit with
an empty image chain. Output the real volume path instead:
virsh blockcommit --verbose --path vda --domain DOMNAME --wait
error: invalid argument: top '/path/somefile' in chain for 'vda' has no backing file
instead of:
error: invalid argument: top '(null)' in chain for 'vda' has no backing file
Eric Blake suggested to change this message to be different from the
glibc's NULL deref protection message in printf to be able to
differentiate errors.
https://bugzilla.redhat.com/show_bug.cgi?id=1046192
Commit b8bf79a, which adds clock='variable', forgets to check
localtime basis in qemuBuildClockArgStr(). So that localtime
basis could not be used.
Reported-by: Jincheng Miao <jmiao@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit 2ce63c1 added imagelabel generation when relabeling is turned
off. But we weren't filling out the sensitivity for type 'none' labels,
resulting in an invalid label:
$ virsh managedsave domain
error: unable to set security context 'system_u:object_r:svirt_image_t'
on fd 28: Invalid argument
Noticed a misuse of 'to' while testing my event regression under
polkit ACLs, and decided to review the entire conf files for
other legibility bugs.
* daemon/libvirtd.conf: Use correct grammar.
* src/qemu/qemu.conf: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.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>
https://bugzilla.redhat.com/show_bug.cgi?id=1057321
pointed out that we weren't honoring the <bandwidth> element in
libvirt networks using <forward mode='bridge'/>. In fact, these
networks are just a method of giving a libvirt network name to an
existing Linux host bridge on the system, and libvirt doesn't have
enough information to know where to set such limits. We are working on
a method of supporting network bandwidths for some specific cases of
<forward mode='bridge'/>, but currently libvirt doesn't support it. So
the proper thing to do now is just log an error when someone tries to
put a <bandwidth> element in that type of network. (It's unclear if we
will be able to do proper bandwidth limiting for macvtap networks, and
most definitely we will not be able to support it for hostdev
networks).
While looking through the network XML documentation and comparing it
to the networkValidate function, I noticed that we also ignore the
presence of a mac address in the config in the same cases, rather than
failing so that the user will understand that their desired action has
not been taken.
This patch updates networkValidate() (which is called any time a
persistent network is defined, or a transient network created) to log
an error and fail if it finds either a <bandwidth> or <mac> element
and the network forward mode is anything except 'route'. 'nat', or
nothing. (Yes, neither of those elements is acceptable for any macvtap
mode, nor for a hostdev network).
NB: This does *not* cause failure to start any existing network that
contains one of those elements, so someone might have erroneously
defined such a network in the past, and that network will continue to
function unmodified. I considered it too disruptive to suddenly break
working configs on the next reboot after a libvirt upgrade.
https://bugzilla.redhat.com/show_bug.cgi?id=1045124
When loading modules, libvirt does not honor the modprobe blacklist.
Use the new virKModLoad() API in order to attempt load with blacklist check.
Use the new virKModIsBlacklisted() API to check if the failure to load
was due to the blacklist
Signed-off-by: John Ferlan <jferlan@redhat.com>
virKModConfig() - Return a buffer containing kernel module configuration
virKModLoad() - Load a specific module into the kernel configuration
virKModUnload() - Unload a specific module from the kernel configuration
virKModIsBlacklisted() - Determine whether a module is blacklisted within
the kernel configuration
commit f094aaac changed qemuPrepareHostdevPCIDevices() such that it
may modify the "backend" (vfio vs. legacy kvm) setting in the
virHostdevDef. However, qemuDomainAttachHostPciDevice() (used by
hotplug) copies the backend setting into a local *before* calling
qemuPrepareHostdevPCIDevices(), and then later makes a decision based
on that pre-change value.
The result is that, if the backend had been set to "default" (i.e. not
specified in the config) and was later updated to "VFIO" by
qemuPrepareHostdevPCIDevices(), the qemu process' MacMemLock is not
increased (as is required for VFIO device assignment).
This patch delays making the local copy of backend until after its
potential modification.
The previous patch fixed "forwardPlainNames" so that it really is
doing only what is intended, but left the default to be
"forwardPlainNames='no'". Discussion around the initial version of
that patch led to the decision that the default should instead be
"forwardPlainNames='yes'" (i.e. the original behavior before commit
f3886825). This patch makes that change to the default.
In commit f386825 we began adding the options
--domain-needed
--local=/$mydomain/
to all dnsmasq commandlines with the stated reason of preventing
forwarding of DNS queries for names that weren't fully qualified
domain names ("FQDN", i.e. a name that included some "."s and a domain
name). This was later changed to
domain-needed
local=/$mydomain/
when we moved the options from the dnsmasq commandline to a conf file.
The original patch on the list, and discussion about it, is here:
https://www.redhat.com/archives/libvir-list/2012-August/msg01594.html
When a domain name isn't specified (mydomain == ""), the addition of
"domain-needed local=//" will prevent forwarding of domain-less
requests to the virtualization host's DNS resolver, but if a domain
*is* specified, the addition of "local=/domain/" will prevent
forwarding of any requests for *qualified* names within that domain
that aren't resolvable by libvirt's dnsmasq itself.
An example of the problems this causes - let's say a network is
defined with:
<domain name='example.com'/>
<dhcp>
..
<host mac='52:54:00:11:22:33' ip='1.2.3.4' name='myguest'/>
</dhcp>
This results in "local=/example.com/" being added to the dnsmasq options.
If a guest requests "myguest" or "myguest.example.com", that will be
resolved by dnsmasq. If the guest asks for "www.example.com", dnsmasq
will not know the answer, but instead of forwarding it to the host, it
will return NOT FOUND to the guest. In most cases that isn't the
behavior an admin is looking for.
A later patch (commit 4f595ba) attempted to remedy this by adding a
"forwardPlainNames" attribute to the <dns> element. The idea was that
if forwardPlainNames='yes' (default is 'no'), we would allow
unresolved names to be forwarded. However, that patch was botched, in
that it only removed the "domain-needed" option when
forwardPlainNames='yes', and left the "local=/mydomain/".
Really we should have been just including the option "--domain-needed
--local=//" (note the lack of domain name) regardless of the
configured domain of the network, so that requests for names without a
domain would be treated as "local to dnsmasq" and not forwarded, but
all others (including those in the network's configured domain) would
be forwarded. We also shouldn't include *either* of those options if
forwardPlainNames='yes'. This patch makes those corrections.
This patch doesn't remedy the fact that default behavior was changed
by the addition of this feature. That will be handled in a subsequent
patch.
We support only one spicevmc channel name anyway and the code is
prepared to use the default one, there's only one check missing. It
is also mentioned in the documentation already and helps defining
domains with spice vdagent for people using virsh.
Signed-off-by: Martin Kletzander <mkletzan@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.
The NWFilter code has as a deadlock race condition between
the virNWFilter{Define,Undefine} APIs and starting of guest
VMs due to mis-matched lock ordering.
In the virNWFilter{Define,Undefine} codepaths the lock ordering
is
1. nwfilter driver lock
2. virt driver lock
3. nwfilter update lock
4. domain object lock
In the VM guest startup paths the lock ordering is
1. virt driver lock
2. domain object lock
3. nwfilter update lock
As can be seen the domain object and nwfilter update locks are
not acquired in a consistent order.
The fix used is to push the nwfilter update lock upto the top
level resulting in a lock ordering for virNWFilter{Define,Undefine}
of
1. nwfilter driver lock
2. nwfilter update lock
3. virt driver lock
4. domain object lock
and VM start using
1. nwfilter update lock
2. virt driver lock
3. domain object lock
This has the effect of serializing VM startup once again, even if
no nwfilters are applied to the guest. There is also the possibility
of deadlock due to a call graph loop via virNWFilterInstantiate
and virNWFilterInstantiateFilterLate.
These two problems mean the lock must be turned into a read/write
lock instead of a plain mutex at the same time. The lock is used to
serialize changes to the "driver->nwfilters" hash, so the write lock
only needs to be held by the define/undefine methods. All other
methods can rely on a read lock which allows good concurrency.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There are a number of pthreads impls available on Win32
these days, in particular the mingw64 project has a good
impl. Delete the native windows thread implementation and
rely on using pthreads everywhere.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The check-augeas-lockd test depends on the file
locking/qemu-lockd.conf, so must be skipped when QEMU
is disabled.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit 10c9ceff6d intended to introduce new argument for the
testing purpose, but it missed the similar changing of the
device's sg_path. The problem was hidden since my laptop has
the /dev/sg0 and /dev/sg1. A later patch will modify the tests
accordingly.
Signed-off-by: Osier Yang <jyang@redhat.com>
Reported-by: Pavel Hrdina <phrdina@redhat.com>
To support passing the path of the test data to the utils, one
more argument is added to virSCSIDeviceGetSgName,
virSCSIDeviceGetDevName, and virSCSIDeviceNew, and the related
code is changed accordingly.
Later tests for the scsi utils will be based on this patch.
Signed-off-by: Osier Yang <jyang@redhat.com>
It doesn't make sense to fail if the SCSI host device is specified
as "shareable" explicitly between domains (NB, it works if and only
if the device is specified as "shareable" for *all* domains,
otherwise it fails).
To fix the problem, this patch introduces an array for virSCSIDevice
struct, which records all the names of domain which are using the
device (note that the recorded domains must specify the device as
shareable). And the change on the data struct brings on many
subsequent changes in the code.
Prior to this patch, the "shareable" tag didn't work as expected,
it actually work like "non-shareable". So this patch also added notes
in formatdomain.html to declare the fact.
* src/util/virscsi.h:
- Remove virSCSIDeviceGetUsedBy
- Change definition of virSCSIDeviceGetUsedBy and virSCSIDeviceListDel
- Add virSCSIDeviceIsAvailable
* src/util/virscsi.c:
- struct virSCSIDevice: Change "used_by" to be an array; Add
"n_used_by" as the array count
- virSCSIDeviceGetUsedBy: Removed
- virSCSIDeviceFree: frees the "used_by" array
- virSCSIDeviceSetUsedBy: Copy the domain name to avoid potential
memory corruption
- virSCSIDeviceIsAvailable: New
- virSCSIDeviceListDel: Change the logic, for device which is already
in the list, just remove the corresponding entry in "used_by". And
since it's only used in one place, we can safely removing the code
to find out the dev in the list first.
- Copyright updating
* src/libvirt_private.sys:
- virSCSIDeviceGetUsedBy: Remove
- virSCSIDeviceIsAvailable: New
* src/qemu/qemu_hostdev.c:
- qemuUpdateActiveScsiHostdevs: Check if the device existing before
adding it to the list;
- qemuPrepareHostdevSCSIDevices: Error out if the not all domains
use the device as "shareable"; Also don't try to add the device
to the activeScsiHostdevs list if it already there; And make
more sensible error w.r.t the current "shareable" value in
driver->activeScsiHostdevs.
- qemuDomainReAttachHostScsiDevices: Change the logic according
to the changes on helpers.
Signed-off-by: Osier Yang <jyang@redhat.com>
This reverts commit 2996e6be19
and some parts of 2636dc8c4d.
The former one tried to implement QoS setting on bridgeless networks.
However, as discussed upstream [1], the patch is far away from being
useful in even a single case. The whole idea of network QoS is to have
aggregated limits over several interfaces. This patch is doing
completely the opposite when merging two QoS settings (from the network
and the domain interface) into one which is then set at the domain
interface itself, not the network.
The latter one is the test for the previous one. Now none of them makes
sense.
1: https://www.redhat.com/archives/libvir-list/2014-January/msg01441.html
Conflicts:
tests/virnetdevbandwidthtest.c: New test has been introduced since
then.
There are some units within libvirt that utilize virCommand API to run
some commands and deserve own unit testing. These units are, however,
not desired to be rewritten to dig virCommand API usage out. As a great
example virNetDevBandwidth could be used. The problem with the bandwidth
unit is: it uses virCommand API heavily. Therefore we need a mechanism
to not really run a command, but rather see its string representation
after which we can decide if the unit construct the correct sequence of
commands or not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add support for specifying various types when doing snapshots. This will
later allow to do snapshots on network backed volumes. Disks of type
'volume' are not supported by snapshots (yet).
Also amend the test suite to check parsing of the various new disk
types that can now be specified.
Commit df36af58 broke parsing of http response from xend. The prior
use of atoi() would happily parse e.g. a string containing "200 OK\r\n",
whereas virStrToLong_i() will fail when called with a NULL end_ptr.
Change the calls to virStrToLong_i() to provide a non-NULL end_ptr.
https://bugzilla.redhat.com/show_bug.cgi?id=1049391
When all source CPU XMLs contain just a single CPU model (with a
possibly varying set of additional feature elements),
virConnectBaselineCPU will try to use this CPU model in the computed
guest CPU. Thus, when used on just a single CPU (useful with
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES), the result will not use a
different CPU model.
If the computed CPU uses the source model, set fallback mode to 'forbid'
to make sure the guest CPU will always be as close as possible to the
source CPUs.
https://bugzilla.redhat.com/show_bug.cgi?id=1049391
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES flag for virConnectBaselineCPU
did not work if the resulting guest CPU would disable some features
present in its base model. This patch makes sure we won't try to add
such features twice.
Implement virProcess{Get,Set}Affinity() using cpuset_getaffinity()
and cpuset_setaffinity() calls. Quick search showed that they are
only available on FreeBSD, so placed it inside existing #ifdef
blocks for FreeBSD instead of adding configure checks.
Creating a qemu VM with /dev/hwrng as backend RNG device throws the
following error - "Could not open '/dev/hwrng': Permission denied"
This patch fixes the issue
Signed-off-by: Pradipta Kr. Banerjee <bpradip@in.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1055484
Currently, libvirt's XML schema of network allows QoS to be defined for
every network even though it has no bridge. For instance:
<network>
<name>vdsm-no-bridge</name>
<forward mode='passthrough'>
<interface dev='em1.10'/>
</forward>
<bandwidth>
<inbound average='1000' peak='5000' burst='1024'/>
<outbound average='1000' burst='1024'/>
</bandwidth>
</network>
The bandwidth limitations can be, however, applied even on such
networks. In fact, they are going to be applied on the interface that
will be connected to the network on a domain startup. This approach,
however, has one limitation. With bridged networks, there are two points
where QoS can be set: bridge and domain interface. The lower limit of
the two is enforced then. For instance, if the interface has 10Mbps
average, but the network only 1Mbps, there's no way for interface to
transmit packets faster than the 1Mbps limit. With two points this is
enforced by kernel. With only one point, we must combine both QoS
settings into one which is set afterwards. Look at
virNetDevBandwidthMinimal() and you'll understand immediately what I
mean.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This patch allows libvirt user to specify 'host-passthrough'
cpu mode while using qemu/kvm backend on aarch64.
It uses 'host' as a CPU model name instead of some other stub
(correct CPU detection is not implemented yet) to allow libvirt
user to specify 'host-model' cpu mode as well.
Signed-off-by: Oleg Strikov <oleg.strikov@canonical.com>
(crobinso: fix some indentation)
Currently the qemuDomainGetBlockInfo will return allocation == physical
for most backing stores. For a qcow2 block backed device it's possible
to return the highest lv extent allocated from qemu for an active guest.
That is a value where allocation != physical and one would hope be less.
However, if the guest is not running, then the code falls back to returning
allocation == physical. This turns out to be problematic for rhev which
monitors the size of the backing store. During a migration, before the
VM has been started on the target and while it is deemed inactive on the
source, there's a small window of time where the allocation is returned
as physical triggering the code to extend the file unnecessarily.
Since rhev uses transient domains and this is edge condition for a transient
domain, rather than returning good status and allocation == physical when
this "window of opportunity" exists, this patch will check for a transient
(or non persistent) domain and return a failure to the caller rather than
returning the defaults. For a persistent domain, the defaults will be
returned. The description for the virDomainGetBlockInfo has been updated
to describe the phenomena.
the array params is allocated by VIR_ALLOC_N in
remoteDispatchDomainGetCPUStats. it had been set
to zero. No need to reset it to zero again, and
this reset here is incorrect too, nparams * ncpus
is the array length not the size of params array.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
Unlike the host devices of other types, SCSI host device XML supports
"shareable" tag. This patch introduces it for the virSCSIDevice struct
for a later patch use (to detect if the SCSI device is shareable when
preparing the SCSI host device in QEMU driver).
The "checkPool" is a bit different for pool with "fc_host"
type source adapter, since the vHBA it's based on might be
not created yet (it's created by "startPool", which is
involked after "checkPool" in storageDriverAutostart). So it
should not fail, otherwise the "autostart" of the pool will
fail either.
The problem is easy to reproduce:
* Enable "autostart" for the pool
* Restart libvirtd service
* Check the pool's state
94f8205 added a space to the string but didn't change the buffer size.
Signed-off-by: Bing Bu Cao <mars@linux.vnet.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
For pool which relies on remote resources, such as a "iscsi" type
pool, since how long it takes to export the corresponding devices
to host's sysfs is really depended, it could depend on the network
connection, it also could depend on the host's udev procedures. So
it's likely that the volumes are not able to be detected during pool
starting process, polling the sysfs doesn't work, since we don't
know how much time is best for the polling, and even worse, the
volumes could still be not detected or partly not detected even after
the polling. So we end up with a documentation to prompt the fact,
in virsh manual.
And as a small improvement, let's explicitly say no LUNs found in
the debug log in that case.
There are 2 issues here: First we shouldn't add "1" to the return
value of numa_max_node(), since the semanteme of the error message
was changed, it's not saying about the number of total NUMA nodes
anymore. Second, the value of "bit" is the position of the first
bit which exceeds either numa_max_node() or NUMA_NUM_NODES, it can
be any number in the range, so saying "bigger than $bit" is quite
confused now. For example, assuming there is a NUMA machine which
has 10 NUMA nodes, and one specifies the "nodeset" as "0,5,88",
the error message will be like:
Nodeset is out of range, host cannot support NUMA node bigger than 88
It sounds like all NUMA node number less than 88 is fine, but
actually the maximum NUMA node number the machine supports is 9.
This patch fixes the issues by removing the addition with "1" and
simplifies the error message as "NUMA node $bit is out of range".
Also simplifies the comparision in the while loop by getting the
smaller one of numa_max_node() and NUMA_NUM_NODES up front.
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>
Bugs have been found in the VirtualBox API C bindings. These bugs have
been fixed in versions 4.2.20 and 4.3.4. However, the changes in the
C bindings are incompatible with the vbox_CAPI_v4_2.h and vbox_CAPI_v4_3.h
files which are bundled in libvirt source code.
This is why the following patch adds vbox_CAPI_v4_2_20.h and
vbox_CAPI_v4_3_4.h.
The actual underlying problem here is that until now,
libvirt assumed that VirtualBox API can only change between minor
versions (4.2 -> 4.3), but we have a case here where it changed
(or got fixed) between patch versions (4.2.18 -> 4.2.20).
This patch makes the VBOX_API_VERSION represent the full API
version number (i.e 4002 => 4002000) so there are specific version
numbers for Vbox 4.2.20 (4002020) and 4.3.4 (4003004)
Libvirtd would crash if a domain contained an empty cdrom drive of
type='volume' as the disk def->srcpool member would be dereferenced. Fix
it by checking if the source pool is present before dereferencing it.
Also alter tests to catch this issue in the future.
Reported by: Kevin Shanahan
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1056328
- Use $XDG_RUNTIME_DIR for re-exec state file when running unprivileged.
- argv[0] may not contain a full path to the binary, however it should
contain something that can be looked up in the PATH. Use execvp() to
do path lookup on re-exec.
- As per list discussion [1], ignore --daemon on re-exec.
[1] https://www.redhat.com/archives/libvir-list/2013-December/msg00514.html
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
To retrieve node cpu statistics on Linux system, the
linuxNodeGetCPUstats function simply uses STRPREFIX() to match the cpuid
with the one read from /proc/stat. However, as the file is read line by
line it may happen, that some CPUs share the same prefix. So if user
requested stats for the first CPU, which is offline, then there's no
cpu1 in the stats file so the one that we match is cpu10. Which is
obviously wrong. Fortunately, the IDs are terminated by a space, so we
can utilize that.
Signed-off-by: Bing Bu Cao <mars@linux.vnet.ibm.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1034993
SCSI passthrough disks (<disk .. device="lun">) can't be used as backing
for snapshots. Currently with upstream qemu the vm crashes on such
attempt.
This patch adds a early check to catch an attempt to do such a snapshot
and rejects it right away. qemu will fix the issue but this will let us
control the error message.
I noticed this problem when adding systemd support to netcf, because I
setup the configure.ac to automatically prefer using systemd over
initscripts when possible - although I had copied the
install-data-local target from the example of libvirt's
"libvirt-guests" service more or less verbatim, "make distcheck" would
fail because it was trying to install the service file directly into
/lib/systemd/system rather than into
/home/user/some/unimportant/name/lib/systemd/system.
This is caused by the install/uninstall rules for the systemd unit
files relying on $(DESTDIR) pointing the installed files to the right
place, but in reality $(DESTDIR) is empty during this part of make
distcheck - it instead sets $(prefix) with the toplevel directory used
for its test build/install/uninstall cycle.
(This problem hasn't been seen when running "make distcheck" in
libvirt because libvirt will never build/install systemd support
unless explicitly told to do so on the configure commandline, and
"make distcheck" doesn't put the "--with-initscript=..." option on the
configure commandline.)
I verified that the same problem does exist in libvirt by modifying
libvirt's configure.ac to set:
init_systemd=yes
with_init_script=systemd+redhat
This forces a build/install of the systemd unit files during
distcheck, which yields an error like this:
/usr/bin/install -c -m 644 virtlockd.service \
/lib/systemd/system/
libtool: install: warning: relinking `libvirt-qemu.la'
/usr/bin/install: cannot remove '/lib/systemd/system/virtlockd.service': Permission denied
make[4]: *** [install-systemd] Error 1
After adding $(prefix) to all the definitions of SYSTEMD_UNIT_DIR,
make distcheck now completes successfully with the modified
configure.ac, and the above lines change to something like this:
/usr/bin/install -c -m 644 virtlockd.service \
/home/laine/devel/libvirt/libvirt-1.2.1/_inst/lib/systemd/system/
We shouldn't access the domain definition while we are in the monitor
section as the domain is unlocked. Additionally after we exit from the
monitor we need to check if the VM is still alive. Not doing so resulted
in a crash if qemu exits while attempting to do an external VM snapshot.
spice-server offers an API to disable file transfer messages
on the agent channel between the client and the guest.
This is supported in qemu through the disable-agent-file-xfer option.
This patch exposes this option to libvirt.
Adds a new element 'filetransfer', with one property,
'enable', which accepts a boolean.
Default is enabled, for backward compatibility.
Depends on the capability exported in the first patch of the series.
Signed-off-by: Francesco Romani <fromani@redhat.com>
spice-server offers an API to disable file transfer messages
on the agent channel between the client and the guest.
This is supported in qemu through the disable-agent-file-xfer option.
This patch detects if QEMU supports this option, and add
a capability if does.
Signed-off-by: Francesco Romani <fromani@redhat.com>
With this patch,user can set throttle blkio cgroup for
lxc domain through virsh tool.
Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com>
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
This is useful in certain circumstances, for example when
libvirtd is being executed by FreeBSD rc script, it cannot find
dmidecode installed from FreeBSD ports because it doesn't have
/usr/local (default prefix for ports) in PATH.
https://bugzilla.redhat.com/show_bug.cgi?id=1046919
If none (KVM, VFIO) of the supported PCI passthrough methods is known to
work on a host, it's better to fail right away with a nice error message
rather than letting attachment fail with a more cryptic message such as
Failed to bind PCI device '0000:07:05.0' to vfio-pci: No such device
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1046919
Since commit v0.9.0-47-g4e8969e (released in 0.9.1) some failures during
device detach were reported to callers of virPCIDeviceBindToStub as
success. For example, even though a device seemed to be detached
virsh # nodedev-detach pci_0000_07_05_0 --driver vfio
Device pci_0000_07_05_0 detached
one could find similar message in libvirt logs:
Failed to bind PCI device '0000:07:05.0' to vfio-pci: No such device
This patch fixes these paths and also avoids overwriting real errors
with errors encountered during a cleanup phase.
https://bugzilla.redhat.com/show_bug.cgi?id=1046919
When a PCI device is not bound to any driver, reattach should just
trigger driver probe rather than failing with
Invalid device 0000:00:19.0 driver file
/sys/bus/pci/devices/0000:00:19.0/driver is not a symlink
While virPCIDeviceGetDriverPathAndName was documented to return success
and NULL driver and path when a device is not attached to any driver but
didn't do so. Thus callers could not distinguish unbound devices from
failures.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
With this patch, user can setup throttle blkio cgroup
through virsh for qemu domain.
Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com>
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
This patch introduces virCgroupSetBlkioDeviceReadIops,
virCgroupSetBlkioDeviceWriteIops,
virCgroupSetBlkioDeviceReadBps and
virCgroupSetBlkioDeviceWriteBps,
we can use these interfaces to set up throttle
blkio cgroup for domain.
This patch also adds the new throttle blkio cgroup
elements to the test xml.
Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com>
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
This patch introduces new xml elements under <blkiotune>,
we use these new elements to setup the throttle blkio
cgroup for domain. The new blkiotune node looks like this:
<blkiotune>
<device>
<path>/path/to/block</path>
<weight>1000</weight>
<read_iops_sec>10000</read_iops_sec>
<write_iops_sec>10000</write_iops_sec>
<read_bytes_sec>1000000</read_bytes_sec>
<write_bytes_sec>1000000</write_bytes_sec>
</device>
</blkiotune>
Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com>
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
https://bugzilla.redhat.com/show_bug.cgi?id=996543
When starting up a domain, the SELinux labeling is done depending on
current configuration. If the labeling fails we check for possible
causes, as not all labeling failures are fatal. For example, if the
labeled file is on NFS which lacks SELinux support, the file can still
be readable to qemu process. These cases are distinguished by the errno
code: NFS without SELinux support returns EOPNOTSUPP. However, we were
missing one scenario. In case there's a read-only disk on a read-only
NFS (and possibly any FS) and the labeling is just optional (not
explicitly requested in the XML) there's no need to make the labeling
error fatal. In other words, read-only file on read-only NFS can fail to
be labeled, but be readable at the same time.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Finish the cleanup of libvirt.c; all uses of virLib*Error have
now been converted to more canonical conventions.
* src/libvirt.c: Use virReportError in remaining errors.
(virLibConnError, virLibDomainError): Delete unused macros.
* cfg.mk (msg_gen_function): Drop unused names.
Signed-off-by: Eric Blake <eblake@redhat.com>
We had a lot of repetition of errors that would occur if we
ever register too many drivers; this is unlikely to occur
unless we start adding a lot of new hypervisor modules, but
if it does occur, it's better to have uniform handling of the
situation, so that a one-line change is all that would be
needed if we decide that an internal error is not the best.
* src/libvirt.c (virDriverCheckTabMaxReturn): New define.
(virRegister*Driver): Use it for less code duplication.
Signed-off-by: Eric Blake <eblake@redhat.com>
The choice of error message and category was not consistent
in the migration code; furthermore, the use of virLibConnError
is no longer necessary now that we have a generic virReportError.
* src/qemu/qemu_migration.c (virDomainMigrate*): Prefer
virReportError over virLibConnError.
Signed-off-by: Eric Blake <eblake@redhat.com>
While auditing the error reporting, I noticed that migration
had some issues. Some of the static helper functions tried
to call virDispatchError(), even though their caller will also
report the error. Also, if a migration is cancelled early
because a uri was not set, we did not guarantee that the finish
stage would not overwrite the first error message.
* src/qemu/qemu_migration.c (doPeer2PeerMigrate2)
(doPeer2PeerMigrate3): Preserve first error when cancelling.
* src/libvirt.c (virDomainMigrateVersion3Full): Likewise.
(virDomainMigrateVersion1, virDomainMigrateVersion2)
(virDomainMigrateDirect): Avoid redundant error dispatch.
(virDomainMigrateFinish2, virDomainMigrateFinish3)
(virDomainMigrateFinish3Params): Don't report error on cleanup
path.
Signed-off-by: Eric Blake <eblake@redhat.com>
The public virConnectRef and virConnectClose API are just thin
wrappers around virObjectRef/virObjectRef, with added object
validation and an error reset. Within our backend drivers, use
of the object validation is just an inefficiency since we always
pass valid objects. More important to think about is what
happens with the error reset; our uses of virConnectRef happened
to be safe (since we hadn't encountered any earlier errors), but
in several cases the use of virConnectClose could lose a real
error.
Ideally, we should also avoid calling virConnectOpen() from
within backend drivers - but that is a known situation that
needs much more design work.
* src/qemu/qemu_process.c (qemuProcessReconnectHelper)
(qemuProcessReconnect): Avoid nested public API call.
* src/qemu/qemu_driver.c (qemuAutostartDomains)
(qemuStateInitialize, qemuStateStop): Likewise.
* src/qemu/qemu_migration.c (doPeer2PeerMigrate): Likewise.
* src/storage/storage_driver.c (storageDriverAutostart):
Likewise.
* src/uml/uml_driver.c (umlAutostartConfigs): Likewise.
* src/lxc/lxc_process.c (virLXCProcessAutostartAll): Likewise.
(virLXCProcessReboot): Likewise, and avoid leaking conn on error.
Signed-off-by: Eric Blake <eblake@redhat.com>
Several APIs clear out a user input buffer before attempting to
populate it; but in a few cases we missed this memset if we
detect a reason for an early exit. Note that these APIs
check for non-NULL arguments, and exit early with an error
message when NULL is passed in; which means that we must be
careful to avoid a NULL deref in order to get to that error
message. Also, we were inconsistent on the use of
sizeof(virType) vs. sizeof(expression); the latter is more
robust if we ever change the type of the expression (although
such action is unlikely since these types are part of our
public API).
* src/libvirt.c (virDomainGetInfo, virDomainGetBlockInfo)
(virStoragePoolGetInfo, virStorageVolGetInfo)
(virDomainGetJobInfo, virDomainGetBlockJobInfo): Move memset
before any returns.
Signed-off-by: Eric Blake <eblake@redhat.com>
There is a number of reported issues when we fail starting a domain.
Turns out that, in some scenarios like high load, 3 second timeout is
not enough for qemu to start up to the phase where the socket is
created. Since there is no downside of waiting longer, raise the
timeout right to 30 seconds.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
To allow using the storage driver APIs to do operation on generic domain
disks we will need to introduce internal storage pools that will give is
a base to support this stuff even on files that weren't originally
defined as a part of the pool.
This patch introduces the 'internal' flag for a storage pool that will
prevent it from being listed along with the user defined storage pools.
Separate the steps to create libvirt's volume metadata from the actual
volume building process. This is already done for regular file based
pools to allow job support for storage APIs.
Currently, during XML parsing, when a call to a FromString() function to
get an enum value fails, the error which is reported is either
VIR_ERR_CONFIG_UNSUPPORTED, VIR_ERR_INTERNAL_ERROR or VIR_ERR_XML_ERROR.
This commit makes such conversion failures consistently return
VIR_ERR_CONFIG_UNSUPPORTED.
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>
We haven't had a release with network events yet, so we are free
to fix the RPC so that it actually does what we want. Doing
client-side filtering of per-network events is inefficient if a
connection is only interested in events on a single network out
of hundreds available on the server. But to do server-side
per-network filtering, the server needs to know which network
to filter on - so we need to pass an optional network over on
registration. Furthermore, it is possible to have a client with
both a global and per-network filter; in the existing code, the
server sends only one event and the client replicates to both
callbacks. But with server-side filtering, the server will send
the event twice, so we need a way for the client to know which
callbackID is sending an event, to ensure that the client can
filter out events from a registration that does not match the
callbackID from the server. Likewise, the existing style of
deregistering by eventID alone is fine; but in the new style,
we have to remember which callbackID to delete.
This patch fixes the RPC wire definition to contain all the
needed pieces of information, and hooks into the server and
client side improvements of the previous patches, in order to
switch over to full server-side filtering of network events.
Also, since we fixed this in time, all released versions of
libvirtd that support network events also support per-network
filtering, so we can hard-code that assumption into
network_event.c.
Converting domain events to server-side filtering will require
the introduction of new RPC numbers, as well as a server
feature bit that the client can use to tell whether to use
old-style (server only supports global events) or new-style
(server supports filtered events), so that is deferred to a
later set of patches.
* src/conf/network_event.c (virNetworkEventStateRegisterClient):
Assume server-side filtering.
* src/remote/remote_protocol.x
(remote_connect_network_event_register_any_args): Add network
argument.
(remote_connect_network_event_register_any_ret): Return callbackID
instead of count.
(remote_connect_network_event_deregister_any_args): Pass
callbackID instead of eventID.
(remote_connect_network_event_deregister_any_ret): Drop unused
type.
(remote_network_event_lifecycle_msg): Add callbackID.
* daemon/remote.c
(remoteDispatchConnectNetworkEventDeregisterAny): Drop unused arg,
and deal with callbackID from client.
(remoteRelayNetworkEventLifecycle): Pass callbackID.
(remoteDispatchConnectNetworkEventRegisterAny): Likewise, and
recognize non-NULL network.
* src/remote/remote_driver.c
(remoteConnectNetworkEventRegisterAny): Pass network, and track
server side id.
(remoteConnectNetworkEventDeregisterAny): Deregister by callback id.
(remoteNetworkBuildEventLifecycle): Pass remote id to event queue.
* src/remote_protocol-structs: Regenerate.
Signed-off-by: Eric Blake <eblake@redhat.com>
In order to mirror a server with per-object filtering, the client
needs to track which server callbackID is servicing the client
callback. This patch introduces the notion of a serverID, as
well as the plumbing to use it for network events, although the
actual complexity of using per-object filtering in the remote
driver is deferred to a later patch.
* src/conf/object_event.h (virObjectEventStateEventID): Add parameter.
(virObjectEventStateQueueRemote, virObjectEventStateSetRemote):
New prototypes.
(virObjectEventStateRegisterID): Move...
* src/conf/object_event_private.h: ...here, and add parameter.
(_virObjectEvent): Add field.
* src/conf/network_event.h (virNetworkEventStateRegisterClient): New
prototype.
* src/conf/object_event.c (_virObjectEventCallback): Add field.
(virObjectEventStateSetRemote): New function.
(virObjectEventStateQueue): Make wrapper around...
(virObjectEventStateQueueRemote): New function.
(virObjectEventCallbackListCount): Tweak return count when remote
id matching is used.
(virObjectEventCallbackLookup, virObjectEventStateRegisterID):
Tweak registration when remote id matching will be used.
(virObjectEventNew): Default to no remote id.
(virObjectEventCallbackListAddID): Likewise, but set remote id
when one is available.
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Adjust return value when
remote id was set.
(virObjectEventStateEventID): Query existing id.
(virObjectEventDispatchMatchCallback): Require matching event id.
(virObjectEventStateCallbackID): Adjust caller.
* src/conf/network_event.c (virNetworkEventStateRegisterClient): New
function.
(virNetworkEventStateRegisterID): Update caller.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Update callers.
* src/remote/remote_driver.c
(remoteConnectNetworkEventRegisterAny)
(remoteConnectNetworkEventDeregisterAny)
(remoteConnectDomainEventDeregisterAny): Likewise.
(remoteEventQueue): Hoist earlier to avoid forward declaration,
and add parameter. Adjust all callers.
* src/libvirt_private.syms (conf/object_event.h): Drop function.
Signed-off-by: Eric Blake <eblake@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1047659
If a VM dies very early during an attempted connect to the guest agent
while the locks are down the domain monitor object will be freed. The
object is then accessed later as any failure during guest agent startup
isn't considered fatal.
In the current upstream version this doesn't lead to a crash as
virObjectLock called when entering the monitor in
qemuProcessDetectVcpuPIDs checks the pointer before attempting to
dereference (lock) it. The NULL pointer is then caught in the monitor
helper code.
Before the introduction of virObjectLockable - observed on 0.10.2 - the
pointer is locked directly via virMutexLock leading to a crash.
To avoid this problem we need to differentiate between the guest agent
not being present and the VM quitting when the locks were down. The fix
reorganizes the code in qemuConnectAgent to add the check and then adds
special handling to the callers.
A "xmlstr" string may not be assigned into a "doc" pointer and it
could cause memory leak. To fix it if the "doc" pointer is NULL and
the "xmlstr" string is not assigned we should free it.
This has been found by coverity.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
There could be a memory leak caused by "managed_system" string, if any
error occurs before "managed_system" is assigned into
"phyp_driver->managed_system". The "managed_system" string wouldn't be
freed at all. The better way is to free the "managed_system" instead
of the one assigned in the "phyp_driver".
This has been found by coverity.
Pointed out by John, that the "phyp_driver->xmlopt" needs to be
unreferenced as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
If there is no error while executing a function "openvzParseBarrierLimit"
a "str" string where is duplicate of a "value" string isn't freed and it
leads into memory leak.
This has been found by coverity.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1047577
When writing commit 173c291, I missed the fact virNetServerClientClose
unlocks the client object before actually clearing client->sock and thus
it is possible to hit a window when client->keepalive is NULL while
client->sock is not NULL. I was thinking client->sock == NULL was a
better check for a closed connection but apparently we have to go with
client->keepalive == NULL to actually fix the crash.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
On my Fedora 20 box with mingw cross-compiler, the build failed with:
../../src/rpc/virnetclient.c: In function 'virNetClientSetTLSSession':
../../src/rpc/virnetclient.c:745:14: error: unused variable 'oldmask' [-Werror=unused-variable]
sigset_t oldmask, blockedsigs;
^
I traced it to the fact that mingw64-winpthreads installs a header
that does #define pthread_sigmask(...) 0, which means any argument
only ever passed to pthread_sigmask is reported as unused. This
patch works around the compilation failure, with behavior no worse
than what mingw already gives us regarding the function being a
no-op.
* configure.ac (pthread_sigmask): Probe for broken mingw macro.
* src/util/virutil.h (pthread_sigmask): Rewrite to something that
avoids unused variables.
Signed-off-by: Eric Blake <eblake@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1047577
When a client closes its connection to libvirtd early during
virConnectOpen, more specifically just after making
REMOTE_PROC_CONNECT_SUPPORTS_FEATURE call to check if
VIR_DRV_FEATURE_PROGRAM_KEEPALIVE is supported without even waiting for
the result, libvirtd may crash due to a race in keep-alive
initialization. Once receiving the REMOTE_PROC_CONNECT_SUPPORTS_FEATURE
call, the daemon's event loop delegates it to a worker thread. In case
the event loop detects EOF on the connection and calls
virNetServerClientClose before the worker thread starts to handle
REMOTE_PROC_CONNECT_SUPPORTS_FEATURE call, client->keepalive will be
disposed by the time virNetServerClientStartKeepAlive gets called from
remoteDispatchConnectSupportsFeature. Because the flow is common for
both authenticated and read-only connections, even unprivileged clients
may cause the daemon to crash.
To avoid the crash, virNetServerClientStartKeepAlive needs to check if
the connection is still open before starting keep-alive protocol.
Every libvirt release since 0.9.8 is affected by this bug.
Any test suite which involves a virDomainDefPtr should
call virDomainDefCheckABIStability with itself just as
a basic sanity check that the identity-comparison always
succeeds. This would have caught the recent NULL pointer
access crash.
Make sure we cope with def->name being NULL since the
VMWare config parser produces NULL names.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Ever since commit 61ac8ce, Coverity complained about
remoteNetworkBuildEventLifecycle not checking for NULL failure
to build an event, compared to other calls in the code base.
But the problem is latent from copy and paste; all 17 of our
remote*BuildEvent* functions in remote_driver.c have the same
issue - if an OOM causes an event to not be built, we happily
pass NULL to remoteEventQueue(), but that function has marked
event as a nonnull parameter. We were getting lucky (the
event queue's first use of the event happened to be a call to
virIsObjectClass(), which acts gracefully on NULL, so there
was no way to crash); but this is a latent bug waiting to bite
us due to the disregard for the nonnull attribute, as well as
a waste of resources in the event queue. Better is to just
refuse to queue NULL. The discard is silent, since the problem
only happens on OOM, and since events are already best effort -
if we fail to get an event, it's not like we have any memory
left to report the issue, nor any idea of who would benefit
from knowing we couldn't create or queue the event.
* src/remote/remote_driver.c (remoteEventQueue): Ignore NULL event.
Signed-off-by: Eric Blake <eblake@redhat.com>
Our fixes for CVE-2013-4400 were so effective at "fixing" bugs
in virt-login-shell that we ended up fixing it into a useless
do-nothing program.
Commit 3e2f27e1 picked the name LIBVIRT_SETUID_RPC_CLIENT for
the witness macro when we are doing secure compilation. But
commit 9cd6a57d checked whether the name IN_VIRT_LOGIN_SHELL,
from an earlier version of the patch series, was defined; with
the net result that virt-login-shell invariably detected that
it was setuid and failed virInitialize.
Commit b7fcc799 closed all fds larger than stderr, but in the
wrong place. Looking at the larger context, we mistakenly did
the close in between obtaining the set of namespace fds, then
actually using those fds to switch namespace, which means that
virt-login-shell will ALWAYS fail.
This is the minimal patch to fix the regressions, although
further patches are also worth having to clean up poor
semantics of the resulting program (for example, it is rude to
not pass on the exit status of the wrapped program back to the
invoking shell).
* tools/virt-login-shell.c (main): Don't close fds until after
namespace swap.
* src/libvirt.c (virGlobalInit): Use correct macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
The existing check of domain snapshots validated that they
point to a domain, but did not validate that the domain
points to a connection, even though any errors blindly assume
the connection is valid. On the other hand, as mentioned in
commit 6e130ddc, any valid domain is already tied to a valid
connection, and VIR_IS_SNAPSHOT vs. VIR_IS_DOMAIN_SNAPSHOT
makes no real difference; it's best to just validate the chain
of all three. For consistency with previous patches, continue
the trend of using a common macro. For now, we don't need
virCheckDomainSnapshotGoto().
* src/datatypes.h (virCheckDomainSnapshotReturn): New macro.
(VIR_IS_SNAPSHOT, VIR_IS_DOMAIN_SNAPSHOT):
Drop unused macros.
* src/libvirt.c: Use macro throughout.
(virLibDomainSnapshotError): Drop unused macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
While all errors related to invalid nwfilters appeared to be
consistent, we might as well continue the trend of using a
common macro. As in commit 6e130ddc, the difference between
VIR_IS_NWFILTER and VIR_IS_CONNECTED_NWFILTER is moot, since
reference counting means any valid nwfilter is also tied to
a valid connection. For now, we don't need virCheckNWFilterGoto().
* src/datatypes.h (virCheckNWFilterReturn): New macro.
(VIR_IS_NWFILTER, VIR_IS_CONNECTED_NWFILTER): Drop unused macros.
* src/libvirt.c: Use macro throughout.
(virLibNWFilterError): Drop unused macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
For streams validation, we weren't consistent on whether to
use VIR_FROM_NONE or VIR_FROM_STREAMS. Furthermore, in many
API, we want to ensure that a stream is tied to the same
connection as the other object we are operating on; while
other API failed to validate the stream at all. And the
difference between VIR_IS_STREAM and VIR_IS_CONNECTED_STREAM
is moot; as in commit 6e130ddc, we know that reference
counting means a valid stream will always be tied to a valid
connection. Similar to previous patches, use a common macro
to make it nicer.
* src/datatypes.h (virCheckStreamReturn, virCheckStreamGoto):
New macros.
(VIR_IS_STREAM, VIR_IS_CONNECTED_STREAM): Drop unused macros.
* src/libvirt.c: Use macro throughout.
(virLibStreamError): Drop unused macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
While all errors related to invalid secrets appeared to be
consistent, we might as well continue the trend of using a
common macro. Just as in commit 6e130ddc, the difference
between VIR_IS_SECRET and VIR_IS_CONNECTED_SECRET is moot
(due to reference counting, any valid secret must be tied to
a valid domain). For now, we don't need virCheckSecretGoto().
* src/datatypes.h (virCheckSecretReturn): New macro.
(VIR_IS_SECRET, VIR_IS_CONNECTED_SECRET): Drop unused macros.
* src/libvirt.c: Use macro throughout.
(virLibSecretError): Drop unused macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
While all errors related to invalid node device appeared to be
consistent, we might as well continue the trend of using a
common macro. For now, we don't need virCheckNodeDeviceGoto().
* src/datatypes.h (virCheckNodeDeviceReturn): New macro.
(VIR_IS_NODE_DEVICE, VIR_IS_CONNECTED_NODE_DEVICE): Drop
unused macros.
* src/libvirt.c: Use macro throughout.
(virLibNodeDeviceError): Drop unused macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
The commit cad3cf9a95 introduced a crash
due to wrong order of parameters being passed to the function. When
deleting an element, the function decreased the iterator instead of
count and if listing volumes after that (or undefining the pool, NULL
was being dereferenced.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
For storage volume validation, we weren't consistent on
whether to use VIR_FROM_NONE or VIR_FROM_STORAGE. Similar
to previous patches, use a common macro to make it nicer.
Furthermore, just as in commit 6e130ddc, the difference
between VIR_IS_STORAGE_VOL and VIR_IS_CONNECTED_STORAGE_VOL
is moot (due to reference counting, any valid volume must
be tied to a valid connection).
virStorageVolCreateXMLFrom allows cross-connection cloning,
where the error is reported against the connection of the
destination pool.
* src/datatypes.h (virCheckStorageVolReturn)
(virCheckStorageVolGoto): New macros.
(VIR_IS_STORAGE_VOL, VIR_IS_CONNECTED_STORAGE_VOL): Drop
unused macros.
* src/libvirt.c: Use macro throughout.
(virLibStorageVolError): Drop unused macro.
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>
virStoragePoolBuild reported an invalid pool as if it were an
invalid network. Likewise, we weren't consistent on whether to
use VIR_FROM_NONE or VIR_FROM_STORAGE. Similar to previous
patches, use a common macro to make it nicer. Furthermore, just
as in commit 6e130ddc, the difference between VIR_IS_STORAGE_POOL
and VIR_IS_CONNECTED_STORAGE_POOL is moot (due to reference
counting, any valid pool must be tied to a valid connection).
For now, we don't need virCheckStoragePoolGoto().
* src/datatypes.h (virCheckStoragePoolReturn): New macro.
(VIR_IS_STORAGE_POOL, VIR_IS_CONNECTED_STORAGE_POOL): Drop
unused macros.
* src/libvirt.c: Use macro throughout.
(virLibStoragePoolError): Drop unused macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
There is no easy way to test authentication against libvirt. This
commit modifies the test driver to allow simple username/password
authentication.
You modify the test XML by adding:
<node>
...
<auth>
<user password="123456">rich</user>
<user>jane</user>
</auth>
</node>
If there are any /node/auth/user elements, then authentication is
required by the test driver (if none are present, then the test driver
will work as before and not require authentication).
In the example above, two phony users are added:
rich password: 123456
jane no password required
The test driver will demand a username. If the password attribute is
present (or if the username entered is wrong), then the password is
also asked for and checked:
$ virsh -c test://$(pwd)/testnode.xml list
Enter username for localhost: rich
Enter rich's password for localhost: ***
Id Name State
----------------------------------------------------
1 fv0 running
2 fc4 running
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
When checking for a valid interface, we weren't consistent on
whether we reported as VIR_FROM_NONE or VIR_FROM_INTERFACE.
Similar to previous patches, use a common macro to make it nicer.
Furthermore, just as in commit 6e130ddc, the difference between
VIR_IS_INTERFACE and VIR_IS_CONNECTED_INTERFACE is moot (due to
reference counting, any valid interface must be tied to a valid
connection). For now, we don't need virCheckInterfaceGoto().
* src/datatypes.h (virCheckInterfaceReturn): New macro.
(VIR_IS_INTERFACE, VIR_IS_CONNECTED_INTERFACE): Drop unused
macros.
* src/libvirt.c: Use macro throughout.
(virLibInterfaceError): Drop unused macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit cfd62c1 was incomplete; I found more cases where error
messages were being overwritten, and where the code between
the three registration/deregistration APIs was not consistent.
Since it is fairly easy to trigger an attempt to deregister an
unregistered object through public API, I also changed the error
message from VIR_ERR_INTERNAL_ERROR to VIR_ERR_INVALID_ARG.
* src/conf/object_event.c (virObjectEventCallbackListEventID):
Inline...
(virObjectEventStateEventID): ...into lone caller, and report
error on failure.
(virObjectEventCallbackListAddID, virObjectEventStateCallbackID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Tweak error category.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister):
Don't leak registration on failure.
(remoteConnectDomainEventDeregisterAny)
(remoteConnectNetworkEventDeregisterAny): Don't overwrite error.
Signed-off-by: Eric Blake <eblake@redhat.com>
When checking for a valid network, we weren't consistent on
whether we reported an invalid network or a connection. Similar
to previous patches such as commit 6e130ddc, the difference
between VIR_IS_NETWORK and VIR_IS_CONNECTED_NETWORK is moot (due
to reference counting, any valid network must be tied to a valid
connection). Use a common macro to make the error reporting
for invalid networks nicer.
* src/datatypes.h (virCheckNetworkReturn, virCheckNetworkGoto): New
macros.
(VIR_IS_NETWORK, VIR_IS_CONNECTED_NETWORK): Drop unused macros.
* src/libvirt.c: Use macro throughout.
(virLibNetworkError): Drop unused macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
Like commit 94a26c7e from Eric Blake, the old fuzzy code should
be replaced by the new array management macros now.
And the type of scsi->count should be changed into "size_t", and
thus virSCSIDeviceListCount should return size_t instead, similar
for vir{PCI,USB}DeviceListCount.
the unix socket /var/run/libvirt/lxc/domain.sock is not created
under the selinux context which configured by <seclabel>.
If we try to connect the domain.sock under the selinux context
of domain in virtLXCProcessConnectMonitor,selinux will deny
this connect operation.
type=AVC msg=audit(1387953696.067:662): avc: denied { connectto } for pid=21206 comm="libvirtd" path="/usr/local/var/run/libvirt/lxc/systemd.sock" scontext=unconfined_u:system_r:svirt_lxc_net_t:s0:c770,c848 tcontext=unconfined_u:system_r:unconfined_t:s0-s0:c0.c1023 tclass=unix_stream_socket
fix this problem by creating socket under selinux context of domain.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
The function checks for @conn to be valid and locks its mutex. Then, it
checks if callee is unregistering the same callback that he registered
previously. If this fails an error is reported and the control jumps to
'error' label. Here, if @conn has some errors (and it certainly does -
the one that's been just reported) the conn->mutex is locked again -
without any previous unlock:
Thread 1 (Thread 0x7fb500ef1800 (LWP 18982)):
#0 __lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
#1 0x00007fb4fd99ce56 in _L_lock_918 () from /lib64/libpthread.so.0
#2 0x00007fb4fd99ccaa in __GI___pthread_mutex_lock (mutex=0x7fb50153b670) at pthread_mutex_lock.c:64
#3 0x00007fb5007e574d in virMutexLock (m=m@entry=0x7fb50153b670) at util/virthreadpthread.c:85
#4 0x00007fb5007b198e in virDispatchError (conn=conn@entry=0x7fb50153b5e0) at util/virerror.c:594
#5 0x00007fb5008a3735 in virConnectUnregisterCloseCallback (conn=0x7fb50153b5e0, cb=cb@entry=0x7fb500f588e0 <vshCatchDisconnect>) at libvirt.c:21025
#6 0x00007fb500f5d690 in vshReconnect (ctl=ctl@entry=0x7fffff60e710) at virsh.c:328
#7 0x00007fb500f5dc50 in vshCommandRun (ctl=ctl@entry=0x7fffff60e710, cmd=0x7fb50152ca80) at virsh.c:1755
#8 0x00007fb500f5861b in main (argc=<optimized out>, argv=<optimized out>) at virsh.c:3393
And since the conn's mutex is not recursive, the virDispatchError will
never ever lock it successfully.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Cleanup after a previous patch, commit 6e130dd. In particular,
note that xenDomainUsedCpus can only be reached from
xenUnifiedDomainGetXMLDesc, which in turn is only reached from
public API that already validated the domain.
* src/xen/xen_driver.c (xenDomainUsedCpus): Drop redundant check.
* src/datatypes.h (VIR_IS_DOMAIN, VIR_IS_CONNECTED_DOMAIN):
Delete, and inline into all callers, since no other file uses it
any more.
Signed-off-by: Eric Blake <eblake@redhat.com>
In datatype.c, virGetDomainSnapshot could result in the message:
error: invalid domain pointer in bad domain
Furthermore, while there are a few functions in libvirt.c that
only care about a virDomainPtr without regards to the connection
(such as virDomainGetName), most functions also require a valid
connection. Yet several functions were blindly dereferencing
the conn member without checking it for validity first (such as
virDomainOpenConsole). Rather than try and correct all usage
of VIR_IS_DOMAIN vs. VIR_IS_CONNECTED_DOMAIN, it is easier to
just blindly require that a valid domain object always has a
valid connection object (which should be true anyways, since
every domain object holds a reference to its connection, so the
connection will not be closed until all domain objects have
also been closed to release their reference).
After this patch, all places that validate a domain consistently
report:
error: invalid domain pointer in someFunc
* src/datatypes.h (virCheckDomainReturn, virCheckDomainGoto): New
macros.
* src/datatypes.c (virGetDomainSnapshot): Use new macro.
(virLibConnError): Delete unused macro.
Signed-off-by: Eric Blake <eblake@redhat.com>
While comparing network and domain events, I noticed that the
test driver had to do a cast in one place and not the other.
For consistency, we should hide the necessary casting as low
as possible in the stack, with everything else using saner
types.
* src/conf/network_event.h (virNetworkEventStateRegisterID): Alter
type.
* src/conf/network_event.c (virNetworkEventStateRegisterID): Hoist
cast here.
* src/test/test_driver.c (testConnectNetworkEventRegisterAny):
Simplify callers.
* src/remote/remote_driver.c
(remoteConnectNetworkEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
If a user registers for a domain event filtered to a particular
domain, but the persistent domain is offline at the time, then
the code silently failed to set up the filter. As a result,
the event fires for all domains, rather than being filtered.
Network events were immune, since they always passed an id
0 argument.
The key to this patch is realizing that
virObjectEventDispatchMatchCallback() only cared about uuid;
so refusing to create a meta for a negative id is pointless,
and in fact, malloc'ing meta at all was overkill; instead,
just directly store a uuid and a flag of whether to filter.
Note that virObjectEventPtr still needs all fields of meta,
because this is how we reconstruct a virDomainPtr inside the
dispatch handler before calling the end user's callback
pointer with the correct object, even though only the uuid
portion of meta is used in deciding whether a callback
matches the given event. So while uuid is optional for
callbacks, it is mandatory for events.
The change to testDomainCreateXMLMixed is merely on the setup
scenario (as you can't register for a domain unless it is either
running or persistent). I actually first wrote that test for
this patch, then rebased it to also cover a prior patch (commit
4221d64), but had to adjust it for that patch to use Create
instead of Define for setting up the domain long enough to
register the event in order to work around this bug. But while
the setup is changed, the main body of the test is still about
whether creation events fire as expected.
* src/conf/object_event_private.h (_virObjectEventCallback):
Replace meta with uuid and flag.
(virObjectEventCallbackListAddID): Update signature.
* src/conf/object_event.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventNew): Document
use of name and uuid in events.
* src/conf/object_event.c (virObjectEventCallbackListAddID): Drop
arguments that don't affect filtering.
(virObjectEventCallbackListRemoveID)
(virObjectEventDispatchMatchCallback)
(virObjectEventStateRegisterID): Update clients.
* src/conf/domain_event.c (virDomainEventCallbackListAdd)
(virDomainEventStateRegisterID): Likewise.
* src/conf/network_event.c (virNetworkEventStateRegisterID):
Likewise.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Consider these two calls, in either order:
id1 = virConnectDomainEventRegisterAny(conn, NULL,
VIR_DOMAIN_EVENT_ID_LIFECYCLE,
VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
virConnectDomainEventRegister(conn, callback, NULL, NULL);
Right now, the second call fails, because under the hood, the
old-style function registration is tightly coupled to the
new style lifecycle eventID, and the two calls both try
to register the same global eventID callback representation.
We've alreay documented that users should avoid old-style
registration and deregistration, so anyone heeding the advice
won't run into this situation. But it would be even nicer if
we pretend the two interfaces are completely separate, and
disallow any cross-linking. That is, a call to old-style
deregister should never remove a new-style callback even if it
is the same function pointer, and a call to new-style callback
using only callbackIDs obtained legitimately should never
remove an old-style callback (of course, since our callback
IDs are sequential, and there is still coupling under the
hood, you can easily guess the callbackID of an old style
registration and use new-style deregistration to nuke it - but
that starts to be blatantly bad coding on your part rather
than a surprising result on what looks like reasonable
stand-alone API).
With this patch, you can now register a global lifecycle event
handler twice, by using both old and new APIs; if such an event
occurs, your callback will be entered twice. But that is not a
problem in practice, since it is already possible to use the
new API to register both a global and per-domain event handler
using the same function, which will likewise fire your callback
twice for that domain. Duplicates are still prevented when
using the same API with same parameters twice (old-style twice,
new-style global twice, or new-style per-domain with same domain
twice), and things are still bounded (it is not possible to
register a single function pointer more than N+2 times per event
id, where N is the number of domains available on the connection).
Besides, it has always been possible to register as many
separate function pointers on the same event id as desired,
through either old or new style API, where the bound there is
the physical limitation of writing a program with enough
distinct function pointers.
Adding another event registration in the testsuite is sufficient
to cover this, where the test fails without the rest of the patch.
* src/conf/object_event.c (_virObjectEventCallback): Add field.
(virObjectEventCallbackLookup): Add argument.
(virObjectEventCallbackListAddID, virObjectEventStateCallbackID):
Adjust callers.
* tests/objecteventtest.c (testDomainCreateXMLMixed): Enhance test.
Signed-off-by: Eric Blake <eblake@redhat.com>
On the surface, this sequence of API calls should succeed:
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
id2 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_RTC_CHANGE,...);
virConnectDomainEventDeregisterAny(id1);
id1 = virConnectDomainEventRegisterAny(..., VIR_DOMAIN_EVENT_ID_LIFECYCLE,...);
And for test:///default, it does. But for qemu:///system, it fails:
libvirt: XML-RPC error : internal error: domain event 0 already registered
Looking closer, the bug is caused by miscommunication between
the object event engine and the client side of the remote driver.
In our implementation, we set up a single server-side event per
eventID, then the client side replicates that one event to all
callbacks that have been registered client side. To know when
to turn the server side eventID on or off, the client side must
track how many events for the same eventID have been registered.
But while our code was filtering by eventID on event registration,
it did not filter on event deregistration. So the above API calls
resulted in the deregister returning 1 instead of 0, so no RPC
deregister was issued, and the final register detects on the
server side that the server is already handling eventID 0.
Unfortunately, since the problem is only observable on remote
connections, it's not possible to enhance objecteventtest to
expose the semantics using only public API entry points.
* src/conf/object_event.c (virObjectEventCallbackListCount): New
function.
(virObjectEventCallbackListAddID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Use it.
Signed-off-by: Eric Blake <eblake@redhat.com>
