On 1/7/26 15:38, Thomas Prescher wrote:
> Fix locking in qemuProcessHandleMemoryFailure. We use a lock guard
> now because we can directly return from the default switch cases.

Yeah, this doesn't look good but IIUC that can never happen. I mean,
qemuMonitorJSONHandleMemoryFailure() parses the incoming event and if
there's an unknown recipient or action then the function throws a
warning and returns early. Nevertheless, the pattern as-is gives a bad
example.

> 
> Issue has been discovered by [email protected]
> 
> On-behalf-of: SAP [email protected]
> Signed-off-by: Thomas Prescher <[email protected]>
> ---
>  src/qemu/qemu_process.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 4e1d713809..cc902e1d37 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1940,7 +1940,7 @@ qemuProcessHandleMemoryFailure(qemuMonitor *mon 
> G_GNUC_UNUSED,
>      virDomainMemoryFailureActionType action;
>      unsigned int flags = 0;
>  
> -    virObjectLock(vm);
> +    VIR_LOCK_GUARD lock = virObjectLockGuard(vm);
>      driver = QEMU_DOMAIN_PRIVATE(vm)->driver;

Nit pick, the 'lock' variable declaration should go into the block
that's declaring other variables.

>  
>      switch (mfp->recipient) {
> @@ -1980,8 +1980,6 @@ qemuProcessHandleMemoryFailure(qemuMonitor *mon 
> G_GNUC_UNUSED,
>  
>      event = virDomainEventMemoryFailureNewFromObj(vm, recipient, action, 
> flags);
>  
> -    virObjectUnlock(vm);
> -
>      virObjectEventStateQueue(driver->domainEventState, event);
>  }
>  

Reviewed-by: Michal Privoznik <[email protected]>

Michal

Reply via email to