On Fri, 14 Jun 2019 11:41:41 -0400
Eric Farman <far...@linux.ibm.com> wrote:
> On 6/14/19 11:06 AM, Auger Eric wrote:
> > Hi Eric,
> >
> > On 6/14/19 4:30 PM, Eric Farman wrote:
> >>
> >>
> >> On 6/14/19 5:27 AM, Cornelia Huck wrote:
> >>> Use the new helper.
> >>>
> >>> Signed-off-by: Cornelia Huck <coh...@redhat.com>
> >>> ---
> >>> hw/vfio/ccw.c | 68 +++++++++++----------------------------------------
> >>> 1 file changed, 14 insertions(+), 54 deletions(-)
> >>>
> >>> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> >>> index 03a2becb3ec9..3dc08721a3db 100644
> >>> --- a/hw/vfio/ccw.c
> >>> +++ b/hw/vfio/ccw.c
> >>> @@ -197,10 +197,7 @@ read_err:
> >>> static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error
> >>> **errp)
> >>> {
> >>> VFIODevice *vdev = &vcdev->vdev;
> >>> - struct vfio_irq_info *irq_info;
> >>> - struct vfio_irq_set *irq_set;
> >>> - size_t argsz;
> >>> - int32_t *pfd;
> >>> + int fd;
> >>>
> >>> if (vdev->num_irqs < VFIO_CCW_IO_IRQ_INDEX + 1) {
> >>> error_setg(errp, "vfio: unexpected number of io irqs %u",
> >>> @@ -208,72 +205,35 @@ static void
> >>> vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp)
> >>> return;
> >>> }
> >>>
> >>> - argsz = sizeof(*irq_info);
> >>> - irq_info = g_malloc0(argsz);
> >>> - irq_info->index = VFIO_CCW_IO_IRQ_INDEX;
> >>> - irq_info->argsz = argsz;
> >>> - if (ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO,
> >>> - irq_info) < 0 || irq_info->count < 1) {
> >>> - error_setg_errno(errp, errno, "vfio: Error getting irq info");
> >>> - goto out_free_info;
> >>> - }
> >>> -
> >>
> >> Don't we still need this hunk? (And the out_free_info label stuff that
> >> cleans it up.) I don't see vfio_set_irq_signaling() covering it.
> >
> > Looks this IRQ index is always implemented and exposed by the driver,
> > isn't it? In such a case it shouldn't be needed to test its presence?
>
> Right; if we were running on an old kernel, both ioctl's would fail the
> same way since they were added concurrently. So the check today doesn't
> seem very useful.
>
> But since it's there, and we're taking it out, it got me wondering
> whether there are/were intentions to expand GET_IRQ_INFO in the future.
> I'm not aware of any reason vfio-ccw would need to, but want some
> confirmation that I'm not overlooking anything.
It seems I actually was a bit too delete-happy... I did not intend to
rip out that code at all, it just somehow was suddenly gone :)
Checking is probably not strictly needed, but let me send a v2 that
does not delete it (and only converts to the new interface).
