Re: [PATCH V6 18/27] vfio-pci: refactor for cpr

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

On Fri,  6 Aug 2021 14:43:52 -0700
Steve Sistare <steven.sistare@oracle.com> wrote:

> Export vfio_address_spaces and vfio_listener_skipped_section.
> Add optional name arg to vfio_add_kvm_msi_virq.
> Refactor vector use into a helper vfio_vector_init.
> All for use by cpr in a subsequent patch.  No functional change.

Why is the name arg optional?  It seems really inconsistent to me that
everything other than MSI/X uses this with a name, but MSI/X use NULL
and in an entirely separate pre-save step we then iterate through all
the {event,irq}fds to save them.  If we asked for a named notifier,
shouldn't we go ahead and save it under that name at that time?  ie.

static int vfio_named_notifier_init(VFIOPCIDevice *vdev, EventNotifier *e,
                                    const char *name, int nr)
{
    int ret, fd = load_event_fd(vdev, name, nr);

    if (fd >= 0) {
        event_notifier_init_fd(e, fd);
    } else {
        ret = event_notifier_init(e, 0);
        if (ret) {
            return ret;
        }
        save_event_fd(vdev, name, nr, e);
    }
    return 0;
}

Are we not doing this to avoid runtime overhead?

In the process, maybe we can use more descriptive names than
"interrupt", ex. "msi" or "msix".

It also feels a bit forced to me that the entire fd saving uses {name,
id} but vfio is the only caller that makes use of a non-zero id.
Should we instead just wrap all the calls from vfio to append the id to
the name so the common code can just use strcmp()?  Thanks,

Alex