Am 19.07.21 um 16:52 schrieb Aaron Lauterer:
We do not expose the parameter 'add_fmt_suffix' used by the internal
implemantion of 'find_free_diskname'. This is something only the plugins
themselves know but cannot be determined easily and reliably from an
outside caller.
This is why the new 'wants_fmt_suffix' method has been introduced. For
most plugins the return value is very clear. For the default
implementation in Plugin.pm we add another check to be on the safe side
and only return true if the '$scfg->{path}' option is present.
It indicates that the volume in question is an actual file which will
need the suffix.
Signed-off-by: Aaron Lauterer <a.laute...@proxmox.com>
--- > rfc -> v1:
dropped $add_fmt_suffix parameter and added the "wants_fmt_suffix"
helper method in each plugin.
PVE/Storage.pm | 11 +++++++++++
PVE/Storage/LVMPlugin.pm | 5 +++++
PVE/Storage/Plugin.pm | 7 +++++++
PVE/Storage/RBDPlugin.pm | 5 +++++
PVE/Storage/ZFSPoolPlugin.pm | 5 +++++
5 files changed, 33 insertions(+)
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index c04b5a2..afeb2e3 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -203,6 +203,17 @@ sub storage_can_replicate {
return $plugin->storage_can_replicate($scfg, $storeid, $format);
}
+sub find_free_diskname {
+ my ($cfg, $storeid, $vmid, $fmt) = @_;
+
+ my $scfg = storage_config($cfg, $storeid);
+ my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+
+ my $add_fmt_suffix = $plugin->wants_fmt_suffix($scfg);
+
+ return $plugin->find_free_diskname($storeid, $scfg, $vmid, $fmt,
$add_fmt_suffix);
Maybe this should rather be find_free_volname and always return the full
volname (i.e. "<VMID>/vm-<VMID>-disk-<N>.<FMT>" for dir-based storages)
so that it can be parsed with parse_volname later?
That would mean something like wants_vmid_prefix() is needed, but I'd
rather extend (and rename) the wants_fmt_suffix method instead.
The rationale is: the value returned here is used for the
$target_volname parameter in the next patch, and in case it's a
dir-based storage, passing in an actual volname like
'123/vm-123-disk-0.raw' currently fails (because it actually expects
only the disk name).
+}
+
sub storage_ids {
my ($cfg) = @_;
diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
index 139d391..3e5b6c8 100644
--- a/PVE/Storage/LVMPlugin.pm
+++ b/PVE/Storage/LVMPlugin.pm
@@ -201,6 +201,11 @@ sub type {
return 'lvm';
}
+sub wants_fmt_suffix {
+ my ($class, $scfg) = @_;
+ return 0;
+}
+
Nit: since there is no $scfg->{path}, the default implementation would
already return 0. Same for {RBD,ZFSPool}Plugin below, but overwriting
the method doesn't really hurt either of course.
sub plugindata {
return {
content => [ {images => 1, rootdir => 1}, { images => 1 }],
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index b1865cb..5c6c659 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -191,6 +191,13 @@ sub default_format {
return wantarray ? ($def_format, $valid_formats) : $def_format;
}
+sub wants_fmt_suffix {
+ my ($class, $scfg) = @_;
+ return 1 if $scfg->{path};
+ return 0;
+}
+
+
PVE::JSONSchema::register_format('pve-storage-path', \&verify_path);
sub verify_path {
my ($path, $noerr) = @_;
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index a8d1243..86ea45a 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -273,6 +273,11 @@ sub type {
return 'rbd';
}
+sub wants_fmt_suffix {
+ my ($class, $scfg) = @_;
+ return 0;
+}
+
sub plugindata {
return {
content => [ {images => 1, rootdir => 1}, { images => 1 }],
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index c4be70f..85e2211 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -18,6 +18,11 @@ sub type {
return 'zfspool';
}
+sub wants_fmt_suffix {
+ my ($class, $scfg) = @_;
+ return 0;
+}
+
sub plugindata {
return {
content => [ {images => 1, rootdir => 1}, {images => 1 , rootdir => 1}],
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
