public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Dominik Csapak <d.csapak@proxmox.com>
Subject: [pve-devel] applied: [PATCH qemu-server] mediated devices: fix race condition in vm reboot
Date: Fri, 8 Mar 2024 14:18:51 +0100	[thread overview]
Message-ID: <460d4de8-39b4-4965-b309-446f23dbb34f@proxmox.com> (raw)
In-Reply-To: <20240307093337.1104294-1-d.csapak@proxmox.com>

Am 07/03/2024 um 10:33 schrieb Dominik Csapak:
> when triggering a vm reboot from the host (via cli/api), the reboot code
> is called under a guest lock, which creates a reboot request, shuts down
> the vm and calls the regular cleanup code (which includes the mdev
> cleanup).
> 
> in parallel the qmeventd sees the vanished vm, and starts 'qm cleanup'
> which is (among other tasks) used to restart a vm with a pending reboot
> request. It does this also under a guest lock, with a default timeout of
> 10 seconds.
> 
> Since we currently wait 10 seconds for the nvidia driver to clean the
> mdev, this creates a race condition for the cleanup lock. Iow. when the
> call to `qm cleanup` starts before we started to sleep for 10 seconds,
> it will not be able to acquire its lock and not start the vm again.
> 
> To fix it, do two things:
> * increase the timeout in `qm cleanup` to 60 seconds
>   (technically this still might run into a timeout, as we can configure
>   up to 16 mediated devices with up to 10 seconds sleep each, but
>   realistically most users won't configure more than two or three of
>   them, if even that)
> 
> * change the `sleep 10` to a loop sleeping for 1 second each before
>   checking the state again. This shortens the timeout when the driver
>   can clean it up in the meantime.
> 
> Further, add a bit of logging, so we can properly see in the (task) log
> what is happening when.
> 
> Fixes: 49c51a60 (pci: workaround nvidia driver issue on mdev cleanup)
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/CLI/qm.pm     |  3 ++-
>  PVE/QemuServer.pm | 16 ++++++++++++----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
>

applied with Mira's R-b, thanks!

Albeit I amended the commit to reword the message slightly and to switch
from using warn to print for such informational messages.
As both stdout and stderr shows up in task log that should not matter,
some code might wire up $SIG{__WARN__} though, causing potentially
confusing syslog noise or even failures.




      parent reply	other threads:[~2024-03-08 13:19 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-07  9:33 [pve-devel] " Dominik Csapak
2024-03-07 13:31 ` Mira Limbeck
2024-03-08 13:18 ` Thomas Lamprecht [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=460d4de8-39b4-4965-b309-446f23dbb34f@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal