[pve-devel] [PATCH-SERIES storage/widget-toolkit/manager] Add wipe disk api call and button

[pve-devel] [PATCH-SERIES storage/widget-toolkit/manager] Add wipe disk api call and button

[Fabian Ebner]

so admins wipe disks that are not actually used, but contain left-overs.

The last patch needs dependency bumps for pve-storage and
proxmox-widget-toolkit.

storage:

Fabian Ebner (5):
  diskmanage: add wipe_blockdev method
  diskmanage: factor out mounted_blockdevs helper
  diskmanage: add is_mounted method
  diskmanage: add has_holder method
  api: add wipedisk call

 PVE/API2/Disks.pm | 42 ++++++++++++++++++++++
 PVE/Diskmanage.pm | 91 ++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 129 insertions(+), 4 deletions(-)


widget-toolkit:

Fabian Ebner (4):
  disk list: fix minor usage renderer issue
  disk list: factor out renderer for type and usage
  disk list: move title bar initialization to initComponent
  disk list: add wipe disk button

 src/panel/DiskList.js | 223 ++++++++++++++++++++++++++++++------------
 1 file changed, 161 insertions(+), 62 deletions(-)


manager:

Fabian Ebner (2):
  ui: add task description for wipe disk
  ui: disk list: enable wipe disk button

 www/manager6/Utils.js       | 1 +
 www/manager6/node/Config.js | 1 +
 2 files changed, 2 insertions(+)

-- 
2.20.1

[pve-devel] [PATCH storage 01/11] diskmanage: add wipe_blockdev method

[Fabian Ebner]

based on the wipe_disks method from pve-manager's Ceph/Tools.pm with the
following main differences:
    * use wipefs to wipe labels first (to avoid sgdisk complaining about the
      backed up GPT structure on a subsequent GPT initialization)
    * only take one device as an argument
    * do not use an absolute path for 'dd'
    * die if one of the command fails

The wipefs command makes checks and complains about e.g. mounted or active
devices. One could supply --force to wipefs, but in many such situations it
does not work as expected, because the device would still be detected as in-use
afterwards, and further manaual steps would be needed.

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

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index b916d2e..612d976 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -7,6 +7,7 @@ use PVE::ProcFSTools;
 use Data::Dumper;
 use Cwd qw(abs_path);
 use Fcntl ':mode';
+use File::Basename;
 use File::stat;
 use JSON;
 
@@ -841,4 +842,30 @@ sub append_partition {
     return $partition;
 }
 
+# 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 {
+    my ($devpath) = @_;
+
+    my $wipefs_cmd = ['wipefs', '--all', $devpath];
+
+    my $dd_cmd = ['dd', 'if=/dev/zero', "of=${devpath}", 'bs=1M', 'conv=fdatasync'];
+
+    my $devname = basename($devpath);
+    my $dev_size = PVE::Tools::file_get_contents("/sys/class/block/$devname/size");
+
+    ($dev_size) = $dev_size =~ m|(\d+)|; # untaint $dev_size
+    die "Couldn't get the size of the device $devname\n" if !defined($dev_size);
+
+    my $size = ($dev_size * 512 / 1024 / 1024);
+    my $count = ($size < 200) ? $size : 200;
+
+    push @{$dd_cmd}, "count=${count}";
+
+    print "wiping disk/partition: ${devpath}\n";
+
+    run_command($wipefs_cmd, errmsg => "error wiping labels for '${devpath}'");
+    run_command($dd_cmd, errmsg => "error wiping '${devpath}'");
+}
+
 1;
-- 
2.20.1

[pve-devel] [PATCH storage 02/11] diskmanage: factor out mounted_blockdevs helper

[Fabian Ebner]

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

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 612d976..c188d83 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -481,10 +481,7 @@ my sub is_ssdlike {
     return $type eq 'ssd' || $type eq 'nvme';
 }
 
-sub get_disks {
-    my ($disks, $nosmart, $include_partitions) = @_;
-    my $disklist = {};
-
+sub mounted_blockdevs {
     my $mounted = {};
 
     my $mounts = PVE::ProcFSTools::parse_proc_mounts();
@@ -494,6 +491,15 @@ sub get_disks {
 	$mounted->{abs_path($mount->[0])} = $mount->[1];
     };
 
+    return $mounted;
+}
+
+sub get_disks {
+    my ($disks, $nosmart, $include_partitions) = @_;
+    my $disklist = {};
+
+    my $mounted = mounted_blockdevs();
+
     my $lsblk_info = get_lsblk_info();
 
     my $journalhash = get_ceph_journals($lsblk_info);
-- 
2.20.1

[pve-devel] [PATCH storage 03/11] diskmanage: add is_mounted method

[Fabian Ebner]

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

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index c188d83..70677ea 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -848,6 +848,32 @@ sub append_partition {
     return $partition;
 }
 
+# Basic check if a disk or any of its partitions is mounted.
+# Can also be called with a partition.
+# Expected to be called with a result of verify_blockdev_path().
+sub is_mounted {
+    my ($devpath) = @_;
+
+    my $mounted = mounted_blockdevs();
+
+    return $devpath if $mounted->{$devpath};
+
+    my $dev = $devpath;
+    $dev =~ s|^/dev/||;
+
+    my $found;
+
+    dir_glob_foreach("/sys/block/${dev}", "${dev}.+", sub {
+	my ($part) = @_;
+
+	my $partpath = "/dev/${part}";
+
+	$found = $partpath if $mounted->{$partpath};
+    });
+
+    return $found;
+}
+
 # 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.20.1

[pve-devel] [PATCH storage 04/11] diskmanage: add has_holder method

[Fabian Ebner]

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

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 70677ea..07bcef2 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -848,6 +848,30 @@ sub append_partition {
     return $partition;
 }
 
+# Check if a disk or any of its partitions has a holder.
+# Can also be called with a partition.
+# Expected to be called with a result of verify_blockdev_path().
+sub has_holder {
+    my ($devpath) = @_;
+
+    my $sysdir = "/sys/class/block/";
+
+    my $dev = $devpath;
+    $dev =~ s|^/dev/||;
+
+    return $devpath if !dir_is_empty("${sysdir}/${dev}/holders");
+
+    my $found;
+
+    dir_glob_foreach("/sys/block/${dev}", "${dev}.+", sub {
+	my ($part) = @_;
+
+	$found = "/dev/${part}" if !dir_is_empty("${sysdir}/${part}/holders");
+    });
+
+    return $found;
+}
+
 # Basic check if a disk or any of its partitions is mounted.
 # Can also be called with a partition.
 # Expected to be called with a result of verify_blockdev_path().
-- 
2.20.1

[pve-devel] [PATCH storage 05/11] api: add wipedisk call

[Fabian Ebner]

Try to detect active mounts and holders early, because it's cheap. The wipefs
command in the worker will detect even more situations where wiping alone is
not enough for the device to show up as unused, or could otherwise be
problematic.

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

diff --git a/PVE/API2/Disks.pm b/PVE/API2/Disks.pm
index 33bca76..6c20931 100644
--- a/PVE/API2/Disks.pm
+++ b/PVE/API2/Disks.pm
@@ -3,6 +3,7 @@ package PVE::API2::Disks;
 use strict;
 use warnings;
 
+use File::Basename;
 use HTTP::Status qw(:constants);
 
 use PVE::Diskmanage;
@@ -68,6 +69,7 @@ __PACKAGE__->register_method ({
 	    { name => 'lvm' },
 	    { name => 'lvmthin' },
 	    { name => 'directory' },
+	    { name => 'wipedisk' },
 	    { name => 'zfs' },
 	];
 
@@ -267,4 +269,44 @@ __PACKAGE__->register_method ({
 	return $rpcenv->fork_worker('diskinit', $diskid, $authuser, $worker);
     }});
 
+__PACKAGE__->register_method ({
+    name => 'wipe_disk',
+    path => 'wipedisk',
+    method => 'PUT',
+    description => "Wipe a disk or partition.",
+    proxyto => 'node',
+    protected => 1,
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    disk => {
+		type => 'string',
+		description => "Block device name",
+		pattern => '^/dev/[a-zA-Z0-9\/]+$',
+	    },
+	},
+    },
+    returns => { type => 'string' },
+    code => sub {
+	my ($param) = @_;
+
+	my $disk = PVE::Diskmanage::verify_blockdev_path($param->{disk});
+
+	my $mounted = PVE::Diskmanage::is_mounted($disk);
+	die "disk/partition '${mounted}' is mounted\n" if $mounted;
+
+	my $held = PVE::Diskmanage::has_holder($disk);
+	die "disk/partition '${held}' has a holder\n" if $held;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+	my $worker = sub { PVE::Diskmanage::wipe_blockdev($disk); };
+
+	my $basename = basename($disk); # avoid '/' in the ID
+
+	return $rpcenv->fork_worker('wipedisk', $basename, $authuser, $worker);
+    }});
+
 1;
-- 
2.20.1

[pve-devel] [PATCH widget-toolkit 06/11] disk list: fix minor usage renderer issue

[Fabian Ebner]

If there is extended information, the variable is overwritten anyways.
Avoid appending an extra space if there is no extended information.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/panel/DiskList.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/panel/DiskList.js b/src/panel/DiskList.js
index cbf163c..ad965c8 100644
--- a/src/panel/DiskList.js
+++ b/src/panel/DiskList.js
@@ -237,7 +237,7 @@ Ext.define('Proxmox.DiskList', {
 	    width: 150,
 	    sortable: false,
 	    renderer: function(v, metaData, rec) {
-		let extendedInfo = ' ';
+		let extendedInfo = '';
 		if (rec) {
 		    let types = [];
 		    if (rec.data.osdid !== undefined && rec.data.osdid >= 0) {
-- 
2.20.1

[pve-devel] [PATCH widget-toolkit 07/11] disk list: factor out renderers for type and usage

[Fabian Ebner]

to be re-used later.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/panel/DiskList.js | 64 +++++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/src/panel/DiskList.js b/src/panel/DiskList.js
index ad965c8..10cf840 100644
--- a/src/panel/DiskList.js
+++ b/src/panel/DiskList.js
@@ -173,6 +173,39 @@ Ext.define('Proxmox.DiskList', {
 	},
     },
 
+    renderDiskType: function(v) {
+	if (v === undefined) return Proxmox.Utils.unknownText;
+	switch (v) {
+	    case 'ssd': return 'SSD';
+	    case 'hdd': return 'Hard Disk';
+	    case 'usb': return 'USB';
+	    default: return v;
+	}
+    },
+
+    renderDiskUsage: function(v, metaData, rec) {
+	let extendedInfo = '';
+	if (rec) {
+	    let types = [];
+	    if (rec.data.osdid !== undefined && rec.data.osdid >= 0) {
+		types.push(`OSD.${rec.data.osdid.toString()}`);
+	    }
+	    if (rec.data.journals > 0) {
+		types.push('Journal');
+	    }
+	    if (rec.data.db > 0) {
+		types.push('DB');
+	    }
+	    if (rec.data.wal > 0) {
+		types.push('WAL');
+	    }
+	    if (types.length > 0) {
+		extendedInfo = `, Ceph (${types.join(', ')})`;
+	    }
+	}
+	return v ? `${v}${extendedInfo}` : Proxmox.Utils.noText;
+    },
+
     tbar: [
 	{
 	    text: gettext('Reload'),
@@ -223,13 +256,8 @@ Ext.define('Proxmox.DiskList', {
 	    sortable: true,
 	    dataIndex: 'disk-type',
 	    renderer: function(v) {
-		if (v === undefined) return Proxmox.Utils.unknownText;
-		switch (v) {
-		    case 'ssd': return 'SSD';
-		    case 'hdd': return 'Hard Disk';
-		    case 'usb': return 'USB';
-		    default: return v;
-		}
+		let me = this;
+		return me.renderDiskType(v);
 	    },
 	},
 	{
@@ -237,26 +265,8 @@ Ext.define('Proxmox.DiskList', {
 	    width: 150,
 	    sortable: false,
 	    renderer: function(v, metaData, rec) {
-		let extendedInfo = '';
-		if (rec) {
-		    let types = [];
-		    if (rec.data.osdid !== undefined && rec.data.osdid >= 0) {
-			types.push(`OSD.${rec.data.osdid.toString()}`);
-		    }
-		    if (rec.data.journals > 0) {
-			types.push('Journal');
-		    }
-		    if (rec.data.db > 0) {
-			types.push('DB');
-		    }
-		    if (rec.data.wal > 0) {
-			types.push('WAL');
-		    }
-		    if (types.length > 0) {
-			extendedInfo = `, Ceph (${types.join(', ')})`;
-		    }
-		}
-		return v ? `${v}${extendedInfo}` : Proxmox.Utils.noText;
+		let me = this;
+		return me.renderDiskUsage(v, metaData, rec);
 	    },
 	    dataIndex: 'used',
 	},
-- 
2.20.1

[pve-devel] [PATCH widget-toolkit 08/11] disk list: move title bar initialization to initComponent

[Fabian Ebner]

to conditionally add more buttons later on.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/panel/DiskList.js | 80 ++++++++++++++++++++++++-------------------
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/src/panel/DiskList.js b/src/panel/DiskList.js
index 10cf840..c6f1638 100644
--- a/src/panel/DiskList.js
+++ b/src/panel/DiskList.js
@@ -206,42 +206,6 @@ Ext.define('Proxmox.DiskList', {
 	return v ? `${v}${extendedInfo}` : Proxmox.Utils.noText;
     },
 
-    tbar: [
-	{
-	    text: gettext('Reload'),
-	    handler: 'reload',
-	},
-	{
-	    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.parent ||
-		    (rec.data.used && rec.data.used !== 'unused')) {
-		    return false;
-		} else {
-		    return true;
-		}
-	    },
-	    handler: 'initGPT',
-	},
-    ],
-
     columns: [
 	{
 	    xtype: 'treecolumn',
@@ -332,4 +296,48 @@ Ext.define('Proxmox.DiskList', {
     listeners: {
 	itemdblclick: 'openSmartWindow',
     },
+
+    initComponent: function() {
+	let me = this;
+
+	let tbar = [
+	    {
+		text: gettext('Reload'),
+		handler: 'reload',
+	    },
+	    {
+		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.parent ||
+			(rec.data.used && rec.data.used !== 'unused')) {
+			return false;
+		    } else {
+			return true;
+		    }
+		},
+		handler: 'initGPT',
+	    },
+	];
+
+	me.tbar = tbar;
+
+	me.callParent();
+    },
 });
-- 
2.20.1

[pve-devel] [PATCH widget-toolkit 09/11] disk list: add wipe disk button

[Fabian Ebner]

which shows a confirm dialog with the most relevant information before actually
issuing the API call.

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

diff --git a/src/panel/DiskList.js b/src/panel/DiskList.js
index c6f1638..b155139 100644
--- a/src/panel/DiskList.js
+++ b/src/panel/DiskList.js
@@ -44,6 +44,8 @@ Ext.define('Proxmox.DiskList', {
     extend: 'Ext.tree.Panel',
     alias: 'widget.pmxDiskList',
 
+    supportsWipeDisk: false,
+
     rootVisible: false,
 
     emptyText: gettext('No Disks found'),
@@ -113,6 +115,34 @@ Ext.define('Proxmox.DiskList', {
 	    });
 	},
 
+	wipeDisk: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection();
+	    if (!selection || selection.length < 1) return;
+
+	    let rec = selection[0];
+	    Proxmox.Utils.API2Request({
+		url: `${view.exturl}/wipedisk`,
+		waitMsgTarget: view,
+		method: 'PUT',
+		params: { disk: rec.data.name },
+		failure: function(response, options) {
+		    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+		},
+		success: function(response, options) {
+		    var upid = response.result.data;
+		    var win = Ext.create('Proxmox.window.TaskProgress', {
+		        upid: upid,
+			taskDone: function() {
+			    me.reload();
+			},
+		    });
+		    win.show();
+		},
+	    });
+	},
+
 	init: function(view) {
 	    let nodename = view.nodename || 'localhost';
 	    view.baseurl = `/api2/json/nodes/${nodename}/disks`;
@@ -336,6 +366,57 @@ Ext.define('Proxmox.DiskList', {
 	    },
 	];
 
+	if (me.supportsWipeDisk) {
+	    tbar.push('-');
+	    tbar.push({
+		xtype: 'proxmoxButton',
+		text: gettext('Wipe Disk'),
+		parentXType: 'treepanel',
+		dangerous: true,
+		confirmMsg: function(rec) {
+		    const data = rec.data;
+
+		    let mainMessage = Ext.String.format(
+			gettext('Are you sure you want to wipe {0}?'),
+			data.devpath,
+		    );
+		    mainMessage += `<br> ${gettext('All data on the device will be lost!')}`;
+
+		    const type = me.renderDiskType(data["disk-type"]);
+
+		    let usage;
+		    if (data.children.length > 0) {
+			const partitionUsage = data.children.map(
+			    partition => me.renderDiskUsage(partition.used),
+			).join(', ');
+			usage = `${gettext('Partitions')} (${partitionUsage})`;
+		    } else {
+			usage = me.renderDiskUsage(data.used, undefined, rec);
+		    }
+
+		    const size = Proxmox.Utils.format_size(data.size);
+		    const serial = Ext.String.htmlEncode(data.serial);
+
+		    let additionalInfo = `${gettext('Type')}: ${type}<br>`;
+		    additionalInfo += `${gettext('Usage')}: ${usage}<br>`;
+		    additionalInfo += `${gettext('Size')}: ${size}<br>`;
+		    additionalInfo += `${gettext('Serial')}: ${serial}`;
+
+		    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',
+	    });
+	}
+
 	me.tbar = tbar;
 
 	me.callParent();
-- 
2.20.1

[pve-devel] [PATCH manager 10/11] ui: add task description for wipe disk

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/Utils.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index db3faaef..e66435ed 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1733,6 +1733,7 @@ Ext.define('PVE.Utils', {
 	    startall: ['', gettext('Start all VMs and Containers')],
 	    stopall: ['', gettext('Stop all VMs and Containers')],
 	    unknownimgdel: ['', gettext('Destroy image from unknown guest')],
+	    wipedisk: ['Device', gettext('Wipe Disk')],
 	    vncproxy: ['VM/CT', gettext('Console')],
 	    vncshell: ['', gettext('Shell')],
 	    vzclone: ['CT', gettext('Clone')],
-- 
2.20.1

[pve-devel] [PATCH manager 11/11] ui: disk list: enable wipe disk button

[Fabian Ebner]

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

Depends on pve-storage and proxmox-widget-toolkit.

 www/manager6/node/Config.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
index 8bc1d3f8..845e89e6 100644
--- a/www/manager6/node/Config.js
+++ b/www/manager6/node/Config.js
@@ -279,6 +279,7 @@ Ext.define('PVE.node.Config', {
 		    iconCls: 'fa fa-hdd-o',
 		    nodename: nodename,
 		    includePartitions: true,
+		    supportsWipeDisk: true,
 		},
 		{
 		    xtype: 'pveLVMList',
-- 
2.20.1

[pve-devel] applied: [PATCH widget-toolkit 06/11] disk list: fix minor usage renderer issue

[Thomas Lamprecht]

On 23.04.21 12:14, Fabian Ebner wrote:
> If there is extended information, the variable is overwritten anyways.
> Avoid appending an extra space if there is no extended information.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  src/panel/DiskList.js | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!

[pve-devel] applied: [PATCH widget-toolkit 07/11] disk list: factor out renderers for type and usage

[Thomas Lamprecht]

On 23.04.21 12:14, Fabian Ebner wrote:
> to be re-used later.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  src/panel/DiskList.js | 64 +++++++++++++++++++++++++------------------
>  1 file changed, 37 insertions(+), 27 deletions(-)
> 
>

applied, thanks!

[pve-devel] applied-partially: [PATCH-SERIES storage/widget-toolkit/manager] Add wipe disk api call and button

[Thomas Lamprecht]

On 23.04.21 12:14, Fabian Ebner wrote:
> so admins wipe disks that are not actually used, but contain left-overs.
> 
> The last patch needs dependency bumps for pve-storage and
> proxmox-widget-toolkit.
> 
> storage:
> 
> Fabian Ebner (5):
>   diskmanage: add wipe_blockdev method
>   diskmanage: factor out mounted_blockdevs helper
>   diskmanage: add is_mounted method
>   diskmanage: add has_holder method
>   api: add wipedisk call
> 
>  PVE/API2/Disks.pm | 42 ++++++++++++++++++++++
>  PVE/Diskmanage.pm | 91 ++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 129 insertions(+), 4 deletions(-)
> 

applied the storage part, with a followup to add the single partitions to the wipefs
command to ensure that really all FS signatures are gone, else, if one creates a
similar partition layout later a mkfs. call may complain about potential existing FS.

[pve-devel] applied-series: [PATCH-SERIES storage/widget-toolkit/manager] Add wipe disk api call and button

[Thomas Lamprecht]

On 23.04.21 12:14, Fabian Ebner wrote:
> widget-toolkit:
> 
> Fabian Ebner (4):
>   disk list: fix minor usage renderer issue
>   disk list: factor out renderer for type and usage
>   disk list: move title bar initialization to initComponent
>   disk list: add wipe disk button
> 
>  src/panel/DiskList.js | 223 ++++++++++++++++++++++++++++++------------
>  1 file changed, 161 insertions(+), 62 deletions(-)
> 
> 
> manager:
> 
> Fabian Ebner (2):
>   ui: add task description for wipe disk
>   ui: disk list: enable wipe disk button
> 
>  www/manager6/Utils.js       | 1 +
>  www/manager6/node/Config.js | 1 +
>  2 files changed, 2 insertions(+)
> 


applied the remaining patches.

Next things to do:

* enabled wiping for partitions
* hint that disks recognized as OSDs should use the OSD destroy dialogue if possible (i.e., if
  that disk is actually an OSD in the current cluster and all is still healthy enough for that)
* I'd change the top bar in general more like the OSD tree one, i.e., only reload left and the
  wipe/smart/init GPT buttons on the right with the selected disk text prefixed
* The confirm dialogue feels like it could get still some improvement about how to convey
  what will be destroyed more nicely, but as I do not have concrete proposals I just mention
  it for the sake of it, maybe someone else has better ideas.