[pve-devel] [PATCH manager 1/2] api: osd: return block devices instead of dm node

[pve-devel] [PATCH manager 1/2] api: osd: return block devices instead of dm node

[Aaron Lauterer]

Returning the block devices is more useful than the device node. The
device node usually points to the DM device for bluestore OSDs:
/dev/dm-x

In almost all situations one will be interested in the physical device
underneath, /dev/sdX or /dev/nvmeXnY. In the rare case that someone
isn't, then one can get a lot of more information by running
`ceph osd metadata <osd nr>`.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/API2/Ceph/OSD.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index 93433b3a..c3d1384e 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -143,9 +143,9 @@ __PACKAGE__->register_method ({
 	    if ($e->{type} eq 'osd' && $osdmd) {
 		if ($osdmd->{bluefs}) {
 		    $new->{osdtype} = 'bluestore';
-		    $new->{blfsdev} = $osdmd->{bluestore_bdev_dev_node};
-		    $new->{dbdev} = $osdmd->{bluefs_db_dev_node};
-		    $new->{waldev} = $osdmd->{bluefs_wal_dev_node};
+		    $new->{blfsdev} = $osdmd->{bluestore_bdev_devices};
+		    $new->{dbdev} = $osdmd->{bluefs_db_devices};
+		    $new->{waldev} = $osdmd->{bluefs_wal_devices};
 		} else {
 		    $new->{osdtype} = 'filestore';
 		}
-- 
2.30.2

[pve-devel] [PATCH manager 2/2] ui: osd: remove NA from wal device column

[Aaron Lauterer]

It adds visual noise and reduces readability.
This brings the WAL device column in alignment with the DB device column
which is showing whitespace if there is none configured.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 www/manager6/ceph/OSD.js | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js
index 78f226ff..adc35464 100644
--- a/www/manager6/ceph/OSD.js
+++ b/www/manager6/ceph/OSD.js
@@ -516,15 +516,6 @@ Ext.define('PVE.node.CephOsdTree', {
 	    return text;
 	},
 
-	render_wal: function(value, metaData, rec) {
-	    if (!value &&
-		rec.data.osdtype === 'bluestore' &&
-		rec.data.type === 'osd') {
-		return 'N/A';
-	    }
-	    return value;
-	},
-
 	render_version: function(value, metadata, rec) {
 	    let vm = this.getViewModel();
 	    let versions = vm.get('versions');
@@ -638,7 +629,6 @@ Ext.define('PVE.node.CephOsdTree', {
 	    text: "WAL Device",
 	    dataIndex: 'waldev',
 	    align: 'right',
-	    renderer: 'render_wal',
 	    width: 75,
 	    hidden: true,
 	},
-- 
2.30.2

Re: [pve-devel] [PATCH manager 1/2] api: osd: return block devices instead of dm node

[Thomas Lamprecht]

On 28.03.22 13:10, Aaron Lauterer wrote:
> Returning the block devices is more useful than the device node. The
> device node usually points to the DM device for bluestore OSDs:
> /dev/dm-x
> 
> In almost all situations one will be interested in the physical device
> underneath, /dev/sdX or /dev/nvmeXnY. In the rare case that someone
> isn't, then one can get a lot of more information by running
> `ceph osd metadata <osd nr>`.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  PVE/API2/Ceph/OSD.pm | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
> index 93433b3a..c3d1384e 100644
> --- a/PVE/API2/Ceph/OSD.pm
> +++ b/PVE/API2/Ceph/OSD.pm
> @@ -143,9 +143,9 @@ __PACKAGE__->register_method ({
>  	    if ($e->{type} eq 'osd' && $osdmd) {
>  		if ($osdmd->{bluefs}) {
>  		    $new->{osdtype} = 'bluestore';
> -		    $new->{blfsdev} = $osdmd->{bluestore_bdev_dev_node};
> -		    $new->{dbdev} = $osdmd->{bluefs_db_dev_node};
> -		    $new->{waldev} = $osdmd->{bluefs_wal_dev_node};
> +		    $new->{blfsdev} = $osdmd->{bluestore_bdev_devices};
> +		    $new->{dbdev} = $osdmd->{bluefs_db_devices};
> +		    $new->{waldev} = $osdmd->{bluefs_wal_devices};
>  		} else {
>  		    $new->{osdtype} = 'filestore';
>  		}


"device_ids" (e.g.:  "sdb=HGST_HDN724040ALE640_PK2334PEGNVD6T") could be also interesting,
but probably separately as its a bit longer but still, could avoid a extra shell lookup.

For the UI patch: do we want to hint when the db/wal is on the OSD itself as fallback?

Re: [pve-devel] [PATCH manager 1/2] api: osd: return block devices instead of dm node

[Aaron Lauterer]

On 3/29/22 18:05, Thomas Lamprecht wrote:
> On 28.03.22 13:10, Aaron Lauterer wrote:
>> Returning the block devices is more useful than the device node. The
>> device node usually points to the DM device for bluestore OSDs:
>> /dev/dm-x
>>
>> In almost all situations one will be interested in the physical device
>> underneath, /dev/sdX or /dev/nvmeXnY. In the rare case that someone
>> isn't, then one can get a lot of more information by running
>> `ceph osd metadata <osd nr>`.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>>   PVE/API2/Ceph/OSD.pm | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
>> index 93433b3a..c3d1384e 100644
>> --- a/PVE/API2/Ceph/OSD.pm
>> +++ b/PVE/API2/Ceph/OSD.pm
>> @@ -143,9 +143,9 @@ __PACKAGE__->register_method ({
>>   	    if ($e->{type} eq 'osd' && $osdmd) {
>>   		if ($osdmd->{bluefs}) {
>>   		    $new->{osdtype} = 'bluestore';
>> -		    $new->{blfsdev} = $osdmd->{bluestore_bdev_dev_node};
>> -		    $new->{dbdev} = $osdmd->{bluefs_db_dev_node};
>> -		    $new->{waldev} = $osdmd->{bluefs_wal_dev_node};
>> +		    $new->{blfsdev} = $osdmd->{bluestore_bdev_devices};
>> +		    $new->{dbdev} = $osdmd->{bluefs_db_devices};
>> +		    $new->{waldev} = $osdmd->{bluefs_wal_devices};
>>   		} else {
>>   		    $new->{osdtype} = 'filestore';
>>   		}
> 
> 
> "device_ids" (e.g.:  "sdb=HGST_HDN724040ALE640_PK2334PEGNVD6T") could be also interesting,
> but probably separately as its a bit longer but still, could avoid a extra shell lookup.

"device_ids" will contain a list of all devices used. With a bit of additional logic we could match that against the different devices to determine which is the bluestore, db and wal device and show the second identifier in a tooltip or so.

I would add that as new return values, keeping in mind your other response about changing that one property too much.

> 
> For the UI patch: do we want to hint when the db/wal is on the OSD itself as fallback?

Hmm, let me think and try it out how we can not clutter up that view but still convey the info.

There are a few ways this could be in reality.
If no DB or WAL is set, both are on the bluestore/OSD dev. If a DB dev is set, but no WAL, then the WAL is on the DB dev. If no DB dev is set, but a WAL, then the DB should be on the OSD.

Re: [pve-devel] [PATCH manager 1/2] api: osd: return block devices instead of dm node

[Thomas Lamprecht]

On 30.03.22 10:19, Aaron Lauterer wrote:
> 
> 
> On 3/29/22 18:05, Thomas Lamprecht wrote:
>> On 28.03.22 13:10, Aaron Lauterer wrote:
>>> Returning the block devices is more useful than the device node. The
>>> device node usually points to the DM device for bluestore OSDs:
>>> /dev/dm-x
>>>
>>> In almost all situations one will be interested in the physical device
>>> underneath, /dev/sdX or /dev/nvmeXnY. In the rare case that someone
>>> isn't, then one can get a lot of more information by running
>>> `ceph osd metadata <osd nr>`.
>>>
>>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>>> ---
>>>   PVE/API2/Ceph/OSD.pm | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
>>> index 93433b3a..c3d1384e 100644
>>> --- a/PVE/API2/Ceph/OSD.pm
>>> +++ b/PVE/API2/Ceph/OSD.pm
>>> @@ -143,9 +143,9 @@ __PACKAGE__->register_method ({
>>>           if ($e->{type} eq 'osd' && $osdmd) {
>>>           if ($osdmd->{bluefs}) {
>>>               $new->{osdtype} = 'bluestore';
>>> -            $new->{blfsdev} = $osdmd->{bluestore_bdev_dev_node};
>>> -            $new->{dbdev} = $osdmd->{bluefs_db_dev_node};
>>> -            $new->{waldev} = $osdmd->{bluefs_wal_dev_node};
>>> +            $new->{blfsdev} = $osdmd->{bluestore_bdev_devices};
>>> +            $new->{dbdev} = $osdmd->{bluefs_db_devices};
>>> +            $new->{waldev} = $osdmd->{bluefs_wal_devices};
>>>           } else {
>>>               $new->{osdtype} = 'filestore';
>>>           }
>>
>>
>> "device_ids" (e.g.:  "sdb=HGST_HDN724040ALE640_PK2334PEGNVD6T") could be also interesting,
>> but probably separately as its a bit longer but still, could avoid a extra shell lookup.
> 
> "device_ids" will contain a list of all devices used. With a bit of additional logic we could match that against the different devices to determine which is the bluestore, db and wal device and show the second identifier in a tooltip or so.
> 
> I would add that as new return values, keeping in mind your other response about changing that one property too much.
> 
>>
>> For the UI patch: do we want to hint when the db/wal is on the OSD itself as fallback?
> 
> Hmm, let me think and try it out how we can not clutter up that view but still convey the info.
> 
> There are a few ways this could be in reality.
> If no DB or WAL is set, both are on the bluestore/OSD dev. If a DB dev is set, but no WAL, then the WAL is on the DB dev. If no DB dev is set, but a WAL, then the DB should be on the OSD.

summarizing a short off-list discussion that happened before I could read this mail:
Plan is to add an OSD info window (including matching API endpoint) that shows a nice
overview of the status and backing device details, memory usage, I though also that
OSD creation time would be cool, but this doesn't seem to be recorded, at least not
in the metadata tree, but that's already details anyway.

This has better visibility for in-depth info without cluttering the tree view and is
something that I thought of as "nice to have" already in the past.

Re: [pve-devel] [PATCH manager 1/2] api: osd: return block devices instead of dm node

[Thomas Lamprecht]

On 28.03.22 13:10, Aaron Lauterer wrote:
> Returning the block devices is more useful than the device node. The
> device node usually points to the DM device for bluestore OSDs:
> /dev/dm-x
> 
> In almost all situations one will be interested in the physical device
> underneath, /dev/sdX or /dev/nvmeXnY. In the rare case that someone
> isn't, then one can get a lot of more information by running
> `ceph osd metadata <osd nr>`.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  PVE/API2/Ceph/OSD.pm | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
> index 93433b3a..c3d1384e 100644
> --- a/PVE/API2/Ceph/OSD.pm
> +++ b/PVE/API2/Ceph/OSD.pm
> @@ -143,9 +143,9 @@ __PACKAGE__->register_method ({
>  	    if ($e->{type} eq 'osd' && $osdmd) {
>  		if ($osdmd->{bluefs}) {
>  		    $new->{osdtype} = 'bluestore';
> -		    $new->{blfsdev} = $osdmd->{bluestore_bdev_dev_node};
> -		    $new->{dbdev} = $osdmd->{bluefs_db_dev_node};
> -		    $new->{waldev} = $osdmd->{bluefs_wal_dev_node};
> +		    $new->{blfsdev} = $osdmd->{bluestore_bdev_devices};
> +		    $new->{dbdev} = $osdmd->{bluefs_db_devices};
> +		    $new->{waldev} = $osdmd->{bluefs_wal_devices};
>  		} else {
>  		    $new->{osdtype} = 'filestore';
>  		}

oh, and note that this was previously a full device path that could be used by tools
interfacing with the API to open the backing device directly, but now this cannot be
done anymore, and while I would see a change to another full path view of the backing
device (/dev/sdX or /dev/disk/by-XYZ/...) as breaking change, dropping the path
characteristics is almost a bit too much for my liking...