Re: [pve-devel] [PATCH-SERIES v8 pve-storage/qemu-server] add external qcow2 snapshot support

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

> Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com> hat am 09.07.2025 18:21 CEST geschrieben:
> This patch series implement qcow2 external snapshot support for files && lvm volumes
> 
> The current internal qcow2 snapshots have bad write performance because no metadatas can be preallocated.
> 
> This is particulary visible on a shared filesystem like ocfs2 or gfs2.
> 
> Also other bugs are freeze/lock reported by users since years on snapshots delete on nfs
> (The disk access seem to be frozen during all the delete duration)
> 
> This also open doors for remote snapshot export-import for storage replication.
> 
> Changelog v8:
>     storage :
>      - fix Fabian comments
>      - add rename_snapshot   
>      - add qemu_resize
>      - plugin: restrict volnames
>      - plugin: use 'external-snapshots' instead 'snapext'
>     qemu-server:
>      - fix efi test with wrong volnames vm-disk-100-0.raw
>      - use rename_snapshot
> MAIN TODO:
>     - add snapshots tests in both pve-storage && qemu-server
>     - better handle snapshot failure with multiple disks

sent a few follow-ups, as I didn't manage to fully test things and depending on the outcome of such tests, it might be okay to apply the series with those follow-ups, and fix up the rest later, or not..

some open issues that I discovered that still need fixing:

1. missing activation when snapshotting an LVM volume if the VM is not running

snapshotting 'drive-scsi0' (test:123/vm-123-disk-0.qcow2)
snapshotting 'drive-scsi1' (lvm:vm-123-disk-0.qcow2)
  Renamed "vm-123-disk-0.qcow2" to "snap_vm-123-disk-0_test.qcow2" in volume group "lvm"
failed to stat '/dev/lvm/snap_vm-123-disk-0_test.qcow2'   <============
Use of uninitialized value $size in division (/) at /usr/share/perl5/PVE/Storage/LVMPlugin.pm line 671.
  Rounding up size to full physical extent 4.00 MiB
  Logical volume "vm-123-disk-0.qcow2" created.
  Logical volume "vm-123-disk-0.qcow2" successfully removed.
  Renamed "snap_vm-123-disk-0_test.qcow2" to "vm-123-disk-0.qcow2" in volume group "lvm"

2. storage migration with external snapshots needs to be implemented or disabled (LVM is raw-only at the moment)
3. moving a 'raw' lvm volume to the same storage with format 'qcow2' fails with "you can't move to the same storage with same format (500)" (UI and CLI, other way round works)
4. all snapshot volumes on extsnap dir storages will print warnings like

`this volume filename is not supported anymore`

when hitting `parse_namedir` - those can likely be avoided by skipping the warning if the name matches the snapshot format and external-snapshots are enabled..

5. the backing file paths are not relative for LVM
6. it's fairly easy to accidentally create qcow2-formatted LVM volumes, as opposed to the requirement to enable a non-UI storage option at storage creation time for dir storages, we might want to add some warning to the UI at least? or we could also guard usage of the format with a config option..
7. the snapshot name related helpers being public would be nice to avoid - one way would be to inline them and duplicate volume_snapshot_info in the LVM plugin, but if a better option is found that would be great
8. renaming a volume needs to be forbidden if snapshots exist, or the whole chain needs to be renamed (this is currently caught higher up in the stack, not sure if we need/want to also check in the storage layer?)

the parse_namedir change also need a close look to see if some other plugins get broken.. (@Wolfgang - since you are working on related changes!)

I am sure there are more rough edges to be found, so don't consider the list above complete!

> 
> 
> pve-storage:
> 
> Alexandre Derumier (13):
>   plugin: add qemu_img_create
>   plugin: add qemu_img_create_qcow2_backed
>   plugin: add qemu_img_info
>   plugin: add qemu_img_measure
>   plugin: add qemu_img_resize
>   rbd && zfs : create_base : remove $running param from volume_snapshot
>   storage: volume_snapshot: add $running param
>   storage: add rename_snapshot method
>   storage: add volume_support_qemu_snapshot
>   plugin: fix volname parsing
>   qcow2: add external snapshot support
>   lvmplugin: add qcow2 snapshot
>   tests: add lvmplugin test
> 
>  ApiChangeLog                         |  15 +
>  src/PVE/Storage.pm                   |  45 ++-
>  src/PVE/Storage/BTRFSPlugin.pm       |   6 +
>  src/PVE/Storage/CIFSPlugin.pm        |   1 +
>  src/PVE/Storage/Common.pm            |  33 ++
>  src/PVE/Storage/DirPlugin.pm         |  11 +
>  src/PVE/Storage/ESXiPlugin.pm        |   8 +-
>  src/PVE/Storage/ISCSIDirectPlugin.pm |   2 +-
>  src/PVE/Storage/LVMPlugin.pm         | 571 +++++++++++++++++++++-----
>  src/PVE/Storage/LvmThinPlugin.pm     |   8 +-
>  src/PVE/Storage/NFSPlugin.pm         |   1 +
>  src/PVE/Storage/PBSPlugin.pm         |   2 +-
>  src/PVE/Storage/Plugin.pm            | 518 +++++++++++++++++++++---
>  src/PVE/Storage/RBDPlugin.pm         |  18 +-
>  src/PVE/Storage/ZFSPlugin.pm         |   4 +-
>  src/PVE/Storage/ZFSPoolPlugin.pm     |   8 +-
>  src/test/Makefile                    |   5 +-
>  src/test/run_test_lvmplugin.pl       | 577 +++++++++++++++++++++++++++
>  18 files changed, 1662 insertions(+), 171 deletions(-)
>  create mode 100755 src/test/run_test_lvmplugin.pl
> 
> qemu-server :
> 
> Alexandre Derumier (4):
>   qemu_img convert : add external snapshot support
>   blockdev: add backing_chain support
>   qcow2: add external snapshot support
>   tests: fix efi vm-disk-100-0.raw -> vm-100-disk-0.raw
> 
>  src/PVE/QemuConfig.pm                         |   4 +-
>  src/PVE/QemuServer.pm                         | 132 +++++--
>  src/PVE/QemuServer/Blockdev.pm                | 345 +++++++++++++++++-
>  src/PVE/QemuServer/QemuImage.pm               |   6 +-
>  src/test/cfg2cmd/efi-raw-old.conf             |   2 +-
>  src/test/cfg2cmd/efi-raw-old.conf.cmd         |   2 +-
>  src/test/cfg2cmd/efi-raw-template.conf        |   2 +-
>  src/test/cfg2cmd/efi-raw-template.conf.cmd    |   2 +-
>  src/test/cfg2cmd/efi-raw.conf                 |   2 +-
>  src/test/cfg2cmd/efi-raw.conf.cmd             |   2 +-
>  src/test/cfg2cmd/efi-secboot-and-tpm-q35.conf |   2 +-
>  .../cfg2cmd/efi-secboot-and-tpm-q35.conf.cmd  |   2 +-
>  src/test/cfg2cmd/efi-secboot-and-tpm.conf     |   2 +-
>  src/test/cfg2cmd/efi-secboot-and-tpm.conf.cmd |   2 +-
>  src/test/cfg2cmd/sev-es.conf                  |   2 +-
>  src/test/cfg2cmd/sev-es.conf.cmd              |   2 +-
>  src/test/cfg2cmd/sev-std.conf                 |   2 +-
>  src/test/cfg2cmd/sev-std.conf.cmd             |   2 +-
>  src/test/cfg2cmd/simple-backingchain.conf     |  25 ++
>  src/test/cfg2cmd/simple-backingchain.conf.cmd |  33 ++
>  src/test/run_config2command_tests.pl          |  47 +++
>  src/test/run_qemu_img_convert_tests.pl        |  59 +++
>  src/test/snapshot-test.pm                     |   4 +-
>  23 files changed, 634 insertions(+), 49 deletions(-)
>  create mode 100644 src/test/cfg2cmd/simple-backingchain.conf
>  create mode 100644 src/test/cfg2cmd/simple-backingchain.conf.cmd
> 
> 
> -- 
> 2.39.5


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