Fill in the required fields in qemuBlockStorageSourceChainData to handle
the hotplug so that we can simplify the cleanup code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
qemuBlockStorageSourceChainData encapsulates the backend of the disk for
startup and hotplug operations. Add the handling for the copy-on-read
filter so that the hotplug code doesn't need to have separate cleanup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Replace the last use of the function by virDomainDiskInsert and remove
the unused helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Pre-extending the disk array size is pointless nowadays since we've
switched to memory APIs which don't return failure.
Switch all uses of reallocation of the array followed by
'virDomainDiskInsertPreAlloced' with direct virDomainDiskInsert.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The "machine-loadparm-multiple-disks-nets-s390" case now requires the
QEMU_CAPS_CCW feature to pass validation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We can skip the formatting of the bootindex for floppies directly at the
place where it's being formatted.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The logic assigning the bootindices from the legacy boot order
configuration was spread through the command line formatters for the
disk device and for the floppy controller.
This patch adds 'effectiveBootindex' property to the disk private data
which holds the calculated boot index and moves the logic of determining
the boot index into 'qemuProcessPrepareDomainDiskBootorder' called from
'qemuProcessPrepareDomainStorage'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Later patches will implement sharing of the backing file, so we'll need
to be able to discriminate the overlays per VM.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The code deals with the startup of the VM and just uses the snapshot
code to achieve the desired outcome.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We store the virQEMUDriverConfig object in the context.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remove all the arguments which are present in qemuSnapshotDiskContext.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The config is used both with the preparation and execution functions, so
we can store it in the context to simplify other helpers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Rather than filling various parts of the context from arguments pass in
the whole context.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Creating the overlay for the disk is needed when starting a new VM only.
Additionally for now migration with transient disks is forbidden
anyways.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The code will be later reused when adding support for sharing the
backing image of the snapshot.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
None of them are currently needed to pass our upstream CI, most were
either for ancient clang versions or coverity for silencing false
positives.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
They were added mostly randomly and we don't really want to keep working
around of false positives.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
After previous patches we have two structures:
virCapsHostNUMACellDistance and virNumaDistance which express the
same thing. And have the exact same members (modulo their names).
Drop the former in favor of the latter.
This change means that distances with value of 0 are no longer
printed out into capabilities XML, because domain XML code allows
partial distance specification and thus threats value of 0 as
unspecified by user (see virDomainNumaGetNodeDistance() which
returns the default LOCAL/REMOTE distance for value of 0).
Also, from ACPI 6.1 specification, section 5.2.17 System Locality
Distance Information Table (SLIT):
Distance values of 0-9 are reserved and have no meaning.
Thus we shouldn't be ever reporting 0 in neither domain nor
capabilities XML.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Expose virNumaDistance XML formatter so that it can be re-used by
other parts of the code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There's nothing domain specific about NUMA distances. Rename the
virDomainNumaDistance structure to just virNumaDistance.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virCapsHostNUMACellSiblingInfo structure really represents
distance to other NUMA node. Rename the structure and variables
of that type to make it more obvious.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This reverts commit <1b22dd6dd44202094e0f78f887cbe790c00e9ebc>.
First of all, the reverted commit is incomplete. It only sets
cpuset.mems in the VM root cgroup when the API is used but there is no
code that would do the same when the VM is started.
Libvirt never places any process into the VM root cgroup directly. All
the supporting processes like slirp-helper or dbus-daemon are placed
into the emulator sub-cgroup and all the QEMU threads are distributed
between emulator, vcpu* and iothread* sub-cgroups. The scenario
described in the reverted commit can happen only if someone manually
adds any process there which we should not care about.
If we would like to set the limit in the VM root cgroup we need to
introduce better logic:
- set both (old and new) numa group in the VM root cgroup
- change the numa group in all sub-cgroups to new value
- finally set only the new value in the VM root cgroup
The simplest fix now is to revert the commit.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The used libxl_domain_build_info, which is contained in
libxl_domain_config, is owned and already initialized by the caller.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The passed libxl_domain_create_info is owned, and already initialized,
by the caller.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
libxl objects are supposed to be initialized and disposed.
Correct the usage of libxl_device_disk objects which are allocated on
the stack. Initialize each one prior usage, and dispose them once done.
Adjust libxlMakeDisk to use an already initialized object, it is owned
by the caller.
Adjust libxlMakeDiskList to initialize the list of objects, before they
are filled by libxlMakeDisk. In case of error, the objects are disposed
by libxl_domain_config_dispose.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The description of the function says that the return value is a
file descriptor on success and negative errno on failure which is
not true. If the 'if' case with check on security labels fails,
the return value is -1 not -errno. The solution is to return
'-EINVAL' instead.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Previously, nvram file was created with user/group owner as
'root', rather than specifications defined in libvirtd.conf. The
solution is to call qemuDomainOpenFile(), which creates file with
defined permissions and qemuSecurityDomainSetPathLabel() to set
security label for created nvram file.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1783255
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
From QEMU docs/interop/qcow2.txt :
Byte 20 - 23: cluster_bits
Number of bits that are used for addressing an offset
within a cluster (1 << cluster_bits is the cluster size).
With this patch libvirt will be able to report the current cluster_size
for all existing storage volumes managed by storage driver.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The default value hard-coded in QEMU (64KiB) is not always the ideal.
Having a possibility to set the cluster_size by user may in specific
use-cases improve performance for QCOW2 images.
QEMU internally has some limits, the value has to be between 512B and
2048KiB and must by power of two, except when the image has Extended L2
Entries the minimal value has to be 16KiB.
Since qemu-img ensures the value is correct and the limit is not always
the same libvirt will not duplicate any of these checks as the error
message from qemu-img is good enough:
Cluster size must be a power of two between 512 and 2048k
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/154
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When building a commandline for a DIMM memory device with
non-default access mode, the qemuBuildMemoryBackendProps() will
tell QEMU to allocate memory from per-domain memory backing dir.
But later, when preparing the host, the
qemuProcessNeedMemoryBackingPath() does not check for memory
devices at all resulting in per-domain memory backing dir not
being created which upsets QEMU.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1961114
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We do not need to look for a suitable binary in the vhost-user
description files, if we aren't the ones starting it.
Otherwise startup will fail with:
error: Failed to start domain 'vm1'
error: operation failed: Unable to find a satisfying virtiofsd
https://bugzilla.redhat.com/show_bug.cgi?id=1855789
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU gained support for 'win-dmp' format in it's release of 3.0,
but libvirt doesn't implement it yet. Fortunately, there not much
needed: new value to virDomainCoreDumpFormat public enum, which
unfortunately means that QEMU driver has to be updated in the
same commit, because of VIR_ENUM_IMPL().
Luckily, we don't need any extra QEMU capability - the code
already checks supported formats via
'query-dump-guest-memory-capability' just before issuing
'dump-guest-memory'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Clang complains:
../libvirt/src/conf/node_device_conf.c:1945:74: error: result of comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-unsigned-enum-zero-compare]
if ((mdev->start = virNodeDevMdevStartTypeFromString(starttype)) < 0) {
Fixes: 42a5585499
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`ULLONG_MAX + value + 1`) for attribute `reg`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute, as it
refers to a 32 bit address space.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `aw_bits`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `id`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `latency`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virNetDevOpenvswitchInterfaceGetMaster is declared twice in
src/util/virnetdevopenvswitch.h. Remove the last one.
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This adds a new element to the mdev capabilities xml schema that
represents the start policy for a defined mediated device. The actual
auto-start functionality is handled behind the scenes by mdevctl, but it
wasn't yet hooked up in libvirt.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The passed libxl_domain_config is owned, and already initialized, by the
caller.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The initial variant of libxlDomainChangeEjectableMedia could just leave
the function earlier. With refcounting this does not work anymore.
Fixes commit a5bf06ba34
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `number`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `port`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `timeout`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attributes `cyls`, `heads` and `secs`.
Allowing negative numbers to be interpreted this way makes no sense for
these attributes.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `startport`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Since Xen 4.5 libxl allows to set affinities during domain creation.
This enables Xen to allocate the domain memory on NUMA systems close to
the specified pcpus.
Libvirt can now handle <domain/cputune/vcpupin> in domU.xml correctly.
Without this change, Xen will create the domU and assign NUMA memory and
vcpu affinities on its own. Later libvirt will adjust the affinity,
which may move the vcpus away from the assigned NUMA node.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The aim of this function is to return whether domain definition
and/or memory device that user intents to hotplug needs a private
path inside cfg->memoryBackingDir. The rule for the memory device
that's being hotplug includes checking whether corresponding
guest NUMA node needs memoryBackingDir. Well, while the rationale
behind makes sense it is not necessary to check for that really -
just a few lines above every guest NUMA node was checked exactly
for that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The aim of qemuProcessNeedHugepagesPath() is to return whether
guest needs private path inside HugeTLBFS mounts (deducted from
domain definition @def) or whether the memory device that user is
hotplugging in needs the private path (deducted from the @mem
argument). The actual creation of the path is done in the only
caller qemuProcessBuildDestroyMemoryPaths().
The rule for the first case (@def) and the second case (@mem) is
the same (domain has a DIMM device that has HP requested) and is
written twice. Move the logic into a function to deduplicate the
code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
As of b4cbdbe90b (and friends) the
minimal QEMU version required is 2.11.0. Let's update our
QEMU_MIN_* macros to reflect that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The basic use case of VIR_IDENTITY_AUTORESTORE() is in
conjunction with virIdentityElevateCurrent(). What happens is
that virIdentityElevateCurrent() gets current identity (which
increases the refcounter of thread local virIdentity object) and
returns a pointer to it. Later, when the variable goes out of
scope the virIdentityRestoreHelper() is called which calls
virIdentitySetCurrent() over the old identity. But this means
that the refcounter is increased again.
Therefore, we have to explicitly decrease the refcounter by
calling g_object_unref().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Recently, a new code was added to virGetConnectGeneric() that
saves the original error into a variable so that it's not lost in
virConnectClose() called under the 'error' label.
However, the error saving code uses virSaveLastError() +
virSetError() combo which leaks the memory allocated for the
error copy. Using virErrorPreserveLast() + virErrorRestore() does
the same job without the memleak.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
A secret can be marked with the "private" attribute. The intent was that
it is not possible for any libvirt client to be able to read the secret
value, it would only be accesible from within libvirtd. eg the QEMU
driver can read the value to launch a guest.
With the modular daemons, the QEMU, storage and secret drivers are all
running in separate daemons. The QEMU and storage drivers thus appear to
be normal libvirt client's from the POV of the secret driver, and thus
they are not able to read a private secret. This is unhelpful.
With the previous patches that introduced a "system token" to the
identity object, we can now distinguish APIs invoked by libvirt daemons
from those invoked by client applications.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When fetching the value of a private secret, we need to use an elevated
identity otherwise the secret driver will deny access.
When using the modular daemons, the elevated identity needs to be active
before the secret driver connection is opened, and it will apply to all
APIs calls made on that conncetion.
When using the monolithic daemon, the identity at time of opening the
connection is ignored, and the elevated identity needs to be active
precisely at the time the virSecretGetValue API call is made.
After acquiring the secret value, the elevated identity should be
cleared.
This sounds complex, but is fairly straightfoward with the automatic
cleanup callbacks.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The drivers can all call virGetConnectXXX to open a connection to a
secondary driver. For example, when creating a encrypted storage volume,
the storage driver has to open a secret driver connection, or when
starting a guest, the QEMU driver has to open the network driver to
lookup a virtual network.
When using monolithic libvirtd, the connection has the same effective
identity as the client, since everything is still in the same process.
When using the modular daemons, however, the remote daemon sees the
identity of the calling daemon. This is a mistake as it results in
the modular daemons seeing the client with elevated privileges.
We need to pass on the current identity explicitly when opening the
secondary drivers. This is the same thing that is done by daemon RPC
dispatcher code when it is directly forwarding top level API calls
from virtproxyd and other daemons.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is essentially a way to determine if the current identity
is that of another libvirt daemon.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When talking to the secret driver, the callers inside libvirt daemons
need to be able to run with an elevated privileges that prove the API
calls are made by a libvirt daemon, not an end user application.
The virIdentityElevateCurrent method will take the current identity
and, if not already present, add the system token. The old current
identity is returned to the caller. With the VIR_IDENTITY_AUTORESTORE
annotation, the old current identity will be restored upon leaving
the codeblock scope.
... early work with regular privileges ...
if (something needing elevated privs) {
VIR_IDENTITY_AUTORESTORE virIdentity *oldident =
virIdentityElevateCurrent();
if (!oldident)
return -1;
... do something with elevated privileges ...
}
... later work with regular privileges ...
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When creating the system identity set the system token. The system
token is currently stored in a local path
/var/run/libvirt/common/system.token
Obviously with only traditional UNIX DAC in effect, this is largely
security through obscurity, if the client is running at the same
privilege level as the daemon. It does, however, reliably distinguish
an unprivileged client from the system daemons.
With a MAC system like SELinux though, or possible use of containers,
access can be further restricted.
A possible future improvement for Linux would be to populate the
kernel keyring with a secret for libvirt daemons to share.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We want a way to distinguish between calls from a libvirt daemon, and a
regular client application when both are running as the same user
account. This is not possible with the current set of attributes
recorded against an identity, as there is nothing that is common to all
of the modular libvirt daemons, while distinct to all other processes.
We thus introduce the idea of a system token, which is simply a random
hex string that is only known by the libvirt daemons, to be recorded
against the system identity.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
A random token is simply a string of random bytes formatted in
hexidecimal.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The width of `unsigned long` differs on 32 bit and 64 bit architectures.
There is no compelling reason why the maximum DHCP lease time should
depend on the architecture.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When QEMU introduces new firmware features libvirt will fail until we
list that feature in our code as well which doesn't sound right.
We should simply ignore the new feature until we add a proper support
for it.
Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Some variables are needed only inside for() loop. They were
declared at the beginning of the function because of VIR_FREE()
calls, but since they are auto-freed they can be declared inside
the loop.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
What this function really does it takes ownership of all pointers
passed (well, except for the first one - caps - to which it
registers new NUMA node). But since all info is passed as a
single pointer it's hard to tell (and use g_auto*). Let's use
double pointers to make the ownership transfer obvious.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The @cpus variable is an array of structs in which each item
contains a virBitmap member. As such it is not enough to just
VIR_FREE() the array - each bitmap has to be freed too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The rest of virCapabilities format functions take virBuffer as
the first argument and struct to format as the second. Also, they
accept NULL (as the second argument). Fix
virCapabilitiesHostNUMAFormat() so that it follows this logic.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since its introduction in v0.9.1~65 the virOnce() was expected to
follow the usual retval logic (0 for success, a negative number
for failure). However, that was never the case.
On the other hand, looking into glibc and musl the pthread_once()
never returns anything other than zero (uclibc-ng seems to not
implement pthread_once()), therefore we never really hit any
problem. But for code cleanliness (and to match POSIX
documentation), let's change to code so that our retval logic is
honoured.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are few cases where we set a default value when using
virXMLPropEnum which can be converted to virXMLPropEnumDefault.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The helper is almost identical to virXMLPropEnum but it allows to pass a
default value to initialize the result to.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 8391cfbc2d converted the code to use virXMLPropEnum unfaithfully
ommitting the check where 'backend' must be non-zero when parsed from the
user.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 38180f87f5 converted the code to use virXMLPropEnum unfaithfully
ommitting the check where 'format' must be non-zero when parsed from the
user.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
In two cases the code needed to be adjusted to preserve functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virXMLPropTristateBool already initializes the value to
VIR_TRISTATE_BOOL_ABSENT so we no longer need to do that for certain
local variables.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Set the backup mode to VIR_TRISTATE_BOOL_YES after virXMLPropTristateBool
left it set to VIR_TRISTATE_BOOL_ABSENT. This will allow fixing
virXMLPropTristateBool to always initialize @result.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
This is done by adding a @defaultResult argument to virXMLPropInt since
many places have a non-0 default.
In certain cases such as in virDomainControllerDefParseXML we pass the
value from the original value, which will still trigger compiler checks
if unused while preserving the existing functionality of keeping the
previous value.
This commit fixes 3 uses of uninitialized value parsed by this function:
in virDomainDiskSourceNetworkParse introduced by 38dc25989c
in virDomainChrSourceDefParseTCP introduced by fa48004af5
in virDomainGraphicsListenDefParseXML introduced by 0b20fd3754
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Automatically free 'iothrid' and remove all the cleanup cruft.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Register virDomainIOThreadIDDefFree to do the cleanup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
VIR_XML_PROP_NONE has value of 0 so it's pointless to include it in an
binary-or expression.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Assign the vcpu count when virXMLPropUInt returns '0' meaning that the
cpu count was not present in the XML. This will allow to always
initialize the value of @result in virXMLPropUInt to prevent use of
uninitialized values.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Compilers aren't able to see whether @result is set or not and thus
don't warn of a potential use of uninitialized value. Always set @result
to prevent uninitialized use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virXMLPropTristateBool/virXMLPropTristateSwitch/virXMLPropEnum can be
implemented using the same internal code. Extract it into a new function
called virXMLPropEnumInternal, which will also simplify adding versions
of these functions with a custom default value.
This way we'll be able to always initialize @result so that unused value
bugs can be prevented.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attributes `voices` (typically 1),
`bufferLength` (measured in milliseconds), `frequency` (in Hz, typically
44100), and `channels` (typically 2 for stereo).
None of these properties benefit from or have a sensible use-case for
wrap-around behavior.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Attributes are mandatory and were incorrectly made optional recently.
Fixes: 2a5e16398e
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It was always allowed, but in a very unusual and weird way. Just
look at the original commit that introduced it (78fc843c7b).
Also, we document that "io" value is accepted (which translates
to VIR_DOMAIN_VIDEO_VGACONF_IO with value of zero).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
While reworking the patch I've mistakenly mangled the attribute
names for VIR_DOMAIN_CHR_TYPE_NMDM.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Commit fc7e1b2f03 which refactored the
video driver parse helper introduced a use of uninitialized variable,
which caused test failure at least when compiled with clang.
Pass 'def->vgaconf' directly to virXMLPropEnum. 'def' needs to be
converted to use g_autofree to handle error scenarios.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
xc_get_max_cpus from Xen version 4.3 may return 0 in case xc_physinfo
fails. This has been fixed in Xen 4.4. Remove the obsolete result check
from libvirt. Just convert libxl error codes to plain -1.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
In Xen 4.2 struct libxl_event_hooks had a member which was erroneously
declared const. Since libvirt requires at least Xen 4.6, remove the dead
code.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
The feature is present in all supported qemu versions (>2.11) and there
isn't a reasonable way to detect it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The feature is present in all supported qemu versions (>2.11) and there
isn't a reasonable way to detect it.
In addition the capability wasn't even used to gate any functionality
except for reporting the presence in the domain capabilities XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The feature is present in all supported qemu versions (>2.11) and there
isn't a reasonable way to detect it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The feature is present in all supported qemu versions (>2.11) and there
isn't a reasonable way to detect it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The feature is present in all supported QEMU versions and there isn't a
more elegant way to detect it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All supported qemus have it, there isn't an elegant way to detect it and
it's unlikely to be ever removed on purpose.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
'query-commandline-options' never returned 'vmport' but we can detect it
in the list of supported object types. This removes it from all non-x86
originating test data as it's platform specific.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The feature is no longer asserted. Remove the checks related to it and
make the code work properly with QEMU_CAPS_DEVICE_INTEL_IOMMU.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All supported QEMU versions now support query-qmp-schema. In the future
it will be possible to use the output of query-qmp-schema to also detect
commands reliably.
Since we are at the point where we have the least amount of .replies
files needing changing for a long time, move the 'query-qmp-schema' bits
before 'query-commands' to prepare for the future.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Modern code uses QMP schema to query for active commit support.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Modern code uses QMP schema to query for supported event types.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All supported qemu versions have 'query-qmp-schema' so we can remove the
check whether it exists and all logic conntected to it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remove the slot reservation for the vga card which doesn't make sense
with supported qemus any more for the q35 machine type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Remove the slot reservation for the vga card which doesn't make sense
with supported qemus any more for the i440fx machine type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All supported qemus now support using '-device' for adding a graphics
device.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
These conveniently don't have any test fallout.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Until we clean up and remove all capabilities which no longer make sense
to have separately, we should use virQEMUCapsInitQMPBasicArch to set the
defaults as it's used by qemuxml2argvtest when testing with fake
capabilities.
This allows us to prevent testing dead code paths with the fake
capability tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Move it under AARCH 64, since it's a platform specific feature, thus it
will be removed from all other platforms.
Since virQEMUCapsInitQMPBasicArch is used in qemuxml2argv test to
initiate qemuCaps for tests with fake capabilities, all the tests gain
GIC support now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT and
QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT are now always asserted on PPC
machine types, move them to virQEMUCapsInitQMPBasicArch.
It's now always set for AARCH64, move it into the function setting basic
caps for the emulator.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
It's now always set for AARCH64, move it into the function setting basic
caps for the emulator.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Now that minimum supported qemu version is 2.11, we can remove the
conditions.
Note that the check enabling QEMU_CAPS_TCG was for < 2.10.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
We no longer have to mask out IOMMU and NVDIMM support as we no longer
support the broken qemu versions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The qemuCaps is left for the device commandline formatters for now as it
might come in handy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
All machine types which have PCI support multibus since qemu 2.0
according to the logic we had, thus we can remove all the machine type
and version checks which are now dead code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
These strings are not constant really. They are allocated in
qemuDomainObjBeginJobInternal() and freed in
qemuDomainReset*Job(). Freeing a pointer to const looks weird.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`.
`bufferCount` does not benefit from being referable as e.g. "-7" for
requesting 4294967289 buffers, as this value is distinctly out of range
for normal use.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `retries`. UINT_MAX holds no
special significance for this attribute and is distinctly out of range
for normal use.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `id`.
`id` must be greater than 0 and does not benefit from being referable as
e.g. "-7" for host audio backend 4294967289, as this value is distinctly
out of range for normal use.
Additionally, this patch fixes a use of NULL string with printf's %s
modifier if the `model` attribute is absent.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
When updating entries in a bridge forwarding database (i.e., when
macTableManager='libvirt' is configured for the bridge), we may end up
in a situation when the entry we want to add is already present. Let's
just ignore the error in such a case.
This fixes an error to resume a domain when fdb entries were not
properly removed when the domain was paused:
virsh # resume test
error: Failed to resume domain test
error: error adding fdb entry for vnet2: File exists
For some reason, fdb entries are only removed when libvirt explicitly
stops CPUs, but nothing happens when we just get STOP event from QEMU.
An alternative approach would be to make sure we always remove the
entries regardless on why a domain was paused (e.g., during migration),
but that would be a significantly more disruptive change with possible
side effects.
https://bugzilla.redhat.com/show_bug.cgi?id=1603155
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Commit 28a8699316 ( v6.9.0-179-g28a8699316 ) incorrectly replaced
VIR_EXPAND_N by g_renew.
VIR_EXPAND_N has these two extra effects apart from reallocating memory:
1) The newly allocated memory is zeroed out
2) The number of elements in the array which is passed to VIR_EXPAND_N
is increased.
This comes into play when used with virDomainLeaseInsertPreAlloced,
which expects that the array element count already includes the space
for the added 'lease', by plainly just assigning to
'leases[nleases - 1]'
Since g_renew does not increase the number of elements in the array
any existing code which calls virDomainLeaseInsertPreAlloced thus either
overwrites a lease definition or corrupts the heap if there are no
leases to start with.
To preserve existing functionality we revert the code back to using
VIR_EXPAND_N which at this point doesn't return any value, so other
commits don't need to be reverted.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1953577
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
virCommandRun() already handles the case where the cmd argument is NULL,
so there's no need for the caller to check. Make all callers consistent
and remove unnecessary NULL checks.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Coverity complained that the 'default' case of the switch in
nodeDeviceGetMdevctlCommand() was falling through without initializing
'cmd'. Return NULL in this case even though it should never happen.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When returning early due to errors, cmd will be leaked. Use an autoptr
to handle these early returns without leaking memory.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jumping back in the code is an anti-pattern that should be avoided if
possible.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We don't require that the data is consistent on the destination if
aborting the migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rename the parameter so that it's more clear what state we are in and
fix all callees.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We want to unify on one block job cancellation API. Use
qemuMonitorBlockJobCancel which has more features.
In case of job refresh, we are killing off any unknown jobs so we don't
care about their fate.
Another difference is that an possible error from the block job
cancellation might be reported, but we don't really care here ince
it's a very unlikely scenario and we also report a warning.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We want to unify on one block job cancellation API. Use
qemuMonitorBlockJobCancel which has more features.
In case of backup jobs we can cancel the jobs forcefully since the code
is on a cleanup path when the job fails.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'block-job-cancel' has one very important semantic difference to
'job-cancel', docummented in qemu as:
Note that if you issue 'block-job-cancel' after 'drive-mirror' has indicated
(via the event BLOCK_JOB_READY) that the source and destination are
synchronized, then the event triggered by this command changes to
BLOCK_JOB_COMPLETED, to indicate that the mirroring has ended and the
destination now has a point-in-time copy tied to the time of the cancellation.
Since libvirt advertises the block copy job as having the synchronous
abort feature we must not use 'job-cancel' here.
Fixes: 4817b5ca1d
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In certain cases such as when aborting migration we don't really care
for completion of the blockjob. Add 'force' as parameter of
'block-job-cancel'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory freeing and remove the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Don't try to setup disk migration and the NBD stuff if we end up
migrating nothing.
The destination side has luckily no setup for the non-NBD cases so
omitting the element fully is okay.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Don't even try to setup storage migration if there are no eligible
disks.
This also fixes migration from older libvirts which didn't format an
empty <nbd/> element in the migration cookie if there weren't any disks
to migrate.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Base the decision on the main API flags (VIR_MIGRATE_NON_SHARED_DISK,
QEMU_MONITOR_MIGRATE_NON_SHARED_INC) via a boolean 'storageMigration'
rather than juggling everything trhough 'migration_flags'.
After this patch 'migration_flags' is updated to contain the legacy
storage migration flags only when we'll be about to use it rather than
setting it and then resetting it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'migrate_flags' can be updated in the only caller and since
qemuMigrationSrcNBDStorageCopy already takes @flags which contains
VIR_MIGRATE_NON_SHARED_INC (used to set
QEMU_MONITOR_MIGRATE_NON_SHARED_INC) we can completely remove the
parameter.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In case the 'nbdURI' schema is not known the code would report an error
but wouldn't return failure.
Fixes: 49186372db
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 518be41aaa refactored qemuMigrationCookieNBDXMLFormat to use
virXMLFormatElement which in comparison to the previous code doesn't
format the element if it's empty.
Unfortunately some crusty bits of our migration code use questionable
logic to assert use of the old-style storage migration parameters which
breaks if no disks are being migrated and the <nbd/> element is not
present.
While later patches will fix the code, re-instate formatting of empty
<nbd/> for increased compatibility.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add a helper which will format an XML element with attributes and
children, but compared to virXMLFormatElement it also formats an empty
element if both buffers are empty.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Function incorrectly returns 0 when property was successfully read.
Fixes: ab5d2776c9
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
When placing vCPUs into CGroups the qemuProcessSetupPid() is
called which then enters a for() loop (around its middle) where
it calls virDomainNumaGetNodeCpumask() for each guest NUMA node.
But the latter returns only a pointer not new reference/copy and
thus the caller must not free it. But the variable is decorated
with g_autoptr() which leads to a double free.
Fixes: 2d37d8dbc9
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
libssh2 has ECDSA and ED25519 support beginning with v1.9.0. libvirt cannot
make use of those because it will handle them as unknown key types.
Add support for those host key types.
Signed-off-by: Bastian Germann <bastiangermann@fishpost.de>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Calling VIR_FREE on a virDomainDef* does not free its various contained
pointers.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Preparatory step to remove virDomainChrSourceDefParseMode.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is the bug I'm facing. I deliberately configured a container
so that the source of a <filesystem/> to passthrough doesn't
exist. The start fails with:
lxcContainerPivotRoot:669 : Failed to create /non-existent/path/.oldroot: Permission denied
which is expected. But what is NOT expected is that CGroup
hierarchy is left behind. This is because the controller sets up
the CGroup hierarchy, user namespace, moves interfaces, etc. and
finally checks whether container setup (done in a separate
process) succeeded. Only after all this the error is propagated
to the LXC driver. The driver aborts the startup and tries to
perform the cleanup, but this is missing CGroups because those
weren't detected yet.
Ideally, whenever a function fails, it tries to unroll back so
that is has no artifacts left behind (look at all those frees/FD
closes/etc. at end of functions). But with CGroups it is
different - the controller process can't clean up after itself,
because it is still running inside that CGroup.
Therefore, what we have to do is to let the driver detect CGroups
as soon as they are created, and proceed with controller
execution only after that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Currently, there is only a single pipe passed to lxc_controller
and it is used by lxc_controller to signal to the LXC driver that
the container is set up and ready to run. However, in the next
commit we will need to signal that the LXC driver has done its
part of startup process and thus the controller can proceed.
Unfortunately, virCommand handshake can't be used for this,
because it's already used to read controller's PID.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Future commits will want to reuse the handshakeFd and thus it
mustn't be closed in virLXCControllerDaemonHandshake(). Do the
closing explicitly afterwards.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The lxc_controller has a structure that's keeping its internal
state, including so called handshakeFd which is the write end of
a pipe that's used to signal to the LXC driver that the container
is set up and ready to run. However, the struct member is not
initialized to -1, so if anything fails before it is set then the
virLXCControllerFree() function tries to close FD 0 (stdin).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Convenience function to return the value of an unsigned long long XML
attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
With the last usage of `aes` and `dea` as int gone, these two can
become virTristateSwitch.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This adds a new XML element
<filesystem>
<binary>
<sandbox mode='chroot|namespace'/>
</binary>
</filesystem>
This will be used by qemu virtiofs
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
The code in storage_backend_fs is used for storage_dir and storage_fs
drivers so some parts need to be guarded by checking for
WITH_STORAGE_FS.
Fixes: 16c69e7aae
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Copy the socket path in qemuExtDevicesStart, because
for libvirt-managed virtiofsd daemons the path is filled there
in qemuVirtioFSStart.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Allow passing a socket of an externally launched virtiofsd
to the vhost-user-fs device.
<filesystem type='mount'>
<driver type='virtiofs' queue='1024'/>
<source socket='/tmp/sock/'/>
</filesystem>
https://bugzilla.redhat.com/show_bug.cgi?id=1855789
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
So far VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH is always set
in virDomainFSDefPostParse, but future commits aim to change
that.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the default setting of accessmode to the post-parse phase.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `speed`, which does not make sense for
a value measured in Mbits per second.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
