[libvirt] [PATCH] qemu: Use scsi-block for lun passthrough instead of scsi-disk

[Paolo Bonzini]

Il 12/03/2012 15:05, Osier Yang ha scritto:
> On 03/12/2012 09:13 PM, Paolo Bonzini wrote:
>> Il 12/03/2012 14:55, Osier Yang ha scritto:
>>> -            if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) {
>>> -                if (disk->info.addr.drive.target>  7) {
>>> -                    qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>> -                                    _("This QEMU doesn't support
>>> target "
>>> -                                      "greater than 7"));
>>> -                    goto error;
>>> -                }
>>> +            if ((disk->device != VIR_DOMAIN_DISK_DEVICE_LUN)) {
>>> +                if (!qemuCapsGet(qemuCaps,
>>> QEMU_CAPS_SCSI_DISK_CHANNEL)) {
>>> +                    if (disk->info.addr.drive.target>  7) {
>>> +                        qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> "%s",
>>> +                                        _("This QEMU doesn't support
>>> target "
>>> +                                          "greater than 7"));
>>> +                        goto error;
>>> +                    }
>>>
>>> -                if ((disk->info.addr.drive.bus !=
>>> disk->info.addr.drive.unit)&&
>>> -                    (disk->info.addr.drive.bus != 0)) {
>>> -                    qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>> -                                    _("This QEMU only supports both
>>> bus and "
>>> -                                      "unit equal to 0"));
>>> -                    goto error;
>>> +                    if ((disk->info.addr.drive.bus !=
>>> disk->info.addr.drive.unit)&&
>>> +                        (disk->info.addr.drive.bus != 0)) {
>>> +                        qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> "%s",
>>> +                                        _("This QEMU only supports
>>> both bus and "
>>> +                                          "unit equal to 0"));
>>> +                        goto error;
>>> +                    }
>>>                   }
>>> +                virBufferAddLit(&opt, "scsi-disk");
>>> +            } else {
>>> +                virBufferAddLit(&opt, "scsi-block");
>>>               }
>>>
>>> -            virBufferAddLit(&opt, "scsi-disk");
>>
>> Why not the simpler:
>>
>> -            virBufferAddLit(&opt, "scsi-disk");
>> +            if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN)
>> +                virBufferAddLit(&opt, "scsi-disk");
>> +            else
>> +                virBufferAddLit(&opt, "scsi-block");
>>
>> as in the case above for lsilogic?  Am I missing something obvious?
>>
> 
> The logic is same as yours, I didn't do any change on previous logic
> (while scsi-disk.channel is not available) excepet the indentions.

Yes, but I don't see the need to complicate the checks and assume
QEMU_CAPS_SCSI_DISK_CHANNEL is present whenever scsi-block is.  Either
you do, and then you do not need the new capability at all (seems bad),
or if you keep them separate you do not need the indentation change.

Paolo