Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once

Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once

[Max Carrara]

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.)

>
> 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.

Though, I really like the idea overall; I don't see why we shouldn't
add this.

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

>
>
> [0] https://github.com/storpool/pve-storpool
> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=5752
>
> Best regards,



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once

[Ivaylo Markov via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 10860 bytes --]

From: Ivaylo Markov <ivaylo.markov@storpool.com>
To: Max Carrara <m.carrara@proxmox.com>, pve-devel@lists.proxmox.com
Subject: Re: Proposal: support for atomic snapshot of all VM disks at once
Date: Tue, 8 Oct 2024 15:49:07 +0300
Message-ID: <87539f65-46c9-4f9b-a9fd-bcffff54b71e@storpool.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



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] Proposal: support for atomic snapshot of all VM disks at once

[Ivaylo Markov via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 6833 bytes --]

From: Ivaylo Markov <ivaylo.markov@storpool.com>
To: pve-devel@lists.proxmox.com
Subject: Proposal: support for atomic snapshot of all VM disks at once
Date: Fri, 4 Oct 2024 16:54:39 +0300
Message-ID: <b16012bc-d4a9-42ff-b9ba-523649c6cfa6@storpool.com>

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.

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.

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.

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.


[0] https://github.com/storpool/pve-storpool
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=5752

Best regards,

-- 
Ivaylo Markov
StorPool Storage
https://www.storpool.com



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once

[Dietmar Maurer]

> 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. 

We already make sure that shaphots of a group of volumes are atomic at qemu level (VM is halted while snapshots are created), so I wonder
if there is any real advantage here?


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once

[Fabian Grünbichler]

> Dietmar Maurer <dietmar@proxmox.com> hat am 04.10.2024 17:13 CEST geschrieben:
> > 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. 
> 
> We already make sure that shaphots of a group of volumes are atomic at qemu level (VM is halted while snapshots are created), so I wonder
> if there is any real advantage here?

I think the main advantage is that if you are fine with crash-equivalent consistency (e.g., the same as pulling the power plug) for your snapshots, then you can skip the freeze/suspend (and associated, hopefully tiny, downtime), and just snapshot on the storage layer. there might also be additional optimizations or administrative benefits on the storage side (like all snapshots being part of a single transaction), but those are probably not relevant for PVE itself..


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once

[Ivaylo Markov via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 6524 bytes --]

From: Ivaylo Markov <ivaylo.markov@storpool.com>
To: pve-devel@lists.proxmox.com
Subject: Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once
Date: Mon, 7 Oct 2024 10:27:09 +0300
Message-ID: <1a8d02ca-ae00-4ab4-bae3-86446452bf6d@storpool.com>

Hi,

On 07/10/2024 09:12, Fabian Grünbichler wrote:
>> Dietmar Maurer <dietmar@proxmox.com> hat am 04.10.2024 17:13 CEST geschrieben:
>>> 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.
>> We already make sure that shaphots of a group of volumes are atomic at qemu level (VM is halted while snapshots are created), so I wonder
>> if there is any real advantage here?
> I think the main advantage is that if you are fine with crash-equivalent consistency (e.g., the same as pulling the power plug) for your snapshots, then you can skip the freeze/suspend (and associated, hopefully tiny, downtime), and just snapshot on the storage layer. there might also be additional optimizations or administrative benefits on the storage side (like all snapshots being part of a single transaction), but those are probably not relevant for PVE itself..
>
This is exactly it - we're looking to avoid any slow downs/interruptions 
in the VMs when creating snapshots. I've not measured how long other 
storage plugins take to create snapshots, but it could be helpful for 
them if it's a slow operation.

-- 
Ivaylo Markov
Quality & Automation Engineer
StorPool Storage
https://www.storpool.com



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once

[DERUMIER, Alexandre via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 13594 bytes --]

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once
Date: Mon, 7 Oct 2024 10:58:51 +0000
Message-ID: <42be55cf5a0ce1c10b72e3cfaf19565362ec98e1.camel@groupe-cyllene.com>

I'm also interested. I have already seen the time drift when take
snapshots, not cool on a database where time transaction is important.


ceph rbd support group snapshot too.


-------- Message initial --------
De: Ivaylo Markov via pve-devel <pve-devel@lists.proxmox.com>
Répondre à: Proxmox VE development discussion <pve-
devel@lists.proxmox.com>
À: pve-devel@lists.proxmox.com
Cc: Ivaylo Markov <ivaylo.markov@storpool.com>
Objet: Re: [pve-devel] Proposal: support for atomic snapshot of all VM
disks at once
Date: 07/10/2024 09:27:09

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://antiphishing.vadesecure.com/v4?f=RzlZTWtkemNSOUVkZTJBYdS-
XA_pzMA4Fz4JRkZPA3dhY7Z3Sd_r2__EuGgZU0WuXFGUbGNFySEFJnvos_-
YVg&i=SGxjaUlnNWlZWlNxaGhBVLimx-
9C6COfka5pjSczJyE&k=NLuj&r=Qnhka2E0dmNmY3lSdFV6VMmOR0tkA89HrIpauh33IaSI
b8HXyemHfLHRjfniwD5E&s=3030c97c49aff97e4946212cc892131a8e5755e717fc137b
93ea997364cb2b8c&u=https%3A%2F%2Flists.proxmox.com%2Fcgi-
bin%2Fmailman%2Flistinfo%2Fpve-devel


[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once

[Daniel Berteaud via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 4706 bytes --]

From: Daniel Berteaud <dani@lapiole.org>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] Proposal: support for atomic snapshot of all VM disks at once
Date: Mon, 7 Oct 2024 09:12:46 +0200 (CEST)
Message-ID: <1324502869.237.1728285166262.JavaMail.zimbra@lapiole.org>



----- Le 4 Oct 24, à 17:13, Dietmar Maurer dietmar@proxmox.com a écrit :

> We already make sure that shaphots of a group of volumes are atomic at qemu
> level (VM is halted while snapshots are created), so I wonder
> if there is any real advantage here?

I think there could be one advantage : reduce the freez time during snapshot. I take regular snapshots of my VM, and it causes timeshifts of a few seconds (which chronyd inside the guest takes a bit a time to correct). It's just a 1-4 sec, but it can cause issues. One example is minio, when used with an OIDC login (as minio doesn't support any clock skew, login can fail if minio lags just a few seconds behind the IDP)

++

-- 
Daniel Berteaud



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel