* [pve-devel] [PATCH qemu-server] drive: volume in-use check: fix fallback path comparison
@ 2021-04-15 10:10 Fabian Ebner
2021-04-18 15:30 ` Thomas Lamprecht
2021-04-18 16:00 ` [pve-devel] applied: " Thomas Lamprecht
0 siblings, 2 replies; 3+ messages in thread
From: Fabian Ebner @ 2021-04-15 10:10 UTC (permalink / raw)
To: pve-devel
When checking whether a volume is still referenced by a snapshot, the volid
itself is first checked. When the volid is different, we fall back to comparing
the path.
As the first value to be compared is a volume's path, the second value better be
a volume's path too, and not a snapshot's path.
See also 77019edfe0c190c949cdc0b0e3b4ad2ca37313b3 for historical context.
The error that led me here:
* had a VM with ZFS over iSCSI storage with an exsiting snapshot
* add new unused drive
* try to remove the unsued drive
* fails, because ZFS (not Pool!) Plugin does not support snapshot paths.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
PVE/QemuServer/Drive.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index 01ea8d7..9016a43 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -590,7 +590,7 @@ sub is_volume_in_use {
next if !$storeid;
my $scfg = PVE::Storage::storage_config($storecfg, $storeid, 1);
next if !$scfg;
- return 1 if $path eq PVE::Storage::path($storecfg, $drive->{file}, $snapname);
+ return 1 if $path eq PVE::Storage::path($storecfg, $drive->{file});
}
}
}
--
2.20.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] drive: volume in-use check: fix fallback path comparison
2021-04-15 10:10 [pve-devel] [PATCH qemu-server] drive: volume in-use check: fix fallback path comparison Fabian Ebner
@ 2021-04-18 15:30 ` Thomas Lamprecht
2021-04-18 16:00 ` [pve-devel] applied: " Thomas Lamprecht
1 sibling, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2021-04-18 15:30 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Ebner
On 15.04.21 12:10, Fabian Ebner wrote:
> When checking whether a volume is still referenced by a snapshot, the volid
> itself is first checked. When the volid is different, we fall back to comparing
> the path.
>
> As the first value to be compared is a volume's path, the second value better be
> a volume's path too, and not a snapshot's path.
>
> See also 77019edfe0c190c949cdc0b0e3b4ad2ca37313b3 for historical context.
>
> The error that led me here:
> * had a VM with ZFS over iSCSI storage with an exsiting snapshot
> * add new unused drive
> * try to remove the unsued drive
> * fails, because ZFS (not Pool!) Plugin does not support snapshot paths.
>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> PVE/QemuServer/Drive.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
> index 01ea8d7..9016a43 100644
> --- a/PVE/QemuServer/Drive.pm
> +++ b/PVE/QemuServer/Drive.pm
> @@ -590,7 +590,7 @@ sub is_volume_in_use {
> next if !$storeid;
> my $scfg = PVE::Storage::storage_config($storecfg, $storeid, 1);
> next if !$scfg;
> - return 1 if $path eq PVE::Storage::path($storecfg, $drive->{file}, $snapname);
> + return 1 if $path eq PVE::Storage::path($storecfg, $drive->{file});
makes the whole $snapname param of that closure unused, which seems OK as $cref
holds the actual info, FWICT after a quick look.
So, if that patch is sound it should probably remove the $snapname from $scan_config
closure too.
> }
> }
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pve-devel] applied: [PATCH qemu-server] drive: volume in-use check: fix fallback path comparison
2021-04-15 10:10 [pve-devel] [PATCH qemu-server] drive: volume in-use check: fix fallback path comparison Fabian Ebner
2021-04-18 15:30 ` Thomas Lamprecht
@ 2021-04-18 16:00 ` Thomas Lamprecht
1 sibling, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2021-04-18 16:00 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Ebner
On 15.04.21 12:10, Fabian Ebner wrote:
> When checking whether a volume is still referenced by a snapshot, the void
> itself is first checked. When the volid is different, we fall back to comparing
> the path.
>
> As the first value to be compared is a volume's path, the second value better be
> a volume's path too, and not a snapshot's path.
>
> See also 77019edfe0c190c949cdc0b0e3b4ad2ca37313b3 for historical context.
>
> The error that led me here:
> * had a VM with ZFS over iSCSI storage with an exsiting snapshot
> * add new unused drive
> * try to remove the unsued drive
> * fails, because ZFS (not Pool!) Plugin does not support snapshot paths.
>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> PVE/QemuServer/Drive.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
applied, thanks! But see my other mail to this patch for a potential follow-up
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-18 16:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-15 10:10 [pve-devel] [PATCH qemu-server] drive: volume in-use check: fix fallback path comparison Fabian Ebner
2021-04-18 15:30 ` Thomas Lamprecht
2021-04-18 16:00 ` [pve-devel] applied: " Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox