Jiri Denemark [Fri, 5 Jan 2018 14:03:12 +0000 (15:03 +0100)]
cputest: Add data for Intel(R) Xeon(R) CPU E5-2623 v4
The CPU contains the updated microcode for CVE-2017-5715.
The *-guest.xml and *-json.xml CPU definitions use Skylake-Client CPU
model rather than Broadwell. This is similar to Xeon-E5-2650-v4 and it
is caused by our CPU model selection code when no model matches the CPU
signature (family + model). We'd need to maintain a complete list of CPU
signatures for our CPU models to fix this.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Marc Hartmayer [Wed, 17 Jan 2018 12:26:08 +0000 (13:26 +0100)]
qemu: Fix segmentation fault when attaching a non iSCSI host device
Add a check if it's a iSCSI hostdev and if it's not then don't use the
union member 'iscsi'. The segmentation fault occured when accessing
secinfo->type, but this can vary from case to case.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Update the min fedora to 26. Use a macro to record the min versions so that the
later error message is always in sync with the earlier version check. Clarify
the comment that refers to guessing of dist which does not actually happen.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Dan Zheng [Tue, 16 Jan 2018 04:21:21 +0000 (12:21 +0800)]
nodedev: Add the missing PCI dev checks for 'mdev_types' capability
Similar to commit @f44ec9c1, commit @500cbc06 introduced a new nested
'mdev_types' capability, however the mentioned commit didn't adjust
virNodeDeviceNumOfCaps and virNodeDeviceListCaps functions accordingly
to provide proper support for this capability.
After applying this patch the following python snippet returns the
expected results:
import libvirt
conn = libvirt.openReadOnly('qemu:///system')
devs = conn.listAllDevices()
for dev in devs:
if 'mdev_types' in dev.listCaps():
print dev.name(),dev.numOfCaps(),dev.listCaps()
Signed-off-by: Dan Zheng <dzheng@redhat.com> Signed-off-by: Erik Skultety <eskultet@redhat.com>
Michal Privoznik [Sun, 14 Jan 2018 13:29:32 +0000 (14:29 +0100)]
m4: Check for rl_completion_quote_character
Apparently we can't assume that people run readline recent enough
to have rl_completion_quote_character (added in readline-5.0
released in 2011). However, we can't compile without it. So if
not present, disable readline.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The functions defined in these sources are referenced all over
the place, however, compiler only when building with readline.
Thus when building without it linker gets sad as it can't find
them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Michal Privoznik [Sun, 14 Jan 2018 10:43:51 +0000 (11:43 +0100)]
vsh: Provide cmdComplete stub for readline disabled builds
When building without readline, this function does nothing but
return false. Without touching any of its arguments which
triggers a build error. Therefore, provide a stub that has
arguments marked as unused.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Michal Privoznik [Fri, 12 Jan 2018 16:05:33 +0000 (17:05 +0100)]
vshReadlineOptionsGenerator: Don't add already specified options to the list
The current state of art is as follows:
1) vshReadlineOptionsGenerator() generate all possible --options
for given command, and then
2) vshReadlineOptionsPrune() clears out already provided ones
from the list.
Not only this brings needless memory complexity it is also not
trivial to get right. We can switch to easier approach: just
don't add already specified --options in the first step.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Andrea Bolognani [Fri, 12 Jan 2018 10:02:38 +0000 (11:02 +0100)]
travis: Sync packages with libvirt-jenkins-ci
Make sure we install the same packages lcitool would install on
the CentOS CI so that we have consistent results. The package
list is current as of libvirt-jenkins-ci commit 3a559ae7bc08.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Scott Garfinkle [Tue, 26 Dec 2017 19:55:08 +0000 (13:55 -0600)]
domcaps: Treat host models as case-insensitive strings
Qemu 2.11 allows case-insensitive specification of CPU models.
This patch fixes the resulting problems on (at least) POWER
arch machines so that Power8 and POWER8 are not different.
Signed-off-by: Scott Garfinkle <scottgar@linux.vnet.ibm.com>
Jiri Denemark [Thu, 11 Jan 2018 19:47:50 +0000 (20:47 +0100)]
qemu: Fix type of a completed job
Libvirt 3.7.0 and earlier libvirt reported a migration job as completed
immediately after QEMU finished sending migration data at which point
migration was not really complete yet. Commit v3.7.0-29-g3f2d6d829e
fixed this, but caused a regression in reporting statistics for
completed jobs which started reporting the job as still running. This
happened because the completed job statistics including the job status
are copied from the running job before we finally mark it as completed.
Let's make sure QEMU_DOMAIN_JOB_STATUS_COMPLETED is always set in the
completed job info even when the job has not finished yet.
Jiri Denemark [Wed, 10 Jan 2018 14:56:21 +0000 (15:56 +0100)]
qemu: Ignore fallback CPU attribute on reconnect
When reconnecting to a running domain with host-model CPU started by old
libvirt which did not store the actual CPU in the status XML, we need to
ignore the fallback attribute to make sure we can translate the detected
host CPU model to a model which is supported by the running QEMU.
Michal Privoznik [Thu, 11 Jan 2018 21:11:15 +0000 (22:11 +0100)]
m4: Don't enable bash-completion by default
Due to the way that check logic was written we basically enabled
bash completion whenever readline was enabled. This is not right
because it made bash-completion pkg-config module required.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
For given domain fetch list of defined interfaces. This can be
used for commands like domif-getlink and others. If available,
the interface name is returned (e.g. "vnet0", usually available
only for running domains), if not the MAC address is returned.
Moreover, the detach-interface command requires only MAC address
and therefore we have new flag that forces the completer to
return just the MAC address.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Tue, 31 Oct 2017 08:24:21 +0000 (09:24 +0100)]
virsh: Introduce virshDomainNameCompleter
Now that we have everything prepared let the fun begin. This
completer is very simple and returns domain names. Moreover,
depending on the command it can return just a subset of domains
(e.g. only running/paused/transient/.. ones).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This command is going to be called from bash completion script in
the following form:
virsh complete -- start --domain
Its only purpose is to return list of possible strings for
completion. Note that this is a 'hidden', unlisted command and
therefore there's no documentation to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Fri, 29 Dec 2017 10:25:38 +0000 (11:25 +0100)]
vsh: Filter --options
Similarly to the previous commit, once we've presented an
--option for a command to the user it makes no sense to offer it
again. Therefore, we can prune all already specified options. For
instance, after this patch:
virsh # migrate --verbose <TAB><TAB>
will no longer offer --verbose option.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Thu, 28 Dec 2017 11:26:41 +0000 (12:26 +0100)]
vsh: Prune string list returned by completer
Instead of having completers prune returned string list based on
user's input we can do that right after the callback is called.
Only strings matching the prefix will be presented to the user
then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Tue, 21 Nov 2017 16:45:50 +0000 (17:45 +0100)]
vsh: Call vshCmdOptDef completer
Now that we have everything prepared we can call options'
completer again. At the same time, pass partially parsed input to
the completer callback - it will help the callbacks to narrow
down the list of returned options based on user's input. For
instance, if the completer is supposed to return list of
interfaces depending on user input it may return just those
interfaces defined for already specified domain. Of course,
completers might ignore this parameter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In the future, completer callbacks will receive partially parsed
command (and thus possibly incomplete). However, we still want
them to use command options fetching APIs we already have (e.g.
vshCommandOpt*()) and at the same time don't report any errors
(nor call any asserts).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Michal Privoznik [Tue, 21 Nov 2017 16:23:11 +0000 (17:23 +0100)]
vshReadlineParse: Use string list
It's better to fetch list of either commands or options just once
and then iterate over it. Moreover, it makes future completers
way simpler as they will return string lists too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Mon, 13 Nov 2017 12:34:54 +0000 (13:34 +0100)]
vshReadlineParse: Escape returned results if needed
When returning a string that needs escaping there are two
scenarios that can happen. Firstly, user already started the
string with a quote (or double quote) in which case we don't need
to do anything - readline takes care of that. However, if they
haven't typed anything yet, we need to escape the string
ourselves.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
vshCommandStringParse: Allow retrieving partial result
In the future, this function is going to be called from
vshReadlineParse() to provide parsed input for completer
callbacks. The idea is to allow the callbacks to provide more
specific data. For instance, for the following input:
the --interface completer callback is going to be called. Now, it
is more user friendly if the completer offers only those
interfaces found in 'fedora' domain. But in order to do that it
needs to be able to retrieve partially parsed result.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Mon, 13 Nov 2017 13:46:36 +0000 (14:46 +0100)]
vshCommandParse: Don't leak @tkdata
When parsing cmd line which has "--" on it, this is leaked.
Problem is, parser->getNextArg() allocates new string and stores
it into tkdata. But as soon as "--" is detected 'continue' is
issued without any free of the allocated memory.
==5304== 3 bytes in 1 blocks are definitely lost in loss record 1 of 782
==5304== at 0x4C2AF50: malloc (vg_replace_malloc.c:299)
==5304== by 0x8BB5AA9: strdup (strdup.c:42)
==5304== by 0x55842CA: virStrdup (virstring.c:941)
==5304== by 0x172B21: _vshStrdup (vsh.c:162)
==5304== by 0x175E8E: vshCommandArgvGetArg (vsh.c:1622)
==5304== by 0x17551D: vshCommandParse (vsh.c:1418)
==5304== by 0x175F25: vshCommandArgvParse (vsh.c:1638)
==5304== by 0x130940: virshParseArgv (virsh.c:820)
==5304== by 0x130C49: main (virsh.c:922)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Bjoern Walk [Tue, 19 Dec 2017 10:08:01 +0000 (11:08 +0100)]
tests: virhostcputest: testcase for S390 system
Let's add a testcase for a S390 system running kernel version 4.14 on
LPAR.
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Bjoern Walk [Tue, 19 Dec 2017 10:08:00 +0000 (11:08 +0100)]
util: virhostcpu: parse frequency information on S390
Since kernel version 4.7, processor frequency information is available
on S390. Let's adjust the parser so this information shows up for virsh
nodeinfo:
# virsh nodeinfo
CPU model: s390x
CPU(s): 8
CPU frequency: 5000 MHz
CPU socket(s): 1
Core(s) per socket: 8
Thread(s) per core: 1
NUMA cell(s): 1
Memory size: 16273908 KiB
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
John Ferlan [Tue, 10 Oct 2017 22:32:40 +0000 (18:32 -0400)]
storage: Complete implementation volume by hash object
Alter the volume logic to use the hash tables instead of forward
linked lists. There are three hash tables to allow for fast lookup
by name, target.path, and key.
Modify the virStoragePoolObjAddVol to place the object in all 3
tables if possible using self locking RWLock on the volumes object.
Conversely when removing the volume, it's a removal of the object
from the various hash tables.
Implement functions to handle remote ForEach and Search Volume
type helpers. These are used by the disk backend in order to
facilitate adding a primary, extended, or logical partition.
Implement the various VolDefFindBy* helpers as simple (and fast)
hash lookups. The NumOfVolumes, GetNames, and ListExport helpers
are all implemented using standard for each hash table calls.
Alter the logic such that we only add the volume to the pool once
we've filled in all the information and cause failure to go to a
common error: label. Patches to place the @vol into a few hash tables
will soon "require" that at least the keys (name, target.path, and key)
be populated with valid data.
John Ferlan [Tue, 9 Jan 2018 13:40:24 +0000 (08:40 -0500)]
storage: When delete volume avoid disk backend removal
For a disk backend, the deleteVol code will clear all the
volumes in the pool and perform a pool refresh, thus the
storageVolDeleteInternal should not use access @voldef
after deleteVol succeeds.
Jiri Denemark [Fri, 5 Jan 2018 16:43:03 +0000 (17:43 +0100)]
cpu_x86: Copy CPU signature from ancestor
When specifying a new CPU model in cpu_map.xml as an extension to an
existing model, we forgot to copy the signature (family + model) from
the original CPU model.
We don't use this way of specifying CPU models, but it's still supported
and it becomes useful when someone wants to quickly hack up a CPU model
for testing or when creating additional variants of existing models to
help with fixing some spectral issues.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Jiri Denemark [Fri, 5 Jan 2018 16:43:27 +0000 (17:43 +0100)]
cpu_x86: Add debug messages to x86DecodeUseCandidate
When translating CPUID data into CPU model + features, the code
sometimes uses an unexpected CPU model. There may be several reasons for
this, starting with wrong expectations and ending with an actual bug in
our code. These debug messages will help determining the reason.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Jiri Denemark [Fri, 5 Jan 2018 13:35:42 +0000 (14:35 +0100)]
cputest: Fix cpu-cpuid.py diff command
The cpuidMap in cpu-cpuid.py was created for converting old data files
(with QEMU's feature-words bits) to the new model-expansion based data.
When I added tests for CPU live update based on disabled/enabled feature
lists I shamelessly used the existing cpuidMap for generating the
*-{enabled,disabled}.xml data files. Thus any new CPUID bits which are
not present in the original cpuidMap would be ignored. The correct thing
to do is to use cpu_map.xml.
All data files were fixed by running the following command:
./cpu-cpuid.py diff *.json
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
virSecurityManagerDomainSetPathLabel is used to make a path known
to the security modules, but today is used interchangably for
- paths to files/dirs to be accessed directly
- paths to a dir, but the access will actually be to files therein
Depending on the security module it is important to know which of
these types it will be.
The argument allowSubtree augments the call to the implementations of
DomainSetPathLabel that can - per security module - decide if extra
actions shall be taken.
For now dac/selinux handle this as before, but apparmor will make
use of it to add a wildcard to the path that was passed.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This came up in discussions around huge pages, but it will cover
more per guest paths that should be added to the guests apparmor profile:
- keys via qemuDomainWriteMasterKeyFile
- per domain dirs via qemuProcessMakeDir
- memory backing paths via qemuProcessBuildDestroyMemoryPathsImpl
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
apparmor, libvirt-qemu: add default pki path of libvirt-spice
Adding the PKI path that is used as default suggestion in src/qemu/qemu.conf
If people use non-default paths they should use local overrides but the
suggested defaults we should open up.
This is the default path as referenced by src/qemu/qemu.conf in libvirt.
While doing so merge the several places we have to cover PKI access into
one.
Allows (multi-arch enabled) access to libraries under the
/usr/lib/@{multiarch}/qemu/*.so path in the Debian/Ubuntu
qemu-block-extra package and all such libs for the paths
of rpm qemu-block-* packages.
Users might use a block device as UEFI VAR store. Or even have
OVMF stored there. Therefore, when starting a domain and separate
mount namespace is used, we have to create all the /dev entries
that are configured for the domain.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Chen Hanxiao [Fri, 5 Jan 2018 02:47:47 +0000 (10:47 +0800)]
qemu: Add support for hot unplugging redirdev device
Commit id '162efa1a' added support hotplug a redirdev, but
did not add the hot unplug. This patch will add that support
to allow usage of the detach-device --live on the device.
Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Andrea Bolognani [Mon, 11 Dec 2017 15:09:20 +0000 (16:09 +0100)]
util: Don't report CPU frequency for ARM hosts
Some ARM platforms, such as the original Raspberry Pi, report the
CPU frequency in the BogoMIPS field of /proc/cpuinfo, so libvirt
parsed that field and returned it through its API.
However, not only many more boards don't report any value there,
but several - including ARMv8-based server hardware, and even the
more recent Raspberry Pi 3 - use this field as originally intended:
to report the BogoMIPS value instead of the CPU frequency.
Since we have no way of detecting how the field is being used,
it's better to report no information at all rather than something
ludicrous like "your shiny 96-core aarch64 virtualization host's
CPUs are running at a whopping 100 MHz".
Andrea Bolognani [Mon, 11 Dec 2017 14:32:49 +0000 (15:32 +0100)]
util: Improve CPU frequency parsing
Make the parser both more strict, by not ignoring errors reported
by virStrToLong_ui(), and more permissive, by not failing due to
unrelated fields which just happen to have a know prefix and
accepting any amount of whitespace before the numeric value.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Bjoern Walk [Thu, 30 Nov 2017 11:50:47 +0000 (12:50 +0100)]
util: virhostcpu: factor out frequency parsing
All different architectures use the same copy-pasted code to parse
processor frequency information from /proc/cpuinfo. Let's extract that
code into a function to avoid repetition.
We now also tolerate if the parsing of /proc/cpuinfo is not successful
and just report a warning instead of bailing out and abandoning the rest
of the CPU information.
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
So imagine you have /dev/blah symlink which points to /dev/sda.
You attach /dev/blah as disk to your domain. Libvirt correctly
creates the /dev/blah -> /dev/sda symlink in the qemu namespace.
However, then you detach the disk, change the symlink so that it
points to /dev/sdb and tries to attach the disk again. This time,
however, the attach fails (well, qemu attaches wrong disk)
because the code assumes that symlinks don't change. Well they
do.
This is inspired by test fix written by Eduardo Habkost.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
qemu: Add support for pseries machine's max-cpu-compat= parameter
When the -machine pseries,max-cpu-compat=X is supported use
machine parameter instead of -cpu host,compat=X parameter as
that is deprecated now with qemu >= v2.10.
Michal Privoznik [Wed, 13 Dec 2017 14:12:01 +0000 (15:12 +0100)]
qemuDomainDiskChangeSupported: Forbid alias change
Since we have user aliases it may happen that users want to
change it using 'update-device'. Instead of ignoring it silently,
error out loudly. Note that we don't limit the check just for
"ua-" prefixes because users might try to change libvirt
generated aliases too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
John Ferlan [Fri, 17 Nov 2017 19:25:01 +0000 (14:25 -0500)]
qemu: Alter dump-guest-memory command generation
The qemuMonitorJSONMakeCommand can properly handle a NULL string
by using the "S:" parameter instead of "s:", so let's use that
of having in if/else condition that only adds the "s:".
Paolo Bonzini [Tue, 12 Dec 2017 15:23:41 +0000 (16:23 +0100)]
qemu: capabilities: force update if the microcode version does not match
A microcode update can cause the CPUID bits to change; an example
from the past was the update that disabled TSX on several Haswell
and Broadwell machines.
Therefore, place microcode version in the virQEMUCaps struct and
XML, and rebuild the cache if the versions do not match.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Paolo Bonzini [Tue, 12 Dec 2017 15:23:40 +0000 (16:23 +0100)]
conf: include x86 microcode version in virsh capabilities
A microcode update can cause the CPUID bits to change; an example
from the past was the update that disabled TSX on several Haswell and
Broadwell machines.
In order to track the x86 microcode version in the QEMU capabilities,
we have to fetch it and store it in the host CPU. This also makes the
version visible in "virsh capabilities", which is a nice side effect.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Commit id 'dc692438' reverted the automagic addition of a SCSI
controller attempt during virDomainHostdevAssignAddress; however,
the logic to determine where to place the next_unit depended upon
the "new" controller being added. Without the new controller the
the next time through the call for the next SCSI hostdev found
would result in the "next_unit" never changing from 0 (zero) and
as a result the addition of the device will fail due to being a
duplicate unit number of the first with the error message:
virDomainDefCheckDuplicateDriveAddresses:$line : unsupported
configuration: SCSI host address controller='0' bus='1'
target='0' unit='0' in use by another SCSI host device
So instead of walking the controller list looking for SCSI
controllers, all we can do is "pretend" that they exist and
allow other code to create them later as necessary.
John Ferlan [Mon, 4 Dec 2017 20:52:57 +0000 (15:52 -0500)]
conf: Use existing SCSI hostdev model to create new
In virDomainDefMaybeAddHostdevSCSIcontroller when we add a new
controller because someone neglected to add one or we're adding
one because the existing one is full, we should copy over the
model number from the existing controller since whatever we
create should at least have the same characteristics as the one
we cannot use because it's full.
NB: This affects the existing hostdev-scsi-autogen-address test
which would add a default ('lsi') SCSI controller for the various
scsi_host's that would create a controller for the hostdev.
John Ferlan [Mon, 4 Dec 2017 19:33:30 +0000 (14:33 -0500)]
qemu: Use same model when adding hostdev SCSI controller
When qemuDomainFindOrCreateSCSIDiskController adds a controller,
let's use the same model as a currently found controller under the
assumption that the reason to add the controller in hotplug is
because virDomainHostdevAssignAddress determined that there were
too many devices on the existing controller, but only assigned a
new controller index and did not add a new controller and we
desire to use the same controller model as any existing controller
and not take a chance that qemuDomainSetSCSIControllerModel would
use a default that may be incompatible.
John Ferlan [Wed, 22 Nov 2017 16:55:10 +0000 (11:55 -0500)]
nodedev: Move device enumumeration out of nodeStateInitialize
Let's move the udevEnumerateDevices into a thread to "speed
up" the initialization process. If the enumeration fails we
can set the Quit flag to ensure that udevEventHandleCallback
will not run.
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Marc Hartmayer [Thu, 21 Dec 2017 14:29:08 +0000 (15:29 +0100)]
rpc: Replace virNetServerClientNeedAuth with virNetServerClientIsAuthenticated
Replace virNetServerClientNeedAuth with
virNetServerClientIsAuthenticated because it makes it clearer what it
means.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Marc Hartmayer [Thu, 21 Dec 2017 14:29:07 +0000 (15:29 +0100)]
rpc: Remove virNetServerClientNeedAuthLocked
'Squash' virNetServerClientNeedAuthLocked into
virNetServerClientNeedAuth and remove virNetServerClientNeedAuthLocked
as it's not longer needed.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Marc Hartmayer [Thu, 21 Dec 2017 14:29:06 +0000 (15:29 +0100)]
tests: virnetdaemontest: Enable testing for 'auth_pending'
Enable testing for 'auth_pending' in the virnetdaemon test case.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Marc Hartmayer [Thu, 21 Dec 2017 14:29:05 +0000 (15:29 +0100)]
rpc: virnetserver: Fix race on srv->nclients_unauth
There is a race between virNetServerProcessClients (main thread) and
remoteDispatchAuthList/remoteDispatchAuthPolkit/remoteSASLFinish (worker
thread) that can lead to decrementing srv->nclients_unauth when it's
zero. Since virNetServerCheckLimits relies on the value
srv->nclients_unauth the underrun causes libvirtd to stop accepting
new connections forever.
Example race scenario (assuming libvirtd is using policykit and the
client is privileged):
1. The client calls the RPC remoteDispatchAuthList =>
remoteDispatchAuthList is executed on a worker thread (Thread
T1). We're assuming now the execution stops for some time before
the line 'virNetServerClientSetAuth(client, 0)'
2. The client closes the connection irregularly. This causes the
event loop to wake up and virNetServerProcessClient to be
called (on the main thread T0). During the
virNetServerProcessClients the srv lock is hold. The condition
virNetServerClientNeedAuth(client) will be checked and as the
authentication is not finished right now
virNetServerTrackCompletedAuthLocked(srv) will be called =>
--srv->nclients_unauth => 0
3. The Thread T1 continues, marks the client as authenticated, and
calls virNetServerTrackCompletedAuthLocked(srv) =>
--srv->nclients_unauth => --0 => wrap around as nclient_unauth is
unsigned
4. virNetServerCheckLimits(srv) will disable the services forever
To fix it, add an auth_pending field to the client struct so that it
is now possible to determine if the authentication process has already
been handled for this client.
Setting the authentication method to none for the client in
virNetServerProcessClients is not a proper way to indicate that the
counter has been decremented, as this would imply that the client is
authenticated.
Additionally, adjust the existing test cases for this new field.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Marc Hartmayer [Thu, 21 Dec 2017 14:29:04 +0000 (15:29 +0100)]
rpc: Introduce virNetServerSetClientAuthenticated
Combine virNetServerClientSetAuth(client,
VIR_NET_SERVER_SERVICE_AUTH_NONE) and virNetServerTrackCompletedAuth
into one new function named virNetServerSetClientAuthenticated.
After using this new function the function
virNetServerTrackCompletedAuth was superfluous and is therefore
removed. In addition, it is not very common that a
'{{function}}' (virNetServerTrackCompletedAuth) does more than just
the locking compared to
'{{function}}Locked' (virNetServerTrackCompletedAuthLocked).
virNetServerTrackPendingAuth was already superfluous and therefore
it's also removed.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Marc Hartmayer [Thu, 21 Dec 2017 14:29:03 +0000 (15:29 +0100)]
rpc: Correct locking and simplify the function
The lock for @client must not only be held for the duration of
checking whether the client wants to close, but also for as long as
we're closing the client. The same applies to the tracking of
authentications.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Marc Hartmayer [Thu, 21 Dec 2017 14:29:02 +0000 (15:29 +0100)]
rpc: Refactor the condition whether a client needs authentication
Add virNetServerClientAuthMethodImpliesAuthenticated() for deciding
whether a authentication method implies that a client is automatically
authenticated or not. Use this new function in
virNetServerClientNeedAuthLocked().
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Marc Hartmayer [Thu, 21 Dec 2017 14:29:01 +0000 (15:29 +0100)]
rpc: First test if authentication is required
This makes the code more efficient.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Marc Hartmayer [Thu, 21 Dec 2017 14:29:00 +0000 (15:29 +0100)]
rpc: Be more precise in which cases the authentication is needed and introduce *Locked
Be more precise in which cases the authentication is needed and
introduce *Locked.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Marc Hartmayer [Thu, 21 Dec 2017 14:28:59 +0000 (15:28 +0100)]
rpc: Add typedef for the anonymous enum used for authentication methods
Add typedef for the anonymous enum used for the authentication methods
and remove the default case. This allows the usage of the type in a
switch statement and taking advantage of the compilers feature to
detect uncovered cases.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
projects / thirdparty / libvirt.git / log
