all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Christian Ebner <c.ebner@proxmox.com>,
	Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH v2 storage] api: fix get content call for volumes
Date: Tue, 07 Mar 2023 09:51:24 +0100	[thread overview]
Message-ID: <1678178711.44w60pfavx.astroid@yuna.none> (raw)
In-Reply-To: <638f7412-b51b-6580-5f9d-3b3259bd404b@proxmox.com>

On March 7, 2023 9:31 am, Fiona Ebner wrote:
> Am 06.03.23 um 14:07 schrieb Christian Ebner:
>> `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
>> 

[..]

>> diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
>> index 9fbd149..acc50d9 100644
>> --- a/PVE/Storage/ZFSPoolPlugin.pm
>> +++ b/PVE/Storage/ZFSPoolPlugin.pm
>> @@ -446,13 +446,17 @@ 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");
>> +    my $used = $class->zfs_get_properties($scfg, 'used', "$scfg->{pool}/$vname");
> 
> used also includes usage of snapshots and children. I think
> usedbydataset is better. Like that used <= size should be guaranteed.
> See man zfsprops for more information.
> 
> Also, one call to zfs_get_properties should be enough by passing both
> "$attr,used".
> 
> While you're at it, I'd also rename $value to $size then.

unfortunately not the case - a zvol can take up (significantly) more space than
volsize:

$ zpool get ashift testpool
NAME      PROPERTY  VALUE   SOURCE
testpool  ashift    12      local
$ zfs get volsize,volblocksize,space testpool/test
NAME           PROPERTY              VALUE          SOURCE
testpool/test  volsize               5G             local
testpool/test  volblocksize          8K             default
testpool/test  name                  testpool/test  -
testpool/test  available             11.2G          -
testpool/test  used                  7.43G          -
testpool/test  usedbysnapshots       0B             -
testpool/test  usedbydataset         7.32G          -
testpool/test  usedbyrefreservation  111M           -
testpool/test  usedbychildren        0B             -

this is the common "ashift=12 / volblocksize=8 / raidz2" pitfall..




  reply	other threads:[~2023-03-07  8:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-06 13:07 Christian Ebner
2023-03-07  8:31 ` Fiona Ebner
2023-03-07  8:51   ` Fabian Grünbichler [this message]
2023-03-07  9:15     ` Fiona Ebner
2023-03-07  9:17     ` Christian Ebner

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=1678178711.44w60pfavx.astroid@yuna.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=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