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