On 2013年03月04日 14:01, Han Cheng wrote: > For scsi-generic, the command line will be like: > > -drive file=/dev/sg0,if=none,id=drive-hostdev-scsi0-0-0-0 -device > scsi-generic,bus=scsi0.0,channel=0,scsi-id=4,lun=8,drive=drive-hostdev-scsi0-0-0-0,id=hostdev-scsi0-0-0-0 > > The relationship between the libvirt address attrs and the qdev > properties are(channel should always be 0): > bus=scsi<controller>.0 > scsi-id=<target> > lun=<unit> > --- > src/qemu/qemu_command.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 153 insertions(+), 3 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 5eb9999..1d2540e 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -49,6 +49,8 @@ > > #include<sys/stat.h> > #include<fcntl.h> > +#include<dirent.h> > +#include<sys/types.h> > > #define VIR_FROM_THIS VIR_FROM_QEMU > > @@ -651,7 +653,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-scsi%d-%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; > } > @@ -3864,6 +3875,110 @@ qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev) > return ret; > } > > +static int > +scsiGetDeviceId(unsigned int adapter, > + unsigned int bus, > + unsigned int target, > + unsigned int unit) This should be a util function, and note that it's Linux platform specific. > +{ > + DIR *dir = NULL; > + struct dirent *entry; > + char *path; > + int id; s/int/unsigned int/, > + > + if (virAsprintf(&path, > + "/sys/bus/scsi/devices/target%d:%d:%d/%d:%d:%d:%d/scsi_generic", > + adapter, bus, target, adapter, bus, target, unit)< 0) { > + virReportOOMError(); > + return -1; > + } > + dir = opendir(path); > + VIR_FREE(path); > + if (!dir) { if (!(dir = opendir(path))) > + VIR_WARN("Failed to open %s", path); Any special reason for this to be a warning, but not an error instead? > + return -1; > + } > + > + while ((entry = readdir(dir))) { > + if (entry->d_name[0] == '.') > + continue; > + > + if (sscanf(entry->d_name, "sg%d",&id) != 1) { > + VIR_WARN("Failed to analyse sg id"); > + return -1; Since here it returns -1, above should be an error. > + } > + } > + > + return id; > +} > + > +static char * > +qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps) > +{ > + virBuffer buf = VIR_BUFFER_INITIALIZER; > + int scsiid; > + > + if ((scsiid = scsiGetDeviceId(dev->source.subsys.u.scsi.adapter, > + dev->source.subsys.u.scsi.bus, > + dev->source.subsys.u.scsi.target, > + dev->source.subsys.u.scsi.unit))< 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("can't get sg* number")); "*" is magic in the error message. s/sg\*/sg/, And actually if you throw error in scsiGetDeviceId, there is no need to report error here anymore. > + goto error; > + } > + > + virBufferAsprintf(&buf, "file=/dev/sg%d,if=none", scsiid); > + virBufferAsprintf(&buf, ",id=%s-%s", > + virDomainDeviceAddressTypeToString(dev->info->type), > + dev->info->alias); > + if (dev->info->readonly&& > + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) > + virBufferAsprintf(&buf, ",readonly=on"); > + > + return virBufferContentAndReset(&buf); Should check if there is any error of the buf here. Something like: if (virBufferError(&buf)) { virReportOOMErrorr(); goto error; } > +error: > + virBufferFreeAndReset(&buf); > + return NULL; > +} > + > + > +static char * > +qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, virDomainHostdevDefPtr dev, > + virQEMUCapsPtr qemuCaps) > +{ > + virBuffer buf = VIR_BUFFER_INITIALIZER; > + int controllerModel = -1; > + > + controllerModel = virDomainInfoFindControllerModel(def, dev->info, > + VIR_DOMAIN_CONTROLLER_TYPE_SCSI); > + if (qemuSetScsiControllerModel(def, qemuCaps,&controllerModel)< 0) > + goto error; > + /* TODO: deal with lsi or ibm controller */ > + if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI) { > + virBufferAsprintf(&buf, "scsi-generic"); > + if (dev->info->addr.drive.bus != 0) > + virReportError(VIR_ERR_INTERNAL_ERROR, > + "%s", _("SCSI controller only supports 1 bus")); XML_ERROR here might be better. > + /* TODO: deal with early version qemu which does not support bus... */ > + virBufferAsprintf(&buf, > + ",bus=scsi%d.0,channel=0,scsi-id=%d,lun=%d", > + dev->info->addr.drive.controller, > + dev->info->addr.drive.target, > + dev->info->addr.drive.unit); > + virBufferAsprintf(&buf, ",drive=%s-%s,id=%s", > + virDomainDeviceAddressTypeToString(dev->info->type), > + dev->info->alias, dev->info->alias); > + if (dev->info->bootIndex) > + virBufferAsprintf(&buf, ",bootindex=%d", dev->info->bootIndex); > + } else { > + goto error; This is magic, you need to give an error message here. > + } > + > + return virBufferContentAndReset(&buf); Need to check if there is error of buf too here. > +error: > + virBufferFreeAndReset(&buf); > + return NULL; > +} > > > /* This function outputs a -chardev command line option which describes only the > @@ -6953,10 +7068,11 @@ qemuBuildCommandLine(virConnectPtr conn, > if (hostdev->info->bootIndex) { > if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || > (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI&& > - hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)) { > + hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB&& > + hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > _("booting from assigned devices is only" > - " supported for PCI and USB devices")); > + " supported for PCI, USB and SCSI devices")); > goto error; > } else { > if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI&& > @@ -6973,6 +7089,40 @@ qemuBuildCommandLine(virConnectPtr conn, > " supported with this version of qemu")); > goto error; > } > + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI&& > + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_HOST_BOOTINDEX)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("booting from assigned SCSI devices is not" > + " supported with this version of qemu")); > + goto error; > + } > + } > + } > + > + /* SCSI */ > + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS&& > + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { > + /* TODO */ > + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE)&& > + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)&& > + virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_GENERIC)) { > + char *drvstr; > + > + virCommandAddArg(cmd, "-drive"); > + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps))) > + goto error; > + virCommandAddArg(cmd, drvstr); > + VIR_FREE(drvstr); > + > + virCommandAddArg(cmd, "-device"); > + if (!(devstr = qemuBuildSCSIHostdevDevStr(def, hostdev, qemuCaps))) > + goto error; > + virCommandAddArg(cmd, devstr); > + VIR_FREE(devstr); > + } else { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("SCSI assignment is not supported by this version of qemu")); In libvirt, we use "passthrough", instead of "assignment". > + goto error; > } > } > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list