Re: [PATCH v1 08/24] qemu_command.c: move QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE check to qemu_validate.c

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 10/14/20 10:42 PM, Daniel Henrique Barboza wrote:
> Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
> ---
>   src/qemu/qemu_command.c                                | 10 +++-------
>   src/qemu/qemu_validate.c                               |  8 ++++++++
>   .../net-virtio-rxqueuesize-invalid-size.err            |  2 +-
>   tests/qemuxml2xmltest.c                                |  3 ++-
>   4 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1629fac659..acbd5b1234 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3401,14 +3401,10 @@ qemuBuildNicDevStr(virDomainDefPtr def,
>               virBufferAsprintf(&buf, ",mq=on,vectors=%zu", 2 * vhostfdSize + 2);
>           }
>       }
> -    if (usingVirtio && net->driver.virtio.rx_queue_size) {
> -        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE)) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                           _("virtio rx_queue_size option is not supported with this QEMU binary"));
> -            return NULL;
> -        }
> +
> +    if (usingVirtio && net->driver.virtio.rx_queue_size)
>           virBufferAsprintf(&buf, ",rx_queue_size=%u", net->driver.virtio.rx_queue_size);
> -    }
> +
>       if (usingVirtio && net->driver.virtio.tx_queue_size) {
>           if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE)) {
>               virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index c874a05f5b..7be5420af1 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1261,6 +1261,14 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net,
>               return -1;
>           }
>   
> +        if (net->driver.virtio.rx_queue_size &&
> +            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("virtio rx_queue_size option is not supported "
> +                             "with this QEMU binary"));
> +            return -1;
> +        }
> +
>           if (net->driver.virtio.rx_queue_size & (net->driver.virtio.rx_queue_size - 1)) {
>               virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                              _("rx_queue_size has to be a power of two"));
> diff --git a/tests/qemuxml2argvdata/net-virtio-rxqueuesize-invalid-size.err b/tests/qemuxml2argvdata/net-virtio-rxqueuesize-invalid-size.err
> index c7f8d7b471..fcb5fc866f 100644
> --- a/tests/qemuxml2argvdata/net-virtio-rxqueuesize-invalid-size.err
> +++ b/tests/qemuxml2argvdata/net-virtio-rxqueuesize-invalid-size.err
> @@ -1 +1 @@
> -unsupported configuration: rx_queue_size has to be a power of two
> +unsupported configuration: virtio rx_queue_size option is not supported with this QEMU binary

Actually, I'd prefer if you added QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE 
flag to this test case and not touch this at all.

diff --git i/tests/qemuxml2argvtest.c w/tests/qemuxml2argvtest.c
index cf1d3c7ded..40ce346a73 100644
--- i/tests/qemuxml2argvtest.c
+++ w/tests/qemuxml2argvtest.c
@@ -1445,7 +1445,8 @@ mymain(void)
     DO_TEST("net-virtio-rxtxqueuesize",
             QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE,
             QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE);
-    DO_TEST_PARSE_ERROR("net-virtio-rxqueuesize-invalid-size", NONE);
+    DO_TEST_PARSE_ERROR("net-virtio-rxqueuesize-invalid-size",
+                        QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE);
     DO_TEST("net-virtio-teaming",
             QEMU_CAPS_VIRTIO_NET_FAILOVER,
             QEMU_CAPS_DEVICE_VFIO_PCI);


Michal