[libvirt] [PATCH REPOST 6/7] qemu: Use switch for qemuCheckIOThreads

[John Ferlan]

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 at 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