There was inconsistency between alias used to create tls-creds-x509
object and alias used to link that object to chardev while hotpluging.
Hotplug ends with this error:
error: Failed to detach device from channel-tcp.xml
error: internal error: unable to execute QEMU command 'chardev-add':
No TLS credentials with id 'objcharchannel3_tls0'
In XML we have for example alias "serial0", but on qemu command line we
generate "charserial0".
The issue was that code, that creates QMP command to hotplug chardev
devices uses only the second alias "charserial0" and that alias is also
used to link the tls-creds-x509 object.
This patch unifies the aliases for tls-creds-x509 to be always generated
from "charserial0".
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
We need to make sure that the chardev is TCP. Without this check we
may access different part of union and corrupt pointers.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1366108
There are couple of things that needs to be done in order to
allow vhost-user hotplug. Firstly, vhost-user requires a chardev
which is connected to vhost-user bridge and through which qemu
communicates with the bridge (no acutal guest traffic is sent
through there, just some metadata). In order to generate proper
chardev alias, we must assign device alias way sooner.
Then, because we are plugging the chardev first, we need to do
the proper undo if something fails - that is remove netdev too.
We don't want anything to be left over in case attach fails at
some point.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Instead of blindly claim support for hot-plugging of every
interface type out there we should copy approach we have for
device types: white listing supported types and explicitly error
out on unsupported ones.
For instance, trying to hotplug vhostuser interface results in
nothing usable from guest currently. vhostuser typed interfaces
require additional work on our side.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The idea is to have function that does some checking at its
beginning and then have one big switch for all the interface
types it supports.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This function for some weird reason returns integer instead of
virDomainNetType type. It is important to return the correct type
so that we know what values we can expect.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Qemu always opens the tray if forced to. Skip the waiting step in such
case.
This also helps if qemu does not report the tray change event when
opening the cdrom forcibly (the documentation says that the event will
not be sent although qemu in fact does trigger it even if @force is
selceted).
This is a workaround for a qemu issue where qemu does not send the tray
change event in some cases (after migration with empty closed locked
drive) and thus renders the cdrom useless from libvirt's point of view.
Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1368368
If the incoming XML defined a path to a TLS X.509 certificate environment,
add the necessary 'tls-creds-x509' object to the VIR_DOMAIN_CHR_TYPE_TCP
character device.
Likewise, if the environment exists the hot unplug needs adjustment as
well. Note that all the return ret were changed to goto cleanup since
the cfg needs to be unref'd
Signed-off-by: John Ferlan <jferlan@redhat.com>
The linkstate setting of an <interface> is only meant to change the
online status reported to the guest system by the emulated network
device driver in qemu, but when support for auto-creating tap devices
for <interface type='ethernet'> was added in commit 9717d6, a chunk of
code was also added to qemuDomainChangeNetLinkState() that sets the
online status of the tap device (i.e. the *host* side of the
interface) for type='ethernet'. This was never done for tap devices
used in type='bridge' or type='network' interfaces, nor was it done in
the past for tap devices created by external scripts for
type='ethernet', so we shouldn't be doing it now.
This patch removes the bit of code in qemuDomainChangeNetLinkState()
that modifies online status of the tap device.
This patch removes the old vcpu unplug code completely and replaces it
with the new code using device_del. The old hotplug code basically never
worked with any recent qemu and thus is useless.
As the new code is using device_del all the implications of using it
are present. Contrary to the device deletion code, the vcpu deletion
code fails if the unplug request is not executed in time.
https://bugzilla.redhat.com/show_bug.cgi?id=1289391
Rather than pass the whole drive string (which contained the alias),
pass only the alias for the qemuMonitorDriveDel call in the error
path when adding a host device in the monitor fails.
Partial fix for:
https://bugzilla.redhat.com/show_bug.cgi?id=1336225
Similar to the other disk types, add the qemuMonitorDriveDel in the failure
to add/hotplug a USB.
Added a couple of other formatting changes just to have a less cluttered look
Since we already have a function that will generate the drivestr from
the alias, let's use it and remove the qemuDeviceDriveHostAlias.
Move the QEMU_DRIVE_HOST_PREFIX definition into qemu_alias.h
Also alter qemuAliasFromDisk to use the QEMU_DRIVE_HOST_PREFIX instead
of "drive-%s".
The current LUKS support has a "luks" volume type which has
a "luks" encryption format.
This partially makes sense if you consider the QEMU shorthand
syntax only requires you to specify a format=luks, and it'll
automagically uses "raw" as the next level driver. QEMU will
however let you override the "raw" with any other driver it
supports (vmdk, qcow, rbd, iscsi, etc, etc)
IOW the intention though is that the "luks" encryption format
is applied to all disk formats (whether raw, qcow2, rbd, gluster
or whatever). As such it doesn't make much sense for libvirt
to say the volume type is "luks" - we should be saying that it
is a "raw" file, but with "luks" encryption applied.
IOW, when creating a storage volume we should use this XML
<volume>
<name>demo.raw</name>
<capacity>5368709120</capacity>
<target>
<format type='raw'/>
<encryption format='luks'>
<secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/>
</encryption>
</target>
</volume>
and when configuring a guest disk we should use
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/home/berrange/VirtualMachines/demo.raw'/>
<target dev='sda' bus='scsi'/>
<encryption format='luks'>
<secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/>
</encryption>
</disk>
This commit thus removes the "luks" storage volume type added
in
commit 318ebb36f1
Author: John Ferlan <jferlan@redhat.com>
Date: Tue Jun 21 12:59:54 2016 -0400
util: Add 'luks' to the FileTypeInfo
The storage file probing code is modified so that it can probe
the actual encryption formats explicitly, rather than merely
probing existance of encryption and letting the storage driver
guess the format.
The rest of the code is then adapted to deal with
VIR_STORAGE_FILE_RAW w/ VIR_STORAGE_ENCRYPTION_FORMAT_LUKS
instead of just VIR_STORAGE_FILE_LUKS.
The commit mentioned above was included in libvirt v2.0.0.
So when querying volume XML this will be a change in behaviour
vs the 2.0.0 release - it'll report 'raw' instead of 'luks'
for the volume format, but still report 'luks' for encryption
format. I think this change is OK because the storage driver
did not include any support for creating volumes, nor starting
guets with luks volumes in v2.0.0 - that only since then.
Clearly if we change this we must do it before v2.1.0 though.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Dropping the caching of virtio serial address set.
Instead of using the cached address set, a function in qemu_hotplug.c
now recalculates it on demand.
Credit goes to Cole Robinson.
Since return code is checked globally at the end of the function, let's
make sure that we set it correctly at any point.
This fixes a regression introduced in commit 0aa19f35 where the first
command to eject changeable media would fail unconditionally.
Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1301021
Generate the luks command line using the AES secret key to encrypt the
luks secret. A luks secret object will be in addition to a an AES secret.
For hotplug, check if the encinfo exists and if so, add the AES secret
for the passphrase for the secret object used to decrypt the device.
Modify/augment the fakeSecret* in qemuxml2argvtest in order to handle
find a uuid or a volume usage with a specific path prefix in the XML
(corresponds to the already generated XML tests). Add error message
when the 'usageID' is not 'mycluster_myname'. Commit id '1d632c39'
altered the error message generation to rely on the errors from the
secret_driver (or it's faked replacement).
Add the .args output for adding the LUKS disk to the domain
Signed-off-by: John Ferlan <jferlan@redhat.com>
Soon we will be adding luks encryption support. Since a volume could require
both a luks secret and a secret to give to the server to use of the device,
alter the alias generation to create a slightly different alias so that
we don't have two objects with the same alias.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Commit id 'a1344f70a' added AES secret processing for RBD when starting
up a guest. As such, when the hotplug code calls qemuDomainSecretDiskPrepare
an AES secret could be added to the disk about to be hotplugged. If an AES
secret was added, then the hotplug code would need to generate the secret
object because qemuBuildDriveStr would add the "password-secret=" to the
returned 'driveStr' rather than the base64 encoded password.
Signed-off-by: John Ferlan <jferlan@redhat.com>
A recent adjustment to qemuDomainAttachRNGDevice to properly cleanup
the props object after a qemuMonitorAddObject also would affect this
code. Alter the cleanup to be similar to RNG changes.
Based on recent review comment - rather than have a spate of goto failxxxx,
change to a boolean based model. Ensures that the original error can be
preserved and cleanup is a bit more orderly if more objects are added.
Based on recent review comment - rather than have a spate of goto failxxxx,
change to a boolean based model. Ensures that the original error can be
preserved and cleanup is a bit more orderly if more objects are added.
Based on recent review comment - rather than have a spate of goto failxxxx,
change to a boolean based model. Ensures that the original error can be
preserved and cleanup is a bit more orderly if more objects are added.
Based on recent review comment - rather than have a spate of goto failxxxx,
change to a boolean based model. Ensures that the original error can be
preserved and cleanup is a bit more orderly if more objects are added.
Based on recent review comment - rather than have a spate of goto failxxxx,
change to a boolean based model. Ensures that the original error can be
preserved and cleanup is a bit more orderly if more objects are added.
Ensure that the given controller and all controllers with a smaller
index exist; there must not be any missing index in between.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
The commit "qemu: hot-plug: Assume support for -device in
qemuDomainAttachSCSIDisk" dropped the code for the automatic SCSI
controller creation used in SCSI disk hot-plugging. If we are
hot-plugging a SCSI disk to a domain and there is no proper SCSI
controller defined, it results in an "error: internal error: Could not
find scsi controller with index X required for device" error.
For that reason reverting a hunk of the commit
d4d32005d6.
This patch also adds an extra comment to the code to clarify the
loop.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
CVE-2016-5008
Setting an empty graphics password is documented as a way to disable
VNC/SPICE access, but QEMU does not always behaves like that. VNC would
happily accept the empty password. Let's enforce the behavior by setting
password expiration to "now".
https://bugzilla.redhat.com/show_bug.cgi?id=1180092
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When support for <interface type='ethernet'> was added in commit
9a4b705f back in 2010, it erroneously looked at <source dev='blah'/>
for a user-specified guest-side interface name. This was never
documented though. (that attribute already existed at the time in the
data.ethernet union member of virDomainNetDef, but apparently had no
practical use - it was only used as a storage place for a NetDef's
bridge name during qemuDomainXMLToNative(), but even then that was
never used for anything).
When support for similar guest-side device naming was added to the lxc
driver several years later, it was put in a new subelement <guest
dev='blah'/>.
In the intervening years, since there was no validation that
ethernet.dev was NULL in the other drivers that didn't actually use
it, innocent souls who were adding other features assuming they needed
to account for non-NULL ethernet.dev when really they didn't, so
little bits of the usual pointless cargo-cult code showed up.
This patch not only switches the openvz driver to use the documented
<guest dev='blah'/> notation for naming the guest-side device (just in
case anyone is still using the openvz driver), and logs an error if
anyone tries to set <source dev='blah'/> for a type='ethernet'
interface, it also removes the cargo-cult uses of ethernet.dev and
<source dev='blah'/>, and eliminates if from the RNG and from
virDomainNetDef.
NB: I decided on this course of action after mentioning the
inconsistency here:
https://www.redhat.com/archives/libvir-list/2016-May/msg02038.html
and getting encouragement do eliminate it in a later IRC discussion
with danpb.
Since commit 7140807917, qemu agent
channel cannot be plugged in because we won't generate its path
automatically. Let's not only fix that, but also add tests for it so
next time it's checked for.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1322210
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Hand-entering indexes for 20 PCI controllers is not as tedious as
manually determining and entering their PCI addresses, but it's still
annoying, and the algorithm for determining the proper index is
incredibly simple (in all cases except one) - just pick the lowest
unused index.
The one exception is USB2 controllers because multiple controllers in
the same group have the same index. For these we look to see if 1) the
most recently added USB controller is also a USB2 controller, and 2)
the group *that* controller belongs to doesn't yet have a controller
of the exact model we're just now adding - if both are true, the new
controller gets the same index, but in all other cases we just assign
the lowest unused index.
With this patch in place and combined with the automatic PCI address
assignment, we can define a PCIe switch with several ports like this:
<controller type='pci' model='pcie-root-port'/>
<controller type='pci' model='pcie-switch-upstream-port'/>
<controller type='pci' model='pcie-switch-downstream-port'/>
<controller type='pci' model='pcie-switch-downstream-port'/>
<controller type='pci' model='pcie-switch-downstream-port'/>
<controller type='pci' model='pcie-switch-downstream-port'/>
<controller type='pci' model='pcie-switch-downstream-port'/>
...
These will each get a unique index, and PCI addresses that connect
them together appropriately with no pesky numbers required.
