When creating /dev nodes in a QEMU domain's namespace the first
thing we simply do is unlink() the path and create it again. This
aims to solve the case when a file changed type/major/minor in
the host and thus we need to reflect this in the guest's
namespace. Fair enough, except we can be a bit more clever about
it: firstly check whether the path doesn't already exist or isn't
already of the correct type/major/minor and do the
unlink+creation only if needed.
Currently, this is implemented only for symlinks and
block/character devices. For regular files/directories (which are
less common) this might be implemented one day, but not today.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When building namespace for a domain there are couple of devices
that are created independent of domain config (see
qemuDomainPopulateDevices()). The idea behind is that these
devices are crucial for QEMU or one of its libraries, or user is
passing through a device and wants us to create it in the
namespace too. That's the reason that these devices are allowed
in the devices CGroup controller as well.
However, during unplug it may happen that a device is configured
to use one of such devices and since we remove /dev nodes on
hotplug we would remove such device too. For example,
/dev/urandom belongs onto the list of implicit devices and users
can hotplug and hotunplug an RNG device with /dev/urandom as
backend.
The fix is fortunately simple - just consult the list of implicit
devices before removing the device from the namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Autofree the temporary string and shuffle around the success path to
avoid the 'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The NBD connection for non-shared storage migration can have the same
issue regarding TLS certificate name match as the migration connection
itself.
Propagate the configured name also for the NBD connections.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1901394
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In cases when the hostname of the NBD server doesn't match the hostname
in the TLS certificate the new attribute 'tlsHostname' can be used to
override it.
Add the XML infrastructure and tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The value will be used to override the hostname used for validation of
TLS certificates.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We do support non-shared storage migration with TLS now. Fix the comment
claiming otherwise.
Fixes: a8dc146a4d
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Detect that qemu can override TLS hostname setting for NBD clients.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The block of code pausing the VM assigns 'resume' to true but it's
already true because of the previous condition.
The code is deliberately kept in two blocks as upcoming changes will
modify both conditions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor the code to use proper types for the memory and disk snapshot
location and fix the parsing code to be compatible with an unsigned
type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Separate the steps of parsing the memory snapshot config from the
post-processing and validation code. The upcoming patch refactoring the
parsing will be simpler.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Assign directly into the definition. The cleanup code can deal with
that.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory cleanup, decrease scope of variables and remove the
'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All callers except the one in the 'esx' driver pass the flag. The 'esx'
driver has a check that 'def->ndisks' is zero after parsing the
definition. This means that we can simply always parse the disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to the external snapshot code the internal inactive snapshot
creation helper should act only when an internal snapshot of the disk is
required. For now the callers ensure that it's either _INTERNAL or _NO
when control reaches this function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The string value associated to the enum is "no". Rename the enum
accordingly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The snapshot location enum is also needed for the disk definition so if
we house it inside domain_conf we can use the proper type for it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use 'virStorageType' as type for the 'type' member and convert the code
to work properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor the function to avoid the cleanup section used to just free
memory associated with the parsed object.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use an if/else branch rather than a expression with a ternary operator.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Preparation steps ensure that the 'snapshot' field can only be
'VIR_DOMAIN_SNAPSHOT_LOCATION_NONE' or
VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL' at this point, but upcoming
patches will change that. Handle only external snapshots.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For the various structs storing lists of objects, the access
to the hash tables is not lockless. The mutex on the object
owning the hash table must be held.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Long ago we adapted to Linux kernel changes which inverted the
behaviour of the conntrack --ctdir setting:
commit a6a04ea47a
Author: Stefan Berger <stefanb@us.ibm.com>
Date: Wed May 15 21:02:11 2013 -0400
nwfilter: check for inverted ctdir
Linux netfilter at some point (Linux 2.6.39) inverted the meaning of the
'--ctdir reply' and newer netfilter implementations now expect
'--ctdir original' instead and vice-versa.
We check for the kernel version and assume that all Linux kernels with version
2.6.39 have the newer inverted logic.
Any distro backporting the Linux kernel patch that inverts the --ctdir logic
(Linux commit 96120d86f) must also backport this patch for Linux and
adapt the kernel version being tested for.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Given our supported platform targets, we no longer need to
consider a version of Linux before 2.6.39, so can drop
support for the old direction behaviour.
The test suite updates are triggered because that never
probed for the ctdir direction, and so the iptables syntax
generator unconditionally dropped the ctdir args.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Long ago we adapted to iptables changes by introducing support
for '-m conntrack':
commit 06844ccbaa
Author: Stefan Berger <stefanb@us.ibm.com>
Date: Tue Aug 6 20:30:46 2013 -0400
nwfilter: Use -m conntrack rather than -m state
Since iptables version 1.4.16 '-m state --state NEW' is converted to
'-m conntrack --ctstate NEW'. Therefore, when encountering this or later
versions of iptables use '-m conntrack --ctstate'.
Given our supported platform targets, we no longer need to
consider a version of iptables before 1.4.16, so can drop
support for the old syntax.
The test suite updates are triggered because that never
probed for the new syntax, and so unconditionally
generated the old syntax.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
QEMU needs to read, write, and lock the NVRAM *.fd files with UEFI
firmware.
Fixes: https://bugs.debian.org/1006324
Fixes: https://launchpad.net/bugs/1962035
Signed-off-by: Martin Pitt <mpitt@debian.org>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
In plenty of places we mention qemu, Qemu but the correct form is
all capitals.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Nothing in daemon code is prepared for the command in
virDomainQemuMonitorCommandWithFiles() to be NULL. In fact, the
client side doesn't expect this either as our RPC describes the
argument as:
remote_nonnull_string cmd;
Validate the argument in the public API implementation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virNWFilterTechDriverForName & virNWFilterUpdateInstantiateFilter
methods are only used within the same source file, so don't need to
be exported.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This method doesn't exist since
commit d1a7c08eb1
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Thu Apr 26 12:26:51 2018 +0100
nwfilter: convert the gentech driver code to use virNWFilterBindingDefPtr
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Now that the virNWFilterBinding APIs are using the nwfilter
update lock directly, there is no need for the virt drivers
to do it themselves.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The nwfilter update lock is historically acquired by the virt
drivers in order to achieve serialization between nwfilter
define/undefine, and instantiation/teardown of filters.
When running in the modular daemons, however, the mutex that
the virt drivers are locking is in a completely different
process from the mutex that the nwfilter driver is locking.
Serialization is lost and thus call from the virt driver to
virNWFilterBindingCreateXML can deadlock with a concurrent
call to the virNWFilterDefineXML method.
The solution is surprisingly easy, the update lock simply
needs acquiring in the virNWFilterBindingCreateXML method
and virNWFilterBindingUndefine method instead of in the
virt drivers.
The only semantic difference here is that when a virtual
machine has multiple NICs, the instantiation and teardown
of filters is no longer serialized for the whole VM, but
rather for each NIC. This should not be a problem since
the virt drivers already need to cope with tearing down
a partially created VM where only some of the NICs are
setup.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Now that we have support for fuse-3 we can detect it during the
configure phase. Even better, we can detect fuse-3 first and
fallback to old fuse only if the newer version doesn't exist.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Plenty of projects switch from FUSE to FUSE3. This commit enables
libvirt to compile with newer fuse-3.1 which allows users to have
just one fuse package on their systems, allows us to set
O_CLOEXEC on the fuse session FD. In general, FUSE3 offers more
features, but apparently we don't need them right now. There is a
rewrite guide at [1] but I've took most inspiration from sshfs
[2].
1: https://github.com/libfuse/libfuse/releases/tag/fuse-3.0.0
2: https://github.com/libfuse/sshfs
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If an app within a container wishes to read from /proc/meminfo
from a different position than the beginning of the file, we can
have FUSE keep track of all the lseek()-s and reflect them in
@offset argument of read callback (lxcProcRead()). This is done
by setting fuse_file_info::nonseekable. If we don't do this, then
FUSE reports errors back the app that does lseek().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When mounting a FUSE it is possible to bypass kernel cache by
specifying -odirect_io mount option. This is what we currently
do. However, FUSEv3 has a different approach - the open callback
(lxcProcOpen() in our case) can set direct_io member of
fuse_file_info struct. This results in the same behaviour, but
also works with both FUSEv1 and FUSEv3. The latter does not have
the mount option and uses per file approach.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The idea behind lxcProcReadMeminfo() is that we read the host's
/proc/meminfo and copy it line by line producing the content for
container, changing only those lines we need. Thus, when a
process inside container opens the file and lseek()-s to a
different position (or reads the content in small chunks), we
mirror the seek in host's /proc/meminfo. But this doesn't work
really. We are not guaranteed to end up aligned on the beginning
of new line. It's better if we construct the new content and then
mimic seeking in it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In the lxcProcReadMeminfo() function we have @buffer variable
which is statically allocated and then @new_meminfo which is just
a pointer to the @buffer. This is needless, the @buffer can be
accessed directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After previous cleanups, the cleanup label is no longer needed
and can be removed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are two functions (lxcProcHostRead() and
lxcProcReadMeminfo()) that could benefit from automatic file
closing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In lxcProcReadMeminfo() there's a variable named @fd which would
suggest it's type of int, but in fact it's type of FILE *. Rename
it to @fp to avoid confusion.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In the lxcSetupFuse() function there are multiple cleanup labels,
but with a bit of rewrite they can be joined into one 'error'
label. And while at it, set the @f argument only in the
successful path (currently is set in error case too).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In lxcProcOpen() we want to check whether the /proc/memfile is
being opened only for read. For that we check the fi->flags which
correspond to flags open() call. Instead of explicitly masking
the last two bits use O_ACCMODE constant, which is deemed to be
more portable.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our style of writing function declarations has changed since the
time the file was introduced. Fix the whole file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are few arguments that are marked as G_GNUC_UNUSED even
though they are clearly used within their respective functions.
Drop the annotation in such cases.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There is no need to include the fuse.h from the header file.
Move the include into the lxc_fuse.c then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Nothing in the lxc_fuse.h header file warrants inclusion of
lxc_conf.h. If anything, virconftypes.h must be included because
of virDomainDef required by lxcSetupFuse().
It's actually lxc_fuse.c that requires some macros from
lxc_fuse.h (e.g. LXC_STATE_DIR).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function that fills virLXCMeminfo struct
(virLXCCgroupGetMeminfo()) lives in lxc_cgroup.h. Move the struct
there too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This structure is not used outside of lxc_fuse.c. There is no need
to define it in the header file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The returned packet can have less strict alignment (u_char) than the struct
(ether_header) we are casting it to, so to avoid alignment issues just copy the
header into the struct on the stack.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
That is the proper POSIX way.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
That is the proper POSIX way.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Switch the operands in the loop condition to make it converge.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In virSystemdActivationClaimFDs, the memory of ent->fds has been stolen
and stored in fds, but fds is never freed, which causes a memory leak.
Fix it by declaring fds as g_autofree.
Reported-by: Jie Tang <tangjie18@huawei.com>
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When I implemented user aliases I've invented this
virDomainDefFeatures flag so that individual drivers can signal
support for user provided aliases. The reasoning was that a
device alias might be part of guest ABI, or used in a different
way then in QEMU. Well, neither applies to the libxl driver, so
it's safe to allow user aliases there.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/231
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Add support for sending one FD from the client along with a monitor
command so that it's possible to use 'getfd' and 'add-fd' to use FDs
passed from the client with other QMP commands.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This API has the same semantics as 'virDomainQemuMonitorCommand' but
accepts file descriptors which are then forwarded to qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The helper splits out the clearing of the FDs transacted inside a
virNetMessage.
APIs transacting FDs both from and to the client at the same time will
need to clear the FDs stored in virNetMessage as the structure is
re-used for the reply and without clearing the list of FDs we'd return
the FDs sent by the client in addition to the new FDs sent by the API.t
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'qemuDomainPrepareDiskSourceData' propagates 'detect_zeroes' only for
the disk source image, but the mirror destination has the ambition to
replace the disk source when the job is finished, so we need to
propagate the 'detect_zeroes' setting also in that case.
Unfortunately it would become very hairy to either set 'disk->mirror'
sooner or propagate that we want this done into
'qemuDomainPrepareDiskSourceData', so the most straightforward solution
is to do the propagation inside 'qemuDomainBlockCopyCommon'.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/277
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It makes no sense to have 'started' variable in the
libxlDomainJobObj as the same one is already in virDomainJobData,
but never used.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently the 'nvram_template' entry is mandatory when parsing the
firmware descriptor based on flash. QEMU is extending the firmware
descriptor spec to make the 'nvram_template' optional, depending
on the value of a new 'mode' field:
- "split"
* "executable" contains read-only CODE
* "nvram_template" contains read-write VARS
- "combined"
* "executable" contains read-write CODE and VARs
* "nvram_template" not present
- "stateless"
* "executable" contains read-only CODE and VARs
* "nvram_template" not present
In the latter case, the guest OS can write vars but the
firmware will make no attempt to persist them, so any changes
will be lost at poweroff.
For now we parse this new 'mode' but discard any firmware
which is not 'mode=split' when matching for a domain.
In the tests we have a mixture of files with and without the
mode attribute.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When undefining a VM, we must optionally delete any NVRAM that might
exist. When using firmware auto-select we always check the generated
path, ignoring any user specified path.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 4e42686ade wrongly assumed how g_variant_new_parsed() works and broke
starting of domains on systems with systemd (machined).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
After v8.0.0-466-g08101bde5d we unconditionally regenerate per
domain NVRAM path even though it might have been parsed earlier
from domain XML. The way we do that leads to a memleak:
43 bytes in 1 blocks are definitely lost in loss record 330 of 682
at 0x483F7E5: malloc (vg_replace_malloc.c:381)
by 0x50D5B18: g_malloc (in /usr/lib64/libglib-2.0.so.0.7000.2)
by 0x50EFA4F: g_strdup (in /usr/lib64/libglib-2.0.so.0.7000.2)
by 0x49E774E: virXPathString (virxml.c:88)
by 0x4A3F0E4: virDomainDefParseBootLoaderOptions (domain_conf.c:18226)
by 0x4A3F49C: virDomainDefParseBootOptions (domain_conf.c:18298)
by 0x4A448C3: virDomainDefParseXML (domain_conf.c:19598)
by 0x4A487A1: virDomainDefParseNode (domain_conf.c:20404)
by 0x117FCF: testCompareXMLToArgv (qemuxml2argvtest.c:726)
by 0x142124: virTestRun (testutils.c:142)
by 0x1423D4: virTestRunLog (testutils.c:197)
by 0x140A76: mymain (qemuxml2argvtest.c:3406)
If we parsed NVRAM path from domain XML we must refrain from
generating new path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In case when a user starts a block copy operation with
VIR_DOMAIN_BLOCK_COPY_SHALLOW and VIR_DOMAIN_BLOCK_COPY_REUSE_EXT and
both the reused image and the original disk have a backing image libvirt
specifically does not insert the backing image until after the job is
asked to be completed via virBlockJobAbort with
VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT.
This is so that management applications can copy the backing image on
the background.
Now when a user aborts the block job instead of cancelling it we'd
ignore the fact that we didn't insert the backing image yet and the
cancellation would result into a 'blockdev-del' of a invalid node name
and thus an 'error' severity entry in the log.
To solve this issue we use the same conditions when the backing image
addition is avoided to remove the internal state for them prior to the
call to unplug the mirror destination.
Reported-by: Kashyap Chamarthy <kchamart@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When building the default memory backend (which has id='pc.ram')
and no guest NUMA is configured then
qemuBuildMemCommandLineMemoryDefaultBackend() is called. However,
its return value is ignored which means that on invalid
configuration (e.g. when non-existent hugepage size was
requested) an error is reported into the logs but QEMU is started
anyway. And while QEMU does error out its error message doesn't
give much clue what's going on:
qemu-system-x86_64: Memory backend 'pc.ram' not found
While at it, introduce a test case. While I could chose a nice
looking value (e.g. 4MiB) that's exactly what I wanted to avoid,
because while such value might not be possible on x84_64 it may
be possible on other arches (e.g. ppc is notoriously known for
supporting wide range of HP sizes). Let's stick with obviously
wrong value of 5MiB.
Reported-by: Charles Polisher <chas@chasmo.org>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is to make it explicit that the template only applies to the NVRAM
store, not the main loader binary, even if the loader is writable.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Before creating a NVRAM path, the qemuDomainNVRAMPathGenerate
method checks whether the config is using the old style
firmware approach. This check is redundant in one of the two
callers. By inlining the check into the other caller, it makes
it clearer to understand that the NVRAM path filling is done
conditionally.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Since libvirt-guests script/service can operate on various URIs and we do
support both socket activation and traditional services, the ordering should be
specified for all the possible sockets and services.
Also remove the Wants= dependency since do not want to start any service. We
cannot know which one libvirt-guests is configured, so we'd have to start all
the daemons which would break if unused colliding services are not
masked (libvirtd.service in the modular case and all the modular daemon service
units in the monolithic scenario). Fortunately we can assume that the system is
configured properly to start services/sockets that are of interest to the user.
That also works with the setup described in https://libvirt.org/daemons.html .
To make it even more robust we add the daemon service into the machine units
created for individual domains as it was missing there.
https://bugzilla.redhat.com/show_bug.cgi?id=1868537
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are some enums that are declared in qemu_monitor.h but
implemented in qemu_monitor_json.c. While from compiler and
linker POV it doesn't matter, the code is cleaner if an enum is
implemented in .c file that corresponds to .h file which declared
the enum.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Using the following spatch, I've identified two places which
could be switched from explicit virDomainObjIsActive() +
virReportError() to virDomainObjCheckActive():
@@
expression dom;
@@
if (
- !virDomainObjIsActive(dom)
+ virDomainObjCheckActive(dom) < 0
) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running"));
...
}
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add calc_mode for dirtyrate statistics retured by
virsh domstats --dirtyrate api, also add vcpu dirtyrate
if dirty-ring mode was used in last measurement.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Extend flags parameter of virDomainStartDirtyRateCalc as a
superset of virDomainDirtyRateCalcFlags, parse the flags and
handle it correspondingly in qemuDomainStartDirtyRateCalc.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add mode parameter to qemuDomainStartDirtyRateCalc API, 'mode'
option of 'calc-dirty-rate' command was introduced since
qemu >= 6.2.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce virDomainDirtyRateCalcFlags to get ready for
adding mode parameter to qemuDomainStartDirtyRateCalc.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Probing QEMU_CAPS_CALC_DIRTY_RATE capability in advance
in case of failure when calculating dirty page rate.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit cc2a3c2a94 missed one case in the libxl driver where virDomainDef
is returned from libxlDomainSaveImageOpen and a g_steal_pointer is needed.
Without it, the virDomainDef object is freed and the driver crashes later
in the restore process when accessing the object.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The systemd version in RHEL-7 lacked support for the LISTEN_FDNAMES env
variable with socket activation. Since we stopped targetting RHEL-7 we
can drop some considerable amount of compatibility code.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
All APIs using FD passing have this check to prevent sending a
'VIR_NET_CALL_WITH_FDS' to an older daemon but
virDomainCreateXMLWithFiles was missing it.
Now the LXC driver was historically not exposing
VIR_DRV_FEATURE_FD_PASSING, but that is not a problem as LXC always goes
through the remote driver which intercepts it and injects
VIR_DRV_FEATURE_FD_PASSING when it was implemented.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The feature implies that fd passing works with RPC. Non-remote impls
thus should always report support.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The fix was on RPC level so everything should advertise it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
This was a lockout to make strings in typed parameters compatible with
versions which didn't have them. Now all drivers need to expose this
capability.
This namely enables it for 'esx' and 'vz' drivers, while they don't seem
to be implementing any parameters for now, they might later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
They are features of the RPC; drivers must say that it's not supported.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
keepalive is a RPC feature, drivers must say that it's not supported.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
VIR_DRV_FEATURE_REMOTE is a special flag which is asserted only when the
connection is remote. All drivers implementing it must return 0 for it
to work. Handle it in the global handler and add a comment why.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The 'virDrvFeature' has a combination of features which are asserted by
the specific driver and features which are actually global.
In many cases the implementation was cargo-culted into newer drivers
without re-assesing whether it makes sense.
This patch introduces a global function which will specifically handle
these global flags and defer the rest to the driver.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The destination daemon would crash in Finish phase due to NULL
dereference which I missed in my review of commit
v8.0.0-428-g0301db44e2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'qemuDomainSnapshotForEachQcow2Raw' doesn't properly handle the
'VIR_DOMAIN_SNAPSHOT_LOCATION_NONE' setting and thus doesn't skip disks
which were excluded from the snapshot due to being read-only.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When installing openrc init scripts, we take whatever mode the
generated files are in an copy them under /etc/init.d/. This is
not ideal, because those files are not executable and they should
be.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/250
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
This transition will make it easier for me to generalize jobs in
the future as they will always use virDomainJobData and
virDomainJobInfo will be only used in the public api..
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
We only need to set statsType in almost every case of setting
something from private data, so it seems unnecessary to pull
privateData out of current / completed job for just this one
thing every time. I think this patch keeps the code cleaner
without variables used just once.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This patch includes:
* introducing new files: src/hypervisor/domain_job.c and src/hypervisor/domain_job.h
* new struct virDomainJobData, which is almost the same as
qemuDomainJobInfo - the only differences are moving qemu specific
job stats into the qemuDomainJobDataPrivate and adding jobType
(possibly more attributes in the future if needed).
* moving qemuDomainJobStatus to the domain_job.h and renaming it
as virDomainJobStatus
* moving and renaming qemuDomainJobStatusToType
* adding callback struct virDomainJobDataPrivateDataCallbacks
taking care of allocation, copying and freeing of private data
of virDomainJobData
* adding functions for virDomainJobDataPrivateDataCallbacks for
qemu hypervisor
* adding 'public' (public between the different hypervisors) functions
taking care of init, copy, free of virDomainJobData
* renaming every occurrence of qemuDomainJobInfo *info to
virDomainJobData *data
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
We don't want to be dealing with real FDs thus we mock
'qemuMonitorIOWriteWithFD' to do the same thing as when no FD is being
passed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Adding an exception for the whole file usually defeats the purpose of a
syntax check and is also likely to get forgotten once the file is
removed.
In case of the suggestion of using 'safewrite' instead of write even the
comment for safewrite states that the function needs to be used only in
certain cases.
Remove the blanket exceptions for files and use an exclude string
instead. The only instance where we keep the full file exception is for
src/libvirt-stream.c as there are multiple uses in example code in
comments where I couldn't find a nicer targetted wapproach.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In order to mock the SCM_RIGHTS sendmsg to simulate sending
filedescriptors to fake qemu in tests we need access to some fields of
'struct _qemuMonitor'. Move its declaration to the private header file.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the declaration of the struct into 'qemu_monitor_priv.h' as other
code has no business in peeking into the monitor messages.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The fields are no longer used since we've deleted support for HMP-only
qemus. The HMP command pass-through works via a QMP command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit dc481f11a6 which converted the function generating properties
for disk '-device' argument to JSON removed the only other use of
qemuBuildDiskFrontendAttributeErrorPolicy, so we can now inline it into
qemuBuildDriveStr.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since 'cancel_path' is constructed from the 'tpmdev' argument, we can
push it down into the function opening the FDs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Automatically free 'path' inside the loop which fills it and return the
values directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Setup the chardev similarly to how we do it on startup so that virtlogd
is properly used with chardevs which are hotplugged to a VM.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When hotplugging a chardev we need the same form of setup for the
character device. Export a version which takes a 'virDomainDeviceDef'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
FD passing and TLS is normally setup via private data for the chardev
source. The monitor implementation didn't support it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our code uses fdsets for the pipe passed from virtlogd to qemu, but the
chardev hot-unplug code neglected to detach the fdset after the chardev
was removed. This kept the FDs open by qemu even after they were not
used any more.
After the refactor to use qemuFDPass for chardevs we now configure the
'opaque' field for fdsets used for chardevs so we can use
qemuHotplugRemoveFDSet to remove the unused fdset.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rewrite the parts which already pass FDs via fdset or directly to use
the new infrastructure.
Apart from simpler code this also adds the appropriate names to the fds
in the fdsets which will allow us to properly remove the fdsets won
hot-unplug of chardevs, which we didn't do for now and resulted in
leaking the FDs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Prefix the file descriptor name with the alias of the network device so
that it's similar to other upcoming use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For now we have only one code path ('vdpa' interface) which actually
cleans up the fdset after it's done, but there are more device types
using fdsets.
In order to unify the handling of fdsets the removal code will now be
able to remove fdsets based on a prefix of the 'opaque' field, which
we'll always prefix with a device alias or e.g. node name once fdsets
are also used for disk backing.
To keep compatibility with old QEMUs, retain the possibility for the
VDPA interface to use the path.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code unplugging the fdset for a 'vdpa' network device can be later
reused. Extract it into 'qemuHotplugRemoveFDSet'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the new helpers for passing of the file descriptor needed for 'vdpa'
interfaces.
Apart from the simplification in this case it will allow further changes
to unify all fdset handling.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The existing helpers we have are very clumsy and there's no integration
with the monitor.
This patch introduces new helpers to bridge the gap and simplify handing
of fdsets and classic FD passing when generating commandline/hotplug
arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When starting a VM we must assign unique IDs for fdsets we add via
'-add-fd'. For now it was done by using the index of the filedescriptor
passed to the virCommand. That approach is not very flexible, because
you need to have already passed the 'fd' to virCommand before generating
the fdset path, and also won't nicely work with fdsets containing two or
more fds.
This patch introduces a counter into the private data of a qemu domain
so that we can allocate unique ids without relying on virCommand.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to the 'qemuMonitorRemoveFdset', it doesn't make sense
to store it as signed when only unsigned values are expected.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'qemuMonitorRemoveFdset' validates that the 'fdset' argument isn't less
than 0. We can turn it to unsigned and thus avoid the error message
completely.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Caller passes 'driver->securityManager', and 'priv->qemuCaps' as
arguments along with 'vm', but both aforementioned objects are
accessible directly from 'vm'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Code paths which don't wish to use FD passing are supposed to not call
the function which sets up the chardev for FD passing.
This is ensured by calling it only in the host prepare step.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In a patch adding similarly named APIs I was asked to use 'ID' instead
of 'Id'. Since the code is being put together fix
qemuDomainStorageIdNew/Reset first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
They're used only inside qemu_domain.c. Move it before their usage,
and unexport them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We can exit early when the input is an empty string, and we can
avoid storing the string length in a variable since we only use
that information once.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The hw.cpufrequency sysctl, which we use to obtain the CPU
frequency on macOS, is not available when running on Apple
Silicon, and as a consequence we currently report an error
whenever such information is requested.
The virNodeInfo.mhz field, where the CPU frequency gets stored,
is documented as being zero when the information could not be
obtained, and we already do that for Linux on aarch64. Extend
this behavior to macOS on Apple Silicon.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If the mutex is part of the `driver` object, it cannot guard that
object's creation and destruction perfectly.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
All callers currently guarantee flags passed to virDomainObjGetMessages
are either zero or contain at least one of the supported flags. But it
doesn't mean we should not check for the possibility an unknown flag was
the only one passed to virDomainObjGetMessages.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If 1024 was not enough to fit the DN, gnutls_x509_crt_get_dn would store
the required size in subjectlen. And since we're not checking the return
value of this function, we would happily overwrite some random memory.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We use 'ret' for storing values to be returned from a function. Return
values from called functions that are not supposed to be returned
further are usually called 'rv' (or 'rc').
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are few places where a virPCIDeviceAddress typed variable
is allocated on the stack but it's not initialized. This can lead
to random values of its members which in turn can lead to a
random behaviour.
Generated with help of the following spatch:
@@
identifier I;
@@
- virPCIDeviceAddress I;
+ virPCIDeviceAddress I = { 0 };
And then fixing bhyveAssignDevicePCISlots() which does declare
the variable and then explicitly zero it by calling memset() only
to set a specific member afterwards.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
After previous commits, the cleanup label shrank to plain
'return' statement. There's no point in having such label, so
drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Nothing inside the qemuPrepareNVRAM function relies on @srcFD
being closed early and nothing closes it early. It's okay then to
close it automatically when leaving the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
After previous commits there is no need for qemuPrepareNVRAM() to
open code virFileRewrite(). Deduplicate the code by calling the
function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When rewriting a file using virFileRewrite() and error occurs
while writing into a temporary file it's actually the callback
that can report the most accurate error. Move error reporting
into very few callback we have currently. Those callbacks are
trivial so the benefit of this change is not obvious, but this
will change shortly when slightly more complicated callback is
introduced.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Currently, due to the way virFileRewrite() works, the rewritten
file is owned by user and group that the daemon runs under. So
far, this is not a problem, because the function is used to write
XML files or secrets for persistent objects (domains, networks,
etc.) and we don't need other users to read/write those files.
But shortly, this function is going to be used for creating files
for QEMU domains. There we want the QEMU process (i.e. different
user) to read the file.
Therefore, introduce two new arguments: @uid and @gid that allow
setting desired owner of the file. Pass -1 to preserve current
behaviour (i.e. create the file owned by the user running the
daemon).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Even though the CH driver doesn't implement virNetworkUpdate()
API, when it does it will see the arguments in correct order.
This is similar to other drivers that don't implement the API,
like ESX, libxl, LXC, etc. Enabling this driver feature stops
clients from swapping the arguments (see comment in the API for
more info).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
No functional change intended. This change makes the refactoring to
automatic mutex management easier to follow.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
No functional change intended. This change makes the recfatoring to
automatic mutex management easier to follow.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are two places where a domain can be started in CH driver:
chDomainCreateXML() and chDomainCreateWithFlags(). Both acquire a
job (good), but neither of them checks whether the domain isn't
already running. This is wrong. Fortunately, both function call
the very same virCHProcessStart() rendering it the best place for
such check.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
There are few places where a call to virDomainObjListRemove() is
guarded with !vm->persistent check. And there are some places
which are missing this check completely (leading us to losing a
domain). To prevent such mistakes introduce
virCHDomainRemoveInactive() which does the check for us. Also
replace all occurrences of virDomainObjListRemove() with the call
to the new function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
When creating a domain failed, then the virCHDomainObjEndJob()
would be jumped over. Fix this by creating enjob label and fixing
one goto.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Inside chDomainCreateXML(), towards the end, the driver is
unlocked even though there is no corresponding driver lock call
before that. Drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
There is no need to lock whole driver when accessing
virDomainObjList. Those APIs were specifically tailored to be
thread safe (when we were dropping QEMU driver lock). Don't
resurrect old history.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
In chConnectGetVersion() the CH driver is locked in order to read
driver->version. This is needless, because not only is the
version set with driver unlocked (chStateInitialize() calls
chExtractVersion() which sets the version), but the version is
practically immutable. Once driver initialized itself it's never
changed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
If VIR_QEMU_PROCESS_START_RESET_NVRAM flag is passed when
starting a domain, then user requested to overwrite the domain
specific NVRAM with the one from template. But it is very likely
that the path to the template is not stored in the domain
definition, which in turn makes the copy function
(qemuPrepareNVRAM()) fail.
The solution is simple - when preparing domain, specifically when
deciding whether the path to the template should be autofilled,
ignore any existing NVRAM file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
In one of my previous commits I've fixed the value of
VIR_QEMU_PROCESS_START_RESET_NVRAM flag (which was masking
another value). But what I forgot to do is update virCheckFlags()
calls in two places where the flag is passed: qemuProcessLaunch()
and qemuProcessStart().
Fixes: 1b636593c7
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Use VIR_AUTOFREE for the temp socket so that the 'error:' label can be
removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
We try to update vlan tag by running virsh update-device command,
libvirtd will report ovs-vsctl arguments error. Vlan tag update
funtion does't consider the xml with no vlan configured circumstances.
The steps to reproduce the problem:
1 define and start domain with its vlan configured as:
<interface type='bridge'>
<mac address='52:54:00:9e:bb:ac'/>
<source bridge='ovs-br0'/>
<vlan>
<tag id='10'/>
</vlan>
<virtualport type='openvswitch'>
</virtualport>
<target dev='vnet4.0'/>
<model type='virtio'/>
<driver name='vhost'/>
</interface>
2 define and run virsh update-device command with no vlan configured as:
<interface type='bridge'>
<mac address='52:54:00:9e:bb:ac'/>
<source bridge='ovs-br0'/>
<virtualport type='openvswitch'>
</virtualport>
<target dev='vnet4.0'/>
<model type='virtio'/>
<driver name='vhost'/>
</interface>
#virsh update-device dom-id novlan.xml
3 virsh command returned error, and we got an error in libvirtd.log:
error : virCommandWait:2584 : internal error: exit status 1: ovs-vsctl: 'set' command requires at least 3 arguments
. Child process (ovs-vsctl --timeout=5 -- --if-exists clear Port vnet4.0 tag -- --if-exists clear Port vnet4.0 trunk
-- --if-exists clear Port vnet4.0 vlan_mode -- --if-exists set Port vnet4.0) unexpected
error : virNetDevOpenvswitchUpdateVlan:540 : internal error: Unable to set vlan configuration on port vnet4.0
Signed-off-by: Tu Qiang <tu.qiang35@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Older kernels did not support this sysctl, but they did not restrict
userfaultfd in any way so everything worked as if
vm.unprivileged_userfaultfd was set to 1. Thus we can safely ignore
errors when setting the value.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemuPrepareNVRAM() function accepts three arguments and the
last one being a boolean type. However, when the function is
called from qemuProcessPrepareHost() the argument passed is a
result of logical and of @flags (unsigned int) and
VIR_QEMU_PROCESS_START_RESET_NVRAM value. In theory this is
unsafe to do because if the value of the flag is ever changed
then this expression might overflow. Do what we do elsewhere:
double negation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In one of recent commits qemuProcessStartFlags enum gained new
value: VIR_QEMU_PROCESS_START_RESET_NVRAM but due to a typo it
has the same value as another member of the enum. Fix that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
While VNC ports auto-allocated by the libxl driver are released in
libxlDomainCleanup, spice ports are overlooked. Rework the existing
logic to release any auto-allocated graphics ports, not just the VNC
port of the first graphics device.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When the <loader> had an explicit readonly='no' attribute we
accidentally still marked the plfash as readonly due to the
bad conversion from virTristateBool to bool. This was missed
because the test cases run with no capabilities set and thus
are validated the -drive approach for pflash configuration,
not the -blockdev approach.
This affected the following config:
<os>
<loader readonly='no' type='pflash'>/var/lib/libvirt/qemu/nvram/test-bios.fd</loader>
</os>
for the sake of completeness, we also add a test XML config
with no readonly attribute at all, to demonstrate that the
default for pflash is intended to be r/w.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We can now replace the existing NVRAM file on startup when
the API requests this.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When starting a guest with pflash based firmware, we will initialize
NVRAM from a template if it does not already exist. In theory if the
firmware code file is updated, the existing NVRAM variables should
continue to work correctly. It is inevitable that this could break
accidentally one day. Or a bug in the firmware might corrupt the
NVRAM storage. Or user might make bad changes to the settings that
prevent booting. Or the user might have re-configured the XML to
point to a different firmware file incompatible with the current
variables.
In all these cases it would be useful to delete the existing NVRAM
and initialize it from the pristine template.
To support this introduce a VIR_DOMAIN_START_RESET_NVRAM constant
for use with virDomainCreate / virDomainCreateXML, along with
VIR_DOMAIN_SAVE_RESET_NVRAM for use with virDomainRestore and
VIR_DOMAIN_SNAPSHOT_REVERT_RESET_NVRAM for use with
virDomainSnapshotRevert.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
If we crash part way through writing the NVRAM file we end up with an
unusable NVRAM on file. To avoid this we need to write to a temporary
file and fsync(2) at the end, then rename to the real NVRAM file path.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In one of my previous commits, I've changed an XPath in
virCPUDefParseXML() from "boolean(./counter...)" to
"./counter...)". Notice the dangling closing bracket? Well, I
didn't back then.
Fixes: 0fe2d8dd33
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This change was generated using the following spatch:
@ rule1 @
expression a;
identifier f;
@@
<...
- f(*a);
... when != a;
- *a = NULL;
+ g_clear_pointer(a, f);
...>
@ rule2 @
expression a;
identifier f;
@@
<...
- f(a);
... when != a;
- a = NULL;
+ g_clear_pointer(&a, f);
...>
Then, I left some of the changes out, like tools/nss/ (which
doesn't link with glib) and put back a comment in
qemuBlockJobProcessEventCompletedActiveCommit() which coccinelle
decided to remove (I have no idea why).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are two places where a variable passed to VBOX_RELEASE()
macro is set to NULL explicitly. There is no need for that
because the macro sets the variable to NULL already.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The libxl driver reconnects to all running VMs when libvirtd is restarted,
but it failed to mark auto-allocated graphics ports as set in the port
allocator. If many VMs are running that use port auto-allocation and
libvirtd is restarted, the port allocator is likely to hand out a port
already in use when a new VM is created that uses auto-allocation. VM
creation will fail due to the port clash.
When reconnecting to running VMs after a libvirtd restart, let the port
allocator know about previously allocated ports.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modify 'qemuProcessGetVCPUQOMPath' to take the detected QOM path of the
first vCPU which is always present as the QOM path used our code probing
CPU flags via 'qom-get'.
This is needed as upcoming qemu will change it.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/272
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2051451
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commit we need to probe the vcpus first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Upcoming changes will require that we have a proper QOM path for cpus
when querying the flags as qemu is going to change it.
By moving the flag probing code later we'll already probe the QOM paths
so no re-query will be needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The QOM path will be needed by code which is querying the cpu flags via
'qom-get' and thus needs a valid QOM path to the vCPU.
Add it into the private data and transfer from the queried data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert all code using the 'QOM_CPU_PATH' macro to accept the QOM path
as an argument.
For now the new helper for fetching the path 'qemuProcessGetVCPUQOMPath'
will always return the same hard-coded value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory clearing and remove the 'ret' variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is used only as a helper in src/qemu/qemu_monitor_json.c
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This directory contains runtime state, not persistent state.
The latter goes into swtpmStorageDir.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
When looping over TPM devices for a domain, we can avoid calling
this function for each iteration and call it once per domain
instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Using the word "create" can give users the impression that disk
operations will be performed, when in reality all these functions
do is string formatting.
Follow the naming convention established by virBuildPath(),
virFileBuildPath() and virPidFileBuildPath().
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
This leaves qemuExtTPMCleanupHost() to only deal with looping
over TPM devices, same as other qemuExtTPMDoThing() functions.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
This leaves qemuExtTPMSetupCgroup() to only deal with looping
over TPM devices, same as other qemuExtTPMDoThing() functions.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Other functions that operate on a single TPM emulator follow
the qemuTPMEmulatorDoThing() naming convention.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
As the parent address is part of the mdev nodedev name lets expose the
internally available parent address in the XML.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When we are about to spawn QEMU, we validate the domain
definition against qemuCaps. Except when domain is/was already
running before (i.e. on incoming migration, snapshots, resume
from a file). However, especially on incoming migration it may
happen that the destination QEMU is different to the source
QEMU, e.g. the destination QEMU may have some devices disabled.
And we have a function that validates devices/features requested
in domain XML against the desired QEMU capabilities (aka
qemuCaps) - it's virDomainDefValidate() which calls
qemuValidateDomainDef() and qemuValidateDomainDeviceDef()
subsequently.
But the problem here is that the validation function is
explicitly skipped over in specific scenarios (like incoming
migration, restore from a snapshot or previously saved file).
This in turn means that we may spawn QEMU and request
device/features it doesn't support. When that happens QEMU fails
to load migration stream:
qemu-kvm: ... 'virtio-mem-pci' is not a valid device model name
(NB, while the example shows one particular device, the problem
is paramount)
This problem is easier to run into since we are slowly moving
validation from qemu_command.c into said validation functions.
The solution is simple: do the validation in all cases. And while
it may happen that users would be unable to migrate/restore a
guest due to a bug in our validator, spawning QEMU without
validation is worse (especially when you consider that users can
supply their own XMLs for migrate/restore operations - these were
never validated).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2048435
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The binary validation in virPidFileReadPathIfAlive may fail with EACCES
if the calling process does not have CAP_SYS_PTRACE capability.
Therefore instead do only the check that the pidfile is locked by the
correct process.
Fixes the same issue as with swtpm.
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Access to /proc/[pid]/exe may be restricted in certain environments (e.g.
in containers) and any attempt to stat(2) or readlink(2) the file will
result in 'permission denied' error if the calling process does not have
CAP_SYS_PTRACE capability. According to proc(5) manpage:
Permission to dereference or read (readlink(2)) this symbolic link is
governed by a ptrace access mode PTRACE_MODE_READ_FSCREDS check; see
ptrace(2).
The binary validation in virPidFileReadPathIfAlive may fail with EACCES.
Therefore instead do only the check that the pidfile is locked by the
correct process. To ensure this is always the case the daemonization and
pidfile handling of the swtpm command is now controlled by libvirt.
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function will attempt to read a pid from @path, and store it in
@pid. The @pid will only be set, however, if @path is locked by
virFileLock() at byte 0 and the pid in @path is running.
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The @unique argument didn't exist even when the function was
introduced in a042275a39, and the @vm argument was not renamed
when the function was changed to take a virDomainDef* instead of
a virDomainObj* in 7ed6934f3b.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
SmartNIC DPUs may not expose some privileged eswitch operations
to the hypervisor hosts. For example, this happens with Bluefield
devices running in the ECPF (default) mode for security reasons. While
VF MAC address programming is possible via an RTM_SETLINK operation,
trying to set a VLAN ID in the same operation will fail with EPERM.
The equivalent ip link commands below provide an illustration:
1. This works:
sudo ip link set enp130s0f0 vf 2 mac de:ad:be:ef:ca:fe
2. Setting (or clearing) a VLAN fails with EPERM:
sudo ip link set enp130s0f0 vf 2 vlan 0
RTNETLINK answers: Operation not permitted
3. This is what Libvirt attempts to do today (when trying to clear a
VF VLAN at the same time as programming a VF MAC).
sudo ip link set enp130s0f0 vf 2 vlan 0 mac de:ad:be:ef:ca:fe
RTNETLINK answers: Operation not permitted
If setting an explicit VLAN ID results in an EPERM, clearing a VLAN
(setting a VLAN ID to 0) can be handled gracefully by ignoring the
EPERM error with the rationale being that if we cannot set this state
in the first place, we cannot clear it either.
In order to keep explicit clearing of VLAN ID working as it used to
be passing a NULL pointer for VLAN ID is used.
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There should be a way to show no intent in programming a VLAN at all
(including clearing it). This allows handling error conditions
differently when VLAN clearing is explicit (vlan id == 0) vs implicit
(vlanid == NULL - try to clear it if possible).
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This has a benefit of being able to handle error codes for those
operations separately which is useful when drivers allow setting a MAC
address but do not allow setting a VLAN (which is the case with some
SmartNIC DPUs).
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virCHMonitorGetIOThreads returns an int, not size_t.
Also return early if it's negative, because promoting it to
an unsigned type in the for loop condition could lead to
an infinte loop.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Report an error upfront if the binary does not exist
or is not executable.
https://bugzilla.redhat.com/show_bug.cgi?id=1999372
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Some files do not include what they use and rely on virutil.h
to pull in the necessary header files.
Fix it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Introduce support for
<serial type='pty'>
<target type='isa-debug'>
<model type='isa-debugcon'/>
</target>
<address type='isa' iobase='0x402'/>
</console>
which is used as a way to receive debug messages from the
firmware on x86 platforms.
Note that the default port is hypervisor specific, with QEMU
currently using 0xe9 since that's the original Bochs debug port.
For use with SeaBIOS/OVMF, the iobase port needs to be explicitly
set to 0x402.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The serial port model cannot be allowed to change across migration
as it affects ABI.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When virNodeDeviceObjListRemove() is called, the passed
virNodeDeviceObj is removed from internal list of node devices
and then unrefed and unlocked. While the former is warranted (the
object was refed at the beginning of the function) the unlock is
not. In fact, it's wrong from conceptual POV. We still want
threads working on the object tu mutually exclude each other.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This mostly overlaps with virDomainAudioType, but in a couple of
cases the string representations are different.
Right now we're doing that in a somewhat sketchy way, in that we
store values of one enumeration and then convert them to strings
using TypeToString() implementation for the other enumeration;
when converting from string, we open-code the handling of the
special values mentioned above.
Drop the second enumeration and introduce two helpers to deal
with conversion. Most calling sites don't need to be changed, and
one can even be simplified significantly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This, along with "pa", is the other case where the libvirt and
QEMU names do not match.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We recently started listing these in the spec file and, since we
were not creating them during the installation phase, that broke
RPM builds.
Fixes: 4b43da0bff
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, memory device (def->mems) part of cmd line is
generated before any controller. In majority of cases it doesn't
matter because neither of memory devices live on a bus that's
created by an exposed controller (e.g. there's no DIMM
controller, at least not exposed). Except for virtio-mem and
virtio-pmem, which do have a PCI address. And if it so happens
that the device goes onto non-default bus (pci.0) starting such
guest fails, because the controller that creates the desired bus
wasn't processed yet. QEMU processes arguments in order.
For instance, if virtio-mem has address with bus='0x01' QEMU
refuses to start with the following message:
Bus 'pci.1' not found
Similarly for virtio-pmem. I've successfully tested migration and
changing the order does not affect migration stream.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2047271
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
MIPS Malta (and no other supported MIPS machine) has a PCI bus.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This identifies various MIPS Malta machines, be it 32-bit or 64-bit,
little-endian or big-endian.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Identifies all of various MIPS sub-architectures: 32-bit or 64-bit,
little-endian or big-endian.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are few places where the g_steal_pointer() is open coded.
Switch them to calling the g_steal_pointer() function instead.
Generated by the following spatch:
@ rule1 @
expression a, b;
@@
<...
- b = a;
... when != b
- a = NULL;
+ b = g_steal_pointer(&a);
...>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
There are a few places where a variable is VIR_FREE()-d and then
explicitly set to NULL. This is not necessary since VIR_FREE()
does that for us.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
In testDomainSetBlockIoTune() the info.group_name is strdup()-ed
and just after the whole @info structure is passed to
virDomainDiskSetBlockIOTune() the @group_name member is set to
NULL. This creates a memleak, because
virDomainDiskSetBlockIOTune() creates its own copy of the string.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Modeled after "WITH_QEMU_LOCK_GUARD" (see qemu's include/qemu/lockable.h).
See comment for typical usage.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Typical usage:
void foobar(virObjectLockable *obj)
{
VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
/* `obj` is locked, and released automatically on scope exit */
...
}
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Modeled after "WITH_QEMU_LOCK_GUARD" (see qemu's include/qemu/lockable.h).
See comment for typical usage.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Locks a virMutex on creation and unlocks it in its destructor.
The VIR_LOCK_GUARD macro is used instead of "g_auto(virLockGuard)" to
work around a clang issue (see https://bugs.llvm.org/show_bug.cgi?id=3888
and https://bugs.llvm.org/show_bug.cgi?id=43482).
Typical usage:
void function(virMutex *m)
{
VIR_LOCK_GUARD lock = virLockGuardLock(m);
/* `m` is locked, and released automatically on scope exit */
...
while (expression) {
VIR_LOCK_GUARD lock2 = virLockGuardLock(...);
/* similar */
}
}
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Using the two-step idiom to force resolution of other macros, e.g.:
#define bar BAR
CONCAT_(foo, bar) // foobar
CONCAT(foo, bar) // fooBAR
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The service files were copied out of the service file for libvirtd and
the name of the corresponding manpage was not fixed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2045959
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The idea behind virNWFilterBindingObjNew() is to create and
return an object of virNWFilterBindingObjClass class. The class
is virObjectLockable (and the corresponding
_virNWFilterBindingObj structure has virObjectLockable parent).
But for some reason plain virObjectNew() is called. This is wrong
because the mutex in the parent is left uninitialized.
Next, the returned object is not locked. This is wrong because in
some cases the returned object is added onto a list of bindings
and then passed to virNWFilterBindingObjEndAPI() which unlocks it
right away. This is potentially dangerous because we might just
have unlocked the object for another thread.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduced in Xen 4.7 by commit:
commit bf7628f087b212052a0e9f024044b2790c33f820
libxl: add pvusb API
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Unused as of:
commit 446d091498
libxl: pass driver config to libxlMakeDomBuildInfo
All other usage of LIBXL_HAVE_DEVICE_CHANNEL was removed by:
commit e58004d70a
Xen: Remove unneeded LIBXL_HAVE_* ifdefs
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduced in 4.3.0 by xen commit:
commit ef496b81f0336f09968a318e7f81151dd4f5a0cc
libxl: postpone backend name resolution
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Any active domain has a copy in the privateData, filled in
qemuProcessInit.
Move the qemu capability check below the activeness check and remove
the extra lookup.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Apparently, some of '&*variable' slipped in. Drop '&*' and access
the variable directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
