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

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

[Alexandre Derumier via pve-devel]

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

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH-SERIES v3 pve-storage/qemu-server/pve-qemu] add external qcow2 snapshot support
Date: Mon, 16 Dec 2024 10:12:14 +0100
Message-ID: <20241216091229.3142660-1-alexandre.derumier@groupe-cyllene.com>

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 v3:
  storage:
    - snapshots files now have the name of the snapshot, and "current" snapshot is the vm volname
    - allow only qcow2 format for base image for simplication
    - merge snapshot code in lvmplugin (qcow2 format auto enable snapshot)
    - the code is a lot more simple now

  qemu-server:
    - convertion -drive to modern -blockdev
      This is needed for blockdev-reopen, where we need to create && switch to snapshot files with same cache,aio,...
      This is also needed to live rename snapshot files (to keep volname for current snapshot)
      I have implemented && tested:
         - disk create,delete,resize,convert,drive_mirror, hotplug,unplug,nbd mirror,cdrom insert/eject
         - block protocol: file,block_device,rbd,nbd,glusterfs
      Note that it's currently incomplete:
         - proxmox backup/restore code need to be converted to blockdev, help is needed
         - iscsi:// path is not yet implemented (I'll look for the v4)
         - efi still in drive format (not blocking, but I'll look for v4)

      The live migration between -drive ---> --blockdev seem to work without breaking, and seem to be 
      transparent for the guest ok, so I think we could try to target pve9 ?



storage.cfg example:

dir: local2
        path /var/liv/vz
        content snippets,vztmpl,backup,images,iso,rootdir
        snapext 1

lvm:test
        vgname test
        content images





pve-storage:

Alexandre Derumier (3):
  qcow2: add external snapshot support
  lvmplugin: add qcow2 snapshot
  storage: vdisk_free: remove external snapshots

 src/PVE/Storage.pm                 |  18 ++-
 src/PVE/Storage/DirPlugin.pm       |   1 +
 src/PVE/Storage/LVMPlugin.pm       | 231 ++++++++++++++++++++++++++---
 src/PVE/Storage/Plugin.pm          | 207 ++++++++++++++++++++++----
 src/test/run_test_zfspoolplugin.pl |  18 +++
 5 files changed, 424 insertions(+), 51 deletions(-)


Alexandre Derumier (11):
  blockdev: cmdline: convert drive to blockdev syntax
  blockdev: fix cfg2cmd tests
  blockdev : convert qemu_driveadd && qemu_drivedel
  blockdev: vm_devices_list : fix block-query
  blockdev: convert cdrom media eject/insert
  blockdev: block_resize: convert to blockdev
  blockdev: nbd_export: block-export-add : use drive-$id for nodename
  blockdev: convert drive_mirror to blockdev_mirror
  blockdev: mirror: change aio on target if io_uring is not default.
  blockdev: add backing_chain support
  qcow2: add external snapshot support

 PVE/QemuConfig.pm                             |   4 +-
 PVE/QemuMigrate.pm                            |   2 +-
 PVE/QemuServer.pm                             | 946 ++++++++++++++----
 test/cfg2cmd/bootorder-empty.conf.cmd         |  12 +-
 test/cfg2cmd/bootorder-legacy.conf.cmd        |  12 +-
 test/cfg2cmd/bootorder.conf.cmd               |  12 +-
 ...putype-icelake-client-deprecation.conf.cmd |   6 +-
 test/cfg2cmd/ide.conf.cmd                     |  23 +-
 test/cfg2cmd/pinned-version-pxe-pve.conf.cmd  |   6 +-
 test/cfg2cmd/pinned-version-pxe.conf.cmd      |   6 +-
 test/cfg2cmd/pinned-version.conf.cmd          |   6 +-
 test/cfg2cmd/q35-ide.conf.cmd                 |  23 +-
 .../q35-linux-hostpci-template.conf.cmd       |   3 +-
 test/cfg2cmd/seabios_serial.conf.cmd          |   6 +-
 ...imple-balloon-free-page-reporting.conf.cmd |   6 +-
 test/cfg2cmd/simple-btrfs.conf.cmd            |   6 +-
 test/cfg2cmd/simple-virtio-blk.conf.cmd       |   6 +-
 test/cfg2cmd/simple1-template.conf.cmd        |  11 +-
 test/cfg2cmd/simple1.conf.cmd                 |   6 +-
 19 files changed, 830 insertions(+), 272 deletions(-)


pve-qemu:

  add block-commit-replaces option patch

 ...052-block-commit-add-replaces-option.patch | 137 ++++++++++++++++++
 debian/patches/series                         |   1 +
 2 files changed, 138 insertions(+)
 create mode 100644 debian/patches/pve/0052-block-commit-add-replaces-option.patch

-- 

2.39.5



[-- 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] [PATCH-SERIES v3 pve-storage/qemu-server/pve-qemu] add external qcow2 snapshot support

[Fabian Grünbichler]

> Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com> hat am 16.12.2024 10:12 CET 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.

a few high level remarks:
- I am not sure whether we want to/can switch over to blockdev on the fly (i.e., without some sort of opt-in phase to iron out kinks). what about upgrades with running VMs? I guess some sort of flag and per-VM switching would be a better plan..
- I am also not sure whether we want to mix and match internal and external snapshots, we probably should require a storage to use it or not at creation time and fix it, to allow us to keep the handling code from exploding complexity-wise..
- if you see a way to name the block graph nodes in a deterministic fashion (i.e., have a 1:1 mapping between snapshot and block graph node name) that would be wonderful, else we'd have to find another way to improve the lookup there..
- the relevant commits and code would really benefit from a summary of the design/semantics, it's not easy to reconstruct this from the code alone
- some tests would be great as well, both to verify that the code actually behaves like we expect, and also to catch regressions when it is touched again in the future
- some performance numbers would also be great :)
-- internal vs external snapshots using the same underlying dir/fs
-- qcow2 vs qcow2 with snapshots vs raw on LVM using the same underlying disk
- did you verify the space consumption on the SAN side with the LVM plugin? would be interesting to know how it holds up in practice :)

I haven't done any real world testing yet since I expect that the delta for the next version won't be small either ;)

> 
> Changelog v3:
>   storage:
>     - snapshots files now have the name of the snapshot, and "current" snapshot is the vm volname
>     - allow only qcow2 format for base image for simplication
>     - merge snapshot code in lvmplugin (qcow2 format auto enable snapshot)
>     - the code is a lot more simple now
> 
>   qemu-server:
>     - convertion -drive to modern -blockdev
>       This is needed for blockdev-reopen, where we need to create && switch to snapshot files with same cache,aio,...
>       This is also needed to live rename snapshot files (to keep volname for current snapshot)
>       I have implemented && tested:
>          - disk create,delete,resize,convert,drive_mirror, hotplug,unplug,nbd mirror,cdrom insert/eject
>          - block protocol: file,block_device,rbd,nbd,glusterfs
>       Note that it's currently incomplete:
>          - proxmox backup/restore code need to be converted to blockdev, help is needed
>          - iscsi:// path is not yet implemented (I'll look for the v4)
>          - efi still in drive format (not blocking, but I'll look for v4)
> 
>       The live migration between -drive ---> --blockdev seem to work without breaking, and seem to be 
>       transparent for the guest ok, so I think we could try to target pve9 ?
> 
> 
> 
> storage.cfg example:
> 
> dir: local2
>         path /var/liv/vz
>         content snippets,vztmpl,backup,images,iso,rootdir
>         snapext 1
> 
> lvm:test
>         vgname test
>         content images
> 
> 
> 
> 
> 
> pve-storage:
> 
> Alexandre Derumier (3):
>   qcow2: add external snapshot support
>   lvmplugin: add qcow2 snapshot
>   storage: vdisk_free: remove external snapshots
> 
>  src/PVE/Storage.pm                 |  18 ++-
>  src/PVE/Storage/DirPlugin.pm       |   1 +
>  src/PVE/Storage/LVMPlugin.pm       | 231 ++++++++++++++++++++++++++---
>  src/PVE/Storage/Plugin.pm          | 207 ++++++++++++++++++++++----
>  src/test/run_test_zfspoolplugin.pl |  18 +++
>  5 files changed, 424 insertions(+), 51 deletions(-)
> 
> 
> Alexandre Derumier (11):
>   blockdev: cmdline: convert drive to blockdev syntax
>   blockdev: fix cfg2cmd tests
>   blockdev : convert qemu_driveadd && qemu_drivedel
>   blockdev: vm_devices_list : fix block-query
>   blockdev: convert cdrom media eject/insert
>   blockdev: block_resize: convert to blockdev
>   blockdev: nbd_export: block-export-add : use drive-$id for nodename
>   blockdev: convert drive_mirror to blockdev_mirror
>   blockdev: mirror: change aio on target if io_uring is not default.
>   blockdev: add backing_chain support
>   qcow2: add external snapshot support
> 
>  PVE/QemuConfig.pm                             |   4 +-
>  PVE/QemuMigrate.pm                            |   2 +-
>  PVE/QemuServer.pm                             | 946 ++++++++++++++----
>  test/cfg2cmd/bootorder-empty.conf.cmd         |  12 +-
>  test/cfg2cmd/bootorder-legacy.conf.cmd        |  12 +-
>  test/cfg2cmd/bootorder.conf.cmd               |  12 +-
>  ...putype-icelake-client-deprecation.conf.cmd |   6 +-
>  test/cfg2cmd/ide.conf.cmd                     |  23 +-
>  test/cfg2cmd/pinned-version-pxe-pve.conf.cmd  |   6 +-
>  test/cfg2cmd/pinned-version-pxe.conf.cmd      |   6 +-
>  test/cfg2cmd/pinned-version.conf.cmd          |   6 +-
>  test/cfg2cmd/q35-ide.conf.cmd                 |  23 +-
>  .../q35-linux-hostpci-template.conf.cmd       |   3 +-
>  test/cfg2cmd/seabios_serial.conf.cmd          |   6 +-
>  ...imple-balloon-free-page-reporting.conf.cmd |   6 +-
>  test/cfg2cmd/simple-btrfs.conf.cmd            |   6 +-
>  test/cfg2cmd/simple-virtio-blk.conf.cmd       |   6 +-
>  test/cfg2cmd/simple1-template.conf.cmd        |  11 +-
>  test/cfg2cmd/simple1.conf.cmd                 |   6 +-
>  19 files changed, 830 insertions(+), 272 deletions(-)
> 
> 
> pve-qemu:
> 
>   add block-commit-replaces option patch
> 
>  ...052-block-commit-add-replaces-option.patch | 137 ++++++++++++++++++
>  debian/patches/series                         |   1 +
>  2 files changed, 138 insertions(+)
>  create mode 100644 debian/patches/pve/0052-block-commit-add-replaces-option.patch
> 
> -- 
> 
> 2.39.5


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

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

[DERUMIER, Alexandre via pve-devel]

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.gruenbichler@proxmox.com" <f.gruenbichler@proxmox.com>
Subject: Re: [pve-devel] [PATCH-SERIES v3 pve-storage/qemu-server/pve-qemu] add external qcow2 snapshot support
Date: Fri, 10 Jan 2025 07:44:34 +0000
Message-ID: <9dee14737f1c78c247486e00a20fa95c420cb366.camel@groupe-cyllene.com>

-------- Message initial --------
De: Fabian Grünbichler <f.gruenbichler@proxmox.com>
À: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Objet: Re: [pve-devel] [PATCH-SERIES v3 pve-storage/qemu-server/pve-
qemu] add external qcow2 snapshot support
Date: 09/01/2025 15:13:14

> Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com> hat am
> 16.12.2024 10:12 CET 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.
>>
>>a few high level remarks:
>>- I am not sure whether we want to/can switch over to blockdev on the
>>fly (i.e., without some sort of opt-in phase to iron out kinks). what
>>about upgrades with running VMs? I guess some sort of flag and per-VM
>>switching would be a better plan..

I have tested live migration, and it's works for the small tests I have
done. (I was surprised myself). I'll try to do more longer test to be
100% sure that they are not corruption of datas.

 on the guest side, it's transparent. on qemu side, the devices and pci
plumbing is still the same, and qemu already use blockdev behind.

If needed, we could make a switch based on qemu version, or or manual
option.


>>- I am also not sure whether we want to mix and match internal and
>>external snapshots, we probably should require a storage to use it or
>>not at creation time and fix it, to allow us to keep the handling
>>code from exploding complexity-wise..

for qcow2, I have a "snapext" option on the storage, so you can't mix
internal/external at the same time.  
(We just need to allow to define the snapext option at storage create
only)

For lvm, only external snapshot are possible. (if format is qcow2)



>>- if you see a way to name the block graph nodes in a deterministic
>>fashion (i.e., have a 1:1 mapping between snapshot and block graph
>>node name) that would be wonderful, else we'd have to find another
>>way to improve the lookup there..

1:1 mapping with snapshot is not possible (I have tried it a lot),
because:
  - snapshot name can be too long (blockdev name is 31 characters max,
hash based on filename is difficult)
  - with external snapshot file renaming, this don't work  (snap-->
current). We can't rename a blockdev, so the mapping will drift.

  So, I don't think that it's possible to avoid lookup. (I really don't
have idea how to manage it).  
I'm not sure it's really a problem ?  it's just an extra qmp call, but
it's super fast.


>>- the relevant commits and code would really benefit from a summary
>>of the design/semantics, it's not easy to reconstruct this from the
>>code alone

ok will do

>>- some tests would be great as well, both to verify that the code
>>actually behaves like we expect, and also to catch regressions when
>>it is touched again in the future

yes, I was planning to add test, as we don't have too much tests on
qemu command line currently, this is a good time to add them.


>>- some performance numbers would also be great :)
>>-- internal vs external snapshots using the same underlying dir/fs
>>-- qcow2 vs qcow2 with snapshots vs raw on LVM using the same
>>underlying disk

I'll prepare different hardware san to compare.  (Note that it's not
only performance, but disk lock/freeze on snapshot rollback for example
with internal snapshot).

I'll try to rebench again ocfs2 && gfs2 too with external snapshot (as
metadatas preallocation is really important with them, and it's not
possible with internal snap)



>>- did you verify the space consumption on the SAN side with the LVM
>>plugin? would be interesting to know how it holds up in practice :)
Well, currently, it's reserving a full lvm volume for each snapshot.

If your storage support thin provisioning, its not too much a problem,
you can create a lun bigger than your physical storage. I have tried
with a netapp san, it's works.

but for low cost san, we thin provisioning not exist, I'm planning to
add dynamic extend fo small lvm.

I have send some test patch in september 2024,  qcow2 size can be
bigger than lvm size. (like qcow2=100G && lvm=1G), then it's possible
to send event with qemu on threshold usage and extend (through a custom
daemon) the lvm volume.
But I was planning to add this in a separated after that the external
snapshot code is commited.

>>I haven't done any real world testing yet since I expect that the
>>delta for the next version won't be small either ;)

Yes, the biggest part is done. (I Hope ^_^ )

I'll try to finish the blockdev convertion of some missing part (iscsi
path, uefi ,...).
The main blocking point for me is the proxmox backup code, I really
don't known how it's works.


> 
> Changelog v3:
>   storage:
>     - snapshots files now have the name of the snapshot, and
> "current" snapshot is the vm volname
>     - allow only qcow2 format for base image for simplication
>     - merge snapshot code in lvmplugin (qcow2 format auto enable
> snapshot)
>     - the code is a lot more simple now
> 
>   qemu-server:
>     - convertion -drive to modern -blockdev
>       This is needed for blockdev-reopen, where we need to create &&
> switch to snapshot files with same cache,aio,...
>       This is also needed to live rename snapshot files (to keep
> volname for current snapshot)
>       I have implemented && tested:
>          - disk create,delete,resize,convert,drive_mirror,
> hotplug,unplug,nbd mirror,cdrom insert/eject
>          - block protocol: file,block_device,rbd,nbd,glusterfs
>       Note that it's currently incomplete:
>          - proxmox backup/restore code need to be converted to
> blockdev, help is needed
>          - iscsi:// path is not yet implemented (I'll look for the
> v4)
>          - efi still in drive format (not blocking, but I'll look for
> v4)
> 
>       The live migration between -drive ---> --blockdev seem to work
> without breaking, and seem to be 
>       transparent for the guest ok, so I think we could try to target
> pve9 ?
> 
> 
> 
> storage.cfg example:
> 
> dir: local2
>         path /var/liv/vz
>         content snippets,vztmpl,backup,images,iso,rootdir
>         snapext 1
> 
> lvm:test
>         vgname test
>         content images
> 
> 
> 
> 
> 
> pve-storage:
> 
> Alexandre Derumier (3):
>   qcow2: add external snapshot support
>   lvmplugin: add qcow2 snapshot
>   storage: vdisk_free: remove external snapshots
> 
>  src/PVE/Storage.pm                 |  18 ++-
>  src/PVE/Storage/DirPlugin.pm       |   1 +
>  src/PVE/Storage/LVMPlugin.pm       | 231 ++++++++++++++++++++++++++-
> --
>  src/PVE/Storage/Plugin.pm          | 207 ++++++++++++++++++++++----
>  src/test/run_test_zfspoolplugin.pl |  18 +++
>  5 files changed, 424 insertions(+), 51 deletions(-)
> 
> 
> Alexandre Derumier (11):
>   blockdev: cmdline: convert drive to blockdev syntax
>   blockdev: fix cfg2cmd tests
>   blockdev : convert qemu_driveadd && qemu_drivedel
>   blockdev: vm_devices_list : fix block-query
>   blockdev: convert cdrom media eject/insert
>   blockdev: block_resize: convert to blockdev
>   blockdev: nbd_export: block-export-add : use drive-$id for nodename
>   blockdev: convert drive_mirror to blockdev_mirror
>   blockdev: mirror: change aio on target if io_uring is not default.
>   blockdev: add backing_chain support
>   qcow2: add external snapshot support
> 
>  PVE/QemuConfig.pm                             |   4 +-
>  PVE/QemuMigrate.pm                            |   2 +-
>  PVE/QemuServer.pm                             | 946 ++++++++++++++--
> --
>  test/cfg2cmd/bootorder-empty.conf.cmd         |  12 +-
>  test/cfg2cmd/bootorder-legacy.conf.cmd        |  12 +-
>  test/cfg2cmd/bootorder.conf.cmd               |  12 +-
>  ...putype-icelake-client-deprecation.conf.cmd |   6 +-
>  test/cfg2cmd/ide.conf.cmd                     |  23 +-
>  test/cfg2cmd/pinned-version-pxe-pve.conf.cmd  |   6 +-
>  test/cfg2cmd/pinned-version-pxe.conf.cmd      |   6 +-
>  test/cfg2cmd/pinned-version.conf.cmd          |   6 +-
>  test/cfg2cmd/q35-ide.conf.cmd                 |  23 +-
>  .../q35-linux-hostpci-template.conf.cmd       |   3 +-
>  test/cfg2cmd/seabios_serial.conf.cmd          |   6 +-
>  ...imple-balloon-free-page-reporting.conf.cmd |   6 +-
>  test/cfg2cmd/simple-btrfs.conf.cmd            |   6 +-
>  test/cfg2cmd/simple-virtio-blk.conf.cmd       |   6 +-
>  test/cfg2cmd/simple1-template.conf.cmd        |  11 +-
>  test/cfg2cmd/simple1.conf.cmd                 |   6 +-
>  19 files changed, 830 insertions(+), 272 deletions(-)
> 
> 
> pve-qemu:
> 
>   add block-commit-replaces option patch
> 
>  ...052-block-commit-add-replaces-option.patch | 137
> ++++++++++++++++++
>  debian/patches/series                         |   1 +
>  2 files changed, 138 insertions(+)
>  create mode 100644 debian/patches/pve/0052-block-commit-add-
> replaces-option.patch
> 
> -- 
> 
> 2.39.5



[-- 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] [PATCH-SERIES v3 pve-storage/qemu-server/pve-qemu] add external qcow2 snapshot support

[Fiona Ebner]

Am 10.01.25 um 08:44 schrieb DERUMIER, Alexandre via pve-devel:
> -------- Message initial --------
> De: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> À: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
> Cc: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
> Objet: Re: [pve-devel] [PATCH-SERIES v3 pve-storage/qemu-server/pve-
> qemu] add external qcow2 snapshot support
> Date: 09/01/2025 15:13:14
> 
>> Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com> hat am
>> 16.12.2024 10:12 CET 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.
>>>
>>> a few high level remarks:
>>> - I am not sure whether we want to/can switch over to blockdev on the
>>> fly (i.e., without some sort of opt-in phase to iron out kinks). what
>>> about upgrades with running VMs? I guess some sort of flag and per-VM
>>> switching would be a better plan..
> 
> I have tested live migration, and it's works for the small tests I have
> done. (I was surprised myself). I'll try to do more longer test to be
> 100% sure that they are not corruption of datas.
> 
>  on the guest side, it's transparent. on qemu side, the devices and pci
> plumbing is still the same, and qemu already use blockdev behind.
> 
> If needed, we could make a switch based on qemu version, or or manual
> option.

Yes, we need to be very careful that all the defaults/behavior would be
the same to not break live-migration. A known difference is format
autodetection, which happens with "-drive file=" but not with
"-blockdev", but not relevant as we explicitly set the format. Dumping
the QObject JSON configs of the drives might be a good heuristic to
check that the properties really are the same before and after the switch.

Switching based on QEMU version would need to be the creation QEMU from
the meta config property. Using machine or running binary version would
mean we would automatically switch for non-Windows OSes which are not
version pinned by default, so that doesn't help if there would be
breakage. I'd really hope it is compatible, because for a per-VM flag,
for backwards-compat reasons (e.g. rolling back to a snapshot with
VMstate) it would need to start out as being off by default.

We wouldn't even need to switch to using '-blockdev' right now (still
good thing to do long-term wise, but if it is opt-in, we can't rely on
all VMs having it, which is bad), you could also set the node-name for
the '-drive'. I.e. switching to '-blockdev' can be done separately to
switching to 'blockdev-*' QMP operations.

>>> - if you see a way to name the block graph nodes in a deterministic
>>> fashion (i.e., have a 1:1 mapping between snapshot and block graph
>>> node name) that would be wonderful, else we'd have to find another
>>> way to improve the lookup there..
> 
> 1:1 mapping with snapshot is not possible (I have tried it a lot),
> because:
>   - snapshot name can be too long (blockdev name is 31 characters max,
> hash based on filename is difficult)
>   - with external snapshot file renaming, this don't work  (snap-->
> current). We can't rename a blockdev, so the mapping will drift.
> 
>   So, I don't think that it's possible to avoid lookup. (I really don't
> have idea how to manage it).  
> I'm not sure it's really a problem ?  it's just an extra qmp call, but
> it's super fast.

Are we sure the node-name for the drive is always stable? I.e. is the
block node that the guest sees inserted in the drive, always the one
named by the 'node-name' that was initially set when attaching the drive
via '-blockdev' or QMP 'blockdev-add'? After all kinds of block
operations? Even if there are partially completed/failed block
operations? After live migration from a not-yet-updated node? Otherwise,
I'd prefer always querying the node-name before doing a QMP 'blockdev-*'
command to make sure it's actually the node that the guest sees as well,
like we currently do for 'block-export-add'. And we wouldn't even need
to set the node-names ourselves at all if always querying first. Seems a
bit more future-proof as well.


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

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

[DERUMIER, Alexandre via pve-devel]

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>, "f.gruenbichler@proxmox.com" <f.gruenbichler@proxmox.com>
Subject: Re: [pve-devel] [PATCH-SERIES v3 pve-storage/qemu-server/pve-qemu] add external qcow2 snapshot support
Date: Fri, 10 Jan 2025 12:30:22 +0000
Message-ID: <8f309dfe189379acf72db07398a37a98e8fc3550.camel@groupe-cyllene.com>

-------- Message initial --------
De: Fiona Ebner <f.ebner@proxmox.com>
À: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
f.gruenbichler@proxmox.com <f.gruenbichler@proxmox.com>
Cc: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
Objet: Re: [pve-devel] [PATCH-SERIES v3 pve-storage/qemu-server/pve-
qemu] add external qcow2 snapshot support
Date: 10/01/2025 10:55:14

Am 10.01.25 um 08:44 schrieb DERUMIER, Alexandre via pve-devel:
> -------- Message initial --------
> De: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> À: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
> Cc: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
> Objet: Re: [pve-devel] [PATCH-SERIES v3 pve-storage/qemu-server/pve-
> qemu] add external qcow2 snapshot support
> Date: 09/01/2025 15:13:14
> 
> > Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com> hat
> > am
> > 16.12.2024 10:12 CET 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.
> > > 
> > > a few high level remarks:
> > > - I am not sure whether we want to/can switch over to blockdev on
> > > the
> > > fly (i.e., without some sort of opt-in phase to iron out kinks).
> > > what
> > > about upgrades with running VMs? I guess some sort of flag and
> > > per-VM
> > > switching would be a better plan..
> 
> I have tested live migration, and it's works for the small tests I
> have
> done. (I was surprised myself). I'll try to do more longer test to be
> 100% sure that they are not corruption of datas.
> 
>  on the guest side, it's transparent. on qemu side, the devices and
> pci
> plumbing is still the same, and qemu already use blockdev behind.
> 
> If needed, we could make a switch based on qemu version, or or manual
> option.

>>Yes, we need to be very careful that all the defaults/behavior would
be
>>the same to not break live-migration. A known difference is format
>>autodetection, which happens with "-drive file=" but not with
>>"-blockdev", but not relevant as we explicitly set the format.
>>Dumping
>>the QObject JSON configs of the drives might be a good heuristic to
>>check that the properties really are the same before and after the
>>switch.
I had looked manually at qdev info, and dumped the blockdevs generated
by -drive command to compare, I didn't see difference (only the node
names and the additional throttle group node)

>>Switching based on QEMU version would need to be the creation QEMU
>>from
>>the meta config property. Using machine or running binary version
>>would
>>mean we would automatically switch for non-Windows OSes which are not
>>version pinned by default, so that doesn't help if there would be
>>breakage.

That's why I was thinking to implement this for pve9. (based on qemu
version)

>>I'd really hope it is compatible, because for a per-VM flag,
>>for backwards-compat reasons (e.g. rolling back to a snapshot with
>>VMstate) it would need to start out as being off by default.

I think that a vmstate is not a problem, because this is only the guest
memory right ? and devices are not changing.


>>We wouldn't even need to switch to using '-blockdev' right now (still
>>good thing to do long-term wise, but if it is opt-in, we can't rely
>>on
>>all VMs having it, which is bad), 
>>you could also set the node-name for the '-drive'. 

Are you sure about this ? I don't have seen any documentation about
adding the node-name to drive.   (and we need it for hotplug hmp
drive_add too :/ )

not even sure this could define specific name to the file nodename,
which is needed for the snapshot renaming with blockdev-reopen.


>>I.e. switching to '-blockdev' can be done separately to
>>switching to 'blockdev-*' QMP operations.


I really don't known if you can implement qmp blockdev-*, with --drive
syntax (where it could be possible to define nodename).

I known that qmp blockdev-* command accept "device" (for --drive) or
"node-name" (for --blockdev)


(BTW,switching to -blockdev is already breaking qmp proxmox backup ^_^
possibly because of the throttle-group top node, I don't remember
exactly).



I'll take time to retest  live migration with differents os, restore 
snapshot with state, and see if I have crash or silent data
corruptions.


> > > - if you see a way to name the block graph nodes in a
> > > deterministic
> > > fashion (i.e., have a 1:1 mapping between snapshot and block
> > > graph
> > > node name) that would be wonderful, else we'd have to find
> > > another
> > > way to improve the lookup there..
> 
> 1:1 mapping with snapshot is not possible (I have tried it a lot),
> because:
>   - snapshot name can be too long (blockdev name is 31 characters
> max,
> hash based on filename is difficult)
>   - with external snapshot file renaming, this don't work  (snap-->
> current). We can't rename a blockdev, so the mapping will drift.
> 
>   So, I don't think that it's possible to avoid lookup. (I really
> don't
> have idea how to manage it).  
> I'm not sure it's really a problem ?  it's just an extra qmp call,
> but
> it's super fast.

>>Are we sure the node-name for the drive is always stable? I.e. is the
>>block node that the guest sees inserted in the drive, always the one
>>named by the 'node-name' that was initially set when attaching the
>>drive
>>via '-blockdev' or QMP 'blockdev-add'? After all kinds of block
>>operations? Even if there are partially completed/failed block
>>operations? After live migration from a not-yet-updated node?
>>Otherwise,

No, the only stable nodename for me (in my implementation) is the top
throttle-group node. as it never change during mirroring, snaphot
rename,...

The drive node (format-node or file-node), can change (2 file-node for
live file renaming with blockdev-reopen for example , 2 format-node
switching after a mirror, ...)


>>I'd prefer always querying the node-name before doing a QMP
>>'blockdev-*'
>>command to make sure it's actually the node that the guest sees as
>>well,
>>like we currently do for 'block-export-add'. 
That's the way I have done it

>>And we wouldn't even >>need
>>to set the node-names ourselves at all if always querying first.
>>Seems a
>>bit more future-proof as well.

blockdev-reopen don't work with autogenerated nodenames (block#<id>)
(not sure if it's a bug or not).
That's why I'm currently naming all of them (including backing chain
snapshots too)








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