On 8/19/20 10:08 AM, Stefan Hajnoczi wrote: > On Tue, Aug 18, 2020 at 06:45:05PM +0200, Philippe Mathieu-Daudé wrote: >> In preparation of using multiple IRQ (thus multiple eventfds) >> make BDRVNVMeState::irq_notifier an array (for now of a single >> element, the admin queue notifier). >> >> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> >> --- >> block/nvme.c | 28 ++++++++++++++++++---------- >> 1 file changed, 18 insertions(+), 10 deletions(-) > > This looks like an intermediate step before using multiple irqs. I think > it makes the code confusing because on one hand INDEX_ADMIN gives the > impression that INDEX_IO() should be used for io queues, while on the > other hand only a single EventNotifier is allocated and we actually > can't use INDEX_IO() yet. > > If this intermediate patch is really necessary, please don't use > INDEX_ADMIN. Define a new constant instead: > > /* This driver shares a single MSIX IRQ for the admin and I/O queues */ > #define MSIX_SHARED_IRQ_IDX 0 > > In the future the array index can be changed to INDEX_ADMIN and > INDEX_IO(n) when there are multiple EventNotifiers. > > I think that would make the code clearer.
Very good idea, thanks!
