[pve-devel] [PATCH storage/manager v5] allow down/upload & import of images in the web UI

[pve-devel] [PATCH storage/manager v5] allow down/upload & import of images in the web UI

[Dominik Csapak]

most of the building blocks are already there:
* we can have image files in an import storage
* we can import image files via the api from such a storage

this series fills in the missing bits & pieces:
* allow uploading/downloading image files into an import storage via the webgui
* adding the possibility to select such a file when creating a vm/disk

One minor "ugliness": when switching between import on/off, the target
storage selector resets. This is normally intended by the component,
since it's most often only disabled when it's still visible, except here
in this case.

If this is a blocker, I can of course add an option to the selector
to not do this here, but IMHO this is a rather odd use case anyway,
so I opted for not handling that explicitely.

changes from v4:
* add an 'extension alias' mechanism for the upload window,
  to have separate extensions to validate for the file selector
  and the file name field (thanks for noticing @Filip)

changes from v3:
* allow raw (.raw, .img) and vmdk files too
* automatically append '.raw' for '.img' files

changes from v2:
* fix correctly unset 'import-from' in wizard when going to summary,
  back to disk, unselecting import, then going forward to the summary
  again
* fixed an issue with the file selector being mistakenly disabled

changes from v1:
* fixed an issue where the file selector would be hidden but invalid

pve-storage:

Dominik Csapak (1):
  import: allow upload of guest images files into import storage

 src/PVE/API2/Storage/Status.pm | 17 ++++++++++++++++-
 src/PVE/Storage.pm             |  3 ++-
 2 files changed, 18 insertions(+), 2 deletions(-)

pve-manager:

Dominik Csapak (3):
  ui: storage content: allow upload of guest images for import type
  ui: form: file selector: allow optional filter
  ui: qemu hd edit: allow importing a disk from the import storage

 www/manager6/form/FileSelector.js           | 10 +++
 www/manager6/qemu/HDEdit.js                 | 71 ++++++++++++++++++++-
 www/manager6/window/DownloadUrlToStorage.js |  4 ++
 www/manager6/window/UploadToStorage.js      | 24 +++++--
 4 files changed, 104 insertions(+), 5 deletions(-)

-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH storage v5 1/1] import: allow upload of guest images files into import storage

[Dominik Csapak]

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

[pve-devel] [PATCH manager v5 1/3] ui: storage content: allow upload of guest images for import type

[Dominik Csapak]

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

[pve-devel] [PATCH manager v5 2/3] ui: form: file selector: allow optional filter

[Dominik Csapak]

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

[pve-devel] [PATCH manager v5 3/3] ui: qemu hd edit: allow importing a disk from the import storage

[Dominik Csapak]

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

Re: [pve-devel] [PATCH storage v5 1/1] import: allow upload of guest images files into import storage

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

> 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

Re: [pve-devel] [PATCH storage v5 1/1] import: allow upload of guest images files into import storage

[Thomas Lamprecht]

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