On 6/1/23 16:22, Thomas Lamprecht wrote:
Am 19/04/2023 um 12:34 schrieb Aaron Lauterer:
Signed-off-by: Aaron Lauterer <a.laute...@proxmox.com>
---

AFAIK we do not have negative sizes anywhere, and if, it is an
indication that something is wrong.

above belongs in the commit message, additionaly some background for why doing
this now (i.e., did you run into this or what made you make this change?)


good point. It happens with the first patch of the series, when we return '-1' to indicate a broken RBD image.



  src/Utils.js | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/src/Utils.js b/src/Utils.js
index ef72630..8cdbe86 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -688,6 +688,9 @@ utilities: {
      },
format_size: function(size, useSI) {
+       if (size < 0) {
+           return gettext("N/A");

catching this seems OK, but I'd rather just return the value then, as "N/A" (Not
Applicable) doesn't really makes sense here and just hides a potential 
underlying
problem.

Since 'format_size' is used in many places all over the place, what about only checking for it in the content view, where we really shouldn't expect a negative size?
I think showing N/A instead of '-1 B' is more obvious. Something like this:

diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
index 2761b48e..c7b3d5ef 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -182,7 +182,12 @@ Ext.define('PVE.storage.ContentView', {
            'size': {
                header: gettext('Size'),
                width: 100,
-               renderer: Proxmox.Utils.format_size,
+               renderer: function(size) {
+                   if (Number(size) === -1) {
+                       return gettext("N/A");
+                   }
+                   return Proxmox.Utils.format_size(size);
+               },
                dataIndex: 'size',
            },
        };


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

Reply via email to