[pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Dominik Csapak]

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

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

We could maybe also allow this for raw/vmdk if we want to, but IMHO
we can start out with qcow2 and add the others as necssary.

(if wanted, I can of course also add the others in a next version or as
a follow up)

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 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 qcow2 files into import storage

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

pve-manager:

Dominik Csapak (3):
  ui: storage content: allow upload of qcow2 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/UploadToStorage.js |  2 +-
 3 files changed, 81 insertions(+), 2 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 v3 1/1] import: allow upload of qcow2 files into import storage

[Dominik Csapak]

so users can upload qcow2 files directly in the ui

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes in v3
 src/PVE/API2/Storage/Status.pm | 17 ++++++++++++++++-
 src/PVE/Storage.pm             |  2 +-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index c854b53..b253392 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 $isQcow2 = 0;
 
 	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/\.qcow2$/) {
+		$isQcow2 = 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 ($isQcow2) {
+		    # checks untrusted image
+		    PVE::Storage::file_size_info($tmpfilename, 10, 'qcow2', 1);
 		}
 	    };
 	    if (my $err = $@) {
@@ -667,6 +676,7 @@ __PACKAGE__->register_method({
 
 	my $path;
 	my $isOva = 0;
+	my $isQcow2 = 0;
 
 	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/\.qcow2$/) {
+		$isQcow2 = 1;
 	    }
 
 	    $path = PVE::Storage::get_import_dir($cfg, $storage);
@@ -717,6 +729,9 @@ __PACKAGE__->register_method({
 
 	    if ($isOva) {
 		assert_ova_contents($tmp_path);
+	    } elsif ($isQcow2) {
+		# checks untrusted image
+		PVE::Storage::file_size_info($tmp_path, 10, 'qcow2', 1);
 	    }
 	};
 
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 3b4f041..deed73f 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -116,7 +116,7 @@ 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)/;
 
 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 v3 1/3] ui: storage content: allow upload of qcow2 for import type

[Dominik Csapak]

partially fixes #2424

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes in v3
 www/manager6/window/UploadToStorage.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/window/UploadToStorage.js b/www/manager6/window/UploadToStorage.js
index cdf548a8..3ce2d1f5 100644
--- a/www/manager6/window/UploadToStorage.js
+++ b/www/manager6/window/UploadToStorage.js
@@ -9,7 +9,7 @@ Ext.define('PVE.window.UploadToStorage', {
     title: gettext('Upload'),
 
     acceptedExtensions: {
-	'import': ['.ova'],
+	'import': ['.ova', '.qcow2'],
 	iso: ['.img', '.iso'],
 	vztmpl: ['.tar.gz', '.tar.xz', '.tar.zst'],
     },
-- 
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 v3 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 v3
 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 v3 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>
---
changes in v3:
* fix correctly unset 'import-from' in wizard when going to summary,
  back to disk, unselecting import, then going forward to the summary
  again
* correctly enable/disable the file selector when import checkbox changes

 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..e9a1d4a8 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) => rec?.data?.format === 'qcow2',
+		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/manager v3] allow upload & import of qcow2 in the web UI

[Filip Schauer]

Tested the following scenarios:
* Uploading a qcow2 image via the Web UI works fine.
* Uploading a file that is not in the correct format fails with "Image
   is not in qcow2 format".
* Adding a qcow2 image to an existing VM via the Web UI works fine.
* Creating a VM with an imported qcow2 disk via the wizard works fine.

The minor "ugliness" that the storage selector resets when the "Import
Image" checkbox is toggled is not a big problem in my opinion.

Overall this looks fine.

Tested-by: Filip Schauer <f.schauer@proxmox.com>

On 25/03/2025 16:14, Dominik Csapak wrote:
> most of the building blocks are already there:
> * we can have qcow2 files in an import storage
> * we can import qcow2 files via the api from such a storage
>
> this series fills in the missing bits & pieces:
> * allow uploading qcow2 files into an import storage via the webgui
> * adding the possibility to select such a file when creating a vm/disk
>
> We could maybe also allow this for raw/vmdk if we want to, but IMHO
> we can start out with qcow2 and add the others as necssary.
>
> (if wanted, I can of course also add the others in a next version or as
> a follow up)
>
> 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 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 qcow2 files into import storage
>
>   src/PVE/API2/Storage/Status.pm | 17 ++++++++++++++++-
>   src/PVE/Storage.pm             |  2 +-
>   2 files changed, 17 insertions(+), 2 deletions(-)
>
> pve-manager:
>
> Dominik Csapak (3):
>    ui: storage content: allow upload of qcow2 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/UploadToStorage.js |  2 +-
>   3 files changed, 81 insertions(+), 2 deletions(-)
>


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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Fiona Ebner]

Am 25.03.25 um 16:14 schrieb Dominik Csapak:
> most of the building blocks are already there:
> * we can have qcow2 files in an import storage
> * we can import qcow2 files via the api from such a storage
> 
> this series fills in the missing bits & pieces:
> * allow uploading qcow2 files into an import storage via the webgui
> * adding the possibility to select such a file when creating a vm/disk
> 
> We could maybe also allow this for raw/vmdk if we want to, but IMHO
> we can start out with qcow2 and add the others as necssary.
> 
> (if wanted, I can of course also add the others in a next version or as
> a follow up)


Yes, please! It would be nice to have all three at the same time. Or is
there any specific reason why you limit it to qcow2? Otherwise, users
will just ask why support for these is missing right away.


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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Dominik Csapak]

On 3/26/25 11:37, Fiona Ebner wrote:
> Am 25.03.25 um 16:14 schrieb Dominik Csapak:
>> most of the building blocks are already there:
>> * we can have qcow2 files in an import storage
>> * we can import qcow2 files via the api from such a storage
>>
>> this series fills in the missing bits & pieces:
>> * allow uploading qcow2 files into an import storage via the webgui
>> * adding the possibility to select such a file when creating a vm/disk
>>
>> We could maybe also allow this for raw/vmdk if we want to, but IMHO
>> we can start out with qcow2 and add the others as necssary.
>>
>> (if wanted, I can of course also add the others in a next version or as
>> a follow up)
> 
> 
> Yes, please! It would be nice to have all three at the same time. Or is
> there any specific reason why you limit it to qcow2? Otherwise, users
> will just ask why support for these is missing right away.

No specific reason, it was just easier/quicker to implement one first. When we also allow raw files,
should we also allow other extensions than '.raw'? not sure if there is one that
is often used (since I think '.raw' is more a PVE thing)


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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Thomas Lamprecht]

Am 26.03.25 um 11:37 schrieb Fiona Ebner:
> Am 25.03.25 um 16:14 schrieb Dominik Csapak:
>> We could maybe also allow this for raw/vmdk if we want to, but IMHO
>> we can start out with qcow2 and add the others as necssary.
>>
>> (if wanted, I can of course also add the others in a next version or as
>> a follow up)
> 
> 
> Yes, please! It would be nice to have all three at the same time. Or is
> there any specific reason why you limit it to qcow2? Otherwise, users
> will just ask why support for these is missing right away.

+1, I would also expect that it's not much extra work and that the lack
for the others would cause some noise in forum/etc., so if those have
no big blocker I'd also vmdk; for raw I question what extension to use,
or is .img or .raw widespread enough?


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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Fiona Ebner]

Am 26.03.25 um 11:47 schrieb Dominik Csapak:
> On 3/26/25 11:37, Fiona Ebner wrote:
>> Am 25.03.25 um 16:14 schrieb Dominik Csapak:
>>> most of the building blocks are already there:
>>> * we can have qcow2 files in an import storage
>>> * we can import qcow2 files via the api from such a storage
>>>
>>> this series fills in the missing bits & pieces:
>>> * allow uploading qcow2 files into an import storage via the webgui
>>> * adding the possibility to select such a file when creating a vm/disk
>>>
>>> We could maybe also allow this for raw/vmdk if we want to, but IMHO
>>> we can start out with qcow2 and add the others as necssary.
>>>
>>> (if wanted, I can of course also add the others in a next version or as
>>> a follow up)
>>
>>
>> Yes, please! It would be nice to have all three at the same time. Or is
>> there any specific reason why you limit it to qcow2? Otherwise, users
>> will just ask why support for these is missing right away.
> 
> No specific reason, it was just easier/quicker to implement one first.
> When we also allow raw files,
> should we also allow other extensions than '.raw'? not sure if there is
> one that
> is often used (since I think '.raw' is more a PVE thing)
> 

Right, raw is actually a bit of a headache because of that :P

We could either:

1) have a list of common extensions for raw: .raw/.img/etc

1b) also treat files without extension as raw?

2) have a list of known extensions that are not raw and treat everything
else as raw, while logging an informational message

I'd prefer 1), because we already require specific extensions for other
uploads.

And likely we want to rename after/during upload, so images that are raw
for us always have a ".raw" extension? Otherwise, we need to be careful
enough to enforce the very same rules when parsing the import volume
name and thus mostly also have them set in stone for the future. The
advantage of the latter would be for the use case where one wants to
manually make accessible their already existing image folders without
using the API.



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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Dominik Csapak]

On 3/26/25 12:41, Fiona Ebner wrote:
> Am 26.03.25 um 11:47 schrieb Dominik Csapak:
>> On 3/26/25 11:37, Fiona Ebner wrote:
>>> Am 25.03.25 um 16:14 schrieb Dominik Csapak:
>>>> most of the building blocks are already there:
>>>> * we can have qcow2 files in an import storage
>>>> * we can import qcow2 files via the api from such a storage
>>>>
>>>> this series fills in the missing bits & pieces:
>>>> * allow uploading qcow2 files into an import storage via the webgui
>>>> * adding the possibility to select such a file when creating a vm/disk
>>>>
>>>> We could maybe also allow this for raw/vmdk if we want to, but IMHO
>>>> we can start out with qcow2 and add the others as necssary.
>>>>
>>>> (if wanted, I can of course also add the others in a next version or as
>>>> a follow up)
>>>
>>>
>>> Yes, please! It would be nice to have all three at the same time. Or is
>>> there any specific reason why you limit it to qcow2? Otherwise, users
>>> will just ask why support for these is missing right away.
>>
>> No specific reason, it was just easier/quicker to implement one first.
>> When we also allow raw files,
>> should we also allow other extensions than '.raw'? not sure if there is
>> one that
>> is often used (since I think '.raw' is more a PVE thing)
>>
> 
> Right, raw is actually a bit of a headache because of that :P
> 
> We could either:
> 
> 1) have a list of common extensions for raw: .raw/.img/etc
> 
> 1b) also treat files without extension as raw?
> 
> 2) have a list of known extensions that are not raw and treat everything
> else as raw, while logging an informational message
> 
> I'd prefer 1), because we already require specific extensions for other
> uploads.
> 
> And likely we want to rename after/during upload, so images that are raw
> for us always have a ".raw" extension? Otherwise, we need to be careful
> enough to enforce the very same rules when parsing the import volume
> name and thus mostly also have them set in stone for the future. The
> advantage of the latter would be for the use case where one wants to
> manually make accessible their already existing image folders without
> using the API.
> 

I'd also use a list (e.g. for now '.raw', '.img')

renaming is a good idea, but how should we do that? e.g.

foo.img -> foo.img.raw ?

because if we'd do foo.img -> foo.raw i think it's more likely
to get a collision than when we keep the .img in the name

what do you think?

Side note (can be done later; or not) do we want to support compressed files?
(gz, xz, etc.?) just noticed that e.g. the home assistant disk image is a
qcow2.xz file


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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Dominik Csapak]

On 3/26/25 12:41, Fiona Ebner wrote:
> Am 26.03.25 um 11:47 schrieb Dominik Csapak:
>> On 3/26/25 11:37, Fiona Ebner wrote:
>>> Am 25.03.25 um 16:14 schrieb Dominik Csapak:
>>>> most of the building blocks are already there:
>>>> * we can have qcow2 files in an import storage
>>>> * we can import qcow2 files via the api from such a storage
>>>>
>>>> this series fills in the missing bits & pieces:
>>>> * allow uploading qcow2 files into an import storage via the webgui
>>>> * adding the possibility to select such a file when creating a vm/disk
>>>>
>>>> We could maybe also allow this for raw/vmdk if we want to, but IMHO
>>>> we can start out with qcow2 and add the others as necssary.
>>>>
>>>> (if wanted, I can of course also add the others in a next version or as
>>>> a follow up)
>>>
>>>
>>> Yes, please! It would be nice to have all three at the same time. Or is
>>> there any specific reason why you limit it to qcow2? Otherwise, users
>>> will just ask why support for these is missing right away.
>>
>> No specific reason, it was just easier/quicker to implement one first.
>> When we also allow raw files,
>> should we also allow other extensions than '.raw'? not sure if there is
>> one that
>> is often used (since I think '.raw' is more a PVE thing)
>>
> 
> Right, raw is actually a bit of a headache because of that :P
> 
> We could either:
> 
> 1) have a list of common extensions for raw: .raw/.img/etc
> 
> 1b) also treat files without extension as raw?
> 
> 2) have a list of known extensions that are not raw and treat everything
> else as raw, while logging an informational message
> 
> I'd prefer 1), because we already require specific extensions for other
> uploads.
> 
> And likely we want to rename after/during upload, so images that are raw
> for us always have a ".raw" extension? Otherwise, we need to be careful
> enough to enforce the very same rules when parsing the import volume
> name and thus mostly also have them set in stone for the future. The
> advantage of the latter would be for the use case where one wants to
> manually make accessible their already existing image folders without
> using the API.
> 

actually thinking of renaming, i don't think that's necessary to do in the backend at all
since the client will provide a target filename, we can just rename it in the ui
to '.raw' for the user?

then we'd also not have to have a list of 'raw' formats on the backend at all?



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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Fiona Ebner]

Am 26.03.25 um 12:47 schrieb Dominik Csapak:
> On 3/26/25 12:41, Fiona Ebner wrote:
>> Am 26.03.25 um 11:47 schrieb Dominik Csapak:
>>> On 3/26/25 11:37, Fiona Ebner wrote:
>>>> Am 25.03.25 um 16:14 schrieb Dominik Csapak:
>>>>> most of the building blocks are already there:
>>>>> * we can have qcow2 files in an import storage
>>>>> * we can import qcow2 files via the api from such a storage
>>>>>
>>>>> this series fills in the missing bits & pieces:
>>>>> * allow uploading qcow2 files into an import storage via the webgui
>>>>> * adding the possibility to select such a file when creating a vm/disk
>>>>>
>>>>> We could maybe also allow this for raw/vmdk if we want to, but IMHO
>>>>> we can start out with qcow2 and add the others as necssary.
>>>>>
>>>>> (if wanted, I can of course also add the others in a next version
>>>>> or as
>>>>> a follow up)
>>>>
>>>>
>>>> Yes, please! It would be nice to have all three at the same time. Or is
>>>> there any specific reason why you limit it to qcow2? Otherwise, users
>>>> will just ask why support for these is missing right away.
>>>
>>> No specific reason, it was just easier/quicker to implement one first.
>>> When we also allow raw files,
>>> should we also allow other extensions than '.raw'? not sure if there is
>>> one that
>>> is often used (since I think '.raw' is more a PVE thing)
>>>
>>
>> Right, raw is actually a bit of a headache because of that :P
>>
>> We could either:
>>
>> 1) have a list of common extensions for raw: .raw/.img/etc
>>
>> 1b) also treat files without extension as raw?
>>
>> 2) have a list of known extensions that are not raw and treat everything
>> else as raw, while logging an informational message
>>
>> I'd prefer 1), because we already require specific extensions for other
>> uploads.
>>
>> And likely we want to rename after/during upload, so images that are raw
>> for us always have a ".raw" extension? Otherwise, we need to be careful
>> enough to enforce the very same rules when parsing the import volume
>> name and thus mostly also have them set in stone for the future. The
>> advantage of the latter would be for the use case where one wants to
>> manually make accessible their already existing image folders without
>> using the API.
>>
> 
> I'd also use a list (e.g. for now '.raw', '.img')
> 
> renaming is a good idea, but how should we do that? e.g.
> 
> foo.img -> foo.img.raw ?
> 
> because if we'd do foo.img -> foo.raw i think it's more likely
> to get a collision than when we keep the .img in the name
> 
> what do you think?

Yes, IMHO, simply attaching .raw is better.

> Side note (can be done later; or not) do we want to support compressed
> files?
> (gz, xz, etc.?) just noticed that e.g. the home assistant disk image is a
> qcow2.xz file

Yeah, I think it makes sense. We'll just need to adapt the "needs
extraction" logic?


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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Fiona Ebner]

Am 26.03.25 um 12:57 schrieb Dominik Csapak:
> On 3/26/25 12:41, Fiona Ebner wrote:
>> Am 26.03.25 um 11:47 schrieb Dominik Csapak:
>>> On 3/26/25 11:37, Fiona Ebner wrote:
>>>> Am 25.03.25 um 16:14 schrieb Dominik Csapak:
>>>>> most of the building blocks are already there:
>>>>> * we can have qcow2 files in an import storage
>>>>> * we can import qcow2 files via the api from such a storage
>>>>>
>>>>> this series fills in the missing bits & pieces:
>>>>> * allow uploading qcow2 files into an import storage via the webgui
>>>>> * adding the possibility to select such a file when creating a vm/disk
>>>>>
>>>>> We could maybe also allow this for raw/vmdk if we want to, but IMHO
>>>>> we can start out with qcow2 and add the others as necssary.
>>>>>
>>>>> (if wanted, I can of course also add the others in a next version
>>>>> or as
>>>>> a follow up)
>>>>
>>>>
>>>> Yes, please! It would be nice to have all three at the same time. Or is
>>>> there any specific reason why you limit it to qcow2? Otherwise, users
>>>> will just ask why support for these is missing right away.
>>>
>>> No specific reason, it was just easier/quicker to implement one first.
>>> When we also allow raw files,
>>> should we also allow other extensions than '.raw'? not sure if there is
>>> one that
>>> is often used (since I think '.raw' is more a PVE thing)
>>>
>>
>> Right, raw is actually a bit of a headache because of that :P
>>
>> We could either:
>>
>> 1) have a list of common extensions for raw: .raw/.img/etc
>>
>> 1b) also treat files without extension as raw?
>>
>> 2) have a list of known extensions that are not raw and treat everything
>> else as raw, while logging an informational message
>>
>> I'd prefer 1), because we already require specific extensions for other
>> uploads.
>>
>> And likely we want to rename after/during upload, so images that are raw
>> for us always have a ".raw" extension? Otherwise, we need to be careful
>> enough to enforce the very same rules when parsing the import volume
>> name and thus mostly also have them set in stone for the future. The
>> advantage of the latter would be for the use case where one wants to
>> manually make accessible their already existing image folders without
>> using the API.
>>
> 
> actually thinking of renaming, i don't think that's necessary to do in
> the backend at all
> since the client will provide a target filename, we can just rename it
> in the ui
> to '.raw' for the user?
> 
> then we'd also not have to have a list of 'raw' formats on the backend
> at all?

Sounds good to me :)


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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Dominik Csapak]

On 3/26/25 13:05, Fiona Ebner wrote:
> 
> 
> Am 26.03.25 um 12:47 schrieb Dominik Csapak:
>> On 3/26/25 12:41, Fiona Ebner wrote:
>>> Am 26.03.25 um 11:47 schrieb Dominik Csapak:
>>>> On 3/26/25 11:37, Fiona Ebner wrote:
>>>>> Am 25.03.25 um 16:14 schrieb Dominik Csapak:
>>>>>> most of the building blocks are already there:
>>>>>> * we can have qcow2 files in an import storage
>>>>>> * we can import qcow2 files via the api from such a storage
>>>>>>
>>>>>> this series fills in the missing bits & pieces:
>>>>>> * allow uploading qcow2 files into an import storage via the webgui
>>>>>> * adding the possibility to select such a file when creating a vm/disk
>>>>>>
>>>>>> We could maybe also allow this for raw/vmdk if we want to, but IMHO
>>>>>> we can start out with qcow2 and add the others as necssary.
>>>>>>
>>>>>> (if wanted, I can of course also add the others in a next version
>>>>>> or as
>>>>>> a follow up)
>>>>>
>>>>>
>>>>> Yes, please! It would be nice to have all three at the same time. Or is
>>>>> there any specific reason why you limit it to qcow2? Otherwise, users
>>>>> will just ask why support for these is missing right away.
>>>>
>>>> No specific reason, it was just easier/quicker to implement one first.
>>>> When we also allow raw files,
>>>> should we also allow other extensions than '.raw'? not sure if there is
>>>> one that
>>>> is often used (since I think '.raw' is more a PVE thing)
>>>>
>>>
>>> Right, raw is actually a bit of a headache because of that :P
>>>
>>> We could either:
>>>
>>> 1) have a list of common extensions for raw: .raw/.img/etc
>>>
>>> 1b) also treat files without extension as raw?
>>>
>>> 2) have a list of known extensions that are not raw and treat everything
>>> else as raw, while logging an informational message
>>>
>>> I'd prefer 1), because we already require specific extensions for other
>>> uploads.
>>>
>>> And likely we want to rename after/during upload, so images that are raw
>>> for us always have a ".raw" extension? Otherwise, we need to be careful
>>> enough to enforce the very same rules when parsing the import volume
>>> name and thus mostly also have them set in stone for the future. The
>>> advantage of the latter would be for the use case where one wants to
>>> manually make accessible their already existing image folders without
>>> using the API.
>>>
>>
>> I'd also use a list (e.g. for now '.raw', '.img')
>>
>> renaming is a good idea, but how should we do that? e.g.
>>
>> foo.img -> foo.img.raw ?
>>
>> because if we'd do foo.img -> foo.raw i think it's more likely
>> to get a collision than when we keep the .img in the name
>>
>> what do you think?
> 
> Yes, IMHO, simply attaching .raw is better.
> 
>> Side note (can be done later; or not) do we want to support compressed
>> files?
>> (gz, xz, etc.?) just noticed that e.g. the home assistant disk image is a
>> qcow2.xz file
> 
> Yeah, I think it makes sense. We'll just need to adapt the "needs
> extraction" logic?

i actually thought more about uncompressing on download/upload, otherwise
we can't check the file until importing...

in any case, this could still be done as a follow up, I'd like to concentrate
on uncompressed images for now if that's ok with you?


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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Fiona Ebner]

Am 26.03.25 um 13:25 schrieb Dominik Csapak:
> i actually thought more about uncompressing on download/upload, otherwise
> we can't check the file until importing...
> 
> in any case, this could still be done as a follow up, I'd like to
> concentrate
> on uncompressed images for now if that's ok with you?

Sure, I think the fact that that's not supported out of the box with the
new feature is much less surprising to users.


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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Dominik Csapak]

sent a v4:
https://lore.proxmox.com/pve-devel/20250326152608.46560-1-d.csapak@proxmox.com/


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

Re: [pve-devel] [PATCH storage/manager v3] allow upload & import of qcow2 in the web UI

[Thomas Lamprecht]

Am 26.03.25 um 12:57 schrieb Dominik Csapak:
> actually thinking of renaming, i don't think that's necessary to do in the backend at all
> since the client will provide a target filename, we can just rename it in the ui
> to '.raw' for the user?
> 
> then we'd also not have to have a list of 'raw' formats on the backend at all?
> 
> 

Yeah, that sounds like a cleaner approach.


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