[PATCH 3/4] qemu: move virtio capability validation
Boris Fiuczynski
fiuczy at linux.ibm.com
Mon Apr 27 10:10:13 UTC 2020
Looks OK to me.
Reviewed-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
On 4/23/20 3:15 PM, Bjoern Walk wrote:
> Move capability validation of virtio options from command line
> generation to post-parse device validation where it belongs.
>
> Signed-off-by: Bjoern Walk <bwalk at linux.ibm.com>
> ---
> src/qemu/qemu_command.c | 41 ++++++--------------
> src/qemu/qemu_validate.c | 70 +++++++++++++++++++++++++++++++--
> tests/qemuxml2argvtest.c | 84 ++++++++++++++++++++--------------------
> 3 files changed, 120 insertions(+), 75 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 95402fc4..ca9d3f10 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -588,39 +588,20 @@ qemuBuildVirtioDevStr(virBufferPtr buf,
>
> static int
> qemuBuildVirtioOptionsStr(virBufferPtr buf,
> - virDomainVirtioOptionsPtr virtio,
> - virQEMUCapsPtr qemuCaps)
> + virDomainVirtioOptionsPtr virtio)
> {
> if (!virtio)
> return 0;
>
> if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) {
> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("the iommu setting is not supported "
> - "with this QEMU binary"));
> - return -1;
> - }
> virBufferAsprintf(buf, ",iommu_platform=%s",
> virTristateSwitchTypeToString(virtio->iommu));
> }
> if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) {
> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_ATS)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("the ats setting is not supported with this "
> - "QEMU binary"));
> - return -1;
> - }
> virBufferAsprintf(buf, ",ats=%s",
> virTristateSwitchTypeToString(virtio->ats));
> }
> if (virtio->packed != VIR_TRISTATE_SWITCH_ABSENT) {
> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PACKED_QUEUES)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("the packed setting is not supported with this "
> - "QEMU binary"));
> - return -1;
> - }
> virBufferAsprintf(buf, ",packed=%s",
> virTristateSwitchTypeToString(virtio->packed));
> }
> @@ -2150,7 +2131,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
> virBufferAsprintf(&opt, ",num-queues=%u", disk->queues);
> }
>
> - if (qemuBuildVirtioOptionsStr(&opt, disk->virtio, qemuCaps) < 0)
> + if (qemuBuildVirtioOptionsStr(&opt, disk->virtio) < 0)
> return NULL;
>
> if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0)
> @@ -2615,7 +2596,7 @@ qemuBuildVHostUserFsCommandLine(virCommandPtr cmd,
> virBufferAsprintf(&opt, ",queue-size=%llu", fs->queue_size);
> virBufferAddLit(&opt, ",tag=");
> virQEMUBuildBufferEscapeComma(&opt, fs->dst);
> - if (qemuBuildVirtioOptionsStr(&opt, fs->virtio, priv->qemuCaps) < 0)
> + if (qemuBuildVirtioOptionsStr(&opt, fs->virtio) < 0)
> return -1;
>
> if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, priv->qemuCaps) < 0)
> @@ -2685,7 +2666,7 @@ qemuBuildFSDevStr(const virDomainDef *def,
> virBufferAddLit(&opt, ",mount_tag=");
> virQEMUBuildBufferEscapeComma(&opt, fs->dst);
>
> - if (qemuBuildVirtioOptionsStr(&opt, fs->virtio, qemuCaps) < 0)
> + if (qemuBuildVirtioOptionsStr(&opt, fs->virtio) < 0)
> return NULL;
>
> if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0)
> @@ -2917,7 +2898,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
> def->iothread);
> }
>
> - if (qemuBuildVirtioOptionsStr(&buf, def->virtio, qemuCaps) < 0)
> + if (qemuBuildVirtioOptionsStr(&buf, def->virtio) < 0)
> return -1;
> break;
> case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
> @@ -2964,7 +2945,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
> virBufferAsprintf(&buf, ",vectors=%d",
> def->opts.vioserial.vectors);
> }
> - if (qemuBuildVirtioOptionsStr(&buf, def->virtio, qemuCaps) < 0)
> + if (qemuBuildVirtioOptionsStr(&buf, def->virtio) < 0)
> return -1;
> break;
>
> @@ -3916,7 +3897,7 @@ qemuBuildNicDevStr(virDomainDefPtr def,
> if (bootindex)
> virBufferAsprintf(&buf, ",bootindex=%u", bootindex);
> if (usingVirtio &&
> - qemuBuildVirtioOptionsStr(&buf, net->virtio, qemuCaps) < 0)
> + qemuBuildVirtioOptionsStr(&buf, net->virtio) < 0)
> return NULL;
>
> return virBufferContentAndReset(&buf);
> @@ -4158,7 +4139,7 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd,
> virTristateSwitchTypeToString(def->memballoon->autodeflate));
> }
>
> - if (qemuBuildVirtioOptionsStr(&buf, def->memballoon->virtio, qemuCaps) < 0)
> + if (qemuBuildVirtioOptionsStr(&buf, def->memballoon->virtio) < 0)
> return -1;
>
> if (qemuCommandAddExtDevice(cmd, &def->memballoon->info) < 0)
> @@ -4250,7 +4231,7 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
> if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
> return NULL;
>
> - if (qemuBuildVirtioOptionsStr(&buf, dev->virtio, qemuCaps) < 0)
> + if (qemuBuildVirtioOptionsStr(&buf, dev->virtio) < 0)
> return NULL;
>
> return virBufferContentAndReset(&buf);
> @@ -4561,7 +4542,7 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
> if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0)
> return NULL;
>
> - if (qemuBuildVirtioOptionsStr(&buf, video->virtio, qemuCaps) < 0)
> + if (qemuBuildVirtioOptionsStr(&buf, video->virtio) < 0)
> return NULL;
>
> return virBufferContentAndReset(&buf);
> @@ -5777,7 +5758,7 @@ qemuBuildRNGDevStr(const virDomainDef *def,
> virBufferAddLit(&buf, ",period=1000");
> }
>
> - if (qemuBuildVirtioOptionsStr(&buf, dev->virtio, qemuCaps) < 0)
> + if (qemuBuildVirtioOptionsStr(&buf, dev->virtio) < 0)
> return NULL;
>
> if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index cb0ff8d6..76691a5b 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1045,6 +1045,40 @@ qemuValidateNetSupportsCoalesce(virDomainNetType type)
> }
>
>
> +static int
> +qemuValidateDomainVirtioOptions(const virDomainVirtioOptions *virtio,
> + virQEMUCapsPtr qemuCaps)
> +{
> + if (!virtio)
> + return 0;
> +
> + if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT &&
> + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("the iommu setting is not supported "
> + "with this QEMU binary"));
> + return -1;
> + }
> +
> + if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT &&
> + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_ATS)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("the ats setting is not supported with this "
> + "QEMU binary"));
> + return -1;
> + }
> +
> + if (virtio->packed != VIR_TRISTATE_SWITCH_ABSENT &&
> + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PACKED_QUEUES)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("the packed setting is not supported with this "
> + "QEMU binary"));
> + return -1;
> + }
> + return 0;
> +}
> +
> +
> static int
> qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net,
> virQEMUCapsPtr qemuCaps)
> @@ -1122,6 +1156,8 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net,
> _("tx_queue_size has to be a power of two"));
> return -1;
> }
> + if (qemuValidateDomainVirtioOptions(net->virtio, qemuCaps) < 0)
> + return -1;
> }
>
> if (net->mtu &&
> @@ -1469,12 +1505,15 @@ qemuValidateDomainSmartcardDef(const virDomainSmartcardDef *def,
>
> static int
> qemuValidateDomainRNGDef(const virDomainRNGDef *def,
> - virQEMUCapsPtr qemuCaps G_GNUC_UNUSED)
> + virQEMUCapsPtr qemuCaps)
> {
> if (def->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
> qemuValidateDomainChrSourceDef(def->source.chardev, qemuCaps) < 0)
> return -1;
>
> + if (qemuValidateDomainVirtioOptions(def->virtio, qemuCaps) < 0)
> + return -1;
> +
> return 0;
> }
>
> @@ -1818,6 +1857,9 @@ qemuValidateDomainDeviceDefVideo(const virDomainVideoDef *video,
> }
> }
>
> + if (qemuValidateDomainVirtioOptions(video->virtio, qemuCaps) < 0)
> + return -1;
> +
> return 0;
> }
>
> @@ -1916,6 +1958,11 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk,
> return -1;
> }
>
> + if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO &&
> + qemuValidateDomainVirtioOptions(disk->virtio, qemuCaps) < 0) {
> + return -1;
> + }
> +
> return 0;
> }
>
> @@ -2146,7 +2193,8 @@ virValidateControllerPCIModelNameToQEMUCaps(int modelName)
>
>
> static int
> -qemuValidateDomainDeviceDefControllerAttributes(const virDomainControllerDef *controller)
> +qemuValidateDomainDeviceDefControllerAttributes(const virDomainControllerDef *controller,
> + virQEMUCapsPtr qemuCaps)
> {
> if (!(controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
> (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI ||
> @@ -2177,6 +2225,13 @@ qemuValidateDomainDeviceDefControllerAttributes(const virDomainControllerDef *co
> _("'iothread' is only supported for virtio-scsi controller"));
> return -1;
> }
> + if (qemuValidateDomainVirtioOptions(controller->virtio, qemuCaps) < 0)
> + return -1;
> + }
> +
> + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL &&
> + qemuValidateDomainVirtioOptions(controller->virtio, qemuCaps) < 0) {
> + return -1;
> }
>
> return 0;
> @@ -2719,7 +2774,7 @@ qemuValidateDomainDeviceDefController(const virDomainControllerDef *controller,
> !qemuValidateCheckSCSIControllerModel(qemuCaps, controller->model))
> return -1;
>
> - if (qemuValidateDomainDeviceDefControllerAttributes(controller) < 0)
> + if (qemuValidateDomainDeviceDefControllerAttributes(controller, qemuCaps) < 0)
> return -1;
>
> switch ((virDomainControllerType)controller->type) {
> @@ -3056,6 +3111,9 @@ qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs,
> return -1;
> }
>
> + if (qemuValidateDomainVirtioOptions(fs->virtio, qemuCaps) < 0)
> + return -1;
> +
> return 0;
> }
>
> @@ -3324,6 +3382,9 @@ qemuValidateDomainDeviceDefInput(const virDomainInputDef *input,
> return -1;
> }
>
> + if (qemuValidateDomainVirtioOptions(input->virtio, qemuCaps) < 0)
> + return -1;
> +
> return 0;
> }
>
> @@ -3353,6 +3414,9 @@ qemuValidateDomainDeviceDefMemballoon(const virDomainMemballoonDef *memballoon,
> return -1;
> }
>
> + if (qemuValidateDomainVirtioOptions(memballoon->virtio, qemuCaps) < 0)
> + return -1;
> +
> return 0;
> }
>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 7ceb3aee..7a9c65f0 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -3039,48 +3039,48 @@ mymain(void)
> QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
> QEMU_CAPS_DEVICE_VHOST_USER_GPU,
> QEMU_CAPS_VIRTIO_PACKED_QUEUES);
> - DO_TEST_FAILURE("virtio-options-controller-iommu", QEMU_CAPS_VIRTIO_SCSI);
> - DO_TEST_FAILURE("virtio-options-disk-iommu", NONE);
> - DO_TEST_FAILURE("virtio-options-fs-iommu", NONE);
> - DO_TEST_FAILURE("virtio-options-input-iommu", QEMU_CAPS_VIRTIO_MOUSE,
> - QEMU_CAPS_VIRTIO_KEYBOARD);
> - DO_TEST_FAILURE("virtio-options-memballoon-iommu", NONE);
> - DO_TEST_FAILURE("virtio-options-net-iommu", NONE);
> - DO_TEST_FAILURE("virtio-options-rng-iommu", QEMU_CAPS_DEVICE_VIRTIO_RNG,
> - QEMU_CAPS_OBJECT_RNG_RANDOM);
> - DO_TEST_FAILURE("virtio-options-video-iommu", QEMU_CAPS_DEVICE_VIRTIO_GPU,
> - QEMU_CAPS_DEVICE_VIRTIO_GPU,
> - QEMU_CAPS_VIRTIO_GPU_VIRGL,
> - QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
> - QEMU_CAPS_DEVICE_VHOST_USER_GPU);
> - DO_TEST_FAILURE("virtio-options-controller-ats", QEMU_CAPS_VIRTIO_SCSI);
> - DO_TEST_FAILURE("virtio-options-disk-ats", NONE);
> - DO_TEST_FAILURE("virtio-options-fs-ats", NONE);
> - DO_TEST_FAILURE("virtio-options-input-ats", QEMU_CAPS_VIRTIO_MOUSE,
> - QEMU_CAPS_VIRTIO_KEYBOARD);
> - DO_TEST_FAILURE("virtio-options-memballoon-ats", NONE);
> - DO_TEST_FAILURE("virtio-options-net-ats", NONE);
> - DO_TEST_FAILURE("virtio-options-rng-ats", QEMU_CAPS_DEVICE_VIRTIO_RNG,
> - QEMU_CAPS_OBJECT_RNG_RANDOM);
> - DO_TEST_FAILURE("virtio-options-video-ats", QEMU_CAPS_DEVICE_VIRTIO_GPU,
> - QEMU_CAPS_DEVICE_VIRTIO_GPU,
> - QEMU_CAPS_VIRTIO_GPU_VIRGL,
> - QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
> - QEMU_CAPS_DEVICE_VHOST_USER_GPU);
> - DO_TEST_FAILURE("virtio-options-controller-packed", QEMU_CAPS_VIRTIO_SCSI);
> - DO_TEST_FAILURE("virtio-options-disk-packed", NONE);
> - DO_TEST_FAILURE("virtio-options-fs-packed", NONE);
> - DO_TEST_FAILURE("virtio-options-input-packed", QEMU_CAPS_VIRTIO_MOUSE,
> - QEMU_CAPS_VIRTIO_KEYBOARD);
> - DO_TEST_FAILURE("virtio-options-memballoon-packed", NONE);
> - DO_TEST_FAILURE("virtio-options-net-packed", NONE);
> - DO_TEST_FAILURE("virtio-options-rng-packed", QEMU_CAPS_DEVICE_VIRTIO_RNG,
> - QEMU_CAPS_OBJECT_RNG_RANDOM);
> - DO_TEST_FAILURE("virtio-options-video-packed", QEMU_CAPS_DEVICE_VIRTIO_GPU,
> - QEMU_CAPS_DEVICE_VIRTIO_GPU,
> - QEMU_CAPS_VIRTIO_GPU_VIRGL,
> - QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
> - QEMU_CAPS_DEVICE_VHOST_USER_GPU);
> + DO_TEST_PARSE_ERROR("virtio-options-controller-iommu", QEMU_CAPS_VIRTIO_SCSI);
> + DO_TEST_PARSE_ERROR("virtio-options-disk-iommu", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-fs-iommu", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-input-iommu", QEMU_CAPS_VIRTIO_MOUSE,
> + QEMU_CAPS_VIRTIO_KEYBOARD);
> + DO_TEST_PARSE_ERROR("virtio-options-net-iommu", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-memballoon-iommu", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-rng-iommu", QEMU_CAPS_DEVICE_VIRTIO_RNG,
> + QEMU_CAPS_OBJECT_RNG_RANDOM);
> + DO_TEST_PARSE_ERROR("virtio-options-video-iommu", QEMU_CAPS_DEVICE_VIRTIO_GPU,
> + QEMU_CAPS_DEVICE_VIRTIO_GPU,
> + QEMU_CAPS_VIRTIO_GPU_VIRGL,
> + QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
> + QEMU_CAPS_DEVICE_VHOST_USER_GPU);
> + DO_TEST_PARSE_ERROR("virtio-options-controller-ats", QEMU_CAPS_VIRTIO_SCSI);
> + DO_TEST_PARSE_ERROR("virtio-options-disk-ats", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-fs-ats", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-input-ats", QEMU_CAPS_VIRTIO_MOUSE,
> + QEMU_CAPS_VIRTIO_KEYBOARD);
> + DO_TEST_PARSE_ERROR("virtio-options-memballoon-ats", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-net-ats", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-rng-ats", QEMU_CAPS_DEVICE_VIRTIO_RNG,
> + QEMU_CAPS_OBJECT_RNG_RANDOM);
> + DO_TEST_PARSE_ERROR("virtio-options-video-ats", QEMU_CAPS_DEVICE_VIRTIO_GPU,
> + QEMU_CAPS_DEVICE_VIRTIO_GPU,
> + QEMU_CAPS_VIRTIO_GPU_VIRGL,
> + QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
> + QEMU_CAPS_DEVICE_VHOST_USER_GPU);
> + DO_TEST_PARSE_ERROR("virtio-options-controller-packed", QEMU_CAPS_VIRTIO_SCSI);
> + DO_TEST_PARSE_ERROR("virtio-options-disk-packed", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-fs-packed", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-input-packed", QEMU_CAPS_VIRTIO_MOUSE,
> + QEMU_CAPS_VIRTIO_KEYBOARD);
> + DO_TEST_PARSE_ERROR("virtio-options-memballoon-packed", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-net-packed", NONE);
> + DO_TEST_PARSE_ERROR("virtio-options-rng-packed", QEMU_CAPS_DEVICE_VIRTIO_RNG,
> + QEMU_CAPS_OBJECT_RNG_RANDOM);
> + DO_TEST_PARSE_ERROR("virtio-options-video-packed", QEMU_CAPS_DEVICE_VIRTIO_GPU,
> + QEMU_CAPS_DEVICE_VIRTIO_GPU,
> + QEMU_CAPS_VIRTIO_GPU_VIRGL,
> + QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
> + QEMU_CAPS_DEVICE_VHOST_USER_GPU);
>
> DO_TEST("fd-memory-numa-topology", QEMU_CAPS_OBJECT_MEMORY_FILE,
> QEMU_CAPS_KVM);
>
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
More information about the libvir-list
mailing list