Peter Krempa [Thu, 15 Aug 2019 09:32:28 +0000 (11:32 +0200)]
qemu: monitor: Change fields in qemuBlockStats to 'unsigned'
None of the fields actually return negative values. The internal
implementation of BlockAcctStats struct in qemu uses uint64_t and the
last place using -1 in libvirt was in the HMP monitor code which was
deleted.
Change the internal type to unsigned long long and ensure that all
public conversions don't overflow.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 19 Sep 2019 07:20:49 +0000 (09:20 +0200)]
util: typedparam: Simplify handling of lists of typed parameters
Introduce a new set of helpers including a new data structure which
simplifies keeping and construction of lists of typed parameters.
The use of VIR_RESIZE_N in the virTypedParamsAdd API has performance
benefits but requires passing around 3 arguments. Use of them lead to a
set of macros with embedded jumps used in the qemu statistics code.
This patch introduces 'virTypedParamList' type which aggregates the
necessary list-keeping variables and also a new set of functions to add
new typed parameters to a list.
These new helpers use printf-like format string and arguments to format
the argument name as the stats code often uses indexed typed parameters.
The accessor function then allows extracting the typed parameter list in
the same format as virTypedParamsAdd* functions would do.
One additional benefit is also that the list function can easily be used
with VIR_AUTOPTR.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 14 Aug 2019 12:52:46 +0000 (14:52 +0200)]
util: typedparam: Optionally copy strings passed to virTypedParameterAssignValue
Some code paths already pass in pointers to strings which should be
added directly as the value of the typed parameter. To allow more
universal use of virTypedParameterAssignValue add a flag which allows to
copy the value in place.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 17 Sep 2019 17:21:14 +0000 (19:21 +0200)]
util: typedparam: Split out public APIs into a separate file
Some of the typed parameter APIs are exported publicly, but the
implementation was intermixed with private functions. Introduce
virtypedparam-public.c, move all public API functions there and purge
the comments stating that some functions are public.
This will decrease the likelihood of messing up the expectations as well
as it will become more clear which of them are actually public.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 16 Sep 2019 10:28:48 +0000 (12:28 +0200)]
qemu_blockjob: Remove secdriver metadata for whole backing chain on job completion
Turns out, block mirror is not the only job a disk can have. It
can also do commits of one layer into the other. Or possibly some
other tricks too. Problem is that while we set seclabels on given
layers of backing chain when the job is starting (via
qemuDomainStorageSourceAccessAllow()) we don't restore them when
job finishes. This leaves XATTRs set and corresponding images
unusable.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ACKed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Wed, 25 Sep 2019 07:54:49 +0000 (09:54 +0200)]
domain_conf: Fix str2enum translation of video driver name
In bc1e924cf0d we've introduced video driver name and whilst
doing so we've utilized VIR_ENUM_IMPL() macro. Then, in domain
XML parsing code the generated
virDomainVideoBackendTypeFromString() is called and its return
value is assigned directly to an unsigned int variable which is
wrong. Also, the video driver enum has 'default' value which is
not formatted into domain XML but is accepted during parsing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Tue, 24 Sep 2019 12:39:21 +0000 (14:39 +0200)]
qemu: snapshot: Don't forbid snapshot if autodestroy is registered
Semantically VIR_DOMAIN_START_AUTODESTROY doesn't really clash with
snapshot operations as the VM stays on the same host and thus bound to
the same connection. Saving the state also doesn't differ from modifying
the state of the VM which is allowed.
Remove the check as it doesn't make much sense.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Tue, 24 Sep 2019 12:59:04 +0000 (14:59 +0200)]
qemu: migration: Forbid only remote migration if autodestroy is active for VM
Semantically we can't guarantee that we'll be able to destroy the VM on
the remote host, thus we can't allow remote migration. All other forms
of migration (e.g. saving to file) are okay though as they don't clash
with semantics of the flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Tue, 24 Sep 2019 12:55:15 +0000 (14:55 +0200)]
lib: Lessen restrictions on VIR_DOMAIN_START_AUTODESTROY
Apart from migrating the VM to a remote host where we can't honour the
VIR_DOMAIN_START_AUTODESTROY flag properly, restricting APIs which just
modify the state of the VM does not make much sense.
Change the wording of the documentation for VIR_DOMAIN_START_AUTODESTROY
so that snapshots and saving to a file may be permitted as they
semantically don't clash with the flag itself. Otherwise we'd have to
forbid other APIs, such as virDomainDestroy as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
For each vhost-user GPUs,
- build a socket chardev, and pass the vhost-user socket to it
- build a vhost-user video device and associate it with the chardev
Similar to the qemu_tpm.c, add a unit with a few functions to
start/stop and setup the cgroup of the external vhost-user-gpu
process. See function documentation.
The vhost-user connection fd is set on qemuDomainVideoPrivate struct.
Python3 versions less than 3.7 have very unhelpful handling
of the C locale where they assume data is 7-bit only. This
violates POSIX which requires the C locale to be 8-bit clean.
Python3 >= 3.7 now assumes that the C locale is always UTF-8.
Set env variables to force LC_CTYPE to en_US.UTF-8 so that
we get UTF-8 handling on all python versions. Note we do
not use C.UTF-8 since not all C libraries support that.
Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Mon, 23 Sep 2019 13:48:06 +0000 (15:48 +0200)]
qemu: snapshot: Do ACL check prior to checkpoint interlocking
Commit 7efe930ec3c introduced interlock of snapshots and checkpoints,
but the check is executed prior to the snapshot API ACL check. This
means that an unauthorized user can see whether a VM exists if it has a
checkpoint.
Move the checks to proper places.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
qemu_domain_address: use virPCIDeviceAddressEqual() in conditionals
A common operation in qemu_domain_address is comparing a
virPCIDeviceAddress and assigning domain, bus, slot and function
to a specific value. The former can be done with the existing
virPCIDeviceAddressEqual() helper, as long as we provide
a virPCIDeviceAddress to compare it to.
The later can be done by direct assignment of the now existing
virPCIDeviceAddress struct. The defined values of domain, bus,
slot and function will be assigned to info->addr.pci, the other
values are zeroed (which happens to be their default values too).
It's also worth noticing that all these assignments are being
conditioned by virDeviceInfoPCIAddressIsPresent() calls, thus it's
sensible to discard any non-zero values that might happen to exist
in @cont->info.addr, if we settled beforehand that @cont->info.addr
is not present or bogus.
Suggested-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Laine Stump [Tue, 17 Sep 2019 00:08:00 +0000 (20:08 -0400)]
conf: reattach interface taps to correct bridge on restart
When the bridge re-attach handling was moved out of the network driver
and into the hypervisor driver (commit b806a60e) as a part of the
refactor to split the network driver into a separate daemon, the check
was accidentally changed to only check for type='bridge'. The check for
type in this case needs to check for type='network' as well.
(at the time we thought that the two types could be conflated for
interface actual type, but this turned out to be too problematic to
do).
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The gnulib 'func' modules provides portability to compilers which lack
the '__func__' symbol. We only care about GCC and CLang compilers so do
not need this compatibility code.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Thu, 19 Sep 2019 15:13:31 +0000 (17:13 +0200)]
qemu: monitor: Remove support for HMP commands with fds
The remaining HMP commands don't require fd passing so we can purge
filedescriptor passing support from qemuMonitorJSONHumanCommandWitFd and
rename it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
qemuSharedDeviceEntryRemove: Free domain name before VIR_DELETE_ELEMENT
The macro VIR_DELETE_ELEMENT assume that the items being deleted
have already been cleared, so we must explicitly free domain name
from the list of domains using the shared device to prevent a
memory leak.
Signed-off-by: Xu Yandong <xuyandong2@huawei.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
object_event: reference state only if virEventAddTimeout succeeded
When registering new callback for an event, the event loop timer
must be created and registered. The timer has domain event state
object as an opaque argument which must be ref()-ed but only if
the timer was being created and registered successfully. We must
not ref it every time the virObjectEventStateRegisterID() runs.
Signed-off-by: Xu Yandong <xuyandong2@huawei.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Peter Krempa [Wed, 18 Sep 2019 06:27:41 +0000 (08:27 +0200)]
Revert "configure: Colorize output"
The colors are not based on the semantics of the message but rather
on the message itself. This means that the default human-perceived
semantics (red = bad, green = good) don't really apply and spotting a
color does not mean anythting.
This is amplified by the sheer amount of output which configure produces
and the fact that some of the messages have negative semantics or
additional output.
In case of any problem the user will have to go through everything
anyways as spotting a red or yellow line has 0 information value.
Here are a few examples:
1) some 'no' messages are not a problem:
checking minix/config.h presence... no
2) some 'no' messages are actually positive:
checking for special C compiler options needed for large files... no
3) in some cases a 'yes' would mean that something is broken or needs
workaround
checking whether stat file-mode macros are broken... no
checking whether wint_t is too small... no
checking whether stdint.h predates C++11... no
checking whether the inttypes.h PRIxNN macros are broken... no
checking whether clang gives bogus warnings for -Wdouble-promotion... no
checking whether gettimeofday clobbers localtime buffer... no
4) due to string match based colors extra text makes messages yellow
checking for a traditional french locale... none
checking for working nanosleep... no (mishandles large arguments)
checking for library containing gethostbyname... none required
checking whether mbrtowc handles incomplete characters... (cached) guessing yes
5) in some cases the yes/no is very context dependant
checking whether pthread_rwlock_rdlock prefers a writer to a reader... no
checking whether this build is done by a static analysis tool... no
6) detected paths to binaries and libs are yellow despite being present
checking for objdump... objdump
checking for atomic ops implementation... gcc
As of the reasons above I don't think the colorization of the configure
output helps users or developers to debug the build process and
thus is not worth the extra code or output clutter.
The colorization based on the string itself makes little to no sense as
the semantic meaning of the color (red = bad, green = good) is not
extracted from the semantics of the message:
1) If there is some additional string a 'yes' is marked yellow:
Peter Krempa [Mon, 9 Sep 2019 08:55:20 +0000 (10:55 +0200)]
virsh: Don't open-code virJSONStringReformat in cmdQemuMonitorCommand
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 9 Sep 2019 08:51:25 +0000 (10:51 +0200)]
virsh: Use VIR_AUTO machinery in cmdQemuMonitorCommand
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 9 Sep 2019 08:41:15 +0000 (10:41 +0200)]
virsh: Use virshDomain type in 'inject-nmi'
With a nice side-effect of fixing alignment.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 9 Sep 2019 08:39:04 +0000 (10:39 +0200)]
virsh: demonstrate use of VIR_AUTOPTR(virshDomain) on 'send-process-signal'
Refactor the command code to use the new type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 9 Sep 2019 08:36:39 +0000 (10:36 +0200)]
virsh: Allow using VIR_AUTOPTR for releasing virDomainPtr in virsh
I opted to alias the 'virDomainType' to 'virshDomain' so that it's
obvious in all cases that this is a virsh-only construct. This is also
somewhat consistent with virsh's use of 'virshDomainFree' wrapper for
the freeing function which actually accepts NULL.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove windows thread implementation in favour of pthreads
Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The fact that qemu is capable -netdev socket is not enough to
start a migratable domain. It also needs dbus-vmstate capability.
Since there are already some qemu releases which have
net-socket-dgram capability and don't have dbus-vmstate we need
to check for dbus-vmstate.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
remote: pass identity across to newly opened daemons
When opening a connection to a second driver inside the daemon, we must
ensure the identity of the current user is passed across. This allows
the second daemon to perform access control checks against the real end
users, instead of against the libvirt daemon that's proxying across the
API calls.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: store identity attrs as virTypedParameter internally
We'll shortly be exposing the identity as virTypedParameter in the
public header, so it simplifies life to use that as the internal
representation too.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: sanitize return values for virIdentity getters
The virIdentity getters are unusual in that they return -1 to indicate
"not found" and don't report any error. Change them to return -1 for
real errors, 0 for not found, and 1 for success.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>