On Fri,  6 Aug 2021 14:43:52 -0700
Steve Sistare <steven.sist...@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


Reply via email to