On Thu, Sep 9, 2021 at 7:43 PM Michal Prívozník <mprivozn@redhat.com> 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@redhat.com>
> ---
>  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.
I recheck it and dropped some cases in v2: https://listman.redhat.com/archives/libvir-list/2021-October/msg00492.html

BTW, is there any way to add Gavi(gavi@nvidia.com) as co-writer when merging the patches? Because the most contents of docs/formatdomain.rst  in the patch 2/3 come from his patch https://listman.redhat.com/archives/libvir-list/2021-May/msg00143.html

Michal