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@xxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list