[PATCH-SERIES storage/manager/container/qemu-server 0/8] fix #6819: lvm plugin: list images: use approximate size for inactive qcow2 volumes

[PATCH-SERIES storage/manager/container/qemu-server 0/8] fix #6819: lvm plugin: list images: use approximate size for inactive qcow2 volumes

[Fiona Ebner]

For qcow2 volumes on LVM, the exact size can only be queried when the
LV is active. For a shared LVM, an LV might already be active on
another node, so it must not be activated. Also, doing so for each
volume would be costly. Instead, return only the approximate size when
the LV is inactive.

Adapt the list volumes API/CLI implementations as well as the UI and
volume rescan operations for guests to handle casees where the exact
size is not present, but only the new approximate size property.

storage:

Fiona Ebner (4):
  api: content: add missing imports and group/order according to style
    guide
  pvesm: add missing imports and group/order according to style guide
  api/cli: list content: declare size optional and add approximate-size
  fix #6819: lvm plugin: list images: use approximate size for inactive
    qcow2 volumes

 src/PVE/API2/Storage/Content.pm | 32 ++++++++++++++++--------
 src/PVE/CLI/pvesm.pm            | 44 ++++++++++++++++++++++++---------
 src/PVE/Storage/LVMPlugin.pm    | 33 ++++++++++++++++---------
 3 files changed, 75 insertions(+), 34 deletions(-)


manager:

Fiona Ebner (1):
  fix #6819: storage: content view: fallback to approximate size

 www/manager6/storage/ContentView.js | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)


container:

Fiona Ebner (2):
  volume rescan: also consider zero-sized volumes
  volume rescan: also consider volumes listed only with an approximate
    size

 src/PVE/LXC.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)


qemu-server:

Fiona Ebner (1):
  volume rescan: also consider volumes listed only with an approximate
    size

 src/PVE/QemuServer.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


Summary over all repositories:
  6 files changed, 95 insertions(+), 41 deletions(-)

-- 
Generated by git-murpp 0.5.0

[PATCH storage 1/8] api: content: add missing imports and group/order according to style guide

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Storage/Content.pm | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/PVE/API2/Storage/Content.pm b/src/PVE/API2/Storage/Content.pm
index 1fe7303..59ac0ba 100644
--- a/src/PVE/API2/Storage/Content.pm
+++ b/src/PVE/API2/Storage/Content.pm
@@ -3,16 +3,19 @@ package PVE::API2::Storage::Content;
 use strict;
 use warnings;
 
-use PVE::SafeSyslog;
 use PVE::Cluster;
+use PVE::Exception qw(raise_param_exc);
+use PVE::INotify;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::ProcFSTools;
+use PVE::RESTHandler;
+use PVE::RPCEnvironment;
+use PVE::SafeSyslog;
+use PVE::SSHInfo;
+use PVE::Tools;
+
 use PVE::Storage;
 use PVE::Storage::Common; # for 'pve-storage-image-format' standard option
-use PVE::INotify;
-use PVE::Exception qw(raise_param_exc);
-use PVE::RPCEnvironment;
-use PVE::RESTHandler;
-use PVE::JSONSchema qw(get_standard_option);
-use PVE::SSHInfo;
 
 use base qw(PVE::RESTHandler);
 
-- 
2.47.3

[PATCH storage 2/8] pvesm: add missing imports and group/order according to style guide

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/CLI/pvesm.pm | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/PVE/CLI/pvesm.pm b/src/PVE/CLI/pvesm.pm
index 06bc4c9..d34e520 100755
--- a/src/PVE/CLI/pvesm.pm
+++ b/src/PVE/CLI/pvesm.pm
@@ -3,30 +3,34 @@ package PVE::CLI::pvesm;
 use strict;
 use warnings;
 
-use POSIX qw(O_RDONLY O_WRONLY O_CREAT O_TRUNC);
 use Fcntl ':flock';
 use File::Path;
-use MIME::Base64 qw(encode_base64);
-
 use IO::Socket::IP;
 use IO::Socket::UNIX;
+use MIME::Base64 qw(encode_base64);
+use POSIX qw(O_RDONLY O_WRONLY O_CREAT O_TRUNC);
 use Socket qw(SOCK_STREAM);
 
-use PVE::SafeSyslog;
+use PVE::CLIFormatter;
+use PVE::CLIHandler;
 use PVE::Cluster;
 use PVE::INotify;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::Network;
+use PVE::PTY;
+use PVE::RESTHandler;
 use PVE::RPCEnvironment;
-use PVE::Storage;
+use PVE::SafeSyslog;
 use PVE::Tools qw(extract_param);
+
+use PVE::Storage;
+use PVE::Storage::Plugin;
+
 use PVE::API2::Storage::Config;
 use PVE::API2::Storage::Content;
 use PVE::API2::Storage::PruneBackups;
 use PVE::API2::Storage::Scan;
 use PVE::API2::Storage::Status;
-use PVE::JSONSchema qw(get_standard_option);
-use PVE::PTY;
-
-use PVE::CLIHandler;
 
 use base qw(PVE::CLIHandler);
 
-- 
2.47.3

[PATCH storage 3/8] api/cli: list content: declare size optional and add approximate-size

[Fiona Ebner]

For inactive qcow2 volumes on LVM, currently no size information is
returned, because that would require activating each LV to scan the
qcow2 header. For a shared LVM, an LV might already be active on
another node, so it cannot be activated. Also, doing so for each
volume would be costly. Prepare the API and CLI for handling an
approximate size returned instead.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Storage/Content.pm | 15 ++++++++++++---
 src/PVE/CLI/pvesm.pm            | 22 +++++++++++++++++++---
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/src/PVE/API2/Storage/Content.pm b/src/PVE/API2/Storage/Content.pm
index 59ac0ba..cffb555 100644
--- a/src/PVE/API2/Storage/Content.pm
+++ b/src/PVE/API2/Storage/Content.pm
@@ -89,6 +89,15 @@ __PACKAGE__->register_method({
                     description => "Volume size in bytes.",
                     type => 'integer',
                     renderer => 'bytes',
+                    optional => 1,
+                },
+                'approximate-size' => {
+                    description =>
+                        "Approximate volume size in bytes. Present instead of 'size' for storages"
+                        . " where determining the exact size has technical limitations.",
+                    type => 'integer',
+                    renderer => 'bytes',
+                    optional => 1,
                 },
                 used => {
                     description => "Used space. Please note that most storage plugins "
@@ -163,9 +172,9 @@ __PACKAGE__->register_method({
                 );
             };
             next if $@;
-            $item->{vmid} = int($item->{vmid}) if defined($item->{vmid});
-            $item->{size} = int($item->{size}) if defined($item->{size});
-            $item->{used} = int($item->{used}) if defined($item->{used});
+            for my $prop (qw(approximate-size size used vmid)) {
+                $item->{$prop} = int($item->{$prop}) if defined($item->{$prop});
+            }
             push @$res, $item;
         }
 
diff --git a/src/PVE/CLI/pvesm.pm b/src/PVE/CLI/pvesm.pm
index d34e520..523970f 100755
--- a/src/PVE/CLI/pvesm.pm
+++ b/src/PVE/CLI/pvesm.pm
@@ -198,30 +198,46 @@ my $print_content = sub {
     my ($list) = @_;
 
     my ($maxlenname, $maxsize) = (0, 0);
+    my $has_approximate_size;
     foreach my $info (@$list) {
         my $volid = $info->{volid};
         my $sidlen = length($volid);
         $maxlenname = $sidlen if $sidlen > $maxlenname;
-        $maxsize = $info->{size} if ($info->{size} // 0) > $maxsize;
+        if (defined($info->{size})) {
+            $maxsize = $info->{size} if $info->{size} > $maxsize;
+        } elsif (defined($info->{'approximate-size'})) {
+            $has_approximate_size = 1;
+            $maxsize = $info->{'approximate-size'} if $info->{'approximate-size'} > $maxsize;
+        }
     }
     my $sizemaxdigits = length($maxsize);
+    $sizemaxdigits += 1 if $has_approximate_size;
 
     my $basefmt = "%-${maxlenname}s %-7s %-9s %${sizemaxdigits}s";
     printf "$basefmt %s\n", "Volid", "Format", "Type", "Size", "VMID";
 
+    my $get_size_str = sub {
+        my ($info) = @_;
+        return "$info->{size}" if defined($info->{size});
+        return "~$info->{'approximate-size'}" if defined($info->{'approximate-size'});
+        return "-";
+    };
+
     foreach my $info (@$list) {
         next if !$info->{vmid};
         my $volid = $info->{volid};
+        my $size_str = $get_size_str->($info);
 
-        printf "$basefmt %d\n", $volid, $info->{format}, $info->{content}, $info->{size},
+        printf "$basefmt %d\n", $volid, $info->{format}, $info->{content}, $size_str,
             $info->{vmid};
     }
 
     foreach my $info (sort { $a->{format} cmp $b->{format} } @$list) {
         next if $info->{vmid};
         my $volid = $info->{volid};
+        my $size_str = $get_size_str->($info);
 
-        printf "$basefmt\n", $volid, $info->{format}, $info->{content}, $info->{size};
+        printf "$basefmt\n", $volid, $info->{format}, $info->{content}, $size_str;
     }
 };
 
-- 
2.47.3

[PATCH storage 4/8] fix #6819: lvm plugin: list images: use approximate size for inactive qcow2 volumes

[Fiona Ebner]

For qcow2 volumes on LVM, the exact size can only be queried when the
LV is active. For a shared LVM, an LV might already be active on
another node, so it must not be activated. Also, doing so for each
volume would be costly. Instead, return only the approximate size when
the LV is inactive.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Storage/LVMPlugin.pm | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 3a35e38..4564f5b 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -833,19 +833,28 @@ sub list_images {
             }
 
             my $format = ($class->parse_volname($volname))[6];
-            my $size =
-                $format eq 'qcow2'
-                ? $class->volume_size_info($scfg, $storeid, $volname)
-                : $info->{lv_size};
+            my $entry = {
+                volid => $volid,
+                format => $format,
+                vmid => $owner,
+                ctime => $info->{ctime},
+            };
 
-            push @$res,
-                {
-                    volid => $volid,
-                    format => $format,
-                    size => $size,
-                    vmid => $owner,
-                    ctime => $info->{ctime},
-                };
+            if ($format eq 'qcow2') {
+                my $size;
+                if ($info->{lv_state} eq 'a') {
+                    $size = $class->volume_size_info($scfg, $storeid, $volname);
+                }
+                if (defined($size)) {
+                    $entry->{size} = $size;
+                } else {
+                    $entry->{'approximate-size'} = $info->{lv_size};
+                }
+            } else {
+                $entry->{size} = $info->{lv_size};
+            }
+
+            push @$res, $entry;
         }
     }
 
-- 
2.47.3

[PATCH manager 5/8] fix #6819: storage: content view: fallback to approximate size

[Fiona Ebner]

For qcow2 volumes on LVM, if the LV is inactive, only an approximate
size is returned instead of the exact size. Handle this case too.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/storage/ContentView.js | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
index 75b81e69..a4c2ef0d 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -187,7 +187,6 @@ Ext.define(
                 size: {
                     header: gettext('Size'),
                     width: 100,
-                    renderer: Proxmox.Utils.format_size,
                     dataIndex: 'size',
                 },
             };
@@ -226,7 +225,20 @@ Ext.define(
                 'volid',
                 'content',
                 'format',
-                'size',
+                {
+                    name: 'size',
+                    convert: function (value, record) {
+                        if (value !== undefined) {
+                            return Proxmox.Utils.format_size(value);
+                        } else if (record.data['approximate-size'] !== undefined) {
+                            return Ext.String.format(
+                                '~{0}',
+                                Proxmox.Utils.format_size(record.data['approximate-size']),
+                            );
+                        }
+                        return Proxmox.Utils.unknownText;
+                    },
+                },
                 'used',
                 'vmid',
                 'channel',
-- 
2.47.3

[PATCH container 6/8] volume rescan: also consider zero-sized volumes

[Fiona Ebner]

A volume of format 'subvol' can also have size zero. Properly detect
such a volume as unused too.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/LXC.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 6d1a3c4..2b3acdd 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -2761,7 +2761,7 @@ sub scan_volids {
     foreach my $storeid (keys %$info) {
         foreach my $item (@{ $info->{$storeid} }) {
             my $volid = $item->{volid};
-            next if !($volid && $item->{size});
+            next if !($volid && defined($item->{size}));
             $item->{path} = PVE::Storage::path($cfg, $volid);
             $all_volumes->{$volid} = $item;
         }
-- 
2.47.3

[PATCH container 7/8] volume rescan: also consider volumes listed only with an approximate size

[Fiona Ebner]

While currently qcow2 volumes on LVM are the only ones for which an
approximate size might be returned instead of an exact size when
listing the volumes, there might be more storages doing this in the
future, so prepare for that.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/LXC.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 2b3acdd..de8f22e 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -2688,9 +2688,9 @@ sub update_disksize {
 
     my $update_mp = sub {
         my ($key, $mp, @param) = @_;
-        my $size = $all_volumes->{ $mp->{volume} }->{size} // 0;
+        my $size = $all_volumes->{ $mp->{volume} }->{size};
 
-        if (!defined($mp->{size}) || $size != $mp->{size}) {
+        if (defined($size) && (!defined($mp->{size}) || $size != $mp->{size})) {
             $changes = 1;
             print "$prefix updated volume size of '$mp->{volume}' in config.\n";
             $mp->{size} = $size;
@@ -2761,7 +2761,7 @@ sub scan_volids {
     foreach my $storeid (keys %$info) {
         foreach my $item (@{ $info->{$storeid} }) {
             my $volid = $item->{volid};
-            next if !($volid && defined($item->{size}));
+            next if !($volid && (defined($item->{size}) || defined($item->{'approximate-size'})));
             $item->{path} = PVE::Storage::path($cfg, $volid);
             $all_volumes->{$volid} = $item;
         }
-- 
2.47.3

[PATCH qemu-server 8/8] volume rescan: also consider volumes listed only with an approximate size

[Fiona Ebner]

For qcow2 volumes on LVM, if the LV is inactive, only an approximate
size is returned instead of the exact size. Handle this case too.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 3ee58ec5..fe2f620e 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -6739,7 +6739,7 @@ sub scan_volids {
     my $volid_hash = {};
     foreach my $storeid (keys %$info) {
         foreach my $item (@{ $info->{$storeid} }) {
-            next if !($item->{volid} && $item->{size});
+            next if !($item->{volid} && ($item->{size} || $item->{'approximate-size'}));
             $item->{path} = PVE::Storage::path($cfg, $item->{volid});
             $volid_hash->{ $item->{volid} } = $item;
         }
@@ -6763,7 +6763,7 @@ sub update_disk_config {
 
     my $referencedpath = {};
 
-    # update size info
+    # update size info, when the exact size is available
     PVE::QemuConfig->foreach_volume(
         $conf,
         sub {
@@ -6781,6 +6781,7 @@ sub update_disk_config {
 
             return if drive_is_cdrom($drive);
             return if !$volume;
+            return if !defined($volume->{size});
 
             my ($updated, $msg) =
                 PVE::QemuServer::Drive::update_disksize($drive, $volume->{size});
-- 
2.47.3