* [pve-devel] [PATCH storage v5 1/1] import: allow upload of guest images files into import storage
2025-04-01 8:23 [pve-devel] [PATCH storage/manager v5] allow down/upload & import of images in the web UI Dominik Csapak
@ 2025-04-01 8:23 ` Dominik Csapak
2025-04-04 14:11 ` Fiona Ebner
2025-04-01 8:23 ` [pve-devel] [PATCH manager v5 1/3] ui: storage content: allow upload of guest images for import type Dominik Csapak
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Dominik Csapak @ 2025-04-01 8:23 UTC (permalink / raw)
To: pve-devel
so users can upload qcow2/raw/vmdk files directly in the ui
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes in v5
src/PVE/API2/Storage/Status.pm | 17 ++++++++++++++++-
src/PVE/Storage.pm | 3 ++-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index c854b53..b23d283 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -456,6 +456,7 @@ __PACKAGE__->register_method ({
my $path;
my $isOva = 0;
+ my $imageFormat;
if ($content eq 'iso') {
if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
@@ -472,7 +473,12 @@ __PACKAGE__->register_method ({
raise_param_exc({ filename => "invalid filename or wrong extension" });
}
- $isOva = 1;
+ if ($filename =~ m/\.ova$/) {
+ $isOva = 1;
+ } elsif ($filename =~ m/${PVE::Storage::UPLOAD_IMPORT_IMAGE_EXT_RE_1}$/) {
+ $imageFormat = $1;
+ }
+
$path = PVE::Storage::get_import_dir($cfg, $storage);
} else {
raise_param_exc({ content => "upload content type '$content' not allowed" });
@@ -543,6 +549,9 @@ __PACKAGE__->register_method ({
if ($isOva) {
assert_ova_contents($tmpfilename);
+ } elsif (defined($imageFormat)) {
+ # checks untrusted image
+ PVE::Storage::file_size_info($tmpfilename, 10, $imageFormat, 1);
}
};
if (my $err = $@) {
@@ -667,6 +676,7 @@ __PACKAGE__->register_method({
my $path;
my $isOva = 0;
+ my $imageFormat;
if ($content eq 'iso') {
if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
@@ -685,6 +695,8 @@ __PACKAGE__->register_method({
if ($filename =~ m/\.ova$/) {
$isOva = 1;
+ } elsif ($filename =~ m/${PVE::Storage::UPLOAD_IMPORT_IMAGE_EXT_RE_1}$/) {
+ $imageFormat = $1;
}
$path = PVE::Storage::get_import_dir($cfg, $storage);
@@ -717,6 +729,9 @@ __PACKAGE__->register_method({
if ($isOva) {
assert_ova_contents($tmp_path);
+ } elsif (defined($imageFormat)) {
+ # checks untrusted image
+ PVE::Storage::file_size_info($tmp_path, 10, $imageFormat, 1);
}
};
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index c5d4ff8..09d9883 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -116,7 +116,8 @@ our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPR
our $IMPORT_EXT_RE_1 = qr/\.(ova|ovf|qcow2|raw|vmdk)/;
-our $UPLOAD_IMPORT_EXT_RE_1 = qr/\.(ova)/;
+our $UPLOAD_IMPORT_EXT_RE_1 = qr/\.(ova|qcow2|raw|vmdk)/;
+our $UPLOAD_IMPORT_IMAGE_EXT_RE_1 = qr/\.(qcow2|raw|vmdk)/;
our $SAFE_CHAR_CLASS_RE = qr/[a-zA-Z0-9\-\.\+\=\_]/;
our $SAFE_CHAR_WITH_WHITESPACE_CLASS_RE = qr/[ a-zA-Z0-9\-\.\+\=\_]/;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pve-devel] [PATCH storage v5 1/1] import: allow upload of guest images files into import storage
2025-04-01 8:23 ` [pve-devel] [PATCH storage v5 1/1] import: allow upload of guest images files into import storage Dominik Csapak
@ 2025-04-04 14:11 ` Fiona Ebner
2025-04-07 9:44 ` Thomas Lamprecht
0 siblings, 1 reply; 7+ messages in thread
From: Fiona Ebner @ 2025-04-04 14:11 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
Am 01.04.25 um 10:23 schrieb Dominik Csapak:
> so users can upload qcow2/raw/vmdk files directly in the ui
>
Pre-existing, but we put all uploads to /var/tmp/pveupload-XYZ first,
right? This already makes some users unhappy with ISOs IIRC and for
images we can expect it to get worse as those are usually even larger.
Should we at least show a warning/hint about this in the UI?
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> no changes in v5
>
> src/PVE/API2/Storage/Status.pm | 17 ++++++++++++++++-
> src/PVE/Storage.pm | 3 ++-
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
> index c854b53..b23d283 100644
> --- a/src/PVE/API2/Storage/Status.pm
> +++ b/src/PVE/API2/Storage/Status.pm
The API method descriptions don't mention support for
uploading/downloading images yet.
> @@ -456,6 +456,7 @@ __PACKAGE__->register_method ({
>
> my $path;
> my $isOva = 0;
> + my $imageFormat;
Style nit: This is not how we usually name multi-word Perl variables
(also pre-existing for isOva).
>
> if ($content eq 'iso') {
> if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
> @@ -472,7 +473,12 @@ __PACKAGE__->register_method ({
> raise_param_exc({ filename => "invalid filename or wrong extension" });
> }
Nit: if you already extract the extension from matching above here, you
don't need to match again below.
>
> - $isOva = 1;
> + if ($filename =~ m/\.ova$/) {
> + $isOva = 1;
> + } elsif ($filename =~ m/${PVE::Storage::UPLOAD_IMPORT_IMAGE_EXT_RE_1}$/) {
> + $imageFormat = $1;
> + }
> +
> $path = PVE::Storage::get_import_dir($cfg, $storage);
> } else {
> raise_param_exc({ content => "upload content type '$content' not allowed" });
> @@ -543,6 +549,9 @@ __PACKAGE__->register_method ({
>
> if ($isOva) {
> assert_ova_contents($tmpfilename);
> + } elsif (defined($imageFormat)) {
> + # checks untrusted image
> + PVE::Storage::file_size_info($tmpfilename, 10, $imageFormat, 1);
> }
> };
> if (my $err = $@) {
> @@ -667,6 +676,7 @@ __PACKAGE__->register_method({
>
> my $path;
> my $isOva = 0;
> + my $imageFormat;
>
> if ($content eq 'iso') {
> if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
> @@ -685,6 +695,8 @@ __PACKAGE__->register_method({
>
Similar here regarding extension matching, then you don't even need to
define a second regex.
> if ($filename =~ m/\.ova$/) {
> $isOva = 1;
> + } elsif ($filename =~ m/${PVE::Storage::UPLOAD_IMPORT_IMAGE_EXT_RE_1}$/) {
> + $imageFormat = $1;
> }
>
> $path = PVE::Storage::get_import_dir($cfg, $storage);
> @@ -717,6 +729,9 @@ __PACKAGE__->register_method({
>
> if ($isOva) {
> assert_ova_contents($tmp_path);
> + } elsif (defined($imageFormat)) {
> + # checks untrusted image
> + PVE::Storage::file_size_info($tmp_path, 10, $imageFormat, 1);
> }
> };
>
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index c5d4ff8..09d9883 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -116,7 +116,8 @@ our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPR
>
> our $IMPORT_EXT_RE_1 = qr/\.(ova|ovf|qcow2|raw|vmdk)/;
>
> -our $UPLOAD_IMPORT_EXT_RE_1 = qr/\.(ova)/;
> +our $UPLOAD_IMPORT_EXT_RE_1 = qr/\.(ova|qcow2|raw|vmdk)/;
> +our $UPLOAD_IMPORT_IMAGE_EXT_RE_1 = qr/\.(qcow2|raw|vmdk)/;
>
> our $SAFE_CHAR_CLASS_RE = qr/[a-zA-Z0-9\-\.\+\=\_]/;
> our $SAFE_CHAR_WITH_WHITESPACE_CLASS_RE = qr/[ a-zA-Z0-9\-\.\+\=\_]/;
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pve-devel] [PATCH storage v5 1/1] import: allow upload of guest images files into import storage
2025-04-04 14:11 ` Fiona Ebner
@ 2025-04-07 9:44 ` Thomas Lamprecht
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2025-04-07 9:44 UTC (permalink / raw)
To: Proxmox VE development discussion, Fiona Ebner, Dominik Csapak
Am 04.04.25 um 16:11 schrieb Fiona Ebner:
> Am 01.04.25 um 10:23 schrieb Dominik Csapak:
>> so users can upload qcow2/raw/vmdk files directly in the ui
>>
>
> Pre-existing, but we put all uploads to /var/tmp/pveupload-XYZ first,
> right? This already makes some users unhappy with ISOs IIRC and for
> images we can expect it to get worse as those are usually even larger.
> Should we at least show a warning/hint about this in the UI?
This is definitively a long-standing design deficiency and was IMO never
a good choice to use a local directory, duplicating IO and messing with
the root filesystem of a PVE.
That said, it's rather orthogonal to this feature and can be fixed later
on, considering the implications of having some not always fully trusted
fie lying around, handling cleanups, ... it might need a bit more work
to implement some sane and central mechanisms here.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH manager v5 1/3] ui: storage content: allow upload of guest images for import type
2025-04-01 8:23 [pve-devel] [PATCH storage/manager v5] allow down/upload & import of images in the web UI Dominik Csapak
2025-04-01 8:23 ` [pve-devel] [PATCH storage v5 1/1] import: allow upload of guest images files into import storage Dominik Csapak
@ 2025-04-01 8:23 ` Dominik Csapak
2025-04-01 8:23 ` [pve-devel] [PATCH manager v5 2/3] ui: form: file selector: allow optional filter Dominik Csapak
2025-04-01 8:23 ` [pve-devel] [PATCH manager v5 3/3] ui: qemu hd edit: allow importing a disk from the import storage Dominik Csapak
3 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2025-04-01 8:23 UTC (permalink / raw)
To: pve-devel
by allowing 'qcow2', 'vmdk', 'raw' and 'img' files to upload.
Implement 'extension aliases' for the upload window, that append the
'real' extension to the filename after selecting a file with such an
alias. This is necessary so that we have seperate lists for the file
selector and the filename validator. Use that to mark '.img' an alias of
'.raw' for import.
For the download, the api checks the filename for us first, so we
just have to rename '.img' to '.img.raw'.
partially fixes #2424
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v4:
* add generic extension alias mechanism instead of hardcodig behavior
for '.img', and have seperate validation for file selector and the
file name field
www/manager6/window/DownloadUrlToStorage.js | 4 ++++
www/manager6/window/UploadToStorage.js | 24 +++++++++++++++++----
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/www/manager6/window/DownloadUrlToStorage.js b/www/manager6/window/DownloadUrlToStorage.js
index 3ac80ac9..aabc9d3c 100644
--- a/www/manager6/window/DownloadUrlToStorage.js
+++ b/www/manager6/window/DownloadUrlToStorage.js
@@ -89,6 +89,10 @@ Ext.define('PVE.window.DownloadUrlToStorage', {
filename = matches[1];
compression = matches[2].toLowerCase();
}
+ } else if (view.content === 'import') {
+ if (filename.endsWith('.img')) {
+ filename += '.raw';
+ }
}
view.setValues({
diff --git a/www/manager6/window/UploadToStorage.js b/www/manager6/window/UploadToStorage.js
index cdf548a8..f29d80f4 100644
--- a/www/manager6/window/UploadToStorage.js
+++ b/www/manager6/window/UploadToStorage.js
@@ -9,19 +9,28 @@ Ext.define('PVE.window.UploadToStorage', {
title: gettext('Upload'),
acceptedExtensions: {
- 'import': ['.ova'],
+ 'import': ['.ova', '.qcow2', '.raw', '.vmdk'],
iso: ['.img', '.iso'],
vztmpl: ['.tar.gz', '.tar.xz', '.tar.zst'],
},
+ // accepted for file selection, will be renamed to real extension
+ extensionAliases: {
+ 'import': {
+ '.img': '.raw',
+ },
+ },
+
cbindData: function(initialConfig) {
const me = this;
const ext = me.acceptedExtensions[me.content] || [];
me.url = `/nodes/${me.nodename}/storage/${me.storage}/upload`;
+ let fileSelectorExt = ext.concat(Object.keys(me.extensionAliases[me.content] ?? {}));
+
return {
- extensions: ext.join(', '),
+ extensions: fileSelectorExt.join(', '),
filenameRegex: RegExp('^.*(?:' + ext.join('|').replaceAll('.', '\\.') + ')$', 'i'),
};
},
@@ -134,8 +143,15 @@ Ext.define('PVE.window.UploadToStorage', {
},
fileChange: function(input) {
- const vm = this.getViewModel();
- const name = input.value.replace(/^.*(\/|\\)/, '');
+ const me = this;
+ const vm = me.getViewModel();
+ const view = me.getView();
+ let name = input.value.replace(/^.*(\/|\\)/, '');
+ for (const [alias, real] of Object.entries(view.extensionAliases[view.content] ?? {})) {
+ if (name.endsWith(alias)) {
+ name += real;
+ }
+ }
const fileInput = input.fileInputEl.dom;
vm.set('filename', name);
vm.set('size', (fileInput.files[0] && Proxmox.Utils.format_size(fileInput.files[0].size)) || '-');
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* [pve-devel] [PATCH manager v5 2/3] ui: form: file selector: allow optional filter
2025-04-01 8:23 [pve-devel] [PATCH storage/manager v5] allow down/upload & import of images in the web UI Dominik Csapak
2025-04-01 8:23 ` [pve-devel] [PATCH storage v5 1/1] import: allow upload of guest images files into import storage Dominik Csapak
2025-04-01 8:23 ` [pve-devel] [PATCH manager v5 1/3] ui: storage content: allow upload of guest images for import type Dominik Csapak
@ 2025-04-01 8:23 ` Dominik Csapak
2025-04-01 8:23 ` [pve-devel] [PATCH manager v5 3/3] ui: qemu hd edit: allow importing a disk from the import storage Dominik Csapak
3 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2025-04-01 8:23 UTC (permalink / raw)
To: pve-devel
this sometimes comes in handy when we only want to show specific files.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes in v5
www/manager6/form/FileSelector.js | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/www/manager6/form/FileSelector.js b/www/manager6/form/FileSelector.js
index ef2bedf9..9db20711 100644
--- a/www/manager6/form/FileSelector.js
+++ b/www/manager6/form/FileSelector.js
@@ -43,6 +43,13 @@ Ext.define('PVE.form.FileSelector', {
url: url,
});
+ if (Ext.isFunction(me.filter)) {
+ me.store.clearFilter();
+ me.store.addFilter([me.filter]);
+ } else {
+ me.store.clearFilter();
+ }
+
me.store.removeAll();
me.store.load();
},
@@ -60,6 +67,9 @@ Ext.define('PVE.form.FileSelector', {
valueField: 'volid',
displayField: 'text',
+ // An optional filter function
+ filter: undefined,
+
listConfig: {
width: 600,
columns: [
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* [pve-devel] [PATCH manager v5 3/3] ui: qemu hd edit: allow importing a disk from the import storage
2025-04-01 8:23 [pve-devel] [PATCH storage/manager v5] allow down/upload & import of images in the web UI Dominik Csapak
` (2 preceding siblings ...)
2025-04-01 8:23 ` [pve-devel] [PATCH manager v5 2/3] ui: form: file selector: allow optional filter Dominik Csapak
@ 2025-04-01 8:23 ` Dominik Csapak
3 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2025-04-01 8:23 UTC (permalink / raw)
To: pve-devel
adds a checkbox 'import image' above the storage selector which:
* hides the original storage selector
* shows a 'source storage' selector
* shows a 'import file' selector
* shows a 'target storage' selector
Since the wizard and the hd edit share this panel, this also works in
the wizard.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes in v5
www/manager6/qemu/HDEdit.js | 71 ++++++++++++++++++++++++++++++++++++-
1 file changed, 70 insertions(+), 1 deletion(-)
diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
index b78647ec..d6357a91 100644
--- a/www/manager6/qemu/HDEdit.js
+++ b/www/manager6/qemu/HDEdit.js
@@ -78,11 +78,14 @@ Ext.define('PVE.qemu.HDInputPanel', {
if (values.hdimage) {
me.drive.file = values.hdimage;
} else {
- me.drive.file = values.hdstorage + ":" + values.disksize;
+ let disksize = values['import-from'] ? 0 : values.disksize;
+ me.drive.file = `${values.hdstorage}:${disksize}`;
+ PVE.Utils.propertyStringSet(me.drive, values['import-from'], 'import-from');
}
me.drive.format = values.diskformat;
}
+
PVE.Utils.propertyStringSet(me.drive, !values.backup, 'backup', '0');
PVE.Utils.propertyStringSet(me.drive, values.noreplicate, 'replicate', 'no');
PVE.Utils.propertyStringSet(me.drive, values.discard, 'discard', 'on');
@@ -168,6 +171,11 @@ Ext.define('PVE.qemu.HDInputPanel', {
var me = this;
me.down('#hdstorage').setNodename(nodename);
me.down('#hdimage').setStorage(undefined, nodename);
+
+ me.lookup('new-disk').setNodename(nodename);
+ me.lookup('import-source').setNodename(nodename);
+ me.lookup('import-source-file').setNodename(nodename);
+ me.lookup('import-target').setNodename(nodename);
},
hasAdvanced: true,
@@ -221,12 +229,73 @@ Ext.define('PVE.qemu.HDInputPanel', {
column1.push(me.unusedDisks);
} else if (me.isCreate) {
column1.push({
+ xtype: 'proxmoxcheckbox',
+ isFormField: false,
+ fieldLabel: gettext("Import Image"),
+ listeners: {
+ change: function(_cb, value) {
+ me.lookup('new-disk').setVisible(!value);
+ me.lookup('new-disk').setDisabled(!!value);
+
+ let importSource = me.lookup('import-source');
+ importSource.setVisible(!!value);
+ importSource.setDisabled(!value);
+ me.lookup('import-source-file').setVisible(!!value);
+ me.lookup('import-source-file').setDisabled(!value || !importSource.getValue());
+
+ me.lookup('import-target').setVisible(!!value);
+ me.lookup('import-target').setDisabled(!value);
+ },
+ },
+ });
+ column1.push({
+ reference: 'new-disk',
xtype: 'pveDiskStorageSelector',
storageContent: 'images',
name: 'disk',
nodename: me.nodename,
autoSelect: me.insideWizard,
});
+ column1.push({
+ xtype: 'pveStorageSelector',
+ reference: 'import-source',
+ fieldLabel: gettext('Import Storage'),
+ name: 'import-source-storage',
+ hidden: true,
+ disabled: true,
+ storageContent: 'import',
+ nodename: me.nodename,
+ autoSelect: me.insideWizard,
+ listeners: {
+ change: function(selector, storage) {
+ me.lookup('import-source-file').setStorage(storage);
+ me.lookup('import-source-file').setDisabled(!storage || selector.isDisabled());
+ },
+ },
+ });
+ column1.push({
+ xtype: 'pveFileSelector',
+ reference: 'import-source-file',
+ fieldLabel: gettext("Select Image"),
+ hidden: true,
+ disabled: true,
+ storageContent: 'import',
+ name: 'import-from',
+ filter: (rec) => ['qcow2', 'vmdk', 'raw'].indexOf(rec?.data?.format) !== -1,
+ nodename: me.nodename,
+ });
+ column1.push({
+ xtype: 'pveDiskStorageSelector',
+ reference: 'import-target',
+ storageLabel: gettext('Target Storage'),
+ hidden: true,
+ disabled: true,
+ hideSize: true,
+ storageContent: 'images',
+ name: 'target',
+ nodename: me.nodename,
+ autoSelect: me.insideWizard,
+ });
} else {
column1.push({
xtype: 'textfield',
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread