Re: [pve-devel] [PATCH pve-storage 2/5] qcow2: add external snapshot support

["DERUMIER, Alexandre via pve-devel" <pve-devel@lists.proxmox.com>]

[-- Attachment #1: Type: message/rfc822, Size: 14184 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 pve-storage 2/5] qcow2: add external snapshot support
Date: Mon, 19 May 2025 13:01:41 +0000
Message-ID: <c5c69c923d03a512b85067473c1f65f4eefb9a0d.camel@groupe-cyllene.com>

> + #if first snapshot,as it should be bigger,  we merge child, and
> rename the snapshot to child
> + if(!$parentsnap) {
> +     print "commit: merge content of $childpath into $snappath\n";
> +     $cmd = ['/usr/bin/qemu-img', 'commit', $childpath];
> +     eval { run_command($cmd) };
> +     if ($@) {

>>should we add an error here about what state this leaves the snapshot
>>in?
>>AFAIU we've potentially written some of the data from $child to
>>$snap, so
>>the state of $snap is technically invalid now?

yes, if you try to clone or rollback it, the content is invalid.
the current is still working, so impact to run the vm, but the
user really need to delete the snapshot later.

(I'm not sure if you should have some kind of snapshot state in vm
config, to avoid to call rollback,clone if state is invalid? as the vm
is locked anyway, maybe some logs is enough ?)

> +     $cmd = ['/usr/bin/qemu-img', 'rebase', '-b', $parentpath, '-F',
> 'qcow2', '-f', 'qcow2', $childpath];
> +     eval { run_command($cmd) };
> +     if ($@) {
> + die "error rebase $childpath from $parentpath; $@\n";

>>same here, but in this case $child just contains some duplicate data
>>so nothing is
>>really broken?

yes, no problem here.



> +     }
> +     #delete the snapshot
> +     eval { $class->free_image($storeid, $scfg, $snap_volname, 0);
> };
> +     if ($@) {

>>and here we just leave a stray volume around that is not part of the
>>backing chain
>>anymore, right?

yes, the end of rebase is switching the backing_file info in the qcow2
metadatas.


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