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

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Wed, Jun 15, 2022 at 10:10:37AM +0200, Hannes Laimer wrote:
> 
> 
> 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

Ah, I should have just used my mail client instead of clicking the link,
would have been more obvious ;-)