qemu_process: Destroy domain's namespace after killing QEMU
After QEMU is killed in qemuProcessStop() its mount namespace
doesn't exist anymore, because it was the only process running
there. Thus we should clear our internal flag that the domain has
namespace enabled so that seclabel restore code does not try to
enter it. We do the same in qemuProcessHandleMonitorEOF() but
when it is us, who decides to kill QEMU rather than QEMU quitting
we haven't seen EOF by the time qemuProcessStop() is called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
kbase: Document how to disable Secure Boot entirely
In most cases, disabling the secure-boot or the enrolled-keys
firmware feature will achieve the same result: allowing an
unsigned operating system to run.
Right now we're only documenting the latter configuration. Add
the former as well, and explain the difference between the two.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
It should be enough to enable or disable the enrolled-keys feature
to control whether Secure Boot is enforced, but there's a slight
complication: many distro packages for edk2 include, in addition
to general purpose firmware images, builds that are targeting the
Confidential Computing use case.
For those, the firmware descriptor will not advertise the
enrolled-keys feature, which will technically make them suitable
for satisfying a configuration such as
In practice, users will expect the general purpose build to be
used in this case. Explicitly asking for the secure-boot feature
to be enabled achieves that result at the cost of some slight
additional verbosity.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The non-Linux version of virHostCPUGetPhysAddrSize() is lacking
G_GNUC_UNUSED attribute to its @size argument which triggers an
error on all non-Linux builds. And while at it, make the function
actually signal error (ENOSYS) since it does not set the
argument.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Passthrough mode can only be used if the chosen CPU model is
'host-passthrough'. Also validate that an explicitly specified
bits value does not exceed the physical address bits on the host.
The feature is available since QEMU 2.7.0.
Signed-off-by: Dario Faggioli <dfaggioli@suse.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
sub element of /domain/cpu, which allows specifying the guest virtual CPU
address size. This can be useful if the guest needs to have a large amount
of memory.
If mode='passthrough', the virtual CPU will have the same number of address
bits as the host. If mode='emulate', the mandatory bits attribute specifies
the number of address bits.
Signed-off-by: Dario Faggioli <dfaggioli@suse.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Supported TPM versions are reported in domain capabilities. These
are used already to validate TPM type and model, but not TPM
version. This is suboptimal, because otherwise we leave users to
meet the error when starting a guest and libvirt spawns swtpm
binary which in turn reports an error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Michal Privoznik [Wed, 20 Jul 2022 07:51:55 +0000 (09:51 +0200)]
testutilsqemu: Fake TPM versions
Because of v8.5.0-rc1~25 we are already faking TPM support for
domaincaps. Might as well fake supported TPM versions.
The swtpm binary supports both TPM versions since its first
release, but pretend it isn't the case. For QEMU-5.2 and older
pretend only TPM-1.2 is available, QEMU-6.* has both TPM-1.2 and
TPM-2.0 and QEMU-7.0 and newer has only TPM-2.0 available.
This way, domaincaps are more dispersed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
The virDomainTPMVersion enum is declared in domain_conf.h among
with its TypeFromString() and TypeToString() helpers (which are
then implemented in domain_conf.c). However, neither of these
helpers is exposed in libvirt_private.syms which makes it
impossible for other modules to use.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Jiri Denemark [Wed, 3 Aug 2022 11:25:06 +0000 (13:25 +0200)]
qemu: Do not try to set memlock on inactive domain
When we call qemuDomainSetMaxMemLock to reset memory locking limit back
to its original value the domain can already be stopped (for example
after the domain shuts down during migration) in which case it does not
make sense to set any limit. Doing so can even be harmful as we may end
up setting the limit for the daemon itself as the PID is 0.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Jiri Denemark [Wed, 3 Aug 2022 11:18:59 +0000 (13:18 +0200)]
qemu: Reset stored memlock limit when stopping QEMU
When resetting private data after stopping QEMU process we should also
reset the original memory locking limit (both normal and pre-migration)
as they are not relevant anymore.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
The qemuDomainDefCPUPostParse() does a bit more than filling in
missing info. It also validates CPU cache configuration. Move
that code into qemuValidateDomainDefCpu() where the code fits
better.
And since I need to fix indentation of existing code in
qemuValidateDomainDefCpu(), I'm taking this opportunity and move
error messages onto single line. Interestingly, this uncovers a
bug we have in sc_prohibit_diagnostic_without_format syntax-check
rule, because previously a virReportError() with a message
spawned over three lines was not caught but not it is. But
trying to understand that regex is a job for another time.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Peter Krempa [Tue, 2 Aug 2022 12:26:41 +0000 (14:26 +0200)]
qemuProcessQMPConnectMonitor: Connect to probing monitor with 'retry' set to false
In 'qemuProcessQMPLaunch' qemu is very specifically launched using it's
internal '-daemonize' flag (see comment in the function) to ensure that
the monitor socket is ready and opened prior to attempting the monitor
connection.
This means we don't have to retry the connection to the monitor in
qemuMonitorOpen as the socket will be already there.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Peter Krempa [Tue, 2 Aug 2022 11:54:35 +0000 (13:54 +0200)]
qemuMonitorTestNew: Call qemuMonitorOpen with 'retry' false
The 'retry' argument makes the monitor connection opening re-try the
connection in case the monitor socket doesn't exist or isn't properly
listening. In case of the test code this can't happen because the socket
is created and made listening in 'qemuMonitorCommonTestNew' which is
called prior to calling 'qemuMonitorOpen'.
We can thus avoit the code which attempts retries in monitor connection.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Peter Krempa [Tue, 2 Aug 2022 11:45:49 +0000 (13:45 +0200)]
qemu: monitor: Remove 'timeout' argument from qemuMonitorOpen
The 'timeout' argument is used by 'qemuMonitorOpenUnix' only when the
'retry' argument is true. The callers of 'qemuMonitorOpen' only pass '0'
for timeout when they call it with 'retry' true and use other values
when 'retry' is false and thus ignored.
This means we can remove the argument and simply have it set to the
default value of QEMU_DEFAULT_MONITOR_WAIT.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
qemu_capabilities: replace code with function call
Since functions virQEMUCapsFillDomainFeatureSEVCaps() and
virQEMUCapsSEVInfoCopy() essentially do the same thing it does
not make sense to have the code duplicated. This patch replaces
the relevant code in the first function with the function call to
the second one.
Cole Robinson [Mon, 1 Aug 2022 19:24:01 +0000 (15:24 -0400)]
virfile: Fix build with glibc 2.36
With glibc 2.36, sys/mount.h and linux/mount.h conflict:
https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
virfile.c imports sys/mount.h and linux/fs.h, which pulls in
linux/mount.h.
Manually define the constants we need from linux/fs.h, like was
done in llvm:
Cole Robinson [Mon, 1 Aug 2022 19:20:38 +0000 (15:20 -0400)]
lxc: containter: fix build with glibc 2.36
With glibc 2.36, sys/mount.h and linux/mount.h conflict:
https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
lxc_container.c imports sys/mount.h and linux/fs.h, which pulls in
linux/mount.h.
linux/fs.h isn't required here though. glibc sys/mount.h has had
MS_MOVE since 2.12 in 2010
Reviewed-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Cole Robinson <crobinso@redhat.com>
Michal Privoznik [Wed, 20 Jul 2022 07:11:38 +0000 (09:11 +0200)]
testutilsqemu: Mock virTPMSwtpmSetupCapsGet()
In a recent commit of v8.5.0-85-g430ab88ab1 I've made domaincaps
XML report supported TPM versions. This was done by calling
virTPMSwtpmSetupCapsGet(). But this function isn't mocked and
thus domaincapstest calls the real implementation, which tries to
execute swtpm_setup binary. This fails, because
virFindFileInPath() is mocked in such way that it returns NULL
for anything else than qemu-*.
Anyway, while the real binary is not executed after all, we
should mock the function which tries to execute it so that
predictable result is returned.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Michal Privoznik [Wed, 20 Jul 2022 07:11:31 +0000 (09:11 +0200)]
virtpm: Use corresponding type for argument for virTPM*CapsGet()
In virtpm.h there are two functions exposed for querying swtpm
and swtpm_setup capabilities: virTPMSwtpmCapsGet() and
virTPMSwtpmSetupCapsGet(), respectively. The capabilities we are
interested in are defined in two separate enums
(virTPMSwtpmFeature and virTPMSwtpmSetupFeature), but these
functions accept capability as an unsigned int rather than their
respective enum. While this makes sense for
virTPMBinaryGetCaps(), which is a module internal helper that
both exposed functions call, there's no need for the functions
themselves to accept unsigned int.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Michal Privoznik [Fri, 15 Jul 2022 16:04:30 +0000 (18:04 +0200)]
conf: Don't lose <active_pcr_banks/> when no TPM version is provided
When no TPM version is provided in the input XML we may default
to version 2.0 (see qemuDomainTPMDefPostParse()). However,
<active_pcr_banks/> are parsed iff a version 2.0 was specified.
This means that this piece of information might be lost.
It's better to parse everything we've been given and then
validate that the configuration is valid.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2084046 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Mon, 18 Jul 2022 07:32:19 +0000 (09:32 +0200)]
qemu: Move TPMs validation out of PostParse
After previous cleanup, the qemuDomainDefTPMsPostParse() function
does nothing more than validates TPM devices. Therefore, it
should live in qemu_validate.c instead of qemu_domain.c. Move it
there and rename to reflect the fact that the function is doing
validation instead of PostParsing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Mon, 18 Jul 2022 07:10:12 +0000 (09:10 +0200)]
qemu_domain: Move TPM post parse code into qemuDomainTPMDefPostParse()
In the qemuDomainDefPostParse() we aim to fill in top level
values, which require overall view of domain, or those parts of
configuration that are not a device in domain XML (e.g. vCPUs).
However, inside of qemuDomainDefTPMsPostParse(), which is called
from aforementioned function, we do two tings:
1) fill in missing info (TPM version), and
2) validate TPM definition.
Now, if 1) is moved into qemuDomainTPMDefPostParse() (the device
post parse callback), then 2) can be moved into validation step.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Fri, 15 Jul 2022 16:04:21 +0000 (18:04 +0200)]
conf: Move _virDomainTPMDef::version into _virDomainTPMDef::data::emulator
The _virDomainTPMDef structure has 'version' member, which is a
bit misplaced. It's only emulator type of TPM that can have a
version, even our documentation says so:
``version``
The ``version`` attribute indicates the version of the TPM. This attribute
only works with the ``emulator`` backend. The following versions are
supported:
Therefore, move the member into that part of union that's
covering emulated TPM devices.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Mon, 18 Jul 2022 07:58:38 +0000 (09:58 +0200)]
conf: Drop needless setting of VIR_DOMAIN_TPM_VERSION_DEFAULT
In previous commit the VIR_DOMAIN_TPM_VERSION_DEFAULT value was
made just an alias to value of 0. And since all newly allocated
memory is zeroed out (due to use of g_new0()), the def->version
inside of virDomainTPMDefParseXML() is also 0 and thus there is
no need to set it explicitly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Mon, 18 Jul 2022 07:55:08 +0000 (09:55 +0200)]
conf: Report error when default TPM version is provided
When "default" version of TPM was provided, our parses accepts it
happily even though the value is forbidden by our RNG and not
documented as accepted value. This is because of < 0 vs <= 0
comparison of virDomainTPMModelTypeFromString() retval.
Make the parser error out explicitly in this case. Users can
always chose to not specify the attribute in which case we pick a
sane default (in qemuDomainDefTPMsPostParse()).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Mon, 18 Jul 2022 07:11:19 +0000 (09:11 +0200)]
conf: Report an error when default TPM model is provided
When "default" model of a TPM was provided, our parses accepts it
happily even though the value is forbidden by our RNG and not
documented as accepted value. This is because of < 0 vs <= 0
comparison of virDomainTPMModelTypeFromString() retval.
Make the parser error out explicitly in this case. Users can
always chose to not specify the attribute in which case we pick a
sane default (in qemuDomainTPMDefPostParse()).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Wed, 27 Jul 2022 08:31:03 +0000 (10:31 +0200)]
qemu_cgroup: Don't ignore ENOENT in qemuCgroupAllowDevicesPaths()
There's no need to skip over ENOENT error in
qemuCgroupAllowDevicesPaths(). The path must exists when
qemuCgroupAllowDevicePath() is called because of virFileExists()
check done right above.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Tue, 26 Jul 2022 13:53:07 +0000 (15:53 +0200)]
qemu_command: Separate domain memory building into a helper
The qemuBuildMachineCommandLine() function is needlessly long.
Separate out parts that generate memory related arguments into
qemuAppendDomainMemoryMachineParams(). Unfortunately, expected
outputs for some qemuxml2argvdata cases needed to be updated
because the order in which arguments are generated is changed.
But there's no functional change.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Tue, 26 Jul 2022 13:45:08 +0000 (15:45 +0200)]
qemu_command: Separate domain features building into a helper
The qemuBuildMachineCommandLine() function is needlessly long.
Separate out parts that generate arguments based on
domainDef->features[] into
qemuAppendDomainFeaturesMachineParam(). Unfortunately, expected
outputs for some qemuxml2argvdata cases needed to be updated
because the order in which features are generated is changed. But
there's no functional change.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Tue, 26 Jul 2022 17:27:06 +0000 (19:27 +0200)]
conf: Validate virDomainMemoryDef::targetNode
Almost all of memory models we currently support allow setting
virDomainMemoryDef::targetNode so that the memory module is
associated with given guest NUMA node. And we do have a check
whether the requested node is within bounds, but it's executed
only when building QEMU's cmd line. Move it into validation
phase.
While this commit is moving the validation to a place that does
not validate all the possible code paths, it's okay, because only
the explicit memory device has user-configurable target node
which could break the assumption.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Tue, 26 Jul 2022 17:42:33 +0000 (19:42 +0200)]
qemuxml2argvtest: Switch memory-hotplug-dimm-addr to latest caps
So far, we are testing memory-hotplug-dimm-addr against a set of
explicitly listed capabilities. While this works, lets switch it
to DO_TEST_CAPS_LATEST() so that the latest capabilities are
used. This in turn means, we have to update the <emulator/>
because the latest capabilities don't contain caps for
qemu-system-i386.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Currently, virJSONValueObjectHasKey() can return one of three
values:
-1 if passed object type is not VIR_JSON_TYPE_OBJECT,
0 if the key is not present, and finally
1 if the key is present.
But, neither of callers is interested in the -1 case. In fact,
some callers call this function treating -1 and 1 cases the same.
Therefore, make the function return just true/false and fix few
callers that explicitly checked for == 1 case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
qemu_migration_params: Avoid deadlock in qemuMigrationParamsReset
In my recent comnmit v8.5.0-188-gc47f1abb81 I accidentally moved
qemuMigrationParamsResetTLS after qemuDomainObjEnterMonitorAsync not
noticing qemuMigrationParamsResetTLS will try to enter the monitor
again. The second call will time out and return with a domain object
locked. But we're still in monitor section and the object should be
unlocked which means qemuDomainObjExitMonitor will deadlock trying to
lock it again.
Fixes: c47f1abb81194461377a0c608a7ecd87f9ce9146 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Wed, 27 Jul 2022 16:14:10 +0000 (12:14 -0400)]
qemu: don't call qemuMigrationSrcIsAllowedHostdev() from qemuMigrationDstPrepareFresh()
This call to qemuMigrationSrcIsAllowedHostdev() (which does a
hardcoded fail of the migration if there is any PCI or mdev hostdev
device in the domain) while doing the destination side of migration
prep was found once the call to that same function was removed from
the source side migration prep (commit 25883cd5).
According to jdenemar, for the V2 migration protocol, prep of the
destination is the first step, so this *was* the proper place to do
the check, but for V3 migration this is in a way redundant (since we
will have already done the check on the source side (updated by 25883cd5 to query QEMU rather than do a hardcoded fail)).
Of course it's possible that the source could support migration of a
particular VFIO device, but the destination doesn't. But the current
check on the destination side is worthless even in that case, since it
is just *always* failing rather than querying QEMU; and QEMU can't be
queried at the point where the destination check is happening, since
it isn't yet running.
Anyway QEMU should complain when it's started if it's going to fail,
so removing this check should just move the failure to happen a bit
later. So the best solution to this problem is to simply remove the
hardcoded check/fail from qemuMigrationDstPrepareFresh() and rely on
QEMU to fail if it needs to.
Fixes: 25883cd5f0b188f2417f294b7d219a77b219f7c2 Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
qemu: Restore original memory locking limit on reconnect
Commit v8.4.0-287-gd4d3bb8130 tried to make sure the original
pre-migration memory locking limit is restored at the end of migration,
but it missed the case when libvirt daemon is restarted during
migration which needs to be aborted on reconnect.
And if this was not enough, I forgot to actually save the status XML
after setting the field in priv (in the commit mentioned above and also
in v8.4.0-291-gd375993ab3).
Normally when an UEFI firmware is marked as read-only, an associated
NVRAM file will be created. Some builds of UEFI firmware, however, wish
to remain stateless and so will be read-only, but never have any NVRAM
file. To represent this concept a 'stateless' tristate bool attribute
is introduced on the <loader/> element.
There are rather a large number of permutations to consider.
With default firmware selection
* <os/>
=> Historic default, no change
* <os>
<loader stateless='yes'/>
</os>
=> Explicit version of historic default, no change
Because qemuMigrationParamsReset used to call qemuMigrationParamsApply
for resetting migration capabilities and parameters, it did not work
well since commit v5.1.0-83-ga1dec315c9 which only allowed capabilities
to be set from an async job. However, when reconnecting to running
domains after daemon restart we do not have an async job. Thus the
capabilities were not properly reset in case the daemon was restarted
during an ongoing migration. We need to avoid calling
qemuMigrationParamsApply to make sure both parameters and capabilities
can be reset by a normal job.
qemuMigrationParamsApply restricts when capabilities can be set, but
this is not useful in all cases. Let's create new helpers for setting
migration capabilities and parameters which can be reused in more places
without the restriction.
qemu_migration: Store original migration params in status XML
We keep original values of migration parameters so that we can restore
them at the end of migration to make sure later migration does not use
some random values. However, this does not really work when libvirt
daemon is restarted on the source host because we failed to explicitly
save the status XML after getting the migration parameters from QEMU.
Actually it might work if the status XML is written later for some other
reason such as domain state change, but that's not how it should work.
Michal Privoznik [Mon, 25 Jul 2022 13:04:54 +0000 (15:04 +0200)]
coding-style: Allow some use of ternary operators
While we all understand that excessive use of ternary operator
may worsen code readability (e.g. nested, multi-line expression),
there are few cases where using it actually improves code
readability. For instance, when a function takes a long list of
arguments out of which one depends on a boolean expression, or
when formatting "yes"/"no" or "on"/"off" values based on a
boolean variable (although one can argue that the latter is a
subset of the former). Just consider alternatives to:
In fact, this pattern occurs plenty in our code. Exempt it from
our "no ternary operators" rule.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Claudio Fontana <cfontana@suse.de> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
It's no longer possible for libvirt to connect over the ssh transport
from RHEL 9 to RHEL 5. This is because SHA1 signatures have been
effectively banned in RHEL 9 at the openssl level. They are required
to check the RHEL 5 host key. Note this is a separate issue from
openssh requiring additional configuration in order to connect to
older servers.
$ virsh -c 'qemu+ssh://root@192.168.0.91/system' list
error: failed to connect to the hypervisor
error: Cannot recv data: ssh_dispatch_run_fatal: Connection to 192.168.0.91 port 22: error in libcrypto: Connection reset by peer
"error in libcrypto: Connection reset by peer" is the characteristic
error of openssl having been modified to disable SHA1 by default.
(You will not see this on non-RHEL-derived distros.)
You could enable the legacy crypto policy which downgrades security on
the entire host, but a more fine-grained way to do this is to create
an alternate openssl configuration file that enables the "forbidden"
signatures. However this requires passing the OPENSSL_CONF
environment variable through to ssh to specify the alternate
configuration. Libvirt filters out this environment variable, but
this commit allows it through. With this commit:
$ OPENSSL_CONF=/var/tmp/openssl.cnf ./run virsh -c 'qemu+ssh://root@192.168.0.91/system' list
root@192.168.0.91's password:
Id Name State
--------------------
Essentially my argument here is that OPENSSL_CONF is sufficiently
similar in nature to KRB5CCNAME, SSH* and XAUTHORITY that we should
permit it to be passed through.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2062360 Signed-off-by: Richard W.M. Jones <rjones@redhat.com> Acked-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Tue, 1 Mar 2022 13:24:33 +0000 (14:24 +0100)]
cmdQemuMonitorCommandQMPWrap: Reset ignored errors from JSON parsing
'cmdQemuMonitorCommandQMPWrap' is checking whether the user provided
string is not valid JSON to avoid wrapping it. In cases where it's not
JSON we ignore the error and add the wrapper.
If the caller then reports a different non-libvirt error the error from
the JSON parsing would be printed as well. Reset errors we ignore:
# virsh qemu-monitor-command cd --pass-fds a asdf
error: Unable to parse FD number 'a'
error: internal error: cannot parse json asdf: lexical error: invalid char in json text.
asdf
(right here) ------^
In the above case 'asdf' is not valid JSON, but the code did wrap it
into '{"execute":"asdf"}', the only problem is the argument for
--pass-fds.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Historically, the dumpxml command reject any unknown arguments,
for instance:
virsh dumpxml fedora xxx
However, after v8.5.0-rc1~31 the second argument ('xxx') is
treated as an XPath, but it's not that clearly visible.
Therefore, require the --xpath switch, like this:
virsh dumpxml fedora --xpath xxx
Yes, this breaks already released virsh, but I think we can argue
that the pool of users of this particular function is very small.
We also document the argument being mandatory:
The same applies for other *dumpxml functions (net-dumpxml,
pool-dumpxml, vol-dumpxl to name a few).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2103524 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Andrea Bolognani [Thu, 21 Jul 2022 09:14:21 +0000 (11:14 +0200)]
qemu: Add IOMMU device alias to command line
Note that we can only do this for intel-iommu and virtio-iommu,
which are configured using -device; smmuv3 is configured using
a machine type property, so there's no room on the command line
for an alias in that case.
Michal Privoznik [Mon, 18 Jul 2022 14:13:12 +0000 (16:13 +0200)]
qemu_hotplug: Create chardev files before attempting to relabel them
When hotplugging a chardev, Libvirt opens corresponding
file/binds to a socket/does whatever necessary to obtain an FD
that is later passed to QEMU. However, due to wrong placement of
the function that does all of this
(qemuProcessPrepareHostBackendChardevHotplug()) it may happen
that a file is set seclabel on, only to be unlink()-ed and
created again (the former is done by
qemuSecuritySetChardevLabel(), the latter by aforementioned
function). The unlink()-ing is done for UNIX sockets with
mode='bind' and happens inside qemuOpenChrChardevUNIXSocket().
However, these steps can be swapped simply.
Fixes: ad81aa8ad07e52c9bd4840de84d2ed59998b4d2a Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Michal Privoznik [Mon, 18 Jul 2022 14:29:07 +0000 (16:29 +0200)]
qemu_hotplug: Close FDs in QEMU on failed chardev hotplug
When hotplugging a chardev, Libvirt opens corresponding
file/binds to a socket/does whatever necessary to obtain an FD
that is later passed to QEMU. However, if something fails after
the FDs were transferred to QEMU and before chardev is actually
added via monitor, these FDs are never closed in QEMU. This is
rather suboptimal.
Pattern of using switch instead of a long if else construction is
used everywhere, so I used it here as well to make the code more
consistent (and remove that else after return). I also included
all the values from the enum.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The else branches are redundant because the execution will never
reach them if the conditions in the previous 'if' branches are
true.
I think this looks cleaner and is more readable, because having
'else' branch indicates that no return / break / goto is in the
previous branch and the function can reach it.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
qemu_migration: Acquire correct job in qemuMigrationSrcIsAllowed
Commit 62627524607f added the acquiring of a job, but it is not always
VIR_ASYNC_JOB_MIGRATION_OUT, so the code fails when doing save or anything else.
Correct the async job by passing it from the caller as another parameter.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Peter Krempa [Tue, 28 Jun 2022 15:08:00 +0000 (17:08 +0200)]
virLXCProcessReboot: Remove the need to re-register autodestroy callback
Add a new flag VIR_LXC_PROCESS_CLEANUP_AUTODESTROY to
virLXCProcessCleanupFlags for skipping removal of the autodestroy
callback so that fake reboot of the container doesn't need to fetch the
connection and re-register it.
Since virLXCProcessReboot is defined before virLXCProcessCleanupFlags,
this patch also moves the flag enum typedef to the beginning of the
file.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Peter Krempa [Tue, 28 Jun 2022 14:52:38 +0000 (16:52 +0200)]
virLXCProcessAutostartDomain: Refactor control flow and variable use
Use automatic unlocking of the 'vm' object, so that we can return early
when no autostart is needed and avoid passing of the 'driver' object
which is already present in 'vm's' private data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Tim Wiederhake <twiederh@redhat.com>