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

On 05/03/2013 02:07 PM, Osier Yang wrote: > From: Han Cheng <hanc.fnst(a)cn.fujitsu.com&gt; > > 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(a)cn.fujitsu.com&gt; > Signed-off-by: Osier Yang <jyang(a)redhat.com&gt; > > --- > 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!