public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions
@ 2021-01-26 11:45 Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 01/14] Disks: return correct journal disk candidates Fabian Ebner
                   ` (14 more replies)
  0 siblings, 15 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

and fix some other little things along the way (see patches #1, #4, #9).

Mostly re-uses existing functionality, but refactors it, so it can be re-used
for partitions as well. New is the detection of filesystems via lsblk and the
detection of more PVE-relevant partuuids.

In the UI, the current list is replaced by a treepanel including the partitions.

This series does not yet make it possible to select partitons for storage
creation for certain types, which is one of the parts of bug #2285. I felt
like this series was getting rather big already and that there was more
discussion to be had about that and it can always be done as a follow-up series.
The usage of a partition currently always defaults to 'partition'. A good
heuristic to find out when a partition is unused would be needed and/or good
ways to warn the user and ask if they're sure they have the correct partition.
This also ties in to the proposed feature of allowing users to wipe disks under
certain conditions.


storage:

Fabian Ebner (11):
  Disks: return correct journal disk candidates
  Diskmanage: replace closure with direct hash access
  Diskmanage: refactor and rename get_parttype_info
  Diskmanage: also check for filesystem type when determining usage
  Diskmanage: introduce get_sysdir_size helper
  Diskmanage: collect partitions in hash
  Diskmanage: introduce usage helper
  Diskmanage: also detect BIOS boot, EFI and ZFS reserved type
    partitions
  Diskmanage: introduce ceph info helper
  Diskmanage: save OSD information for individual partitions
  Diskmanage: also include partitions with get_disks if flag is set

 PVE/API2/Disks.pm                             |  24 +-
 PVE/Diskmanage.pm                             | 207 +++++++++++-------
 test/disk_tests/usages/disklist               |   1 +
 test/disk_tests/usages/disklist_expected.json |  15 ++
 test/disk_tests/usages/lsblk                  |   7 +-
 test/disk_tests/usages/sdd/sdd1/size          |   1 +
 test/disk_tests/usages/sdd/sdd2/size          |   1 +
 test/disk_tests/usages/sde/sde1/size          |   1 +
 test/disk_tests/usages/sdf/sdf1/size          |   1 +
 test/disk_tests/usages/sdm/sdm1/size          |   1 +
 test/disk_tests/usages/sdm/sdm9/size          |   1 +
 test/disk_tests/usages/sdn/device/vendor      |   1 +
 test/disk_tests/usages/sdn/queue/rotational   |   1 +
 test/disk_tests/usages/sdn/size               |   1 +
 test/disk_tests/usages/sdn_udevadm            |  14 ++
 test/disklist_test.pm                         |  12 +
 16 files changed, 207 insertions(+), 82 deletions(-)
 create mode 100644 test/disk_tests/usages/sdd/sdd1/size
 create mode 100644 test/disk_tests/usages/sdd/sdd2/size
 create mode 100644 test/disk_tests/usages/sde/sde1/size
 create mode 100644 test/disk_tests/usages/sdf/sdf1/size
 create mode 100644 test/disk_tests/usages/sdm/sdm1/size
 create mode 100644 test/disk_tests/usages/sdm/sdm9/size
 create mode 100644 test/disk_tests/usages/sdn/device/vendor
 create mode 100644 test/disk_tests/usages/sdn/queue/rotational
 create mode 100644 test/disk_tests/usages/sdn/size
 create mode 100644 test/disk_tests/usages/sdn_udevadm


manager:

Fabian Ebner (1):
  api: Ceph: add reminder to remove 'disks' call

 PVE/API2/Ceph.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


widget-toolkit:

Fabian Ebner (2):
  convert disk list to disk tree including the partitions
  move DiskList.js from grid/ to panel/

 src/Makefile                    |  2 +-
 src/{grid => panel}/DiskList.js | 97 ++++++++++++++++++++++++++-------
 2 files changed, 79 insertions(+), 20 deletions(-)
 rename src/{grid => panel}/DiskList.js (72%)

-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH storage 01/14] Disks: return correct journal disk candidates
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 02/14] Diskmanage: replace closure with direct hash access Fabian Ebner
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

Previously any GPT initialized disk without an osdid (i.e. equal to -1) would
be included in the list of journal disk candidates, for example a ZFS disk. But
the OSD creation API call will fail for those. To fix it, re-use the condition
from the corresponding check in that API call (in PVE/API2/Ceph/OSD.pm).
Now, included disks are unused disks, those with usage 'partitions' and GPT, and
those with usage 'LVM'.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Disks.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Disks.pm b/PVE/API2/Disks.pm
index 807e9b2..c0456be 100644
--- a/PVE/API2/Disks.pm
+++ b/PVE/API2/Disks.pm
@@ -137,7 +137,10 @@ __PACKAGE__->register_method ({
 	    my $entry = $disks->{$disk};
 	    if ($type eq 'journal_disks') {
 		next if $entry->{osdid} >= 0;
-		next if !($entry->{gpt} || !$entry->{used} || $entry->{used} eq 'LVM');
+		if (my $usage = $entry->{used}) {
+		    next if !($usage eq 'partitions' && $entry->{gpt}
+			|| $usage eq 'LVM');
+		}
 	    } elsif ($type eq 'unused') {
 		next if $entry->{used};
 	    } elsif ($type ne '') {
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH storage 02/14] Diskmanage: replace closure with direct hash access
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 01/14] Disks: return correct journal disk candidates Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 03/14] Diskmanage: refactor and rename get_parttype_info Fabian Ebner
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Diskmanage.pm | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 116a99a..f28e007 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -480,11 +480,6 @@ sub get_disks {
 	$mounted->{abs_path($mount->[0])} = $mount->[1];
     };
 
-    my $dev_is_mounted = sub {
-	my ($dev) = @_;
-	return $mounted->{$dev};
-    };
-
     my $parttype_map = get_parttype_info();
 
     my $journalhash = get_ceph_journals($parttype_map);
@@ -567,7 +562,7 @@ sub get_disks {
 
 	$used = 'LVM' if $lvmhash->{$devpath};
 
-	$used = 'mounted' if &$dev_is_mounted($devpath);
+	$used = 'mounted' if $mounted->{$devpath};
 
 	$used = 'ZFS' if $zfshash->{$devpath};
 
@@ -621,7 +616,7 @@ sub get_disks {
 
 	    $found_partitions = 1;
 
-	    if (my $mp = &$dev_is_mounted("$partpath/$part")) {
+	    if (my $mp = $mounted->{"$partpath/$part"}) {
 		$found_mountpoints = 1;
 		if ($mp =~ m|^/var/lib/ceph/osd/ceph-(\d+)$|) {
 		    $osdid = $1;
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH storage 03/14] Diskmanage: refactor and rename get_parttype_info
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 01/14] Disks: return correct journal disk candidates Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 02/14] Diskmanage: replace closure with direct hash access Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 04/14] Diskmanage: also check for filesystem type when determining usage Fabian Ebner
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

in preparation to also query the file system type from lsblk. Note that the
result now also includes devices without a parttype, so a definedness check in
get_devices_by_partuuid is needed. This will be useful when the whole device
contains a filesystem.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Diskmanage.pm | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index f28e007..e20484b 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -157,7 +157,7 @@ sub get_smart_data {
     return $smartdata;
 }
 
-sub get_parttype_info() {
+sub get_lsblk_info() {
     my $cmd = [$LSBLK, '--json', '-o', 'path,parttype'];
     my $output = "";
     my $res = {};
@@ -174,30 +174,29 @@ sub get_parttype_info() {
     warn "$@\n" if $@;
     my $list = $parsed->{blockdevices} // [];
 
-    foreach my $dev (@$list) {
-	next if !($dev->{parttype});
-	my $type = $dev->{parttype};
-	$res->{$type} = [] if !defined($res->{$type});
-	push @{$res->{$type}}, $dev->{path};
-    }
+    $res = { map {
+	$_->{path} => { parttype => $_->{parttype} }
+    } @{$list} };
 
     return $res;
 }
 
 my $get_devices_by_partuuid = sub {
-    my ($parttype_map, $uuids, $res) = @_;
+    my ($lsblk_info, $uuids, $res) = @_;
 
     $res = {} if !defined($res);
 
-    foreach my $uuid (sort keys %$uuids) {
-       map { $res->{$_} = $uuids->{$uuid} } @{$parttype_map->{$uuid}};
+    foreach my $dev (sort keys %{$lsblk_info}) {
+	my $uuid = $lsblk_info->{$dev}->{parttype};
+	next if !defined($uuid) || !defined($uuids->{$uuid});
+	$res->{$dev} = $uuids->{$uuid};
     }
 
     return $res;
 };
 
 sub get_zfs_devices {
-    my ($parttype_map) = @_;
+    my ($lsblk_info) = @_;
     my $res = {};
 
     return {} if ! -x $ZPOOL;
@@ -225,13 +224,13 @@ sub get_zfs_devices {
     };
 
 
-    $res = $get_devices_by_partuuid->($parttype_map, $uuids, $res);
+    $res = $get_devices_by_partuuid->($lsblk_info, $uuids, $res);
 
     return $res;
 }
 
 sub get_lvm_devices {
-    my ($parttype_map) = @_;
+    my ($lsblk_info) = @_;
     my $res = {};
     eval {
 	run_command([$PVS, '--noheadings', '--readonly', '-o', 'pv_name'], outfunc => sub{
@@ -251,13 +250,13 @@ sub get_lvm_devices {
 	"e6d6d379-f507-44c2-a23c-238f2a3df928" => 1,
     };
 
-    $res = $get_devices_by_partuuid->($parttype_map, $uuids, $res);
+    $res = $get_devices_by_partuuid->($lsblk_info, $uuids, $res);
 
     return $res;
 }
 
 sub get_ceph_journals {
-    my ($parttype_map) = @_;
+    my ($lsblk_info) = @_;
     my $res = {};
 
     my $uuids = {
@@ -267,7 +266,7 @@ sub get_ceph_journals {
 	'cafecafe-9b03-4f30-b4c6-b4b80ceff106' => 4, # block
     };
 
-    $res = $get_devices_by_partuuid->($parttype_map, $uuids, $res);
+    $res = $get_devices_by_partuuid->($lsblk_info, $uuids, $res);
 
     return $res;
 }
@@ -480,14 +479,14 @@ sub get_disks {
 	$mounted->{abs_path($mount->[0])} = $mount->[1];
     };
 
-    my $parttype_map = get_parttype_info();
+    my $lsblk_info = get_lsblk_info();
 
-    my $journalhash = get_ceph_journals($parttype_map);
+    my $journalhash = get_ceph_journals($lsblk_info);
     my $ceph_volume_infos = get_ceph_volume_infos();
 
-    my $zfshash = get_zfs_devices($parttype_map);
+    my $zfshash = get_zfs_devices($lsblk_info);
 
-    my $lvmhash = get_lvm_devices($parttype_map);
+    my $lvmhash = get_lvm_devices($lsblk_info);
 
     my $disk_regex = ".*";
     if (defined($disks)) {
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH storage 04/14] Diskmanage: also check for filesystem type when determining usage
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (2 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 03/14] Diskmanage: refactor and rename get_parttype_info Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 05/14] Diskmanage: introduce get_sysdir_size helper Fabian Ebner
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

Like this, a non-ZFS filesystem living on a whole disk will also be detected
when it is not mounted.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Diskmanage.pm                             | 15 +++++++++++++--
 test/disk_tests/usages/disklist               |  1 +
 test/disk_tests/usages/disklist_expected.json | 15 +++++++++++++++
 test/disk_tests/usages/lsblk                  |  7 ++++---
 test/disk_tests/usages/sdn/device/vendor      |  1 +
 test/disk_tests/usages/sdn/queue/rotational   |  1 +
 test/disk_tests/usages/sdn/size               |  1 +
 test/disk_tests/usages/sdn_udevadm            | 14 ++++++++++++++
 8 files changed, 50 insertions(+), 5 deletions(-)
 create mode 100644 test/disk_tests/usages/sdn/device/vendor
 create mode 100644 test/disk_tests/usages/sdn/queue/rotational
 create mode 100644 test/disk_tests/usages/sdn/size
 create mode 100644 test/disk_tests/usages/sdn_udevadm

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index e20484b..5672d0f 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -158,7 +158,7 @@ sub get_smart_data {
 }
 
 sub get_lsblk_info() {
-    my $cmd = [$LSBLK, '--json', '-o', 'path,parttype'];
+    my $cmd = [$LSBLK, '--json', '-o', 'path,parttype,fstype'];
     my $output = "";
     my $res = {};
     eval {
@@ -175,7 +175,10 @@ sub get_lsblk_info() {
     my $list = $parsed->{blockdevices} // [];
 
     $res = { map {
-	$_->{path} => { parttype => $_->{parttype} }
+	$_->{path} => {
+	    parttype => $_->{parttype},
+	    fstype => $_->{fstype}
+	}
     } @{$list} };
 
     return $res;
@@ -565,6 +568,14 @@ sub get_disks {
 
 	$used = 'ZFS' if $zfshash->{$devpath};
 
+	if (defined($lsblk_info->{$devpath})) {
+	    my $fstype = $lsblk_info->{$devpath}->{fstype};
+	    if (defined($fstype)) {
+		$used = $fstype;
+		$used .= ' (mounted)' if $mounted->{$devpath};
+	    }
+	}
+
 	# we replaced cciss/ with cciss! above
 	# but in the result we need cciss/ again
 	# because the caller might want to check the
diff --git a/test/disk_tests/usages/disklist b/test/disk_tests/usages/disklist
index 92c3622..648bea5 100644
--- a/test/disk_tests/usages/disklist
+++ b/test/disk_tests/usages/disklist
@@ -11,3 +11,4 @@ sdj
 sdk
 sdl
 sdm
+sdn
diff --git a/test/disk_tests/usages/disklist_expected.json b/test/disk_tests/usages/disklist_expected.json
index b2e37c0..93dc251 100644
--- a/test/disk_tests/usages/disklist_expected.json
+++ b/test/disk_tests/usages/disklist_expected.json
@@ -203,5 +203,20 @@
 	"rpm" : 0,
 	"type" : "hdd",
 	"osdid" : -1
+    },
+    "sdn" : {
+	"serial" : "SERIAL1",
+	"vendor" : "ATA",
+	"wwn" : "0x0000000000000000",
+	"devpath" : "/dev/sdn",
+	"model" : "MODEL1",
+	"used" : "xfs",
+	"wearout" : "N/A",
+	"health" : "UNKNOWN",
+	"gpt" : 0,
+	"size" : 1536000,
+	"rpm" : 0,
+	"type" : "hdd",
+	"osdid" : -1
     }
 }
diff --git a/test/disk_tests/usages/lsblk b/test/disk_tests/usages/lsblk
index cbb18b9..6a725ab 100644
--- a/test/disk_tests/usages/lsblk
+++ b/test/disk_tests/usages/lsblk
@@ -1,7 +1,8 @@
 {
    "blockdevices": [
-      {"path":"/dev/sdm", "parttype":null},
-      {"path":"/dev/sdm1", "parttype":"6a898cc3-1dd2-11b2-99a6-080020736631"},
-      {"path":"/dev/sdm9", "parttype":"6a945a3b-1dd2-11b2-99a6-080020736631"}
+      {"path":"/dev/sdm", "parttype":null, "fstype":null},
+      {"path":"/dev/sdm1", "parttype":"6a898cc3-1dd2-11b2-99a6-080020736631", "fstype":"zfs_member"},
+      {"path":"/dev/sdm9", "parttype":"6a945a3b-1dd2-11b2-99a6-080020736631", "fstype":null},
+      {"path":"/dev/sdn", "parttype":null, "fstype":"xfs"}
    ]
 }
diff --git a/test/disk_tests/usages/sdn/device/vendor b/test/disk_tests/usages/sdn/device/vendor
new file mode 100644
index 0000000..531030d
--- /dev/null
+++ b/test/disk_tests/usages/sdn/device/vendor
@@ -0,0 +1 @@
+ATA
diff --git a/test/disk_tests/usages/sdn/queue/rotational b/test/disk_tests/usages/sdn/queue/rotational
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/test/disk_tests/usages/sdn/queue/rotational
@@ -0,0 +1 @@
+1
diff --git a/test/disk_tests/usages/sdn/size b/test/disk_tests/usages/sdn/size
new file mode 100644
index 0000000..13de30f
--- /dev/null
+++ b/test/disk_tests/usages/sdn/size
@@ -0,0 +1 @@
+3000
diff --git a/test/disk_tests/usages/sdn_udevadm b/test/disk_tests/usages/sdn_udevadm
new file mode 100644
index 0000000..5ec4a92
--- /dev/null
+++ b/test/disk_tests/usages/sdn_udevadm
@@ -0,0 +1,14 @@
+E: DEVNAME=/dev/sdn
+E: DEVTYPE=disk
+E: ID_ATA_ROTATION_RATE_RPM=0
+E: ID_BUS=ata
+E: ID_MODEL=MODEL1
+E: ID_SERIAL=SERIAL1
+E: ID_SERIAL_SHORT=SERIAL1
+E: ID_TYPE=disk
+E: ID_FS_UUID=ab54fba8-48fe-4d37-bbe7-b403f94d3bed
+E: ID_FS_UUID_ENC=ab54fba8-48fe-4d37-bbe7-b403f94d3bed
+E: ID_FS_TYPE=xfs
+E: ID_FS_USAGE=filesystem
+E: ID_WWN=0x0000000000000000
+E: ID_WWN_WITH_EXTENSION=0x0000000000000000
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH storage 05/14] Diskmanage: introduce get_sysdir_size helper
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (3 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 04/14] Diskmanage: also check for filesystem type when determining usage Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 06/14] Diskmanage: collect partitions in hash Fabian Ebner
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

to be used for partitions as well.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Diskmanage.pm                    | 18 ++++++++++++------
 test/disk_tests/usages/sdd/sdd1/size |  1 +
 test/disk_tests/usages/sdd/sdd2/size |  1 +
 test/disk_tests/usages/sde/sde1/size |  1 +
 test/disk_tests/usages/sdf/sdf1/size |  1 +
 test/disk_tests/usages/sdm/sdm1/size |  1 +
 test/disk_tests/usages/sdm/sdm9/size |  1 +
 test/disklist_test.pm                | 12 ++++++++++++
 8 files changed, 30 insertions(+), 6 deletions(-)
 create mode 100644 test/disk_tests/usages/sdd/sdd1/size
 create mode 100644 test/disk_tests/usages/sdd/sdd2/size
 create mode 100644 test/disk_tests/usages/sde/sde1/size
 create mode 100644 test/disk_tests/usages/sdf/sdf1/size
 create mode 100644 test/disk_tests/usages/sdm/sdm1/size
 create mode 100644 test/disk_tests/usages/sdm/sdm9/size

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 5672d0f..6d96e77 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -369,6 +369,17 @@ sub get_udev_info {
     return $data;
 }
 
+sub get_sysdir_size {
+    my ($sysdir) = @_;
+
+    my $size = file_read_firstline("$sysdir/size");
+    return if !$size;
+
+    # linux always considers sectors to be 512 bytes,
+    # independently of real block size
+    return $size * 512;
+}
+
 sub get_sysdir_info {
     my ($sysdir) = @_;
 
@@ -376,12 +387,7 @@ sub get_sysdir_info {
 
     my $data = {};
 
-    my $size = file_read_firstline("$sysdir/size");
-    return undef if !$size;
-
-    # linux always considers sectors to be 512 bytes,
-    # independently of real block size
-    $data->{size} = $size * 512;
+    $data->{size} = get_sysdir_size($sysdir) or return;
 
     # dir/queue/rotational should be 1 for hdd, 0 for ssd
     $data->{rotational} = file_read_firstline("$sysdir/queue/rotational") // -1;
diff --git a/test/disk_tests/usages/sdd/sdd1/size b/test/disk_tests/usages/sdd/sdd1/size
new file mode 100644
index 0000000..83b33d2
--- /dev/null
+++ b/test/disk_tests/usages/sdd/sdd1/size
@@ -0,0 +1 @@
+1000
diff --git a/test/disk_tests/usages/sdd/sdd2/size b/test/disk_tests/usages/sdd/sdd2/size
new file mode 100644
index 0000000..8bd1af1
--- /dev/null
+++ b/test/disk_tests/usages/sdd/sdd2/size
@@ -0,0 +1 @@
+2000
diff --git a/test/disk_tests/usages/sde/sde1/size b/test/disk_tests/usages/sde/sde1/size
new file mode 100644
index 0000000..13de30f
--- /dev/null
+++ b/test/disk_tests/usages/sde/sde1/size
@@ -0,0 +1 @@
+3000
diff --git a/test/disk_tests/usages/sdf/sdf1/size b/test/disk_tests/usages/sdf/sdf1/size
new file mode 100644
index 0000000..13de30f
--- /dev/null
+++ b/test/disk_tests/usages/sdf/sdf1/size
@@ -0,0 +1 @@
+3000
diff --git a/test/disk_tests/usages/sdm/sdm1/size b/test/disk_tests/usages/sdm/sdm1/size
new file mode 100644
index 0000000..83b33d2
--- /dev/null
+++ b/test/disk_tests/usages/sdm/sdm1/size
@@ -0,0 +1 @@
+1000
diff --git a/test/disk_tests/usages/sdm/sdm9/size b/test/disk_tests/usages/sdm/sdm9/size
new file mode 100644
index 0000000..8bd1af1
--- /dev/null
+++ b/test/disk_tests/usages/sdm/sdm9/size
@@ -0,0 +1 @@
+2000
diff --git a/test/disklist_test.pm b/test/disklist_test.pm
index 9cb6763..bfce1ea 100644
--- a/test/disklist_test.pm
+++ b/test/disklist_test.pm
@@ -87,6 +87,16 @@ sub mocked_get_sysdir_info {
     return &$originalsub($param);
 }
 
+sub mocked_get_sysdir_size {
+    my ($param) = @_;
+
+    my $originalsub = $diskmanage_module->original('get_sysdir_size');
+
+    $param =~ s|/sys/block|disk_tests/$testcasedir|;
+
+    return &$originalsub($param);
+}
+
 sub mocked_is_iscsi {
     return 0;
 }
@@ -219,6 +229,8 @@ $diskmanage_module->mock('dir_glob_foreach' => \&mocked_dir_glob_foreach);
 print("\tMocked dir_glob_foreach\n");
 $diskmanage_module->mock('get_sysdir_info' => \&mocked_get_sysdir_info);
 print("\tMocked get_sysdir_info\n");
+$diskmanage_module->mock('get_sysdir_size' => \&mocked_get_sysdir_size);
+print("\tMocked get_sysdir_size\n");
 $diskmanage_module->mock('is_iscsi' => \&mocked_is_iscsi);
 print("\tMocked is_iscsi\n");
 $diskmanage_module->mock('assert_blockdev' => sub { return 1; });
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH storage 06/14] Diskmanage: collect partitions in hash
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (4 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 05/14] Diskmanage: introduce get_sysdir_size helper Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 07/14] Diskmanage: introduce usage helper Fabian Ebner
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Diskmanage.pm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 6d96e77..a06f8ae 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -615,7 +615,6 @@ sub get_disks {
 	my $db_count = 0;
 	my $wal_count = 0;
 
-	my $found_partitions;
 	my $found_lvm;
 	my $found_mountpoints;
 	my $found_zfs;
@@ -627,10 +626,15 @@ sub get_disks {
 	# e.g. from /dev/cciss/c0d0 get /dev/cciss
 	$partpath =~ s/\/[^\/]+$//;
 
+	my $partitions = {};
+
 	dir_glob_foreach("$sysdir", "$dev.+", sub {
 	    my ($part) = @_;
 
-	    $found_partitions = 1;
+	    $partitions->{$part}->{devpath} = "$partpath/$part";
+	    $partitions->{$part}->{gpt} = $data->{gpt};
+	    $partitions->{$part}->{size} =
+		get_sysdir_size("$sysdir/$part") // 0;
 
 	    if (my $mp = $mounted->{"$partpath/$part"}) {
 		$found_mountpoints = 1;
@@ -674,7 +678,7 @@ sub get_disks {
 	$used = 'LVM' if $found_lvm && !$used;
 	$used = 'ZFS' if $found_zfs && !$used;
 	$used = 'Device Mapper' if $found_dm && !$used;
-	$used = 'partitions' if $found_partitions && !$used;
+	$used = 'partitions' if scalar(keys %{$partitions}) && !$used;
 
 	# multipath, software raid, etc.
 	# this check comes in last, to show more specific info
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH storage 07/14] Diskmanage: introduce usage helper
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (5 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 06/14] Diskmanage: collect partitions in hash Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 08/14] Diskmanage: also detect BIOS boot, EFI and ZFS reserved type partitions Fabian Ebner
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

Note that this is a slight behavior change, because now the first
partition's usage which is not simply 'partition' will become the disk's
usage. Previously, if any partition was 'mounted', it would become the disk's
usage, then 'LVM', 'ZFS', etc.

A partitions usage defaults to 'partition' if nothing more specific can be
found, and is never treated as unused for now.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Diskmanage.pm | 71 +++++++++++++++++++++--------------------------
 1 file changed, 31 insertions(+), 40 deletions(-)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index a06f8ae..32789f2 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -566,22 +566,6 @@ sub get_disks {
 	    };
 	}
 
-	my $used;
-
-	$used = 'LVM' if $lvmhash->{$devpath};
-
-	$used = 'mounted' if $mounted->{$devpath};
-
-	$used = 'ZFS' if $zfshash->{$devpath};
-
-	if (defined($lsblk_info->{$devpath})) {
-	    my $fstype = $lsblk_info->{$devpath}->{fstype};
-	    if (defined($fstype)) {
-		$used = $fstype;
-		$used .= ' (mounted)' if $mounted->{$devpath};
-	    }
-	}
-
 	# we replaced cciss/ with cciss! above
 	# but in the result we need cciss/ again
 	# because the caller might want to check the
@@ -615,10 +599,6 @@ sub get_disks {
 	my $db_count = 0;
 	my $wal_count = 0;
 
-	my $found_lvm;
-	my $found_mountpoints;
-	my $found_zfs;
-	my $found_dm;
 	my $partpath = $devpath;
 
 	# remove part after last / to
@@ -626,6 +606,28 @@ sub get_disks {
 	# e.g. from /dev/cciss/c0d0 get /dev/cciss
 	$partpath =~ s/\/[^\/]+$//;
 
+	my $determine_usage = sub {
+	    my ($devpath, $sysdir, $is_partition) = @_;
+
+	    return 'LVM' if $lvmhash->{$devpath};
+	    return 'ZFS' if $zfshash->{$devpath};
+
+	    my $info = $lsblk_info->{$devpath} // {};
+	    my $fstype = $info->{fstype};
+	    if (defined($fstype)) {
+		return "${fstype} (mounted)" if $mounted->{$devpath};
+		return "${fstype}";
+	    }
+	    return 'mounted' if $mounted->{$devpath};
+
+	    return if !$is_partition;
+
+	    # for devices, this check is done explicitly later
+	    return 'Device Mapper' if !dir_is_empty("$sysdir/holders");
+
+	    return 'partition';
+	};
+
 	my $partitions = {};
 
 	dir_glob_foreach("$sysdir", "$dev.+", sub {
@@ -635,32 +637,21 @@ sub get_disks {
 	    $partitions->{$part}->{gpt} = $data->{gpt};
 	    $partitions->{$part}->{size} =
 		get_sysdir_size("$sysdir/$part") // 0;
+	    $partitions->{$part}->{used} =
+		$determine_usage->("$partpath/$part", "$sysdir/$part", 1);
 
 	    if (my $mp = $mounted->{"$partpath/$part"}) {
-		$found_mountpoints = 1;
 		if ($mp =~ m|^/var/lib/ceph/osd/ceph-(\d+)$|) {
 		    $osdid = $1;
 		}
 	    }
 
-	    if ($lvmhash->{"$partpath/$part"}) {
-		$found_lvm = 1;
-	    }
-
-	    if ($zfshash->{"$partpath/$part"}) {
-		$found_zfs = 1;
-	    }
-
 	    if (my $journal_part = $journalhash->{"$partpath/$part"}) {
 		$journal_count++ if $journal_part == 1;
 		$db_count++ if $journal_part == 2;
 		$wal_count++ if $journal_part == 3;
 		$bluestore = 1 if $journal_part == 4;
 	    }
-
-	    if (!dir_is_empty("$sysdir/$part/holders") && !$found_lvm)  {
-		$found_dm = 1;
-	    }
 	});
 
 	if (my $ceph_volume = $ceph_volume_infos->{$devpath}) {
@@ -674,16 +665,16 @@ sub get_disks {
 	    }
 	}
 
-	$used = 'mounted' if $found_mountpoints && !$used;
-	$used = 'LVM' if $found_lvm && !$used;
-	$used = 'ZFS' if $found_zfs && !$used;
-	$used = 'Device Mapper' if $found_dm && !$used;
-	$used = 'partitions' if scalar(keys %{$partitions}) && !$used;
-
+	my $used = $determine_usage->($devpath, $sysdir, 0);
+	foreach my $part (sort keys %{$partitions}) {
+	    next if $partitions->{$part}->{used} eq 'partition';
+	    $used //= $partitions->{$part}->{used};
+	}
+	$used //= 'partitions' if scalar(keys %{$partitions});
 	# multipath, software raid, etc.
 	# this check comes in last, to show more specific info
 	# if we have it
-	$used = 'Device Mapper' if !$used && !dir_is_empty("$sysdir/holders");
+	$used //= 'Device Mapper' if !dir_is_empty("$sysdir/holders");
 
 	$disklist->{$dev}->{used} = $used if $used;
 	$disklist->{$dev}->{osdid} = $osdid;
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH storage 08/14] Diskmanage: also detect BIOS boot, EFI and ZFS reserved type partitions
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (6 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 07/14] Diskmanage: introduce usage helper Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 09/14] Diskmanage: introduce ceph info helper Fabian Ebner
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

as they are relevant to most PVE setups.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Diskmanage.pm | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 32789f2..b8ba498 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -613,6 +613,17 @@ sub get_disks {
 	    return 'ZFS' if $zfshash->{$devpath};
 
 	    my $info = $lsblk_info->{$devpath} // {};
+
+	    my $parttype = $info->{parttype};
+	    if (defined($parttype)) {
+		return 'BIOS boot'
+		    if $parttype eq '21686148-6449-6e6f-744e-656564454649';
+		return 'EFI'
+		    if $parttype eq 'c12a7328-f81f-11d2-ba4b-00a0c93ec93b';
+		return 'ZFS reserved'
+		    if $parttype eq '6a945a3b-1dd2-11b2-99a6-080020736631';
+	    }
+
 	    my $fstype = $info->{fstype};
 	    if (defined($fstype)) {
 		return "${fstype} (mounted)" if $mounted->{$devpath};
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH storage 09/14] Diskmanage: introduce ceph info helper
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (7 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 08/14] Diskmanage: also detect BIOS boot, EFI and ZFS reserved type partitions Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 10/14] Diskmanage: save OSD information for individual partitions Fabian Ebner
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

so it can be re-used for partitions.

Also changes the regular expression in get_ceph_volume_info to match the full
device/partition name the LV is on. Not only is this needed for partitions,
especially if there's multiple partitions with an OSD, but it also fixes
handling NVMe devices with an OSD as a side effect. Previuosly those were not
detected here, because of the digits in the name, e.g. /dev/nvme0n1

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Diskmanage.pm | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index b8ba498..ee04419 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -288,7 +288,7 @@ sub get_ceph_volume_infos {
 	my $fields = [ split(';', $line) ];
 
 	# lvs syntax is /dev/sdX(Y) where Y is the start (which we do not need)
-	my ($dev) = $fields->[0] =~ m|^(/dev/[a-z]+)|;
+	my ($dev) = $fields->[0] =~ m|^(/dev/[a-z]+[^(]*)|;
 	if ($fields->[1] =~ m|^osd-([^-]+)-|) {
 	    my $type = $1;
 	    # $result autovivification is wanted, to not creating empty hashes
@@ -639,6 +639,21 @@ sub get_disks {
 	    return 'partition';
 	};
 
+	my $collect_ceph_info = sub {
+	    my ($devpath) = @_;
+
+	    my $ceph_volume = $ceph_volume_infos->{$devpath} or return;
+	    $journal_count += $ceph_volume->{journal} // 0;
+	    $db_count += $ceph_volume->{db} // 0;
+	    $wal_count += $ceph_volume->{wal} // 0;
+	    if (defined($ceph_volume->{osdid})) {
+		$osdid = $ceph_volume->{osdid};
+		$bluestore = 1 if $ceph_volume->{bluestore};
+		$osdencrypted = 1 if $ceph_volume->{encrypted};
+	    }
+	    return 1;
+	};
+
 	my $partitions = {};
 
 	dir_glob_foreach("$sysdir", "$dev.+", sub {
@@ -651,6 +666,14 @@ sub get_disks {
 	    $partitions->{$part}->{used} =
 		$determine_usage->("$partpath/$part", "$sysdir/$part", 1);
 
+	    my $lvm_based_osd = $collect_ceph_info->("$partpath/$part");
+
+	    # Avoid counting twice (e.g. partition on which the LVM for the
+	    # DB OSD resides is present in the $journalhash)
+	    return if $lvm_based_osd;
+
+	    # Legacy handling for non-LVM based OSDs
+
 	    if (my $mp = $mounted->{"$partpath/$part"}) {
 		if ($mp =~ m|^/var/lib/ceph/osd/ceph-(\d+)$|) {
 		    $osdid = $1;
@@ -665,17 +688,6 @@ sub get_disks {
 	    }
 	});
 
-	if (my $ceph_volume = $ceph_volume_infos->{$devpath}) {
-	    $journal_count += $ceph_volume->{journal} // 0;
-	    $db_count += $ceph_volume->{db} // 0;
-	    $wal_count += $ceph_volume->{wal} // 0;
-	    if (defined($ceph_volume->{osdid})) {
-		$osdid = $ceph_volume->{osdid};
-		$bluestore = 1 if $ceph_volume->{bluestore};
-		$osdencrypted = 1 if $ceph_volume->{encrypted};
-	    }
-	}
-
 	my $used = $determine_usage->($devpath, $sysdir, 0);
 	foreach my $part (sort keys %{$partitions}) {
 	    next if $partitions->{$part}->{used} eq 'partition';
@@ -688,6 +700,9 @@ sub get_disks {
 	$used //= 'Device Mapper' if !dir_is_empty("$sysdir/holders");
 
 	$disklist->{$dev}->{used} = $used if $used;
+
+	$collect_ceph_info->($devpath);
+
 	$disklist->{$dev}->{osdid} = $osdid;
 	$disklist->{$dev}->{journals} = $journal_count if $journal_count;
 	$disklist->{$dev}->{bluestore} = $bluestore if $osdid != -1;
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH storage 10/14] Diskmanage: save OSD information for individual partitions
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (8 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 09/14] Diskmanage: introduce ceph info helper Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 11/14] Diskmanage: also include partitions with get_disks if flag is set Fabian Ebner
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

in preparation to including partitions for get_disks()

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Diskmanage.pm | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index ee04419..195b706 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -651,7 +651,11 @@ sub get_disks {
 		$bluestore = 1 if $ceph_volume->{bluestore};
 		$osdencrypted = 1 if $ceph_volume->{encrypted};
 	    }
-	    return 1;
+
+	    my $result = { %{$ceph_volume} };
+	    $result->{journals} = delete $result->{journal}
+		if $result->{journal};
+	    return $result;
 	};
 
 	my $partitions = {};
@@ -659,14 +663,16 @@ sub get_disks {
 	dir_glob_foreach("$sysdir", "$dev.+", sub {
 	    my ($part) = @_;
 
+	    $partitions->{$part} = $collect_ceph_info->("$partpath/$part");
+	    my $lvm_based_osd = defined($partitions->{$part});
+
 	    $partitions->{$part}->{devpath} = "$partpath/$part";
 	    $partitions->{$part}->{gpt} = $data->{gpt};
 	    $partitions->{$part}->{size} =
 		get_sysdir_size("$sysdir/$part") // 0;
 	    $partitions->{$part}->{used} =
 		$determine_usage->("$partpath/$part", "$sysdir/$part", 1);
-
-	    my $lvm_based_osd = $collect_ceph_info->("$partpath/$part");
+	    $partitions->{$part}->{osdid} //= -1;
 
 	    # Avoid counting twice (e.g. partition on which the LVM for the
 	    # DB OSD resides is present in the $journalhash)
@@ -677,6 +683,7 @@ sub get_disks {
 	    if (my $mp = $mounted->{"$partpath/$part"}) {
 		if ($mp =~ m|^/var/lib/ceph/osd/ceph-(\d+)$|) {
 		    $osdid = $1;
+		    $partitions->{$part}->{osdid} = $osdid;
 		}
 	    }
 
@@ -685,6 +692,11 @@ sub get_disks {
 		$db_count++ if $journal_part == 2;
 		$wal_count++ if $journal_part == 3;
 		$bluestore = 1 if $journal_part == 4;
+
+		$partitions->{$part}->{journals} = 1 if $journal_part == 1;
+		$partitions->{$part}->{db} = 1 if $journal_part == 2;
+		$partitions->{$part}->{wal} = 1 if $journal_part == 3;
+		$partitions->{$part}->{bluestore} = 1 if $journal_part == 4;
 	    }
 	});
 
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH storage 11/14] Diskmanage: also include partitions with get_disks if flag is set
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (9 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 10/14] Diskmanage: save OSD information for individual partitions Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH manager 12/14] api: Ceph: add reminder to remove 'disks' call Fabian Ebner
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

and have a parent key for partitions, to be able to see the associated disk in
the result without having to rely on naming heuristics (just adding a number at
the end doesn't work for NVMes).

The disk's usage will not be based on the partitions usage if the flag is set,
but will simply be 'partitions'.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Disks.pm | 19 ++++++++++++++++++-
 PVE/Diskmanage.pm | 17 +++++++++++++----
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Disks.pm b/PVE/API2/Disks.pm
index c0456be..d2ee81d 100644
--- a/PVE/API2/Disks.pm
+++ b/PVE/API2/Disks.pm
@@ -88,6 +88,12 @@ __PACKAGE__->register_method ({
 	additionalProperties => 0,
 	properties => {
 	    node => get_standard_option('pve-node'),
+	    'include-partitions' => {
+		description => "Also include partitions.",
+		type => 'boolean',
+		optional => 1,
+		default => 0,
+	    },
 	    skipsmart => {
 		description => "Skip smart checks.",
 		type => 'boolean',
@@ -120,6 +126,12 @@ __PACKAGE__->register_method ({
 		serial =>  { type => 'string', optional => 1 },
 		wwn => { type => 'string', optional => 1},
 		health => { type => 'string', optional => 1},
+		parent => {
+		    type => 'string',
+		    description => 'For partitions only. The device path of ' .
+			'the disk the partition resides on.',
+		    optional => 1
+		},
 	    },
 	},
     },
@@ -127,8 +139,13 @@ __PACKAGE__->register_method ({
 	my ($param) = @_;
 
 	my $skipsmart = $param->{skipsmart} // 0;
+	my $include_partitions = $param->{'include-partitions'} // 0;
 
-	my $disks = PVE::Diskmanage::get_disks(undef, $skipsmart);
+	my $disks = PVE::Diskmanage::get_disks(
+	    undef,
+	    $skipsmart,
+	    $include_partitions
+	);
 
 	my $type = $param->{type} // '';
 	my $result = [];
diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 195b706..8967ca7 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -476,7 +476,7 @@ my sub is_ssdlike {
 }
 
 sub get_disks {
-    my ($disks, $nosmart) = @_;
+    my ($disks, $nosmart, $include_partitions) = @_;
     my $disklist = {};
 
     my $mounted = {};
@@ -667,6 +667,7 @@ sub get_disks {
 	    my $lvm_based_osd = defined($partitions->{$part});
 
 	    $partitions->{$part}->{devpath} = "$partpath/$part";
+	    $partitions->{$part}->{parent} = "$devpath";
 	    $partitions->{$part}->{gpt} = $data->{gpt};
 	    $partitions->{$part}->{size} =
 		get_sysdir_size("$sysdir/$part") // 0;
@@ -701,9 +702,11 @@ sub get_disks {
 	});
 
 	my $used = $determine_usage->($devpath, $sysdir, 0);
-	foreach my $part (sort keys %{$partitions}) {
-	    next if $partitions->{$part}->{used} eq 'partition';
-	    $used //= $partitions->{$part}->{used};
+	if (!$include_partitions) {
+	    foreach my $part (sort keys %{$partitions}) {
+		next if $partitions->{$part}->{used} eq 'partition';
+		$used //= $partitions->{$part}->{used};
+	    }
 	}
 	$used //= 'partitions' if scalar(keys %{$partitions});
 	# multipath, software raid, etc.
@@ -721,6 +724,12 @@ sub get_disks {
 	$disklist->{$dev}->{osdencrypted} = $osdencrypted if $osdid != -1;
 	$disklist->{$dev}->{db} = $db_count if $db_count;
 	$disklist->{$dev}->{wal} = $wal_count if $wal_count;
+
+	if ($include_partitions) {
+	    foreach my $part (keys %{$partitions}) {
+		$disklist->{$part} = $partitions->{$part};
+	    }
+	}
     });
 
     return $disklist;
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH manager 12/14] api: Ceph: add reminder to remove 'disks' call
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (10 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH storage 11/14] Diskmanage: also include partitions with get_disks if flag is set Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH widget-toolkit 13/14] convert disk list to disk tree including the partitions Fabian Ebner
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

This API call is the predecessor of /nodes/{node}/disks/list, which has seen a
few more improvements. The latter API call should be used instead, and the web
UI already does so.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Ceph.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
index 0c647489..ad247af1 100644
--- a/PVE/API2/Ceph.pm
+++ b/PVE/API2/Ceph.pm
@@ -94,7 +94,7 @@ __PACKAGE__->register_method ({
 	    { name => 'crush' },
 	    { name => 'config' },
 	    { name => 'log' },
-	    { name => 'disks' },
+	    { name => 'disks' }, # FIXME: remove with 7.0
 	    { name => 'flags' }, # FIXME: remove with 7.0
 	    { name => 'rules' },
 	];
@@ -102,6 +102,7 @@ __PACKAGE__->register_method ({
 	return $result;
     }});
 
+# FIXME: Remove with PVE 7.0
 __PACKAGE__->register_method ({
     name => 'disks',
     path => 'disks',
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH widget-toolkit 13/14] convert disk list to disk tree including the partitions
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (11 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH manager 12/14] api: Ceph: add reminder to remove 'disks' call Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-01-26 11:45 ` [pve-devel] [PATCH widget-toolkit 14/14] move DiskList.js from grid/ to panel/ Fabian Ebner
  2021-02-06 13:13 ` [pve-devel] partially-applied: [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Thomas Lamprecht
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

Assigning the store directly to the treepanel doesn't work, more manual
handling is needed. This is mostly based on what we do for PBS's datastore
content view. The store monitoring also needs to be changed slightly.

The buttons are restricted to work on disks only, based on the parent
attribute that only partitions have.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/grid/DiskList.js | 97 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 78 insertions(+), 19 deletions(-)

diff --git a/src/grid/DiskList.js b/src/grid/DiskList.js
index 4d8ef1b..4a4f239 100644
--- a/src/grid/DiskList.js
+++ b/src/grid/DiskList.js
@@ -36,20 +36,32 @@ Ext.define('pmx-disk-list', {
 });
 
 Ext.define('Proxmox.DiskList', {
-    extend: 'Ext.grid.GridPanel',
+    extend: 'Ext.tree.Panel',
     alias: 'widget.pmxDiskList',
 
+    rootVisible: false,
+
     emptyText: gettext('No Disks found'),
 
     stateful: true,
-    stateId: 'grid-node-disks',
+    stateId: 'tree-node-disks',
 
     controller: {
 	xclass: 'Ext.app.ViewController',
 
 	reload: function() {
 	    let me = this;
-	    me.getView().getStore().load();
+	    let view = me.getView();
+
+	    let url = `${view.baseurl}/list`;
+	    me.store.setProxy({
+		type: 'proxmox',
+		extraParams: {
+		    'include-partitions': 1,
+		},
+		url: url,
+	    });
+	    me.store.load();
 	},
 
 	openSmartWindow: function() {
@@ -94,27 +106,63 @@ Ext.define('Proxmox.DiskList', {
 	},
 
 	init: function(view) {
-	    Proxmox.Utils.monStoreErrors(view, view.getStore(), true);
-
 	    let nodename = view.nodename || 'localhost';
 	    view.baseurl = `/api2/json/nodes/${nodename}/disks`;
 	    view.exturl = `/api2/extjs/nodes/${nodename}/disks`;
-	    view.getStore().getProxy().setUrl(`${view.baseurl}/list`);
-	    view.getStore().load();
+
+	    this.store = Ext.create('Ext.data.Store', {
+		model: 'pmx-disk-list',
+	    });
+	    this.store.on('load', this.onLoad, this);
+
+	    Proxmox.Utils.monStoreErrors(view, this.store);
+	    this.reload();
 	},
-    },
 
-    store: {
-	model: 'pmx-disk-list',
-	proxy: {
-	    type: 'proxmox',
+	onLoad: function(store, records, success, operation) {
+	    let me = this;
+	    let view = this.getView();
+
+	    if (!success) {
+		Proxmox.Utils.setErrorMask(
+		    view,
+		    Proxmox.Utils.getResponseErrorMessage(operation.getError()),
+		);
+		return;
+	    }
+
+	    let disks = {};
+
+	    for (const item of records) {
+		let data = item.data;
+		data.leaf = true;
+		data.expanded = true;
+		data.children = [];
+		data.iconCls = 'fa fa-fw fa-hdd-o x-fa-tree';
+		if (!data.parent) {
+		    disks[data.devpath] = data;
+		}
+	    }
+	    for (const item of records) {
+		let data = item.data;
+		if (data.parent) {
+		    disks[data.parent].leaf = false;
+		    disks[data.parent].children.push(data);
+		}
+	    }
+
+	    let children = [];
+	    for (const [_, device] of Object.entries(disks)) {
+		children.push(device);
+	    }
+
+	    view.setRootNode({
+		expanded: true,
+		children: children,
+	    });
+
+	    Proxmox.Utils.setErrorMask(view, false);
 	},
-	sorters: [
-	    {
-		property: 'dev',
-		direction: 'ASC',
-	    },
-	],
     },
 
     tbar: [
@@ -125,15 +173,25 @@ Ext.define('Proxmox.DiskList', {
 	{
 	    xtype: 'proxmoxButton',
 	    text: gettext('Show S.M.A.R.T. values'),
+	    parentXType: 'treepanel',
 	    disabled: true,
+	    enableFn: function(rec) {
+		if (!rec || rec.data.parent) {
+		    return false;
+		} else {
+		    return true;
+		}
+	    },
 	    handler: 'openSmartWindow',
 	},
 	{
 	    xtype: 'proxmoxButton',
 	    text: gettext('Initialize Disk with GPT'),
+	    parentXType: 'treepanel',
 	    disabled: true,
 	    enableFn: function(rec) {
-		if (!rec || (rec.data.used && rec.data.used !== 'unused')) {
+		if (!rec || rec.data.parent ||
+		    (rec.data.used && rec.data.used !== 'unused')) {
 		    return false;
 		} else {
 		    return true;
@@ -145,6 +203,7 @@ Ext.define('Proxmox.DiskList', {
 
     columns: [
 	{
+	    xtype: 'treecolumn',
 	    header: gettext('Device'),
 	    width: 150,
 	    sortable: true,
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] [PATCH widget-toolkit 14/14] move DiskList.js from grid/ to panel/
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (12 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH widget-toolkit 13/14] convert disk list to disk tree including the partitions Fabian Ebner
@ 2021-01-26 11:45 ` Fabian Ebner
  2021-02-06 13:13 ` [pve-devel] partially-applied: [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Thomas Lamprecht
  14 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-01-26 11:45 UTC (permalink / raw)
  To: pve-devel

because it's a treepanel now.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/Makefile                    | 2 +-
 src/{grid => panel}/DiskList.js | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename src/{grid => panel}/DiskList.js (100%)

diff --git a/src/Makefile b/src/Makefile
index fbc2627..46b90ae 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -37,7 +37,7 @@ JSSRC=					\
 	button/HelpButton.js		\
 	grid/ObjectGrid.js		\
 	grid/PendingObjectGrid.js	\
-	grid/DiskList.js		\
+	panel/DiskList.js		\
 	panel/InputPanel.js		\
 	panel/InfoWidget.js		\
 	panel/LogView.js		\
diff --git a/src/grid/DiskList.js b/src/panel/DiskList.js
similarity index 100%
rename from src/grid/DiskList.js
rename to src/panel/DiskList.js
-- 
2.20.1





^ permalink raw reply	[flat|nested] 17+ messages in thread

* [pve-devel] partially-applied: [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions
  2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
                   ` (13 preceding siblings ...)
  2021-01-26 11:45 ` [pve-devel] [PATCH widget-toolkit 14/14] move DiskList.js from grid/ to panel/ Fabian Ebner
@ 2021-02-06 13:13 ` Thomas Lamprecht
  2021-02-08  7:58   ` Fabian Ebner
  14 siblings, 1 reply; 17+ messages in thread
From: Thomas Lamprecht @ 2021-02-06 13:13 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Ebner

On 26.01.21 12:45, Fabian Ebner wrote:
> and fix some other little things along the way (see patches #1, #4, #9).
> 
> Mostly re-uses existing functionality, but refactors it, so it can be re-used
> for partitions as well. New is the detection of filesystems via lsblk and the
> detection of more PVE-relevant partuuids.
> 
> In the UI, the current list is replaced by a treepanel including the partitions.
> 
> This series does not yet make it possible to select partitons for storage
> creation for certain types, which is one of the parts of bug #2285. I felt
> like this series was getting rather big already and that there was more
> discussion to be had about that and it can always be done as a follow-up series.
> The usage of a partition currently always defaults to 'partition'. A good
> heuristic to find out when a partition is unused would be needed and/or good
> ways to warn the user and ask if they're sure they have the correct partition.
> This also ties in to the proposed feature of allowing users to wipe disks under
> certain conditions.
> 


Applied storage and manager patches, thanks!

For widget toolkit I'm wondering about backward compatibility towards older
API daemons still running and other products - you mentioned PBS so I guess
you tested it?

Also, the Type column for partitions shouldn't be "unknown" but rather
"partition", or even "GPT Partition"/"MBR Partition" or the like.




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [pve-devel] partially-applied: [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions
  2021-02-06 13:13 ` [pve-devel] partially-applied: [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Thomas Lamprecht
@ 2021-02-08  7:58   ` Fabian Ebner
  0 siblings, 0 replies; 17+ messages in thread
From: Fabian Ebner @ 2021-02-08  7:58 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion

Am 06.02.21 um 14:13 schrieb Thomas Lamprecht:
> On 26.01.21 12:45, Fabian Ebner wrote:
>> and fix some other little things along the way (see patches #1, #4, #9).
>>
>> Mostly re-uses existing functionality, but refactors it, so it can be re-used
>> for partitions as well. New is the detection of filesystems via lsblk and the
>> detection of more PVE-relevant partuuids.
>>
>> In the UI, the current list is replaced by a treepanel including the partitions.
>>
>> This series does not yet make it possible to select partitons for storage
>> creation for certain types, which is one of the parts of bug #2285. I felt
>> like this series was getting rather big already and that there was more
>> discussion to be had about that and it can always be done as a follow-up series.
>> The usage of a partition currently always defaults to 'partition'. A good
>> heuristic to find out when a partition is unused would be needed and/or good
>> ways to warn the user and ask if they're sure they have the correct partition.
>> This also ties in to the proposed feature of allowing users to wipe disks under
>> certain conditions.
>>
> 
> 
> Applied storage and manager patches, thanks!
> 
> For widget toolkit I'm wondering about backward compatibility towards older
> API daemons still running and other products - you mentioned PBS so I guess
> you tested it?
> 

No, sorry, I missed this. Because of the new parameter in the API call 
it's not backwards compatible. I'll send a v2 for those patches and make 
sure it's backwards compatible.

> Also, the Type column for partitions shouldn't be "unknown" but rather
> "partition", or even "GPT Partition"/"MBR Partition" or the like.
> 

And I'll also add a patch for this.




^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-02-08  7:59 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-26 11:45 [pve-devel] [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH storage 01/14] Disks: return correct journal disk candidates Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH storage 02/14] Diskmanage: replace closure with direct hash access Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH storage 03/14] Diskmanage: refactor and rename get_parttype_info Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH storage 04/14] Diskmanage: also check for filesystem type when determining usage Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH storage 05/14] Diskmanage: introduce get_sysdir_size helper Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH storage 06/14] Diskmanage: collect partitions in hash Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH storage 07/14] Diskmanage: introduce usage helper Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH storage 08/14] Diskmanage: also detect BIOS boot, EFI and ZFS reserved type partitions Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH storage 09/14] Diskmanage: introduce ceph info helper Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH storage 10/14] Diskmanage: save OSD information for individual partitions Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH storage 11/14] Diskmanage: also include partitions with get_disks if flag is set Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH manager 12/14] api: Ceph: add reminder to remove 'disks' call Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH widget-toolkit 13/14] convert disk list to disk tree including the partitions Fabian Ebner
2021-01-26 11:45 ` [pve-devel] [PATCH widget-toolkit 14/14] move DiskList.js from grid/ to panel/ Fabian Ebner
2021-02-06 13:13 ` [pve-devel] partially-applied: [PATCH-SERIES] partially fix #2285: extend Diskmanage to also list partitions Thomas Lamprecht
2021-02-08  7:58   ` Fabian Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal