From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>,
"pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH v3 pve-storage 1/3] qcow2: add external snapshot support
Date: Fri, 10 Jan 2025 13:20:11 +0100 (CET) [thread overview]
Message-ID: <1161584966.1974.1736511611934@webmail.proxmox.com> (raw)
In-Reply-To: <1caecaa23e5d57030a9e31f2f0e67648f1930d6a.camel@groupe-cyllene.com>
> DERUMIER, Alexandre <alexandre.derumier@groupe-cyllene.com> hat am 10.01.2025 12:51 CET geschrieben:
> > > + } else {
> > > + print"commit $snappath\n";
> > > + $cmd = ['/usr/bin/qemu-img', 'commit', $snappath];
> >
> > > > leftover from previous version? not used/overwritten below ;)
> >
> > no, this is really to commit the the snapshot to parent
>
> >>but it is not executed..
>
> Ah, ok ! sorrry ! I think I have dropped some code during rebase before
> sending patches, because I had tested it a lot of time !
>
>
>
> > this is for this usecase :
> >
> > A<----B<----C.
> >
> > you commit B to A, then you need to change the backing file of C to
> > A
> > (instead B)
> >
> > A<----C
>
> >>but this is the wrong semantics.. the writes/delta in B need to go to
> >>C (they happened after A), not to A!
>
> I think they can go to A (commit) or C (stream)
>
> here an example:
>
> current (1TB)
> - take snap A
>
> (A (1TB)<------new current 500MB (backing file A))
>
> - take snap B
>
> (A (1TB)<------B 500MB (backingfile A)<------new current 10MB
> (backingfile B))
>
>
> Then, you want to delete B.
>
>
> so, you stream it to current. (so copy 500MB to current in this
> example)
>
> Then, you want to delete snapshot A
> you don't want stream A to current, because A is the big initial image.
> So, instead, you need to commit the current to A (with the extra 500MB)
>
>
> So, if you have a lot of snapshot to delete, you are going do a copy
> same datas each time to the upper snapshot for nothing, because at the
> end we are going to commit to the initial "first" snapshot/image.
but you don't know up front that you want to collapse all the snapshots. for each single removal, you have to merge the delta towards the overlay, not the base, else the base contents is no longer matching its name.
think about it this way:
you take a snapshot B at time X. this snapshot must never contain a modification that happened after X. that means you cannot ever commit a newer snapshot into B, unless you are removing and renaming B.
if you start with a chain A -> B -> C -> D (with A being the first snapshot/base, and D being the current active overlay. if you want to remove B, you can either
- stream B into C, remove B
- commit C into B, remove C, rename B to C
in both cases you will end up with a chain A -> C' -> D where C' is the combination of the old B and C.
the downside of the streaming variant is that if B's delta is bigger than C's, you have more I/O. the upside is that there is no inbetween state where the backing chain is broken and error handling can go very wrong.
what you are doing right now is:
chain A->B->C->D as before. remove B by commiting B into A and then rebasing C on top of A. that means you end up with:
A'->C->D where A' is A+B. but now this snapshot A contains writes that happened after the original A was taken. this is wrong.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2025-01-10 12:21 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20241216091229.3142660-1-alexandre.derumier@groupe-cyllene.com>
2024-12-16 9:12 ` [pve-devel] [PATCH v1 pve-qemu 1/1] add block-commit-replaces option patch Alexandre Derumier via pve-devel
2025-01-08 13:27 ` Fabian Grünbichler
2025-01-10 7:55 ` DERUMIER, Alexandre via pve-devel
[not found] ` <34a164520eba035d1db5f70761b0f4aa59fecfa5.camel@groupe-cyllene.com>
2025-01-10 9:15 ` Fiona Ebner
2025-01-10 9:32 ` DERUMIER, Alexandre via pve-devel
2024-12-16 9:12 ` [pve-devel] [PATCH v3 qemu-server 01/11] blockdev: cmdline: convert drive to blockdev syntax Alexandre Derumier via pve-devel
2025-01-08 14:17 ` Fabian Grünbichler
2025-01-10 13:50 ` DERUMIER, Alexandre via pve-devel
2024-12-16 9:12 ` [pve-devel] [PATCH v3 pve-storage 1/3] qcow2: add external snapshot support Alexandre Derumier via pve-devel
2025-01-09 12:36 ` Fabian Grünbichler
2025-01-10 9:10 ` DERUMIER, Alexandre via pve-devel
[not found] ` <f25028d41a9588e82889b3ef869a14f33cbd216e.camel@groupe-cyllene.com>
2025-01-10 11:02 ` Fabian Grünbichler
2025-01-10 11:51 ` DERUMIER, Alexandre via pve-devel
[not found] ` <1caecaa23e5d57030a9e31f2f0e67648f1930d6a.camel@groupe-cyllene.com>
2025-01-10 12:20 ` Fabian Grünbichler [this message]
2025-01-10 13:14 ` DERUMIER, Alexandre via pve-devel
2024-12-16 9:12 ` [pve-devel] [PATCH v3 qemu-server 02/11] blockdev: fix cfg2cmd tests Alexandre Derumier via pve-devel
2024-12-16 9:12 ` [pve-devel] [PATCH v3 pve-storage 2/3] lvmplugin: add qcow2 snapshot Alexandre Derumier via pve-devel
2025-01-09 13:55 ` Fabian Grünbichler
2025-01-10 10:16 ` DERUMIER, Alexandre via pve-devel
2024-12-16 9:12 ` [pve-devel] [PATCH v3 qemu-server 03/11] blockdev : convert qemu_driveadd && qemu_drivedel Alexandre Derumier via pve-devel
2025-01-08 14:26 ` Fabian Grünbichler
2025-01-10 14:08 ` DERUMIER, Alexandre via pve-devel
2024-12-16 9:12 ` [pve-devel] [PATCH v3 pve-storage 3/3] storage: vdisk_free: remove external snapshots Alexandre Derumier via pve-devel
2024-12-16 9:12 ` [pve-devel] [PATCH v3 qemu-server 04/11] blockdev: vm_devices_list : fix block-query Alexandre Derumier via pve-devel
2025-01-08 14:31 ` Fabian Grünbichler
2024-12-16 9:12 ` [pve-devel] [PATCH v3 qemu-server 05/11] blockdev: convert cdrom media eject/insert Alexandre Derumier via pve-devel
2025-01-08 14:34 ` Fabian Grünbichler
2024-12-16 9:12 ` [pve-devel] [PATCH v3 qemu-server 06/11] blockdev: block_resize: convert to blockdev Alexandre Derumier via pve-devel
2024-12-16 9:12 ` [pve-devel] [PATCH v3 qemu-server 07/11] blockdev: nbd_export: block-export-add : use drive-$id for nodename Alexandre Derumier via pve-devel
2024-12-16 9:12 ` [pve-devel] [PATCH v3 qemu-server 08/11] blockdev: convert drive_mirror to blockdev_mirror Alexandre Derumier via pve-devel
2025-01-08 15:19 ` Fabian Grünbichler
2024-12-16 9:12 ` [pve-devel] [PATCH v3 qemu-server 09/11] blockdev: mirror: change aio on target if io_uring is not default Alexandre Derumier via pve-devel
2025-01-09 9:51 ` Fabian Grünbichler
2024-12-16 9:12 ` [pve-devel] [PATCH v3 qemu-server 10/11] blockdev: add backing_chain support Alexandre Derumier via pve-devel
2025-01-09 11:57 ` Fabian Grünbichler
2024-12-16 9:12 ` [pve-devel] [PATCH v3 qemu-server 11/11] qcow2: add external snapshot support Alexandre Derumier via pve-devel
2025-01-09 11:57 ` Fabian Grünbichler
2025-01-09 13:19 ` Fabio Fantoni via pve-devel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1161584966.1974.1736511611934@webmail.proxmox.com \
--to=f.gruenbichler@proxmox.com \
--cc=alexandre.derumier@groupe-cyllene.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox