From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <f.ebner@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id CC3FA997D7
 for <pve-devel@lists.proxmox.com>; Wed, 11 Oct 2023 09:38:46 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id A33533987
 for <pve-devel@lists.proxmox.com>; Wed, 11 Oct 2023 09:38:46 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Wed, 11 Oct 2023 09:38:45 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 475BC449D6
 for <pve-devel@lists.proxmox.com>; Wed, 11 Oct 2023 09:38:45 +0200 (CEST)
Message-ID: <e190bff9-56b3-4f87-99f7-de3a93164e8e@proxmox.com>
Date: Wed, 11 Oct 2023 09:38:39 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: pve-devel@lists.proxmox.com, Thomas Lamprecht <t.lamprecht@proxmox.com>,
 Filip Schauer <f.schauer@proxmox.com>
References: <20231009132519.115727-1-f.schauer@proxmox.com>
 <01c649c9-0717-404c-98d8-17dc8a5cc4eb@proxmox.com>
 <d59420f3-2b1f-5de0-2106-38441c5bbf4d@proxmox.com>
From: Fiona Ebner <f.ebner@proxmox.com>
In-Reply-To: <d59420f3-2b1f-5de0-2106-38441c5bbf4d@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.087 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH v3 qemu-server] Fix ACPI-suspended VMs
 resuming after migration
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Wed, 11 Oct 2023 07:38:46 -0000

Am 10.10.23 um 16:19 schrieb Fiona Ebner:
>> Also, with that change we might have added a race for suspend-mode backups,
>> at least if VMs really can wake up without a QMP command (which I find likely).
>> I.e., between the time we checked and set vm_was_paused until we actually
>> suspend, because if the VM would wake up in between we might get inconsistent
>> stuff and skip things like fs-freeze.
> 
> That race is already there without the patch. QEMU does not transition
> from 'suspended' state to 'paused' state when QMP 'stop' is issued, i.e.
> what our 'qm suspend' or vm_suspend() actually does. So it doesn't
> matter if we call that during backup or not when the VM is already in
> 'suspended' state.
> 
> The window for the race is a bit larger though:
> now: VM wakes up between check if paused and 'backup' QMP
> before: VM wakes up after fsfreeze was skipped because guest agent was
> detected as not running
> 

What's written above actually applies to 'snapshot' mode backup, but not
to 'suspend' mode backup. 'suspend' mode backup is and was broken in
this regard already:

> root@pve8a1 ~ # vzdump 100 --storage pbs --mode suspend 
> INFO: starting new backup job: vzdump 100 --mode suspend --storage pbs
> INFO: Starting Backup of VM 100 (qemu)
> INFO: Backup started at 2023-10-11 09:25:53
> INFO: status = running
> INFO: backup mode: suspend
> INFO: ionice priority: 7
> INFO: VM Name: Copy-of-VM-apache
> INFO: include disk 'scsi0' 'rbd:vm-100-disk-0' 4618348134
> INFO: suspending guest
> INFO: snapshots found (not included into backup)
> INFO: creating Proxmox Backup Server archive 'vm/100/2023-10-11T07:25:53Z'
> INFO: skipping guest-agent 'fs-freeze', agent configured but not running?
> INFO: started backup task '923c95cf-6bb6-4476-825e-390c147a4e76'
> INFO: resuming VM again after 3 seconds
> INFO: scsi0: dirty-bitmap status: OK (8.0 MiB of 4.3 GiB dirty)
> INFO: using fast incremental mode (dirty-bitmap), 8.0 MiB dirty of 4.3 GiB total
> INFO: 100% (8.0 MiB of 8.0 MiB) in 1s, read: 8.0 MiB/s, write: 8.0 MiB/s
> INFO: backup was done incrementally, reused 4.29 GiB (99%)
> INFO: transferred 8.00 MiB in 1 seconds (8.0 MiB/s)
> INFO: adding notes to backup
> INFO: resume vm
> INFO: Finished Backup of VM 100 (00:00:06)
> INFO: Backup finished at 2023-10-11 09:25:59
> INFO: Backup job finished successfully

We never got the fsfreeze in suspend mode, because we do it after the
QMP 'stop' when the vCPUs are paused, so the guest agent won't be
running. The backup just ends up with whatever state the filesystem was
in when QMP 'stop' is issued.

Time to finally deprecate 'suspend' mode backup for VMs for good? It
really has just disadvantages compared to 'snapshot' mode and our docs
state "This mode is provided for compatibility reason" for a long time now.

> We could either:
> 
> 1. (ironically) disallow 'suspend' mode backup when in 'suspended' runstate.
> 2. resume and pause the VM upon 'suspend' mode backup, but is also
> surprising
> 3. make the race window smaller again by doing the qga_check_running()
> and fs-freeze if true, if VM was in 'suspended' runstate.
> 
> I don't see how to avoid the possibility of a wake-up during an
> inconvenient time except with one of the first two options.
>