[PATCH v1 00/24] move more validations to qemu_validate.c
Hi, This is another step following up the work done in [1]. Recapping, the idea is to move any validation that can be done in parse/define time to qemu_validate.c instead of let it sit inside a command line build function in qemu_command.c. I'll again copy/paste the reasoning Cole Robinson gave for this work back then (see [2] for more info): ------- The benefits of moving to validate time is that XML is rejected by 'virsh define' rather than at 'virsh start' time. It also makes it easier to follow the cli building code, and makes it easier to verify qemu_command.c test suite code coverage for the important stuff like covering every CLI option. It's also a good intermediate step for sharing validation with domain capabilities building, like is done presently for rng models. ------- Not all validations were moved with these series, but I covered most of them. One thing worth noticing is the existence of 'post start' validations, most of them related to PCI addressing, and other notable cases such as NUMA nodes and CPU models, that can't be moved to parse time. The reason is that the QEMU driver will change them during start time. I contemplated move them to the existing qemuBuildCommandLineValidate(), but that would remove them from the hotplug path that uses the qemuBuild*DevStr() functions. I don't have an easy answer on how to handle these 'post start' validations ATM, so for now they remain in qemu_command.c. [1] https://www.redhat.com/archives/libvir-list/2019-December/msg00570.html [2] https://www.redhat.com/archives/libvir-list/2019-October/msg00568.html src/qemu/qemu_command.c | 379 +-------- src/qemu/qemu_command.h | 4 +- src/qemu/qemu_hotplug.c | 4 +- src/qemu/qemu_validate.c | 740 ++++++++++++++---- tests/qemuhotplugtest.c | 1 + .../disk-sata-incompatible-address.err | 2 +- .../net-virtio-rxqueuesize-invalid-size.err | 2 +- .../pseries-panic-address.err | 2 +- tests/qemuxml2argvtest.c | 28 +- tests/qemuxml2xmltest.c | 62 +- 10 files changed, 706 insertions(+), 518 deletions(-) -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 27 --------------------------- src/qemu/qemu_validate.c | 31 +++++++++++++++++++++++++++++-- tests/qemuxml2argvtest.c | 4 +++- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 697a2db62b..5a693e143f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1567,14 +1567,6 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, return NULL; } - if (disk->wwn && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_IDE_DRIVE_WWN)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Setting wwn for ide disk is not supported " - "by this QEMU")); - return NULL; - } - if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) virBufferAddLit(&opt, "ide-cd"); else @@ -1607,25 +1599,6 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, } } - if (disk->wwn && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_WWN)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Setting wwn for scsi disk is not supported " - "by this QEMU")); - return NULL; - } - - /* Properties wwn, vendor and product were introduced in the - * same QEMU release (1.2.0). - */ - if ((disk->vendor || disk->product) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_WWN)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Setting vendor or product for scsi disk is not " - "supported by this QEMU")); - return NULL; - } - controllerModel = qemuDomainFindSCSIControllerModel(def, &disk->info); if (controllerModel < 0) return NULL; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index bc3043bb3f..e747ce2cb0 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2066,13 +2066,40 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, _("Only ide and scsi disk support wwn")); return -1; } + + if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_IDE_DRIVE_WWN)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Setting wwn for ide disk is not supported " + "by this QEMU")); + return -1; + } + + if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_WWN)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Setting wwn for scsi disk is not supported " + "by this QEMU")); + return -1; + } } - if ((disk->vendor || disk->product) && - disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) { + if (disk->vendor || disk->product) { + if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Only scsi disk supports vendor and product")); return -1; + } + + /* Properties wwn, vendor and product were introduced in the + * same QEMU release (1.2.0). + */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_WWN)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Setting vendor or product for scsi disk is not " + "supported by this QEMU")); + return -1; + } } if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 8aa791d9f7..cd15ce5138 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2252,7 +2252,9 @@ mymain(void) DO_TEST_PARSE_ERROR("mach-virt-serial-invalid-machine", NONE); DO_TEST("disk-ide-split", NONE); - DO_TEST("disk-ide-wwn", QEMU_CAPS_IDE_DRIVE_WWN); + DO_TEST("disk-ide-wwn", + QEMU_CAPS_IDE_DRIVE_WWN, + QEMU_CAPS_SCSI_DISK_WWN); DO_TEST("disk-geometry", NONE); DO_TEST("disk-blockio", QEMU_CAPS_BLOCKIO); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 15 --------------- src/qemu/qemu_validate.c | 14 ++++++++++++++ tests/qemuxml2xmltest.c | 12 +++++++++--- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5a693e143f..b8b5ac1246 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1561,12 +1561,6 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, switch ((virDomainDiskBus) disk->bus) { case VIR_DOMAIN_DISK_BUS_IDE: - if (disk->info.addr.drive.target != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("target must be 0 for ide controller")); - return NULL; - } - if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) virBufferAddLit(&opt, "ide-cd"); else @@ -1590,15 +1584,6 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, break; case VIR_DOMAIN_DISK_BUS_SCSI: - if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_BLOCK)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support scsi-block for " - "lun passthrough")); - return NULL; - } - } - controllerModel = qemuDomainFindSCSIControllerModel(def, &disk->info); if (controllerModel < 0) return NULL; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index e747ce2cb0..10c770d318 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2112,6 +2112,15 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, return -1; } + if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_BLOCK)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support scsi-block for " + "lun passthrough")); + return -1; + } + + if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("copy_on_read is not compatible with 'lun' disk '%s'"), @@ -2165,6 +2174,11 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, _("Only 1 IDE controller is supported")); return -1; } + if (disk->info.addr.drive.target != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("target must be 0 for ide controller")); + return -1; + } break; case VIR_DOMAIN_DISK_BUS_FDC: diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2bf8dd5b14..d8040e8871 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -344,7 +344,8 @@ mymain(void) DO_TEST_CAPS_VER("disk-cache", "2.12.0"); DO_TEST_CAPS_LATEST("disk-cache"); DO_TEST("disk-network-nbd", NONE); - DO_TEST("disk-network-iscsi", QEMU_CAPS_VIRTIO_SCSI); + DO_TEST("disk-network-iscsi", QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_SCSI_BLOCK); DO_TEST("disk-network-gluster", NONE); DO_TEST("disk-network-rbd", NONE); DO_TEST("disk-network-source-auth", NONE); @@ -355,7 +356,9 @@ mymain(void) DO_TEST("disk-nvme", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_QCOW2_LUKS); DO_TEST_CAPS_LATEST("disk-scsi"); DO_TEST("disk-virtio-scsi-reservations", - QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_PR_MANAGER_HELPER); + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_PR_MANAGER_HELPER, + QEMU_CAPS_SCSI_BLOCK); DO_TEST("controller-virtio-scsi", QEMU_CAPS_VIRTIO_SCSI); DO_TEST("disk-virtio-s390-zpci", QEMU_CAPS_DEVICE_ZPCI, @@ -660,7 +663,10 @@ mymain(void) DO_TEST("numad-static-vcpu-no-numatune", NONE); DO_TEST("disk-scsi-lun-passthrough-sgio", - QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_DISK_WWN); + QEMU_CAPS_SCSI_LSI, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_SCSI_DISK_WWN, + QEMU_CAPS_SCSI_BLOCK); DO_TEST("disk-scsi-disk-vpd", QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_DISK_WWN); DO_TEST("disk-source-pool", NONE); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 24 ----------------------- src/qemu/qemu_validate.c | 41 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b8b5ac1246..9ec5ace1c7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1616,35 +1616,11 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, return NULL; if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { - if (disk->info.addr.drive.target != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("target must be 0 for controller " - "model 'lsilogic'")); - return NULL; - } - virBufferAsprintf(&opt, ",bus=%s.%d,scsi-id=%d", contAlias, disk->info.addr.drive.bus, disk->info.addr.drive.unit); } else { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { - if (disk->info.addr.drive.target > 7) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support target " - "greater than 7")); - return NULL; - } - - if (disk->info.addr.drive.bus != 0 && - disk->info.addr.drive.unit != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU only supports both bus and " - "unit equal to 0")); - return NULL; - } - } - virBufferAsprintf(&opt, ",bus=%s.0,channel=%d,scsi-id=%d,lun=%d", contAlias, disk->info.addr.drive.bus, diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 10c770d318..5ad13d9fd6 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2004,8 +2004,12 @@ qemuValidateDomainDeviceDefDiskSerial(const char *value) static int qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, + const virDomainDef *def, virQEMUCapsPtr qemuCaps) { + virDomainDeviceInfoPtr diskInfo; + int cModel; + if (disk->geometry.cylinders > 0 && disk->geometry.heads > 0 && disk->geometry.sectors > 0) { @@ -2146,6 +2150,9 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, switch (disk->bus) { case VIR_DOMAIN_DISK_BUS_SCSI: + diskInfo = (virDomainDeviceInfoPtr)&disk->info; + cModel = qemuDomainFindSCSIControllerModel(def, diskInfo); + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unexpected address type for scsi disk")); @@ -2160,6 +2167,38 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, "%s", _("SCSI controller only supports 1 bus")); return -1; } + + /* We allow hotplug/hotunplug disks without a controller, + * hence we don't error out if cModel is < 0. These + * validations were originally made under the assumption of + * a controller being found though. */ + if (cModel > 0) { + if (cModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { + if (disk->info.addr.drive.target != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("target must be 0 for controller " + "model 'lsilogic'")); + return -1; + } + } else { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { + if (disk->info.addr.drive.target > 7) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support target " + "greater than 7")); + return -1; + } + + if (disk->info.addr.drive.bus != 0 && + disk->info.addr.drive.unit != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU only supports both bus and " + "unit equal to 0")); + return -1; + } + } + } + } break; case VIR_DOMAIN_DISK_BUS_IDE: @@ -2433,7 +2472,7 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk, int idx; int partition; - if (qemuValidateDomainDeviceDefDiskFrontend(disk, qemuCaps) < 0) + if (qemuValidateDomainDeviceDefDiskFrontend(disk, def, qemuCaps) < 0) return -1; if (qemuValidateDomainDeviceDefDiskBlkdeviotune(disk, def, qemuCaps) < 0) -- 2.26.2
On 10/14/20 10:42 PM, Daniel Henrique Barboza wrote:
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 24 ----------------------- src/qemu/qemu_validate.c | 41 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b8b5ac1246..9ec5ace1c7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1616,35 +1616,11 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, return NULL;
if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { - if (disk->info.addr.drive.target != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("target must be 0 for controller " - "model 'lsilogic'")); - return NULL; - } - virBufferAsprintf(&opt, ",bus=%s.%d,scsi-id=%d", contAlias, disk->info.addr.drive.bus, disk->info.addr.drive.unit); } else { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { - if (disk->info.addr.drive.target > 7) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support target " - "greater than 7")); - return NULL; - } - - if (disk->info.addr.drive.bus != 0 && - disk->info.addr.drive.unit != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU only supports both bus and " - "unit equal to 0")); - return NULL; - } - } - virBufferAsprintf(&opt, ",bus=%s.0,channel=%d,scsi-id=%d,lun=%d", contAlias, disk->info.addr.drive.bus, diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 10c770d318..5ad13d9fd6 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2004,8 +2004,12 @@ qemuValidateDomainDeviceDefDiskSerial(const char *value)
static int qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, + const virDomainDef *def, virQEMUCapsPtr qemuCaps) { + virDomainDeviceInfoPtr diskInfo; + int cModel; + if (disk->geometry.cylinders > 0 && disk->geometry.heads > 0 && disk->geometry.sectors > 0) { @@ -2146,6 +2150,9 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk,
switch (disk->bus) { case VIR_DOMAIN_DISK_BUS_SCSI: + diskInfo = (virDomainDeviceInfoPtr)&disk->info; + cModel = qemuDomainFindSCSIControllerModel(def, diskInfo); + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unexpected address type for scsi disk")); @@ -2160,6 +2167,38 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, "%s", _("SCSI controller only supports 1 bus")); return -1; } + + /* We allow hotplug/hotunplug disks without a controller, + * hence we don't error out if cModel is < 0. These + * validations were originally made under the assumption of + * a controller being found though. */ + if (cModel > 0) { + if (cModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { + if (disk->info.addr.drive.target != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("target must be 0 for controller " + "model 'lsilogic'")); + return -1; + } + } else { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) {
This if() could be put right after 'else' and thus save one level of indentation. Michal
A few tweaks were made during the move: - the error messages were changed to mention 'sata controller' instead of 'ide controller'; - a check for address type 'drive' was added like it is done with other bus types. The error message of qemuxml2argdata was updated to reflect that now, instead of erroring it out from the common code in virDomainDiskDefValidate(), we're failing earlier with a different error message. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 11 ----------- src/qemu/qemu_validate.c | 19 +++++++++++++++++++ .../disk-sata-incompatible-address.err | 2 +- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9ec5ace1c7..b2c6bd43a2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1634,17 +1634,6 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, break; case VIR_DOMAIN_DISK_BUS_SATA: - if (disk->info.addr.drive.bus != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("bus must be 0 for ide controller")); - return NULL; - } - if (disk->info.addr.drive.target != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("target must be 0 for ide controller")); - return NULL; - } - if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) virBufferAddLit(&opt, "ide-cd"); else diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 5ad13d9fd6..289b99eea7 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2245,6 +2245,25 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, } break; + case VIR_DOMAIN_DISK_BUS_SATA: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unexpected address type for sata disk")); + return -1; + } + + if (disk->info.addr.drive.bus != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("bus must be 0 for sata controller")); + return -1; + } + if (disk->info.addr.drive.target != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("target must be 0 for sata controller")); + return -1; + } + break; + case VIR_DOMAIN_DISK_BUS_VIRTIO: case VIR_DOMAIN_DISK_BUS_XEN: case VIR_DOMAIN_DISK_BUS_SD: diff --git a/tests/qemuxml2argvdata/disk-sata-incompatible-address.err b/tests/qemuxml2argvdata/disk-sata-incompatible-address.err index cdb176b7d6..09395bcd6b 100644 --- a/tests/qemuxml2argvdata/disk-sata-incompatible-address.err +++ b/tests/qemuxml2argvdata/disk-sata-incompatible-address.err @@ -1 +1 @@ -unsupported configuration: Invalid address type 'pci' for the disk 'sda' with the bus type 'sata' +internal error: unexpected address type for sata disk -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 7 ------- src/qemu/qemu_validate.c | 10 ++++++++++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b2c6bd43a2..a5abd814a2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1683,13 +1683,6 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, } if (disk->queues) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("num-queues property isn't supported by this " - "QEMU binary")); - return NULL; - } - virBufferAsprintf(&opt, ",num-queues=%u", disk->queues); } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 289b99eea7..4a74265ced 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2265,6 +2265,16 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, break; case VIR_DOMAIN_DISK_BUS_VIRTIO: + if (disk->queues) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("num-queues property isn't supported by this " + "QEMU binary")); + return -1; + } + } + break; + case VIR_DOMAIN_DISK_BUS_XEN: case VIR_DOMAIN_DISK_BUS_SD: break; -- 2.26.2
On 10/14/20 10:42 PM, Daniel Henrique Barboza wrote:
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 7 ------- src/qemu/qemu_validate.c | 10 ++++++++++ 2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b2c6bd43a2..a5abd814a2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1683,13 +1683,6 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, }
if (disk->queues) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("num-queues property isn't supported by this " - "QEMU binary")); - return NULL; - } - virBufferAsprintf(&opt, ",num-queues=%u", disk->queues); }
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 289b99eea7..4a74265ced 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2265,6 +2265,16 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, break;
case VIR_DOMAIN_DISK_BUS_VIRTIO: + if (disk->queues) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES)) {
Could be merged into one if (). Michal
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 20 -------------------- src/qemu/qemu_validate.c | 25 +++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 6 +++--- tests/qemuxml2xmltest.c | 4 ++-- 4 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a5abd814a2..06ac175b67 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1693,19 +1693,6 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, break; case VIR_DOMAIN_DISK_BUS_USB: - if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("unexpected address type for usb disk")); - return NULL; - } - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support '-device " - "usb-storage'")); - return NULL; - - } virBufferAddLit(&opt, "usb-storage"); if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0) @@ -1772,13 +1759,6 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, virBufferAddLit(&opt, ",removable=on"); else virBufferAddLit(&opt, ",removable=off"); - } else { - if (disk->removable != VIR_TRISTATE_SWITCH_ABSENT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support setting the " - "removable flag of USB storage devices")); - return NULL; - } } } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 4a74265ced..beaa1aedce 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2275,6 +2275,31 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, } break; + case VIR_DOMAIN_DISK_BUS_USB: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support '-device " + "usb-storage'")); + return -1; + } + + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unexpected address type for usb disk")); + return -1; + } + + if (disk->removable != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_STORAGE_REMOVABLE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support setting the " + "removable flag of USB storage devices")); + return -1; + } + + break; + case VIR_DOMAIN_DISK_BUS_XEN: case VIR_DOMAIN_DISK_BUS_SD: break; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index cd15ce5138..cf1d3c7ded 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1220,14 +1220,14 @@ mymain(void) DO_TEST_PARSE_ERROR("disk-device-lun-type-invalid", QEMU_CAPS_VIRTIO_SCSI); DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-attaching-partition-nosupport"); - DO_TEST_FAILURE("disk-usb-nosupport", NONE); + DO_TEST_PARSE_ERROR("disk-usb-nosupport", NONE); DO_TEST("disk-usb-device", QEMU_CAPS_DEVICE_USB_STORAGE); DO_TEST("disk-usb-device-removable", QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_USB_STORAGE_REMOVABLE); - DO_TEST_FAILURE("disk-usb-pci", - QEMU_CAPS_DEVICE_USB_STORAGE); + DO_TEST_PARSE_ERROR("disk-usb-pci", + QEMU_CAPS_DEVICE_USB_STORAGE); DO_TEST_CAPS_LATEST("disk-scsi"); DO_TEST_CAPS_VER("disk-scsi-device-auto", "1.5.3"); DO_TEST_CAPS_LATEST("disk-scsi-device-auto"); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index d8040e8871..f4a5aa18f7 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -328,9 +328,9 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-aio-io_uring"); DO_TEST("disk-cdrom", NONE); DO_TEST_CAPS_LATEST("disk-cdrom-empty-network-invalid"); - DO_TEST("disk-cdrom-bus-other", NONE); + DO_TEST("disk-cdrom-bus-other", QEMU_CAPS_DEVICE_USB_STORAGE); DO_TEST("disk-floppy", NONE); - DO_TEST("disk-usb-device", NONE); + DO_TEST("disk-usb-device", QEMU_CAPS_DEVICE_USB_STORAGE); DO_TEST("disk-virtio", NONE); DO_TEST("floppy-drive-fat", NONE); DO_TEST("disk-virtio-queues", QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 4 ---- src/qemu/qemu_validate.c | 8 ++++++++ tests/qemuxml2xmltest.c | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 06ac175b67..1629fac659 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3330,10 +3330,6 @@ qemuBuildNicDevStr(virDomainDefPtr def, net->driver.virtio.txmode); return NULL; } - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio-net-pci 'tx' option not supported in this QEMU binary")); - return NULL; } } if (usingVirtio) { diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index beaa1aedce..c874a05f5b 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1253,6 +1253,14 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net, } if (virDomainNetIsVirtioModel(net)) { + if (net->driver.virtio.txmode && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TX_ALG)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio-net-pci 'tx' option not supported in " + "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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f4a5aa18f7..c1b70025b1 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -467,7 +467,7 @@ mymain(void) DO_TEST("net-user", NONE); DO_TEST("net-user-addr", NONE); DO_TEST("net-virtio", NONE); - DO_TEST("net-virtio-device", NONE); + DO_TEST("net-virtio-device", QEMU_CAPS_VIRTIO_TX_ALG); DO_TEST("net-virtio-disable-offloads", NONE); DO_TEST("net-eth", NONE); DO_TEST("net-eth-ifname", NONE); @@ -795,7 +795,7 @@ mymain(void) DO_TEST("numad-auto-memory-vcpu-no-cpuset-and-placement", NONE); DO_TEST("numad-auto-memory-vcpu-cpuset", NONE); DO_TEST("usb-ich9-ehci-addr", NONE); - DO_TEST("disk-copy_on_read", NONE); + DO_TEST("disk-copy_on_read", QEMU_CAPS_VIRTIO_TX_ALG); DO_TEST_CAPS_LATEST("tpm-passthrough"); DO_TEST_CAPS_LATEST("tpm-passthrough-crb"); DO_TEST_CAPS_LATEST("tpm-emulator"); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- 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 diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c1b70025b1..19beb85f96 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -474,7 +474,8 @@ mymain(void) DO_TEST("net-eth-hostip", NONE); DO_TEST("net-eth-unmanaged-tap", NONE); DO_TEST("net-virtio-network-portgroup", NONE); - DO_TEST("net-virtio-rxtxqueuesize", NONE); + DO_TEST("net-virtio-rxtxqueuesize", + QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE); DO_TEST("net-virtio-teaming", QEMU_CAPS_VIRTIO_NET_FAILOVER, QEMU_CAPS_DEVICE_VFIO_PCI); -- 2.26.2
On 10/14/20 10:42 PM, Daniel Henrique Barboza wrote:
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- 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
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 8 +------- src/qemu/qemu_validate.c | 8 ++++++++ tests/qemuxml2xmltest.c | 3 ++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index acbd5b1234..9f45faa490 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3405,14 +3405,8 @@ qemuBuildNicDevStr(virDomainDefPtr def, 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", - _("virtio tx_queue_size option is not supported with this QEMU binary")); - return NULL; - } + if (usingVirtio && net->driver.virtio.tx_queue_size) virBufferAsprintf(&buf, ",tx_queue_size=%u", net->driver.virtio.tx_queue_size); - } if (usingVirtio && net->mtu) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_HOST_MTU)) { diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 7be5420af1..9623ebed96 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1269,6 +1269,14 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net, return -1; } + if (net->driver.virtio.tx_queue_size && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio tx_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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 19beb85f96..e85f2188db 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -475,7 +475,8 @@ mymain(void) DO_TEST("net-eth-unmanaged-tap", NONE); DO_TEST("net-virtio-network-portgroup", NONE); DO_TEST("net-virtio-rxtxqueuesize", - QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE); + QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, + QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE); DO_TEST("net-virtio-teaming", QEMU_CAPS_VIRTIO_NET_FAILOVER, QEMU_CAPS_DEVICE_VFIO_PCI); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 9 ++------- src/qemu/qemu_validate.c | 8 ++++++++ tests/qemuxml2xmltest.c | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9f45faa490..62b8df1a13 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3408,14 +3408,9 @@ qemuBuildNicDevStr(virDomainDefPtr def, if (usingVirtio && net->driver.virtio.tx_queue_size) virBufferAsprintf(&buf, ",tx_queue_size=%u", net->driver.virtio.tx_queue_size); - if (usingVirtio && net->mtu) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_HOST_MTU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("setting MTU is not supported with this QEMU binary")); - return NULL; - } + if (usingVirtio && net->mtu) virBufferAsprintf(&buf, ",host_mtu=%u", net->mtu); - } + if (usingVirtio && net->teaming.type == VIR_DOMAIN_NET_TEAMING_TYPE_PERSISTENT) virBufferAddLit(&buf, ",failover=on"); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 9623ebed96..ebc70aaf00 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1277,6 +1277,14 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net, return -1; } + if (net->mtu && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_HOST_MTU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting MTU 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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e85f2188db..a9037101de 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -499,7 +499,7 @@ mymain(void) DO_TEST("watchdog", NONE); DO_TEST("net-bandwidth", QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_VNC); DO_TEST("net-bandwidth2", QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_VNC); - DO_TEST("net-mtu", NONE); + DO_TEST("net-mtu", QEMU_CAPS_VIRTIO_NET_HOST_MTU); DO_TEST("net-coalesce", NONE); DO_TEST("net-many-models", NONE); -- 2.26.2
We have a lot of "if (usingVirtio)" checks being done while constructing the NIC command line. Let's put all of them in a single "if". Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 50 ++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 62b8df1a13..a4f1cbce11 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3306,8 +3306,9 @@ qemuBuildNicDevStr(virDomainDefPtr def, virBufferAddStr(&buf, virDomainNetGetModelString(net)); } - if (usingVirtio && net->driver.virtio.txmode) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TX_ALG)) { + if (usingVirtio) { + if (net->driver.virtio.txmode && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TX_ALG)) { virBufferAddLit(&buf, ",tx="); switch (net->driver.virtio.txmode) { case VIR_DOMAIN_NET_VIRTIO_TX_MODE_IOTHREAD: @@ -3331,8 +3332,6 @@ qemuBuildNicDevStr(virDomainDefPtr def, return NULL; } } - } - if (usingVirtio) { qemuBuildIoEventFdStr(&buf, net->driver.virtio.ioeventfd, qemuCaps); if (net->driver.virtio.event_idx && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_EVENT_IDX)) { @@ -3387,32 +3386,33 @@ qemuBuildNicDevStr(virDomainDefPtr def, virBufferAsprintf(&buf, ",guest_ufo=%s", virTristateSwitchTypeToString(net->driver.virtio.guest.ufo)); } - } - if (usingVirtio && vhostfdSize > 1) { - if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - /* ccw provides a one to one relation of fds to queues and - * does not support the vectors option - */ - virBufferAddLit(&buf, ",mq=on"); - } else { - /* As advised at https://www.linux-kvm.org/page/Multiqueue - * we should add vectors=2*N+2 where N is the vhostfdSize - */ - virBufferAsprintf(&buf, ",mq=on,vectors=%zu", 2 * vhostfdSize + 2); + + if (vhostfdSize > 1) { + if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + /* ccw provides a one to one relation of fds to queues and + * does not support the vectors option + */ + virBufferAddLit(&buf, ",mq=on"); + } else { + /* As advised at https://www.linux-kvm.org/page/Multiqueue + * we should add vectors=2*N+2 where N is the vhostfdSize + */ + virBufferAsprintf(&buf, ",mq=on,vectors=%zu", 2 * vhostfdSize + 2); + } } - } - if (usingVirtio && net->driver.virtio.rx_queue_size) - virBufferAsprintf(&buf, ",rx_queue_size=%u", net->driver.virtio.rx_queue_size); + if (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) - virBufferAsprintf(&buf, ",tx_queue_size=%u", net->driver.virtio.tx_queue_size); + if (net->driver.virtio.tx_queue_size) + virBufferAsprintf(&buf, ",tx_queue_size=%u", net->driver.virtio.tx_queue_size); - if (usingVirtio && net->mtu) - virBufferAsprintf(&buf, ",host_mtu=%u", net->mtu); + if (net->mtu) + virBufferAsprintf(&buf, ",host_mtu=%u", net->mtu); - if (usingVirtio && net->teaming.type == VIR_DOMAIN_NET_TEAMING_TYPE_PERSISTENT) - virBufferAddLit(&buf, ",failover=on"); + if (net->teaming.type == VIR_DOMAIN_NET_TEAMING_TYPE_PERSISTENT) + virBufferAddLit(&buf, ",failover=on"); + } virBufferAsprintf(&buf, ",netdev=host%s", net->info.alias); virBufferAsprintf(&buf, ",id=%s", net->info.alias); -- 2.26.2
On 10/14/20 10:42 PM, Daniel Henrique Barboza wrote:
We have a lot of "if (usingVirtio)" checks being done while constructing the NIC command line. Let's put all of them in a single "if".
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 50 ++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 25 deletions(-)
This is exactly what I wanted to suggest when looking at previous patches. Kudos! Michal
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 6 ------ src/qemu/qemu_validate.c | 9 +++++++++ tests/qemuhotplugtest.c | 1 + 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a4f1cbce11..f9387a65e0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3823,12 +3823,6 @@ qemuBuildUSBInputDevStr(const virDomainDef *def, virBufferAsprintf(&buf, "usb-tablet,id=%s", dev->info.alias); break; case VIR_DOMAIN_INPUT_TYPE_KBD: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_KBD)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("usb keyboard is not supported by this " - "QEMU binary")); - return NULL; - } virBufferAsprintf(&buf, "usb-kbd,id=%s", dev->info.alias); break; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index ebc70aaf00..5de468af60 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3993,6 +3993,15 @@ qemuValidateDomainDeviceDefInput(const virDomainInputDef *input, return -1; } + if (input->bus == VIR_DOMAIN_INPUT_BUS_USB && + input->type == VIR_DOMAIN_INPUT_TYPE_KBD && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_KBD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("usb keyboard is not supported by this " + "QEMU binary")); + return -1; + } + if (input->bus != VIR_DOMAIN_INPUT_BUS_VIRTIO) return 0; diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 2d12cacf28..0948c8792d 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -89,6 +89,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SCSI_BLOCK); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_USB_KBD); if (qemuTestCapsCacheInsert(driver.qemuCapsCache, priv->qemuCaps) < 0) return -1; -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 6 ------ src/qemu/qemu_validate.c | 8 ++++++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f9387a65e0..29bfa665c4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4461,12 +4461,6 @@ qemuBuildSCSIVHostHostdevDevStr(const virDomainDef *def, g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; virDomainHostdevSubsysSCSIVHostPtr hostsrc = &dev->source.subsys.u.scsi_host; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_SCSI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support vhost-scsi devices")); - return NULL; - } - if (qemuBuildVirtioDevStr(&buf, "vhost-scsi", qemuCaps, VIR_DOMAIN_DEVICE_HOSTDEV, dev) < 0) { return NULL; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 5de468af60..1895ad857b 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1895,6 +1895,14 @@ qemuValidateDomainDeviceDefHostdev(const virDomainHostdevDef *hostdev, "supported by vhost SCSI devices")); return -1; } + + if (hostdev->source.subsys.u.scsi_host.protocol == + VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_HOST_PROTOCOL_TYPE_VHOST && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_SCSI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support vhost-scsi devices")); + return -1; + } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: return qemuValidateDomainMdevDef(hostdev, def, qemuCaps); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 28 ++-------------------------- src/qemu/qemu_command.h | 1 - src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_validate.c | 38 ++++++++++++++++++++++++++++++++++++-- tests/qemuxml2xmltest.c | 9 ++++++--- 5 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 29bfa665c4..f350800ff0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5342,7 +5342,6 @@ qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager, int qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, - virQEMUCapsPtr qemuCaps, virJSONValuePtr *props) { g_autofree char *objAlias = NULL; @@ -5352,13 +5351,6 @@ qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, switch ((virDomainRNGBackend) rng->backend) { case VIR_DOMAIN_RNG_BACKEND_RANDOM: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_RNG_RANDOM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this qemu doesn't support the rng-random " - "backend")); - return -1; - } - if (qemuMonitorCreateObjectProps(props, "rng-random", objAlias, "s:filename", rng->source.file, NULL) < 0) @@ -5367,13 +5359,6 @@ qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, break; case VIR_DOMAIN_RNG_BACKEND_EGD: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_RNG_EGD)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this qemu doesn't support the rng-egd " - "backend")); - return -1; - } - if (!(charBackendAlias = qemuAliasChardevFromDevAlias(rng->info.alias))) return -1; @@ -5385,13 +5370,6 @@ qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, break; case VIR_DOMAIN_RNG_BACKEND_BUILTIN: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_RNG_BUILTIN)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this qemu doesn't support the rng-builtin " - "backend")); - return -1; - } - if (qemuMonitorCreateObjectProps(props, "rng-builtin", objAlias, NULL) < 0) return -1; @@ -5399,9 +5377,7 @@ qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, break; case VIR_DOMAIN_RNG_BACKEND_LAST: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("unknown rng-random backend")); - return -1; + break; } return 0; @@ -5478,7 +5454,7 @@ qemuBuildRNGCommandLine(virLogManagerPtr logManager, if (chardev) virCommandAddArgList(cmd, "-chardev", chardev, NULL); - if (qemuBuildRNGBackendProps(rng, qemuCaps, &props) < 0) + if (qemuBuildRNGBackendProps(rng, &props) < 0) return -1; rc = virQEMUBuildObjectCommandlineFromJSON(&buf, props); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 8a30f2852c..12014b1451 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -167,7 +167,6 @@ char *qemuBuildRNGDevStr(const virDomainDef *def, virDomainRNGDefPtr dev, virQEMUCapsPtr qemuCaps); int qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, - virQEMUCapsPtr qemuCaps, virJSONValuePtr *props); /* Current, best practice */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 79fc8baa5c..fdb3801af0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2264,7 +2264,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, if (!(devstr = qemuBuildRNGDevStr(vm->def, rng, priv->qemuCaps))) goto cleanup; - if (qemuBuildRNGBackendProps(rng, priv->qemuCaps, &props) < 0) + if (qemuBuildRNGBackendProps(rng, &props) < 0) goto cleanup; if (!(charAlias = qemuAliasChardevFromDevAlias(rng->info.alias))) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 1895ad857b..c97b4770b8 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1646,9 +1646,43 @@ static int qemuValidateDomainRNGDef(const virDomainRNGDef *def, virQEMUCapsPtr qemuCaps) { - if (def->backend == VIR_DOMAIN_RNG_BACKEND_EGD && - qemuValidateDomainChrSourceDef(def->source.chardev, qemuCaps) < 0) + switch ((virDomainRNGBackend) def->backend) { + case VIR_DOMAIN_RNG_BACKEND_RANDOM: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_RNG_RANDOM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this qemu doesn't support the rng-random " + "backend")); + return -1; + } + break; + + case VIR_DOMAIN_RNG_BACKEND_EGD: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_RNG_EGD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this qemu doesn't support the rng-egd " + "backend")); + return -1; + } + + if (qemuValidateDomainChrSourceDef(def->source.chardev, qemuCaps) < 0) + return -1; + + break; + + case VIR_DOMAIN_RNG_BACKEND_BUILTIN: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_RNG_BUILTIN)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this qemu doesn't support the rng-builtin " + "backend")); + return -1; + } + break; + + case VIR_DOMAIN_RNG_BACKEND_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unknown rng-random backend")); return -1; + } if (qemuValidateDomainVirtioOptions(def->virtio, qemuCaps) < 0) return -1; diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index a9037101de..2213dc8615 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -682,9 +682,11 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("disk-arm-virtio-sd", "aarch64"); DO_TEST("virtio-rng-random", - QEMU_CAPS_DEVICE_VIRTIO_RNG); + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM); DO_TEST("virtio-rng-egd", - QEMU_CAPS_DEVICE_VIRTIO_RNG); + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_EGD); DO_TEST_CAPS_LATEST("virtio-rng-builtin"); DO_TEST("pseries-nvram", @@ -1124,7 +1126,8 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-network-http"); DO_TEST("chardev-label", - QEMU_CAPS_DEVICE_VIRTIO_RNG); + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_EGD); DO_TEST("cpu-numa1", NONE); DO_TEST("cpu-numa2", NONE); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 9 +-------- src/qemu/qemu_validate.c | 7 +++++++ tests/qemuxml2xmltest.c | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f350800ff0..87d76827e8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5758,7 +5758,6 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd, static int qemuBuildSysinfoCommandLine(virCommandPtr cmd, - virQEMUCapsPtr qemuCaps, const virDomainDef *def) { size_t i; @@ -5771,12 +5770,6 @@ qemuBuildSysinfoCommandLine(virCommandPtr cmd, if (def->sysinfo[i]->type != VIR_SYSINFO_FWCFG) continue; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_FW_CFG)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("fw_cfg is not supported with this QEMU")); - return -1; - } - for (j = 0; j < def->sysinfo[i]->nfw_cfgs; j++) { const virSysinfoFWCfgDef *f = &def->sysinfo[i]->fw_cfgs[j]; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; @@ -9928,7 +9921,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildSmbiosCommandLine(cmd, driver, def) < 0) return NULL; - if (qemuBuildSysinfoCommandLine(cmd, qemuCaps, def) < 0) + if (qemuBuildSysinfoCommandLine(cmd, def) < 0) return NULL; if (qemuBuildVMGenIDCommandLine(cmd, def) < 0) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index c97b4770b8..6200d40f28 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -782,6 +782,13 @@ qemuValidateDomainDefSysinfo(const virSysinfoDef *def, { size_t i; + if (def->type == VIR_SYSINFO_FWCFG && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_FW_CFG)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("fw_cfg is not supported with this QEMU")); + return -1; + } + for (i = 0; i < def->nfw_cfgs; i++) { const virSysinfoFWCfgDef *f = &def->fw_cfgs[i]; diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2213dc8615..6c551f18ab 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1153,7 +1153,7 @@ mymain(void) QEMU_CAPS_DEVICE_IVSHMEM_PLAIN, QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL); DO_TEST("smbios", NONE); DO_TEST("smbios-multiple-type2", NONE); - DO_TEST("smbios-type-fwcfg", NONE); + DO_TEST("smbios-type-fwcfg", QEMU_CAPS_FW_CFG); DO_TEST_CAPS_LATEST("os-firmware-bios"); DO_TEST_CAPS_LATEST("os-firmware-efi"); -- 2.26.2
On 10/14/20 10:42 PM, Daniel Henrique Barboza wrote:
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 9 +-------- src/qemu/qemu_validate.c | 7 +++++++ tests/qemuxml2xmltest.c | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f350800ff0..87d76827e8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5758,7 +5758,6 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd,
static int qemuBuildSysinfoCommandLine(virCommandPtr cmd, - virQEMUCapsPtr qemuCaps, const virDomainDef *def) { size_t i; @@ -5771,12 +5770,6 @@ qemuBuildSysinfoCommandLine(virCommandPtr cmd, if (def->sysinfo[i]->type != VIR_SYSINFO_FWCFG) continue;
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_FW_CFG)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("fw_cfg is not supported with this QEMU")); - return -1; - } - for (j = 0; j < def->sysinfo[i]->nfw_cfgs; j++) { const virSysinfoFWCfgDef *f = &def->sysinfo[i]->fw_cfgs[j]; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; @@ -9928,7 +9921,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildSmbiosCommandLine(cmd, driver, def) < 0) return NULL;
- if (qemuBuildSysinfoCommandLine(cmd, qemuCaps, def) < 0) + if (qemuBuildSysinfoCommandLine(cmd, def) < 0) return NULL;
if (qemuBuildVMGenIDCommandLine(cmd, def) < 0) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index c97b4770b8..6200d40f28 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -782,6 +782,13 @@ qemuValidateDomainDefSysinfo(const virSysinfoDef *def, { size_t i;
+ if (def->type == VIR_SYSINFO_FWCFG && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_FW_CFG)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("fw_cfg is not supported with this QEMU")); + return -1; + } +
Don't forget to remove G_GNUC_UNUSED attribute from qemuCaps, because now it's clearly used in the function. Michal
All but VIR_CPU_MODE_HOST_MODEL were moved. 'host_model' mode has nuances that forbid the verification to be moved to parse time. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 20 ++++---------- src/qemu/qemu_validate.c | 56 ++++++++++++++++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 6 ++--- 3 files changed, 64 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 87d76827e8..34d57606f0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6243,13 +6243,6 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, if (def->os.arch == VIR_ARCH_ARMV7L && driver->hostarch == VIR_ARCH_AARCH64) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_AARCH64_OFF)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("QEMU binary does not support CPU " - "host-passthrough for armv7l on " - "aarch64 host")); - return -1; - } virBufferAddLit(buf, ",aarch64=off"); } @@ -6257,19 +6250,16 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_MIGRATABLE)) { virBufferAsprintf(buf, ",migratable=%s", virTristateSwitchTypeToString(cpu->migratable)); - } else if (ARCH_IS_X86(def->os.arch) && - cpu->migratable == VIR_TRISTATE_SWITCH_OFF) { - /* This is the default on x86 */ - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Migratable attribute for host-passthrough " - "CPU is not supported by QEMU binary")); - return -1; } } break; case VIR_CPU_MODE_HOST_MODEL: + /* HOST_MODEL is a valid CPU mode for domain XMLs of all archs, meaning + * that we can't move this validation to parse time. By the time we reach + * this point, all non-PPC64 archs must have translated the CPU model to + * something else and set the CPU mode to MODE_CUSTOM. + */ if (ARCH_IS_PPC64(def->os.arch)) { virBufferAddLit(buf, "host"); if (cpu->model && diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 6200d40f28..7e6313b5c8 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -352,6 +352,59 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, } +static int +qemuValidateDomainDefCpu(virQEMUDriverPtr driver, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + virCPUDefPtr cpu = def->cpu; + + if (!cpu) + return 0; + + if (!cpu->model && cpu->mode == VIR_CPU_MODE_CUSTOM) + return 0; + + switch ((virCPUMode) cpu->mode) { + case VIR_CPU_MODE_HOST_PASSTHROUGH: + if (def->os.arch == VIR_ARCH_ARMV7L && + driver->hostarch == VIR_ARCH_AARCH64) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_AARCH64_OFF)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("QEMU binary does not support CPU " + "host-passthrough for armv7l on " + "aarch64 host")); + return -1; + } + } + + if (cpu->migratable && + cpu->migratable != VIR_TRISTATE_SWITCH_OFF && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_MIGRATABLE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Migratable attribute for host-passthrough " + "CPU is not supported by QEMU binary")); + return -1; + } + break; + + case VIR_CPU_MODE_HOST_MODEL: + /* qemu_command.c will error out if cpu->mode is HOST_MODEL for + * every arch but PPC64. However, we can't move this validation + * here because non-PPC64 archs will translate HOST_MODEL to + * something else during domain start, changing cpu->mode to + * CUSTOM. + */ + break; + case VIR_CPU_MODE_CUSTOM: + case VIR_CPU_MODE_LAST: + break; + } + + return 0; +} + + static int qemuValidateDomainDefClockTimers(const virDomainDef *def, virQEMUCapsPtr qemuCaps) @@ -935,6 +988,9 @@ qemuValidateDomainDef(const virDomainDef *def, } } + if (qemuValidateDomainDefCpu(driver, def, qemuCaps) < 0) + return -1; + if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0) return -1; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index cf1d3c7ded..5076d774f8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2891,9 +2891,9 @@ mymain(void) QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_DEVICE_PL011, QEMU_CAPS_KVM, QEMU_CAPS_CPU_AARCH64_OFF); - DO_TEST_FAILURE("aarch64-kvm-32-on-64", - QEMU_CAPS_DEVICE_VIRTIO_MMIO, - QEMU_CAPS_KVM); + DO_TEST_PARSE_ERROR("aarch64-kvm-32-on-64", + QEMU_CAPS_DEVICE_VIRTIO_MMIO, + QEMU_CAPS_KVM); DO_TEST("aarch64-pci-serial", QEMU_CAPS_DEVICE_PCI_SERIAL, QEMU_CAPS_CHARDEV_LOGFILE, -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 5 ----- src/qemu/qemu_validate.c | 15 +++++++++++++++ tests/qemuxml2xmltest.c | 2 +- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 34d57606f0..225d1d7491 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7291,11 +7291,6 @@ qemuBuildNumaCommandLine(virQEMUDriverConfigPtr cfg, if (rc == 0) needBackend = true; } - } else if (needBackend) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("NUMA without specified memory backing is not " - "supported with this QEMU binary")); - goto cleanup; } if (!needBackend && diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 7e6313b5c8..9bb7cddb40 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -701,6 +701,7 @@ qemuValidateDomainDefNuma(const virDomainDef *def, bool hasMemoryCap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE) || virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_MEMFD); + bool needBacking = false; if (virDomainNumatuneHasPerNodeBinding(def->numa) && !hasMemoryCap) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -752,6 +753,20 @@ qemuValidateDomainDefNuma(const virDomainDef *def, return -1; } + if (virDomainNumaHasHMAT(def->numa) || + !virQEMUCapsGetMachineNumaMemSupported(qemuCaps, + def->virtType, + def->os.machine)) { + needBacking = true; + } + + if (needBacking && !hasMemoryCap) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("NUMA without specified memory backing is not " + "supported with this QEMU binary")); + return -1; + } + return 0; } diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 6c551f18ab..93287d0a55 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1141,7 +1141,7 @@ mymain(void) DO_TEST("numatune-memnode-no-memory", QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("numatune-distances", QEMU_CAPS_NUMA, QEMU_CAPS_NUMA_DIST); DO_TEST("numatune-no-vcpu", QEMU_CAPS_NUMA); - DO_TEST("numatune-hmat", QEMU_CAPS_NUMA_HMAT); + DO_TEST("numatune-hmat", QEMU_CAPS_NUMA_HMAT, QEMU_CAPS_OBJECT_MEMORY_RAM); DO_TEST("bios-nvram", NONE); DO_TEST("bios-nvram-os-interleave", NONE); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 18 ----------------- src/qemu/qemu_validate.c | 43 +++++++++++++++++++++++++++++++++++++++- tests/qemuxml2argvtest.c | 2 +- tests/qemuxml2xmltest.c | 2 +- 4 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 225d1d7491..7008f14028 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8410,13 +8410,6 @@ qemuBuildShmemDevLegacyStr(virDomainDefPtr def, { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("ivshmem device is not supported " - "with this QEMU binary")); - return NULL; - } - virBufferAddLit(&buf, "ivshmem"); virBufferAsprintf(&buf, ",id=%s", shmem->info.alias); @@ -8450,17 +8443,6 @@ qemuBuildShmemDevStr(virDomainDefPtr def, { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - if ((shmem->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM_PLAIN)) || - (shmem->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("shmem model '%s' is not supported " - "by this QEMU binary"), - virDomainShmemModelTypeToString(shmem->model)); - return NULL; - } - virBufferAdd(&buf, virDomainShmemModelTypeToString(shmem->model), -1); virBufferAsprintf(&buf, ",id=%s", shmem->info.alias); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 9bb7cddb40..65d6e18fe5 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4401,6 +4401,44 @@ qemuValidateDomainDeviceDefMemory(virDomainMemoryDefPtr mem, } +static int +qemuValidateDomainDeviceDefShmem(virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps) +{ + switch (shmem->model) { + case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ivshmem device is not supported " + "with this QEMU binary")); + return -1; + } + break; + + case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM_PLAIN)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("shmem model '%s' is not supported " + "by this QEMU binary"), + virDomainShmemModelTypeToString(shmem->model)); + return -1; + } + break; + + case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("shmem model '%s' is not supported " + "by this QEMU binary"), + virDomainShmemModelTypeToString(shmem->model)); + return -1; + } + } + + return 0; +} + + int qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev, const virDomainDef *def, @@ -4515,8 +4553,11 @@ qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev, ret = qemuValidateDomainDeviceDefMemory(dev->data.memory, qemuCaps); break; - case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_SHMEM: + ret = qemuValidateDomainDeviceDefShmem(dev->data.shmem, qemuCaps); + break; + + case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_PANIC: case VIR_DOMAIN_DEVICE_AUDIO: case VIR_DOMAIN_DEVICE_NONE: diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 5076d774f8..fdcfadf4c8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2945,7 +2945,7 @@ mymain(void) DO_TEST("shmem-plain-doorbell", QEMU_CAPS_DEVICE_IVSHMEM, QEMU_CAPS_DEVICE_IVSHMEM_PLAIN, QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL); - DO_TEST_FAILURE("shmem", NONE); + DO_TEST_PARSE_ERROR("shmem", NONE); DO_TEST_FAILURE("shmem-invalid-size", QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST_FAILURE("shmem-invalid-address", diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 93287d0a55..a897dfe17a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1148,7 +1148,7 @@ mymain(void) DO_TEST("tap-vhost", NONE); DO_TEST("tap-vhost-incorrect", NONE); - DO_TEST("shmem", NONE); + DO_TEST("shmem", QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST("shmem-plain-doorbell", QEMU_CAPS_DEVICE_IVSHMEM_PLAIN, QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL); DO_TEST("smbios", NONE); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 21 --------------------- src/qemu/qemu_validate.c | 29 ++++++++++++++++++++++++++--- tests/qemuxml2xmltest.c | 10 +++++++--- 3 files changed, 33 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7008f14028..16bb8dbdbe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8925,31 +8925,10 @@ qemuBuildRedirdevDevStr(const virDomainDef *def, g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; virDomainRedirFilterDefPtr redirfilter = def->redirfilter; - if (dev->bus != VIR_DOMAIN_REDIRDEV_BUS_USB) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Redirection bus %s is not supported by QEMU"), - virDomainRedirdevBusTypeToString(dev->bus)); - return NULL; - } - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_REDIR)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("USB redirection is not supported " - "by this version of QEMU")); - return NULL; - } - virBufferAsprintf(&buf, "usb-redir,chardev=char%s,id=%s", dev->info.alias, dev->info.alias); if (redirfilter && redirfilter->nusbdevs) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_REDIR_FILTER)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("USB redirection filter is not " - "supported by this version of QEMU")); - return NULL; - } - virBufferAddLit(&buf, ",filter="); for (i = 0; i < redirfilter->nusbdevs; i++) { diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 65d6e18fe5..7969571b4a 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1770,11 +1770,34 @@ qemuValidateDomainRNGDef(const virDomainRNGDef *def, static int -qemuValidateDomainRedirdevDef(const virDomainRedirdevDef *def, +qemuValidateDomainRedirdevDef(const virDomainRedirdevDef *dev, + const virDomainDef *def, virQEMUCapsPtr qemuCaps) { - if (qemuValidateDomainChrSourceDef(def->source, qemuCaps) < 0) + if (qemuValidateDomainChrSourceDef(dev->source, qemuCaps) < 0) + return -1; + + if (dev->bus != VIR_DOMAIN_REDIRDEV_BUS_USB) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Redirection bus %s is not supported by QEMU"), + virDomainRedirdevBusTypeToString(dev->bus)); return -1; + } + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_REDIR)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("USB redirection is not supported " + "by this version of QEMU")); + return -1; + } + + if (def->redirfilter && def->redirfilter->nusbdevs && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_REDIR_FILTER)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("USB redirection filter is not " + "supported by this version of QEMU")); + return -1; + } return 0; } @@ -4483,7 +4506,7 @@ qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev, break; case VIR_DOMAIN_DEVICE_REDIRDEV: - ret = qemuValidateDomainRedirdevDef(dev->data.redirdev, qemuCaps); + ret = qemuValidateDomainRedirdevDef(dev->data.redirdev, def, qemuCaps); break; case VIR_DOMAIN_DEVICE_WATCHDOG: diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index a897dfe17a..7160653471 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -631,9 +631,13 @@ mymain(void) QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, QEMU_CAPS_PIIX3_USB_UHCI); DO_TEST("usb-port-missing", QEMU_CAPS_USB_HUB); - DO_TEST("usb-redir", NONE); - DO_TEST("usb-redir-filter", NONE); - DO_TEST("usb-redir-filter-version", NONE); + DO_TEST("usb-redir", QEMU_CAPS_USB_REDIR); + DO_TEST("usb-redir-filter", + QEMU_CAPS_USB_REDIR, + QEMU_CAPS_USB_REDIR_FILTER); + DO_TEST("usb-redir-filter-version", + QEMU_CAPS_USB_REDIR, + QEMU_CAPS_USB_REDIR_FILTER); DO_TEST_CAPS_LATEST("blkdeviotune"); DO_TEST_CAPS_LATEST("blkdeviotune-max"); DO_TEST_CAPS_LATEST("blkdeviotune-group-num"); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 78 +-------------- src/qemu/qemu_validate.c | 96 +++++++++++++++++++ .../pseries-panic-address.err | 2 +- tests/qemuxml2argvtest.c | 10 +- tests/qemuxml2xmltest.c | 8 +- 5 files changed, 111 insertions(+), 83 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 16bb8dbdbe..ed14e1d01f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9325,78 +9325,13 @@ qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd, static int qemuBuildPanicCommandLine(virCommandPtr cmd, - const virDomainDef *def, - virQEMUCapsPtr qemuCaps) + const virDomainDef *def) { size_t i; for (i = 0; i < def->npanics; i++) { switch ((virDomainPanicModel) def->panics[i]->model) { - case VIR_DOMAIN_PANIC_MODEL_S390: - /* For s390 guests, the hardware provides the same - * functionality as the pvpanic device. The address - * cannot be configured by the user */ - if (!ARCH_IS_S390(def->os.arch)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only S390 guests support " - "panic device of model 's390'")); - return -1; - } - if (def->panics[i]->info.type != - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("setting the panic device address is not " - "supported for model 's390'")); - return -1; - } - break; - - case VIR_DOMAIN_PANIC_MODEL_HYPERV: - /* Panic with model 'hyperv' is not a device, it should - * be configured in cpu commandline. The address - * cannot be configured by the user */ - if (!ARCH_IS_X86(def->os.arch)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only i686 and x86_64 guests support " - "panic device of model 'hyperv'")); - return -1; - } - if (def->panics[i]->info.type != - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("setting the panic device address is not " - "supported for model 'hyperv'")); - return -1; - } - break; - - case VIR_DOMAIN_PANIC_MODEL_PSERIES: - /* For pSeries guests, the firmware provides the same - * functionality as the pvpanic device. The address - * cannot be configured by the user */ - if (!qemuDomainIsPSeries(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only pSeries guests support panic device " - "of model 'pseries'")); - return -1; - } - if (def->panics[i]->info.type != - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("setting the panic device address is not " - "supported for model 'pseries'")); - return -1; - } - break; - case VIR_DOMAIN_PANIC_MODEL_ISA: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the QEMU binary does not support the " - "ISA panic device")); - return -1; - } - switch (def->panics[i]->info.type) { case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA: virCommandAddArg(cmd, "-device"); @@ -9407,14 +9342,11 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE: virCommandAddArgList(cmd, "-device", "pvpanic", NULL); break; - - default: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("panic is supported only " - "with ISA address type")); - return -1; } + case VIR_DOMAIN_PANIC_MODEL_S390: + case VIR_DOMAIN_PANIC_MODEL_HYPERV: + case VIR_DOMAIN_PANIC_MODEL_PSERIES: /* default model value was changed before in post parse */ case VIR_DOMAIN_PANIC_MODEL_DEFAULT: case VIR_DOMAIN_PANIC_MODEL_LAST: @@ -10017,7 +9949,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildSeccompSandboxCommandLine(cmd, cfg, qemuCaps) < 0) return NULL; - if (qemuBuildPanicCommandLine(cmd, def, qemuCaps) < 0) + if (qemuBuildPanicCommandLine(cmd, def) < 0) return NULL; for (i = 0; i < def->nshmems; i++) { diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 7969571b4a..a5e3849ae5 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -878,6 +878,99 @@ qemuValidateDomainDefSysinfo(const virSysinfoDef *def, } +static int +qemuValidateDomainDefPanic(const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->npanics; i++) { + switch ((virDomainPanicModel) def->panics[i]->model) { + case VIR_DOMAIN_PANIC_MODEL_S390: + /* For s390 guests, the hardware provides the same + * functionality as the pvpanic device. The address + * cannot be configured by the user */ + if (!ARCH_IS_S390(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only S390 guests support " + "panic device of model 's390'")); + return -1; + } + if (def->panics[i]->info.type != + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting the panic device address is not " + "supported for model 's390'")); + return -1; + } + break; + + case VIR_DOMAIN_PANIC_MODEL_HYPERV: + /* Panic with model 'hyperv' is not a device, it should + * be configured in cpu commandline. The address + * cannot be configured by the user */ + if (!ARCH_IS_X86(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only i686 and x86_64 guests support " + "panic device of model 'hyperv'")); + return -1; + } + if (def->panics[i]->info.type != + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting the panic device address is not " + "supported for model 'hyperv'")); + return -1; + } + break; + + case VIR_DOMAIN_PANIC_MODEL_PSERIES: + /* For pSeries guests, the firmware provides the same + * functionality as the pvpanic device. The address + * cannot be configured by the user */ + if (!qemuDomainIsPSeries(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only pSeries guests support panic device " + "of model 'pseries'")); + return -1; + } + if (def->panics[i]->info.type != + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting the panic device address is not " + "supported for model 'pseries'")); + return -1; + } + break; + + case VIR_DOMAIN_PANIC_MODEL_ISA: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the QEMU binary does not support the " + "ISA panic device")); + return -1; + } + + if (def->panics[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + def->panics[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("panic is supported only " + "with ISA address type")); + return -1; + } + break; + + /* default model value was changed before in post parse */ + case VIR_DOMAIN_PANIC_MODEL_DEFAULT: + case VIR_DOMAIN_PANIC_MODEL_LAST: + break; + } + } + + return 0; +} + + int qemuValidateDomainDef(const virDomainDef *def, void *opaque) @@ -1112,6 +1205,9 @@ qemuValidateDomainDef(const virDomainDef *def, return -1; } + if (qemuValidateDomainDefPanic(def, qemuCaps) < 0) + return -1; + return 0; } diff --git a/tests/qemuxml2argvdata/pseries-panic-address.err b/tests/qemuxml2argvdata/pseries-panic-address.err index c7a512c51e..63afb4ff00 100644 --- a/tests/qemuxml2argvdata/pseries-panic-address.err +++ b/tests/qemuxml2argvdata/pseries-panic-address.err @@ -1 +1 @@ -unsupported configuration: 'spapr-vty' is not supported in this QEMU binary +unsupported configuration: setting the panic device address is not supported for model 'pseries' diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index fdcfadf4c8..73e3a91282 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2064,8 +2064,8 @@ mymain(void) DO_TEST("pseries-panic-no-address", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, QEMU_CAPS_DEVICE_SPAPR_VTY); - DO_TEST_FAILURE("pseries-panic-address", - QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); + DO_TEST_PARSE_ERROR("pseries-panic-address", + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); DO_TEST("pseries-phb-simple", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); @@ -2377,9 +2377,9 @@ mymain(void) DO_TEST("s390-panic-no-address", QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390); - DO_TEST_FAILURE("s390-panic-address", - QEMU_CAPS_CCW, - QEMU_CAPS_VIRTIO_S390); + DO_TEST_PARSE_ERROR("s390-panic-address", + QEMU_CAPS_CCW, + QEMU_CAPS_VIRTIO_S390); DO_TEST("s390-panic-missing", QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7160653471..1cd57c6bc1 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1116,12 +1116,12 @@ mymain(void) QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_Q35_PCI_HOLE64_SIZE); - DO_TEST("panic", NONE); - DO_TEST("panic-isa", NONE); + DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC); + DO_TEST("panic-isa", QEMU_CAPS_DEVICE_PANIC); DO_TEST("panic-pseries", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); - DO_TEST("panic-double", NONE); - DO_TEST("panic-no-address", NONE); + DO_TEST("panic-double", QEMU_CAPS_DEVICE_PANIC); + DO_TEST("panic-no-address", QEMU_CAPS_DEVICE_PANIC); DO_TEST("disk-backing-chains", NONE); DO_TEST("disk-backing-chains-index", NONE); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 11 ++--------- src/qemu/qemu_command.h | 3 +-- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_validate.c | 19 ++++++++++++++----- tests/qemuxml2xmltest.c | 3 ++- 5 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ed14e1d01f..faa64c28b6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3199,8 +3199,7 @@ qemuBuildMemoryDimmBackendStr(virBufferPtr buf, char * -qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, - qemuDomainObjPrivatePtr priv) +qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem) { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; const char *device; @@ -3236,12 +3235,6 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, } if (mem->readonly) { - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_NVDIMM_UNARMED)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("nvdimm readonly property is not available " - "with this QEMU binary")); - return NULL; - } virBufferAddLit(&buf, "unarmed=on,"); } @@ -7415,7 +7408,7 @@ qemuBuildMemoryDeviceCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, "-object"); virCommandAddArgBuffer(cmd, &buf); - if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], priv))) + if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i]))) return -1; virCommandAddArgList(cmd, "-device", dimmStr, NULL); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 12014b1451..d452905fdf 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -154,8 +154,7 @@ int qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, const virDomainMemoryDef *mem, bool force); -char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, - qemuDomainObjPrivatePtr priv); +char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem); /* Current, best practice */ char *qemuBuildPCIHostdevDevStr(const virDomainDef *def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index fdb3801af0..f76e773f64 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2384,7 +2384,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, objalias = g_strdup_printf("mem%s", mem->info.alias); - if (!(devstr = qemuBuildMemoryDeviceStr(mem, priv))) + if (!(devstr = qemuBuildMemoryDeviceStr(mem))) goto cleanup; if (qemuBuildMemoryBackendProps(&props, objalias, cfg, diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index a5e3849ae5..0f4cb3c983 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4509,11 +4509,20 @@ static int qemuValidateDomainDeviceDefMemory(virDomainMemoryDefPtr mem, virQEMUCapsPtr qemuCaps) { - if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("nvdimm isn't supported by this QEMU binary")); - return -1; + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("nvdimm isn't supported by this QEMU binary")); + return -1; + } + + if (mem->readonly && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM_UNARMED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("nvdimm readonly property is not available " + "with this QEMU binary")); + return -1; + } } return 0; diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 1cd57c6bc1..c11f09e04a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1284,7 +1284,8 @@ mymain(void) DO_TEST("memory-hotplug-nvdimm-label", QEMU_CAPS_DEVICE_NVDIMM); DO_TEST("memory-hotplug-nvdimm-align", QEMU_CAPS_DEVICE_NVDIMM); DO_TEST("memory-hotplug-nvdimm-pmem", QEMU_CAPS_DEVICE_NVDIMM); - DO_TEST("memory-hotplug-nvdimm-readonly", QEMU_CAPS_DEVICE_NVDIMM); + DO_TEST("memory-hotplug-nvdimm-readonly", QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_DEVICE_NVDIMM_UNARMED); DO_TEST("memory-hotplug-nvdimm-ppc64", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, QEMU_CAPS_DEVICE_NVDIMM); DO_TEST("net-udp", NONE); -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_validate.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 0f4cb3c983..b5bea4a0ee 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -753,13 +753,22 @@ qemuValidateDomainDefNuma(const virDomainDef *def, return -1; } - if (virDomainNumaHasHMAT(def->numa) || - !virQEMUCapsGetMachineNumaMemSupported(qemuCaps, + if (!virQEMUCapsGetMachineNumaMemSupported(qemuCaps, def->virtType, def->os.machine)) { needBacking = true; } + if (virDomainNumaHasHMAT(def->numa)) { + needBacking = true; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA_HMAT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("HMAT is not supported with this QEMU")); + return -1; + } + } + if (needBacking && !hasMemoryCap) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("NUMA without specified memory backing is not " @@ -1068,13 +1077,6 @@ qemuValidateDomainDef(const virDomainDef *def, } } - if (virDomainNumaHasHMAT(def->numa) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA_HMAT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("HMAT is not supported with this QEMU")); - return -1; - } - if (def->genidRequested && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMGENID)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 2.26.2
Rename the function to qemuValidateDomainVCpuTopology() to reflect what it is currently doing as well. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_validate.c | 174 +++++++++++++++++++-------------------- 1 file changed, 87 insertions(+), 87 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index b5bea4a0ee..5b5c2ab3dd 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -613,8 +613,38 @@ qemuValidateDomainDefBoot(const virDomainDef *def, } +/** + * qemuValidateDefGetVcpuHotplugGranularity: + * @def: domain definition + * + * With QEMU 2.7 and newer, vCPUs can only be hotplugged in groups that + * respect the guest's hotplug granularity; because of that, QEMU will + * not allow guests to start unless the initial number of vCPUs is a + * multiple of the hotplug granularity. + * + * Returns the vCPU hotplug granularity. + */ +static unsigned int +qemuValidateDefGetVcpuHotplugGranularity(const virDomainDef *def) +{ + /* If the guest CPU topology has not been configured, assume we + * can hotplug vCPUs one at a time */ + if (!def->cpu || def->cpu->sockets == 0) + return 1; + + /* For pSeries guests, hotplug can only be performed one core + * at a time, so the vCPU hotplug granularity is the number + * of threads per core */ + if (qemuDomainIsPSeries(def)) + return def->cpu->threads; + + /* In all other cases, we can hotplug vCPUs one at a time */ + return 1; +} + + static int -qemuValidateDomainCpuCount(const virDomainDef *def, virQEMUCapsPtr qemuCaps) +qemuValidateDomainVCpuTopology(const virDomainDef *def, virQEMUCapsPtr qemuCaps) { unsigned int maxCpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, def->virtType, def->os.machine); @@ -632,6 +662,61 @@ qemuValidateDomainCpuCount(const virDomainDef *def, virQEMUCapsPtr qemuCaps) return -1; } + /* QEMU 2.7 (detected via the availability of query-hotpluggable-cpus) + * enforces stricter rules than previous versions when it comes to guest + * CPU topology. Verify known constraints are respected */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS)) { + unsigned int topologycpus; + unsigned int granularity; + unsigned int numacpus; + + /* Starting from QEMU 2.5, max vCPU count and overall vCPU topology + * must agree. We only actually enforce this with QEMU 2.7+, due + * to the capability check above */ + if (virDomainDefGetVcpusTopology(def, &topologycpus) == 0) { + if (topologycpus != virDomainDefGetVcpusMax(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("CPU topology doesn't match maximum vcpu count")); + return -1; + } + + numacpus = virDomainNumaGetCPUCountTotal(def->numa); + if ((numacpus != 0) && (topologycpus != numacpus)) { + VIR_WARN("CPU topology doesn't match numa CPU count; " + "partial NUMA mapping is obsoleted and will " + "be removed in future"); + } + } + + /* vCPU hotplug granularity must be respected */ + granularity = qemuValidateDefGetVcpuHotplugGranularity(def); + if ((virDomainDefGetVcpus(def) % granularity) != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("vCPUs count must be a multiple of the vCPU " + "hotplug granularity (%u)"), + granularity); + return -1; + } + } + + if (ARCH_IS_X86(def->os.arch) && + virDomainDefGetVcpusMax(def) > QEMU_MAX_VCPUS_WITHOUT_EIM) { + if (!qemuDomainIsQ35(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("more than %d vCPUs are only supported on " + "q35-based machine types"), + QEMU_MAX_VCPUS_WITHOUT_EIM); + return -1; + } + if (!def->iommu || def->iommu->eim != VIR_TRISTATE_SWITCH_ON) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("more than %d vCPUs require extended interrupt " + "mode enabled on the iommu device"), + QEMU_MAX_VCPUS_WITHOUT_EIM); + return -1; + } + } + return 0; } @@ -823,36 +908,6 @@ qemuValidateDomainDefConsole(const virDomainDef *def, } -/** - * qemuValidateDefGetVcpuHotplugGranularity: - * @def: domain definition - * - * With QEMU 2.7 and newer, vCPUs can only be hotplugged in groups that - * respect the guest's hotplug granularity; because of that, QEMU will - * not allow guests to start unless the initial number of vCPUs is a - * multiple of the hotplug granularity. - * - * Returns the vCPU hotplug granularity. - */ -static unsigned int -qemuValidateDefGetVcpuHotplugGranularity(const virDomainDef *def) -{ - /* If the guest CPU topology has not been configured, assume we - * can hotplug vCPUs one at a time */ - if (!def->cpu || def->cpu->sockets == 0) - return 1; - - /* For pSeries guests, hotplug can only be performed one core - * at a time, so the vCPU hotplug granularity is the number - * of threads per core */ - if (qemuDomainIsPSeries(def)) - return def->cpu->threads; - - /* In all other cases, we can hotplug vCPUs one at a time */ - return 1; -} - - static int qemuValidateDomainDefSysinfo(const virSysinfoDef *def, virQEMUCapsPtr qemuCaps G_GNUC_UNUSED) @@ -1113,64 +1168,9 @@ qemuValidateDomainDef(const virDomainDef *def, if (qemuValidateDomainDefBoot(def, qemuCaps) < 0) return -1; - /* QEMU 2.7 (detected via the availability of query-hotpluggable-cpus) - * enforces stricter rules than previous versions when it comes to guest - * CPU topology. Verify known constraints are respected */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS)) { - unsigned int topologycpus; - unsigned int granularity; - unsigned int numacpus; - - /* Starting from QEMU 2.5, max vCPU count and overall vCPU topology - * must agree. We only actually enforce this with QEMU 2.7+, due - * to the capability check above */ - if (virDomainDefGetVcpusTopology(def, &topologycpus) == 0) { - if (topologycpus != virDomainDefGetVcpusMax(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("CPU topology doesn't match maximum vcpu count")); - return -1; - } - - numacpus = virDomainNumaGetCPUCountTotal(def->numa); - if ((numacpus != 0) && (topologycpus != numacpus)) { - VIR_WARN("CPU topology doesn't match numa CPU count; " - "partial NUMA mapping is obsoleted and will " - "be removed in future"); - } - } - - /* vCPU hotplug granularity must be respected */ - granularity = qemuValidateDefGetVcpuHotplugGranularity(def); - if ((virDomainDefGetVcpus(def) % granularity) != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("vCPUs count must be a multiple of the vCPU " - "hotplug granularity (%u)"), - granularity); - return -1; - } - } - - if (qemuValidateDomainCpuCount(def, qemuCaps) < 0) + if (qemuValidateDomainVCpuTopology(def, qemuCaps) < 0) return -1; - if (ARCH_IS_X86(def->os.arch) && - virDomainDefGetVcpusMax(def) > QEMU_MAX_VCPUS_WITHOUT_EIM) { - if (!qemuDomainIsQ35(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("more than %d vCPUs are only supported on " - "q35-based machine types"), - QEMU_MAX_VCPUS_WITHOUT_EIM); - return -1; - } - if (!def->iommu || def->iommu->eim != VIR_TRISTATE_SWITCH_ON) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("more than %d vCPUs require extended interrupt " - "mode enabled on the iommu device"), - QEMU_MAX_VCPUS_WITHOUT_EIM); - return -1; - } - } - if (def->nresctrls && def->virtType != VIR_DOMAIN_VIRT_KVM) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 2.26.2
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_validate.c | 58 ++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 5b5c2ab3dd..eeec2270e6 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -591,6 +591,35 @@ static int qemuValidateDomainDefBoot(const virDomainDef *def, virQEMUCapsPtr qemuCaps) { + if (def->os.loader && + def->os.loader->secure == VIR_TRISTATE_BOOL_YES) { + /* These are the QEMU implementation limitations. But we + * have to live with them for now. */ + + if (!qemuDomainIsQ35(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Secure boot is supported with q35 machine types only")); + return -1; + } + + /* Now, technically it is possible to have secure boot on + * 32bits too, but that would require some -cpu xxx magic + * too. Not worth it unless we are explicitly asked. */ + if (def->os.arch != VIR_ARCH_X86_64) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Secure boot is supported for x86_64 architecture only")); + return -1; + } + + /* SMM will be enabled by qemuFirmwareFillDomain() if needed. */ + if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE && + def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Secure boot requires SMM feature enabled")); + return -1; + } + } + if (def->os.bios.rt_set) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_REBOOT_TIMEOUT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -1103,35 +1132,6 @@ qemuValidateDomainDef(const virDomainDef *def, return -1; } - if (def->os.loader && - def->os.loader->secure == VIR_TRISTATE_BOOL_YES) { - /* These are the QEMU implementation limitations. But we - * have to live with them for now. */ - - if (!qemuDomainIsQ35(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Secure boot is supported with q35 machine types only")); - return -1; - } - - /* Now, technically it is possible to have secure boot on - * 32bits too, but that would require some -cpu xxx magic - * too. Not worth it unless we are explicitly asked. */ - if (def->os.arch != VIR_ARCH_X86_64) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Secure boot is supported for x86_64 architecture only")); - return -1; - } - - /* SMM will be enabled by qemuFirmwareFillDomain() if needed. */ - if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE && - def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Secure boot requires SMM feature enabled")); - return -1; - } - } - if (def->genidRequested && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMGENID)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 2.26.2
On 10/14/20 10:42 PM, Daniel Henrique Barboza wrote:
Hi,
This is another step following up the work done in [1].
Recapping, the idea is to move any validation that can be done in parse/define time to qemu_validate.c instead of let it sit inside a command line build function in qemu_command.c. I'll again copy/paste the reasoning Cole Robinson gave for this work back then (see [2] for more info):
------- The benefits of moving to validate time is that XML is rejected by 'virsh define' rather than at 'virsh start' time. It also makes it easier to follow the cli building code, and makes it easier to verify qemu_command.c test suite code coverage for the important stuff like covering every CLI option. It's also a good intermediate step for sharing validation with domain capabilities building, like is done presently for rng models. -------
Not all validations were moved with these series, but I covered most of them.
One thing worth noticing is the existence of 'post start' validations, most of them related to PCI addressing, and other notable cases such as NUMA nodes and CPU models, that can't be moved to parse time. The reason is that the QEMU driver will change them during start time. I contemplated move them to the existing qemuBuildCommandLineValidate(), but that would remove them from the hotplug path that uses the qemuBuild*DevStr() functions. I don't have an easy answer on how to handle these 'post start' validations ATM, so for now they remain in qemu_command.c.
[1] https://www.redhat.com/archives/libvir-list/2019-December/msg00570.html [2] https://www.redhat.com/archives/libvir-list/2019-October/msg00568.html
src/qemu/qemu_command.c | 379 +-------- src/qemu/qemu_command.h | 4 +- src/qemu/qemu_hotplug.c | 4 +- src/qemu/qemu_validate.c | 740 ++++++++++++++---- tests/qemuhotplugtest.c | 1 + .../disk-sata-incompatible-address.err | 2 +- .../net-virtio-rxqueuesize-invalid-size.err | 2 +- .../pseries-panic-address.err | 2 +- tests/qemuxml2argvtest.c | 28 +- tests/qemuxml2xmltest.c | 62 +- 10 files changed, 706 insertions(+), 518 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On 10/15/20 8:46 AM, Michal Privoznik wrote:
On 10/14/20 10:42 PM, Daniel Henrique Barboza wrote:
Hi,
This is another step following up the work done in [1].
Recapping, the idea is to move any validation that can be done in parse/define time to qemu_validate.c instead of let it sit inside a command line build function in qemu_command.c. I'll again copy/paste the reasoning Cole Robinson gave for this work back then (see [2] for more info):
------- The benefits of moving to validate time is that XML is rejected by 'virsh define' rather than at 'virsh start' time. It also makes it easier to follow the cli building code, and makes it easier to verify qemu_command.c test suite code coverage for the important stuff like covering every CLI option. It's also a good intermediate step for sharing validation with domain capabilities building, like is done presently for rng models. -------
Not all validations were moved with these series, but I covered most of them.
One thing worth noticing is the existence of 'post start' validations, most of them related to PCI addressing, and other notable cases such as NUMA nodes and CPU models, that can't be moved to parse time. The reason is that the QEMU driver will change them during start time. I contemplated move them to the existing qemuBuildCommandLineValidate(), but that would remove them from the hotplug path that uses the qemuBuild*DevStr() functions. I don't have an easy answer on how to handle these 'post start' validations ATM, so for now they remain in qemu_command.c.
[1] https://www.redhat.com/archives/libvir-list/2019-December/msg00570.html [2] https://www.redhat.com/archives/libvir-list/2019-October/msg00568.html
src/qemu/qemu_command.c | 379 +-------- src/qemu/qemu_command.h | 4 +- src/qemu/qemu_hotplug.c | 4 +- src/qemu/qemu_validate.c | 740 ++++++++++++++---- tests/qemuhotplugtest.c | 1 + .../disk-sata-incompatible-address.err | 2 +- .../net-virtio-rxqueuesize-invalid-size.err | 2 +- .../pseries-panic-address.err | 2 +- tests/qemuxml2argvtest.c | 28 +- tests/qemuxml2xmltest.c | 62 +- 10 files changed, 706 insertions(+), 518 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thanks for the review! Applied all your suggestions in patches 03, 05, 08 and 15 and pushed. DHB
Michal
participants (2)
-
Daniel Henrique Barboza -
Michal Privoznik