[pve-devel] [PATCH manager/storage 0/3] Improve return schema for storage listing

[pve-devel] [PATCH manager/storage 0/3] Improve return schema for storage listing

[Dominik Csapak]

when calling the storage listing with `format=1`, we return additional information
about the storage about which formats are supported.

The current return values/schema is bad and not describable by the JSONSchema,
so add a new property with a better schema.

With this, our rust api schema generator will be able to handle this properly.

This was noticed by dietmar when trying to use this api from rust.

pve-storage:

Dominik Csapak (2):
  api: try to add more return schema information
  api: list: return 'formats' info in a better structured way

 src/PVE/API2/Storage/Status.pm | 40 ++++++++++++++++++++++++++++++++++
 src/PVE/Storage.pm             | 12 ++++++++++
 2 files changed, 52 insertions(+)


pve-manager:

Dominik Csapak (1):
  ui: disk storage selector: use new api return format

 www/manager6/form/DiskStorageSelector.js | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)


Summary over all repositories:
  3 files changed, 68 insertions(+), 4 deletions(-)

-- 
Generated by git-murpp 0.8.1


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

[pve-devel] [PATCH storage 1/2] api: try to add more return schema information

[Dominik Csapak]

no problem for 'select_existing', but we cannot actually describe
'format' with our JSONSchema, since it uses an array as a form of tuple,
and even with oneOf this cannot be described currently.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/API2/Storage/Status.pm | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index 7bde4ec..ff32782 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -171,6 +171,23 @@ __PACKAGE__->register_method({
                     renderer => 'fraction_as_percentage',
                     optional => 1,
                 },
+                select_existing => {
+                    description => "Instead of creating new volumes, one must select one that"
+                        . " is already existing. Only included if 'format' parameter is set.",
+                    type => 'boolean',
+                    optional => 1,
+                },
+
+                # we can't include this return schema, since we cannot properly
+                # describe what it actually is with the json schema:
+                #
+                # a tuple in form of an array where the first element is an
+                # object, and the second is a string.
+                #format => {
+                #    description => "Lists the supported and default format."
+                #        . " Only included if 'format' parameter is set.",
+                #    optional => 1,
+                #},
             },
         },
         links => [{ rel => 'child', href => "{storage}" }],
-- 
2.47.3



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

[pve-devel] [PATCH storage 2/2] api: list: return 'formats' info in a better structured way

[Dominik Csapak]

returning the formats in the way of:
```
"format": [
    {
        "format1" => 1,
        "format2" => 1,
        ...
    },
    "defaultFormat"
]
```

is not a very good return format, since it abuses an array as a
tuple, and unnecessarily encodes a list of formats as an object.
Also, we can't describe it properly in JSONSchema in perl, nor our
perl->rust generator is able to handle that.

Instead, return it like this:
```
"formats": {
    "default": "defaultFormat",
    "supported": ["format1", "format2", ...]
}
```

which makes it much more sensible for an api return schema, and it's
possible to annotate it in the JSONSchema.

For compatibility reasons, keep the old property around, and add a
comment to remove with 10.0

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/API2/Storage/Status.pm | 25 ++++++++++++++++++++++++-
 src/PVE/Storage.pm             | 12 ++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index ff32782..8107401 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -178,6 +178,7 @@ __PACKAGE__->register_method({
                     optional => 1,
                 },
 
+                # FIXME: remove with 10.0
                 # we can't include this return schema, since we cannot properly
                 # describe what it actually is with the json schema:
                 #
@@ -185,9 +186,31 @@ __PACKAGE__->register_method({
                 # object, and the second is a string.
                 #format => {
                 #    description => "Lists the supported and default format."
-                #        . " Only included if 'format' parameter is set.",
+                #        . "Deprecated (use 'formats' instead). Only included "
+                #        . "if 'format' parameter is set.",
                 #    optional => 1,
                 #},
+                formats => {
+                    description => "Lists the supported and default format. Use"
+                        . " 'formats' instead. Only included if 'format' parameter is set.",
+                    optional => 1,
+                    type => 'object',
+                    properties => {
+                        supported => {
+                            type => 'array',
+                            description => 'The list of supported formats',
+                            items => {
+                                type => 'string',
+                                enum => [qw(qcow2 raw subvol vmdk)],
+                            },
+                        },
+                        default => {
+                            description => "The default format of the storage.",
+                            type => 'string',
+                            enum => [qw(qcow2 raw subvol vmdk)],
+                        },
+                    },
+                },
             },
         },
         links => [{ rel => 'child', href => "{storage}" }],
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 1dde2b7..935d457 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -1485,6 +1485,18 @@ sub storage_info {
         my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
         if ($includeformat) {
             my $formats = $plugin->get_formats($scfg, $storeid);
+
+            my $supported = [];
+            for my $format (sort keys $formats->{valid}->%*) {
+                push $supported->@*, $format if $formats->{valid}->{$format};
+            }
+
+            $info->{$storeid}->{'formats'} = {
+                supported => $supported,
+                default => $formats->{default},
+            };
+
+            # FIXME: deprecated, remove with 10.0
             $info->{$storeid}->{format} = [$formats->{valid}, $formats->{default}];
 
             my $pd = $plugin->plugindata();
-- 
2.47.3



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

[pve-devel] [PATCH manager 1/1] ui: disk storage selector: use new api return format

[Dominik Csapak]

since that will be used in the future. For compatibility with older
nodes, keep the old logic around.

No functional change intended.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/form/DiskStorageSelector.js | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/www/manager6/form/DiskStorageSelector.js b/www/manager6/form/DiskStorageSelector.js
index ec22ef58..edfdfad4 100644
--- a/www/manager6/form/DiskStorageSelector.js
+++ b/www/manager6/form/DiskStorageSelector.js
@@ -58,16 +58,28 @@ Ext.define('PVE.form.DiskStorageSelector', {
         }
 
         let validFormats = {};
-        let selectFormat = 'raw';
-        if (rec.data.format) {
-            validFormats = rec.data.format[0]; // 0 is the formats, 1 the default in the backend
+        let defaultFormat = 'raw';
+        let selectFormat = defaultFormat;
+        if (rec.data.formats) {
+            for (const format of rec.data.formats.supported) {
+                validFormats[format] = true;
+            }
+            defaultFormat = rec.data.formats.default;
+        } else if (rec.data.format) {
+            // legacy api, just for compatibility
+            // 0 is the formats, 1 the default in the backend
+            validFormats = rec.data.format[0];
+            defaultFormat = rec.data.format[1];
+        }
+
+        if (Object.keys(validFormats).length > 0) {
             delete validFormats.subvol; // we never need subvol in the gui
             if (validFormats.qcow2) {
                 selectFormat = 'qcow2';
             } else if (validFormats.raw) {
                 selectFormat = 'raw';
             } else {
-                selectFormat = rec.data.format[1];
+                selectFormat = defaultFormat;
             }
         }
 
-- 
2.47.3



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