LXC from native: migrate veth network configuration
Some of the LXC configuration properties aren't migrated since they
would only cause problems in libvirt-lxc:
* lxc.network.ipv[46]: LXC driver doesn't setup IP address of guests,
see rhbz#1059624
* lxc.network.name, see rhbz#1059630
If no network configuration is provided, LXC only provides the loopback
interface. To match this, we need to use the privnet feature. LXC will
also define a 'none' network type in its 1.0.0 version that fits
libvirt LXC driver's default.
LXC rootfs can be either a directory or a block device or an image
file. The first two types have been implemented, but the image file is
still to be done since LXC auto-guesses the file format at mount time
and the LXC driver doesn't support the 'auto' format.
LXC driver: started implementing connectDomainXMLFromNative
This function aims at converting LXC configuration into a libvirt
domain XML description to help users migrate from LXC to libvirt.
Here is an example of how the lxc configuration works:
virsh -c lxc:/// domxml-from-native lxc-tools /var/lib/lxc/migrate_test/config
It is possible that some parts couldn't be properly mapped into a
domain XML fragment, so users should carefully review the result
before creating the domain.
fstab files in lxc.mount lines will need to be merged into the
configuration file as lxc.mount.entry.
As we can't know the amount of memory of the host, we have to set a
default value for max_balloon that users will probably want to adjust.
virConf now honours a VIR_CONF_FLAG_LXC_FORMAT flag to handle LXC
configuration files. The differences are that property names can
contain '.' character and values are all strings without any bounding
quotes.
Provide a new virConfWalk function calling a handler on all non-comment
values. This function will be used by the LXC conversion code to loop
over LXC configuration lines.
Eric Blake [Wed, 29 Jan 2014 00:41:34 +0000 (17:41 -0700)]
event: pass reason for PM events
Commit 57ddcc23 (v0.9.11) introduced the pmwakeup event, with
an optional 'reason' field reserved for possible future expansion.
But it failed to wire the field through RPC, so even if we do
add a reason in the future, we will be unable to get it back
to the user.
Worse, commit 7ba5defb (v1.0.0) repeated the same mistake with
the pmsuspend_disk event.
As long as we are adding new RPC calls, we might as well fix
the events to actually match the signature so that we don't have
to add yet another RPC in the future if we do decide to start
using the reason field.
* src/remote/remote_protocol.x
(remote_domain_event_callback_pmwakeup_msg)
(remote_domain_event_callback_pmsuspend_msg)
(remote_domain_event_callback_pmsuspend_disk_msg): Add reason
field.
* daemon/remote.c (remoteRelayDomainEventPMWakeup)
(remoteRelayDomainEventPMSuspend)
(remoteRelayDomainEventPMSuspendDisk): Pass reason to client.
* src/conf/domain_event.h (virDomainEventPMWakeupNewFromDom)
(virDomainEventPMSuspendNewFromDom)
(virDomainEventPMSuspendDiskNewFromDom): Require additional
parameter.
* src/conf/domain_event.c (virDomainEventPMClass): New class.
(virDomainEventPMDispose): New function.
(virDomainEventPMWakeupNew*, virDomainEventPMSuspendNew*)
(virDomainEventPMSuspendDiskNew*)
(virDomainEventDispatchDefaultFunc): Use new class.
* src/remote/remote_driver.c (remoteDomainBuildEvent*PM*): Pass
reason through.
* src/remote_protocol-structs: Regenerate.
Eric Blake [Thu, 9 Jan 2014 18:22:53 +0000 (11:22 -0700)]
event: convert remaining domain events to new style
Following the patterns established by lifecycle events, this
creates all the new RPC calls needed to pass callback IDs
for every domain event, and changes the limits in client and
server codes to use modern style when possible.
I've tested all combinations: both 'old client and new server'
and 'new client and old server' continue to work with the old
RPCs, and 'new client and new server' benefit from server-side
filtering with the new RPCs.
* src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_EVENT_*): Add
REMOTE_PROC_DOMAIN_EVENT_CALLBACK_* counterparts.
* daemon/remote.c (remoteRelayDomainEvent*): Send callbackID via
newer RPC when used with new-style registration.
(remoteDispatchConnectDomainEventCallbackRegisterAny): Extend to
cover all domain events.
* src/remote/remote_driver.c (remoteDomainBuildEvent*): Add new
Callback and Helper functions.
(remoteEvents): Match order of RPC numbers, register new handlers.
(remoteConnectDomainEventRegisterAny)
(remoteConnectDomainEventDeregisterAny): Extend to cover all
domain events.
* src/remote_protocol-structs: Regenerate.
Eric Blake [Wed, 8 Jan 2014 14:57:25 +0000 (07:57 -0700)]
event: client RPC protocol tweaks for domain lifecycle events
The counterpart to the server RPC additions; here, a single
function can serve both old and new calls, while incoming
events must be serviced by two different functions. Again,
some wise choices in our XDR made it easier to share code
managing similar events.
While this only supports lifecycle events, it covers the
harder part of how Register and RegisterAny interact; the
remaining 15 events will be a mechanical change in a later
patch. For Register, we now have a callbackID locally for
more efficient cleanup if the RPC fails; we also prefer to
use the newer RPC where we know it is supported (the older
RPC must be used if we don't know if RegisterAny is
supported).
* src/remote/remote_driver.c (remoteEvents): Register new RPC
event handler.
(remoteDomainBuildEventLifecycle): Move guts...
(remoteDomainBuildEventLifecycleHelper): ...here.
(remoteDomainBuildEventCallbackLifecycle): New function.
(remoteConnectDomainEventRegister)
(remoteConnectDomainEventDeregister)
(remoteConnectDomainEventRegisterAny)
(remoteConnectDomainEventDeregisterAny): Use new RPC when supported.
Eric Blake [Wed, 8 Jan 2014 04:00:54 +0000 (21:00 -0700)]
event: prepare client to track domain callbackID
We want to convert over to server-side events, even for older
APIs. To do that, the client side of the remote driver wants
to distinguish between legacy virConnectDomainEventRegister and
normal virConnectDomainEventRegisterAny, while knowing the
client callbackID and the server's serverID for both types of
registration. The client also needs to probe whether the
server supports server-side filtering. However, for ease of
review, we don't actually use the new RPCs until a later patch.
* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Add parameter.
* src/conf/object_event.c (virObjectEventCallbackListAddID)
(virObjectEventStateRegisterID): Separate legacy from callbackID.
(virObjectEventStateCallbackID): Pass through parameter.
(virObjectEventCallbackLookup): Let legacy and global domain
lifecycle events share a common remoteID.
* src/conf/network_event.c (virNetworkEventStateRegisterID):
Update caller.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateRegisterID, virDomainEventStateDeregister):
Likewise.
(virDomainEventStateRegisterClient)
(virDomainEventStateCallbackID): Implement new functions.
* src/conf/domain_event.h (virDomainEventStateRegisterClient)
(virDomainEventStateCallbackID): New prototypes.
* src/remote/remote_driver.c (private_data): Add field.
(doRemoteOpen): Probe server feature.
(remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Use new function.
Eric Blake [Wed, 8 Jan 2014 20:59:35 +0000 (13:59 -0700)]
event: server RPC protocol tweaks for domain lifecycle events
This patch adds some new RPC call numbers, but for ease of review,
they sit idle until a later patch adds the client counterpart to
drive the new RPCs. Also for ease of review, I limited this patch
to just the lifecycle event; although converting the remaining
15 domain events will be quite mechanical. On the server side,
we have to have a function per RPC call, largely with duplicated
bodies (the key difference being that we store in our callback
opaque pointer whether events should be fired with old or new
style); meanwhile, a single function can drive multiple RPC
messages. With a strategic choice of XDR struct layout, we can
make the event generation code for both styles fairly compact.
I debated about adding a tri-state witness variable per
connection (values 'unknown', 'legacy', 'modern'). It would start
as 'unknown', move to 'legacy' if any RPC call is made to a legacy
event call, and move to 'modern' if the feature probe is made;
then the event code could issue an error if the witness state is
incorrect (a legacy RPC call while in 'modern', a modern RPC call
while in 'unknown' or 'legacy', and a feature probe while in
'legacy' or 'modern'). But while it might prevent odd behavior
caused by protocol fuzzing, I don't see that it would prevent
any security holes, so I considered it bloat.
Note that sticking @acl markers on the new RPCs generates unused
functions in access/viraccessapicheck.c, because there is no new
API call that needs to use the new checks; however, having a
consistent .x file is worth the dead code.
* src/libvirt_internal.h (VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK):
New feature.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_CALLBACK_REGISTER_ANY)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_CALLBACK_DEREGISTER_ANY)
(REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE): New RPCs.
* daemon/remote.c (daemonClientCallback): Add field.
(remoteDispatchConnectDomainEventCallbackRegisterAny)
(remoteDispatchConnectDomainEventCallbackDeregisterAny): New
functions.
(remoteDispatchConnectDomainEventRegisterAny)
(remoteDispatchConnectDomainEventDeregisterAny): Mark legacy use.
(remoteRelayDomainEventLifecycle): Change message based on legacy
or new use.
(remoteDispatchConnectSupportsFeature): Advertise new feature.
* src/remote_protocol-structs: Regenerate.
This patch continues the earlier conversion made for network
events, with a goal of introducing server-side event filtering
in a later patch. Actual behavior is unchanged without
further RPC changes.
* daemon/libvirtd.h (daemonClientPrivate): Alter the tracking of
domain events.
* daemon/remote.c (remoteClientInitHook, remoteClientFreeFunc)
(remoteRelayDomainEvent*)
(remoteDispatchConnectDomainEventRegister)
(remoteDispatchConnectDomainEventRegisterAny): Track domain
callbacks dynamically.
Ján Tomko [Wed, 12 Feb 2014 13:54:05 +0000 (14:54 +0100)]
Revert "storage: disk: Separate creating of the volume from building"
This reverts commit 67ccf91bf29488783bd1fda46b362450f71a2078.
We only generate the volume key after we've built it, but the storage
driver expects it to be filled after createVol finishes.
Squash the volume building back with creating to fulfill this
expectation.
Ján Tomko [Wed, 12 Feb 2014 13:23:30 +0000 (14:23 +0100)]
Revert "storage: lvm: Separate creating of the volume from building"
This reverts commit af1fb38f55d4fb87e0fcaee1e973fa9c6713b1e6.
With it, creating new logical volumes fails:
https://www.redhat.com/archives/libvir-list/2014-February/msg00658.html
In the storage driver, we expect CreateVol to fill out the volume key,
but the LVM backend fills the key with the uuid reported by lvs after the
logical volume is created.
Cédric Bosdonnat [Wed, 12 Feb 2014 09:41:17 +0000 (10:41 +0100)]
AppArmor: Fix the place where the template should be installed
The security driver expects /etc/apparmor.d/libvirt/TEMPLATE but we
installed it to /etc/apparmor.d/libvirtd/TEMPLATE. Move the template to
the expected place since that code was here long before.
Oleg Strikov [Tue, 11 Feb 2014 15:51:41 +0000 (19:51 +0400)]
qemu: Implement a stub cpuArchDriver.baseline() handler for aarch64
Openstack Nova calls virConnectBaselineCPU() during initialization
of the instance to get a full list of CPU features.
This patch adds a stub to aarch64-specific code to handle
this request (no actual work is done). That's enough to have
this stub with limited functionality because qemu/kvm backend
supports only 'host-passthrough' cpu mode on aarch64.
Jim Fehlig [Thu, 6 Feb 2014 21:36:49 +0000 (14:36 -0700)]
libxl: register for domain events immediately after creation
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.
Ján Tomko [Tue, 11 Feb 2014 15:35:33 +0000 (16:35 +0100)]
Rename 'index' in virCapabilitiesGetCpusForNode
This shadows the index function on some systems (RHEL-6.4, FreeBSD 9):
../../src/conf/capabilities.c: In function 'virCapabilitiesGetCpusForNode':
../../src/conf/capabilities.c:1005: warning: declaration of'index'
shadows a global declaration [-Wshadow]
/usr/include/strings.h:57: warning: shadowed declaration is here [-Wshadow]
On some platforms like IBM PowerNV the NUMA node numbers can be
non-sequential. For eg. numactl --hardware o/p from such a machine looks
as given below
Add a new backend for any character device. This backend uses channel
in spice connection. This channel is similar to spicevmc, but
all-purpose in contrast to spicevmc.
Apart from spicevmc, spiceport-backed chardev will not be formatted
into the command-line if there is no spice to use (with test for that
as well). For this I moved the def->graphics counting to the start
of the function so its results can be used in rest of the code even in
the future.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This patch is here just to ease the code review and make related
changes look more sensible. Apart from removing the condition this is
merely a whitespace (indentation) change.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
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>
rbd: Use rbd_create3 to create RBD format 2 images by default
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>
Joel SIMOES [Tue, 11 Feb 2014 11:25:33 +0000 (11:25 +0000)]
Libvirt lose sheepdogs volumes on pool refresh or restart.
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
Laine Stump [Mon, 10 Feb 2014 14:08:26 +0000 (16:08 +0200)]
build: correctly check for SOICGIFVLAN GET_VLAN_VID_CMD command
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.
Michal Privoznik [Mon, 10 Feb 2014 18:26:16 +0000 (19:26 +0100)]
virNetworkLoadState: Disallow mangled 'floor' element
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>
Peter Krempa [Fri, 7 Feb 2014 17:42:27 +0000 (18:42 +0100)]
qemu: Use correct permissions when determining the image chain
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.
Michal Privoznik [Fri, 31 Jan 2014 14:36:13 +0000 (15:36 +0100)]
networkStartNetwork: Be more verbose
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>
Peter Krempa [Tue, 21 Jan 2014 17:50:12 +0000 (18:50 +0100)]
qemu: hyperv: Add support for timer enlightenments
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.
Peter Krempa [Wed, 5 Feb 2014 09:10:14 +0000 (10:10 +0100)]
conf: Enforce supported options for certain timers
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.
Peter Krempa [Tue, 4 Feb 2014 18:02:04 +0000 (19:02 +0100)]
schema: Fix guest timer specification schema according to the docs
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.
John Ferlan [Thu, 6 Feb 2014 17:30:57 +0000 (12:30 -0500)]
virpci: Resolve coverity issues
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.
qemu: keep pre-migration domain state after failed migration
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.
Jim Fehlig [Wed, 5 Feb 2014 19:50:52 +0000 (12:50 -0700)]
libxl: remove unneeded locking of driver when restoring
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.
Jim Fehlig [Thu, 16 Jan 2014 23:16:37 +0000 (23:16 +0000)]
libxl: improve subprocess handling
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
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.
Jim Fehlig [Sat, 1 Feb 2014 06:06:35 +0000 (23:06 -0700)]
libxl: handle domain shutdown events in a thread
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.
Jim Fehlig [Sat, 1 Feb 2014 01:09:53 +0000 (18:09 -0700)]
libxl: remove list of timer registrations from libxlDomainObjPrivate
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.
Jim Fehlig [Fri, 31 Jan 2014 23:57:09 +0000 (16:57 -0700)]
libxl: fix leaking libxlDomainObjPrivate
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.
qemu_driver: Introduce <filesystem/> support in device attach/detach
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.
With my recent work on the test, both time() and localtime() are used.
While mocking the former one, we get predictable result for UTC. But
since the latter function uses timezone to get local time, the result of
localtime() is not so predictive. Therefore, we must set the TZ variable
at the beginning of the test. To be able to catch some things that work
just by a blind chance, I'm choosing a virtual timezone that (hopefully)
no libvirt developer resides in.
qemuxml2argvmock: Mock time() on non-linux platforms too
The qemuxml2argvtest is run on more platforms than linux. For instance
FreeBSD. On these platforms we are, however, not mocking time() which
results in current time being fetched from system and hence tests number
32 and 33 failing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Jiri Denemark [Wed, 5 Feb 2014 10:15:31 +0000 (11:15 +0100)]
qemu: Fix crash in virDomainMemoryStats with old qemu
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.
Peter Krempa [Wed, 5 Feb 2014 17:40:39 +0000 (18:40 +0100)]
qemu: blockjob: Print correct file name in error message
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
Peter Krempa [Wed, 5 Feb 2014 17:39:23 +0000 (18:39 +0100)]
maint: Change the text of the NULLSTR() macro to "<null>"
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.
When trying to introduce a test for previous patch, I've
noticed that the command line is constructed using current
time. This won't work in our test suite (unless you guys
wants to set a specific time prior to each test run :) ).
Therefore we need to mock calls to time(2) to return the
same value every time it's called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Ján Tomko [Wed, 5 Feb 2014 17:34:51 +0000 (18:34 +0100)]
Generate a valid imagelabel even for type 'none'
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
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.
Eric Blake [Tue, 4 Feb 2014 19:37:15 +0000 (12:37 -0700)]
rpm: create libvirt-wireshark sub-package
On Fedora 20, with wireshark-devel installed, 'make rpm' failed
due to installed but unpackaged files related to wireshark. As
F20 is already released without wireshark, I chose to add a new
sub-package that is enabled only for F21 and later. Furthermore,
all existing wireshark plugins belong to the wireshark package,
so I got to invent behavior of how the first third-party wireshark
module will behave.
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.
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
John Ferlan [Wed, 29 Jan 2014 14:36:26 +0000 (09:36 -0500)]
utils: Introduce functions for kernel module manipulation
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
Laine Stump [Tue, 4 Feb 2014 11:12:47 +0000 (13:12 +0200)]
qemu: be sure we're using the updated value of backend during hotplug
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 project has historically operated as a meritocratic
consensus based community. Formally document what has
always been an unwritten assumption amongst the community
participants. Also include an explicit code of conduct
to preempt any potential, but unlikely, future problems.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Laine Stump [Fri, 31 Jan 2014 13:50:16 +0000 (15:50 +0200)]
network: change default of forwardPlainNames to 'yes'
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.
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:
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:
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.
I've received a notice over IRC that on some systems, the
virnetdevbandwidthtest is not linked with libxml:
/usr/bin/ld: virnetdevbandwidthtest.o: undefined reference to symbol 'xmlStrEqual@@LIBXML2_2.4.30'
/usr/lib/x86_64-linux-gnu/libxml2.so.2: error adding symbols: DSO missing from command line
Trivial way avoiding this is to add LIBXML_LIBS to
virnetdevbandwidthtest_LDADD.
spice: don't force user to specify spicevmc channel
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>
John Ferlan [Fri, 31 Jan 2014 16:32:28 +0000 (11:32 -0500)]
Resolve Coverity dead_error_begin
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.
Michal Privoznik [Fri, 31 Jan 2014 14:04:03 +0000 (15:04 +0100)]
virnetdevbandwidthtest: Introduce some more tests
And while doing this, fix one error raised by coverity. With
current code, @actual_cmd is allowed to be NULL for the whole
run of testVirNetDevBandwidthSet. However, if something else
was expected, the @actal_cmd is passed to virtTestDifference
which dereference it immediately.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Cédric Bosdonnat [Fri, 31 Jan 2014 08:52:30 +0000 (09:52 +0100)]
virnetdevbandwidthtest: fix hard coded /sbin/tc
On openSuse, (and possibly other distros), tc isn't located in
/sbin/tc. To get rid of that problem, use TC constant instead of hard
coded /sbin/tc in the expected string
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
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>
Remove windows thread implementation in favour of pthreads
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>
Fix pthread_sigmask check for mingw32 without winpthreads
On Fedora 19 and older the pthreads impl provided with
mingw does not have any pthread_sigmask impl at all. The
configure.ac check was not distinguishing this scenario
from that of a broken pthread_sigmask impl, so was
mistakenly enabling the libvirt workaround even when it
was not needed. This in turn conflicted with the gnulib
provided pthread_sigmask impl.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Osier Yang [Thu, 30 Jan 2014 11:48:23 +0000 (19:48 +0800)]
tests: Modify the scsi util tests
Add tests/virscsidata/sg0 and tests/virscsidata/sg8 as the test
input for constructing scsi->sg_path. And change the scsi generic
number of "1:0:0:0", because it's easy to hide the problem (assuming
most machines have a CDROM drive).
Osier Yang [Thu, 30 Jan 2014 11:48:22 +0000 (19:48 +0800)]
util: Accept test data path for scsi device's sg_path
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>
It breaks the build on RHEL-5.10 and because it's only optional we
could remove it from the code. The default namespace will be used.
This hunk was introduced by commit 237a088ba44633b08e0528fe5103d99b1e213b71.
Osier Yang [Thu, 30 Jan 2014 07:05:59 +0000 (15:05 +0800)]
util: Add one argument for several scsi utils
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.
Osier Yang [Wed, 29 Jan 2014 17:22:42 +0000 (01:22 +0800)]
qemu: Don't fail if the SCSI host device is shareable between domains
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.
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.
Michal Privoznik [Tue, 28 Jan 2014 18:18:43 +0000 (19:18 +0100)]
virCommand: Introduce virCommandSetDryRun
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>
Peter Krempa [Tue, 12 Nov 2013 13:15:51 +0000 (14:15 +0100)]
snapshot: Add support for specifying snapshot disk backing type
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.
Jim Fehlig [Wed, 29 Jan 2014 01:15:48 +0000 (18:15 -0700)]
xen: fix parsing xend http response
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.