From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id AC9831FF161 for ; Tue, 8 Oct 2024 14:48:53 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8C14515B1F; Tue, 8 Oct 2024 14:49:18 +0200 (CEST) Date: Tue, 8 Oct 2024 15:49:07 +0300 To: Max Carrara , pve-devel@lists.proxmox.com References: In-Reply-To: MIME-Version: 1.0 Message-ID: List-Id: Proxmox VE development discussion List-Post: From: Ivaylo Markov via pve-devel Precedence: list Cc: Ivaylo Markov X-Mailman-Version: 2.1.29 X-BeenThere: pve-devel@lists.proxmox.com List-Subscribe: , List-Unsubscribe: , List-Archive: Reply-To: Proxmox VE development discussion List-Help: Subject: Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once Content-Type: multipart/mixed; boundary="===============2273488556529555514==" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" --===============2273488556529555514== Content-Type: message/rfc822 Content-Disposition: inline Return-Path: X-Original-To: pve-devel@lists.proxmox.com Delivered-To: pve-devel@lists.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 B1583C37F1 for ; Tue, 8 Oct 2024 14:49:16 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8EFF815A6D for ; Tue, 8 Oct 2024 14:49:16 +0200 (CEST) Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com [IPv6:2a00:1450:4864:20::52d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 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 ; Tue, 8 Oct 2024 14:49:15 +0200 (CEST) Received: by mail-ed1-x52d.google.com with SMTP id 4fb4d7f45d1cf-5c88c9e45c2so11463318a12.0 for ; Tue, 08 Oct 2024 05:49:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=storpool.com; s=google; t=1728391749; x=1728996549; darn=lists.proxmox.com; h=content-transfer-encoding:in-reply-to:organization:from :content-language:references:to:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=71xjEaPIC+LF7vKBhmwxrWnDwtVyhXgSlXPqZv+0Im8=; b=JRL0qRvk54qR7SxpK01zxjDx35ZH+TAU5zHusc5B+mjy/h1YtKbxkGXZhDx64xy5go GXRjgNmwRmGwJ3gFWgQTVQRokNBVa8Q+Hps4XxD9CnyGg7sOgNvBsOuQ5vWUlGXcwfnV nWuQkPiJkfzLRn5+9Hkf9qhH7gKKfcUJaSjwvKmZicGTiwVAzSV3VpKjOS9xSleG9XpP hEylzu+y+1PbK4TCFGr7KPPsteDG2V9b8pwa6orm4Y5+G96lSCwmC7I4qK/W7hdJF5r5 lUkIvqh3xA2iKBcyrJWBUp3znCuq1SKD60QJb+G7MNftBsYy2dRt6BrZ8hVoZYEhcJLr MPyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728391749; x=1728996549; h=content-transfer-encoding:in-reply-to:organization:from :content-language:references:to:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=71xjEaPIC+LF7vKBhmwxrWnDwtVyhXgSlXPqZv+0Im8=; b=CcOn1XqFkJPynfpQQKyLJzsVtBg8OfaeJdsOsCQV41Q1XsNYVBvLuwujwWJoUI85Sh XnHcsfBJ8OFxZpi45XBCWwZxkiERq4vFIOi0tvTmi4q96AcFgZa3MXCwXbSZ7KhRawzW GEcrcDY3H6Y77dwZQcfbVK5Cw0te+JaQgPm/uITuwHSUz23rrUGREXFwRmJec1gjsyQg y+wHX6DLqbYMb1fpx+urDJLPRCJC4qp3M2r0LnXqAr9rFEfqBWtxs5+bAKhRyndJJ3vp aLGIFfgKTQANmD8xXMImsY59exmS+AwQosDMK2ZxTTXO+ntfn//w+hNlVr1h1H1iBzmW UGMA== X-Forwarded-Encrypted: i=1; AJvYcCV7lLxHFPxtycIiV5cTDzUAbiXlhz19odyoiA0lEjn0wyJU559K48lt9lcV1Mtex5C/evodysNI6fM=@lists.proxmox.com X-Gm-Message-State: AOJu0YzZr5gFxme5IDRmyRD/ul7u4lw7hVbFK/kis91GgdAu81sQhPzV PJqvXtHrluaBTxw/RX0mwD4kp1nKvkc5cViYVTiqhWmnur5Ajjm6zZWdLOuWk5o= X-Google-Smtp-Source: AGHT+IGK7+Q2WDFX8oQDnuSDTOf08ZacamYxjGc3JtfE1XO9e88n8kdvMppclnr0kUNef6fIjFkM5A== X-Received: by 2002:a05:6402:280d:b0:5c8:97b9:58a6 with SMTP id 4fb4d7f45d1cf-5c91a28de78mr49207a12.1.1728391749041; Tue, 08 Oct 2024 05:49:09 -0700 (PDT) Received: from ?IPV6:2a05:5e40:f00f:31:4b57:5aee:9066:f627? ([2a05:5e40:f00f:31:4b57:5aee:9066:f627]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5c8e05c0f6fsm4311846a12.57.2024.10.08.05.49.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 08 Oct 2024 05:49:08 -0700 (PDT) Message-ID: <87539f65-46c9-4f9b-a9fd-bcffff54b71e@storpool.com> Date: Tue, 8 Oct 2024 15:49:07 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: Proposal: support for atomic snapshot of all VM disks at once To: Max Carrara , pve-devel@lists.proxmox.com References: Content-Language: en-US From: Ivaylo Markov Organization: StorPool Storage In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain DMARC_PASS -0.1 DMARC pass policy RCVD_IN_DNSWL_NONE -0.0001 Sender listed at https://www.dnswl.org/, no trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [storpool.com,proxmox.com] Hi On 08/10/2024 13:50, Max Carrara wrote: > On Fri Oct 4, 2024 at 3:54 PM CEST, Ivaylo Markov wrote: >> Greetings, >> >> I am the maintainer of StorPool’s external storage plugin for PVE[0] >> which integrates our storage solution as a backend for VM disks. Our >> software has the ability to create atomic (crash-consistent) snapshots >> of a group of storage volumes. I’d like to use this feature in our >> plugin so that customers can perform whole VM snapshots, but that does >> not seem possible currently - the snapshot creation method is called >> individually for every disk. > Hello! > > This does sound quite interesting. > >> I was directed here to discuss this proposal and my implementation idea >> after an initial post in Bugzilla[1]. The goal is to give storage >> plugins the option to perform atomic crash-consistent snapshots of the >> virtual disks associated with a virtual machine where the backend >> supports it  (e.g. Ceph, StorPool, and ZFS) without affecting those >> without such a feature. > Since you mentioned that the backends without such a feature won't be > affected, will the disks of the storage types that *do* support it still > be addressable individually? As in: Would it be possible to run both > group snapshots and individual snapshots on a VM's disks? (I'm assuming > that they will, but still wanted to ask.) No reason to remove the individual snapshot capability, my proposal is concerned entirely with whole-VM snapshots (perhaps I should've stated that explicitly). >> I would add a `can_snapshot_volume_group` method to the base >> `PVE::Storage::Plugin` class, which would accept an array of the VM’s >> disks, and return a binary result whether an atomic snapshot is >> possible. The default implementation would return 0, but plugins with >> support can override it based on backend capabilities. For example, ZFS >> supports atomic snapshot of volume groups, but requires all volumes to >> be in the same pool. >> The actual snapshot can be performed by a `snapshot_volume_group >> method`, which is not expected to be called unless the driver supports >> this operation. > For both of these methods you'll have to keep in mind that the > `...::Plugin` class is designed to only handle a single volume at once. > > I'm not against implementing methods that support addressing multiple > volumes (disks) at once, though, but the `PVE::Storage` API must handle > this gracefully. See more down below. > >> In `PVE::AbstractConfig::snapshot_create` these two methods can be used >> to check and perform the atomic snapshots where possible, otherwise it >> would keep the current behavior of creating a snapshot for each disk >> separately. If the VM has drives with completely different storage >> backends (e.g. ZFS and LVM for whatever reason), the driver check can be >> skipped, and the current behavior used again. > This sounds good to me, though one thing that should be noted here is > that everything should go through the `PVE::Storage` API, *not* > `PVE::Storage::Plugin`. > > This means that there need to be API subroutines for the `PVE::Storage` > module that are able to handle all the cases you described above. These > subs then work with `PVE::Storage::Plugin::can_snapshot_volume_group` > and `...::snapshot_volume_group`. > > In general, this looks something like this: > * `PVE::Storage::Plugin` defines interface for concrete storage plugin > implementations > * `PVE::Storage` isn't aware of the concrete implementations and > strictly uses only the `...::Plugin` interface > * `PVE::AbstractConfig` and others only use the API methods provided > by `PVE::Storage`, allowing us to abstract over many different types > of storages > > Since you already got your own storage plugin, I assume you're aware of > all of this, but I wanted to mention it regardless to avoid any > misunderstandings. > > Sooo, all of this means that this will also require subroutines in > `PVE::Storage` that expose this kind of functionality (checking if group > snapshots are possible, performing group snapshots, ...). > > Overall, you'd have to handle the following cases in `PVE::Storage`, > from what I can think of at the moment: > * Whether there is more than one underlying storage type > (if there is, don't support group snapshots) > * Whether the underlying storage type supports group snapshots *in > general* > * Whether a group snapshot may actually be performed on the passed > constellation of volumes (disks) > * ... > > You'll probably need to introduce more than the two methods in > `PVE::Storage::Plugin` you mentioned above in order to really support > group snapshots for each different storage type and handle all the > (edge) cases. Noted, thank you for the clarification. I will have to take another look at this part of the code, but I think I get the gist of it. > > Though, I really like the idea overall; I don't see why we shouldn't > add this. Great to hear. > > If you haven't already, you should have a look at this regarding > contributions: https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright Will have to run this by the company before I get started, but I don't see it being an issue at all. > >> >> [0] https://github.com/storpool/pve-storpool >> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=5752 >> >> Best regards, > -- Ivaylo Markov Quality & Automation Engineer StorPool Storage https://www.storpool.com --===============2273488556529555514== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel --===============2273488556529555514==--