Commit Graph

50595 Commits

Author SHA1 Message Date
Michal Privoznik
1abcba9d4d qemu_capabilities: Introduce QEMU_CAPS_SEV_SNP_GUEST
This capability tracks sev-snp-guest object availability.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-21 09:56:18 +02:00
Michal Privoznik
be26d0ebbe qemu: Report snp-policy in virDomainGetLaunchSecurityInfo()
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-21 09:36:04 +02:00
Michal Privoznik
914b986275 qemu_monitor: Allow querying SEV-SNP state in 'query-sev'
In QEMU commit v9.0.0-1155-g59d3740cb4 the return type of
'query-sev' monitor command changed to accommodate SEV-SNP. Even
though we currently support launching plain SNP guests, this will
soon change.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-21 09:35:32 +02:00
Michal Privoznik
7d16c296e3 src: Convert some _virDomainSecDef::sectype checks to switch()
In a few instances there is a plain if() check for
_virDomainSecDef::sectype. While this works perfectly for now,
soon there'll be another type and we can utilize compiler to
identify all the places that need adaptation. Switch those if()
statements to switch().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-21 09:32:09 +02:00
Michal Privoznik
a44a43361f Drop needless typecast to virDomainLaunchSecurity
The sectype member of _virDomainSecDef struct is already declared
as of virDomainLaunchSecurity type. There's no need to typecast
it to the very same type when passing it to switch().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-21 09:31:33 +02:00
Michal Privoznik
faa3548ed5 conf: Separate SEV formatting into a function
To avoid convolution of switch() inside of virDomainSecDefFormat() even
more (as new sectypes are added), move formatting into a separate
function.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-21 09:30:24 +02:00
Michal Privoznik
d2cad18ca3 conf: Move some members of virDomainSEVDef into virDomainSEVCommonDef
Some parts of SEV are to be shared with SEV SNP. In order to
reuse XML parsing / formatting code cleanly, let's move those
common bits into a new struct (virDomainSEVCommonDef) and adjust
rest of the code.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-21 09:28:54 +02:00
Michal Privoznik
66efdfabd9 qemu_monitor_json: Report error in error paths in SEV related code
While working on qemuMonitorJSONGetSEVMeasurement() and
qemuMonitorJSONGetSEVInfo() I've noticed that if these functions
fail, they do so without appropriate error set. Fill in error
reporting.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-21 09:25:32 +02:00
Peter Krempa
e6b94cba7e qemu: migration: Preserve error across qemuDomainSetMaxMemLock() on error paths
When a VM terminates itself while it's being migrated in running state
libvirt would report wrong error:

 error: cannot get locked memory limit of process 2502057: No such file or directory

rather than the proper error:

 error: operation failed: domain is not running

Remember the error on error paths in qemuMigrationSrcConfirmPhase and
qemuMigrationSrcPerformPhase.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:58:52 +02:00
Peter Krempa
e00a58c10a qemuMigrationSrcRun: Re-check whether VM is active before accessing job data
'qemuProcessStop()' clears the 'current' job data. While the code under
the 'error' label in 'qemuMigrationSrcRun()' does check that the VM is
active before accessing the job, it also invokes multiple helper
functions to clean up the migration including
'qemuMigrationSrcNBDCopyCancel()' which calls 'qemuDomainObjWait()'
invalidating the result of the liveness check as it unlocks the VM.

Duplicate the liveness check and explain why. The rest of the code e.g.
accessing the monitor is safe as 'qemuDomainEnterMonitorAsync()'
performs a liveness check. The cleanup path just ignores the return
values of those functions.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:58:52 +02:00
Peter Krempa
9243e87820 qemu: migration: Inline 'qemuMigrationDstFinishResume()'
The function is a pointless wrapper on top of
qemuMigrationDstWaitForCompletion.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:58:52 +02:00
Peter Krempa
a52e125d56 qemu: migration: Properly check for live VM after qemuDomainObjWait()
Similarly to the one change in commit 4d1a1fdffd
we should be checking that the VM is not being yet destroyed if we've
invoked qemuDomainObjWait().

Use the new helper qemuDomainObjIsActive().

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:58:52 +02:00
Peter Krempa
9eb33b7f03 qemu: domain: Introduce qemuDomainObjIsActive helper
The helper checks whether VM is active including the internal qemu
state. This helper will become useful in situations when an async job
is in use as VIR_JOB_DESTROY can run along async jobs thus both checks
are necessary.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:58:52 +02:00
Peter Krempa
d9935a5c4f qemu: process: Ensure that 'beingDestroyed' gets cleared only after VM id is reset
Prevent the possibility that a VM could be considered as alive while
inside qemuProcessStop.

A recently fixed bug which unlocked the domain object while inside
qemuProcessStop showed that there's possibility to confuse the state of
the VM to be considered active while 'qemuProcessStop' is processing
shutdown of the VM. Ensure that this doesn't happen by clearing the
'beingDestroyed' flag only after the VM id is cleared.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:58:52 +02:00
Peter Krempa
3865410e7f qemuProcessStop: Move code not depending on 'vm->def->id' after reset of the ID
There are few function calls done while cleaning up a stopped VM which
do require the old VM id, to e.g. clean up paths containing the 'short'
domain name in the path.

Anything else, which doesn't strictly require it can be moved after
clearing the 'id' in order to decrease likelyhood of potential bugs.

This patch moves all the code which does not require the 'id' (except
for the log entry and closing the monitor socket) after the statement
clearing the id and adds a comment explaining that anything in the
section must not unlock the VM object.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:58:52 +02:00
Peter Krempa
d29e0f3d4a qemuProcessStop: Prevent crash when qemuDomainObjStopWorker() unlocks the VM
'qemuDomainObjStopWorker()' which is meant to dispose of the event loop
thread for the monitor unlocks the VM object while disposing the thread
to prevent possible deadlocks with events waiting on the monitor thread.

Unfortunately 'qemuDomainObjStopWorker()' is called *before* the VM is
marked as inactive by clearing 'vm->def->id', but at the same time it's
no longer marked as 'beingDestroyed' when we're inside
'qemuProcessStop()'.

If 'vm' would be kept locked this wouldn't be a problem. Same way it's
not a problem for anything that uses non-ASYNC VM jobs, or when the
monitor is accessed in an async job, as the 'destroy' job interlocks
with those.

It is a problem for code inside an async job which uses
'qemuDomainObjWait()' though. The API contract of qemuDomainObjWait()
ensures the caller that the VM on successful return from it, but in this
specific reason it's not the case, as both 'beingDestroyed' is already
false, and 'vm->def->id' is not yet cleared.

To fix the issue move the 'qemuDomainObjStopWorker()' call *after*
clearing 'vm->def->id' and also add a note stating what the function is
doing.

Fixes: 860a999802
Closes: https://gitlab.com/libvirt/libvirt/-/issues/640
Reported-by: luzhipeng <luzhipeng@cestc.cn>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:58:21 +02:00
Peter Krempa
da8d97e4e2 qemuDomainObjWait: Add documentation
Document why this function exists and meaning of return values.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:52:55 +02:00
Peter Krempa
f9ad21996d qemuDomainDeviceBackendChardevForeach: Fix typo in comment
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:52:54 +02:00
Peter Krempa
b4423a753b qemuDomainDiskPrivateDispose: Prevent dangling 'disk' pointer in blockjob data
Clear the 'disk' member of 'blockjob' as we're freeing the disk object
at this point. While this should not normally happen it was observed
when other bug allowed the VM to be cleared while other threads didn't
yet finish.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:52:54 +02:00
Peter Krempa
737f897c29 qemuBlockJobProcessEventConcludedBackup: Handle potentially NULL 'job->disk'
Similarly to other blockjob handlers, if there's no disk associated with
the blockjob the handler needs to behave correctly. This is needed as
the disk might have been de-associated on unplug or other operations.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:52:54 +02:00
Boris Fiuczynski
09cc83dcf6 nodedev: add ccw device state and remove fencing
Instead of fencing offline ccw devices add the state to the ccw
capability.

Resolves: https://issues.redhat.com/browse/RHEL-39497
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:38:46 +02:00
Boris Fiuczynski
69d8a327f1 nodedev: prevent invalid DASD node object creation
Prevent the creation of a new DASD node object when the device does not
exist.

Resolves: https://issues.redhat.com/browse/RHEL-39497
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:34:54 +02:00
Boris Fiuczynski
e9c23d906f nodedev: improve DASD detection
In newer DASD driver versions the ID_TYPE tag is supported. This tag is
missing after a system reboot but when the ccw device is set offline and
online the tag is included. To fix this version independently we need to
check if devices detected as type disk is actually a DASD to maintain
the node object consistency and not end up with multiple node objects
for DASDs.

Resolves: https://issues.redhat.com/browse/RHEL-39497
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:34:19 +02:00
Boris Fiuczynski
4062440b4b nodedev: refactor storage type fixup
Refactor the storage type fixup into a reusable method.

Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-20 09:33:32 +02:00
Michal Privoznik
43d2edc08f virnetworkobj: Free fwRemoval before setting another one in virNetworkObjSetFwRemoval()
The virNetworkObjSetFwRemoval() function is called at least two
times when there's a network running and network driver
initializes:

1) when loading state XMLs:
  #0  virNetworkObjSetFwRemoval (obj=0x7fffd4028250, fwRemoval=0x7fffd4020ad0) at ../src/conf/virnetworkobj.c:258
  #1  0x00007ffff7a69c68 in virNetworkLoadState (...) at ../src/conf/virnetworkobj.c:952
  #2  0x00007ffff7a6a35d in virNetworkObjLoadAllState (...) at ../src/conf/virnetworkobj.c:1072
  #3  0x00007ffff7f9625f in networkStateInitialize (...) at ../src/network/bridge_driver.c:624

2) when firewall rules are being reloaded:
  #0  virNetworkObjSetFwRemoval (obj=0x7fffd4028250, fwRemoval=0x7fffd402e5b0) at ../src/conf/virnetworkobj.c:258
  #1  0x00007ffff7f997b4 in networkReloadFirewallRulesHelper (obj=0x7fffd4028250, opaque=0x0) at ../src/network/bridge_driver.c:1703
  #2  0x00007ffff7a6b09b in virNetworkObjListForEachHelper (payload=0x7fffd4028250, ...) at ../src/conf/virnetworkobj.c:1414
  #3  0x00007ffff79287b6 in virHashForEachSafe (...) at ../src/util/virhash.c:387
  #4  0x00007ffff7a6b119 in virNetworkObjListForEach (...) at ../src/conf/virnetworkobj.c:1441
  #5  0x00007ffff7f99978 in networkReloadFirewallRules (...) at ../src/network/bridge_driver.c:1742
  #6  0x00007ffff7f962f2 in networkStateInitialize (...) at ../src/network/bridge_driver.c:645

Since virNetworkObjSetFwRemoval() does not free the object stored
in the first call, the second call just overwrites the stored
pointer leading to a memory leak:

  5,530 (48 direct, 5,482 indirect) bytes in 1 blocks are definitely lost in loss record 1,863 of 1,880
     at 0x4848C43: calloc (vg_replace_malloc.c:1595)
     by 0x4F1E979: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.7800.6)
     by 0x4976E32: virFirewallNew (virfirewall.c:118)
     by 0x4979BA9: virFirewallParseXML (virfirewall.c:1071)
     by 0x4ABEB1E: virNetworkLoadState (virnetworkobj.c:938)
     by 0x4ABF35C: virNetworkObjLoadAllState (virnetworkobj.c:1072)
     by 0x4E9A25E: networkStateInitialize (bridge_driver.c:624)
     by 0x4CB1FA6: virStateInitialize (libvirt.c:665)
     by 0x15A6C6: daemonRunStateInit (remote_daemon.c:611)
     by 0x49E69F0: virThreadHelper (virthread.c:256)
     by 0x532B428: start_thread (in /lib64/libc.so.6)
     by 0x5397373: clone (in /lib64/libc.so.6)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-19 16:31:23 +02:00
Michal Privoznik
be1e745cd2 virfirewall: Fir a memleak in virFirewallParseXML()
As a part of parsing XML, virFirewallParseXML() calls
virXMLNodeContentString() and then passes the return value
further. But virXMLNodeContentString() is documented so that it's
the caller's responsibility to free the returned string, which
virFirewallParseXML() never does. This leads to a memory leak:

  14,300 bytes in 220 blocks are definitely lost in loss record 1,879 of 1,891
     at 0x4841858: malloc (vg_replace_malloc.c:442)
     by 0x5491E3C: xmlBufCreateSize (in /usr/lib64/libxml2.so.2.12.6)
     by 0x54C2401: xmlNodeGetContent (in /usr/lib64/libxml2.so.2.12.6)
     by 0x49F7791: virXMLNodeContentString (virxml.c:354)
     by 0x4979F25: virFirewallParseXML (virfirewall.c:1134)
     by 0x4ABEB1E: virNetworkLoadState (virnetworkobj.c:938)
     by 0x4ABF35C: virNetworkObjLoadAllState (virnetworkobj.c:1072)
     by 0x4E9A25E: networkStateInitialize (bridge_driver.c:624)
     by 0x4CB1FA6: virStateInitialize (libvirt.c:665)
     by 0x15A6C6: daemonRunStateInit (remote_daemon.c:611)
     by 0x49E69F0: virThreadHelper (virthread.c:256)
     by 0x532B428: start_thread (in /lib64/libc.so.6)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-19 16:31:23 +02:00
Weblate
3f589def04 Translated using Weblate (Georgian)
Currently translated at 4.3% (457 of 10442 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/ka/

Co-authored-by: Weblate <noreply-mt-weblate@weblate.org>
Signed-off-by: Weblate <noreply-mt-weblate@weblate.org>
2024-06-19 14:25:29 +02:00
Göran Uddeborg
93b8cf12dc Translated using Weblate (Swedish)
Currently translated at 76.3% (7982 of 10454 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/sv/

Translated using Weblate (Swedish)

Currently translated at 76.1% (7962 of 10454 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/sv/

Translated using Weblate (Swedish)

Currently translated at 75.9% (7942 of 10454 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/sv/

Translated using Weblate (Swedish)

Currently translated at 75.5% (7902 of 10454 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/sv/

Translated using Weblate (Swedish)

Currently translated at 75.3% (7882 of 10454 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/sv/

Translated using Weblate (Swedish)

Currently translated at 75.2% (7862 of 10454 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/sv/

Translated using Weblate (Swedish)

Currently translated at 74.8% (7822 of 10454 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/sv/

Co-authored-by: Göran Uddeborg <goeran@uddeborg.se>
Signed-off-by: Göran Uddeborg <goeran@uddeborg.se>
2024-06-19 14:25:29 +02:00
Martin Kletzander
025925a901 vmx: Accept more serial variations
Commit 23c4794488 added parsing of serial ports connected to vspc, but
the VM can also have a network serial port with an empty filename or no
filename at all.  Parse these the same way, as a <serial type='null'>.

Resolves: https://issues.redhat.com/browse/RHEL-32182

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-06-19 14:28:38 +02:00
Adam Julis
503a4e6a79 conf: Drop unused virDomainDiskFindByBusAndDst() declaration
Remove unused declaration of the virDomainDiskFindByBusAndDst()
function. Removed in v5.9.0-rc1~91 and then mistakenly
re-introduced in v5.9.0-rc1~65.

Signed-off-by: Adam Julis <ajulis@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-19 13:55:53 +02:00
Swapnil Ingle
c772f1982d Pass shutoff reason to release hook
Sometimes in release hook it is useful to know if the VM shutdown was graceful
or not. This is especially useful to do cleanup based on the VM shutdown failure
reason in release hook. This patch proposes to use the last argument 'extra'
to pass VM shutoff reason in the call to release hook.
Making this change for Qemu and LXC.

Signed-off-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-06-19 12:15:26 +02:00
Marc Hartmayer
2b199ad3f1 node_device_udev: remove incorrect G_GNUC_UNUSED
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
2024-06-18 09:00:37 -05:00
Marc Hartmayer
65214fcebd node_device_udev: Pass the udevEventData via parameter and use refcounting
Instead of accessing the global `driver` object pass the `udevEventData` as
parameter to the thread handler and watch callback. This has the advantage that:
1. proper refcounting
2. easier to read and test

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:36 -05:00
Marc Hartmayer
0f8717b1c7 node_device_udev: Add support for g_autoptr to udevEventData
Use this feature in `udevEventDataNew`.

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:34 -05:00
Marc Hartmayer
140cdf7f9a node_device_udev: Make the code easier to read
There is only one case where force is true, therefore let's inline that case.

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:33 -05:00
Marc Hartmayer
b56458d443 node_device_udev: Use a worker pool for processing events and emitting nodedev event
Use a worker pool for processing the events (e.g. udev, mdevctl config changes)
and the initialization instead of a separate initThread and a mdevctl-thread.
This has the large advantage that we can leverage the job API and now this
thread pool is responsible to do all the "costly-work" and emitting the libvirt
nodedev events.

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:32 -05:00
Marc Hartmayer
01ab7047e9 node_device_udev: Pass the driver state as parameter in preparation for the next commit
It's better practice for all functions called by the threads to pass the driver
via parameter and not global variables. Easier to test and cleaner.

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:30 -05:00
Marc Hartmayer
2c3e4a0f6e node_device_udev: nodeStateShutdownPrepare: Disconnect the signals explicitly
The documentation of gobject signals reads:

"If you are connecting handlers to signals and using a GObject instance as your
signal handler user data, you should remember to pair calls to
g_signal_connect() with calls to g_signal_handler_disconnect() or
g_signal_handlers_disconnect_by_func(). While signal handlers are automatically
disconnected when the object emitting the signal is finalised..." [1]

This means that the signal handlers are automatically disconnected as soon as
the `priv->mdevCtlMonitors` are finalised/released by `udevEventDataDispose`.
But this also means that it's possible that new work is tried to be scheduled
for the workerpool by the `mdevctlEventHandleCallback` (main thread context)
even if the workerpool has already been stopped by `nodeStateShutdownWait`. To
fully understand this, it's important to know that the main loop of the main
thread is still running for some time even after `nodeStateShutdownPrepare` has
been called. Let's avoid this situation by explicitly disconnect the signals
during `nodeStateShutdownPrepare`, which is called in the main thread, so that
no new work is attempted to be scheduled for the worker pool.

[1] https://docs.gtk.org/gobject/signals.html#memory-management-of-signal-handlers

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:29 -05:00
Marc Hartmayer
e89d39f5b8 node_device_udev: Introduce and use stateShutdownPrepare and stateShutdownWait
Introduce and use the driver functions for the node state shutdown preparation
and wait. As they're also called in the error/cleanup path of
`nodeStateInitialize`, they must be written in a way, that they can safely be
executed even if not everything is initialized.

In the next commit, these functions will be extended.

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:28 -05:00
Marc Hartmayer
6e727d8bdc node_device_udev: Fix leak of mdevctlLock, udevThreadCond, and mdevCtlMonitors
Even if `priv->udev_monitor` was never initialized, the mdevctlLock, udevThread
were. Therefore let's match the order of releasing the resources the order of
allocating the resources in `nodeStateInitialize`.

In addition, use `g_steal_pointer` in `g_list_free_full`.

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:26 -05:00
Marc Hartmayer
4daa362706 node_device_udev: Move responsibility to release (init|udev)Thread to udevEventDataDispose
Everything is released in `udevEventDataDispose` except for the threads, change
this as this makes the code easier to read as it can be simplified a little.

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:24 -05:00
Marc Hartmayer
d7c8908be8 node_device_udev: Inline udevRemoveOneDevice
Inline `udevRemoveOneDevice` as it's used only once.

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:23 -05:00
Marc Hartmayer
e6b70ae0c3 node_device_udev: Add prefix udev for udev related data
The new names make it easier to understand the purpose of the data.

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:20 -05:00
Marc Hartmayer
f51d729dd0 node_device_udev: Take lock if driver->privateData is modified
Since @driver->privateData is modified take the lock.

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:19 -05:00
Marc Hartmayer
c320c37917 node_device_udev: Don't take mdevctlLock for mdevctl list and add comments about locking
Commit a99d876a0f ("node_device: Use automatic mutex management") replaced the
locking mechanism and accidentally removed the comment with the reason why the
lock is taken. The reason was to "ensure only a single thread can query mdevctl
at a time", but this reason is no longer valid or maybe it never was. Therefore,
let's remove this lock and add a comment to `mdevCtl` what it protects.

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:18 -05:00
Marc Hartmayer
1606d7ec99 node_device_udev: Test for mdevctlTimeout != -1
It is done a little differently everywhere in libvirt, but most common is to
test for != -1.

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:16 -05:00
Marc Hartmayer
b13ddadc51 node_device_udev: Remove the timeout if the data is disposed
Remove the timeout when the udevEventData is disposed, analogous to priv->watch.

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:15 -05:00
Boris Fiuczynski
0f87a53a0a nodedev: reset active config data on udev remove event
When a mdev device is destroyed or stopped the udev remove event
handling needs to reset the active config data of the node object
representing a persisted mdev.

Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
2024-06-18 09:00:11 -05:00
Boris Fiuczynski
23df65de2f nodedev: immediate update of active config on udev events
When an udev add, change or remove event occurs the mdev active config data
requires an update via mdevctl as the udev does not contain all config data.
This update needs to occur immediately and to be finished before the libvirt
nodedev event is issued to keep the API usage reliable.

After this change, scheduleMdevctlUpdate call is already called in
`udevAddOneDevice` and can therefore be removed in `udevHandleOneDevice`.

Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 09:00:05 -05:00
Marc Hartmayer
30354f5b1f node_device_udev: Set @def to NULL
@def is owned by @obj after adding it the node device object list. As soon as
the @obj lock has been released, another thread could free @obj and therefore
@def. If now someone accesses @def this would lead to a heap-use-after-free and
therefore most likely to a segmentation fault, therefore set @def to NULL after
the ownership has moved.

While at it, add comments to other code places why @def is set to NULL.

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
2024-06-18 08:59:46 -05:00