Peter Krempa [Mon, 12 Apr 2021 15:24:22 +0000 (17:24 +0200)]
qemuBlockJobProcessEventCompletedPull: Avoid dangling pointer after blockpull
When doing a full block pull job (base == NULL) and the config XML
contains a compatible disk, the completer function would leave a
dangling pointer in 'cfgdisk->src->backingStore' as cfgdisk->src would
be set to the value of 'cfgbase' which was always set to
'cfgdisk->src->backingStore'.
This is wrong though since for the live definition XML we set the
respective counterpart to 'job->data.pull.base' which is NULL in the
above scenario.
This leads to a invalid pointer read when saving the config XML and may
end up in a crash.
Resolve it by setting 'cfgbase' only when 'job->data.pull.base' is
non-NULL.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1946918 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Michal Privoznik [Mon, 12 Apr 2021 15:26:06 +0000 (17:26 +0200)]
nodedev: Only set up mdevctl monitors if mdevctl.d exist
During its initialization, the nodedev driver tries to set up
monitors for /etc/mdevctl.d directory, so that it can register
mdevs as they come and go. However, if the file doesn't exist
there is nothing to monitor and therefore we can exit early. In
fact, we have to otherwise monitorFileRecursively() fails and
whole driver initialization fails with it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Michal Privoznik [Mon, 12 Apr 2021 14:25:13 +0000 (16:25 +0200)]
nodedev: Don't join not spawned threads
During the nodedev driver initialization two threads are created:
one for listening on udev events (like device plug/unplug) and
the other for enumerating devices (so that the main thread doing
the driver init is not blocked). If something goes wrong at any
point then nodeStateCleanup() is called which joins those two
threads (possibly) created before. But it tries to join them even
they weren't created which is undefined behaviour (and it just so
happens that it crashes on my system).
If those two virThread variables are turned into pointers then we
can use comparison against NULL to detect whether threads were
created.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Michal Privoznik [Mon, 12 Apr 2021 14:20:57 +0000 (16:20 +0200)]
nodedev: Lock @priv sooner
The nodedev driver private data object @priv is created by
calling udevEventDataNew(). After that, driver->privateData
pointer is set to the freshly allocated object and only a few
lines after all of this the object is locked. Technically it is
safe because there should not be any other thread at this point,
but defensive style of programming says it's better if the object
is locked before driver's privateData is set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Michal Privoznik [Mon, 12 Apr 2021 14:18:24 +0000 (16:18 +0200)]
nodedev: Unlock @priv if initialization of mdevctlMonitors fails
If initialization of priv->mdevctlMonitors fails, then the
control jumps over to cleanup label where nodeStateCleanup() is
called which tries to lock @priv. But since @priv was already
locked before taking the jump a deadlock occurs. The solution is
to jump onto @unlock label, just like the code around is doing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Mon, 12 Apr 2021 16:18:49 +0000 (18:18 +0200)]
bhyve: Fix declaration of 'params' in 'bhyveParsePCIFbuf'
In commit ad80bba90a3 I mistakenly didn't delete '**' from the
variable declaration when converting it to 'GStrv' and deleted the
'separator' variable since it was declared on the same line as a
different variable.
Fixes: ad80bba90a3 Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Peter Krempa [Thu, 8 Apr 2021 14:50:18 +0000 (16:50 +0200)]
tests: qemucapabilitiesdata: Fix formatting of manually added hunk
Commit 66c5674e797 added a query for the device properties of 'usb-host'
but the command header isn't formated the same way as if it were
autogenerated. Reformat all the files.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 9 Apr 2021 13:18:41 +0000 (15:18 +0200)]
qemuxml2argvtest: Parse 'arch' from XML early
If we want to provide correct (fake) caps already for the XML parser we
need to be able to parse the arch early so that we can properly
initialize the caps cache prior to calling the XML parser.
This patch adds code which parses the arch and updates the caps cache
prior to the parse step.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Wed, 31 Mar 2021 14:11:42 +0000 (16:11 +0200)]
qemuxml2argvtest: Rewrite parsing of XMLs to provide earlier parsing
In upcoming patches we'll need to parse a certain bit of XML before
calling the full XML parser. This effectively open-codes what
virDomainDefParseFile to reach virDomainDefParseNode.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Wed, 31 Mar 2021 08:46:36 +0000 (10:46 +0200)]
bhyvexml2argvtest: Use internal wrapping of command line arguments
virCommandToString has the possibility to return an already wrapped
string with better format than what we get from the test wrapper script.
The main advantage is that arguments for an option are always on the
same line which makes it more easy to see what changed in a diff and
prevents re-wrapping of the line if a wrapping point moves over the
threshold.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Wed, 31 Mar 2021 08:46:36 +0000 (10:46 +0200)]
qemuxml2argvtest: Use internal wrapping of command line arguments
virCommandToString has the possibility to return an already wrapped
string with better format than what we get from the test wrapper script.
The main advantage is that arguments for an option are always on the
same line which makes it more easy to see what changed in a diff and
prevents re-wrapping of the line if a wrapping point moves over the
threshold.
Additionally the used output is the same we have in the VM log file when
a VM is starting.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Wed, 31 Mar 2021 07:18:57 +0000 (09:18 +0200)]
virTestCompareToFile: Add possibility to skip unwrapping of input file
In some cases we might want to compare already wrapped data against a
wrapped file. Introduce virTestCompareToFileFull with a 'unwrap' boolean
which will control the unwrapping.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Tue, 6 Apr 2021 08:56:23 +0000 (10:56 +0200)]
virCommandSetDryRun: Add flags to linebreak and strip prefix from the command buffer
virCommandToStringFull used internally when virCommandSetDryRun is
requested allows to strip command path and wrap lines nicely. Expose
these via virCommandSetDryRun so that tests can use those features
instead of local hacks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Thu, 1 Apr 2021 15:54:09 +0000 (17:54 +0200)]
virCommandSetDryRun: Rework resetting of the dry run data
While virCommandSetDryRun is used in tests only, there were some cases
when error paths would not call the function with NULL arguments to
reset the dry run infrastructure.
Introduce virCommandDryRunToken type which must be allocated via
virCommandDryRunTokenNew and passed to virCommandSetDryRun.
This way we can use automatic variable cleaning to trigger the cleanup
of virCommandSetDryRun parameters and also the use of the token variable
ensures that all callers of virCommandSetDryRun clean up after
themselves and also that the token isn't left unused in the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Fri, 26 Mar 2021 16:39:43 +0000 (17:39 +0100)]
qemuxml2argvdata: Remove unused 'args' files
The files were added in error (audio-*) for test cases which produce an
error, left over after converting to DO_TEST_CAPS_LATEST
(disk-detect-zeroes), or left over after splitting test cases
(disk-network-tlsx509).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Mon, 22 Mar 2021 16:21:12 +0000 (17:21 +0100)]
virStorageSourceGetMetadata: Use depth limit instead of unique path checking
Prevent unbounded chains by limiting the recursion depth of
virStorageSourceGetMetadataRecurse to the maximum number of image layers
we limit anyways.
This removes the last use of virStorageSourceGetUniqueIdentifier which
will allow us to delete some crusty old infrastructure which isn't
really needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Both FreeBSD ports and Homebrew on macOS have readline 8.1 now,
and that version contains a correct pkg-config file so the kludge
is no longer necessary.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
When calling virNodeDeviceDefineXML() to define a new mediated device,
we call virMdevctlDefine() and then wait for the new device to appear in
the driver's device list before returning. This caused long delays due
to the behavior of nodeDeviceFindNewMediatedDevice(). This function
checks to see if the device is in the list and then waits for 5s before
checking again.
Because mdevctl is relatively slow to query the list of defined
devices[0], the newly-defined device was generally not in the device
list when we first checked. This results in libvirt almost always taking
at least 5s to complete this API call for mediated devices, which is
unacceptable.
In order to avoid this long delay, we resort to a workaround. If the
call to virMdevctlDefine() was successful, we can assume that this new
device will exist the next time we query mdevctl for new devices. So we
simply add this provisional device definition directly to the nodedev
driver's device list and return from the function. At some point in the
future, the mdevctl handler will run and the "official" device will be
processed, which will update the provisional device if any new details
need to be added.
The reason that this is not necessary for virNodeDeviceCreateXML() is
because detecting newly-created (not defined) mdevs happens through
udev instead of mdevctl. And nodeDeviceFindNewMediatedDevice() always
calls 'udevadm settle' before checking to see whether the device is in
the list. This allows us to wait just long enough for all udev events to
be processed, so the device is almost always in the list the first time
we check and so we almost never end up hitting the 5s sleep.
[0] on my machine, 'mdevctl list --defined' took around 0.8s to
complete for only 3 defined mdevs.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Calling `mdevctl stop` for a mediated device that is in use by an active
domain will block until that vm exits (or the vm closes the device).
Since the nodedev driver cannot query the hypervisor driver to see
whether any active domains are using the device, we resort to a
workaround that relies on the fact that a vfio group can only be opened
by one user at a time. If we get an EBUSY error when attempting to open
the group file, we assume the device is in use and refuse to try to
destroy that device.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Jonathon Jongsma [Fri, 29 Jan 2021 21:24:10 +0000 (15:24 -0600)]
nodedev: add <uuid> element to mdev caps
It will be useful to be able to specify a particular UUID for a mediated
device when defining the node device. To accomodate that, allow this to
be specified in the xml schema. This patch also parses and formats that
value to the xml, but does not yet use it.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
This virsh command maps to virNodeDeviceCreate(), which starts a node
device that has been previously defined by virNodeDeviceDefineXML().
This is only supported for mediated devices.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Jonathon Jongsma [Thu, 18 Jun 2020 15:21:13 +0000 (10:21 -0500)]
api: add virNodeDeviceCreate()
This new API function provides a way to start a persistently-defined
mediate device that was defined by virNodeDeviceDefineXML() (or one that
was defined externally via mdevctl)
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Jonathon Jongsma [Mon, 17 Aug 2020 21:21:48 +0000 (16:21 -0500)]
virsh: Factor out function to find node device
Several functions accept providing a node device by name or by wwnn,wwpn
pair. Extract the logic to do this into a function that can be used by
both callers.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>