Laine Stump [Thu, 17 Jan 2019 20:05:54 +0000 (15:05 -0500)]
qemu: fix i6300esb watchdog hotplug on Q35
When commit 361c8dc17 added support for hotplugging the i6300esb
watchdog device (first in libvirt-3.9.0), it accidentally contstructed
the commandline for the device_add command before allocating a PCI
address for the device. With no PCI address specified in the command,
the watchdog would simply be placed at the lowest unused PCI slot.
On a 440fx guest, this doesn't cause a problem, because libvirt's PCI
address allocation algorithm would most likely give the same address
anyway (usually a slot on pci-root), so nobody noticed the omission of
address from the command.
But on a Q35 guest, the lowest unused PCI slot is on pcie-root, which
doesn't support hotplug; libvirt knows enough to assign a PCI address
that is on a pcie-to-pci-bridge (because its slots *do* support
hotplug), but qemu doesn't, so if there is no PCI address in the
command, qemu just tries to plug the new device into pcie-root, and
fails because it doesn't support hotplug, e.g.:
error: Failed to attach device from watchdog.xml
error: internal error: unable to execute QEMU command 'device_add':
Bus 'pcie.0' does not support hotplugging
The solution is simply to build the command string after assigning a
PCI address, not before.
Resolves: https://bugzilla.redhat.com/1666559 Signed-off-by: Laine Stump <laine@laine.org> Reviewed-by: John Ferlan <jferlan@redhat.com>
Wang Yechao [Mon, 17 Dec 2018 11:30:34 +0000 (19:30 +0800)]
qemu: Assign device addresses earlier in qemuDomainAttachNetDevice
If code in the @actualType switch needs to have/know which PCI
Address is being used, then we must assign it earlier. In particular
a vhost-user device needs to call qemuDomainSupportsNicdev which
requires an address to be defined.
Signed-off-by: Wang Yechao <wang.yechao255@zte.com.cn> Reviewed-by: John Ferlan <jferlan@redhat.com>
rpc: virNetClientNew: fix socket leak on error path
if virNetClientNew finishes with error before sock is set
to client object then sock does not get unrefed. This is
unexpected by function clients like virNetClientNewUNIX.
Let's make sure sock gets unrefed on any error path.
Next some clients like virNetClientNewLibSSH2 try to unref
sock on virNetClientNew errors. This is not correct even
before this patch because in some cases virNetClientNew
unrefed sock on error path by itself. Let's give up
sock managment to virNetClientNew entirely.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Thu, 17 Jan 2019 16:01:55 +0000 (17:01 +0100)]
qemu: blockjob: Add job name into the data
Currently the job name corresponds to the disk the job belongs to. For
jobs which will not correspond to disks we'll need to track the name
separately.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 17 Oct 2018 15:22:38 +0000 (17:22 +0200)]
qemu: Allocate diskPriv->blockjob only when there's a blockjob
Rather than storing the presence of the blockjob in a flag we can bind
together the lifecycle of the job with the lifecycle of the object which
is tracking the data for it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 26 Nov 2018 14:29:55 +0000 (15:29 +0100)]
qemu: blockjob: Consume new block job state in the processing function
The processing function modifies the job state so it should make sure
that the variable holding the new state is cleared properly and not the
caller. The caller should only deal with the job state and not the
transition that happened.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 17 Jan 2019 15:34:11 +0000 (16:34 +0100)]
qemu: blockjob: Track current state of blockjob
Add a field tracking the current state of job so that it can be queried
later. Until now the job state e.g. that the job is _READY for
finalizing was tracked only for mirror jobs. Add tracking of state for
all jobs.
Similarly to 'qemuBlockJobType' this maps the existing states of the
blockjob from virConnectDomainEventBlockJobStatus to
'qemuBlockJobState' so that we can track some internal states as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 23 Nov 2018 11:45:32 +0000 (12:45 +0100)]
qemu: blockjob: Record job type when starting the job
We can properly track the job type when starting the job so that we
don't have to infer it later.
This patch also adds an enum of block job types specific to qemu
(qemuBlockjobType) which mirrors the public block job types
(virDomainBlockJobType) but allows for other types to be added later
which will not be public.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 21 Nov 2018 14:36:47 +0000 (15:36 +0100)]
qemu: blockjob: Add reference to disk into struct qemuBlockJobData
Block jobs can also happen on objects which are not a disk at a given
point (e.g. the frontend was not hotplugged yet) and thus will be
eventually kept separately. Add a reference back to the disk for
blockjobs which do correspond to a disk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 19 Nov 2018 15:48:09 +0000 (16:48 +0100)]
qemu: blockjob: Add functions for block job state control
Rather than directly modifying fields in the qemuBlockJobDataPtr
structure add a bunch of fields which allow to do the transitions.
This will help later when adding more complexity to the job handling.
APIs introduced in this patch are:
qemuBlockJobDiskNew - prepare for starting a new blockjob on a disk
qemuBlockJobDiskGetJob - get the block job data structure for a disk
For individual job state manipulation the following APIs are added:
qemuBlockJobStarted - Sets the job as started with qemu. Until that
the job can be cancelled without asking qemu.
qemuBlockJobStartupFinalize - finalize job startup. If the job was
started in qemu already, just releases
reference to the job object. Otherwise
clears everything as if the job was never
started.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 22 Nov 2018 14:05:50 +0000 (15:05 +0100)]
qemu: blockjob: Clarify that job 'status' field contains new state
The field is used to note the state the job has transitioned to while
handling the blockjob state change event. Rename the field so that it's
obvious that this is the new state and not the general state of the
blockjob.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 12 Nov 2018 06:46:49 +0000 (07:46 +0100)]
qemu: driver: Remove block job status reprobing from qemuDomainBlockPivot
Now that we reprobe the status of blockjobs when reconnecting in
addition to handling job status events, the status reprobing can be
removed as we always track the correct status internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 18 Oct 2018 10:34:49 +0000 (12:34 +0200)]
qemu: process: refresh block jobs on reconnect
Block job state was widely untracked by libvirt across restarts which
was allowed by a stateless block job finishing handler which discarded
disk state and redetected it. This is undesirable since we'll need to
track more information for individual blockjobs due to -blockdev
integration requirements.
In case of legacy blockjobs we can recover whether the job is present at
reconnect time by querying qemu. Adding tracking whether a job is
present will allow simplification of the non-shared-storage cancellation
code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 18 Oct 2018 11:11:08 +0000 (13:11 +0200)]
qemu: migration: Properly note that non-shared-storage migration uses a blockjob
Internally we do a 'block-copy' to accomodate non-shared storage
migration but the code did not fill in that the block job was active on
the disk when starting the copy job. Since we handle block jobs finishes
regardless of having it registered it's not a problem but soon will
become one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 16 Oct 2018 14:43:49 +0000 (16:43 +0200)]
qemu: blockjob: Remove header dependency on qemu_domain.h
The blockjob module uses 'qemuDomainAsyncJob' in it's public headers.
As I plan adding a new structure containing job data which will need to
be included in "qemu_domain.h" it's necessary to break the circular
dependency.
Convert 'qemuDomainAsyncJob' type to 'int' as it's an enum.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 16 Oct 2018 14:36:18 +0000 (16:36 +0200)]
qemu: blockjob: Rename public APIs
All the public APIs of the qemu_blockjob module operate on a 'disk'.
Since I'll be adding APIs which operate on a job later let's rename the
existing ones.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 16 Oct 2018 09:47:32 +0000 (11:47 +0200)]
qemu: processBlockJobEvent: Use qemuBlockJobUpdate to process block job events
Replace use of qemuBlockJobEventProcess with the general helper. A small
tweak is required to pass in the 'type' and 'status' of the job via the
appropriate private data variables.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 17 Jan 2019 12:52:09 +0000 (13:52 +0100)]
qemu: blockjob: Emit VIR_DOMAIN_EVENT_ID_BLOCK_JOB only for local disks
The event reports the disk path to identify the disk which makes sense
only for local disks. Additionally network backed disks like NBD don't
need to have a path so the callback would return NULL.
Report VIR_DOMAIN_EVENT_ID_BLOCK_JOB only for non-empty local disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ján Tomko [Mon, 14 Jan 2019 22:06:06 +0000 (23:06 +0100)]
qemu: fill out usage-specific TLS settings after parsing
Instead of copying the default default values upfront
and then wondering whether the user has given us a new default,
leave the per-usage TLS certdirs and secrets empty during
parsing and only fill them afterwards if they weren't provided
by the user.
This means that instead of looking whether the specific certdir
paths match the default default, the Validate function (which
is called in between parsing and setting the defaults) can error
out for missing directories if the value is present, because
it must've come from the user.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Andrea Bolognani [Fri, 11 Jan 2019 10:03:53 +0000 (11:03 +0100)]
src: Don't use double-colon rules
According to the GNU Make manual, "double-colon rules are
somewhat obscure and not often very useful". Looking at
the few instances we have in libvirt, that certainly seems
to be the case, so just drop them.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Michal Privoznik [Mon, 14 Jan 2019 16:53:43 +0000 (17:53 +0100)]
qemu: Temporary disable owner remembering
Turns out, that there are few bugs that are not that trivial to
fix (e.g. around block jobs). Instead of rushing in not
thoroughly tested fixes disable the feature temporarily for the
release.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ACKed-by: Peter Krempa <pkrempa@redhat.com>
Laine Stump [Sat, 12 Jan 2019 21:14:50 +0000 (16:14 -0500)]
util: make forgotten changes suggested during review of commit d40b820c
I had intended to make these changes to commit d40b820c before
pushing, but forgot about it during the day between the initial review
and ACK.
Neither change is significant - just returning immediately when
virNetDevGetName() fails (instead of logging a debug message first)
and eliminating a comment that adds to confusion rather than
eliminating it. Still, the changes should be made to be more
consistent with nearly identical code just a few lines up (added in
commit 7282f455)
Signed-off-by: Laine Stump <laine@laine.org> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Mon, 14 Jan 2019 13:32:31 +0000 (14:32 +0100)]
news: Move entry for libvirt picking DRI devices
The entry, introduced by commit 3934beb8579c, ended up
inside a comment instead of the XML document proper, and
as such didn't show up in the generated files.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Laine Stump [Tue, 8 Jan 2019 22:07:12 +0000 (17:07 -0500)]
util: check accept_ra for all nexthop interfaces of multipath routes
When checking the setting of accept_ra, we have assumed that all
routes have a single nexthop, so the interface of the route would be
in the RTA_OIF attribute of the netlink RTM_NEWROUTE message. But
multipath routes don't have an RTA_OIF; instead, they have an
RTA_MULTIPATH attribute, which is an array of rtnexthop, with each
rtnexthop having an interface. This patch adds a loop to look at the
setting of accept_ra of the interface for every rtnexthop in the
array.
Signed-off-by: Laine Stump <laine@laine.org> Reviewed-by: Erik Skultety <eskultet@redhat.com>
John Ferlan [Thu, 10 Jan 2019 23:05:12 +0000 (18:05 -0500)]
qemu: Filter non SCSI hostdevs in qemuHostdevPrepareSCSIDevices
When commit 1d94b3e7 added code to walk the [n]hostdevs list looking
to add shared hostdevs, it should've filtered any hostdevs that were
not SCSI hostdev's.
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Sun, 6 Jan 2019 22:35:47 +0000 (17:35 -0500)]
util: remove const specifier from nlmsghdr arg to virNetlinkDumpCallback()
This is problematic if a callback function wants to send the nlmsghdr
to a library function that has no "const" in its prototype
(e.g. nlmsg_find_attr())
Signed-off-by: Laine Stump <laine@laine.org> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Instead of defining targets conditionally and depending on
them unconditionally, define a couple of variables and
conditionally add targets to them.
In addition to removing a bunch of useless code, this has
the nice effect of no longer requiring the main Makefile.am
to have any knowledge about the contents of the various
snippets it includes.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Jiri Denemark [Thu, 10 Jan 2019 13:35:07 +0000 (14:35 +0100)]
cpu_map: Add support for arch-capabilities feature
The feature was added to QEMU in 3.1.0 and it is currently blocking
migration, which is expected to change in the future. Luckily 3.1.0 is
new enough to give us migratability hints on each feature via
query-cpu-model-expension, which means we don't need to use the
"migratable" attribute on the CPU map XML.
The kernel calls this feature arch_capabilities and RHEL/CentOS 7.* use
arch-facilities. Apparently some CPU test files were gathered with the
RHEL version of QEMU. Let's update the test files to avoid possible
confusion about the correct naming.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Thu, 10 Jan 2019 12:53:33 +0000 (13:53 +0100)]
qemu: Don't enable seclabel remembering for session mode
The session daemon is unable to set XATTRs in 'trusted'
namespace because it doesn't run as privileged process.
Therefore, when creating the default qemu config enable
rememberOwner only when running as privileged process.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ján Tomko [Fri, 4 Jan 2019 09:17:46 +0000 (10:17 +0100)]
qemu: require reply from guest agent in qemuAgentGetInterfaces
Since its introduction in commit 0977b8aa071 (released in v1.2.14)
qemuAgentGetInterfaces calls qemuAgentCommand with needReply=false,
which allows qemuAgentCommand to return 0 even when it did not get
any reply from the agent.
Set needReply to true, since we dereference it right after.
This can be hit if libvirt is waiting for an event from the agent
(e.g. shutdown) and the agent cannot reply in time (e.g. due to
the guest being shut down), as reported in:
https://bugzilla.redhat.com/show_bug.cgi?id=1663051
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
qemu: Use ULL instead of uint64_t for RDMA GID event
In the previous commit we are using uint64_t for storing subnet
prefix and interface id that qemu reports in
RDMA_GID_STATUS_CHANGED event. We also report them in some debug
messages. This poses a problem because uint64_t can be UL or ULL
depending on the host architecture and hence we wouldn't know
which format to use. Switch to ULL which is big enough and
doesn't suffer from the issue.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Yuval Shaia [Mon, 24 Dec 2018 10:15:12 +0000 (12:15 +0200)]
qemu: Process RDMA GID state change event
This event is emitted on the monitor when a GID table in pvrdma device
is modified and the change needs to be propagate to the backend RDMA
device's GID table.
The control over the RDMA device's GID table is done by updating the
device's Ethernet function addresses.
Usually the first GID entry is determine by the MAC address, the second
by the first IPv6 address and the third by the IPv4 address. Other
entries can be added by adding more IP addresses. The opposite is the
same, i.e. whenever an address is removed, the corresponding GID entry
is removed.
The process is done by the network and RDMA stacks. Whenever an address
is added the ib_core driver is notified and calls the device driver's
add_gid function which in turn update the device.
To support this in pvrdma device we need to hook into the create_bind
and destroy_bind HW commands triggered by pvrdma driver in guest.
Whenever a changed is made to the pvrdma device's GID table a special
QMP messages is sent to be processed by libvirt to update the address of
the backend Ethernet device.
Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Eric Blake [Tue, 8 Jan 2019 19:41:28 +0000 (13:41 -0600)]
examples: Work around lack of mingw localtime_r()
mingw lacks localtime_r(); we were getting it from gnulib. But since
commit acf522e8 stopped linking examples against gnulib, we are
getting a build failure. Keep the examples standalone, and work
around mingw by using the non-reentrant localtime() (safe since our
examples are single-threaded), and add a necessary exemption to our
syntax check.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Eric Blake [Tue, 8 Jan 2019 19:46:48 +0000 (13:46 -0600)]
examples: Work around lack of mingw sigaction()
mingw lacks sigaction(); we were getting it from gnulib. But since
commit acf522e8 stopped linking examples against gnulib, we are
getting a build failure. Keep the examples standalone, and work
around mingw by using signal() instead.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Eric Blake [Tue, 8 Jan 2019 19:46:48 +0000 (13:46 -0600)]
examples: Work around mingw printf() weakness
mingw lacks %lld and %zu support in printf(); we were getting it
from gnulib. But since commit acf522e8 stopped linking examples
against gnulib, we are getting a build failure due to -Wformat
flagging these strings. Keep the examples standalone, and work
around mingw by using manual casts to types we can portably print.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
syntax-check: Simplify and fix unmarked_diagnostics
The check was concerning itself with whitespace where it
didn't need to, and used some confusing escaping for one
of its regular expressions - which GNU sed was fine with,
but FreeBSD's sed didn't like one bit.
Switch to extended regular expressions (which, incidentally,
were already in use in the same rule when calling grep) and
remove all whitespace handling.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Eric Blake [Tue, 8 Jan 2019 04:15:48 +0000 (22:15 -0600)]
examples: Avoid gnulib, have standalone examples
Commit 0c6ad476 updated gnulib, which rearranged some of the
conditions in gnulib wrapper headers such that compilation
started failing on BSD systems when the normal system <unistd.h>
tried to include another system header but instead got a
gnulib wrapper header in an incomplete state; this is because
gnulib headers only work if <config.h> is included first.
Commit b6f78259 papered over the symptoms of that by including
<config.h> in all the examples. But this logic is backwards -
if our examples are truly meant to be stand-alone, they should
NOT depend on how libvirt was configured, and should NOT
depend on the gnulib fixes for system quirks. In particular,
if an example does not need to link against libgnulib.la,
then it also does not need to use -Ignulib in its compile
flags, and likewise does not need to include <config.h> since
none of the gnulib wrapper headers should be interfering.
So, revert (most of) b6f78259 (except for the bogus pre-patch
use of "config.h" in admin/logging.c: if config.h is included,
it should be via <> rather than "", and must be before any
system headers); then additionally nuke all mention of
<config.h>, -Ignulib, and -llibgnu.la, making all of the
examples truly standalone.
Signed-off-by: Eric Blake <eblake@redhat.com> Acked-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Eric Blake [Tue, 8 Jan 2019 05:23:52 +0000 (23:23 -0600)]
examples: Drop event-test.c dependency on gnulib <verify.h>
Pulling in gnulib just for the <verify.h> header is rather
expensive, especially since that header does not require us
to link against gnulib. It's better to make the event-test
example be standalone by just open-coding a more limited form
of a verify() macro that depends on modern gcc (we have enough
CI coverage that even though the verify is now a no-op in
older setups, we will still notice if we fail to add an event
- as a quick test, I was still able to provoke a compile
failure on Fedora 29 when deleting a line from domainEvents).
Signed-off-by: Eric Blake <eblake@redhat.com> Acked-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Erik Skultety [Thu, 3 Jan 2019 13:32:36 +0000 (14:32 +0100)]
util: Fix the default log output to 'journald' when running under systemd
Essentially, bring back the old behaviour as of commit eba36a38 which
was later changed by commit ae06048bf5d. Even though all the stderr
messages will eventually end up in the journal, we're not making use of
the fields journald provides.
Eric Blake [Tue, 8 Jan 2019 03:50:08 +0000 (21:50 -0600)]
maint: Fix VPATH build
In a VPATH build, <config.h> is in the builddir (which automake
includes automatically), but it includes <config-post.h> from the
top source directory (which is not automatic); hence, we need to
keep the -I(top_srcdir) directive that was accidentally removed
from commit 7a879323 (the problem is not visible in an in-tree
build).
projects / libvirt.git / log