Continue consolidation of video device validation started in previous
patch.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
The goal is to move all of the video device validation to a single place
and use domain caps to validate the supported video device models. Since
qemuDomainDeviceDefValidateVideo() is called from
qemuProcessStartValidate(), these changes should not change anny
behavior.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
In a follow-up commit, we will use the domain capabilities to validate
video device configurations, which means that we also need to make sure
that the domain capabilities include the "none" video device.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
commit 9bfcf0f62d added the
QEMU_CAPS_DEVICE_RAMFB capability but did not set the domain capability.
This patch sets the domain capability for the ramfb device and updates
the tests.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Several tests were not specifying the necessary qemu capabilities for
what they were testing. Due to the way that the video devices are
currently validated, this is not causing any problems. But a change to
video device validation in a following patch would have exposed this
issue and resulted in multiple test failures about the domain
configuration not supporting particular video models.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
This allows us to simplify the function and avoid jumping to 'cleanup'.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
When the virDomainCapsDeviceDefValidate() function returned an error
status (-1), we were aborting the function early, but returning the
default return value (0). This patch properly returns an error in that
case.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
The ordering of lock manager locks in the libxl driver has a flaw that was
uncovered by a migration error path. In the perform phase of migration, the
source host calls virDomainLockProcessPause to release the lock before
sending the VM to the destination host. If the send fails an attempt is made
to reacquire the lock with virDomainLockProcessResume, but that too can fail
if the destination host has not finished cleaning up the failed VM and
releasing the lock it acquired when starting to receive the VM.
This change delays calling virDomainLockProcessResume in libxlDomainStart
until the VM is successfully created, but before it is unpaused. A similar
approach is used by the qemu driver, avoiding the need to release the lock
if VM creation fails. In the migration perform phase, releasing the lock
with virDomainLockProcessPause is delayed until the VM is successfully
sent to the destination, which avoids reacquiring the lock if the send
fails.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
The virHostGetBootTimeProcfs() function is defined only for Linux
and therefore it's only call should also be done if we're on
Linux.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add a helper which converts qemu emulator capabilities to the domain
capability XML. This will simplify future additions of new features.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Declare the capabilities as enum values and store them in an array. This
makes adding new features more straightforward and simplifies the
formatter which now doesn't require changing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While the qemu driver currently implements all domain capability
features, we should initialize all features using the helper similarly
to how we do it in drivers which don't support any.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For future extensions of the domain caps it's useful to have a single
point that initializes all capabilities as unsupported by a driver. The
driver then can enable specific ones.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract it to virDomainCapsFormatFeatures so that the main function does
not get so bloated over time.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce qemuDomainCapsFeatureFormatSimple which does exactly the same
thing but it's a function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use our helper instead of the gnulib one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make it more obvious that the function will return NULL if the file is
not executable and stop reusing variables.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Simplify the final lookup loop by freeing memory automatically and thus
being able to directly return the result.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When parsing allowed authentication methods for the native ssh lib
transports we used strsep. Since we have virStringSplit helper let's use
that one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When we want to know the boot timestamp of the host, we can call
virHostGetBootTime(). Under the hood, it uses getutxid() which is
defined by POSIX and properly check for in configure. However,
musl took a path where it declares the function but instead of
providing any useful implementation it returns NULL meaning "no
record found". If that's the case, use our second best option -
/proc/uptime and a bit of maths.
https://bugzilla.redhat.com/show_bug.cgi?id=1760885
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
It was lifted with c92b6023e8.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
With this patch users can cold plug some sound devices.
use "virsh attach-device vm sound.xml --config" command.
Consider the following sound.xml for a domain:
<sound model='ich6'>
<address type='pci' domain='0x0000' bus='0x00' slot='xxx' function='0'/>
</sound>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jidong Xia <xiajidong@cmss.chinamobile.com>
A function virStringParseYesNo was added to convert
string 'yes' to true and 'no' to false, so use this
helper to replace 'STREQ(.*, \"yes\")' and
'STREQ(.*, \"no\")' as it allows us to drop several
repetitive if-then-else string->bool conversion blocks.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
A function virStringParseYesNo was added to convert
string 'yes' to true and 'no' to false, so use this
helper to replace 'STREQ(.*, \"yes\")' and
'STREQ(.*, \"no\")' as it allows us to drop several
repetitive if-then-else string->bool conversion blocks.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
This helper performs a conversion from a "yes|no" string
to a corresponding boolean, and several conversions were
already done, but there are still some omissions.
For most of the remaining usages in domain_conf.c only
"yes" is explicitly checked for. This means all other
values are implicitly handled as 'false'. In this case,
use virStringParseYesNo to handle the conversion and
reserve the original logic of not raise an error, so
ignore the return value of helper.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Instead of vsnprintf from gnulib, use g_vsnprintf from GLib.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The callers don't actually use the returned errno for reporting errors.
Additionally virFileResolveAllLinks returns -1 rather than -errno on
error thus you'd get a spurious EPERM even on other errors.
Don't try to return errno in this case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Return -1 on failure rather than -errno since none of the callers
actually cares about the return value. This specifically fixes returns
of -ENOMEM in cases of bad usage, which would report wrong error
anyways.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The caller doesn't care about the actual return value, so return -1
rather than errno.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The callers don't care about the actual return value, so return -1
rather than errno.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In an effort to remove as much gnulib usage as possible let's
reimplement virFileReadLink. Since it's used in two places only I opted
to open-code it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The inactive external snapshot code replaced the file name in the
virStorageSource but did not touch the backing files. This meant that
after an inactive snapshot the backing chain recorded in the inactive
XML (which is used with -blockdev) would be incorrect.
Fix it by adding a new layer if there is an existing chain and replacing
the virStorageSource struct fully when there is no chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When commiting a different image becomes the disk source. Since we store
the readonly flag per-image we must update it to the same state the
original image had.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The current 'setvcpus' timeout message requires a deeper
understanding of QEMU/Libvirt internals to proper react to it.
One who knows how setvcpus unplug work (it is an asynchronous
operation between QEMU and guest that Libvirt can't know for
sure if it failed, unless an explicit error happened during the
timeout period) will read the message and not assume a failed
operation. But the regular user, most often than not, will read
it and believe that the unplug operation failed.
This leads to situations where the user isn't exactly relieved
when accessing the guest and seeing that the unplug operation
worked. Instead, the user feel mislead by the timeout message
setvcpus threw.
Changing the timeout message to let the user know that the
unplug status is not known, and manual inspection in the guest
is required, is not a silver bullet. But it gives a more
realistic expectation of what happened, as best as we can tell
from Libvirt side anyways.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemu_hotplugpriv.h is a header file created to share a global variable
called 'qemuDomainRemoveDeviceWaitTime', declared in qemu_hotplug.c,
to other files that would want to change the timeout value
(currently, only tests/qemuhotplugtest.c).
Previous patch deprecated the variable, using qemu_driver->unplugTimeout
to set the timeout instead. This means that the header file is now
unused, and can be safely discarded.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
For some architectures and setups, device removal can take
longer than the default 5 seconds. This results in commands
such as 'virsh setvcpus' to fire timeout messages even if
the operation were successful in the guest, confusing the
user.
This patch sets a new 10 seconds unplug timeout for PPC64
guests. All other archs will keep the default 5 seconds
timeout.
Instead of putting 'if PPC64' conditionals inside qemu_hotplug.c
to set the new timeout value, a new function called
qemuDomainGetUnplugTimeout was added. The timeout value is then
retrieved when needed, by passing the correspondent DomainDef
object. This approach allows for different guest architectures
to have distint unplug timeout intervals, regardless of the
host architecture. This design also makes it easier to
modify/enhance the unplug timeout logic in the future
(allow for special timeouts for TCG domains, for example).
A new mock file was created to work with qemuhotplugtest.c,
given that the test timeout is significantly shorter than
the actual timeout value in qemu_hotplug.c.
The now unused 'qemuDomainRemoveDeviceWaitTime' global can't
be simply erased from qemu_hotplug.c though. Next patch will
remove it properly.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Suggested-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
In preparation for some other improvements, switch to using glib
allocation and g_autofree when parsing the 'acceleration' and
'resolution' properties of the video device.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Just above in the function, we return from the function if either x or y
are NULL, so there's no need to re-check whether x or y are NULL.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
The video resolution support that was introduced in
7286279797 is specified as a <resolution>
sub-element of <model>, not optional attributes of model.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Commit 72862797 introduced resolution settings for QEMU video drivers.
It includes a new structure inside video definition. So, the code needs
to clear pointer allocation for that structure into clear function
virDomainVideoDefClear(). This commit adds this missing VIR_FREE().
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Use g_strndup in all the cases where we check upfront whether a pointer
is non-NULL and then use it to calculate the copied length.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
