* [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result
@ 2021-10-21 12:49 Lorenz Stechauner
2021-10-21 12:49 ` [pve-devel] [PATCH v3 storage 2/2] pvesm: using $API instead of PVE::API2::Storage Lorenz Stechauner
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Lorenz Stechauner @ 2021-10-21 12:49 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
PVE/CLI/pvesm.pm | 158 +++++++++--------------------------------------
1 file changed, 30 insertions(+), 128 deletions(-)
diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index 190de91..94847cd 100755
--- a/PVE/CLI/pvesm.pm
+++ b/PVE/CLI/pvesm.pm
@@ -178,68 +178,6 @@ __PACKAGE__->register_method ({
return;
}});
-my $print_content = sub {
- my ($list) = @_;
-
- my ($maxlenname, $maxsize) = (0, 0);
- 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;
- }
- my $sizemaxdigits = length($maxsize);
-
- my $basefmt = "%-${maxlenname}s %-7s %-9s %${sizemaxdigits}s";
- printf "$basefmt %s\n", "Volid", "Format", "Type", "Size", "VMID";
-
- foreach my $info (@$list) {
- next if !$info->{vmid};
- my $volid = $info->{volid};
-
- printf "$basefmt %d\n", $volid, $info->{format}, $info->{content}, $info->{size}, $info->{vmid};
- }
-
- foreach my $info (sort { $a->{format} cmp $b->{format} } @$list) {
- next if $info->{vmid};
- my $volid = $info->{volid};
-
- printf "$basefmt\n", $volid, $info->{format}, $info->{content}, $info->{size};
- }
-};
-
-my $print_status = sub {
- my $res = shift;
-
- my $maxlen = 0;
- foreach my $res (@$res) {
- my $storeid = $res->{storage};
- $maxlen = length ($storeid) if length ($storeid) > $maxlen;
- }
- $maxlen+=1;
-
- printf "%-${maxlen}s %10s %10s %15s %15s %15s %8s\n", 'Name', 'Type',
- 'Status', 'Total', 'Used', 'Available', '%';
-
- foreach my $res (sort { $a->{storage} cmp $b->{storage} } @$res) {
- my $storeid = $res->{storage};
-
- my $active = $res->{active} ? 'active' : 'inactive';
- my ($per, $per_fmt) = (0, '% 7.2f%%');
- $per = ($res->{used}*100)/$res->{total} if $res->{total} > 0;
-
- if (!$res->{enabled}) {
- $per = 'N/A';
- $per_fmt = '% 8s';
- $active = 'disabled';
- }
-
- printf "%-${maxlen}s %10s %10s %15d %15d %15d $per_fmt\n", $storeid,
- $res->{type}, $active, $res->{total}/1024, $res->{used}/1024,
- $res->{avail}/1024, $per;
- }
-};
-
__PACKAGE__->register_method ({
name => 'export',
path => 'export',
@@ -573,14 +511,26 @@ my $print_api_result = sub {
PVE::CLIFormatter::print_api_result($data, $schema, undef, $options);
};
+my $print_content = sub {
+ my ($data, $schema, $options) = @_;
+ my $order = [qw(volid format content size used vmid ctime parent encrypted verification notes)];
+ PVE::CLIFormatter::print_api_result($data, $schema, $order, $options);
+};
+
+my $print_status = sub {
+ my ($data, $schema, $options) = @_;
+ my $order = [qw(storage type active enabled shared total used avail used_fraction content)];
+ PVE::CLIFormatter::print_api_result($data, $schema, $order, $options);
+};
+
our $cmddef = {
add => [ "PVE::API2::Storage::Config", 'create', ['type', 'storage'] ],
set => [ "PVE::API2::Storage::Config", 'update', ['storage'] ],
remove => [ "PVE::API2::Storage::Config", 'delete', ['storage'] ],
- status => [ "PVE::API2::Storage::Status", 'index', [],
- { node => $nodename }, $print_status ],
- list => [ "PVE::API2::Storage::Content", 'index', ['storage'],
- { node => $nodename }, $print_content ],
+ status => [ "PVE::API2::Storage::Status", 'index', [], { node => $nodename },
+ $print_status, $PVE::RESTHandler::standard_output_options ],
+ list => [ "PVE::API2::Storage::Content", 'index', ['storage'], { node => $nodename },
+ $print_content, $PVE::RESTHandler::standard_output_options ],
alloc => [ "PVE::API2::Storage::Content", 'create', ['storage', 'vmid', 'filename', 'size'],
{ node => $nodename }, sub {
my $volid = shift;
@@ -589,61 +539,18 @@ our $cmddef = {
free => [ "PVE::API2::Storage::Content", 'delete', ['volume'],
{ node => $nodename } ],
scan => {
- nfs => [ "PVE::API2::Storage::Scan", 'nfsscan', ['server'], { node => $nodename }, sub {
- my $res = shift;
-
- my $maxlen = 0;
- foreach my $rec (@$res) {
- my $len = length ($rec->{path});
- $maxlen = $len if $len > $maxlen;
- }
- foreach my $rec (@$res) {
- printf "%-${maxlen}s %s\n", $rec->{path}, $rec->{options};
- }
- }],
- cifs => [ "PVE::API2::Storage::Scan", 'cifsscan', ['server'], { node => $nodename }, sub {
- my $res = shift;
-
- my $maxlen = 0;
- foreach my $rec (@$res) {
- my $len = length ($rec->{share});
- $maxlen = $len if $len > $maxlen;
- }
- foreach my $rec (@$res) {
- printf "%-${maxlen}s %s\n", $rec->{share}, $rec->{description};
- }
- }],
- glusterfs => [ "PVE::API2::Storage::Scan", 'glusterfsscan', ['server'], { node => $nodename }, sub {
- my $res = shift;
-
- foreach my $rec (@$res) {
- printf "%s\n", $rec->{volname};
- }
- }],
- iscsi => [ "PVE::API2::Storage::Scan", 'iscsiscan', ['portal'], { node => $nodename }, sub {
- my $res = shift;
-
- my $maxlen = 0;
- foreach my $rec (@$res) {
- my $len = length ($rec->{target});
- $maxlen = $len if $len > $maxlen;
- }
- foreach my $rec (@$res) {
- printf "%-${maxlen}s %s\n", $rec->{target}, $rec->{portal};
- }
- }],
- lvm => [ "PVE::API2::Storage::Scan", 'lvmscan', [], { node => $nodename }, sub {
- my $res = shift;
- foreach my $rec (@$res) {
- printf "$rec->{vg}\n";
- }
- }],
- lvmthin => [ "PVE::API2::Storage::Scan", 'lvmthinscan', ['vg'], { node => $nodename }, sub {
- my $res = shift;
- foreach my $rec (@$res) {
- printf "$rec->{lv}\n";
- }
- }],
+ nfs => [ "PVE::API2::Storage::Scan", 'nfsscan', ['server'], { node => $nodename },
+ $print_api_result, $PVE::RESTHandler::standard_output_options ],
+ cifs => [ "PVE::API2::Storage::Scan", 'cifsscan', ['server'], { node => $nodename },
+ $print_api_result, $PVE::RESTHandler::standard_output_options ],
+ glusterfs => [ "PVE::API2::Storage::Scan", 'glusterfsscan', ['server'], { node => $nodename },
+ $print_api_result, $PVE::RESTHandler::standard_output_options ],
+ iscsi => [ "PVE::API2::Storage::Scan", 'iscsiscan', ['portal'], { node => $nodename },
+ $print_api_result, $PVE::RESTHandler::standard_output_options ],
+ lvm => [ "PVE::API2::Storage::Scan", 'lvmscan', [], { node => $nodename },
+ $print_api_result, $PVE::RESTHandler::standard_output_options ],
+ lvmthin => [ "PVE::API2::Storage::Scan", 'lvmthinscan', ['vg'], { node => $nodename },
+ $print_api_result, $PVE::RESTHandler::standard_output_options ],
pbs => [
"PVE::API2::Storage::Scan",
'pbsscan',
@@ -652,13 +559,8 @@ our $cmddef = {
$print_api_result,
$PVE::RESTHandler::standard_output_options,
],
- zfs => [ "PVE::API2::Storage::Scan", 'zfsscan', [], { node => $nodename }, sub {
- my $res = shift;
-
- foreach my $rec (@$res) {
- printf "$rec->{pool}\n";
- }
- }],
+ zfs => [ "PVE::API2::Storage::Scan", 'zfsscan', [], { node => $nodename },
+ $print_api_result, $PVE::RESTHandler::standard_output_options ],
},
nfsscan => { alias => 'scan nfs' },
cifsscan => { alias => 'scan cifs' },
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH v3 storage 2/2] pvesm: using $API instead of PVE::API2::Storage
2021-10-21 12:49 [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result Lorenz Stechauner
@ 2021-10-21 12:49 ` Lorenz Stechauner
2021-10-21 12:50 ` [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result Lorenz Stechauner
2021-10-25 14:17 ` Dominik Csapak
2 siblings, 0 replies; 5+ messages in thread
From: Lorenz Stechauner @ 2021-10-21 12:49 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
PVE/CLI/pvesm.pm | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index 94847cd..78fc51b 100755
--- a/PVE/CLI/pvesm.pm
+++ b/PVE/CLI/pvesm.pm
@@ -523,43 +523,45 @@ my $print_status = sub {
PVE::CLIFormatter::print_api_result($data, $schema, $order, $options);
};
+my $API = 'PVE::API2::Storage';
+
our $cmddef = {
- add => [ "PVE::API2::Storage::Config", 'create', ['type', 'storage'] ],
- set => [ "PVE::API2::Storage::Config", 'update', ['storage'] ],
- remove => [ "PVE::API2::Storage::Config", 'delete', ['storage'] ],
- status => [ "PVE::API2::Storage::Status", 'index', [], { node => $nodename },
+ add => [ "${API}::Config", 'create', ['type', 'storage'] ],
+ set => [ "${API}::Config", 'update', ['storage'] ],
+ remove => [ "${API}::Config", 'delete', ['storage'] ],
+ status => [ "${API}::Status", 'index', [], { node => $nodename },
$print_status, $PVE::RESTHandler::standard_output_options ],
- list => [ "PVE::API2::Storage::Content", 'index', ['storage'], { node => $nodename },
+ list => [ "${API}::Content", 'index', ['storage'], { node => $nodename },
$print_content, $PVE::RESTHandler::standard_output_options ],
- alloc => [ "PVE::API2::Storage::Content", 'create', ['storage', 'vmid', 'filename', 'size'],
+ alloc => [ "${API}::Content", 'create', ['storage', 'vmid', 'filename', 'size'],
{ node => $nodename }, sub {
my $volid = shift;
print "successfully created '$volid'\n";
}],
- free => [ "PVE::API2::Storage::Content", 'delete', ['volume'],
+ free => [ "${API}::Content", 'delete', ['volume'],
{ node => $nodename } ],
scan => {
- nfs => [ "PVE::API2::Storage::Scan", 'nfsscan', ['server'], { node => $nodename },
+ nfs => [ "${API}::Scan", 'nfsscan', ['server'], { node => $nodename },
$print_api_result, $PVE::RESTHandler::standard_output_options ],
- cifs => [ "PVE::API2::Storage::Scan", 'cifsscan', ['server'], { node => $nodename },
+ cifs => [ "${API}::Scan", 'cifsscan', ['server'], { node => $nodename },
$print_api_result, $PVE::RESTHandler::standard_output_options ],
- glusterfs => [ "PVE::API2::Storage::Scan", 'glusterfsscan', ['server'], { node => $nodename },
+ glusterfs => [ "${API}::Scan", 'glusterfsscan', ['server'], { node => $nodename },
$print_api_result, $PVE::RESTHandler::standard_output_options ],
- iscsi => [ "PVE::API2::Storage::Scan", 'iscsiscan', ['portal'], { node => $nodename },
+ iscsi => [ "${API}::Scan", 'iscsiscan', ['portal'], { node => $nodename },
$print_api_result, $PVE::RESTHandler::standard_output_options ],
- lvm => [ "PVE::API2::Storage::Scan", 'lvmscan', [], { node => $nodename },
+ lvm => [ "${API}::Scan", 'lvmscan', [], { node => $nodename },
$print_api_result, $PVE::RESTHandler::standard_output_options ],
- lvmthin => [ "PVE::API2::Storage::Scan", 'lvmthinscan', ['vg'], { node => $nodename },
+ lvmthin => [ "${API}::Scan", 'lvmthinscan', ['vg'], { node => $nodename },
$print_api_result, $PVE::RESTHandler::standard_output_options ],
pbs => [
- "PVE::API2::Storage::Scan",
+ "${API}::Scan",
'pbsscan',
['server', 'username'],
{ node => $nodename },
$print_api_result,
$PVE::RESTHandler::standard_output_options,
],
- zfs => [ "PVE::API2::Storage::Scan", 'zfsscan', [], { node => $nodename },
+ zfs => [ "${API}::Scan", 'zfsscan', [], { node => $nodename },
$print_api_result, $PVE::RESTHandler::standard_output_options ],
},
nfsscan => { alias => 'scan nfs' },
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result
2021-10-21 12:49 [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result Lorenz Stechauner
2021-10-21 12:49 ` [pve-devel] [PATCH v3 storage 2/2] pvesm: using $API instead of PVE::API2::Storage Lorenz Stechauner
@ 2021-10-21 12:50 ` Lorenz Stechauner
2021-10-25 14:17 ` Dominik Csapak
2 siblings, 0 replies; 5+ messages in thread
From: Lorenz Stechauner @ 2021-10-21 12:50 UTC (permalink / raw)
To: Proxmox VE development discussion
On 21.10.21 14:49, Lorenz Stechauner wrote:
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
changes to v2:
* ordering rows for pvesm {status|list}
> PVE/CLI/pvesm.pm | 158 +++++++++--------------------------------------
> 1 file changed, 30 insertions(+), 128 deletions(-)
>
> diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
> index 190de91..94847cd 100755
> --- a/PVE/CLI/pvesm.pm
> +++ b/PVE/CLI/pvesm.pm
> @@ -178,68 +178,6 @@ __PACKAGE__->register_method ({
> return;
> }});
>
> -my $print_content = sub {
> - my ($list) = @_;
> -
> - my ($maxlenname, $maxsize) = (0, 0);
> - 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;
> - }
> - my $sizemaxdigits = length($maxsize);
> -
> - my $basefmt = "%-${maxlenname}s %-7s %-9s %${sizemaxdigits}s";
> - printf "$basefmt %s\n", "Volid", "Format", "Type", "Size", "VMID";
> -
> - foreach my $info (@$list) {
> - next if !$info->{vmid};
> - my $volid = $info->{volid};
> -
> - printf "$basefmt %d\n", $volid, $info->{format}, $info->{content}, $info->{size}, $info->{vmid};
> - }
> -
> - foreach my $info (sort { $a->{format} cmp $b->{format} } @$list) {
> - next if $info->{vmid};
> - my $volid = $info->{volid};
> -
> - printf "$basefmt\n", $volid, $info->{format}, $info->{content}, $info->{size};
> - }
> -};
> -
> -my $print_status = sub {
> - my $res = shift;
> -
> - my $maxlen = 0;
> - foreach my $res (@$res) {
> - my $storeid = $res->{storage};
> - $maxlen = length ($storeid) if length ($storeid) > $maxlen;
> - }
> - $maxlen+=1;
> -
> - printf "%-${maxlen}s %10s %10s %15s %15s %15s %8s\n", 'Name', 'Type',
> - 'Status', 'Total', 'Used', 'Available', '%';
> -
> - foreach my $res (sort { $a->{storage} cmp $b->{storage} } @$res) {
> - my $storeid = $res->{storage};
> -
> - my $active = $res->{active} ? 'active' : 'inactive';
> - my ($per, $per_fmt) = (0, '% 7.2f%%');
> - $per = ($res->{used}*100)/$res->{total} if $res->{total} > 0;
> -
> - if (!$res->{enabled}) {
> - $per = 'N/A';
> - $per_fmt = '% 8s';
> - $active = 'disabled';
> - }
> -
> - printf "%-${maxlen}s %10s %10s %15d %15d %15d $per_fmt\n", $storeid,
> - $res->{type}, $active, $res->{total}/1024, $res->{used}/1024,
> - $res->{avail}/1024, $per;
> - }
> -};
> -
> __PACKAGE__->register_method ({
> name => 'export',
> path => 'export',
> @@ -573,14 +511,26 @@ my $print_api_result = sub {
> PVE::CLIFormatter::print_api_result($data, $schema, undef, $options);
> };
>
> +my $print_content = sub {
> + my ($data, $schema, $options) = @_;
> + my $order = [qw(volid format content size used vmid ctime parent encrypted verification notes)];
> + PVE::CLIFormatter::print_api_result($data, $schema, $order, $options);
> +};
> +
> +my $print_status = sub {
> + my ($data, $schema, $options) = @_;
> + my $order = [qw(storage type active enabled shared total used avail used_fraction content)];
> + PVE::CLIFormatter::print_api_result($data, $schema, $order, $options);
> +};
> +
> our $cmddef = {
> add => [ "PVE::API2::Storage::Config", 'create', ['type', 'storage'] ],
> set => [ "PVE::API2::Storage::Config", 'update', ['storage'] ],
> remove => [ "PVE::API2::Storage::Config", 'delete', ['storage'] ],
> - status => [ "PVE::API2::Storage::Status", 'index', [],
> - { node => $nodename }, $print_status ],
> - list => [ "PVE::API2::Storage::Content", 'index', ['storage'],
> - { node => $nodename }, $print_content ],
> + status => [ "PVE::API2::Storage::Status", 'index', [], { node => $nodename },
> + $print_status, $PVE::RESTHandler::standard_output_options ],
> + list => [ "PVE::API2::Storage::Content", 'index', ['storage'], { node => $nodename },
> + $print_content, $PVE::RESTHandler::standard_output_options ],
> alloc => [ "PVE::API2::Storage::Content", 'create', ['storage', 'vmid', 'filename', 'size'],
> { node => $nodename }, sub {
> my $volid = shift;
> @@ -589,61 +539,18 @@ our $cmddef = {
> free => [ "PVE::API2::Storage::Content", 'delete', ['volume'],
> { node => $nodename } ],
> scan => {
> - nfs => [ "PVE::API2::Storage::Scan", 'nfsscan', ['server'], { node => $nodename }, sub {
> - my $res = shift;
> -
> - my $maxlen = 0;
> - foreach my $rec (@$res) {
> - my $len = length ($rec->{path});
> - $maxlen = $len if $len > $maxlen;
> - }
> - foreach my $rec (@$res) {
> - printf "%-${maxlen}s %s\n", $rec->{path}, $rec->{options};
> - }
> - }],
> - cifs => [ "PVE::API2::Storage::Scan", 'cifsscan', ['server'], { node => $nodename }, sub {
> - my $res = shift;
> -
> - my $maxlen = 0;
> - foreach my $rec (@$res) {
> - my $len = length ($rec->{share});
> - $maxlen = $len if $len > $maxlen;
> - }
> - foreach my $rec (@$res) {
> - printf "%-${maxlen}s %s\n", $rec->{share}, $rec->{description};
> - }
> - }],
> - glusterfs => [ "PVE::API2::Storage::Scan", 'glusterfsscan', ['server'], { node => $nodename }, sub {
> - my $res = shift;
> -
> - foreach my $rec (@$res) {
> - printf "%s\n", $rec->{volname};
> - }
> - }],
> - iscsi => [ "PVE::API2::Storage::Scan", 'iscsiscan', ['portal'], { node => $nodename }, sub {
> - my $res = shift;
> -
> - my $maxlen = 0;
> - foreach my $rec (@$res) {
> - my $len = length ($rec->{target});
> - $maxlen = $len if $len > $maxlen;
> - }
> - foreach my $rec (@$res) {
> - printf "%-${maxlen}s %s\n", $rec->{target}, $rec->{portal};
> - }
> - }],
> - lvm => [ "PVE::API2::Storage::Scan", 'lvmscan', [], { node => $nodename }, sub {
> - my $res = shift;
> - foreach my $rec (@$res) {
> - printf "$rec->{vg}\n";
> - }
> - }],
> - lvmthin => [ "PVE::API2::Storage::Scan", 'lvmthinscan', ['vg'], { node => $nodename }, sub {
> - my $res = shift;
> - foreach my $rec (@$res) {
> - printf "$rec->{lv}\n";
> - }
> - }],
> + nfs => [ "PVE::API2::Storage::Scan", 'nfsscan', ['server'], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> + cifs => [ "PVE::API2::Storage::Scan", 'cifsscan', ['server'], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> + glusterfs => [ "PVE::API2::Storage::Scan", 'glusterfsscan', ['server'], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> + iscsi => [ "PVE::API2::Storage::Scan", 'iscsiscan', ['portal'], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> + lvm => [ "PVE::API2::Storage::Scan", 'lvmscan', [], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> + lvmthin => [ "PVE::API2::Storage::Scan", 'lvmthinscan', ['vg'], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> pbs => [
> "PVE::API2::Storage::Scan",
> 'pbsscan',
> @@ -652,13 +559,8 @@ our $cmddef = {
> $print_api_result,
> $PVE::RESTHandler::standard_output_options,
> ],
> - zfs => [ "PVE::API2::Storage::Scan", 'zfsscan', [], { node => $nodename }, sub {
> - my $res = shift;
> -
> - foreach my $rec (@$res) {
> - printf "$rec->{pool}\n";
> - }
> - }],
> + zfs => [ "PVE::API2::Storage::Scan", 'zfsscan', [], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> },
> nfsscan => { alias => 'scan nfs' },
> cifsscan => { alias => 'scan cifs' },
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result
2021-10-21 12:49 [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result Lorenz Stechauner
2021-10-21 12:49 ` [pve-devel] [PATCH v3 storage 2/2] pvesm: using $API instead of PVE::API2::Storage Lorenz Stechauner
2021-10-21 12:50 ` [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result Lorenz Stechauner
@ 2021-10-25 14:17 ` Dominik Csapak
2021-10-27 9:55 ` Thomas Lamprecht
2 siblings, 1 reply; 5+ messages in thread
From: Dominik Csapak @ 2021-10-25 14:17 UTC (permalink / raw)
To: Proxmox VE development discussion, Lorenz Stechauner
both patches LGTM, commit message could have used some more info
(like which fields were added, etc.) but no big problem
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak<d.csapak@proxmox.com>
On 10/21/21 14:49, Lorenz Stechauner wrote:
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
> PVE/CLI/pvesm.pm | 158 +++++++++--------------------------------------
> 1 file changed, 30 insertions(+), 128 deletions(-)
>
> diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
> index 190de91..94847cd 100755
> --- a/PVE/CLI/pvesm.pm
> +++ b/PVE/CLI/pvesm.pm
> @@ -178,68 +178,6 @@ __PACKAGE__->register_method ({
> return;
> }});
>
> -my $print_content = sub {
> - my ($list) = @_;
> -
> - my ($maxlenname, $maxsize) = (0, 0);
> - 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;
> - }
> - my $sizemaxdigits = length($maxsize);
> -
> - my $basefmt = "%-${maxlenname}s %-7s %-9s %${sizemaxdigits}s";
> - printf "$basefmt %s\n", "Volid", "Format", "Type", "Size", "VMID";
> -
> - foreach my $info (@$list) {
> - next if !$info->{vmid};
> - my $volid = $info->{volid};
> -
> - printf "$basefmt %d\n", $volid, $info->{format}, $info->{content}, $info->{size}, $info->{vmid};
> - }
> -
> - foreach my $info (sort { $a->{format} cmp $b->{format} } @$list) {
> - next if $info->{vmid};
> - my $volid = $info->{volid};
> -
> - printf "$basefmt\n", $volid, $info->{format}, $info->{content}, $info->{size};
> - }
> -};
> -
> -my $print_status = sub {
> - my $res = shift;
> -
> - my $maxlen = 0;
> - foreach my $res (@$res) {
> - my $storeid = $res->{storage};
> - $maxlen = length ($storeid) if length ($storeid) > $maxlen;
> - }
> - $maxlen+=1;
> -
> - printf "%-${maxlen}s %10s %10s %15s %15s %15s %8s\n", 'Name', 'Type',
> - 'Status', 'Total', 'Used', 'Available', '%';
> -
> - foreach my $res (sort { $a->{storage} cmp $b->{storage} } @$res) {
> - my $storeid = $res->{storage};
> -
> - my $active = $res->{active} ? 'active' : 'inactive';
> - my ($per, $per_fmt) = (0, '% 7.2f%%');
> - $per = ($res->{used}*100)/$res->{total} if $res->{total} > 0;
> -
> - if (!$res->{enabled}) {
> - $per = 'N/A';
> - $per_fmt = '% 8s';
> - $active = 'disabled';
> - }
> -
> - printf "%-${maxlen}s %10s %10s %15d %15d %15d $per_fmt\n", $storeid,
> - $res->{type}, $active, $res->{total}/1024, $res->{used}/1024,
> - $res->{avail}/1024, $per;
> - }
> -};
> -
> __PACKAGE__->register_method ({
> name => 'export',
> path => 'export',
> @@ -573,14 +511,26 @@ my $print_api_result = sub {
> PVE::CLIFormatter::print_api_result($data, $schema, undef, $options);
> };
>
> +my $print_content = sub {
> + my ($data, $schema, $options) = @_;
> + my $order = [qw(volid format content size used vmid ctime parent encrypted verification notes)];
> + PVE::CLIFormatter::print_api_result($data, $schema, $order, $options);
> +};
> +
> +my $print_status = sub {
> + my ($data, $schema, $options) = @_;
> + my $order = [qw(storage type active enabled shared total used avail used_fraction content)];
> + PVE::CLIFormatter::print_api_result($data, $schema, $order, $options);
> +};
> +
> our $cmddef = {
> add => [ "PVE::API2::Storage::Config", 'create', ['type', 'storage'] ],
> set => [ "PVE::API2::Storage::Config", 'update', ['storage'] ],
> remove => [ "PVE::API2::Storage::Config", 'delete', ['storage'] ],
> - status => [ "PVE::API2::Storage::Status", 'index', [],
> - { node => $nodename }, $print_status ],
> - list => [ "PVE::API2::Storage::Content", 'index', ['storage'],
> - { node => $nodename }, $print_content ],
> + status => [ "PVE::API2::Storage::Status", 'index', [], { node => $nodename },
> + $print_status, $PVE::RESTHandler::standard_output_options ],
> + list => [ "PVE::API2::Storage::Content", 'index', ['storage'], { node => $nodename },
> + $print_content, $PVE::RESTHandler::standard_output_options ],
> alloc => [ "PVE::API2::Storage::Content", 'create', ['storage', 'vmid', 'filename', 'size'],
> { node => $nodename }, sub {
> my $volid = shift;
> @@ -589,61 +539,18 @@ our $cmddef = {
> free => [ "PVE::API2::Storage::Content", 'delete', ['volume'],
> { node => $nodename } ],
> scan => {
> - nfs => [ "PVE::API2::Storage::Scan", 'nfsscan', ['server'], { node => $nodename }, sub {
> - my $res = shift;
> -
> - my $maxlen = 0;
> - foreach my $rec (@$res) {
> - my $len = length ($rec->{path});
> - $maxlen = $len if $len > $maxlen;
> - }
> - foreach my $rec (@$res) {
> - printf "%-${maxlen}s %s\n", $rec->{path}, $rec->{options};
> - }
> - }],
> - cifs => [ "PVE::API2::Storage::Scan", 'cifsscan', ['server'], { node => $nodename }, sub {
> - my $res = shift;
> -
> - my $maxlen = 0;
> - foreach my $rec (@$res) {
> - my $len = length ($rec->{share});
> - $maxlen = $len if $len > $maxlen;
> - }
> - foreach my $rec (@$res) {
> - printf "%-${maxlen}s %s\n", $rec->{share}, $rec->{description};
> - }
> - }],
> - glusterfs => [ "PVE::API2::Storage::Scan", 'glusterfsscan', ['server'], { node => $nodename }, sub {
> - my $res = shift;
> -
> - foreach my $rec (@$res) {
> - printf "%s\n", $rec->{volname};
> - }
> - }],
> - iscsi => [ "PVE::API2::Storage::Scan", 'iscsiscan', ['portal'], { node => $nodename }, sub {
> - my $res = shift;
> -
> - my $maxlen = 0;
> - foreach my $rec (@$res) {
> - my $len = length ($rec->{target});
> - $maxlen = $len if $len > $maxlen;
> - }
> - foreach my $rec (@$res) {
> - printf "%-${maxlen}s %s\n", $rec->{target}, $rec->{portal};
> - }
> - }],
> - lvm => [ "PVE::API2::Storage::Scan", 'lvmscan', [], { node => $nodename }, sub {
> - my $res = shift;
> - foreach my $rec (@$res) {
> - printf "$rec->{vg}\n";
> - }
> - }],
> - lvmthin => [ "PVE::API2::Storage::Scan", 'lvmthinscan', ['vg'], { node => $nodename }, sub {
> - my $res = shift;
> - foreach my $rec (@$res) {
> - printf "$rec->{lv}\n";
> - }
> - }],
> + nfs => [ "PVE::API2::Storage::Scan", 'nfsscan', ['server'], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> + cifs => [ "PVE::API2::Storage::Scan", 'cifsscan', ['server'], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> + glusterfs => [ "PVE::API2::Storage::Scan", 'glusterfsscan', ['server'], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> + iscsi => [ "PVE::API2::Storage::Scan", 'iscsiscan', ['portal'], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> + lvm => [ "PVE::API2::Storage::Scan", 'lvmscan', [], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> + lvmthin => [ "PVE::API2::Storage::Scan", 'lvmthinscan', ['vg'], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> pbs => [
> "PVE::API2::Storage::Scan",
> 'pbsscan',
> @@ -652,13 +559,8 @@ our $cmddef = {
> $print_api_result,
> $PVE::RESTHandler::standard_output_options,
> ],
> - zfs => [ "PVE::API2::Storage::Scan", 'zfsscan', [], { node => $nodename }, sub {
> - my $res = shift;
> -
> - foreach my $rec (@$res) {
> - printf "$rec->{pool}\n";
> - }
> - }],
> + zfs => [ "PVE::API2::Storage::Scan", 'zfsscan', [], { node => $nodename },
> + $print_api_result, $PVE::RESTHandler::standard_output_options ],
> },
> nfsscan => { alias => 'scan nfs' },
> cifsscan => { alias => 'scan cifs' },
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result
2021-10-25 14:17 ` Dominik Csapak
@ 2021-10-27 9:55 ` Thomas Lamprecht
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2021-10-27 9:55 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak, Lorenz Stechauner
On 25/10/2021 16:17, Dominik Csapak wrote:
> both patches LGTM, commit message could have used some more info
> (like which fields were added, etc.) but no big problem
yeah, mentioning that instead of three or so selected columns all are now
dumped and some reasoning for doing that (after all #3633 did not asked for that)
would be good ;)
Would be nice to reduce that a bit for the default output, ideally the content
list would made dependent of the content type passed, as `used`, `vmid`, `parent`,
`encrypted` and `verification` do not make much sense for `--content vztmpl`.
The headers are now all the internal property, `used_fraction` does not read
to nicely for a CLI tool human readable output IMO (would be naturally Ok for
the machine readable JSON one).
There's also ctime in content list that isn't printed more human friendly, which
would be the point of #3633,
And we really need to reverse the default for (no)border, as the content list
for vztmpl on the same storage here produced 72 lines previously (easily fitting
on one console page here) and now spews out 145 lines (which needs scrolling),
without adding really any value and making copying not exactly easier, but at
least that part isn't the job of this patch series.
> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
> Tested-by: Dominik Csapak<d.csapak@proxmox.com>
thanks, it works as in breaks not something, but cannot say as advertised as it
does quite a bit more than the report requested, which can be fine but would need
some commit message to state at least some reasoning or thoughts on it.
I'm open to take it in now with at least the columns being the same as previously,
fancy auto-column selection and the border stuff can be done later/independent of
this.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-27 9:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21 12:49 [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result Lorenz Stechauner
2021-10-21 12:49 ` [pve-devel] [PATCH v3 storage 2/2] pvesm: using $API instead of PVE::API2::Storage Lorenz Stechauner
2021-10-21 12:50 ` [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result Lorenz Stechauner
2021-10-25 14:17 ` Dominik Csapak
2021-10-27 9:55 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox