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
Attachment:
signature.asc
Description: PGP signature
