There are separate Detach functions for PCI, USB, SCSI, Vhost, and
Mediated hostdevs, but the functions are all 100% the same code,
except that the PCI function checks for the guest side of the device
being a PCI Multifunction device, while the other 4 check that the
device's alias != NULL.
The check for multifunction PCI devices should be done for *all*
devices that are connected to the PCI bus in the guest, not just PCI
hostdevs, and qemuIsMultiFunctionDevice() conveniently returns false
if the queried device doesn't connect with PCI, so it is safe to make
this check for all hostdev devices. (It also needs to be done for many
other device types, but that will be addressed in a future patch).
Likewise, since all hostdevs are detached by calling
qemuDomainDeleteDevice(), which requires the device's alias, checking
for a valid alias is a reasonable thing for PCI hostdevs too.
Signed-off-by: Laine Stump <laine@laine.org>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Having an InfoPtr named "dev" made my brain hurt. Renaming it to
"info" gives one less thing to confuse when looking at the code.
Signed-off-by: Laine Stump <laine@laine.org>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
When support for hotplug/unplug of SCSI controllers was added way back
in December 2009 (commit da9d937b), unplug was handled by calling the
now-extinct function qemuMonitorRemovePCIDevice(), which required a
PCI address as an argument. At the same time, the idea of every device
in the config having a PCI address apparently was not yet fully
implemented, because the author of the patch including a check for a
valid PCI address in the device object.
These days, all PCI devices are guaranteed to have a valid PCI
address. But more important than that, we no longer detach devices by
PCI address, but instead use qemuDomainDeleteDevice(), which
identifies the device by its alias. So checking for a valid PCI
address is just pointless extra code that obscures the high level of
similarity between all the individual qemuDomainDetach*Device()
functions.
Signed-off-by: Laine Stump <laine@laine.org>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuDomainRemoveRNGDevice() calls qemuDomainDetachExtensionDevice().
According to commit 1d1e264f1 that added this code, it should not be
necessary to explicitly remove the zPCI extension device for a PCI
device during unplug, because "QEMU implements an unplug callback
which will unplug both PCI and zPCI device in a cascaded way". In
fact, no other devices call qemuDomainDetachExtensionDevice() during
their qemuDomainRemove*Device() function, so it should be removed from
qemuDomainRemoveRNGDevice as well.
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
qemuDomainDetachControllerDevice() calls
qemuDomainDetachExtensionDevice() when the controller type is
PCI. This is incorrect in multiple ways:
* Any code that tears down a device should be in the
qemuDomainRemove*Device() function (which is called after libvirt
gets a DEVICE_DELETED event from qemu indicating that the guest is
finished with the device on its end. The qemuDomainDetach*Device()
functions should only contain code that ensures the requested
operation is valid, and sends the command to qemu to initiate the
unplug.
* qemuDomainDetachExtensionDevice() is a function that applies to
devices that plug into a PCI slot, *not* necessarily PCI controllers
(which is what's being checked in the offending code). The proper
way to check for this would be to see if the DeviceInfo for the
controller device had a PCI address, not to check if the controller
is a PCI controller (the code being removed was doing the latter).
* According to commit 1d1e264f1 that added this code (and other
support for hotplugging zPCI devices on s390), it's not necessary to
explicitly detach the zPCI device when unplugging a PCI device. To
quote:
There's no need to implement hot unplug for zPCI as QEMU
implements an unplug callback which will unplug both PCI and
zPCI device in a cascaded way.
and the evidence bears this out - all the other uses of
qemuDomainDetachExtensionDevice() (except one, which I believe is
also in error, and is being removed in a separate patch) are only to
remove the zPCI extension device in cases where it was successfully
added, but there was some other failure later in the hotplug process
(so there was no regular PCI device to remove and trigger removal of
the zPCI extension device).
* PCI controllers are not hot pluggable, so this is dead code
anyway. (The only controllers that can currently be
hotplugged/unplugged are SCSI controllers).
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
The functions do basically exactly the same thing modulo few checks.
In case of virtio disks we check that the device is not multifunction as
that can't be unplugged at once. In case of USB and SCSI disks we
checked that no active block job is running.
The check for running blockjobs should have also been done for virtio
disks. By moving the multifunction check into the common function we fix
this case and also simplify the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the correct type in switch and populate the missing cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We don't have any cleanup section, we can return the value directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1623389
If a device is detached twice from the same domain the following
race condition may happen:
1) The first DetachDevice() call will issue "device_del" on qemu
monitor, but since the DEVICE_DELETED event did not arrive in
time, the API ends claiming "Device detach request sent
successfully".
2) The second DetachDevice() therefore still find the device in
the domain and thus proceeds to detaching it again. It calls
EnterMonitor() and qemuMonitorSend() trying to issue "device_del"
command again. This gets both domain lock and monitor lock
released.
3) At this point, qemu sends us the DEVICE_DELETED event which is
going to be handled by the event loop which ends up calling
qemuDomainSignalDeviceRemoval() to determine who is going to
remove the device from domain definition. Whether it is the
caller that marked the device for removal or whether it is going
to be the event processing thread.
4) Because the device was marked for removal,
qemuDomainSignalDeviceRemoval() returns true, which means the
event is to be processed by the thread that has marked the device
for removal (and is currently still trying to issue "device_del"
command)
5) The thread finally issues the "device_del" command, which
fails (obviously) and therefore it calls
qemuDomainResetDeviceRemoval() to reset the device marking and
quits immediately after, NOT removing any device from the domain
definition.
At this point, the device is still present in the domain
definition but doesn't exist in qemu anymore. Worse, there is no
way to remove it from the domain definition.
Solution is to note down that we've seen the event and if the
second "device_del" fails, not take it as a failure but carry on
with the usual execution.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
The aim of this function will be to fix return value of
qemuMonitorDelDevice() in one specific case. But that is yet to
come. Right now this is nothing but a plain substitution.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Luckily, the function returns only 0 or -1 so all the checks work
as expected. Anyway, our rule is that a positive value means
success so if the function ever returns a positive value these
checks will fail. Make them check for a negative value properly.
At the same time fix qemuDomainDetachExtensionDevice() reval
check. It is somewhat related to the aim of this patch.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In these cases the check that is removed has been done a few
lines above already (as can even be seen in the context). Drop
them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Now that virStorageSource is a subclass of virObject we can use
virObjectUnref and remove virStorageSourceFree which was a thin wrapper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
My change in 112f3a8d0f was too drastic. The @charAlias
variable is initialized only if @monitor == true. However, it is
used even outside of that condition, at which point it's just
uninitialized pointer.
Reported-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The @tmpChr is looked up in domain definition based on user
provided chardev XML. Therefore, the alias must have been
allocated already when domain was started up.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This is basically an old artefact from 24b0821926 when the idea
was:
1) Build device string only to see if chardev has any -device
associated with it and thus if device_del is needed
2) Detach chardev using chardev_del
Now, that DEVICE and DEVICE_DELETED capabilities are assumed for
every domain 1) does not make sense anymore.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1624204
The guestfwd channels are -netdevs really. Hotunplug them as
such. Also, DEVICE_DELETED event is not triggered (surprisingly,
since we're not issuing device_del rather than netdev_del) and
associated chardev is removed automagically too. This means that
we need to do qemuDomainRemoveChrDevice() minus monitor call to
remove the chardev.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1624204
The guestfwd channels are -netdevs really. Hotplug them as such.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
So far we are passing @chr to qemuBuildChrDeviceStr. This is
suboptimal (in fact wrong) because @chr is just parsed XML
definition provided by user which by definition may lack some
information. On the other hand, @tmpChr is the one that was found
using @chr in domain definition so it contains the same amount of
information or more.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Rather than passing in a virStorageSource which would override the
originally passed disk->src we can now drop passing in a disk completely
as all functions called inside here require a virStorageSource.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Use the functions designed to deal with single images as the *Disk
functions were just wrappers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The same can be achieved by using qemuSecurity[Set|Restore]ImageLabel.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Since the disk is necessary only to get the source modify the functions
to take the source directly and rename them to
qemu[Setup|Teardown]ImageChainCgroup.
Additionally drop a pointless comment containing the old function name.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When we need to detect a chain for a image which will become the new
source for a disk (e.g. after a disk media change or a blockjob) we'd
need to replace disk->src temporarily to do so.
Move the 'disksrc' temporary variable to an argument and adjust callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Now that it's no longer needed, remove the argument.
This removes the last helper variable in
qemuBuildControllerDevCommandLine.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
This will be extended in the future, so let's simplify things by
centralizing the checks.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
When commit 361c8dc17 added support for hotplugging the i6300esb
watchdog device (first in libvirt-3.9.0), it accidentally contstructed
the commandline for the device_add command before allocating a PCI
address for the device. With no PCI address specified in the command,
the watchdog would simply be placed at the lowest unused PCI slot.
On a 440fx guest, this doesn't cause a problem, because libvirt's PCI
address allocation algorithm would most likely give the same address
anyway (usually a slot on pci-root), so nobody noticed the omission of
address from the command.
But on a Q35 guest, the lowest unused PCI slot is on pcie-root, which
doesn't support hotplug; libvirt knows enough to assign a PCI address
that is on a pcie-to-pci-bridge (because its slots *do* support
hotplug), but qemu doesn't, so if there is no PCI address in the
command, qemu just tries to plug the new device into pcie-root, and
fails because it doesn't support hotplug, e.g.:
error: Failed to attach device from watchdog.xml
error: internal error: unable to execute QEMU command 'device_add':
Bus 'pcie.0' does not support hotplugging
The solution is simply to build the command string after assigning a
PCI address, not before.
Resolves: https://bugzilla.redhat.com/1666559
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If code in the @actualType switch needs to have/know which PCI
Address is being used, then we must assign it earlier. In particular
a vhost-user device needs to call qemuDomainSupportsNicdev which
requires an address to be defined.
Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn>
Reviewed-by: John Ferlan <jferlan@redhat.com>
According to the result parsing from xml, add the unarmed property
into QEMU command line:
-device nvdimm,...[,unarmed=on]
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Require that all headers are guarded by a symbol named
LIBVIRT_$FILENAME
where $FILENAME is the uppercased filename, with all characters
outside a-z changed into '_'.
Note we do not use a leading __ because that is technically a
namespace reserved for the toolchain.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This introduces a syntax-check script that validates header files use a
common layout:
/*
...copyright header...
*/
<one blank line>
#ifndef SYMBOL
# define SYMBOL
....content....
#endif /* SYMBOL */
For any file ending priv.h, before the #ifndef, we will require a
guard to prevent bogus imports:
#ifndef SYMBOL_ALLOW
# error ....
#endif /* SYMBOL_ALLOW */
<one blank line>
The many mistakes this script identifies are then fixed.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In many files there are header comments that contain an Author:
statement, supposedly reflecting who originally wrote the code.
In a large collaborative project like libvirt, any non-trivial
file will have been modified by a large number of different
contributors. IOW, the Author: comments are quickly out of date,
omitting people who have made significant contribitions.
In some places Author: lines have been added despite the person
merely being responsible for creating the file by moving existing
code out of another file. IOW, the Author: lines give an incorrect
record of authorship.
With this all in mind, the comments are useless as a means to identify
who to talk to about code in a particular file. Contributors will always
be better off using 'git log' and 'git blame' if they need to find the
author of a particular bit of code.
This commit thus deletes all Author: comments from the source and adds
a rule to prevent them reappearing.
The Copyright headers are similarly misleading and inaccurate, however,
we cannot delete these as they have legal meaning, despite being largely
inaccurate. In addition only the copyright holder is permitted to change
their respective copyright statement.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
So far we have two arguments that we are passing to
qemuBuildMemoryBackendProps() and that are taken from domain
private data: @qemuCaps and @autoNodeset. In the next commit I
will use one more item from there. Therefore, instead of having
it as yet another argument to the function, pass pointer to the
private data object.
There is one change in qemuDomainAttachMemory() where previously
@autoNodeset was NULL but now is priv->autoNodeset (which may be
set). This is safe to do as @autoNodeset is advisory only.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1656014
An RNG device can consists of more devices than RND device
itself. For instance, in case of EGD there is a chardev that
connects to EGD daemon and feeds the qemu with random data. When
doing RNG device removal we have to remove the associated chardev
as well.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit adds hotplug support for PCI devices on S390 guests.
There's no need to implement hot unplug for zPCI as QEMU implements
an unplug callback which will unplug both PCI and zPCI device in a
cascaded way.
Currently, the following PCI devices are supported:
virtio-blk-pci
virtio-net-pci
virtio-rng-pci
virtio-input-host-pci
virtio-keyboard-pci
virtio-mouse-pci
virtio-tablet-pci
vfio-pci
SCSIVhost device
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
We now explicitly handle media change elsewhere so we can drop the
switch statement. This will also make it more intuitive once CDROM
device hotplug might be supported.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Disk hotplug has slightly different semantics from media changing. Move
the media change code out and add proper initialization of the new
source object and proper cleanups if something fails.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
The disk hotplug code also overloads media change which is not ideal.
This will allow splitting out of the media change code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
The disk storage source needs to be prepared if we want to use -blockdev
or secrets for the new media image. It does not hurt to do the same for
the legacy hotplug code as well.
Unfortunately helpers like qemuDomainPrepareDiskSource take
virDomainDiskDef as an argument and it would be hard to fix them to take
an explicit source, so the function also temporarily replaces disk->src
for the new source in this function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Some functions require us to replace disk->src with the new source for
them to work properly. To avoid confusion all places which allow
explicit virStorageSource should get the appropriate definition.
The legacy code fortunately does not need anything from the old source
so that does not require modifications.
Blockdev does require the old definition so we'll pass it explicitly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Since the code is also used when changing media we need to allow
specifying explicit source for which we are going to prepare. With this
change callers don't have to replace disk->src with the new source
definition for generating these.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
qemu media changing code tried to assume old media's format for the new
one if that was not specified. Since the format will always be present
it does not make sense to keep the code around.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Old media changing code does not bother setting up the secrets for new
media or actually removing/adding of the corresponding objects.
Additionally it uses secrets setup for the old image to be removed as
the secret for the new image which is wrong.
Remove the support for secrets while changing media for the legacy
approach. The only reasonable way to fix it is when using blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
While the idea was good the implementation not so much as we need to
take into account the old disk data and the new source. The code will be
consolidated later in a different way.
This reverts commit 663b1d55de.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Preparing the storage source prior to assigning the alias will not work
as the names of the certain objects depend on the alias for the legacy
hotplug case as we generate the object names for the secrets based on
the alias.
This reverts commit 192fdaa614.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
It was already available in 1.5.0, so we can assume it's
present and avoid checking for it at runtime.
This commit is best viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1630164
Since 2a13a0a103 we are querying the vhostuser's interface name
when building qemu command line. However, we forgot to do so on
hotplug.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
At the time of the addition of 'pci-assign' in QEMU commit
v1.3.0-rc0~572^2 the configfd argument was already supported.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Commit f7b5566 added 'save_error' even though the function
already has 'originalError' used in the 'try_remove' section.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This patch ensures that changes in attributes of interfaces will emit
errors except if they are missing from the XML.
Previously we were falsely reporting successful updates, because some
changed attributes got overwritten before the validity checks.
https://bugzilla.redhat.com/show_bug.cgi?id=1599513
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuDomainDiskGetBackendAlias allocates a copy of the nodename string so
we need to free it at the end.
Found by Coverity.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
qemuDomainDetachControllerDevice contained code which implied that alias
might be NULL when detaching the disk and tried to generate it. This is
no longer possible so we can remove the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Use the new APIs which allow to manipulate the tray and media separately
and also allow using a nodename to refer to a media to implement media
changing.
With the new approach we don't have to call eject twice as the media is
removed by calling qemuMonitorBlockdevMediumRemove.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Initialize data for the whole backing chain when plugging in or removing
disks when a machine supports -blockdev.
Similarly to startup we need to prepare the structures for the whole
backing chain and take care of the copy-on-read feature.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When we stop using -drive qemu stops reporting it in some of the monitor
commands. To allow referring the disk frontends and the corresponding
block backends we need to know these names. Unfortunately different
buses require different names.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It should be impossible to lack an alias in the domain definition. Other
disk types don't generate it so remove it here as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the preparation steps from qemuDomainAttachDiskGeneric up into
qemuDomainAttachDeviceDiskLive so that also media changing can use the
prepared file.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use qemuDomainAttachDeviceDiskLive to change the media in
qemuDomainChangeDiskLive as the former function already does all the
necessary steps to prepare the new medium.
This also allows us to turn qemuDomainChangeEjectableMedia static.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
commit 5c81c342a7 forgot to skip the detaching of the shmem backend
when async unplug is requested which meant that we've tried to unplug
the backend prior to delivery of the DEVICE_DELETED event.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1618622
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Now that the argument is unused we can remove it transitively from all
the call graphs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If we'd fail to enter or exit the monitor the saved error would be
leaked. Introduced in 8498a1e222 .
Pointed out by coverity.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In some cases backing chain needs to be cleared prior to re-detection.
Move this step out of qemuDomainDetermineDiskChain as only certain
places need it and the function itself is able to skip to the end of the
chain to perform detection.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When changing cdrom media we did not handle the managed PR objects thus
we'd either have a stale PR object left behind or the media change would
fail.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the issue of the monitor command to the caller so that the
function can be used with the modern approach.
Additionally improve the error message.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Prepare for the -blockdev implementation of ejectable media changing by
splitting up the old bits.
Additionally since both callers make sure that the device is a cdrom or
floppy the check is no longer necessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to qemuDomainDiskRemoveManagedPR make it enter monitor on
its own so that it can be reused. Future users will be in the snapshot
code and in removable media change code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract the (possible) removal of the PR backend and daemon into a
separate helper which enters monitor on its own. This simplifies the
code and allows reuse of this function in the future e.g. for blockjobs
where removing a image with PR may result into PR not being necessary.
Since the PR is not used often the overhead of entering monitor again
should be negligible.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add code which will convert a disk definition into
qemuHotplugDiskSourceData and then reuse qemuHotplugDiskSourceRemove to
remove all the backend related objects.
This unifies the detach code as much as possible with the already
existing helpers and will allow reuse this infrastructure when changing
removable disk media.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuDomainRemoveDiskDevice would leak the disk to be removed if the VM
crashed since it was removed from the definition but not freed.
Broken in commit 105bcdde76 which moved the removal from the definition
earlier.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to how we've intergrated data belonging to a single
virStorageSource for purposes of attaching it to a qemu instance we will
need to agregate data relevant for the whole disk. With blockdev there
will be some disk-wide backing chain members such as the copy-on-read
handler.
Introduce qemuHotplugDiskSourceData which agregates the backing chain
and other data relevant for the disk and functions which generate it
and apply and rollback it.
In addition to disk hotplug this will also be reused for media changing
where we need to exchange the full disk backend.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The warning messages which include the disk source could potentially
format NULL using %s as virDomainDiskGetSource may return NULL for e.g.
NBD disks. As most of the APIs are NOOP for remote disks the usage of
the source string only should be fine for now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that there's only one use of it, replace it directly by the code
filling it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuHotplugPrepareDiskAccess can be used to tear down disk access so we
can replace the open-coded version collecting the same function calls.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since 2.10 QEMU supports a new display type egl-headless which uses the
drm nodes for OpenGL rendering copying back the rendered bits back to
QEMU into a dma-buf which can be accessed by standard "display" apps
like VNC or SPICE. Although this display type can be used on its own,
for any practical use case it makes sense to pair it with either VNC or
SPICE display. The clear benefit of this display is that VNC gains
OpenGL support, which it natively doesn't have, and SPICE gains remote
OpenGL support (native OpenGL support only works locally through a UNIX
socket, i.e. listen type=socket/none).
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Since commit f14c37, virDomainConfVMNWFilterTeardown is reporting errors
thus any previously reported error gets overwritten.
We need to save the errors in qemuDomainAttachNetDevice before calling
this function when we are in cleanup code.
https://bugzilla.redhat.com/show_bug.cgi?id=1598311
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The check whether the object holding secret for decryption of the TLS
environment was wrong and would always attempt to add the object. This
lead to a crash due to recent refactors.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1598015
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It builds the string for '-device' from a virDomainDiskDef.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
SD card hotplug should not be implemented until they can be used via
-blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
from src/qemu/qemu_domain_address.c to src/conf/domain_addr.c
and rename to virDomainCCWAddressSetCreateFromDomain
(rename to have Address in full instead of Addr to follow
the naming convention of other virDomainCCWAddress functions)
Signed-off-by: Anya Harter <aharter@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The private data of a virStorageSource which is backing an iSCSI hostdev
may be NULL if no authentication is present. The code handling the
hotplug would attempt to extract the authentication info stored in
'secinfo' without checking if it is allocated which resulted in a crash.
Here we opt the easy way to check if srcPriv is not NULL so that we
don't duplicate all the logic which selects whether the disk source has
a secret.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1597550
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1585108
When updating a live device users might pass different alias than
the one the device has. Currently, this is silently ignored which
goes against our behaviour for other parts of the device where we
explicitly allow only certain changes and error out loudly on
anything else.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Mediated devices of model 'vfio-ccw' are using CCW addresses, so make
sure to call the correct address preparation code for the model.
Reviewed-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Remove the callbacks that the nwfilter driver registers with the domain
object config layer. Instead make the current helper methods call into
the public API for creating/deleting nwfilter bindings.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
VMs with hardcoded platform network devices are forced to use old
style '-net nic' command line config. Current we use qemu's vlan
option to hook this with the '-netdev' host side of things.
However since qemu 1.2 there is '-net nic,netdev=X' option for
explicitly referencing a netdev ID, which is more inline with
typical VM commandlines, so let's switch to that
Signed-off-by: Cole Robinson <crobinso@redhat.com>
And replace all calls with virObjectEventStateQueue such that:
qemuDomainEventQueue(driver, event);
becomes:
virObjectEventStateQueue(driver->domainEventState, event);
And remove NULL checking from all callers.
Signed-off-by: Anya Harter <aharter@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Replace instances where we previously called virGetLastError just to
either get the code or to check if an error exists with
virGetLastErrorCode to avoid a validity pre-check.
Signed-off-by: Ramy Elkest <ramyelkest@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Now that we have one place that sets up all disk-related objects to
qemuBlockStorageSourceAttachDataPtr we can easily reuse the data in the
command-line formatter by implementing a worker which will convert the
data.
A huge advantage is that it will be way easier to integrate this with
-blockdev later on.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce a new setup function for all the related configuration and
move the setup and attachment of the PR code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Create a new "Prepare" function and move the drive add code into the new
helpers. This will eventually allow to simplify and unify the attaching
code for use with blockdev at the same time as providing compatibility
with older qemus.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add code that will handle the managed persistent reservations object
separately from the unmanaged one. There is only one managed object so
handling it with disks is awkward and does not scale well when backing
chains come into view.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Also since we don't do any conditional formatting, fix the comment for
the function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need to check if TLS is enabled as the variable is a tristate.
Currently we'd setup TLS even if it was explicitly turned off.
Thankfully TLS for disks was only used with the vxhs protocol so hardly
anybody would ever be able to hit the problem.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Disks are client-only so we don't need to have this variable. We also
always pass false for 'isListen' to qemuBuildTLSx509BackendProps for all
disk-related code-paths so the 'tlsVerify' is ignored anyways.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Note that it's okay to pass NULL to qemuDomainDelTLSObjects in
qemuDomainAddTLSObjects as the tls-creds-x509 object was either not
created or qemu crashed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function adds the object of a certain type. Change the name so that
we make room for the generic function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function generates JSON properties rather than a string so rename
it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'secinfo' is present also for migrations. Delete the misleading comment.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Setting up the 'secinfo' for the TLS private key password also generates
the given alias, so we don't need to generate another one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the TLS object alias setup earlier. Also make sure that the alias
is not overwritten on hotplug.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For some reason the function returned an error if secAlias was not
passed in. It's not an error, in fact it's desired.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Using 'haveTLS' to do this is pointless if the alias is not set.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that we remember the alias we've used to attach the secret objects
we should reuse them rather than trying to infer them from the disk
configuration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function checks whether the storage source requires authentication
secret setup. Rename it accordingly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In commit 8bebb2b735 I've refactored how the detach of disk with a
managed persistent reservations object is handled. After the commit if
any disk with a managed PR object would be removed libvirt would also
attempt to remove the shared 'pr-manager-helper' object potentially used
by other disks.
Thankfully this should not have practical impact as qemu should reject
deletion of the object if it was still used and the rest of the code is
correct.
Fix this by removing the disk from the definition earlier and checking
if the shared/managed pr-manager-helper object is still needed.
This basically splits the detach code for the managed PR object from the
unmanaged ones. The same separation will follow for the attachment code
as well as it greatly simplifies -blockdev support for this.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Add a new 'vsock' element for the vsock device.
The 'model' attribute is optional.
A <source cid> subelement should be used to specify the guest cid,
or <source auto='yes'/> should be used.
https://bugzilla.redhat.com/show_bug.cgi?id=1291851
Signed-off-by: Ján Tomko <jtomko@redhat.com>
The virDomainDetachDeviceAlias API is designed so that it only
sends detach request to qemu. It's user's responsibility to wait
for DEVICE_DELETED event, not libvirt's. Add @async flag to
qemuDomainDetach*Device() functions so that caller can chose if
detach is semi-synchronous (old virDomainDetachDeviceFlags()) or
fully asynchronous (new virDomainDetachDeviceFlags()).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We are overwriting @ret a lot. It makes hard to see what is
actually going on. Use more gotos. Two functions are fixed here:
qemuDomainDetachShmemDevice() and qemuDomainDetachWatchdog().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
On watchdog unplug, when qemu doesn't support DEVICE_DELETED event
(or couple of other reasons) we do two things:
1) release watchdog device address,
2) call qemuDomainRemoveWatchdog() which does 1) again.
This is potentially dangerous.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
On shmem unplug, when qemu doesn't support DEVICE_DELETED event
(or couple of other reasons) we do two things:
1) release shmem device address,
2) call qemuDomainRemoveShmemDevice() which does 1) again.
This is potentially dangerous.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of releasing address only sometimes in
qemuDomainDetachChrDevice() let's release it whenever the device
is actually removed from the domain definition.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The last caller not passing a comma was removed by:
commit ad8a7c4f85
Author: Ján Tomko <jtomko@redhat.com>
CommitDate: 2018-04-12 17:17:16 +0200
qemu: deprecate QEMU_CAPS_NETDEV
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Libvirt only manages one PR daemon. This means that we don't need to
pass the 'disk' object and also rename the functions dealing with this
so that it's obvious we only deal with the managed PR daemon.
Signed-off-by: Peter Krempa <pkrempa@redhat st.com>
Rather than always checking which path to use pre-assign it when
preparing storage source.
This reduces the need to pass 'vm' around too much. For later use the
path can be retrieved from the status XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
For use with blockdev the PR manager will be bound to a virStorageSource
rather than a virDomainDiskDef, so we will need to use the correct
alias.
Allow passing a string rather than the whole disk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Let us introduce the capability QEMU_CAPS_CCW for virtual-css-bridge
and replace QEMU_CAPS_VIRTIO_CCW with QEMU_CAPS_CCW in code segments
which identify support for ccw devices.
The virtual-css-bridge is part of the ccw support introduced in QEMU 2.7.
The QEMU_CAPS_CCW capability is based on the existence of the QEMU type.
Let us also add the capability QEMU_CAPS_CCW to the tests which
require support for ccw devices.
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If we are the last one to use pr-manager object we need to remove
it and also kill the qemu-pr-helper process.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
When attaching a disk that requires pr-manager we might need to
plug the pr-manager object and start the pr-helper process.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The vm name is not needed for any functional requirement, but it will be
useful when debugging problems to identify which VM is associated with a
filter, since UUID is not human friendly.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There were two places where we'd check this independently. Move it to
the disk definition validation callback. This also fixes possible use of
NULL in a printf for network storage.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Now that we assume QEMU_CAPS_NETDEV, the only thing left to check
is whether we need to use the legacy -net syntax because of
a non-conforming armchitecture.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Now that we assume -netdev support, we no longer set the VLAN
or need the hostPlugged bool.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
This makes qemuDomainSupportsNetdev identical to
qemuDomainSupportsNicdev and leaves some code in
qemuDomainAttachNetDevice to be cleaned up later.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Mediated devices support hot-{plug,unplug} since their introduction in
kernel 4.10, however libvirt has still been missing support for this.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Mediated devices support hot-{plug,unplug} since their introduction in
kernel 4.10, however libvirt has still been missing support for this.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
qemuDomainDetachWatchdog uses the infrastructure for waiting
for the DEVICE_DELETED event, but the asynchronous delete
was not implemented.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Libvirt provides full path to the backing file since commit
fec8f9c49a. This made qemu create the backend object but did not
delete it. This was fixed for unplug case in 4d83a6722f but not in case
of failure to hotplug the frontend. We'd leave the files behind which
would make memory unusable in case of hugepages.
https://bugzilla.redhat.com/show_bug.cgi?id=1553085
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Rather than having the caller check, if the input @addrs is NULL
(e.g. priv->usbaddrs), then just return 0. This also removes the
need for ATTRIBUTE_NONNULL which only really helped if someone
passed a NULL as a parameter not if the passed parameter is NULL.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than having the caller check, if the input @addrs is NULL
(e.g. priv->usbaddrs), then just return 0. This also removes the
need for ATTRIBUTE_NONNULL which only really helped if someone
passed a NULL as a parameter not if the passed parameter is NULL.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Ensure all enum cases are listed in switch statements, or cast away
enum type in places where we don't wish to cover all cases.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The controller model is slightly unusual in that the default value is
-1, not 0. As a result the default value is not covered by any of the
existing enum cases. This in turn means that any switch() statements
that think they have covered all cases, will in fact not match the
default value at all. In the qemuDomainDeviceCalculatePCIConnectFlags()
method this has caused a serious mistake where we fallthrough from the
SCSI controller case, to the VirtioSerial controller case, and from
the USB controller case to the IDE controller case.
By adding explicit enum constant starting at -1, we can ensure switches
remember to handle the default case.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
During domain startup there are many places where we need to acquire
secrets. Currently code passes around a virConnectPtr, except in the
places where we pass in NULL. So there are a few codepaths where ability
to start guests using secrets will fail. Change to acquire a handle to
the secret driver when needed.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Rather than expecting callers to pass a virConnectPtr into the
virDomainDiskTranslateSourcePool() method, just acquire a connection
to the storage driver when needed.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This reverts commit 038eb472a0.
On reflection adding defaults for arbitrary guest XML device config
settings to the qemu.conf is not a sustainable path. Removing the
support for rx/tx queue size so that it doesn't set a bad precedent.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 'f0f2a5ec2' neglected to adjust the if condition to split
out the possibility that the @watchdog is NULL when altering the
message to add detail about the model.
Just split out the condition and use previous/original message, but
with the new message code.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com>
The virStorageTranslateDiskSourcePool method modifies a virDomainDiskDef
to resolve any storage pool reference. For some reason this was added
into the storage driver code, despite working entirely in terms of the
public APIs. Move it into the domain conf file and rename it to match the
object it modifies.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently virt drivers will call directly into the network driver impl
to allocate domain interface devices where type=network. This introduces
a callback system to allow us to decouple the virt drivers from the
network driver.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1461214
Since fec8f9c49a we try to use predictable file names for
'memory-backend-file' objects. But that made us provide full path
to qemu when hot plugging the object while previously we provided
merely a directory. But this makes qemu behave differently. If
qemu sees a path terminated with a directory it calls mkstemp()
and unlinks the file immediately. But if it sees full path it
just calls open(path, O_CREAT ..); and never unlinks the file.
Therefore it's up to libvirt to unlink the file and not leave it
behind.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In 2074ef6cd4 and c56cdf259 (and friends) we've added two
attributes to virtio NICs: rx_queue_size and tx_queue_size.
However, sysadmins might want to set these on per-host basis but
don't necessarily have an access to domain XML (e.g. because they
are generated by some other app). So let's expose them under
qemu.conf (the settings from domain XML still take precedence as
they are more specific ones).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Modify OPERATION_FAILED and INTERNAL_ERROR error codes to
use DEVICE_MISSING instead for failures associated with the
inability to find the device. This makes it easier for consumers
to key off the error code rather than the error message.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Now that the controller model is updated during post parse callback,
this code no longer needs to fetch the model based on the capabilities
and can just return the model directly if the controller is found.
Removal of @qemuCaps cascades through various callers which are now
updated to not pass the capabilities.
If we're going to add a controller to the domain, let's set the
default SCSI model value if we cannot find another SCSI controller
already present.
NB: Requires updating the live output test data since the model
will now be formatted.
Add a check if it's a iSCSI hostdev and if it's not then don't use the
union member 'iscsi'. The segmentation fault occured when accessing
secinfo->type, but this can vary from case to case.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Commit id '162efa1a' added support hotplug a redirdev, but
did not add the hot unplug. This patch will add that support
to allow usage of the detach-device --live on the device.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
When qemuDomainFindOrCreateSCSIDiskController adds a controller,
let's use the same model as a currently found controller under the
assumption that the reason to add the controller in hotplug is
because virDomainHostdevAssignAddress determined that there were
too many devices on the existing controller, but only assigned a
new controller index and did not add a new controller and we
desire to use the same controller model as any existing controller
and not take a chance that qemuDomainSetSCSIControllerModel would
use a default that may be incompatible.
Similar to qemuDomainAddChardevTLSObjects let's move the chardev
source must be TCP and it has the @haveTLS flag set checks before
trying to delete the TLS objects.
For the Chr device this represents no change; however, for RNG device
this is an additionaly check that was missed in commit id '68808516'.
Before adding the objects, TCP and haveTLS are checked.
Let's make a comment deletion helper similar to the Add helper
that can be called after the ExitMonitor.
The modify qemuDomainRemoveChrDevice and qemuDomainRemoveRNGDevice
to call the helper instead of inlining the copy and pasted code.
When the <bandwidth> of an interface is changed with update-device,
the old settings are cleared with tc, then new settings added with
tc. But if the <bandwidth has been removed, the old settings weren't
being removed, so the bandwidth restrictions would still be active on
the interface although the interface status in libvirt showed that
they had been removed.
This patch fixes it by calling virNetDevBandwidthClear() if the
"modification" to the interface bandwidth was to completely clear
it.
An alternative could have been to modify virNetDevBandwidthSet() to
always clear existing bandwith settings at the beginning of the
function (currently it short circuits in that case, doing nothing),
but that would have led to cases where virNetDevBandwidthClear() was
now being called in cases where it previously wasn't, and while many
of those cases would be NOPs, there could be cases where it would
cause an error. The way this patch works, the ...Clear() function is
only called in cases where the ...Set() function had previously been
called successfully, so the risk of regression is minimized.
Resolves: https://bugzilla.redhat.com/1454709
Also call qemuDomainRemoveInputDevice if we receive the
event after the Detach API ends.
Commit 67486bb failed to include this.
https://bugzilla.redhat.com/show_bug.cgi?id=1524837
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Commit id 'c5c96545' neglected to validate that the srcPriv was
non-NULL before dereferencing. Similar problem to what was fixed
by commit id '8056721c' but missed during multiple rebases and
code reworks.
https://bugzilla.redhat.com/show_bug.cgi?id=1425757
The blockdev-add code provides a mechanism to sanely provide user
and password-secret arguments for iscsi without placing them on the
command line to be viewable by a 'ps -ef' type command or needing
to create separate -iscsi devices for each disk/volume found.
So modify the iSCSI command line building to check for the presence
of the capability in order properly setup and use the domain master
secret object to encrypt the password in a secret object and alter
the parameters for the command line to utilize.
Modify the xml2argvtest to exhibit the syntax for both disk and
hostdev configurations.
Rather than picking apart the two pieces we need/want (path, hosts,
and auth)- let's allocate/use a virStorageSourcePtr for iSCSI storage.
The end result is that qemuBuildSCSIiSCSIHostdevDrvStr doesn't need
to "fake" one for the qemuBuildNetworkDriveStr call.
Move the setup of the disk attribute to the disk source prepare function
which will allow proper usage with JSON props and move the fallback
(legacy) generating code into the block which is executed with legacy
options.
As a side-effect of this change we can clean up propagation of 'cfg'
into the command generator.
Also it's nice to see that the test output is the same even when the
value is generated in a different place.
In some cases it does not make sense to pursue that the private data
will be allocated (especially when we don't need to put anything in it).
Ensure that the code works without it.
This also fixes few crashes pointed out in
https://bugzilla.redhat.com/show_bug.cgi?id=1510323
At the same time, move its internals into a separate function so
that they can be reused.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Since the encryption information can also be disk source specific
move it from qemuDomainDiskPrivate to qemuDomainStorageSourcePrivate
Since the last allocated element from qemuDomainDiskPrivate is
removed, that means we no longer need qemuDomainDiskPrivateDispose.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Since the secret information is really virStorageSource specific
piece of data, let's manage the privateData from there instead of
at the Disk level.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Back in the times of using 'pci_del', unplugging a device without
a PCI address was not wired up.
After completely removing support for qemu without QEMU_CAPS_DEVICE,
aliases are used to uniquely identify devices in all cases.
Remove the pointless validation of data that was already present
in the domain definition.
