Hi Igor,
On Wed, August 30, 2023 6:49 PM, Igor Mammedov wrote:
>
> On Wed, 30 Aug 2023 10:03:33 +0000
> "Liu, Jing2" <jing2....@intel.com> wrote:
>
...
> > > > +/*
> > > > + * Get MSI-X enabled, but no vector enabled, by setting vector 0
> > > > +with an invalid
> > > > + * fd to kernel.
> > > > + */
> > > > +static int vfio_enable_msix_no_vec(VFIOPCIDevice *vdev)> +{
> > > > + struct vfio_irq_set *irq_set;
> > >
> > > This could be a 'g_autofree' variable.
> >
> > Thanks for pointing this to me. I just realized QEMU doc recommends
> > g_autofree/g_autoptr to do automatic memory deallocation.
> >
> > I will replace to g_autofree style in next version.
> >
> > I have a question for a specific example (not related to this patch),
> > and I failed to find an example in current QEMU code to understand it.
> > If one g_autofree structure includes a pointer that we also allocate
> > space for the inside pointer, could g_autofree release the inside space?
>
> it might be too cumbersome for 1-off use see following for how 'auto' works
> https://docs.gtk.org/glib/auto-cleanup.html
Thank you for the guide. It's now clear to me that for such type, there need
define
specific free function by macros and g_auto can help call it automatically.
Jing
>
> > struct dummy1 {
> > int data;
> > struct *p;
> > }
> > struct p {
> > char member[];
> > }
> > void func() {
> > g_autofree struct *dummy1 = NULL;
> >
> > dummy1 = g_malloc();
> > dummy1->p = g_malloc();
> > ...
> > return; // is this correct or need g_free(p)?
> > }
> >
> > >
> > > > + int ret = 0, argsz;
> > > > + int32_t *fd;
> > > > +
> > > > + argsz = sizeof(*irq_set) + sizeof(*fd);
> > > > +
> > > > + irq_set = g_malloc0(argsz);
> > > > + irq_set->argsz = argsz;
> > > > + irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> > > > + VFIO_IRQ_SET_ACTION_TRIGGER;
> > > > + irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
> > > > + irq_set->start = 0;
> > > > + irq_set->count = 1;
> > > > + fd = (int32_t *)&irq_set->data;
> > > > + *fd = -1;
> > > > +
> > > > + ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
> > > > + if (ret) {
> > > > + error_report("vfio: failed to enable MSI-X with vector 0
> > > > trick, %d",
> > > > + ret);
> > >
> > > The above message seems redundant. I would simply return 'ret' and
> > > let the caller report the error. Same as vfio_enable_vectors().
> >
> > Understood. Will change.
> >
> > Thanks,
> > Jing
> >
> > > Thanks,
> > >
> > > C.
