On 4/22/24 13:34, Fiona Ebner wrote:
Am 22.04.24 um 13:09 schrieb Dominik Csapak:
On 4/22/24 13:00, Fiona Ebner wrote:
Am 19.04.24 um 11:45 schrieb Dominik Csapak:
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 22a9729..39a8354 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -654,6 +654,10 @@ sub parse_volname {
return ('backup', $fn);
} elsif ($volname =~ m!^snippets/([^/]+)$!) {
return ('snippets', $1);
+ } elsif ($volname =~
m!^import/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$!)
{
+ return ('import', $1);
Wouldn't it be nicer to return 'ovf' and 'ova' as the $file_format here
and check for that at the call sites? Currently you rely on the presence
or absence of $file_format in copy_needs_extraction() and
get_import_metadata() and then re-match on the ova extension. Having the
format right away would be a bit cleaner and more future-proof or is
there a specific reason against doing it?
i explained it in either the cover letter or in some commit message,
probably would have fit
in here too:
we currently only support raw/vmdk/qcow2/subvol here and it is intended
only for image formats
IIUC.
Hmm, maybe we can lift that limitation? Needs a bit of consideration of
course, but I don't see why the format returned by parse_volname()
should be limited to images, since there already is a vtype to
distinguish. And your series already uses the format for 'import' vtype
too ;)
Also we would have to adapt the `verify_format` for that, since it
will be
tested by that at least once. (not sure where exactly though, found out
by testing)
and that would mean we could set it as 'default' format on the storage,
which is not what we want...
We shouldn't allow that of course, but I don't see verify_format()
called other than for the "pve-storage-format" schema format, so maybe
some other check that fails? Or some use site of "pve-storage-format"
that should first check that it's an image? And I guess we should rename
it to "pve-storage-image-format", because it's used as that to avoid
future confusion.
i have to apologize, it seems my previous message was wrong.
after re-checking and re-testing a variant i had before changing it
to not returning any format for ova/ovfs it seems to work fine.
not sure where the error i got came from (maybe i added some extra check and
forgot about/missed it?)
anyway, yes, i can send a v3 where i return ova/ovf as format for those
files, and even use 'ova+vmdk' etc. for the files contained in ovas
which makes checking for extraction a bit better
does sound ok for you?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
