[pve-devel] [PATCH-SERIES v2 storage/widget-toolkit/manager] fix #3610: properly build ZFS detail tree

[pve-devel] [PATCH-SERIES v2 storage/widget-toolkit/manager] fix #3610: properly build ZFS detail tree

[Fabian Ebner]

Correctly add top-level vdevs like log or special as children of the
root, instead of the previous outer vdev.

Since the API returns only one element, it has to be the pool itself,
but there also is a vdev with the same name. In the GUI, hide the
pool and show its health as part of the upper grid instead.


Changes from v1:
    * add GUI patches


See the pve-manager patch for the needed dependency bumps.


pve-storage:

Fabian Ebner (1):
  fix #3610: properly build ZFS detail tree

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


proxmox-widget-toolkit:

Fabian Ebner (2):
  zfs detail: increase window height
  zfs detail: hide the pool itself in tree view

 src/window/ZFSDetail.js | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)


pve-manager:

Fabian Ebner (1):
  ui: node: zfs: use ZFSDetail window from widget-toolkit

 www/manager6/node/ZFS.js | 167 +--------------------------------------
 1 file changed, 3 insertions(+), 164 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH v2 storage 1/1] fix #3610: properly build ZFS detail tree

[Fabian Ebner]

Previously, top-level vdevs like log or special were wrongly added as
children of the previous outer vdev instead of the root.

Fix it by also showing the vdev with the same name as the pool and
start counting from level 1 (the pool itself serves as the root and
should be the only one with level 0). This results in the same kind
of structure as in PBS and (except for the root) zpool status itself.

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

No changes from v1.

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

diff --git a/PVE/API2/Disks/ZFS.pm b/PVE/API2/Disks/ZFS.pm
index 0418794..60077c4 100644
--- a/PVE/API2/Disks/ZFS.pm
+++ b/PVE/API2/Disks/ZFS.pm
@@ -240,8 +240,8 @@ __PACKAGE__->register_method ({
 		$config = 1;
 	    } elsif ($config && $line =~ m/^(\s+)(\S+)\s*(\S+)?(?:\s+(\S+)\s+(\S+)\s+(\S+))?\s*(.*)$/) {
 		my ($space, $name, $state, $read, $write, $cksum, $msg) = ($1, $2, $3, $4, $5, $6, $7);
-		if ($name ne "NAME" and $name ne $param->{name}) {
-		    my $lvl= int(length($space)/2); # two spaces per level
+		if ($name ne "NAME") {
+		    my $lvl = int(length($space) / 2) + 1; # two spaces per level
 		    my $vdev = {
 			name => $name,
 			msg => $msg,
-- 
2.30.2

[pve-devel] [PATCH v2 widget-toolkit 1/2] zfs detail: increase window height

[Fabian Ebner]

If there are mirrors and log/special vdevs it just feels too small.
It also doesn't help if there are errors in the upper part taking up
space. Make it 600, which was used in PVE before.

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

New in v2.

 src/window/ZFSDetail.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/window/ZFSDetail.js b/src/window/ZFSDetail.js
index 8eb6a87..5606ca0 100644
--- a/src/window/ZFSDetail.js
+++ b/src/window/ZFSDetail.js
@@ -45,7 +45,7 @@ Ext.define('Proxmox.window.ZFSDetail', {
 
     modal: true,
     width: 800,
-    height: 400,
+    height: 600,
     resizable: true,
     cbind: {
 	title: '{title}',
-- 
2.30.2

[pve-devel] [PATCH v2 widget-toolkit 2/2] zfs detail: hide the pool itself in tree view

[Fabian Ebner]

and show the overall health in the grid instead.

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

New in v2.

 src/window/ZFSDetail.js | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/window/ZFSDetail.js b/src/window/ZFSDetail.js
index 5606ca0..9ad46b8 100644
--- a/src/window/ZFSDetail.js
+++ b/src/window/ZFSDetail.js
@@ -78,6 +78,10 @@ Ext.define('Proxmox.window.ZFSDetail', {
 		nodename: '{nodename}',
 	    },
 	    rows: {
+		state: {
+		    header: gettext('Health'),
+		    renderer: Proxmox.Utils.render_zfs_health,
+		},
 		scan: {
 		    header: gettext('Scan'),
 		},
@@ -98,7 +102,8 @@ Ext.define('Proxmox.window.ZFSDetail', {
 	    title: gettext('Devices'),
 	    stateful: true,
 	    stateId: 'grid-node-zfsstatus',
-	    rootVisible: true,
+	    // the root is the pool itself and the information is shown by the grid
+	    rootVisible: false,
 	    fields: ['name', 'status',
 		{
 		    type: 'string',
-- 
2.30.2

[pve-devel] [PATCH v2 manager 1/1] ui: node: zfs: use ZFSDetail window from widget-toolkit

[Fabian Ebner]

The only functional differences I could see are the missing
defaultValue for 'Scan' and the reduced height. For restoring the
height, there is a proposed patch for widget-toolkit.

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

Needs a dependency bump for pve-storage, because new widget-toolkit
hides the root, but old storage still wants that to be shown.

The ZFSDetail window was already introduced in widget-toolkit 2.2-9,
so no new bump for that needed.

It can still make sense to bump widget-toolkit for the patches in this
series. That would avoid the situation with new storage + old
widget-toolkit showing the pool as the root and the vdev with the same
name, and the reduced height.

 www/manager6/node/ZFS.js | 167 +--------------------------------------
 1 file changed, 3 insertions(+), 164 deletions(-)

diff --git a/www/manager6/node/ZFS.js b/www/manager6/node/ZFS.js
index 8ea364bf..b2fcc87a 100644
--- a/www/manager6/node/ZFS.js
+++ b/www/manager6/node/ZFS.js
@@ -174,125 +174,6 @@ Ext.define('PVE.node.CreateZFS', {
     },
 });
 
-Ext.define('PVE.node.ZFSDevices', {
-    extend: 'Ext.tree.Panel',
-    xtype: 'pveZFSDevices',
-
-    stateful: true,
-    stateId: 'grid-node-zfsstatus',
-    columns: [
-	{
-	    xtype: 'treecolumn',
-	    text: gettext('Name'),
-	    dataIndex: 'name',
-	    flex: 1,
-	},
-	{
-	    text: gettext('Health'),
-	    renderer: PVE.Utils.render_zfs_health,
-	    dataIndex: 'state',
-	},
-	{
-	    text: 'READ',
-	    dataIndex: 'read',
-	},
-	{
-	    text: 'WRITE',
-	    dataIndex: 'write',
-	},
-	{
-	    text: 'CKSUM',
-	    dataIndex: 'cksum',
-	},
-	{
-	    text: gettext('Message'),
-	    dataIndex: 'msg',
-	},
-    ],
-
-    reload: function() {
-	let me = this;
-	let sm = me.getSelectionModel();
-	Proxmox.Utils.API2Request({
-	    url: `/nodes/${me.nodename}/disks/zfs/${me.zpool}`,
-	    waitMsgTarget: me,
-	    method: 'GET',
-	    failure: (response, opts) => Proxmox.Utils.setErrorMask(me, response.htmlStatus),
-	    success: function(response, opts) {
-		sm.deselectAll();
-		me.setRootNode(response.result.data);
-		me.expandAll();
-	    },
-	});
-    },
-
-    rootVisible: true,
-    selModel: 'treemodel',
-    fields: [
-	'name',
-	'status',
-	{
-	    type: 'string',
-	    name: 'iconCls',
-	    calculate: data => data.leaf ? `fa x-fa-tree fa-hdd-o` : undefined,
-	},
-    ],
-
-    initComponent: function() {
-        let me = this;
-
-	if (!me.nodename) {
-	    throw "no node name specified";
-	}
-	if (!me.zpool) {
-	    throw "no zpool specified";
-	}
-
-	me.callParent();
-
-	me.reload();
-    },
-});
-
-Ext.define('PVE.node.ZFSStatus', {
-    extend: 'Proxmox.grid.ObjectGrid',
-    xtype: 'pveZFSStatus',
-    layout: 'fit',
-    border: false,
-
-    initComponent: function() {
-	let me = this;
-
-	if (!me.nodename) {
-	    throw "no node name specified";
-	}
-	if (!me.zpool) {
-	    throw "no zpool specified";
-	}
-
-	me.url = `/api2/extjs/nodes/${me.nodename}/disks/zfs/${me.zpool}`;
-
-	me.rows = {
-	    scan: {
-		header: gettext('Scan'),
-		defaultValue: gettext('No Data'),
-	    },
-	    status: {
-		header: gettext('Status'),
-	    },
-	    action: {
-		header: gettext('Action'),
-	    },
-	    errors: {
-		header: gettext('Errors'),
-	    },
-	};
-
-	me.callParent();
-	me.reload();
-    },
-});
-
 Ext.define('PVE.node.ZFSList', {
     extend: 'Ext.grid.Panel',
     xtype: 'pveZFSList',
@@ -383,52 +264,10 @@ Ext.define('PVE.node.ZFSList', {
     show_detail: function(zpool) {
 	let me = this;
 
-	let detailsgrid = Ext.create('PVE.node.ZFSStatus', {
-	    layout: 'fit',
-	    nodename: me.nodename,
-	    flex: 0,
-	    zpool: zpool,
-	});
-	let devicetree = Ext.create('PVE.node.ZFSDevices', {
-	    title: gettext('Devices'),
+	Ext.create('Proxmox.window.ZFSDetail', {
+	    zpool,
 	    nodename: me.nodename,
-	    flex: 1,
-	    zpool: zpool,
-	});
-
-	Ext.create('Ext.window.Window', {
-	    modal: true,
-	    width: 800,
-	    height: 600,
-	    resizable: true,
-	    layout: 'fit',
-	    title: gettext('Status') + ': ' + zpool,
-	    items: [
-		{
-		    xtype: 'panel',
-		    region: 'center',
-		    layout: {
-			type: 'vbox',
-			align: 'stretch',
-		    },
-		    items: [
-			detailsgrid,
-			devicetree,
-		    ],
-		    tbar: [
-			{
-			    text: gettext('Reload'),
-			    iconCls: 'fa fa-refresh',
-			    handler: function() {
-				devicetree.reload();
-				detailsgrid.reload();
-			    },
-			},
-		    ],
-		},
-	    ],
-	    autoShow: true,
-	});
+	}).show();
     },
 
     set_button_status: function() {
-- 
2.30.2

[pve-devel] applied: [PATCH v2 storage 1/1] fix #3610: properly build ZFS detail tree

[Thomas Lamprecht]

On 10.09.21 13:45, Fabian Ebner wrote:
> Previously, top-level vdevs like log or special were wrongly added as
> children of the previous outer vdev instead of the root.
> 
> Fix it by also showing the vdev with the same name as the pool and
> start counting from level 1 (the pool itself serves as the root and
> should be the only one with level 0). This results in the same kind
> of structure as in PBS and (except for the root) zpool status itself.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> No changes from v1.
> 
>  PVE/API2/Disks/ZFS.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
>

applied, thanks!

Re: [pve-devel] [PATCH-SERIES v2 storage/widget-toolkit/manager] fix #3610: properly build ZFS detail tree

[Fabian Ebner]

Ping for the widget-toolkit and manager patches.

Am 10.09.21 um 13:45 schrieb Fabian Ebner:
> Correctly add top-level vdevs like log or special as children of the
> root, instead of the previous outer vdev.
> 
> Since the API returns only one element, it has to be the pool itself,
> but there also is a vdev with the same name. In the GUI, hide the
> pool and show its health as part of the upper grid instead.
> 
> 
> Changes from v1:
>      * add GUI patches
> 
> 
> See the pve-manager patch for the needed dependency bumps.
> 
> 
> pve-storage:
> 
> Fabian Ebner (1):
>    fix #3610: properly build ZFS detail tree
> 
>   PVE/API2/Disks/ZFS.pm | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 
> proxmox-widget-toolkit:
> 
> Fabian Ebner (2):
>    zfs detail: increase window height
>    zfs detail: hide the pool itself in tree view
> 
>   src/window/ZFSDetail.js | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> 
> pve-manager:
> 
> Fabian Ebner (1):
>    ui: node: zfs: use ZFSDetail window from widget-toolkit
> 
>   www/manager6/node/ZFS.js | 167 +--------------------------------------
>   1 file changed, 3 insertions(+), 164 deletions(-)
> 

[pve-devel] applied-series: [PATCH-SERIES v2 storage/widget-toolkit/manager] fix #3610: properly build ZFS detail tree

[Thomas Lamprecht]

On 13.01.22 12:21, Fabian Ebner wrote:
> Ping for the widget-toolkit and manager patches.
> 

applied remaining parts of this series, thanks!