ramyelkest [Sat, 5 May 2018 12:04:21 +0000 (13:04 +0100)]
all: Replace virGetLastError with virGetLastErrorCode where we can
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>
ramyelkest [Sat, 5 May 2018 12:04:20 +0000 (13:04 +0100)]
util: virerror: Introduce virGetLastError{Code,Domain} public APIs
Many places in the code call virGetLastError() just to check the
raised error code, or domain. However virGetLastError() can return
NULL, so the code has to check for that first. This patch therefore
introduces virGetLasError{Code,Domain} functions which always return a
valid error code or domain respectively, thus dropping the need to
perform any checks on the error object.
Signed-off-by: Ramy Elkest <ramyelkest@gmail.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
there was code that would loop and retry the connection when opening
the agent socket. At this time, the only thing done in between the
opening of the monitor socket & opening of the agent socket was a
call to set the monitor capabilities. This was a no-op on non-QMP
versions, so in theory there could be a race which let us connect
to the monitor while the agent socket was still not created by QEMU.
In the modern world, however, we long ago mandated the use of QMP
for managing QEMU, so we're guaranteed to have a set capabilities
QMP call. Once we've seen a reply to this, we're guaranteed that
QEMU has fully initialized all backends and is in its event loop.
We can thus be sure the QEMU agent socket is present and don't need
to retry connections to it, even without having the chardev FD passing
feature.
Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Since libvirt called bind() and listen() on the UNIX socket, it is
guaranteed that connect() will immediately succeed, if QEMU is running
normally. It will only fail if QEMU has closed the monitor socket by
mistake or if QEMU has exited, letting the kernel close it.
With this in mind we can remove the retry loop and timeout when
connecting to the QEMU monitor if we are doing FD passing. Libvirt can
go straight to sending the QMP greeting and will simply block waiting
for a reply until QEMU is ready.
Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
qemu: support passing pre-opened UNIX socket listen FD
There is a race condition when spawning QEMU where libvirt has spawned
QEMU but the monitor socket is not yet open. Libvirt has to repeatedly
try to connect() to QEMU's monitor until eventually it succeeds, or
times out. We use kill() to check if QEMU is still alive so we avoid
waiting a long time if QEMU exited, but having a timeout at all is still
unpleasant.
With QEMU 2.12 we can pass in a pre-opened FD for UNIX domain or TCP
sockets. If libvirt has called bind() and listen() on this FD, then we
have a guarantee that libvirt can immediately call connect() and
succeed without any race.
Although we only really care about this for the monitor socket and agent
socket, this patch does FD passing for all UNIX socket based character
devices since there appears to be no downside to it.
We don't do FD passing for TCP sockets, however, because it is only
possible to pass a single FD, while some hostnames may require listening
on multiple FDs to cover IPv4 and IPv6 concurrently.
Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Tue, 5 Jun 2018 07:35:12 +0000 (09:35 +0200)]
qemu: Fix detaching from persistent def in qemuDomainDetachDeviceAliasLiveAndConfig
The code that detaches the device from persistent definition copies the
persistent definition first so that it can easily be rolled back. The
actual detaching is then made in the copy which is assigned back on
success (if the live operation succeeded as well).
This is not the case in qemuDomainDetachDeviceAliasLiveAndConfig where
the definition was copied and put back, but the detaching happened from
the other object which was overwritten.
Now that GnuTLS is a requirement, we can drop a lot of
conditionally built code. However, not all ifdef-s can go because
we still want libvirt_setuid to build without gnutls.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We are building with GnuTLS everywhere because GnuTLS is widely
available. Also, it is desirable to prefer cryptographically
strong PRNG over "/dev/urandom" which is just a fallback.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Tue, 29 May 2018 06:26:18 +0000 (08:26 +0200)]
virrandom: Make virRandomBits better
Now that we have strong PRNG generator implemented in
virRandomBytes() let's use that instead of gnulib's random_r.
Problem with the latter is in way we seed it: current UNIX time
and libvirtd's PID are not that random as one might think.
Imagine two hosts booting at the same time. There's a fair chance
that those hosts spawn libvirtds at the same time and with the
same PID. This will result in both daemons generating the same
sequence of say MAC addresses [1].
To unify our vir*Random() functions we need to make
virCryptoGenerateRandom NOT allocate return buffer. It should
just fill given buffer with random data.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Tue, 29 May 2018 05:50:29 +0000 (07:50 +0200)]
virCryptoGenerateRandom: Explain gnults error
When generating random stream using gnults fails an error is
reported. However, the error is not helpful as it contains only
an integer error code (a negative number). Use gnutls_strerror()
to turn the error code into a string explaining what went wrong.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Tue, 29 May 2018 05:48:02 +0000 (07:48 +0200)]
virCryptoGenerateRandom: rename ret
This function allocates a buffer, fills it in with random bytes
and then returns it. However, the buffer is held in @buf
variable, therefore having @ret variable which does not hold
return value of the function is misleading.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Michal Privoznik [Tue, 29 May 2018 07:02:57 +0000 (09:02 +0200)]
virRandomBytes: Fix return value
In libvirt when a function wants to return an error code it
should be a negative value. Returning a positive value (or zero)
means success. But virRandomBytes() does not follow this rule.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Libvirt relies on being able to kill the destination domain and resume
the source one during migration until we called "cont" on the
destination. Unfortunately, QEMU automatically activates block devices
at the end of migration even when it's called with -S. This wasn't a big
issue in the past since the guest is not running and thus no data are
written to the block devices. However, when QEMU introduced its internal
block device locks, we can no longer resume the source domain once the
destination domain already activated the block devices (and thus
acquired all locks) unless the destination domain is killed first.
Since it's impossible to synchronize the destination and the source
libvirt daemons after a failed migration, QEMU introduced a new
migration capability called "late-block-activate" which ensures QEMU
won't activate block devices until it gets "cont". The only thing we
need to do is to enable this capability whenever QEMU supports it.
When parsing domain XML the virCapsDomainData lookup is performed
in order to fill in missing def->os.arch and def->os.machine
strings. Well, when doing copy of already existing virDomainDef
we don't want any automagic fill in of defaults (and those two
strings are going to be provided at this point anyway by first
parse of the domain XML).
What is even worse is that we do not look up capabilities for
parsed emulator path rather some generic capabilities for parsed
arch. Therefore, if emulator points to qemu under non-default
path (say $HOME/qemu-system-arm) but there's no such qemu under
the default path (say /usr/bin/qemu-system-arm) the capabilities
lookup fails and creating the copy is denied.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 1 Jun 2018 14:57:44 +0000 (16:57 +0200)]
qemu: command: Refactor disk commandline formatting
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>
Peter Krempa [Wed, 16 May 2018 11:39:22 +0000 (13:39 +0200)]
qemu: Reuse qemuBlockStorageSourceAttachApply in disk hotplug
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>
Peter Krempa [Thu, 31 May 2018 11:56:35 +0000 (13:56 +0200)]
qemu: Split handling of managed and unmanaged persistent reservations
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>
Peter Krempa [Thu, 17 May 2018 10:52:02 +0000 (12:52 +0200)]
util: storage: Add helper for determining whether a backing chain requires PR
With blockdev support we will need to introspect whether any of the
backing chain members requires PR rather just one of them. Add a helper
and reuse it in virDomainDefHasManagedPR.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 1 Jun 2018 15:21:02 +0000 (17:21 +0200)]
qemu: hotplug: Fix TLS setup on disk hotplug
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>
Peter Krempa [Thu, 31 May 2018 12:10:47 +0000 (14:10 +0200)]
util: storage: remove virStorageSource->tlsVerify
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>
Peter Krempa [Tue, 22 May 2018 05:38:22 +0000 (07:38 +0200)]
qemu: hotplug: Refactor tls-credential props formatting to qemuMonitorCreateObjectProps
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>
Peter Krempa [Thu, 17 May 2018 14:43:58 +0000 (16:43 +0200)]
qemu: monitor: Add better APIs for adding of objects to qemu
Use the new monitor command internal API to allow wrapping of the object
name and alias into the JSON props so that they don't have to be passed
out of band.
The new API also takes a double pointer so that it can be cleared when
the value is consumed so that it does not need to happen in every single
caller.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 29 May 2018 16:04:42 +0000 (18:04 +0200)]
qemu: command: Don't generate alias for TLS private key password secret
qemuBuildTLSx509CommandLine has no business guessing which alias should
be used. The alias needs to be passed in.
Note that there's a lingering bad design of this, since the secret
object alias is based on the device name and not on the fact that the
secret is used for decrypting of the TLS private key. If we ever add
authentication for chardevs this will bite us.
Thankfully disk code does not support encrypted private keys for TLS so
it can be happily refactored there.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 30 May 2018 10:48:34 +0000 (12:48 +0200)]
qemu: domain: Process only one object in qemuDomainPrepareDiskSourceTLS
Remove the loop from qemuDomainPrepareDiskSourceTLS and rename it to
qemuDomainPrepareStorageSourceTLS. Currently there is no backing chain
to prepare so fixing one device is equivalent. In the future it will be
reused in a function which will do the looping.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 29 May 2018 14:38:50 +0000 (16:38 +0200)]
qemu: domain: aggregate setup of disk drive options for -drive
When using blockdev the approach to base aliases will change. Add a
helper function that will aggregate all code which needs to be called
with the disk alias for the -drive to setup internal data.
qemuDomainSecretDiskPrepare wrapper is no longer necessary as the
contents were moved to a function which is designed to use the old
aliases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 29 May 2018 15:05:05 +0000 (17:05 +0200)]
qemu: domain: don't loop through images in qemuDomainPrepareDiskSourceChain
Convert the function to just prepare data for the disk. Callers need to
do the looping since there's more to do than just copy the data around.
The code path in qemuDomainPrepareDiskSource doesn't need to loop over
the chain yet, since there currently is no chain at this point. This
will be addressed later in the blockdev series where we will setup much
more stuff.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 29 May 2018 14:52:17 +0000 (16:52 +0200)]
qemu: domain: Properly setup data relevant for top disk image
qemuDomainPrepareDiskSourceChain should set up the disk zero detection
mode only for the top level image. Since it's invoked also for the
middle of the chain we need to check that it's really only the top level
image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 30 May 2018 10:18:37 +0000 (12:18 +0200)]
qemu: domain: Regenerate alias for the TLS x509 credential object
When restarting libvirt would previously lose the alias of the x509
certificate object. Upon unplug we would then not delete the
corresponding objects.
Restore the alias if we know it should be there.
Luckily for disks we don't support encrypted TLS environment, so there's
no need to regenerate the 'secret' alias for decryption.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 30 May 2018 10:03:41 +0000 (12:03 +0200)]
qemu: domain: Store and restore TLS object alias of a disk
Libvirt uses the stored alias to detach the TLS x509 object on disk
unplug. As the alias was not stored, the object would not be detached
if unplugging disks after libvirtd restart.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 28 May 2018 13:15:16 +0000 (15:15 +0200)]
qemu: hotplug: Don't try to infer secret object alias/presence
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>
Peter Krempa [Thu, 24 May 2018 16:24:13 +0000 (18:24 +0200)]
qemu: domain: Regenerate auth/enc secret aliases when restoring status XML
Previously we did not store the aliases but rather re-generated them
when unplug was necessary. This is very cumbersome since the knowledge
when and which alias to use needs to be stored in the hotplug code as
well.
While this patch will not strictly improve this situation since there
still will be two places containing this code it at least will allow to
remove the mess from the disk-unplug code and will prevent introducing
more mess when adding blockdev support.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 28 May 2018 13:17:01 +0000 (15:17 +0200)]
qemu: domain: Add helpers for partially clearing qemuDomainSecretInfoPtr
It's desired to keep the alias around to allow referencing of the secret
object used with qemu. Add set of APIs which will destroy all data
except the alias.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
projects / libvirt.git / log