>-----Original Message----- >From: Eric Auger <eric.au...@redhat.com> >Sent: Wednesday, September 27, 2023 5:16 PM >Subject: Re: [PATCH v2 08/12] vfio/ap: Use vfio_[attach/detach]_device > >Hi Zhenzhong, > >On 9/26/23 13:32, Zhenzhong Duan wrote: >> From: Eric Auger <eric.au...@redhat.com> >> >> Let the vfio-ap device use vfio_attach_device() and >> vfio_detach_device(), hence hiding the details of the used >> IOMMU backend. >> >> Signed-off-by: Eric Auger <eric.au...@redhat.com> >> Signed-off-by: Yi Liu <yi.l....@intel.com> >> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com> >> --- >> hw/vfio/ap.c | 68 +++++++++------------------------------------------- >> 1 file changed, 11 insertions(+), 57 deletions(-) >> >> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c >> index 6e21d1da5a..16ea7fb3c2 100644 >> --- a/hw/vfio/ap.c >> +++ b/hw/vfio/ap.c >> @@ -53,40 +53,6 @@ struct VFIODeviceOps vfio_ap_ops = { >> .vfio_compute_needs_reset = vfio_ap_compute_needs_reset, >> }; >> >> -static void vfio_ap_put_device(VFIOAPDevice *vapdev) >> -{ >> - g_free(vapdev->vdev.name); >> - vfio_put_base_device(&vapdev->vdev); >> -} >> - >> -static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp) >> -{ >> - GError *gerror = NULL; >> - char *symlink, *group_path; >> - int groupid; >> - >> - symlink = g_strdup_printf("%s/iommu_group", vapdev->vdev.sysfsdev); >> - group_path = g_file_read_link(symlink, &gerror); >> - g_free(symlink); >> - >> - if (!group_path) { >> - error_setg(errp, "%s: no iommu_group found for %s: %s", >> - TYPE_VFIO_AP_DEVICE, vapdev->vdev.sysfsdev, >> gerror->message); >> - g_error_free(gerror); >> - return NULL; >> - } >> - >> - if (sscanf(basename(group_path), "%d", &groupid) != 1) { >> - error_setg(errp, "vfio: failed to read %s", group_path); >> - g_free(group_path); >> - return NULL; >> - } >> - >> - g_free(group_path); >> - >> - return vfio_get_group(groupid, &address_space_memory, errp); >> -} >> - >> static void vfio_ap_req_notifier_handler(void *opaque) >> { >> VFIOAPDevice *vapdev = opaque; >> @@ -189,22 +155,15 @@ static void >vfio_ap_unregister_irq_notifier(VFIOAPDevice *vapdev, >> static void vfio_ap_realize(DeviceState *dev, Error **errp) >> { >> int ret; >> - char *mdevid; >> Error *err = NULL; >> - VFIOGroup *vfio_group; >> APDevice *apdev = AP_DEVICE(dev); >> VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev); >> + VFIODevice *vbasedev = &vapdev->vdev; >> >> - vfio_group = vfio_ap_get_group(vapdev, errp); >> - if (!vfio_group) { >> - return; >> - } >> - >> - vapdev->vdev.ops = &vfio_ap_ops; >> - vapdev->vdev.type = VFIO_DEVICE_TYPE_AP; >> - mdevid = basename(vapdev->vdev.sysfsdev); >> - vapdev->vdev.name = g_strdup_printf("%s", mdevid); >> - vapdev->vdev.dev = dev; >> + vbasedev->name = g_path_get_basename(vbasedev->sysfsdev); >I think we shall document in the commit msg the fact we use
Yes, will do. > >g_path_get_basename instead of basename here to match other device init >see 3e015d815b use g_path_get_basename instead of basename > >also leak of vbasedev->name I free it in vfio_ap_unrealize(). Thanks Zhenzhong