all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Christian Ebner <c.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v3 storage] api: fix get content call for volumes
Date: Thu,  9 Mar 2023 10:41:23 +0100	[thread overview]
Message-ID: <20230309094123.564813-1-c.ebner@proxmox.com> (raw)

`pvesh get /nodes/{node}/storage/{storage}/content/{volume}` failed for
several storage types, because the respective storage plugins returned
only the volumes `size` on `volume_size_info` calls, while also the format
is required.

This patch fixes the issue by returning also `format` and where possible `used`.

The issue was reported in the forum:
https://forum.proxmox.com/threads/pvesh-get-nodes-node-storage-storage-content-volume-returns-error.123747/

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---

Changes since v1:
 * Remove errous check for $used being set, rely on fallback to 0 if undef
 * Return `parent` for RBD and ZFS
 * Return `used` for ZFS

Changes since v2:
 * Add conditional call to `rbd du` to get `used` for RBD based volumes
 * Get `usedbydataset` instead of `used` for ZFS volumes, refactor
   zfs_get_properties call
 
 Note: The file_size_info for iscsi direct targets unfortunately does
       not return anything usefull for `used` storage size, so it stayed as
       is.

 PVE/Storage/ISCSIDirectPlugin.pm |  2 +-
 PVE/Storage/RBDPlugin.pm         | 44 ++++++++++++++++++++++++++++++--
 PVE/Storage/ZFSPoolPlugin.pm     | 11 +++++---
 3 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/PVE/Storage/ISCSIDirectPlugin.pm b/PVE/Storage/ISCSIDirectPlugin.pm
index 9777969..eb329d4 100644
--- a/PVE/Storage/ISCSIDirectPlugin.pm
+++ b/PVE/Storage/ISCSIDirectPlugin.pm
@@ -208,7 +208,7 @@ sub volume_size_info {
     my $vollist = iscsi_ls($scfg,$storeid);
     my $info = $vollist->{$storeid}->{$volname};
 
-    return $info->{size};
+    return wantarray ? ($info->{size}, 'raw', 0, undef) : $info->{size};
 }
 
 sub volume_resize {
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index 9047504..35b2372 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -308,6 +308,45 @@ sub rbd_volume_info {
     return $volume->@{qw(size parent format protected features)};
 }
 
+sub rbd_volume_du {
+    my ($scfg, $storeid, $volname) = @_;
+
+    my @options = ('du', $volname, '--format', 'json');
+    my $cmd = $rbd_cmd->($scfg, $storeid, @options);
+
+    my $raw = '';
+    my $parser = sub { $raw .= shift };
+
+    run_rbd_command($cmd, errmsg => "rbd error", errfunc => sub {}, outfunc => $parser);
+
+    my $volume;
+    if ($raw eq '') {
+	$volume = {};
+    } elsif ($raw =~ m/^(\{.*\})$/s) { # untaint
+	$volume = JSON::decode_json($1);
+    } else {
+	die "got unexpected data from rbd du: '$raw'\n";
+    }
+
+    if (!defined($volume->{images})) {
+	die "got no images from rbd du\n";
+    }
+
+    # `rbd du` returns array of images for name matching `volname`,
+    # including snapshots.
+    my $images = $volume->{images};
+    foreach my $image (@$images) {
+	next if defined($image->{snapshot});
+	next if !defined($image->{used_size}) || !defined($image->{name});
+
+	# Return `used_size` of first volume with matching name which
+	# is not a snapshot.
+	return $image->{used_size} if $image->{name} eq $volname;
+    }
+
+    die "got no matching image from rbd du\n";
+}
+
 # Configuration
 
 sub type {
@@ -729,8 +768,9 @@ sub volume_size_info {
     my ($class, $scfg, $storeid, $volname, $timeout) = @_;
 
     my ($vtype, $name, $vmid) = $class->parse_volname($volname);
-    my ($size, undef) = rbd_volume_info($scfg, $storeid, $name);
-    return $size;
+    my ($size, $parent) = rbd_volume_info($scfg, $storeid, $name);
+    my $used = wantarray ? rbd_volume_du($scfg, $storeid, $name) : 0;
+    return wantarray ? ($size, 'raw', $used, $parent) : $size;
 }
 
 sub volume_resize {
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 9fbd149..54dd2ae 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -446,13 +446,16 @@ sub status {
 sub volume_size_info {
     my ($class, $scfg, $storeid, $volname, $timeout) = @_;
 
-    my (undef, $vname, undef, undef, undef, undef, $format) =
+    my (undef, $vname, undef, $parent, undef, undef, $format) =
         $class->parse_volname($volname);
 
     my $attr = $format eq 'subvol' ? 'refquota' : 'volsize';
-    my $value = $class->zfs_get_properties($scfg, $attr, "$scfg->{pool}/$vname");
-    if ($value =~ /^(\d+)$/) {
-	return $1;
+    my ($size, $used) = $class->zfs_get_properties($scfg, "$attr,usedbydataset", "$scfg->{pool}/$vname");
+
+    $used = ($used =~ /^(\d+)$/) ? $1 : 0;
+    
+    if ($size =~ /^(\d+)$/) {
+	return wantarray ? ($1, $format, $used, $parent) : $1;
     }
 
     die "Could not get zfs volume size\n";
-- 
2.30.2





             reply	other threads:[~2023-03-09  9:41 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-09  9:41 Christian Ebner [this message]
2023-03-20 15:36 ` [pve-devel] applied: " Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230309094123.564813-1-c.ebner@proxmox.com \
    --to=c.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal