On 05/03/2016 09:42 AM, Cole Robinson wrote:
On 05/02/2016 06:30 PM, John Ferlan wrote:
Rather than an if statement, use a switch (we're about to add more support)
Are you? I don't see anything touching this function in patch #7
Hmmm.. wrote this commit message before I realized this call is only "valid" for <disk> checks... the <iothread> for virtio-scsi is on the controller. I'll adjust the commit message. The code I think is still necessary since it wasn't checked previously.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bd564db..4d37410 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1468,12 +1468,29 @@ qemuCheckIOThreads(const virDomainDef *def, virDomainDiskDefPtr disk) { /* Right "type" of disk" */ - if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO || - (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && - disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IOThreads only available for virtio pci and " - "virtio ccw disk")); + switch ((virDomainDiskBus)disk->bus) { + case VIR_DOMAIN_DISK_BUS_VIRTIO: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads only available for virtio pci and " + "virtio ccw disk")); + return false; + } + break; + + case VIR_DOMAIN_DISK_BUS_IDE: + case VIR_DOMAIN_DISK_BUS_FDC: + case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SATA: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + ("IOThreads not available for bus %s dst %s"), + virDomainDiskBusTypeToString(disk->bus), disk->dst); return false; }
This error should be marked as translatable. I'd suggest just doing "IOThreads not available for disk bus %s", but if you want to list the 'dst' field call it 'target' in the string
hmmm... cut-n-paste error it seems missing the "_"... good catch - I'll adjust the message. John