The result of libssh2_userauth_password is being assigned to 'ret' in
one branch and 'rc' in the other branch. Checks are all done against the
'ret' variable, so one branch never does the correct check.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Boris Fiuczynski [Thu, 18 Oct 2018 14:54:07 +0000 (16:54 +0200)]
qemu: vfio-ap device support
Adjusting domain format documentation, adding device address
support and adding command line generation for vfio-ap.
Since only one mediated hostdev with model vfio-ap is supported a check
disallows to define domains with more than one such hostdev device.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Chris Venteicher <cventeic@redhat.com>
Boris Fiuczynski [Thu, 18 Oct 2018 14:54:06 +0000 (16:54 +0200)]
qemu: add vfio-ap capability
Introduce vfio-ap capability.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Chris Venteicher <cventeic@redhat.com>
Jie Wang [Wed, 17 Oct 2018 17:14:42 +0000 (01:14 +0800)]
qemu: Fix IOThread pids lost after qemuProcessReconnect
IOThread pids info will lost after libvirtd restart, then
if we call pinIOThread, sched_setaffinity will be called with
pid 0, not IOThread pid. So pinIOThread cannot work normally.
Signed-off-by: Jie Wang <wangjie88.huawei.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Eric Blake [Thu, 18 Oct 2018 20:05:22 +0000 (15:05 -0500)]
qemu: Avoid memleak on failure to format blockjobs
virXMLFormatElement() frees attrBuf on success, but not necessarily
on failure. Most other callers of this function take the time to
reset attrBuf afterwords, but qemuDomainObjPrivateXMLFormatBlockjobs()
was relying on it succeeding, and could thus result in a memory leak.
Signed-off-by: Eric Blake <eblake@redhat.com> ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Weirdly enough, there can be symlinks in the path we are trying
to fix. If it is the case our clever algorithm that finds matches
against mount table won't work. Canonicalize path at the
beginning then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
The virFileInData() function should return to the caller if the
current position the passed file is in is a data section or a
hole (and also how long the current section is). At any rate,
upon return from this function (be it successful or not) the
original position in the file is restored. This may mess up with
errno which might have been set earlier. Save the errno into a
local variable so it can be restored for the caller's sake.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
If the learning thread is configured to learn on all ethernet frames
(which is hardcoded) then chances are high that there is a packet on
every iteration of inspecting frames loop. As result we will hang on
shutdown because we don't check threadsTerminate if there is packet.
Let's just check termination conditions on every iteration. Since
we'll check each iteration, the check after pcap_next essentially
is unnecessary since on failure we'd loop back to the top and timeout
and then fail.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
When doing a SCSI passthrough we don't put format= onto the
command line. This causes qemu to probe the format automatically
which ends up in a warning in the domain log and possible qemu
disabling writes to the first block (according to the warning
message).
Based-on-work-of: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Ján Tomko [Thu, 4 Oct 2018 16:24:53 +0000 (18:24 +0200)]
tests: add smartcard-passthrough-unix
Test CCID smartcard passthrough from a unix listen socket.
Use the capabilities of QEMU 2.5.0 which did not support
chardev FD passing and the latest one, which (at the time
of this commit) it does.
Peter Krempa [Thu, 11 Oct 2018 10:22:21 +0000 (12:22 +0200)]
util: storage: Properly parse URIs with missing trailing slash
The URI parser used by libvirt does not populate uri->path if the
trailing slash is missing. The code virStorageSourceParseBackingURI
would then not populate src->path.
As only NBD network disks are allowed to have the 'name' field in the
XML defining the disk source omitted we'd generate an invalid XML which
we'd not parse again.
Fix it by populating src->path with an empty string if the uri is
lacking slash.
As pointed out above NBD is special in this case since we actually allow
it being NULL. The URI path is used as export name. Since an empty
export does not make sense the new approach clears the src->path if the
trailing slash is present but nothing else.
Add test cases now to cover all the various cases for NBD and non-NBD
uris as there was to time only 1 test abusing the quirk witout slash for
NBD and all other URIs contained the slash or in case of NBD also the
export name.
Eric Blake [Thu, 11 Oct 2018 16:29:58 +0000 (11:29 -0500)]
virsh: Fix regression with duplicated error messages
Commit 4f4c3b13 (v3.3) fixed an issue where performing cleanup of
libvirt objects could sometimes lose error messages, by adding code
to copy the libvirt error into last_error prior to cleanup paths.
However, it caused a regression: on other paths, some errors are now
printed twice, if libvirt still remembers in its thread-local
storage that an error was set even after virsh cleared last_error.
For example:
$ virsh -c test:///default snapshot-delete test blah
error: Domain snapshot not found: no domain snapshot with matching name 'blah'
error: Domain snapshot not found: no domain snapshot with matching name 'blah'
Fix things by telling libvirt to discard any thread-local errors at
the same time virsh prints an error message (whether or not the libvirt
error is the same as what is stored in last_error).
Update the virsh-undefine testsuite (partially reverting portions of
commit b620bdee, by removing -q, to more easily pinpoint which commands
are causing which messages), now that there is only one error message
instead of two.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Olaf Hering [Wed, 10 Oct 2018 09:15:37 +0000 (11:15 +0200)]
rpc: reproducible genprotocol output
If the same source gets built twice ('build same source on different
hosts at different times') the resulting files may differ.
Fix this by sorting the hash keys before usage.
There are couple of things wrong with the current implementation.
The first one is that in the first loop the code tries to build a
list of fuse.glusterfs mount points. Well, since the strings are
allocated in a temporary buffer and are not duplicated this
results in wrong decision made later in the code.
The second problem is that the code does not take into account
subtree mounts. For instance, if there's a fuse.gluster mounted
at /some/path and another FS mounted at /some/path/subdir the
code would not recognize this subdir mount.
Reported-by: Han Han <hhan@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
If the given path is already a mount point (e.g. a bind mount of
a file, or simply a direct mount point of a FS), then our code
fails to detect that because the first thing it does is cutting
off part after last slash '/'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Introduce some basic test cases for virFileIsSharedFS(). More
will be added later. In order to achieve desired result, mocks
for setmntent() and statfs() need to be invented because the
first thing that virFileIsSharedFS() does is calling the latter.
If it finds a FUSE mount it'll call the former.
The mock might look a bit complicated, but in fact it's quite
simple. The test sets LIBVIRT_MTAB env variable to hold the
absolute path to a file containing mount table. Then, statfs()
returns matching FS it finds, and setmntent() is there just to
replace /proc/mounts with the file the test wants to load.
Adding this test also exposed a bug we have - because we assume
the given path points to a file we cut off what we assume is a
file name to obtain directory path and only then we call
statfs(). This is buggy because the passed path could be already
a mount point.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Marc Hartmayer [Wed, 10 Oct 2018 08:04:04 +0000 (10:04 +0200)]
virfile: fix cast-align error
On s390x the struct member f_type of statsfs is hard coded to 'unsigned
int'. Change virFileIsSharedFixFUSE() to take a 'long long int' and use
a temporary to avoid pointer-casting.
This fixes the following error:
../../src/util/virfile.c:3578:38: error: cast increases required alignment of target type [-Werror=cast-align]
virFileIsSharedFixFUSE(path, (long *) &sb.f_type);
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com>
We now explicitly handle media change elsewhere so we can drop the
switch statement. This will also make it more intuitive once CDROM
device hotplug might be supported.
Peter Krempa [Tue, 25 Sep 2018 12:47:36 +0000 (14:47 +0200)]
qemu: hotplug: Split out media change code from disk hotplug
Disk hotplug has slightly different semantics from media changing. Move
the media change code out and add proper initialization of the new
source object and proper cleanups if something fails.
Peter Krempa [Thu, 27 Sep 2018 14:50:55 +0000 (16:50 +0200)]
qemu: hotplug: Prepare disk source for media changing
The disk storage source needs to be prepared if we want to use -blockdev
or secrets for the new media image. It does not hurt to do the same for
the legacy hotplug code as well.
Unfortunately helpers like qemuDomainPrepareDiskSource take
virDomainDiskDef as an argument and it would be hard to fix them to take
an explicit source, so the function also temporarily replaces disk->src
for the new source in this function.
Peter Krempa [Thu, 4 Oct 2018 15:56:46 +0000 (17:56 +0200)]
qemu: hotplug: Be explicit about old/new sources when changing media
Some functions require us to replace disk->src with the new source for
them to work properly. To avoid confusion all places which allow
explicit virStorageSource should get the appropriate definition.
The legacy code fortunately does not need anything from the old source
so that does not require modifications.
Blockdev does require the old definition so we'll pass it explicitly.
Peter Krempa [Thu, 4 Oct 2018 15:49:00 +0000 (17:49 +0200)]
qemu: hotplug: Allow specifying explicit source for disk backend hotplug code
Since the code is also used when changing media we need to allow
specifying explicit source for which we are going to prepare. With this
change callers don't have to replace disk->src with the new source
definition for generating these.
Peter Krempa [Thu, 4 Oct 2018 13:32:37 +0000 (15:32 +0200)]
qemu: hotplug: Remove code handling possible missing disk source format
qemu media changing code tried to assume old media's format for the new
one if that was not specified. Since the format will always be present
it does not make sense to keep the code around.
Peter Krempa [Tue, 25 Sep 2018 12:21:27 +0000 (14:21 +0200)]
Revert "qemu: hotplug: consolidate media change code paths"
While the idea was good the implementation not so much as we need to
take into account the old disk data and the new source. The code will be
consolidated later in a different way.
Peter Krempa [Mon, 24 Sep 2018 14:49:01 +0000 (16:49 +0200)]
Revert "qemu: hotplug: Prepare disk source in qemuDomainAttachDeviceDiskLive"
Preparing the storage source prior to assigning the alias will not work
as the names of the certain objects depend on the alias for the legacy
hotplug case as we generate the object names for the secrets based on
the alias.
For non-Linux platforms we have
virHostValidateCGroupControllers() stub which only reports an
error. But we are not marking the ignored arguments the way we
should.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Pavel Hrdina [Sat, 29 Sep 2018 19:37:22 +0000 (21:37 +0200)]
virt-host-validate: rewrite cgroup detection to use util/vircgroup
This removes code duplication and simplifies cgroup detection.
As a drawback we will not have separate messages to enable cgroup
controller in kernel or to mount it. On the other side the rewrite
adds support for cgroup v2.
The kernel config support was wrong because it was parsing
'/proc/self/cgroup' instead of '/proc/cgroups/' file.
The mount suggestion is removed as well because it will not work
with cgroup v2.
Pavel Hrdina [Tue, 18 Sep 2018 10:33:21 +0000 (12:33 +0200)]
vircgroupmock: add support to test cgroup v2
We need to create the cgroup v2 sysfs the same way as we do for
cgroup v1.
This introduces new VIR_CGROUP_MOCK_MODE env variable which will
configure which cgroup mode each test requires. There are three
different modes:
- legacy: only cgroup v1 is available and it's the default mode
- hybrid: both cgroup v1 and cgroup v2 are available and have some
controllers
- unified: only cgroup v2 is available
Pavel Hrdina [Tue, 18 Sep 2018 07:28:54 +0000 (09:28 +0200)]
vircgroupmock: change cgroup prefix
Remove the trailing '/' from prefix. This change is required in order
to introduce tests for unified cgroups. They are usually mounted in
'/sys/fs/cgroup'.
Pavel Hrdina [Fri, 28 Sep 2018 17:53:05 +0000 (19:53 +0200)]
vircgroup: add support for hybrid configuration
This enables to use both cgroup v1 and v2 at the same time together
with libvirt. It is supported by kernel and there is valid use-case,
not all controllers are implemented in cgroup v2 so there might be
configurations where administrator would enable these missing
controllers in cgroup v1.
In order to set CPU cfs period using cgroup v2 'cpu.max' interface
we need to load the current value of CPU cfs quota first because
format of 'cpu.max' interface is '$quota $period' and in order to
change 'period' we need to write 'quota' as well. Writing only one
number changes only 'quota'.
Pavel Hrdina [Fri, 17 Aug 2018 14:49:33 +0000 (16:49 +0200)]
vircgroup: introduce virCgroupV2AddTask
In cgroups v2 we need to handle threads and processes differently.
If you need to move a process you need to write its pid into
cgrou.procs file and it will move the process with all its threads
as well. The whole process will be moved if you use tid of any thread.
In order to move only threads at first we need to create threaded group
and after that we can write the relevant thread tids into cgroup.threads
file. Threads can be moved only into cgroups that are children of
cgroup of its process.
Pavel Hrdina [Tue, 18 Sep 2018 15:49:19 +0000 (17:49 +0200)]
vircgroup: introduce virCgroupV2MakeGroup
When creating cgroup hierarchy we need to enable controllers in the
parent cgroup in order to be usable. That means writing "+{controller}"
into cgroup.subtree_control file. We can enable only controllers that
are enabled for parent cgroup, that means we need to do that for the
whole cgroup tree.
Cgroups for threads needs to be handled differently in cgroup v2. There
are two types of controllers:
- domain controllers: these cannot be enabled for threads
- threaded controllers: these can be enabled for threads
In addition there are multiple types of cgroups:
- domain: normal cgroup
- domain threaded: a domain cgroup that serves as root for threaded
cgroups
- domain invalid: invalid cgroup, can be changed into threaded, this
is the default state if you create subgroup inside
domain threaded group or threaded group
- threaded: threaded cgroup which can have domain threaded or
threaded as parent group
In order to create threaded cgroup it's sufficient to write "threaded"
into cgroup.type file, it will automatically make parent cgroup
"domain threaded" if it was only "domain". In case the parent cgroup
is already "domain threaded" or "threaded" it will modify only the type
of current cgroup. After that we can enable threaded controllers.
Pavel Hrdina [Tue, 25 Sep 2018 12:37:21 +0000 (14:37 +0200)]
vircgroup: introduce virCgroupV2DetectControllers
Cgroup v2 has only single mount point for all controllers. The list
of controllers is stored in cgroup.controllers file, name of controllers
are separated by space.
In cgroup v2 there is no cpuacct controller, the cpu.stat file always
exists with usage stats.
Pavel Hrdina [Fri, 14 Sep 2018 21:46:42 +0000 (23:46 +0200)]
vircgroup: introduce virCgroupV2DetectPlacement
If the placement was copied from parent or set to absolute path
there is nothing to do, otherwise set the placement based on
process placement from /proc/self/cgroup or /proc/{pid}/cgroup.