On Tue, Feb 18, 2025 at 12:15:46PM +0100, Peter Krempa wrote:
> On Tue, Feb 18, 2025 at 13:53:31 +0800, Stefan Hajnoczi wrote:
> > On Fri, Feb 14, 2025 at 05:29:34PM +0100, Peter Krempa wrote:
> > > The first part of the series refactors the existing code for reuse and
> > > then uses the new helpers to implement the feature.
> > >
> > > Note that this series is in RFC state as the qemu patches are still
> > > being discussed. Thus also the capability bump is not final.
> > >
> > > Also note that we should discuss the libvirt interface perhaps as it
> > > turns out that 'virtio-scsi' has two internal queues that need to be
> > > mapped as well.
> > >
> > > For now I've solved this administratively by instructing users to also
> > > add mapping for queue '0' and '1' which are the special ones in case of
> > > virtio-scsi.
> >
> > Thanks for tackling this upcoming QEMU feature! I thought about the
> > pros/cons of exposing all virtqueues in iothread-vq-mapping= whereas
> > just the command virtqueues are exposed by num_queues=. Although it's
> > confusing and inconvenient that the implicit ctrl and event virtqueues
> > need to be covered by iothread-vq-mapping= but are not counted in
> > num_queues=, I'd rather not introduce magic to hide this detail. If
> > users do need to control these virtqueues explicitly then the magic will
> > get in the way.
>
> I thought about it a bit as well and I think we should:
>
> 1) Document the round robin assignment a bit more prominently, so that
> users don't feel compelled to do a full mapping. Currently the
> example spells out the full maping configuration, so the first
> example should be the simpler one.
Good idea.
> 2) Modify the qemu code so that the mapping XML will explicitly name the
> 'ctrl/event' queues. Leave the numbered ones for the explicit ones
> configured via queues. That way it'll be obvious what's happening.
>
> <driver name='qemu' queues='2'>
> <iothreads>
> <iothread id='2'>
> <queue id='event'/>
> <queue id='ctrl'/>
> </iothread>
> <iothread id='3'>
> <queue id='0'/>
> </iothread>
> <iothread id='4'>
> <queue id='1'/>
> </iothread>
> </iothreads>
> </driver>
>
>
> Libvirt will then map the above config to mapping for queues 0,1,2,3 for
> use with qemu. That way there will be no ambiguity and weirdness when
> comparing the config between virtio-blk and virtio-scsi.
Nice!
> > Does anyone have a different opinion?
>
> I agree. We should hint the users to use the simpler configs and use the
> full mapping only if necessary. When using the full mapping the above
> XML will IMO make it obvious what's happening.
>
> In qemu no change will be needed although the role of queues 0 and 1
> should be formalized in docs so that it doesn't change later on at least
> without notifying libvirt so that we adapt if needed.
The roles of the queues are defined in the VIRTIO specification. ctrl
and event are always 0 and 1. The only way they could change is if a new
feature bit is introduced, but this is very unlikely.
https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-3450002
Stefan