virDomainDriverGetIOThreadsConfig: Automatically free virBitmap

Use g_autoptr for the temp bitmap. To achieve this the variable must be
moved down to the appropriate scope.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virDomainNumatuneParseXML: Refactor cleanup

Use automatic memory clearing for the temporary strings and bitmap and
remove the cleanup section. There are multiple temporary strings added
so that we don't reuse one.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virDomainSchedulerParse: Refactor cleanup

Automatically free the 'ret' temporary bitmap and get rid of the cleanup
section.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

conf: capabilities: Clean up freeing of virBitmap

Use automatic freeing where possible and use g_clear_pointer instead of
manual NULL-ing.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virCapabilitiesInitCaches: Refactor freeing of temporary variables

Move the 'path' and 'type' variables down to the appropriate block and
use automatic freeing for them as well as the temporary virBitmap.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Revert "lxc: controller: Fix container launch on cgroup v1"

Unfortunately, this fix breakes machinectl in a very nasty way,
for instance 'machinectl shell' drops into the host shell. It's
worse than being unable to start a container with CGroupsV1.

Revert until a proper fix is figured out.

This reverts commit 1b9ce05ce241a581d4e80228c92ceb0266f21f94.

References: https://gitlab.com/libvirt/libvirt/-/issues/182
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

remote: Avoid crash in remoteSplitURIScheme()

We need to make sure the URI scheme is present before passing
it to strchr(), otherwise we're going to get

  $ virt-ssh-helper foo
  Segmentation fault (core dumped)

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Revert "qemu: Avoid crash in qemuStateShutdownPrepare() and qemuStateShutdownWait()"

This reverts commit 69977ff10560a80bcf5bf93f1a3f819a2d1623ca.

After previous commit it's no longer possible that QEMU driver is
not initialized in qemuStateShutdownPrepare() nor
qemuStateShutdownWait().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

remote_daemon: Set shutdown callbacks only after init is done

The initialization of drivers happens in a separate thread.
However, the main thread continues initialization and sets
shutdown callbacks (virStateShutdownPrepare() and
virStateShutdownWait()) even though the driver init thread is
still running. This is dangerous because if the daemon decides to
quit early (e.g. because SIGINT was delivered) the
shutdownPrepare and shutdownWait callback are called over
partially init drivers.

Set callbacks only after all drivers were initialized.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/218
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2027400

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

qemu_monitor_json.h: Unify header formatting

Use the modern style and fix all offenders since new functions were
already using the contemporary style.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuValidateDomainDeviceDefDiskFrontend: Aggregate disk iomode validation

Move the two checks under a common block.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuValidateDomainDeviceDefDiskFrontend: Fix error message if io='native' is unsupported

The error is a hard error, so the part about fallback doesn't make
sense. Spell the attribute the same way as it's in XML.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

docs: Update more links to point to the new Go modules

A couple of links were still pointing to the obsolete Go
packages instead of the current module-aware ones.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>

NEWS: Fix spacing between releases

Signed-off-by: Andrea Bolognani <abologna@redhat.com>

qemu: do not compare missing cpu data

For x86, we invalidate qemu caps cache if the host CPUID changed.
However other cpu drivers do not have the 'getHostData' function
implemented.

Skip the comparison if we do not have host CPUData available,
since virCPUDataIsIdentical always returns an error in that case.

https://bugzilla.redhat.com/show_bug.cgi?id=2030119

Fixes: 3bc6f46d305ed82f7314ffc4c2a66847b831a6bd
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

NEWS: Mention synchronous copy job additions

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

docs: migration: Add a paragraph about non-shared storage migration

Outline some of the basics and the caveats of the non-shared migration
code.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

docs: Convert 'migration' doc to RST

After conversion the table doesn't have to custom colors, but otherwise
seems to hold well.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: migration: Implement VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES

Use it to enable the 'write-blocking' mode of 'blockdev-mirror'.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virDomainMigrate: Introduce VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES flag

Non-shared storage migration of guests which are disk I/O intensive and
have fast local storage may actually never converge if the guest happens
to dirty the disk faster than it can be copied.

This patch introduces a new flag
'VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES' which will instruct
hypervisors to synchronize local I/O writes with the writes to remote
storage used for migration so that the guest can't overwhelm the
migration. This comes at a cost of decreased local I/O performance for
guests which behave well on average.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

VIR_REQUIRE_FLAG_(GOTO|RET): Add parens around arguments in expansion

Make the macro useful also for cases when one of multiple flags is
required.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

man: virsh: Separate paragraphs describing distinct flags

Separate the paragraphs where the topic changes to simplify further
additions.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

include: virDomainMigrateFlags: Remove "block alignment" whitespace

Using whitespace to align the '=' and values doesn't make sense for the
virDomainMigrateFlags enum as the visual block is interrupted by
comments.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuDomainBlockCopy: Implement VIR_DOMAIN_BLOCK_COPY_SYNCHRONOUS_WRITES

Wire up the flag to enable the 'write-blocking' 'copy-mode' of
'blockdev-mirror'.

It's not supported by all qemu versions but it is with those which we
use -blockdev with so we can use that instead of adding another custom
capability as we use blockdev for some time now.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virDomainBlockCopy: Introduce VIR_DOMAIN_BLOCK_COPY_SYNCHRONOUS_WRITES flag

In cases when the destination storage is slower than the normal VM
storage and the VM does intensive I/O to the disk a block copy job may
never converge.

Switching it to synchronous mode will ensure that all writes done by the
guest are propagated to the destination at the cost of slowing down I/O
of the guest to the synchronous speed.

This patch adds the new API flag and implements virsh support.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

include: virDomainBlockCopyFlags: Convert to prefix comments

Switch to the comment style allowing more text.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: monitor: Add support for 'write-blocking' copy mode for blockdev-mirror

Forces the data to be written synchronously to both the original and the
mirrored images which ensures that the job will reach synchronized
phase.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuMonitorJSONGraphicsRelocate: Clean up command argument construction

Move the construction of the command from the variable declaration so
that it doesn't exceed the line length and we can also move the logic of
determining the protocol outside of the command construction.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuMonitorJSONMigrate: Extract flags prior to constructing command

The migration API takes specific flags which are then converted to
boolean parameters for the command. Extract the flag into helper
variables rather than using ternary operators while constructing the
command itself.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuMonitorJSONEjectMedia: Use a bool directly for constructing JSON with 'b' modifier

It actually already expects a bool.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuMonitorJSONHandleShutdown: Use virTristateBoolFromBool

Instead of a ternary operator we can use the existing helper.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: monitor: Avoid ternary operators in helpers for drive/blockdev-mirror

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: agent: Remove unneeded cleanup sections

Remove the cleanup sections where not needed after we've converted to
automatic freeing of virJSONValue.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: agent: Automatically free virJSONValue-s

Convert the code to use g_autoptr for the few cases sill using explicit
cleanup.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuAgentIOProcessLine: refactor cleanup

Refactor the control flow so we can remove the cleanup label.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuMonitorAddObject: Use g_clear_pointer for a free and reset operation

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuMonitorJSONGetCPUModelExpansion: Don't use goto for looping

Don't use 'goto' for looping. Extract the monitor interaction code into
a new function and restructure the logic to avoid jumping back in the
code.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuAgentGuestSync: Don't use goto for looping

Don't use 'goto' for looping. Extract the sync sending code into a new
function and restructure the logic to avoid jumping back in the code.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

tests/virnetdaemontest.c: testExecRestart: Automatically free virJSONValue-s

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virCHProcessUpdateInfo: Automatically free virJSONValue

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virLogDaemonPostExecRestart: Refactor cleanup

Move the unlinking of the state file right after reading it so that we
can get rid of the cleanup section.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virLogDaemonPostExecRestart: Use automatic freeing for variables

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virLockDaemonPostExecRestart: Refactor cleanup

Move the unlinking of the state file earlier and get rid of the cleanup
label.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virLockDaemonPostExecRestart: Automatically free temporary variables

Convert two temp strings and one virJSONValue to g_auto(free|ptr).

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virLockDaemonClientPreExecRestart: Modernize JSON object construction

Use virJSONValueObjectAdd instead of step-by-step construction of the
object. This also removes a handful impossible to reach errors with
translatable messages.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuMonitorBlockdevCreate: Use double pointer instead of always consuming '@props'

We use this approach for other APIs which take a virJSONValue as
argument and the logic is also simpler.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuDomainHotplugAddIOThread: Automatically free virJSONValue

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: hotplug: Use automatic freeing for virJSONValue

There are a few uses which still explicitly free JSON objects, fix them.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

util: call virNetDevGetPhysPortID() in less places

Whenever virPCIGetNetName() is called, it is either called with
physPortID = NULL, or with it set by the caller calling
virNetDevGetPhysPortID() soon before virPCIGetNetName(). The
physPortID is then used *only* in virPCIGetNetName().

Rather than replicating that same call to virNetDevGetPhysPortID() in
all the callers of virPCIGetNetName(), lets just have all those
callers send the NetDevName whose physPortID they want down to
virPCIGetNetName(), and let virPCIGetNetName() call
virNetDevGetPhysPortID().

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

util: fix erroneous requirement for phys_port_id to get ifname of a VF

Commit 795e9e05c3 (libvirt-7.7.0) refactored the code in virpci.c and
virnetdev.c that gathered lists of the Virtual Functions (VF) of an
SRIOV Physical Function (PF) to simplify the code.

Unfortunately the simplification made the assumption, in the new
function virPCIGetVirtualFunctionsFull(), that a VF's netdev
interface name should only be retrieved if the PF had a valid
phys_port_id. That is an incorrect assumption - only a small handful
of (now previous-generation) Mellanox SRIOV cards actually use
phys_port_id (this is for an odd design where there are multiple
physical network ports on a single PCI address); all other SRIOV cards
(including new Mellanox cards) have a file in sysfs called
phys_port_id, but it can't be read, and so the pfPhysPortID string is
NULL.

The result of this logic error is that virtual networks that are a
pool of VFs to be used for macvtap connections will be unable to
start, giving an errror like this:

 VF 0 of SRIOV PF enp130s0f0 couldn't be added to the interface pool because it isn't bound to a network driver - possibly in use elsewhere

This error message is misinformed - the caller of
virNetDevGetVirtualFunctionsFull() only *thinks* that the VF isn't
bound to a network driver because it doesn't see a netdev name for the
VF in the list. But that's only because
virNetDevGetVirtualFunctionsFull() didn't even try to get the names!

We do need a way for virPCIGetVirtualFunctionsFull() to sometimes
retrieve the netdev names and sometimes not. One way of doing that
would be to send down the netdev name of the PF whenever we also want
to know the netdev names of the VFs, but send a NULL when we
don't. This can conveniently be done by just *replacing* pfPhysPortID
in the arglist with pfNetDevName - pfPhysPortID is determined by
simply calling virNetDevGetPhysPortID(pfNetDevName) so we can just
make that call down in virPCIGetVirtualFunctionsFull() (when needed).

This solves the regression introduced by commit 795e9e05c3, and also
nicely sets us up to (in a subsequent commit) move the call to
virNetDevGetPhysPortID() down one layer further to virPCIGetNetName(),
where it really belongs!

Resolves: https://bugzilla.redhat.com/2025432
Fixes: 795e9e05c3b6b9ef3abe6f6078a6373a136ec23b
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

meson: Improve network driver enablement logic

The Homebrew package explicitly enables this driver despite us
disabling it by default on macOS, so it must be functional to
at least some extent and certainly can't be causing any build
failures.

Additionally, if the user has explicitly asked for the network
driver to be enabled but libvirtd is disabled for whatever
reason, we should error out instead of silently disabling the
network driver.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

lib: Drop some needless labels

After previous cleanups some labels became needless because they
contain just a return statement. There's no point in having such
labels.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

lib: Use g_autoptr() for virDomainDef

Instead of calling virDomainDefFree() explicitly, we can annotate
variables with g_autoptr().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virdomainobjlist: Use g_autofree

Instead of calling VIR_FREE() explicitly, we can annotate
variables with g_autofree.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

libxl: libxlLoggerFileFree: remove redundant NULL assignment

The 'file' pointer is already zeroed in virFileFclose
and never read again.

Signed-off-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: move inactive snapshot to separate function

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: rename qemuSnapshotRevertInactive

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: move active snapshot revert to separate function

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: save metadata within qemu process job

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: move saving metadata to separate function

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: refactor cleanup section

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: emit event right after they are created

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: move config prepare code to separate function

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: move validation to separate function

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: remove cleanup label

Now the cleanup label is not necessary so we can drop it.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: jump to endjob instead of calling qemuProcessEndJob

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: use g_autoptr

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: drop unused variable

Commit <f33ce12e9cd9cab7e6022e91d3765c33d99bf777> dropped unused code
but missed one variable.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu_snapshot: revert: fix incorrect jump to cleanup

Fixes: 6a6f6b91e0e76480ea961f83135efcb4faf3284a
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Fix reboot command for LXC containers

The virNetDaemonQuit(dmn) command in virLXCControllerSignalChildIO triggers an
early close of all clients of lxc_controller. Here, libvirtd itself is a client
of this controller, and the client connection is used to notify libvirtd if a
reboot of the container is required. However, the client connection was closed
before such a status could be sent to libvirtd. To fix this bug, we will
immediately send the reboot or shutdown status of the container to libvirtd,
and only after client disconnect will we trigger virNetDaemonQuit.

Fixes: https://gitlab.com/libvirt/libvirt/-/issues/237
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=991773
Signed-off-by: Joachim Falk <joachim.falk@gmx.de>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

libxl: Protect access to libxlLogger files hash table

The hash table of log file objects in libxlLogger is not protected against
concurrent access. It is possible for one thread to remove an entry while
another is updating it. Add a mutex to the libxlLogger object and lock it
when accessing the files hash table.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

libxl: Search for virDomainObj in event handler threads

libxl can deliver events and invoke callbacks on any application thread
calling into libxl. This can cause deadlock in the libvirt libxl driver

Thread 19 (Thread 0x7f31411ec700 (LWP 14068) "libvirtd"):
#0  0x00007f318520cc7d in __lll_lock_wait () from /lib64/libpthread.so.0
#1  0x00007f3185205ed5 in pthread_mutex_lock () from /lib64/libpthread.so.0
#2  0x00007f3189488015 in virMutexLock (m=<optimized out>) at ../../src/util/virthread.c:79
#3  0x00007f3189463f3b in virObjectLock (anyobj=<optimized out>) at ../../src/util/virobject.c:433
#4  0x00007f31894f2f41 in virDomainObjListSearchID (payload=0x7f317400a6d0, name=<optimized out>, data=0x7f31411eaeac) at ../../src/conf/virdomainobjlist.c:105
#5  0x00007f3189437ac5 in virHashSearch (ctable=0x7f3124025a30, iter=iter@entry=0x7f31894f2f30 <virDomainObjListSearchID>, data=data@entry=0x7f31411eaeac, name=name@entry=0x0) at ../../src/util/virhash.c:745
#6  0x00007f31894f3919 in virDomainObjListFindByID (doms=0x7f3124025430, id=<optimized out>) at ../../src/conf/virdomainobjlist.c:121
#7  0x00007f3152f292e5 in libxlDomainEventHandler (data=0x7f3124023d80, event=0x7f310c010ae0) at ../../src/libxl/libxl_domain.c:660
#8  0x00007f3152c6ff5d in egc_run_callbacks (egc=egc@entry=0x7f31411eaf50) at libxl_event.c:1427
#9  0x00007f3152c718bd in libxl__egc_cleanup (egc=0x7f31411eaf50) at libxl_event.c:1458
#10 libxl__ao_inprogress (ao=ao@entry=0x7f310c00b8a0, file=file@entry=0x7f3152cce987 "libxl_domain.c", line=line@entry=730, func=func@entry=0x7f3152ccf750 <__func__.22238> "libxl_domain_unpause") at libxl_event.c:2047
#11 0x00007f3152c8c5b8 in libxl_domain_unpause (ctx=0x7f3124015a40, domid=<optimized out>, ao_how=ao_how@entry=0x0) at libxl_domain.c:730
#12 0x00007f3152f2a584 in libxl_domain_unpause_0x041200 (domid=<optimized out>, ctx=<optimized out>) at /usr/include/libxl.h:1756
#13 libxlDomainStart (driver=driver@entry=0x7f3124023d80, vm=vm@entry=0x7f317400a6d0, start_paused=start_paused@entry=false, restore_fd=restore_fd@entry=-1, restore_ver=<optimized out>, restore_ver@entry=2) at ../../src/libxl/libxl_domain.c:1482
#14 0x00007f3152f2a6e3 in libxlDomainStartNew (driver=driver@entry=0x7f3124023d80, vm=vm@entry=0x7f317400a6d0, start_paused=start_paused@entry=false) at ../../src/libxl/libxl_domain.c:1545
#15 0x00007f3152f2a789 in libxlDomainShutdownHandleRestart (driver=0x7f3124023d80, vm=0x7f317400a6d0) at ../../src/libxl/libxl_domain.c:464
#16 0x00007f3152f2a9e4 in libxlDomainShutdownThread (opaque=<optimized out>) at ../../src/libxl/libxl_domain.c:559
#17 0x00007f3189487ee2 in virThreadHelper (data=<optimized out>) at ../../src/util/virthread.c:196
#18 0x00007f3185203539 in start_thread () from /lib64/libpthread.so.0
#19 0x00007f3184f3becf in clone () from /lib64/libc.so.6

Frame 16 runs a thread created to handle domain shutdown processing for
domid 28712. In this case the event contained the reboot reason, so the
old domain is destroyed and a new one is created by libxlDomainStart new.
After starting the domain, it is unpaused by calling libxl_domain_unpause
in frame 12. While the thread is running within libxl, libxl takes the
opportunity to deliver a pending domain shutdown event for unrelated domid
28710. While searching for the associated virDomainObj by ID, a deadlock is
encountered when attempting to lock the virDomainObj for domid 28712, which
is already locked since this thread is processing its shutdown event.

The deadlock can be avoided by moving the search for a virDomainObj
associated with the event domid to the shutdown thread. The same is done
for the death thread.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

libxl: Handle domain death events in a thread

Similar to domain shutdown events, processing domain death events can be a
lengthy process and we don't want to block the event handler while the
operation completes. Move the death handling function to a thread.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

libxl: Modify name of shutdown thread

The current thread name 'ev-<domid>' is a bit terse. Change the name
to 'shutdown-event-<domid>', allowing it to be distinguished between
thread handling other event types.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

libxl: Rename libxlShutdownThreadInfo struct

An upcoming change will use the struct in a thread created to process
death events. Rename libxlShutdownThreadInfo to libxlEventHandlerThreadInfo
to reflect the more generic usage.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

libxl: Disable death events after receiving a shutdown event

The libxl driver will handle all domain destruction and cleanup
when receiving a domain shutdown event from libxl. Commit fa30ee04a2a
introduced the ignoreDeathEvent boolean in the DomainObjPrivate struct
to ignore subsequent death events from libxl. But libxl already provides
a mechanism to disable death events via libxl_evdisable_domain_death.

This patch partially reverts commit fa30ee04a2a and instead uses
libxl_evdisable_domain_death to disable subsequent death events when
processing a shutdown event.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemuConnectGetAllDomainStats: Simplify qemuDomainGetStats() error handling

In qemuConnectGetAllDomainStats() there a loop that iterates over
all domains that stats are to be fetched for. Within this loop
the qemuDomainGetStats() is called which is responsible for
fetching stats for an individual domain. Now, the code that
handles successful and failure cases is almost the same. Rework
it, so that the code is deduplicated. Note, that the check for
!tmp is dropped because upon successful return from
qemuDomainGetStats() it is always allocated.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: prefer .requiredCaps for VIR_DOMAIN_STATS_IOTHREAD

Since f29d7c3e698 we have an option for checking capabilities
required for given type of statistics upfront, instead of the
callback. Switch qemuDomainGetStatsIOThread() callback to the new
style.

This will now error out properly if user requests IOTHREAD stats
forcibly (via VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS
flag) but QEMU doesn't support IOThreads. Previously, this was
silently ignored.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: Drop comma after QEMU_CAPS_LAST in queryDirtyRateRequired[]

The idea of queryDirtyRateRequired[] is that it lists QEMU
capabilities required for given domstats record
(VIR_DOMAIN_STATS_DIRTYRATE in this particular case) and
QEMU_CAPS_LAST is used as a sentinel. Therefore, there can never
be anything after it. Drop the comma to make it more obvious.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virnetdevveth: Do report error if creating veth fails

For some weird reason we are ignoring errors when creating veth
pair that netlink reports. This affects the LXC driver which
creates interfaces for container in
virLXCProcessSetupInterfaces(). If creating a veth pair fails, no
error is reported and the control jumps onto cleanup label where
some cryptic error message is reported instead (something about
inability to remove veth pair).

Let's report error that netlink returned - it's probably the most
accurate reason anyways.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/225
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

virnetserver: Make pool job name less generic

The generic "rpc-worker" name becomes a name of the associated task,
which may than appear in logs and bring some confusion. Let's add a
server name to it so that one can easily see which daemon the task
belongs to, which is especially useful for split daemons. And since the
name would be too long, we can drop the "-worker" part and just keep it
as "rpc-*" and "prio-rpc-*".

Such confusing entries can, for example, be found in audit log when
SELinux is complaining that "rpc-worker" was denied access to something.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

virnetserver: Use autoptr for virNetServer and virNetServerClient

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

virnetserver: Format functions consistently

The file used a pretty inconsistent style for formatting function
headers. Return types were both separate and on the same line as
function names and functions were separated by one, two, and sometimes
even three empty lines. Let's make it consistent by honoring our
preferred coding style.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

virthreadpool: Copy job name

Currently virThreadPoolNewFull relies on the caller to ensure the job
name outlives the thread pool. Which basically enforces static strings.
Let's drop this implicit requirement by making a copy of the job name.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

util: virhash: Replace 'virHashDataFree' by 'GDestroyNotify'

We pass through to glib's hash table functions so we can also use glibs
function prototype definition.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

util: virhash: Remove 'virHashFree'

The code was converted to stop using this function.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

Switch away from virHashFree

Use 'g_clear_pointer(&ptr, g_hash_table_unref)' instead.

In few instances it allows us to also remove explicit clearing of
pointers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemuxml2argvtest: Use 'g_hash_table_unref' for clearing the qapi schema cache

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemumonitorjsontest: mymain: Automatically free GHashTable

Use separate automatically cleared variables for the x86_64 and s390
versions of the QAPI schema.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemumonitorjsontest: testBlockNodeNameDetect: Automatically free GHashTable

Additionally we no longer need the cleanup section.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

nwfilterxml2firewalltest: testCompareXMLToArgvFiles: Automatically free GHashTable

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

nwfilterxml2firewalltest: virNWFilterIncludeDefToRuleInst: Automatically free GHashTable

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemuCheckpointGetXMLDescUpdateSize: Don't combine automatic freeing with manual

'blockNamedNodeData' is declared for automatic freeing but we also free
it manually and reuse which is a code pattern we don't normally allow.

Rewrite the code to have actually two separate hash tables.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemuProcessRefreshLegacyBlockjobs: Automatically free GHashTable and refactor cleanup

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemuProcessRefreshDisks: Automatically free GHashTable and refactor cleanup

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemuProcessWaitForMonitor: Automatically free GHashTable

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemuRefreshPRManagerState: Automatically free GHashTable and refactor cleanup

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemuRefreshVirtioChannelState: Automatically free GHashTable and refactor cleanup

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemuMigrationSrcFetchMirrorStats: Automatically free GHashTable

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemuDomainGetDiskErrors: Automatically free GHashTable

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>