../../work/libvirt/src/qemu/qemu_hotplug.c: In function ‘qemuDomainAttachFSDevice’:
../../work/libvirt/src/qemu/qemu_hotplug.c:3458:68: error: ordered comparison of pointer with integer zero [-Werror=extra]
3458 | if (qemuBuildVHostUserFsDevProps(fs, vm->def, charAlias, priv) < 0)
| ^
cc1: all warnings being treated as errors
Fixes: b987873034
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Build the properties of 'vhost-vsock' device via JSON. In comparison to
previous similar refactors this also modifies the hotplug code to attach
the vhost fd handle explicitly rather than using
'qemuMonitorAddDeviceWithFd'.
The properties of vhost-vsock have the following types according to
QEMU:
guest-cid=<uint64> - (default: 0)
vhostfd=<str>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Build the properties of 'vhost-scsi' device via JSON. In comparison to
previous similar refactors this also modifies the hotplug code to attach
the vhost fd handle explicitly rather than using
'qemuMonitorAddDeviceWithFd'.
The 'vhost-scsi' device doesn't have any special (non-string) properties.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Build commandlines for character devices via JSON.
For devices using 'VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL' address
type 'qemuBuildDeviceAddressProps' will now generate the address. The
only special property is 'nr'. QEMU declares it as:
nr=<uint32> - (default: 4294967295)
The test fallout is caused by formatting addresses as decimal numbers
instead of hex as described in the commit which added
'qemuBuildDeviceAddressProps'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The handlers for PCI, SCSI and USB controllers already use JSON
internally. This patch converts 'virtio-serial', 'ccid' and 'sata' to do
the same and passes out the JSON directly so that it can be used in
monitor code to avoid conversion.
From the controllers converted in this patch only 'virtio-serial' has
special properties. QEMU thinks they have the following types:
max_ports=<uint32> - (default: 31)
vectors=<uint32> - (default: 2)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The types for the special fields of the 'virtio-blk-pci' according to
QEMU are:
iothread=<link<iothread>>
ioeventfd=<bool> - on/off (default: true)
event_idx=<bool> - on/off (default: true)
scsi=<bool> - on/off (default: false)
num-queues=<uint16> - (default: 65535)
queue-size=<uint16> - (default: 256)
For all disks we also use the following properties (based on 'scsi-hd'):
device_id=<str>
share-rw=<bool> - (default: false)
drive=<str> - Node name or ID of a block device to use as a backend
chardev=<str> - ID of a chardev to use as a backend <- vhost-user-blk-pci
bootindex=<int32>
logical_block_size=<size> - A power of two between 512 B and 2 MiB (default: 0)
physical_block_size=<size> - A power of two between 512 B and 2 MiB (default: 0)
wwn=<uint64> - (default: 0)
rotation_rate=<uint16> - (default: 0)
vendor=<str>
product=<str>
removable=<bool> - on/off (default: false)
write-cache=<OnOffAuto> - on/off/auto (default: "auto")
cyls=<uint32> - (default: 0)
heads=<uint32> - (default: 0)
secs=<uint32> - (default: 0)
bios-chs-trans=<BiosAtaTranslation> - Logical CHS translation algorithm, auto/none/lba/large/rechs (default: "auto") <- ide-hd
serial=<str>
werror=<BlockdevOnError> - Error handling policy, report/ignore/enospc/stop/auto (default: "auto")
rerror=<BlockdevOnError> - Error handling policy, report/ignore/enospc/stop/auto (default: "auto")
The 'wwn' field is changed from a hex string to a number since qemu
actually treats it as a number.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since 'qemuBuildDeviceAddressProps' now also builds 'drive' addresses
the generator is way simpler and doesn't use any special fields.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For properties we use these are the QEMU types:
host=<str> - Address (bus/device/function) of the host device, example: 04:10.0
bootindex=<int32>
failover_pair_id=<str>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For 'usb-mouse'/'usb-tablet'/'usb-kbd' we don't use any special
property.
For 'virtio-input-pci' we only use the 'evdev' argument which is a
string so this conversion doesn't impact anything.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'usb-redir' device has the following types according to QEMU for
properties we control:
chardev=<str> - ID of a chardev to use as a backend
filter=<str>
bootindex=<int32>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'usb-host' device has the following types according to QEMU for
properties we control:
hostdevice=<str>
hostbus=<uint32> - (default: 0)
hostaddr=<uint32> - (default: 0)
bootindex=<int32>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'vfio-pci-nohotplug' device has the following property types
according to QEMU:
display=<OnOffAuto> - on/off/auto (default: "off")
sysfsdev=<str>
ramfb=<bool>
bootindex=<int32>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'virtio-rng' has the following property types according to QEMU:
rng=<link<rng-backend>>
max-bytes=<uint64> - (default: 9223372036854775807)
period=<uint32> - (default: 65536)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Note that the legacy 'ivshmem' device was already removed upstream, but
it's converted so that the code is identical.
For the two modern devices QEMU considers the properties being of
following types:
'ivshmem-doorbell'
chardev=<str> - ID of a chardev to use as a backend
ioeventfd=<bool> - on/off (default: true)
master=<OnOffAuto> - on/off/auto (default: "off")
vectors=<uint32> - (default: 1)
'ivshmem-plain'
master=<OnOffAuto> - on/off/auto (default: "off")
memdev=<link<memory-backend>>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The watchdog doesn't have any special properties.
Convert the command line generator and hotplug code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the now unused boot-index related attributes and the code which
is assigning it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We commonly use 'props' for the JSON object describing something. Rename
the monitor device addition code.
Additionally the common approach is to clear the pointer if it was
consumed so the arguments are adjusted to do so.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use g_autoptr for 'vcpuprops' and remove the 'cleanup' label and 'ret'
varlaible which is no longer needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As advertised in one of previous commits, we want to be able to
change 'requested-size' attribute of virtio-mem on the fly. This
commit does exactly that. Changing anything else is checked for
and forbidden.
Once guest has changed the allocation, QEMU emits an event which
we will use to track the allocation. In the next commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that we've removed support for plaintext secrets qemuDomainSecretInfo
can be simplified by removing the 'type' field and merging in all the
fields from 'qemuDomainSecretAES'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Attaching a newly created vhostuser port to a VM fails due to an
apparmor denial
internal error: unable to execute QEMU command 'chardev-add': Failed
to bind socket to /run/openvswitch/vhu838c4d29-c9: Permission denied
In the case of a net device type VIR_DOMAIN_NET_TYPE_VHOSTUSER, the
underlying chardev is not labeled in qemuDomainAttachNetDevice prior
to calling qemuMonitorAttachCharDev.
A simple fix would be to call qemuSecuritySetChardevLabel using the
embedded virDomainChrSourceDef in the virDomainNetDef vhostuser data,
but this incurs the risk of incorrectly restoring the label. E.g.
consider the DAC driver behavior with a vhostuser net device, which
uses a socket for the chardev backend. The DAC driver uses XATTRS to
store original labelling information, but XATTRS are not compatible
with sockets. Without the original labelling information, the socket
labels will be restored with root ownership, preventing other
less-privileged processes from connecting to the socket.
This patch avoids overloading chardev labelling with vhostuser net
devices by introducing virSecurityManager{Set,Restore}NetdevLabel,
which is currently only implemented for the apparmor driver. The
new APIs are then used to set and restore labels for the vhostuser
net devices.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If current qemu supports 'set-action' use it instead of the single-use
command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
All supported QEMU versions have all the fields so we can remove the
booleans controlling which fields are used on the monitor.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virAppendElement instead of virInsertElementsN to implement
VIR_APPEND_ELEMENT_COPY which allows us to remove error handling as the
only relevant errors were removed when switching to aborting memory
allocation functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If the attempt to attach a device failed, we erased the
unattached device from the namespace. This resulted in erasing an
already attached device in case of a duplicate. We need to check
for existing file in the namespace in order to determine erasing
it in case of a failure.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1780508
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When qos is set or delete, we have to check if the port is an ovs managed
port. If true, call the virNetDevOpenvswitchInterfaceSetQos function when qos
is set, and call the virNetDevOpenvswitchInterfaceClearQos function when
the interface is to be destroyed.
Signed-off-by: Jinsheng Zhang <zhangjl02@inspur.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Originally qemuDomainAttachNetDevice() would wait until the cleanup at
the very end of the function to add newly hotplugged interfaces to the
domain's nets list. commit 7b8bec4560 modified it to add the new
interface to the nets list earlier (but not all the way at the
beginning of the function either, because there are some operations
(PCI address assignment in particular) that need the new device to not
yet be visible in the domaindef).
But hostdev interfaces short-circuit past most of the body of
qemuDomainAttachNetDevice() (since none of it applies to hostdev
interfaces). In the past that was okay, but since the line that adds
the new interface to the domaindef's nets list is in that "most of the
body", after that commit hotplugged hostdev interfaces are no longer
being properly added to the domaindef nets list, so they don't show up
in the status XML or the virsh domiflist output.
It really *is* important to add interfaces to the nets list earlier,
so we can't revert commit 7b8bec4560, and we also can't move the
insert to common code *earlier* in the function, so instead this patch
duplicates the VIR_APPEND_ELEMENT_COPY() just before the code path for
hostdev interfaces jumps to cleanup.
Resolves: https://bugzilla.redhat.com/1972468
Fixes: 7b8bec4560
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
126db34a81 had previously switched various
flows over to this from VIR_ERR_OPERATION_FAILED.
This change simply does the same for qemuDomainDetachPrepDisk,
qemuDomainDetachPrepInput and qemuDomainDetachPrepVsock to allow
management apps to centralise their error handling on just
VIR_ERR_DEVICE_MISSING for missing devices during a detach.
Signed-off-by: Lee Yarwood <lyarwood@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
For validation of explicitly configured addresses we already ported the
same style of checks to qemuValidateDomainDeviceDefAddress and implicit
address assignment should do the right thing in the first place, thus
the function is redundant and can be removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modify the code in the last two instances in the code to behave as if
the flag is not asserted.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When <transient shareBacking='yes'> is set to a disk and the overlay
disk already exists because of something abnormal, libvirt is terminated
by Segmentation fault.
# virsh start Test0
error: Disconnected from qemu:///system due to end of file
error: Failed to start domain 'Test0'
error: End of file while reading data: Input/output error
Add NULL check for snapdiskdef so that the rollback can work correctly.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Fixes: 2e94002d2a
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
and re-adjust if the hotplug fails.
This fixes a bug found during testing of
https://bugzilla.redhat.com/1939776, which was supposed to be resolved
by commit 98e22ff749, but failed to account for the case of device
hotplug.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
An upcoming patch will be checking if the addition of a new net device
requires adjusting the domain locked memory limit, which must be done
prior to sending the command to qemu to add the new device. But
qemuDomainAdjustMaxMemLock() checks all (and only) the devices that
are currently in the domain definition, and currently we are adding
new net devices to the domain definition only at the very end of the
hotplug operation, after qemu has already executed the device_add
command.
In order for the upcoming patch to work, this patch changes
qemuDomainAttachNetDevice() to add the device to the domain nets list
at an earlier time. It can't be added until after PCI address and
alias name have been determined (because both of those examine
existing devices in the domain to figure out a unique value for the
new device), but must be done before making the qemu monitor call.
Since the device has been added to the list earlier, we need to
potentially remove it on failure. This is done by replacing the
existing call to virDomainNetRemoveHostdev() (which checks if this is
a hostdev net device, and if so removes it from the hostdevs list,
since it could have already been added to that list) with a call to
the new virDomainNetRemoveByObj(), which looks for the device on both
nets and hostdevs lists, and removes it where it finds it.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We have many places where the earliest error returns from a function
skip any cleanup label at the bottom (the assumption being that it is
so early in the function that there isn't yet anything that needs to
be explicitly undone on failure). But in general it is a bad sign if
there are any direct "return" statements in a function at any time
after there has been a "goto cleanup" - that indicates someone thought
that an earlier point in the code had done something needing cleanup,
so we shouldn't be skipping it.
There were two occurences of a "return -1" after "goto cleanup" in
qemuDomainAttachDeviceNet(). The first of these has been around for a
very long time (since 2013) and my assumption is that the earlier
"goto cleanup" didn't exist at that time (so it was proper), and when
the code further up in the function was added, the this return -1 was
missed. The second was added during a mass change to check the return
from qemuInterfacePrepareSlirp() in several places (commit
99a1cfc438); in this case it was erroneous from the start.
Change both of these "return -1"s to "goto cleanup". Since we already
have code paths earlier in the function that goto cleanup, this should
not cause any new problem.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The qemuDomainAttachDiskGeneric will also be used on startup for
transient disks which share the overlay. The VM startup code passes the
asyncJob around so we need to pass it into qemuDomainAttachDiskGeneric.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Add code which creates the transient overlay after hotplugging the disk
backend before attaching the disk frontend.
The state of the topmost image is modified to be already read-only to
prevent the need to open the image in read-write mode.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Split up the monitor contexts to attach the backend of the disk and the
frontend device in preparation for hotplugging transient disks where
we'll need to add the code for adding the transient overlay between
these two steps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Modify the rollback section to use its own monitor context so that we
can later split up the hotplug into multiple steps and move the
detachment of the extension device into the rollback section rather than
doing it inline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Similarly to previous refactors we want to move all hotplug related
setup which isn't strictly relevant to attaching the disk into
qemuDomainAttachDeviceDiskLiveInternal.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remove the 'ret' variable and 'cleanup' label in favor of directly
returning the value since we don't have anything under the 'cleanup:'
label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remove two empty lines.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Move the auditing entry and insertion into the disk definition from the
function which deals with qemu to 'qemuDomainAttachDeviceDiskLiveInternal'
which deals with the hotplug related specifics.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
