RE: [PATCH v1 2/2] vfio/mdev: Removed unused and redundant API for mdev UUID

[Parav Pandit <parav@mellanox.com>]

> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Thursday, August 8, 2019 2:00 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: kvm@vger.kernel.org; wankhede@nvidia.com; linux-
> kernel@vger.kernel.org; alex.williamson@redhat.com; cjia@nvidia.com
> Subject: Re: [PATCH v1 2/2] vfio/mdev: Removed unused and redundant API
> for mdev UUID
> 
> On Wed, 7 Aug 2019 16:33:11 +0000
> Parav Pandit <parav@mellanox.com> wrote:
> 
> > > -----Original Message-----
> > > From: Cornelia Huck <cohuck@redhat.com>
> > > Sent: Wednesday, August 7, 2019 2:58 PM
> > > To: Parav Pandit <parav@mellanox.com>
> > > Cc: kvm@vger.kernel.org; wankhede@nvidia.com; linux-
> > > kernel@vger.kernel.org; alex.williamson@redhat.com; cjia@nvidia.com
> > > Subject: Re: [PATCH v1 2/2] vfio/mdev: Removed unused and redundant
> > > API for mdev UUID
> > >
> > > On Tue,  6 Aug 2019 09:18:26 -0500
> > > Parav Pandit <parav@mellanox.com> wrote:
> > >
> > > > There is no single production driver who is interested in mdev
> > > > device uuid. Currently UUID is mainly used to derive a device name.
> > > > Additionally mdev device name is already available using core
> > > > kernel API dev_name().
> > >
> > > Well, the mdev code actually uses the uuid to check for duplicates
> > > before registration with the driver core would fail... I'd just drop
> > > the two sentences
> > Yes, it does the check. But its mainly used to derive a device name.
> > And to ensure that there are no two devices with duplicate name, it
> compares with the uuid.
> >
> > Even this 16 bytes storage is redundant.
> > Subsequently, I will submit a patch to get rid of storing this 16 bytes of
> UUID too.
> > Because for duplicate name check, device name itself is pretty good
> enough.
> >
> > Since I ran out of time and rc-4 is going on, I differed the 3rd simplification
> patch.
> 
> I'm not sure why we'd want to ditch the uuid; it's not like it is taking up huge
> amounts of space... and I see the device name being derived from the
> unique identifier that is the uuid, and not as the unique identifier itself.
>
Its just extra storage where ID is already present in device name.
Its redundant. Same functionality can be achieved without its storage, so it's better to simplify.
Anyways, will handle it right after this two patches.

I realized that I had typo in the email of Kirti. So resending it with corrected email.

> >
> > Commit message actually came from the thoughts of 3rd patch, but I see
> that without it, its not so intuitive.
> >
> > > talking about the device name, IMHO they don't really add useful
> > > information; but I'll leave that decision to the maintainers.
> > >
> > > >
> > > > Hence removed unused exported symbol.
> > > >
> > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > ---
> > > > Changelog:
> > > > v0->v1:
> > > >  - Updated commit log to address comments from Cornelia
> > > > ---
> > > >  drivers/vfio/mdev/mdev_core.c | 6 ------
> > > >  include/linux/mdev.h          | 1 -
> > > >  2 files changed, 7 deletions(-)
> > >
> > > Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> > Thanks for the review.