Re: [PATCH 3/3] qemu: Add support for virtio device option paeg-per-vq

On Fri, Sep 17, 2021 at 3:17 PM Jonathon Jongsma <jjongsma(a)redhat.com&gt; wrote: > > On Thu, Sep 9, 2021 at 6:51 AM Michal Prívozník <mprivozn(a)redhat.com&gt; wrote: > > > > On 9/6/21 4:06 PM, Han Han wrote: > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1925363 > > > > > > Signed-off-by: Han Han <hhan(a)redhat.com&gt; > > > --- > > > src/qemu/qemu_command.c | 4 ++ > > > src/qemu/qemu_hotplug.c | 3 +- > > > src/qemu/qemu_validate.c | 8 ++++ > > > .../virtio-options-controller-page_per_vq.err | 1 + > > > ...-controller-page_per_vq.x86_64-latest.args | 37 ++++++++++++++++++ > > > .../virtio-options-controller-page_per_vq.xml | 38 ++++++++++++++++++ > > > .../virtio-options-disk-page_per_vq.err | 1 + > > > ...ptions-disk-page_per_vq.x86_64-latest.args | 39 +++++++++++++++++++ > > > .../virtio-options-disk-page_per_vq.xml | 34 ++++++++++++++++ > > > .../virtio-options-fs-page_per_vq.err | 1 + > > > ...-options-fs-page_per_vq.x86_64-latest.args | 37 ++++++++++++++++++ > > > .../virtio-options-fs-page_per_vq.xml | 34 ++++++++++++++++ > > > .../virtio-options-input-page_per_vq.err | 1 + > > > ...tions-input-page_per_vq.x86_64-latest.args | 35 +++++++++++++++++ > > > .../virtio-options-input-page_per_vq.xml | 30 ++++++++++++++ > > > .../virtio-options-memballoon-page_per_vq.err | 1 + > > > ...-memballoon-page_per_vq.x86_64-latest.args | 33 ++++++++++++++++ > > > .../virtio-options-memballoon-page_per_vq.xml | 23 +++++++++++ > > > .../virtio-options-net-page_per_vq.err | 1 + > > > ...options-net-page_per_vq.x86_64-latest.args | 37 ++++++++++++++++++ > > > .../virtio-options-net-page_per_vq.xml | 34 ++++++++++++++++ > > > .../virtio-options-rng-page_per_vq.err | 1 + > > > ...options-rng-page_per_vq.x86_64-latest.args | 37 ++++++++++++++++++ > > > .../virtio-options-rng-page_per_vq.xml | 32 +++++++++++++++ > > > .../virtio-options-video-page_per_vq.err | 1 + > > > ...tions-video-page_per_vq.x86_64-latest.args | 37 ++++++++++++++++++ > > > .../virtio-options-video-page_per_vq.xml | 36 +++++++++++++++++ > > > .../virtio-options.x86_64-latest.args | 26 ++++++------- > > > tests/qemuxml2argvdata/virtio-options.xml | 26 ++++++------- > > > tests/qemuxml2argvtest.c | 22 +++++++++++ > > > 30 files changed, 623 insertions(+), 27 deletions(-) > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-page_per_vq.err > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-page_per_vq.x86_64-latest.args > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-controller-page_per_vq.xml > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-page_per_vq.err > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-page_per_vq.x86_64-latest.args > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-disk-page_per_vq.xml > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-page_per_vq.err > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-page_per_vq.x86_64-latest.args > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-fs-page_per_vq.xml > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-input-page_per_vq.err > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-input-page_per_vq.x86_64-latest.args > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-input-page_per_vq.xml > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-memballoon-page_per_vq.err > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-memballoon-page_per_vq.x86_64-latest.args > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-memballoon-page_per_vq.xml > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-net-page_per_vq.err > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-net-page_per_vq.x86_64-latest.args > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-net-page_per_vq.xml > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-rng-page_per_vq.err > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-rng-page_per_vq.x86_64-latest.args > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-rng-page_per_vq.xml > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-video-page_per_vq.err > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-video-page_per_vq.x86_64-latest.args > > > create mode 100644 tests/qemuxml2argvdata/virtio-options-video-page_per_vq.xml > > > > Wow, that's a lot of test cases. Do we need all of them? > > > > > > > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > > > index b230314f7f..549f11dbe8 100644 > > > --- a/src/qemu/qemu_command.c > > > +++ b/src/qemu/qemu_command.c > > > @@ -645,6 +645,10 @@ qemuBuildVirtioOptionsStr(virBuffer *buf, > > > virBufferAsprintf(buf, ",packed=%s", > > > virTristateSwitchTypeToString(virtio->packed)); > > > } > > > + if (virtio->page_per_vq != VIR_TRISTATE_SWITCH_ABSENT) { > > > + virBufferAsprintf(buf, ",page-per-vq=%s", > > > + virTristateSwitchTypeToString(virtio->page_per_vq)); > > > + } > > > } > > > > > > static int > > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > > > index 9c16ab4567..f2553a6831 100644 > > > --- a/src/qemu/qemu_hotplug.c > > > +++ b/src/qemu/qemu_hotplug.c > > > @@ -3678,7 +3678,8 @@ qemuDomainChangeNet(virQEMUDriver *driver, > > > (olddev->virtio && newdev->virtio && > > > (olddev->virtio->iommu != newdev->virtio->iommu || > > > olddev->virtio->ats != newdev->virtio->ats || > > > - olddev->virtio->packed != newdev->virtio->packed))) { > > > + olddev->virtio->packed != newdev->virtio->packed || > > > + olddev->virtio->page_per_vq != newdev->virtio->page_per_vq))) { > > > virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", > > > _("cannot modify virtio network device driver options")); > > > goto cleanup; > > > diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c > > > index 1a470f1ff5..93e8b55651 100644 > > > --- a/src/qemu/qemu_validate.c > > > +++ b/src/qemu/qemu_validate.c > > > @@ -1478,6 +1478,14 @@ qemuValidateDomainVirtioOptions(const virDomainVirtioOptions *virtio, > > > "QEMU binary")); > > > return -1; > > > } > > > + > > > + if (virtio->page_per_vq != VIR_TRISTATE_SWITCH_ABSENT && > > > + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PAGE_PER_VQ)) { > > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > > > + _("the page_per_vq setting is not supported with this " > > > + "QEMU binary")); > > > > While we like lines to be 80 chars most we have an exception for error > > messages so that they can be easily 'git grep'-ped. > > > > I can fix this before pushing, but please let me know if we really need > > all the test cases. They seem to be very similar thus I'm in doubt. > > > > Michal > > > > Also, please fix the typo in the commit message: "paeg" -> "page" > > Jonathon Han, this patch series has been sitting for a few weeks now. Would you like help tying up the final loose ends and getting it upstream? OK. New version:

Jonathon