[pve-devel] [PATCH-SERIES v2 storage/widget-toolkit/manager] disk creation and wiping improvements

[pve-devel] [PATCH-SERIES v2 storage/widget-toolkit/manager] disk creation and wiping improvements

[Fabian Ebner]

fixing the remaining parts of #2285, i.e. extending the diskmanage
module and disk creation via API/UI to support partitions.

Also change the partition type in the appropriate places as that
information is used when querying disk info.


pve-manager depends on both pve-storage and proxmox-widget-toolkit.


Changes from v1:
    * Dropped already applied patches.
    * Add is_partition helper in an earlier patch, before it is first
      used.


pve-storage:

Fabian Ebner (6):
  diskmanage: add change_parttype and is_partition helpers
  diskmanage: wipe blockdev: also change partition type
  diskmanage: don't set usage for unused partitions
  api: disks: initgpt: explicitly abort for partitions
  partially fix #2285: api: disks: allow partitions for creation paths
  api: disks: create: set correct partition type

 PVE/API2/Disks.pm           |  1 +
 PVE/API2/Disks/Directory.pm | 29 +++++++++++++++++-----------
 PVE/API2/Disks/LVM.pm       |  5 +++++
 PVE/API2/Disks/LVMThin.pm   |  5 +++++
 PVE/API2/Disks/ZFS.pm       | 22 ++++++++++++++++++++-
 PVE/Diskmanage.pm           | 38 ++++++++++++++++++++++++++++++++-----
 6 files changed, 83 insertions(+), 17 deletions(-)


proxmox-widget-toolkit:

Fabian Ebner (1):
  disk list: allow wiping individual partitions

 src/panel/DiskList.js | 8 --------
 1 file changed, 8 deletions(-)


pve-manager:

Fabian Ebner (6):
  api: ceph: create osd: set correct parttype for DB/WAL
  partially fix #2285: api: ceph: create osd: allow using partitions
  api: ceph: create osd: set correct partition type
  partially fix #2285: ui: ceph: allow selecting partitions
  ui: zfs create: switch to using widget-toolkit's multiDiskSelector
  partially fix #2285: ui: disk create: allow selecting partitions

 PVE/API2/Ceph/OSD.pm           | 24 +++++++++-
 www/manager6/ceph/OSD.js       |  3 ++
 www/manager6/node/Directory.js |  1 +
 www/manager6/node/LVM.js       |  1 +
 www/manager6/node/LVMThin.js   |  1 +
 www/manager6/node/ZFS.js       | 81 +++-------------------------------
 6 files changed, 33 insertions(+), 78 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH v2 storage 1/6] diskmanage: add change_parttype and is_partition helpers

[Fabian Ebner]

For change_parttype, only GPT-partitioned disks are supported, as I
didn't see an option for sgdisk to make it also work with
MBR-partitioned disks. And while sfdisk could be used instead (or
additionally) it would be a new dependency, and AFAICS require some
conversion of partition type GUIDs to MBR types on our part.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * Add is_partition helper here, because it is used in the next
      patch.

 PVE/Diskmanage.pm | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 36f452c..9d5b037 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -813,6 +813,12 @@ sub get_blockdev {
     return $block_dev;
 }
 
+sub is_partition {
+    my ($dev_path) = @_;
+
+    return defined(eval { get_partnum($dev_path) });
+}
+
 sub locked_disk_action {
     my ($sub) = @_;
     my $res = PVE::Tools::lock_file('/run/lock/pve-diskmanage.lck', undef, $sub);
@@ -903,6 +909,23 @@ sub is_mounted {
     return $found;
 }
 
+# Currently only supports GPT-partitioned disks.
+sub change_parttype {
+    my ($partpath, $parttype) = @_;
+
+    my $err = "unable to change partition type for $partpath";
+
+    my $partnum = get_partnum($partpath);
+    my $blockdev = get_blockdev($partpath);
+    my $dev = strip_dev($blockdev);
+
+    my $info = get_disks($dev, 1);
+    die "$err - unable to get disk info for '$blockdev'\n" if !defined($info->{$dev});
+    die "$err - disk '$blockdev' is not GPT partitioned\n" if !$info->{$dev}->{gpt};
+
+    run_command(['sgdisk', "-t${partnum}:${parttype}", $blockdev], errmsg => $err);
+}
+
 # Wipes all labels and the first 200 MiB of a disk/partition (or the whole if it is smaller).
 # Expected to be called with a result of verify_blockdev_path().
 sub wipe_blockdev {
-- 
2.30.2

[pve-devel] [PATCH v2 storage 2/6] diskmanage: wipe blockdev: also change partition type

[Fabian Ebner]

when called with a partition. Since get_disks uses the partition type
(among other things) to detect LVM and ZFS volumes, such volumes would
still be seen as in-use after wiping. Thus, also change the partition
type and simply use 0x83 "Linux filesystem".

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

 PVE/Diskmanage.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 9d5b037..5041fb3 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -927,6 +927,7 @@ sub change_parttype {
 }
 
 # Wipes all labels and the first 200 MiB of a disk/partition (or the whole if it is smaller).
+# If called with a partition, also sets the partition type to 0x83 'Linux filesystem'.
 # Expected to be called with a result of verify_blockdev_path().
 sub wipe_blockdev {
     my ($devpath) = @_;
@@ -959,6 +960,11 @@ sub wipe_blockdev {
 	['dd', 'if=/dev/zero', "of=${devpath}", 'bs=1M', 'conv=fdatasync', "count=${count}"],
 	errmsg => "error wiping '${devpath}'",
     );
+
+    if (is_partition($devpath)) {
+	eval { change_parttype($devpath, '8300'); };
+	warn $@ if $@;
+    }
 }
 
 1;
-- 
2.30.2

[pve-devel] [PATCH v2 storage 3/6] diskmanage: don't set usage for unused partitions

[Fabian Ebner]

The disk type is already 'partition' so there's no additional
information here. And it would need to serve as a code-word for
unused partitions. The cleaner approach is to not set the usage.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

Was not a smart decision back then...If this is considered a breaking
change for the disk list API call, I'll go for a different approach of
course, but AFAICS it would be one of:
1. Keep usage 'partition', but also return the unused partitions when
'include-partitions=1,type=unused', and have disk_is_used not complain
about usage 'partition'.
2. Add a new type filter/extend it so it can handle multiple types,
i.e. type=unused,partition.
3. Request all disks in the frontend and filter there.

 PVE/Diskmanage.pm | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 5041fb3..c896c5a 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -661,7 +661,7 @@ sub get_disks {
 	    # for devices, this check is done explicitly later
 	    return 'Device Mapper' if !dir_is_empty("$sysdir/holders");
 
-	    return 'partition';
+	    return; # unused partition
 	};
 
 	my $collect_ceph_info = sub {
@@ -730,7 +730,6 @@ sub get_disks {
 	my $used = $determine_usage->($devpath, $sysdir, 0);
 	if (!$include_partitions) {
 	    foreach my $part (sort keys %{$partitions}) {
-		next if $partitions->{$part}->{used} eq 'partition';
 		$used //= $partitions->{$part}->{used};
 	    }
 	} else {
-- 
2.30.2

[pve-devel] [PATCH v2 storage 4/6] api: disks: initgpt: explicitly abort for partitions

[Fabian Ebner]

In preparation to extend disk_is_used to support partitions. Without
this new check, initgpt would also allow partitions once disk_is_used
supports partitions, which is not desirable.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * is_partition helper is now already added in an earlier patch.

 PVE/API2/Disks.pm | 1 +
 PVE/Diskmanage.pm | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Disks.pm b/PVE/API2/Disks.pm
index 96c19fd..25c9ded 100644
--- a/PVE/API2/Disks.pm
+++ b/PVE/API2/Disks.pm
@@ -260,6 +260,7 @@ __PACKAGE__->register_method ({
 
 	my $authuser = $rpcenv->get_user();
 
+	die "$disk is a partition\n" if PVE::Diskmanage::is_partition($disk);
 	die "disk $disk already in use\n" if PVE::Diskmanage::disk_is_used($disk);
 	my $worker = sub {
 	    PVE::Diskmanage::init_disk($disk, $param->{uuid});
diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index c896c5a..f0e14dc 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -63,8 +63,8 @@ sub init_disk {
 
     assert_blockdev($disk);
 
-    # we should already have checked if it is in use in the api call
-    # but we check again for safety
+    # we should already have checked these in the api call, but we check again for safety
+    die "$disk is a partition\n" if is_partition($disk);
     die "disk $disk is already in use\n" if disk_is_used($disk);
 
     my $id = $uuid || 'R';
-- 
2.30.2

[pve-devel] [PATCH v2 storage 5/6] partially fix #2285: api: disks: allow partitions for creation paths

[Fabian Ebner]

The calls for directory and ZFS need slight adaptations. Except for
those, the only thing that needs to be done is support partitions in
the disk_is_used helper.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

 PVE/API2/Disks/Directory.pm | 28 ++++++++++++++++------------
 PVE/API2/Disks/ZFS.pm       | 11 ++++++++++-
 PVE/Diskmanage.pm           |  2 +-
 3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/PVE/API2/Disks/Directory.pm b/PVE/API2/Disks/Directory.pm
index 1285274..52f0c86 100644
--- a/PVE/API2/Disks/Directory.pm
+++ b/PVE/API2/Disks/Directory.pm
@@ -214,20 +214,24 @@ __PACKAGE__->register_method ({
 	    PVE::Diskmanage::locked_disk_action(sub {
 		PVE::Diskmanage::assert_disk_unused($dev);
 
-		# create partition
-		my $cmd = [$SGDISK, '-n1', '-t1:8300', $dev];
-		print "# ", join(' ', @$cmd), "\n";
-		run_command($cmd);
-
-		my ($devname) = $dev =~ m|^/dev/(.*)$|;
-		my $part = "/dev/";
-		dir_glob_foreach("/sys/block/$devname", qr/\Q$devname\E.+/, sub {
-		    my ($partition) = @_;
-		    $part .= $partition;
-		});
+		my $part = $dev;
+
+		if (!PVE::Diskmanage::is_partition($dev)) {
+		    # create partition
+		    my $cmd = [$SGDISK, '-n1', '-t1:8300', $dev];
+		    print "# ", join(' ', @$cmd), "\n";
+		    run_command($cmd);
+
+		    my ($devname) = $dev =~ m|^/dev/(.*)$|;
+		    $part = "/dev/";
+		    dir_glob_foreach("/sys/block/$devname", qr/\Q$devname\E.+/, sub {
+			my ($partition) = @_;
+			$part .= $partition;
+		    });
+		}
 
 		# create filesystem
-		$cmd = [$MKFS, '-t', $type, $part];
+		my $cmd = [$MKFS, '-t', $type, $part];
 		print "# ", join(' ', @$cmd), "\n";
 		run_command($cmd);
 
diff --git a/PVE/API2/Disks/ZFS.pm b/PVE/API2/Disks/ZFS.pm
index 1534631..6486404 100644
--- a/PVE/API2/Disks/ZFS.pm
+++ b/PVE/API2/Disks/ZFS.pm
@@ -373,7 +373,16 @@ __PACKAGE__->register_method ({
 	    PVE::Diskmanage::locked_disk_action(sub {
 		for my $dev (@$devs) {
 		    PVE::Diskmanage::assert_disk_unused($dev);
-		    my $sysfsdev = $dev =~ s!^/dev/!/sys/block/!r;
+
+		    my $is_partition = PVE::Diskmanage::is_partition($dev);
+		    my $sysfsdev = $is_partition ? PVE::Diskmanage::get_blockdev($dev) : $dev;
+
+		    $sysfsdev =~ s!^/dev/!/sys/block/!;
+		    if ($is_partition) {
+			my $part = $dev =~ s!^/dev/!!r;
+			$sysfsdev .= "/${part}";
+		    }
+
 		    my $udevinfo = PVE::Diskmanage::get_udev_info($sysfsdev);
 		    $dev = $udevinfo->{by_id_link} if defined($udevinfo->{by_id_link});
 		}
diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index f0e14dc..18459f9 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -78,7 +78,7 @@ sub disk_is_used {
     my $dev = $disk;
     $dev =~ s|^/dev/||;
 
-    my $disklist = get_disks($dev, 1);
+    my $disklist = get_disks($dev, 1, 1);
 
     die "'$disk' is not a valid local disk\n" if !defined($disklist->{$dev});
     return 1 if $disklist->{$dev}->{used};
-- 
2.30.2

[pve-devel] [PATCH v2 storage 6/6] api: disks: create: set correct partition type

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

 PVE/API2/Disks/Directory.pm |  5 ++++-
 PVE/API2/Disks/LVM.pm       |  5 +++++
 PVE/API2/Disks/LVMThin.pm   |  5 +++++
 PVE/API2/Disks/ZFS.pm       | 11 +++++++++++
 4 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Disks/Directory.pm b/PVE/API2/Disks/Directory.pm
index 52f0c86..3a90a2e 100644
--- a/PVE/API2/Disks/Directory.pm
+++ b/PVE/API2/Disks/Directory.pm
@@ -216,7 +216,10 @@ __PACKAGE__->register_method ({
 
 		my $part = $dev;
 
-		if (!PVE::Diskmanage::is_partition($dev)) {
+		if (PVE::Diskmanage::is_partition($dev)) {
+		    eval { PVE::Diskmanage::change_parttype($dev, '8300'); };
+		    warn $@ if $@;
+		} else {
 		    # create partition
 		    my $cmd = [$SGDISK, '-n1', '-t1:8300', $dev];
 		    print "# ", join(' ', @$cmd), "\n";
diff --git a/PVE/API2/Disks/LVM.pm b/PVE/API2/Disks/LVM.pm
index eb8f5c0..885e02b 100644
--- a/PVE/API2/Disks/LVM.pm
+++ b/PVE/API2/Disks/LVM.pm
@@ -156,6 +156,11 @@ __PACKAGE__->register_method ({
 	    PVE::Diskmanage::locked_disk_action(sub {
 		PVE::Diskmanage::assert_disk_unused($dev);
 
+		if (PVE::Diskmanage::is_partition($dev)) {
+		    eval { PVE::Diskmanage::change_parttype($dev, '8E00'); };
+		    warn $@ if $@;
+		}
+
 		PVE::Storage::LVMPlugin::lvm_create_volume_group($dev, $name);
 
 		# FIXME: Remove once we depend on systemd >= v249.
diff --git a/PVE/API2/Disks/LVMThin.pm b/PVE/API2/Disks/LVMThin.pm
index 2fd8484..83ebc46 100644
--- a/PVE/API2/Disks/LVMThin.pm
+++ b/PVE/API2/Disks/LVMThin.pm
@@ -110,6 +110,11 @@ __PACKAGE__->register_method ({
 	    PVE::Diskmanage::locked_disk_action(sub {
 		PVE::Diskmanage::assert_disk_unused($dev);
 
+		if (PVE::Diskmanage::is_partition($dev)) {
+		    eval { PVE::Diskmanage::change_parttype($dev, '8E00'); };
+		    warn $@ if $@;
+		}
+
 		PVE::Storage::LVMPlugin::lvm_create_volume_group($dev, $name);
 		my $pv = PVE::Storage::LVMPlugin::lvm_pv_info($dev);
 		# keep some free space just in case
diff --git a/PVE/API2/Disks/ZFS.pm b/PVE/API2/Disks/ZFS.pm
index 6486404..7f96bb7 100644
--- a/PVE/API2/Disks/ZFS.pm
+++ b/PVE/API2/Disks/ZFS.pm
@@ -375,6 +375,17 @@ __PACKAGE__->register_method ({
 		    PVE::Diskmanage::assert_disk_unused($dev);
 
 		    my $is_partition = PVE::Diskmanage::is_partition($dev);
+
+		    if ($is_partition) {
+			eval {
+			    PVE::Diskmanage::change_parttype(
+				$dev,
+				'6a898cc3-1dd2-11b2-99a6-080020736631',
+			    );
+			};
+			warn $@ if $@;
+		    }
+
 		    my $sysfsdev = $is_partition ? PVE::Diskmanage::get_blockdev($dev) : $dev;
 
 		    $sysfsdev =~ s!^/dev/!/sys/block/!;
-- 
2.30.2

[pve-devel] [PATCH v2 widget-toolkit 1/1] disk list: allow wiping individual partitions

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

 src/panel/DiskList.js | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/src/panel/DiskList.js b/src/panel/DiskList.js
index 90a6553..eb8b1a8 100644
--- a/src/panel/DiskList.js
+++ b/src/panel/DiskList.js
@@ -399,14 +399,6 @@ Ext.define('Proxmox.DiskList', {
 		    return `${mainMessage}<br><br>${additionalInfo}`;
 		},
 		disabled: true,
-		enableFn: function(rec) {
-		    // TODO enable for partitions once they can be selected for ZFS,LVM,etc. creation
-		    if (!rec || rec.data.parent) {
-			return false;
-		    } else {
-			return true;
-		    }
-		},
 		handler: 'wipeDisk',
 	    });
 	}
-- 
2.30.2

[pve-devel] applied: [PATCH v2 widget-toolkit 1/1] disk list: allow wiping individual partitions

[Thomas Lamprecht]

On 06.10.21 11:18, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> No changes from v1.
> 
>  src/panel/DiskList.js | 8 --------
>  1 file changed, 8 deletions(-)
> 
>

applied, thanks!

[pve-devel] [PATCH v2 manager 1/6] api: ceph: create osd: set correct parttype for DB/WAL

[Fabian Ebner]

The get_ceph_journals function in pve-storage uses this information.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

Dependency bump for pve-storage needed.

 PVE/API2/Ceph/OSD.pm | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index fbbc2139..028b37d0 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -427,8 +427,18 @@ __PACKAGE__->register_method ({
 
 	    } elsif ($dev->{used} eq 'partitions' && $dev->{gpt}) {
 		# create new partition at the end
+		my $parttypes = {
+		    'osd-db' => '30CD0809-C2B2-499C-8879-2D6B78529876',
+		    'osd-wal' => '5CE17FCE-4087-4169-B7FF-056CC58473F9',
+		};
 
 		my $part = PVE::Diskmanage::append_partition($dev->{devpath}, $size * 1024);
+
+		if (my $parttype = $parttypes->{$type}) {
+		    eval { PVE::Diskmanage::change_parttype($part, $parttype); };
+		    warn $@ if $@;
+		}
+
 		push @udev_trigger_devs, $part;
 		return $part;
 	    }
-- 
2.30.2

[pve-devel] [RFC v2 manager 2/6] partially fix #2285: api: ceph: create osd: allow using partitions

[Fabian Ebner]

Note that this does not only allow partitions to be used, but for DB
and WAL disks, one more type of disk, that wasn't allowed before.
Namely, GPT-partitioned disks with any partitions detected as used.
The reason is get_disks' behavior:
  * Without $include_partitions=1, the disk will have the same usage
    as it's first used partition, and thus wasn't allowed. (Except in
    the case that usage was LVM, where the check was bypassed, but
    luckily OSD creation just failed later because no Ceph volume
    group would be detected).
  * With $include_partitions=1, the disk will have usage 'partitions'
    and thus be allowed.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

RFC, because of the weird situation with the DB/WAL disks...

Dependency bump for pve-storage is needed.

 PVE/API2/Ceph/OSD.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index 028b37d0..c511f315 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -346,7 +346,7 @@ __PACKAGE__->register_method ({
 
 	# test disk requirements early
 	my $devlist = [ map { $_->{name} } values %$devs ];
-	my $disklist = PVE::Diskmanage::get_disks($devlist, 1);
+	my $disklist = PVE::Diskmanage::get_disks($devlist, 1, 1);
 	$test_disk_requirements->($disklist);
 
 	# get necessary ceph infos
@@ -451,7 +451,7 @@ __PACKAGE__->register_method ({
 
 	    PVE::Diskmanage::locked_disk_action(sub {
 		# update disklist and re-test requirements
-		$disklist = PVE::Diskmanage::get_disks($devlist, 1);
+		$disklist = PVE::Diskmanage::get_disks($devlist, 1, 1);
 		$test_disk_requirements->($disklist);
 
 		my $dev_class = $param->{'crush-device-class'};
-- 
2.30.2

[pve-devel] [RFC v2 manager 3/6] api: ceph: create osd: set correct partition type

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

RFC, because the previous one is and this only makes sense with it.

 PVE/API2/Ceph/OSD.pm | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index c511f315..18329eeb 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -396,6 +396,11 @@ __PACKAGE__->register_method ({
 		PVE::Storage::LVMPlugin::lvm_create_volume_group($dev->{devpath}, $vg);
 		PVE::Storage::LVMPlugin::lvcreate($vg, $lv, "${size}k");
 
+		if (PVE::Diskmanage::is_partition($dev->{devpath})) {
+		    eval { PVE::Diskmanage::change_parttype($dev->{devpath}, '8E00'); };
+		    warn $@ if $@;
+		}
+
 		push @udev_trigger_devs, $dev->{devpath};
 
 		return "$vg/$lv";
@@ -494,6 +499,11 @@ __PACKAGE__->register_method ({
 
 		PVE::Diskmanage::wipe_blockdev($devpath);
 
+		if (PVE::Diskmanage::is_partition($devpath)) {
+		    eval { PVE::Diskmanage::change_parttype($devpath, '8E00'); };
+		    warn $@ if $@;
+		}
+
 		run_command($cmd);
 
 		# FIXME: Remove once we depend on systemd >= v249.
-- 
2.30.2

[pve-devel] [RFC v2 manager 4/6] partially fix #2285: ui: ceph: allow selecting partitions

[Fabian Ebner]

For DB and WAL disks, not only partitions will show up now, but one
more type of disk, that didn't show up before: Namely, GPT-partitioned
disks with any partitions detected as used.

It's confusing as the size shown is of the full disk, with no
indication that a new partition will be appended at the end. This
problem was already present before, but only affected GPT-partitioned
disks where no usage on a partition was detected.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

RFC, because of the weird situation with the DB/WAL disks...Even if we
choose to extend the support in the back-end, it might make sense to
limit what we show in the front-end.

Dependency bump for widget-toolkit needed.

 www/manager6/ceph/OSD.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js
index aea38d6c..3e775606 100644
--- a/www/manager6/ceph/OSD.js
+++ b/www/manager6/ceph/OSD.js
@@ -38,6 +38,7 @@ Ext.define('PVE.CephCreateOsd', {
 			    name: 'dev',
 			    nodename: me.nodename,
 			    diskType: 'unused',
+			    includePartitions: true,
 			    fieldLabel: gettext('Disk'),
 			    allowBlank: false,
 			},
@@ -48,6 +49,7 @@ Ext.define('PVE.CephCreateOsd', {
 			    name: 'db_dev',
 			    nodename: me.nodename,
 			    diskType: 'journal_disks',
+			    includePartitions: true,
 			    fieldLabel: gettext('DB Disk'),
 			    value: '',
 			    autoSelect: false,
@@ -101,6 +103,7 @@ Ext.define('PVE.CephCreateOsd', {
 			    name: 'wal_dev',
 			    nodename: me.nodename,
 			    diskType: 'journal_disks',
+			    includePartitions: true,
 			    fieldLabel: gettext('WAL Disk'),
 			    value: '',
 			    autoSelect: false,
-- 
2.30.2

[pve-devel] [PATCH v2 manager 5/6] ui: zfs create: switch to using widget-toolkit's multiDiskSelector

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

For this one, the dependency requirement is already new enough.

 www/manager6/node/ZFS.js | 80 ++--------------------------------------
 1 file changed, 4 insertions(+), 76 deletions(-)

diff --git a/www/manager6/node/ZFS.js b/www/manager6/node/ZFS.js
index 8ea364bf..2537ee5a 100644
--- a/www/manager6/node/ZFS.js
+++ b/www/manager6/node/ZFS.js
@@ -16,35 +16,13 @@ Ext.define('PVE.node.CreateZFS', {
 	    throw "no node name specified";
 	}
 
-	let update_disklist = function() {
-	    let grid = me.down('#disklist');
-	    let disks = grid.getSelection();
-
-	    disks.sort(function(a, b) {
-		let aOrder = a.get('order') || 0;
-		let bOrder = b.get('order') || 0;
-		return aOrder - bOrder;
-	    });
-
-	    let selectedDevices = disks.map(disk => disk.get('devpath')).join(';');
-
-	    me.down('field[name=devices]').setValue(selectedDevices);
-	};
-
 	Ext.apply(me, {
 	    url: `/nodes/${me.nodename}/disks/zfs`,
 	    method: 'POST',
 	    items: [
 		{
 		    xtype: 'inputpanel',
-		    onGetValues: values => values, // FIXME leftover?
 		    column1: [
-			{
-			    xtype: 'textfield',
-			    hidden: true,
-			    name: 'devices',
-			    allowBlank: false,
-			},
 			{
 			    xtype: 'proxmoxtextfield',
 			    name: 'name',
@@ -99,62 +77,13 @@ Ext.define('PVE.node.CreateZFS', {
 		    ],
 		    columnB: [
 			{
-			    xtype: 'grid',
+			    xtype: 'pmxMultiDiskSelector',
+			    name: 'devices',
+			    nodename: me.nodename,
+			    diskType: 'unused',
 			    height: 200,
 			    emptyText: gettext('No Disks unused'),
 			    itemId: 'disklist',
-			    selModel: 'checkboxmodel',
-			    listeners: {
-				selectionchange: update_disklist,
-			    },
-			    store: {
-				proxy: {
-				    type: 'proxmox',
-				    url: `/api2/json/nodes/${me.nodename}/disks/list?type=unused`,
-				},
-			    },
-			    columns: [
-				{
-				    text: gettext('Device'),
-				    dataIndex: 'devpath',
-				    flex: 2,
-				},
-				{
-				    text: gettext('Model'),
-				    dataIndex: 'model',
-				    flex: 2,
-				},
-				{
-				    text: gettext('Serial'),
-				    dataIndex: 'serial',
-				    flex: 2,
-				},
-				{
-				    text: gettext('Size'),
-				    dataIndex: 'size',
-				    renderer: Proxmox.Utils.render_size,
-				    flex: 1,
-				},
-				{
-				    header: gettext('Order'),
-				    xtype: 'widgetcolumn',
-				    dataIndex: 'order',
-				    sortable: true,
-				    flex: 1,
-				    widget: {
-					xtype: 'proxmoxintegerfield',
-					minValue: 1,
-					isFormField: false,
-					listeners: {
-					    change: function(numberfield, value, old_value) {
-						let record = numberfield.getWidgetRecord();
-						record.set('order', value);
-						update_disklist(record);
-					    },
-					},
-				    },
-				},
-			    ],
 			},
 		    ],
 		},
@@ -170,7 +99,6 @@ Ext.define('PVE.node.CreateZFS', {
 	});
 
         me.callParent();
-	me.down('#disklist').getStore().load();
     },
 });
 
-- 
2.30.2

[pve-devel] [PATCH v2 manager 6/6] partially fix #2285: ui: disk create: allow selecting partitions

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

Dependency bump for widget-toolkit needed.

 www/manager6/node/Directory.js | 1 +
 www/manager6/node/LVM.js       | 1 +
 www/manager6/node/LVMThin.js   | 1 +
 www/manager6/node/ZFS.js       | 1 +
 4 files changed, 4 insertions(+)

diff --git a/www/manager6/node/Directory.js b/www/manager6/node/Directory.js
index 50c8d677..abb562f5 100644
--- a/www/manager6/node/Directory.js
+++ b/www/manager6/node/Directory.js
@@ -26,6 +26,7 @@ Ext.define('PVE.node.CreateDirectory', {
 		    name: 'device',
 		    nodename: me.nodename,
 		    diskType: 'unused',
+		    includePartitions: true,
 		    fieldLabel: gettext('Disk'),
 		    allowBlank: false,
 		},
diff --git a/www/manager6/node/LVM.js b/www/manager6/node/LVM.js
index 4b5225d8..dc09bc8b 100644
--- a/www/manager6/node/LVM.js
+++ b/www/manager6/node/LVM.js
@@ -26,6 +26,7 @@ Ext.define('PVE.node.CreateLVM', {
 		    name: 'device',
 		    nodename: me.nodename,
 		    diskType: 'unused',
+		    includePartitions: true,
 		    fieldLabel: gettext('Disk'),
 		    allowBlank: false,
 		},
diff --git a/www/manager6/node/LVMThin.js b/www/manager6/node/LVMThin.js
index db9ea249..2af510ef 100644
--- a/www/manager6/node/LVMThin.js
+++ b/www/manager6/node/LVMThin.js
@@ -24,6 +24,7 @@ Ext.define('PVE.node.CreateLVMThin', {
 		    name: 'device',
 		    nodename: me.nodename,
 		    diskType: 'unused',
+		    includePartitions: true,
 		    fieldLabel: gettext('Disk'),
 		    allowBlank: false,
 		},
diff --git a/www/manager6/node/ZFS.js b/www/manager6/node/ZFS.js
index 2537ee5a..af475a21 100644
--- a/www/manager6/node/ZFS.js
+++ b/www/manager6/node/ZFS.js
@@ -81,6 +81,7 @@ Ext.define('PVE.node.CreateZFS', {
 			    name: 'devices',
 			    nodename: me.nodename,
 			    diskType: 'unused',
+			    includePartitions: true,
 			    height: 200,
 			    emptyText: gettext('No Disks unused'),
 			    itemId: 'disklist',
-- 
2.30.2

[pve-devel] partially-applied: [PATCH-SERIES v2 storage/widget-toolkit/manager] disk creation and wiping improvements

[Thomas Lamprecht]

On 06.10.21 11:18, Fabian Ebner wrote:
> pve-storage:
> 
> Fabian Ebner (6):
>   diskmanage: add change_parttype and is_partition helpers
>   diskmanage: wipe blockdev: also change partition type
>   diskmanage: don't set usage for unused partitions
>   api: disks: initgpt: explicitly abort for partitions
>   partially fix #2285: api: disks: allow partitions for creation paths
>   api: disks: create: set correct partition type

applied that part for now, thanks

[pve-devel] applied-series: [PATCH-SERIES v2 storage/widget-toolkit/manager] disk creation and wiping improvements

[Thomas Lamprecht]

On 06.10.21 11:18, Fabian Ebner wrote:
> pve-manager:
> 
> Fabian Ebner (6):
>   api: ceph: create osd: set correct parttype for DB/WAL
>   partially fix #2285: api: ceph: create osd: allow using partitions
>   api: ceph: create osd: set correct partition type
>   partially fix #2285: ui: ceph: allow selecting partitions
>   ui: zfs create: switch to using widget-toolkit's multiDiskSelector
>   partially fix #2285: ui: disk create: allow selecting partitions
> 
>  PVE/API2/Ceph/OSD.pm           | 24 +++++++++-
>  www/manager6/ceph/OSD.js       |  3 ++
>  www/manager6/node/Directory.js |  1 +
>  www/manager6/node/LVM.js       |  1 +
>  www/manager6/node/LVMThin.js   |  1 +
>  www/manager6/node/ZFS.js       | 81 +++-------------------------------
>  6 files changed, 33 insertions(+), 78 deletions(-)
> 

applied remaining manager patches, thanks!