Re: [pbs-devel] [PATCH proxmox-backup/proxmox-widget-toolkit v2 0/4] add partitions to disks/list endpoint

[Hannes Laimer <h.laimer@proxmox.com>]

Am 15.06.22 um 10:08 schrieb Dominik Csapak:
> On 6/15/22 09:58, Wolfgang Bumiller wrote:
>> On Wed, Jun 08, 2022 at 08:51:50AM +0000, Hannes Laimer wrote:
>>> In order to work with the existing frontend component for displying the
>>> disklist, either
>>> * the partition data has to be return with the same struct a disk is
>>>    returned. Which leads to the same struct being used for different
>>>    things -> quite a few fields are always empty for partitions and a 
>>> new
>>>    'type' field would be needed. Also the code structure in PBS has 
>>> to be
>>>    changed quite a bit.
>>> * the existing frontend has to be adjusted to handle data from PVE and
>>>    PBS properly.
>>>
>>> I went with the second option because the adjustments nedded in the UI
>>> compenent were minimal and, IMHO, adjusting the API to fit the UI is the
>>> wrong direction.
>>>
>>> For the mount status to shown in the UI, the patch[1] sent to 
>>> pve-devel for
>>> the 'mounted' column is needed.
>>>
>>> NOTE: The partition data will be needed in later patches for removable
>>> datastores.
>>>
>>> v1->v2:
>>>   * use builder pattern for queries as suggested by Wolfgang
>>>   * move mounted out of Enum and add filesystem string
>>>   * add missing zfsreserve usage type
>>>   * add 'mounted' column to disklist (separate patch[1])
>>>
>>>
>>> [1] https://lists.proxmox.com/pipermail/pve-devel/2022-June/053211.html
>>
>> @Dominik: Not sure how to deal with the above patch linked, since AFAICT
>> right now the added column is (currently) pbs specific.
>>
>> Should the visibility of the mounted column be configurable?
>> Then again, we probably want to change PVE::Diskmanage & API to return
>> the mounted boolean as well? Currently it just slams a " (mounted)" text
>> onto the file system type, which is rather awkward (and does not happen
>> at all for eg. an ESP partition, which can definitely be mounted...).
>>
>> Though I suppose the series would still work without it, so I guess we
>> can apply this regardless?
> 
> 
> mhmm... i agree that we probably want to add that column for pve too...
> as an interim solution we could have a 'computed' field/renderer
> that takes either the 'mounted' column or tries to parse the '(mounted)' 
> part
> of the 'used' one.
> 
> once we have the mounted field in pve too, we can drop that and switch
> to that
> 
btw, there was a seconds patch that added 'mounted' to the pve API [1], 
I just did not link it

[1] https://lists.proxmox.com/pipermail/pve-devel/2022-June/053213.html