Since a9331394 (first release v2.1.0), specifying a manual
security_driver setting in qemu.conf causes the daemon to fail to
start, erroring with 'Duplicate security driver X'.
The duplicate checking was incorrectly comparing every entry
against itself, guaranteeing a false positive.
https://bugzilla.redhat.com/show_bug.cgi?id=1365607
A nodedev device definition like this is required for testing
NodeDeviceCreateXML and NodeDeviceDestroy. So unless it's part
of the stock test:///default set there's no way to actually
invoke those functions for the default URI
Convert the individual XML documents into one big XML document
in the format expected by the non-default test://$PATH URI, and
use the same internal helpers for assembling the driver contents.
Every driver provides a refreshPool impl, and many other critical
places in the code unconditionally call it without checking if
it exists, so this check is pointless
This code was attempting to handle some implicit <console> XML
formatting for manually assembled DomainDef, since previously the
console<->serial compat copying was only done at XML parse time.
Nowadays it's done via virDomainDefPostParse ->
virDomainDefAddConsoleCompat, which all manual DomainDef builders
already call, so we can drop this workaround.
The other two DomainHasBlockJob usage error messages don't contain
'an', so unify things to save translators some effort. Dropping
the 'an' is closer to the sentence structure in the errors from
qemuDomainDiskBlockJobIsActive as well
Fedora now ships edk2 firmware in its official repos, so adapt
the nvram path list to match. Eventually we can remove the nightly
links as well once some integration kinks have been worked out,
and documentation updated.
Move the macro building into the %build target, which lets us
build up a shell variable and make things a bit more readable
https://bugzilla.redhat.com/show_bug.cgi?id=1335395
All qemu versions we support have QEMU_CAPS_DEVICE, so checking
for it is redundant. Remove the usage.
The code diff isn't clear, but all that code is just inindented
with no other change.
Test cases that hit qemuDomainAssignAddresses but don't have
infrastructure for specifying qemuCaps values see lots of
churn, since now PCI addresses are in the XML output.
Upcoming patches are going to make the disk portion of these
test cases fail. In order to make it work, we would need to
extend the qemuargv2xml test infrastructure to handle qemuCaps.
This is worthwhile to do at some point but isn't critical.
Instead just drop the offending portion, which isn't even the
target of the test cases anyways
hotplug APIs with the AFFECT_CONFIG flag are essentially replicating
'insert <device> into XML document, and redefine XML'. Thinking of
it this way, it's natural that we call virDomainDefPostParse after
manually editing the XML here.
Not only does doing so allow us to drop a bunch of open coded calls
to qemuDomainAssignAddresses, but it also means we are going through
the standard channels for XML validation and potentially catching
errors in user submitted XML.
This wires up qemuDomainAssignAddresses into the new
virDomainDefAssignAddressesCallback, so it's always triggered
via virDomainDefPostParse. We are essentially doing this already
with open coded calls sprinkled about.
qemu argv parse output changes slightly since previously it wasn't
hitting qemuDomainAssignAddresses.
It was only needed for rpm versions that are much older than our
minimally supported distro
Some more details here: https://fedorahosted.org/fpc/ticket/77
Requires adding the plumbing for <device><video>
The value is <enum name='modelType'> to match the associated domain
XML of <video><model type='XXX'/>
Wire it up for qemu too
This removes the opencoded payload freeing in the client, to use
the shared virNetMessageClearPayload call. Two changes:
- ClearPayload sets nfds=0, which fixes a potential crash if
an error path called virNetMessageFree/Clear on the message
after fds was free'd
- We drop the inner loop VIR_FORCE_CLOSE... this may mean fds are
kept open a little bit longer if the call is blocking but in
practice I don't think it will have any effect
This adds a ports= attribute to usb controller XML, like
<controller type='usb' model='nec-xhci' ports='8'/>
This maps to:
qemu -device nec-usb-xhci,p2=8,p3=8
Meaning, 8 ports that support both usb2 and usb3 devices. Gerd
suggested to just expose them as one knob.
https://bugzilla.redhat.com/show_bug.cgi?id=1271408
The iohelper dies on SIGPIPE if the stream is closed before all data
is processed. IMO this should be an error condition for virStreamFinish
according to docs like:
* This method is a synchronization point for all asynchronous
* errors, so if this returns a success code the application can
* be sure that all data has been successfully processed.
However for virStreamAbort, not so much:
* Request that the in progress data transfer be cancelled
* abnormally before the end of the stream has been reached.
* For output streams this can be used to inform the driver
* that the stream is being terminated early. For input
* streams this can be used to inform the driver that it
* should stop sending data.
Without this, virStreamAbort will realistically always error for
active streams like domain console. So, treat the SIGPIPE case
as non-fatal if abort is requested.
Note, this will only affect an explicit user requested abort. An
abnormal abort, like from a server error, always raises an error
in the daemon.
Every time a client aborts a stream via the virStreamAbort API,
the daemon always logs an error like:
error : daemonStreamHandleAbort:617 : stream aborted at client request
and that same error is returned to the client. Meaning virStreamAbort
always returns -1, which seems strange.
This reworks the error handling to only raise an error on virStreamAbort
if the actual server side abort call raises an error. This is similar
to how virStreamFinish works.
If the abort code path is triggered by an unexpected message type
then we continue to raise an unconditional error. Also drop a redundant
VIR_WARN call there, since virReportError will raise a VIR_ERROR anyways
These are the only places where we don't set stream->closed when
aborting the stream. This leads to spurious errors when the client
hangs up unexpectedly:
error : virFDStreamUpdateCallback:127 : internal error: stream is not open
Calling virStreamFinish prematurely seems to trigger this code path
even after the stream is closed, which ends up hitting this error
message later:
error : virFDStreamUpdateCallback:127 : internal error: stream is not open
Skip this function if stream->closed, which is used in many other places
like read/write handlers
This is the only place in daemon/stream.c that sets
'stream->closed = true' but neglects to actually abort the stream
and remove the callback, which seems wrong.
libvirt-daemon-config-nwfilter will put a bunch of xml configs
into /etc/libvirt/nwfilter. These configs don't hardcode a UUID
and depends on libvirt to generate one. However the generated UUID
is never saved to disk, unless the user manually calls Define.
This makes daemon reload quite noisy with many errors like:
error : virNWFilterObjAssignDef:3101 : operation failed: filter 'allow-incoming-ipv4' already exists with uuid 50def3b5-48d6-46a3-b005-cc22df4e5c5c
Because a new UUID is generated every time the config is read from
disk, so libvirt constantly thinks it's finding a new nwfilter.
Detect if we generated a UUID when the config file is loaded; if so,
resave the new contents to disk to ensure the UUID is persisteny.
This is similar to what was done in commit a47ae7c0 with virtual
networks and generated MAC addresses
In virNWFilterObjLoad we can still fail after virNWFilterObjAssignDef,
but we don't unlock and free the created virNWFilterObjPtr in the
cleanup path.
The bit we are trying to do after AssignDef is just STRDUP in the
configFile path. However caching the configFile in the NWFilterObj
is largely redundant and doesn't follow the same pattern we use
for domain and network objects.
So just remove all the configFile caching which fixes the latent
bug as a side effect.
We will segfault of a daemon reload picks up a new network config
that needs to be autostarted. We shouldn't be passing NULL for
network_driver here. This seems like it was missed in the larger
rework in commit 1009a61e
We historically format runtime seclabel selinux/apparmor values,
however we skip formatting runtime DAC values. This was added in
commit 990e46c454
Author: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
Date: Fri Aug 31 13:40:41 2012 +0200
conf: Avoid formatting auto-generated DAC labels
to maintain migration compatibility with libvirt < 0.10.0.
However the formatting was skipped unconditionally. Instead only
skip formatting in the VIR_DOMAIN_DEF_FORMAT_MIGRATABLE case.
https://bugzilla.redhat.com/show_bug.cgi?id=1215833
Trying to define a pool name containing an embedded '/'
will immediately fail when trying to write the XML to disk.
This patch explicitly rejects names containing a '/'
Besides our stateful driver, there are two other storage impls:
esx and phyp. esx doesn't support pool creation, so this should
doesn't apply.
phyp does support pool creation, and the name is passed to the
'mksp' tool, which google doesn't reveal whether it accepts '/'
or not. IMO the likeliness of this impacting any users is near zero
Trying to define a network name containing an embedded '/'
will immediately fail when trying to write the XML to disk.
This patch explicitly rejects names containing a '/'
Besides the network bridge driver, the only other network
implementation is a very thin one for virtualbox, which seems to
use the network name as a host interface name, which won't
accept '/' anyways, so I think this is fine to do unconitionally.
https://bugzilla.redhat.com/show_bug.cgi?id=787604
Trying to define a domain name containing an embedded '/'
will immediately fail when trying to write the XML to disk for
our stateful drivers. This patch explicitly rejects names
containing a '/', and provides an xmlopt feature for drivers
to avoid this validation check, which is enabled in every
non-stateful driver that already has xmlopt handling wired up.
(Technically this could reject a previously accepted vmname like
'/foo', however at least for the qemu driver that falls over
later when starting qemu)
https://bugzilla.redhat.com/show_bug.cgi?id=639923
Currently we only allow /dev/random and /dev/hwrng as host input
for <rng><backend model='random'/> device. This was added after
various upstream discussions in commit 4932ef45
However this restriction has generated quite a few complaints over
the years, so a new discussion was initiated:
http://www.redhat.com/archives/libvir-list/2016-April/msg00987.html
Several people suggested removing the restriction, and nobody really
spoke up to defend it. So this patch drops the path restriction
entirely
https://bugzilla.redhat.com/show_bug.cgi?id=1074464
If you compile a client --without-polkit, and connect to a URI that needs
polkit auth, the connection will fail with:
$ ./tools/virsh --connect qemu+ssh://crobinso@machine/system
error: failed to connect to the hypervisor
error: authentication failed: unsupported authentication type 2
This is because the client side portion of the polkit handling is
compiled out. However, nothing polkit specific is actually required
of the client.
Fix that error by unconditionally compiling the basic polkit client
handling.
https://bugzilla.redhat.com/show_bug.cgi?id=635529
If libvirt-daemon-config-network is installed while libvirtd is already
running, the daemon doesn't notice the network. Users then have to
manually restart libvirtd (or reboot) to pick up the network.
Instead let's trigger a daemon restart when the package is first installed.
Then the default network is available immediately if libvirtd was already
running.
https://bugzilla.redhat.com/show_bug.cgi?id=867546
This unifies the test scripts to all use the similar pattern added for
schematests in ace4aecd. This gives the following
- Enables running all tests from outside of tests/ dir
- Drops redundant abs_* definitions, which are set by test-lib.sh
- Drops unnecessary srcdir variable which was only used for sourcing
test-lib.sh
Behavior changes:
- srcdir can no longer be overwritten, but I don't know why anyone would
really need to...
- Script VERBOSE setting no longer prints commands executed by test-lib.sh.
if anyone cares I suggest handling this in test-lib.sh which already
has other verbose style handling
These old tests expect to run against a real xen connection via
xend running on the host. Our intentions for the test suite are
that it doesn't require interacting with any specific host resources,
so these don't really belong here.
Store the test list in libvirtd_test_scripts, and use it where
appropriate. This also fixes the fact that we didn't ship
virsh-uriprecedence when libvirtd build is disabled.
$ echo -n 'log_level=1' > ~/.config/libvirt/libvirtd.conf
$ libvirtd --timeout=10
2014-10-10 10:30:56.394+0000: 6626: info : libvirt version: 1.1.3.6, package: 1.fc20 (Fedora Project, 2014-09-08-17:50:42, buildvm-05.phx2.fedoraproject.org)
2014-10-10 10:30:56.394+0000: 6626: error : main:1261 : Can't load config file: configuration file syntax error: /home/rjones/.config/libvirt/libvirtd.conf:1: expecting a value: /home/rjones/.config/libvirt/libvirtd.conf
Rather than try to fix this in the depths of the parser, just catch
the case when a config file doesn't end in a newline, and manually
append a newline to the content before parsing
https://bugzilla.redhat.com/show_bug.cgi?id=1151409
virsh # list --all
Id Name State
----------------------------------------------------
1 test running
virsh # connect frob
error: Failed to connect to the hypervisor
error: no connection driver available for frob
virsh # list --all
error: failed to connect to the hypervisor
error: no valid connection
error: no connection driver available for frob
Seems sensible IMO to just not clear out the old connection state
until the new virConnectOpen succeeds.
https://bugzilla.redhat.com/show_bug.cgi?id=829160
My commit 0d1579572 crashes on a URI without a scheme, like via
'virsh --connect frob'
Add a check on uri->server too while we are at it, and centralize
them all
Remove all the plumbing needed for the different qcow-create/kvm-img
non-raw file creation.
We can drop the error messages because CreateQemuImg will thrown an
error for us but with slightly less fidelity (unable to find qemu-img),
which I think is acceptable given the unlikeliness of that error in
practice.
This an ubuntu/debian packaging convention. At one point it may have
been an actually different binary, but at least as of ubuntu precise
(the oldest supported ubuntu distro, released april 2012) kvm-img is
just a symlink to qemu-img for back compat.
I think it's safe to drop support for it
qcow-create was a crippled qemu-img impl that shipped with xen. I
think supporting this was only relevant for really old distros
that didn't have a proper qemu package, like early RHEL5. I think
it's fair to drop support
VIR_ERR_NO_SUPPORT maps to the error string
this function is not supported by the connection driver
and is largely only used for when a driver doesn't have any
implementation for a public API. So its usage with invalid
net-update requests is a bit out of place. Instead use
VIR_ERR_OPERATION_UNSUPPORTED which maps to:
Operation not supported
And is what qemu's hotplug routines use in similar scenarios
This reverts commit 1e9808d3a1.
We shouldn't advertise libvirtd.socket activation, since currently
it means VM/network/... autostart won't work as expected.
We tried to find a middle ground by installing the config file without
an [Install] section, since systemd won't allow .socket to be enabled
without one... or at least it did do that; presently on f24 it allows
activating the socket quite happily. This also caused user confusion[1]
Just remove the socket file. I've filed a new RFE to track coming up
with a solution to the autostart problem[2], we can point users at that
if there's more confusion:
[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1279348
[2]: https://bugzilla.redhat.com/show_bug.cgi?id=1326136
The common idiom in the driver API implementations is roughly:
- ACL check
- BeginJob (if needed)
- AgentAvailable (if needed)
- !IsActive
A few calls had an extra !IsActive before BeginJob, which doesn't
seem to serve much use. Drop them
It isn't implemented and does not work:
error: internal error: guest failed to start: /usr/lib/libvirt/libvirt_lxc: option '--veth' requires an argument
syntax: /usr/lib/libvirt/libvirt_lxc [OPTIONS] ...
We previously threw an explicit error, but this changed in
22cff52a2b , which I suspect was
untested for LXC
Move some API specific documentation out of -docs package and into
-devel, and some end user docs out of -devel and into -docs, then
drop the -devel dep on -docs. This is more in line with the suggested
Fedora guidelines.
https://bugzilla.redhat.com/show_bug.cgi?id=1310155
The only caller of this code is:
for (i = 0; i < dom->def->ngraphics; i++) {
if (dom->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
if (!(mig->graphics =
qemuMigrationCookieGraphicsAlloc(driver, dom->def->graphics[i])))
return -1;
mig->flags |= QEMU_MIGRATION_COOKIE_GRAPHICS;
break;
}
}
So this is never triggered for VNC, and in fact VNC has no support for
seamless migration anyways so that seems correct. Drop the dead VNC
handling.
We use device-mapper to enumerate all dm devices, and filter out
the list of multipath devices by checking the target_type string
name. The code however cancels all scanning if we encounter
target_type=NULL
I don't know how to reproduce that situation, but a user was hitting
it in their setup, and inspecting the lvm2/device-mapper code shows
many places where !target_type is explicitly ignored and processing
continues on to the next device. So I think we should do the same
https://bugzilla.redhat.com/show_bug.cgi?id=1069317
Cole Robinson