Re: [Qemu-devel] [RFC PATCH] vfio: Add sysfsdev property for pci & platform

[Alex Williamson <alex.williamson@redhat.com>]

On Tue, 2016-01-26 at 16:03 +0100, Eric Auger wrote:
> 
> Hi Alex,
> 
> I did a try with both legacy cmd line and new one and it works fine for
> vfio platform too:
> -device vfio-calxeda-xgmac,host="fff51000.ethernet"
> -device
> vfio-calxeda-xgmac,sysfsdev="/sys/bus/platform/devices/fff51000.ethernet"
> 
> Tested-by: Eric Auger <eric.auger@linaro.org>
> Reviewed-by: Eric Auger <eric.auger@linaro.org>

Thanks!

> just 1 question below.
...
> > diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> > index 289b498..99f0642 100644
> > --- a/hw/vfio/platform.c
> > +++ b/hw/vfio/platform.c
> > @@ -559,38 +559,45 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
> >  {
> >      VFIOGroup *group;
> >      VFIODevice *vbasedev_iter;
> > -    char path[PATH_MAX], iommu_group_path[PATH_MAX], *group_name;
> > +    char *tmp, group_path[PATH_MAX], *group_name;
> >      ssize_t len;
> >      struct stat st;
> >      int groupid;
> >      int ret;
> >  
> > -    /* name must be set prior to the call */
> > -    if (!vbasedev->name || strchr(vbasedev->name, '/')) {
> > -        return -EINVAL;
> > -    }
> > +    /* @sysfsdev takes precedence over @host */
> > +    if (vbasedev->sysfsdev) {
> > +        g_free(vbasedev->name);
> > +        vbasedev->name = g_strdup(basename(vbasedev->sysfsdev));
> do we need the g_strdup here?


Versus pointing ->name to the offset within sysfsdev where the name
starts?  My concern was that both @sysfsdev and @name are allocated via
device properties and presumably automatically collected when the
device is destroyed.  If I set one within the buffer of another, I'd
likely get a double free.  So creating a new string buffer seemed like
the safest approach.  Agree?  Thanks,

Alex