[libvirt] [PATCH 04/25] qemu: Build qemu command line for scsi host device

[Osier Yang]

On 07/05/13 01:37, John Ferlan wrote:
> On 05/03/2013 02:07 PM, Osier Yang wrote:
>> From: Han Cheng <hanc.fnst at cn.fujitsu.com>
>>
>> Except the scsi host device's controller is "lsilogic", mapping
>> between the libvirt attributes and scsi-generic properties is:
>>
>>    libvirt     qemu
>> -----------------------------------------
>>    controller  bus ($libvirt_controller.0)
>>    bus         channel
>>    target      scsi-id
>>    unit        lun
>>
>> For scsi host device with "lsilogic" controller, the mapping is:
>> ('target (libvirt)' must be 0, as it's not used; 'unit (libvirt)
>> must <= 7).
>>
>>    libvirt            qemu
>> ----------------------------------------------------------
>>    controller && bus  bus ($libvirt_controller.$libvirt_bus)
>>    unit               scsi-id
>>
> Might be nice to include this mapping in the code comments especially
> for those looking for this type of information some day that may not
> think to look in a git commit log...
>
>> It's not good to hardcode/hard-check limits of these attributes,
>> and even worse, these limits are not documented, one has to find
>> out by either testing or reading the qemu code, I'm looking forward
>> to qemu expose limits like these one day). For example, exposing
>> "max_target", "max_lun" for megasas:
>>
>> static const struct SCSIBusInfo megasas_scsi_info = {
>>      .tcq = true,
>>      .max_target = MFI_MAX_LD,
>>      .max_lun = 255,
>>
>>      .transfer_data = megasas_xfer_complete,
>>      .get_sg_list = megasas_get_sg_list,
>>      .complete = megasas_command_complete,
>>      .cancel = megasas_command_cancel,
>> };
>>
>> Example of the qemu command line (lsilogic controller):
>>
>>    -drive file=/dev/sg2,if=none,id=drive-hostdev-scsi_host7-0-0-0 \
>>    -device scsi-generic,bus=scsi0.0,scsi-id=8,\
>>    drive=drive-hostdev-scsi_host7-0-0-0,id=hostdev-scsi_host7-0-0-0
>>
>> Example of the qemu command line (virtio-scsi controller):
>>
>>    -drive file=/dev/sg2,if=none,id=drive-hostdev-scsi_host7-0-0-0 \
>>    -device scsi-generic,bus=scsi0.0,channel=0,scsi-id=128,lun=128,\
>>    drive=drive-hostdev-scsi_host7-0-0-0,id=hostdev-scsi_host7-0-0-0
>>
>> Signed-off-by: Han Cheng <hanc.fnst at cn.fujitsu.com>
>> Signed-off-by: Osier Yang <jyang at redhat.com>
>>
>> ---
>> v3 - v4:
>>    * Remove checking for "bug == 0"
>>    * Split "bootindex" and "readonly" support into another patch
>>
>> v2.5 - v3:
>>    * Add support for all other controllers, but not only virtio-scsi
>>    * Add checking for "bus == 0"
>>    * Add checking for "target == 0" && "unit <= 7" for scsi host device
>>      which is on "lsilogic" controller.
>>    * Integrate xml2argv test from 10/10 of v2.5 into this patch
>> ---
>>   src/qemu/qemu_command.c                            | 132 ++++++++++++++++++++-
>>   src/qemu/qemu_command.h                            |   6 +
>>   .../qemuxml2argv-hostdev-scsi-lsi.args             |   9 ++
>>   .../qemuxml2argv-hostdev-scsi-lsi.xml              |  35 ++++++
>>   .../qemuxml2argv-hostdev-scsi-virtio-scsi.args     |   9 ++
>>   ...l => qemuxml2argv-hostdev-scsi-virtio-scsi.xml} |   0
>>   tests/qemuxml2argvtest.c                           |   9 ++
>>   tests/qemuxml2xmltest.c                            |   3 +-
>>   8 files changed, 199 insertions(+), 4 deletions(-)
>>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi.args
>>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi.xml
>>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-scsi.args
>>   rename tests/qemuxml2argvdata/{qemuxml2argv-hostdev-scsi.xml => qemuxml2argv-hostdev-scsi-virtio-scsi.xml} (100%)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 575dce1..df896aa 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -48,6 +48,7 @@
>>   #include "device_conf.h"
>>   #include "virstoragefile.h"
>>   #include "virtpm.h"
>> +#include "virscsi.h"
>>   #if defined(__linux__)
>>   # include <linux/capability.h>
>>   #endif
>> @@ -745,7 +746,16 @@ qemuAssignDeviceHostdevAlias(virDomainDefPtr def, virDomainHostdevDefPtr hostdev
>>           }
>>       }
>>   
>> -    if (virAsprintf(&hostdev->info->alias, "hostdev%d", idx) < 0) {
>> +    if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
>> +        if (virAsprintf(&hostdev->info->alias, "hostdev-%s-%d-%d-%d",
>> +                        hostdev->source.subsys.u.scsi.adapter,
>> +                        hostdev->source.subsys.u.scsi.bus,
>> +                        hostdev->source.subsys.u.scsi.target,
>> +                        hostdev->source.subsys.u.scsi.unit) < 0) {
>> +            virReportOOMError();
>> +            return -1;
>> +        }
>> +    } else if (virAsprintf(&hostdev->info->alias, "hostdev%d", idx) < 0) {
>>           virReportOOMError();
>>           return -1;
>>       }
>> @@ -4673,7 +4683,96 @@ qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev)
>>       return ret;
>>   }
>>   
>> +char *
>> +qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
>> +                           virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED)
>> +{
>> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
>> +    char *sg = NULL;
>> +
>> +    if (!(sg = virSCSIDeviceGetDevStr(dev->source.subsys.u.scsi.adapter,
>> +                                      dev->source.subsys.u.scsi.bus,
>> +                                      dev->source.subsys.u.scsi.target,
>> +                                      dev->source.subsys.u.scsi.unit))) {
>> +        goto error;
>> +    }
>> +
>> +    virBufferAsprintf(&buf, "file=/dev/%s,if=none", sg);
>> +    virBufferAsprintf(&buf, ",id=%s-%s",
>> +                      virDomainDeviceAddressTypeToString(dev->info->type),
>> +                      dev->info->alias);
>> +
>> +    if (virBufferError(&buf)) {
>> +        virReportOOMError();
>> +        goto error;
>> +    }
>> +
> VIR_FREE(sg);
>
>
> No comments on remainder. I assume the test config is good - at least
> it's there!
>
Fixed, and pushed.