Revert "logging: Use default timeout of 120 seconds for virtlogd"
The same timeout as libvirtd can't be used for virtlogd: even with
socket activation in place, any message produced by QEMU on its
standard output/error between when virtlogd quits due to the timeout
and when it's started again due to socket activation will get lost.
Peter Krempa [Thu, 2 Apr 2020 05:29:58 +0000 (07:29 +0200)]
qemuDomainSnapshotDiskPrepareOne: Fix logic of relative backing store update
Commit 2ace7a87a8aced68c250 introduced a logic bug by an improperly
modified condition where we'd skip to the else branch when reusing of
external images was requested and blockdev is available.
The original intentions were to skip the backing store update with
blockdev.
Fix it by only asserting the boolean which was used to track whether we
support update of the backing store only when blockdev is not present
along with the appropriate rename.
Ján Tomko [Wed, 1 Apr 2020 08:34:10 +0000 (10:34 +0200)]
qemu: use QEMU_CAPS_STORAGE_WERROR for disk-error attributes
When moving the formatting of this attributes from -drive
to -device, the QEMU_CAPS_USB_STORAGE_WERROR capability
was used, because usb-storage was the last device to gain
this capability.
However this lead to the assumption that QEMU binaries
without the usb-storage device do not support this,
leading to breakage on s390x with blockdev.
Ján Tomko [Wed, 1 Apr 2020 08:22:30 +0000 (10:22 +0200)]
qemu: add QEMU_CAPS_STORAGE_WERROR
Detect the werror property on SCSI and virtio disks.
But clear it if the QEMU supports usb-storage device without it
also supporting this option for usb-storage.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The Fedora rawhide job started failing with the latest container build.
Since we now have broad CI coverage on GitLab CI, there's not a strong
reason to continue using Travis for Linux jobs. Deleting the redundant
jobs is a better use of time than trying to debug the failure.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
A bogus comment was added claiming we didn't need to shutdown the
event thread in the qemuProcessStop method, because this would be
done in the monitor EOF callback. This was wrong because the EOF
callback only runs in the case of a QEMU crash or a guest initiated
clean shutdown & poweroff. In the case where the libvirt admin
calls virDomainDestroy, the EOF callback never fires because we
have already unregistered the event callbacks. We must thus always
attempt to stop the event thread in qemuProcessStop.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reported-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Mon, 30 Mar 2020 09:18:32 +0000 (11:18 +0200)]
qemu: block: Support VIR_DOMAIN_BLOCK_COMMIT/PULL/REBASE_RELATIVE with blockdev
Preservation of the relative relationship requires us to load the
backing store strings from the disk images. With blockdev we stopped
detecting the backing chain if it's specified in the XML so the relative
links were not loaded at that point. To preserve the functionality from
the pre-blockdev without accessing the backing chain unnecessarily
during VM startup we must refresh the relative links when relative
block commit or block pull is requested.
While it is impossible for VIR_ALLOC() to return an error, we
should be consistent with the rest of the code and not continue
initializing the virSecurityDeviceLabelDef structure.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Mon, 30 Mar 2020 08:56:08 +0000 (10:56 +0200)]
virQEMUCapsNewBinary: Avoid NULL dereference
Even with GLib it is still possible for virQEMUCapsNew() to
return NULL because it calls virQEMUCapsInitialize() which is a
wrapper over pthread_once() which may fail. At least, we still
check for its retval. If it so happens that the virQEMUCapsNew()
fails and returns NULL, we should not dereference it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Mon, 30 Mar 2020 08:37:09 +0000 (10:37 +0200)]
Don't pass NULL to yajl_free()
Unfortunately, yajl_free() is not NOP on NULL. It really does
expect a valid pointer. Therefore, check whether the pointer we
want to pass to it is NULL or not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Jiri Denemark [Fri, 27 Mar 2020 20:05:28 +0000 (21:05 +0100)]
docs: Clarify semantics of model/@usable attribute in dom caps
The documentation could confuse people to expect that CPU models with
usable='no' attribute are not usable at all on the current host. But
they cannot be only used without explicitly disabling some features.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Rafael Fonseca [Fri, 27 Mar 2020 17:40:47 +0000 (18:40 +0100)]
util: virdaemon: fix compilation on mingw
The daemons are not supported on Win32 and therefore were not compiled
in that platform. However, with the daemon code sharing, all the code in
utils *is* compiled and it failed because `waitpid`, `fork`, and
`setsid` are not available. So, as before, let's not build them on
Win32 and make the code more portable by using existing vir* wrappers.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Fri, 27 Mar 2020 15:40:05 +0000 (16:40 +0100)]
gitlab: Add script_variables and define $MAKEFLAGS there
This new template provides a standardized place where environment
variables which are not static, but rather depend on the output of
some shell command, can be defined for later use. This pattern is
already used in libosinfo's GitLab CI integration.
Defining $MAKEFLAGS there means we don't need to call getconf over
and over, and the actual build steps don't end up drowned in the
noise.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Rafael Fonseca [Thu, 26 Mar 2020 15:18:03 +0000 (16:18 +0100)]
remote: use shared daemon startup code
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rafael Fonseca [Thu, 26 Mar 2020 15:18:02 +0000 (16:18 +0100)]
logging: use shared daemon startup code
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rafael Fonseca [Thu, 26 Mar 2020 15:18:01 +0000 (16:18 +0100)]
locking: use shared daemon startup code
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rafael Fonseca [Thu, 26 Mar 2020 15:18:00 +0000 (16:18 +0100)]
util: introduce shared daemon startup code
Several daemons have similar code around general daemon startup code.
Let's move it into a file and share it among them.
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
scripts: avoid error in DCO check on empty branches
If the DCO check is run on an empty branch (ie one which has no commits
different from master), it throws an error due to trying to interpret
the empty string as a git commit SHA.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Running the code style syntax-check as part of the build jobs leads to
all jobs failing in the same way. Have a prebuild job for validating
syntax-check to catch code style problems upfront and thus avoid needing
to run all the build jobs.
Reviewed-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>a Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
gitlab: introduce a check for validate DCO sign-off
This introduces a CI job for validating DCO sign-off in every commit
message. The CI jobs are not provided any information on what the
baseline commit for the branch was. We can't compare against the forked
repo's master branch, as there's no guarantee the user is keeping master
up2date in their fork. Thus we add the master upstream repo as a git
remote and identify the common ancestor.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
gitlab: introduce use of ccache for speeding up rebuilds
For any given job there is a high likelihood that ccache will be able to
reuse previously built object files. This will result in faster build
pipelines in later updates.
Reviewed-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Whenever there is a change to the translatable strings we need to push
a new libvirt.pot to weblate. This only needs to be done when code
merges into git master, so the job is restricted to that branch.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This patch adds x86_64 native CI jobs for all distros that we currently
build container images for. This is a superset of the Linux jobs run on
current Jenkins and Travis platforms.
The remaining missing platforms are FreeBSD and macOS, neither of which
can use the shared runner container based infrastructure.
We may add further native jobs in the future which are not x86_64 based,
if we get access to suitable hardware, thus the jobs all have an arch
prefix in their name, just like the cross-built jobs do.
As with the cross-arch builds, the native jobs are split into two
groups. One group is run in all situations, while the other group is
only run on the master branch, or branches with a name prefix
'ci-full-'. This avoids the build time getting too long when
developers are testing their code prior to submission, while keeping
full coverage of code that is merged.
Reviewed-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This pulls in the mingw cross build jobs using Fedora 30 as a base,
matching what is done on Jenkins and Travis.
Reviewed-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The pipeline UI will truncate the names of jobs after about 15
characters. As a result with the cross-builds, we truncate the
most important part of the job name. Putting the most important
part first is robust against truncation, and we can drop the
redundant "-cross" stub.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <skultety.erik@gmail.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
gitlab: reduce number of cross build jobs run by default
Currently we have nine different cross build jobs, but as we introduce
more native jobs this is going to result in a very long CI execution
time. For developers testing their personal branches under development
it is generally sufficient to just look at a couple of interesting
scenarios, namely 32-bit and big endian.
This splits the cross build jobs so that by default only the armv7
and s390x archs are built. The remainining archs are setup so that they
are only built for code on the master branch, which will have the effect
of doing post-merge testing. Developers can opt-in to full testing of
their pre-merge code by pushing it to a branch with a name prefix of
"ci-full-".
Reviewed-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Run the bare minimum build that is possible to create the docs, avoiding
compiling code which other jobs will deal with.
The generated website is published as an artifact and thus is browsable
by developers on build completion and can be downloaded as a zip file.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
gitlab: create an explicit stage for cross build jobs
As we introduce more build jobs, it will be useful to have a grouping of
jobs to more easily visualize the results and potentially control build
ordering.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We don't need the full git history when running CI jobs. From a code POV
we only need the most recent commit, but we want to be able to run
checks on the commits too. In particular to validate the DCO signoff for
each commit.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
qemu_domain.c: remove unneeded size check in NVDIMM alignment
Commit 5540acb9a2b added a minimum size verification for the target
size of ppc64 NVDIMMs but forgot to remove a MAX() size check that
was being used in earlier reviews of that commit. The size
verification makes this check unneeded since we're making sure
that guestArea will always be at least equal to ppc64AlignSize.
Fixes: 5540acb9a2bd33fb97a7446a610b602c9e8629a3 Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 26 Mar 2020 14:37:44 +0000 (15:37 +0100)]
qemuBackupBegin: Fix monitor access when rolling back due to failure
The code attempting to clean up after a failed pull mode backup job
wrongly entered monitor but didn't clean up nor exit monitor due to a
logic bug. Fix the condition.
Jiri Denemark [Tue, 17 Mar 2020 21:32:16 +0000 (22:32 +0100)]
cpu_map: Don't use new noTSX models for host-model CPUs
Host-model CPU definitions (and domain capabilities) will use the
original CPU models (without noTSX in their name) and explicitly disable
hle and rtm features. This way domains with host-model CPUs will be
migratable even to older versions of libvirt which do not support the
noTSX model variants.
The new models will be advertised in host capabilities and they may
be used explicitly with custom CPUs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Jiri Denemark [Thu, 12 Mar 2020 16:39:37 +0000 (17:39 +0100)]
cpu_map: Add <decode> element to x86 CPU model definitions
The element specifies whether a particular CPU model can be used when
creating a CPU definition from raw CPUID/MSR data. The @host attribute
determines whether the CPU model can be used (host='on') for creating
CPU definition for host capabilities. Usability of the model for domain
capabilities and host-model CPU definitions is controlled by the @guest
attribute.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
One of the mitigation methods for TAA[1] is to disable TSX
support on the host system. Linux added a mechanism to disable
TSX globally through the kernel command line, and many Linux
distributions now default to tsx=off. This makes existing CPU
models that have HLE and RTM enabled not usable anymore.
Add new versions of all CPU models that have the HLE and RTM
features enabled, that can be used when TSX is disabled in the
host system.
On systems disabling the features without those types defined
in cpu-maps users end up without modern CPU types in the list
of usable CPUs to use in the likes of virsh domcapabilities
or tools higher in the stack like virt-manager.
This adds:
-Cascadelake-Server-noTSX
-Icelake-Client-noTSX
-Icelake-Server-noTSX
-Skylake-Server-noTSX-IBRS
-Skylake-Client-noTSX-IBRS
qemu: allow PMU feature to be enabled for ppc64 guests
The PMU feature is enabled by default in ppc64 guests and can't
be disabled via Libvirt or QEMU [1]. The current PMU feature
implementation does not allow PMU to enabled or disabled in the
ppc64 guest. Declaring the PMU feature will make the 'pmu'
property to be passed on to QEMU, but this property isn't
available for ppc64:
qemu-kvm: can't apply global host-powerpc64-cpu.pmu=on: Property '.pmu' not found
A similar error is thrown when trying to disable the PMU.
This patch standardizes the PMU handling for ppc64 guests by:
- throwing an error if the user attempts to set the feature to
'off', given that this feature can't be turned off at all;
- allowing the feature to be declared as 'on' in the domain XML.
This is done by skipping ppc64 guests when creating the command
line for this feature.
qemu: make Hyperv settings exclusive to x86 and aarch64
Hyperv features are supported by both x86 and aarch64. The <hyperv/>
declaration in the XML by itself is benign to other architectures,
but any of its 14 current features will break QEMU with an error
like this (from ppc64):
qemu-kvm: Expected key=value format, found hv_relaxed
This is a more extreme case than the one for apic eoi because we
would need an extra 'switch' statement, with all current Hyperv
features in the body of qemuDomainDefValidateFeatures(), to
check if the user attempted to activate any of them. It's easier to
simply fail to launch with any 'hyperv' declaration in the XML for
every arch which is not x86 and aarch64.
A fair disclaimer about Windows and PowerPC: the last Windows version
that ran in the architecture is the hall of famer Windows NT 4.0,
launched in 1996 and with end of extended support for the Server
version in 2004 [1]. I am acknowledging that there might be Windows
NT 4.0 users running in PowerPC, but not enough people running it
under KVM/QEMU to justify Libvirt allowing 'hyperv' to exist in the
domain XML of ppc64 domains.
[1] https://en.wikipedia.org/wiki/Windows_NT_4.0
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
qemu: avoid launching non-x86 guests with 'pvspinlock' setting
The 'pvspinlock' feature is x86 only. The "<pvspinlock/>" declaration
will always have a value 'on' or 'off', and both will break QEMU when
launching non-x86 guests. This is the error message for
"<pvspinlock state='on'/>" when running a ppc64 guest:
qemu-kvm: Expected key=value format, found +kvm_pv_unhalt
A similar error message is thrown for "<pvspinlock state='off'/>".
This patch prevents non-x86 guests from launching with any
pvspinlock setting with a more informative error message:
error: unsupported configuration: The 'pvspinlock' feature is not
supported for architecture 'ppc64' or machine type 'pseries'
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
qemu: avoid launching non-x86 guests with APIC-EOI setting
The "<apic/>" feature, although it's available only for x86 guests,
can be declared in the domain XML of other archs without errors.
But setting its 'eoi' attribute will break QEMU. For "<apic eoi='on'/>",
in a ppc64 guest:
qemu-kvm: Expected key=value format, found +kvm_pv_eoi
A similar error happens with eoi='off'.
One can argue that it's better to simply forbid launching non-x86
guests with "<apic/>" declared in the XML - it is a feature that
the architecture doesn't support and this would make it clearer
about it. This is sensible, but there are non-x86 guests that are
running with "<apic/>" declared in the domain (and A LOT of guests
running with "<acpi/>" for that matter, probably reminiscent of x86
templates that were reused for other archs) that will stop working if we
go this route.
A more subtle approach is to detect if the 'eoi' element is being set
for non-x86 guests and warn the user about it with a better error
message than the one QEMU provides. This is the new error message
when any value is set for the 'eoi' element in a ppc64 XML:
error: unsupported configuration: The 'eoi' attribute of the 'apic'
feature is not supported for architecture 'ppc64' or machine type
'pseries'.
Use qemuAgentCommandFull in qemuAgentGetHostname so that we can suppress
error reports if the caller will not require them. Callers for now
always require error reporting but will be fixed later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 13 Mar 2020 08:49:35 +0000 (09:49 +0100)]
qemuAgentCommand: Wire up suppressing of error reporting for unsupported commands
In some cases we don't want to log errors if an agent command is
unsupported. Wire it up into qemuAgentCheckError via qemuAgentCommandFull
and provide a thin wrapper (qemuAgentCommand) to prevent having to fix
all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 13 Mar 2020 08:05:48 +0000 (09:05 +0100)]
qemuDomainGetGuestInfo: Validate supported information types
'qemuDomainGetGuestInfoCheckSupport' despite its name was not checking
whether the info types are supported. Convert the function to return
integers and include the check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Unfortunately, advisory record locking lose the lock if any fd refering
to the file is closed. There doesn't seem to be a way to preserve the
lock atomically. We could eventually retake the lock if low pidfilefd
is required.
This fixes processes being leaked, as they are not killed in
virPidFileForceCleanupPath() if the lock can be taken. Here also, we may
consider this is not good enough, as a process may leak by simply
closing the pidfilefd.
Helper processes may have their state migrated with QEMU data stream
thanks to the QEMU "dbus-vmstate".
libvirt maintains the list of helpers to be migrated. The
"dbus-vmstate" is added when required, and given the list of helper
Ids that must be migrated, on save & load sides.
See also:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus-vmstate.rst
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The daemon is meant to be started on demand, and associated with a
QEMU process. It should be stopped when the QEMU process is stopped.
The current policy is permissive like a session bus. Stricter
policies can be added later, following recommendations from:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus.rst
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This code was based on a per-helper instance and peer-to-peer
connections. The code that landed in qemu master for v5.0 is relying
on a single instance and DBus bus.
Instead of trying to adapt the existing dbus-vmstate code, let's
remove it and resubmit. That should make reviewing easier.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Fri, 13 Mar 2020 16:06:19 +0000 (17:06 +0100)]
bridge_driver: Replace and drop networkKillDaemon
In the network driver code there's networkKillDaemon() which is
the same as virProcessKillPainfully(). Replace the former with
the later and drop what becomes unused function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Michal Privoznik [Fri, 13 Mar 2020 15:12:59 +0000 (16:12 +0100)]
qemuSlirpStop: Simplify helper kill
Now, that we know that the slirp helper will have the pidfile
open and locked we can use virPidFileForceCleanupPath() to kill
it and unlink the pidfile.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Michal Privoznik [Fri, 13 Mar 2020 14:57:24 +0000 (15:57 +0100)]
qemuProcessStartManagedPRDaemon: Don't pass -f pidfile to the daemon
Now, that our virCommandSetPidFile() is more intelligent we don't
need to rely on the daemon to create and lock the pidfile and use
virCommandSetPidFile() at the same time.
NOTE that as advertised in the previous commit, this was
temporarily broken, because both virCommand and
qemuProcessStartManagedPRDaemon() would try to lock the pidfile.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Michal Privoznik [Fri, 13 Mar 2020 12:12:49 +0000 (13:12 +0100)]
virCommand: Actually acquire pidfile instead of just writing it
Our virCommand module allows us to set a pidfile for commands we
want to spawn. The caller constructs the string of pidfile path
and then uses virCommandSetPidFile() to tell the module to write
the pidfile once the command is ran. This usually works, but has
two flaws:
1) the child process does not hold the pidfile open & locked.
Therefore, the caller (or anybody else) can't use our fancy
virPidFileForceCleanupPath() function to kill the command
afterwards. Also, for everybody else on the system it's
needlessly harder to check if the pid from the pidfile is still
alive or not.
2) if the caller ever makes a mistake and passes the same pidfile
path for two different commands, the start of the second command
will overwrite the pidfile even though the first command might
still be running.
NOTE that this temporarily renders some command spawning
unusable, specifically those code patterns where both
virCommandSetPidFile() is used together with instructing spawned
command to acquire pidfile itself. Fortunately, there is only one
occurrence of such pattern and it is in
qemuProcessStartManagedPRDaemon(). This is fixed in next commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Michal Privoznik [Tue, 24 Mar 2020 12:48:58 +0000 (13:48 +0100)]
commandtest: Fix test28 error detection
As a part of c799d150d5e9dae I've introduced a test case that
tests whether passing error object between processes works. The
test spawns a child which reports a system error, parent process
then reads the error and compares with expected output. Problem
with this approach is that error message contains stringified
errno which is not portable. FreeBSD has generally different
messages than Linux. Therefore, use g_strerror() to do the errno
to string translation for us.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Mon, 23 Mar 2020 17:06:56 +0000 (18:06 +0100)]
storage: Parse 'nvme' disk source properties from json:{} pseudo-uri
Our code allows snapshots of NVMe based disks which means we create
overlay file with a 'json:{}' pseudo-uri refering to the NVME device.
Our parser code doesn't handle them though. Add the parser and test it
via the XML->json->XML round-trip and reference data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Mon, 23 Mar 2020 15:36:22 +0000 (16:36 +0100)]
qemuBlockGetBackingStoreString: Properly handle 'http/s' with cookies and others
Format cookies into the backing store string without encryption as they
will not be visible on the command line when formatting a 'target' only
string. In cases when cookies or other options are used we must use the
JSON format rather than pure URI.
Add tests to validate the scenario.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
projects / libvirt.git / log