Now, there is only SCSI bus and controller type in libvirt. And when configuring VSCSI controller, it needs to configure the spapr-vio bus address type externally. It's a little inconvenient to configure the controller type.

This patch is to add VSCSI bus type and VSCSI controller type. And handle with the problems when VSCSI and SCSI devices working together, by assign the even number to index of VSCSI controller and odd number to index of SCSI controller. And when the VSCSI controller is always assigned as SPAPRVIO bus address type and SCSI controller will be always assigned as PCI bus address, which is implemented according to the controllers' type. So when one disk is based on VSCSI controller, then assign the bus as 'vscsi', and one right VSCSI controller will be selected. If one disk is based on VSCSI controller, the XML file is as the following: <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='vscsi'/> </disk> VSCSI controllers' configuration in XML file will be like this: <controller type='vscsi' index='0'> </controller> If one disk is based on 'vscsi', the 'vscsi' bus should be assigned externally. Otherwise, it will be set as 'scsi' bus according to the 'sd' target prefix of the disk. Signed-off-by: Li Zhang&lt;zhlcindy(a)linux.vnet.ibm.com&gt; --- src/conf/domain_conf.c | 62 ++++++++++++++++++++++++++++++++++++++++++----- src/conf/domain_conf.h | 2 ++ src/qemu/qemu_command.c | 11 +++++++-- 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 184ff23..99005b7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -177,7 +177,8 @@ VIR_ENUM_IMPL(virDomainDiskBus, VIR_DOMAIN_DISK_BUS_LAST, "xen", "usb", "uml", - "sata") + "sata", + "vscsi") VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST, "default", @@ -236,7 +237,8 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "sata", "virtio-serial", "ccid", - "usb") + "usb", + "vscsi") VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, "auto", @@ -2973,6 +2975,7 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) switch (def->bus) { case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_VSCSI: def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; if (caps->hasWideScsiBus) { @@ -2981,6 +2984,16 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) * Unit 7 is the SCSI controller itself. Therefore unit 7 * cannot be assigned to disks and is skipped. */ + + /* assign even number to index of vscsi controller, + * and odd number to index of scsi controller, which can + * make vscsi controller and scsi controller work together. + */ + if (def->bus == VIR_DOMAIN_DISK_BUS_VSCSI) + def->info.addr.drive.controller = (idx / 15) * 2; + else + def->info.addr.drive.controller = (idx / 15) * 2 + 1; + def->info.addr.drive.controller = idx / 15; def->info.addr.drive.bus = 0; def->info.addr.drive.unit = idx % 15; @@ -2992,6 +3005,17 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) } else { /* For a narrow SCSI bus we define the default mapping to be * 7 units per bus, 1 bus per controller, many controllers */ + + /* assign even number to index of vscsi controller, + * and odd number to index of scsi controller, which can + * make vscsi controller and scsi controller work together. + */ + + if (def->bus == VIR_DOMAIN_DISK_BUS_VSCSI) + def->info.addr.drive.controller = (idx / 7) * 2; + else + def->info.addr.drive.controller = (idx / 7 ) * 2 + 1; + def->info.addr.drive.controller = idx / 7; def->info.addr.drive.bus = 0; def->info.addr.drive.unit = idx % 7; @@ -4061,6 +4085,10 @@ virDomainControllerDefParseXML(xmlNodePtr node, break; } + case VIR_DOMAIN_CONTROLLER_TYPE_VSCSI: + def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; + break; + default: break; } @@ -7606,6 +7634,9 @@ static int virDomainDefMaybeAddController(virDomainDefPtr def, cont->idx = idx; cont->model = -1; + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_VSCSI) + cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) { cont->opts.vioserial.ports = -1; cont->opts.vioserial.vectors = -1; @@ -10133,9 +10164,19 @@ static int virDomainDefAddDiskControllersForType(virDomainDefPtr def, maxController = def->disks[i]->info.addr.drive.controller; } - for (i = 0 ; i<= maxController ; i++) { - if (virDomainDefMaybeAddController(def, controllerType, i)< 0) - return -1; + /* To make scsi and vscsi can work together correctly, + * assign even number to index of vscsi controller + * and odd number of scsi controller.*/ + if (diskBus == VIR_DOMAIN_DISK_BUS_VSCSI) { + for (i = 0 ; i * 2<= maxController ; i ++ ) { + if (virDomainDefMaybeAddController(def, controllerType, 2 * i)< 0) + return -1; + } + } else { + for (i = 0 ; i * 2 + 1<= maxController ; i ++ ) { + if (virDomainDefMaybeAddController(def, controllerType, 2 * i + 1)< 0) + return -1; + } } return 0; @@ -10247,6 +10288,11 @@ int virDomainDefAddImplicitControllers(virDomainDefPtr def) VIR_DOMAIN_DISK_BUS_SATA)< 0) return -1; + if (virDomainDefAddDiskControllersForType(def, + VIR_DOMAIN_CONTROLLER_TYPE_VSCSI, + VIR_DOMAIN_DISK_BUS_VSCSI)< 0) + return -1; + if (virDomainDefMaybeAddVirtioSerialController(def)< 0) return -1; @@ -13175,7 +13221,11 @@ int virDiskNameToBusDeviceIndex(const virDomainDiskDefPtr disk, *devIdx = idx % 2; break; case VIR_DOMAIN_DISK_BUS_SCSI: - *busIdx = idx / 7; + *busIdx = (idx / 7) * 2 + 1; + *devIdx = idx % 7; + break; + case VIR_DOMAIN_DISK_BUS_VSCSI: + *busIdx = (idx / 7) * 2; *devIdx = idx % 7; break; case VIR_DOMAIN_DISK_BUS_FDC: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5aa8fc1..0066eba 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -418,6 +418,7 @@ enum virDomainDiskBus { VIR_DOMAIN_DISK_BUS_USB, VIR_DOMAIN_DISK_BUS_UML, VIR_DOMAIN_DISK_BUS_SATA, + VIR_DOMAIN_DISK_BUS_VSCSI, VIR_DOMAIN_DISK_BUS_LAST }; @@ -597,6 +598,7 @@ enum virDomainControllerType { VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, VIR_DOMAIN_CONTROLLER_TYPE_CCID, VIR_DOMAIN_CONTROLLER_TYPE_USB, + VIR_DOMAIN_CONTROLLER_TYPE_VSCSI, VIR_DOMAIN_CONTROLLER_TYPE_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 070d13e..bb3be17 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -57,7 +57,8 @@ VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST, "xen", "usb", "uml", - "sata") + "sata", + "vscsi") VIR_ENUM_DECL(qemuDiskCacheV1) @@ -429,6 +430,7 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk) ret = virAsprintf(&dev_name, "ide%d-cd%d", busid, devid); break; case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_VSCSI: if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) ret = virAsprintf(&dev_name, "scsi%d-hd%d", busid, devid); else @@ -1836,6 +1838,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, switch (disk->bus) { case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_VSCSI: if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unexpected address type for scsi disk")); @@ -2223,6 +2226,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def, disk->info.addr.drive.unit); break; case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_VSCSI: if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_BLOCK)) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -6447,6 +6451,8 @@ qemuParseCommandLineDisk(virCapsPtr caps, def->bus = VIR_DOMAIN_DISK_BUS_IDE; else if (STREQ(values[i], "scsi")) def->bus = VIR_DOMAIN_DISK_BUS_SCSI; + else if (STREQ(values[i], "vscsi")) + def->bus = VIR_DOMAIN_DISK_BUS_VSCSI; else if (STREQ(values[i], "virtio")) def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO; else if (STREQ(values[i], "xen")) @@ -6577,7 +6583,8 @@ qemuParseCommandLineDisk(virCapsPtr caps, if (def->bus == VIR_DOMAIN_DISK_BUS_IDE) { def->dst = strdup("hda"); - } else if (def->bus == VIR_DOMAIN_DISK_BUS_SCSI) { + } else if (def->bus == VIR_DOMAIN_DISK_BUS_SCSI || + def->bus == VIR_DOMAIN_DISK_BUS_VSCSI) { def->dst = strdup("sda"); } else if (def->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { def->dst = strdup("vda");

On Fri, May 04, 2012 at 10:05:13AM +0800, Li Zhang wrote: > Now, there is only SCSI bus and controller type in libvirt. > And when configuring VSCSI controller, it needs to configure > the spapr-vio bus address type externally. It's a little > inconvenient to configure the controller type. > > This patch is to add VSCSI bus type and VSCSI controller type. > And handle with the problems when VSCSI and SCSI devices > working together, by assign the even number to index of > VSCSI controller and odd number to index of SCSI controller. > > And when the VSCSI controller is always assigned as SPAPRVIO > bus address type and SCSI controller will be always assigned > as PCI bus address, which is implemented according to the > controllers' type. > > So when one disk is based on VSCSI controller, then assign > the bus as 'vscsi', and one right VSCSI controller will be > selected. I'm not at all convinced by this description that we need a new controller type. At most we need a separate<address/> type for the VSCSI controller under an existing SCSI controller element.

> --- > src/conf/domain_conf.c | 62 ++++++++++++++++++++++++++++++++++++++++++----- > src/conf/domain_conf.h | 2 ++ > src/qemu/qemu_command.c | 11 +++++++-- > 3 files changed, 67 insertions(+), 8 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 184ff23..99005b7 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -177,7 +177,8 @@ VIR_ENUM_IMPL(virDomainDiskBus, VIR_DOMAIN_DISK_BUS_LAST, > "xen", > "usb", > "uml", > - "sata") > + "sata", > + "vscsi") > > VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST, > "default", > @@ -236,7 +237,8 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, > "sata", > "virtio-serial", > "ccid", > - "usb") > + "usb", > + "vscsi") > > VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, > "auto", > @@ -2973,6 +2975,7 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) > > switch (def->bus) { > case VIR_DOMAIN_DISK_BUS_SCSI: > + case VIR_DOMAIN_DISK_BUS_VSCSI: > def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; > > if (caps->hasWideScsiBus) { > @@ -2981,6 +2984,16 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) > * Unit 7 is the SCSI controller itself. Therefore unit 7 > * cannot be assigned to disks and is skipped. > */ > + > + /* assign even number to index of vscsi controller, > + * and odd number to index of scsi controller, which can > + * make vscsi controller and scsi controller work together. > + */ > + if (def->bus == VIR_DOMAIN_DISK_BUS_VSCSI) > + def->info.addr.drive.controller = (idx / 15) * 2; > + else > + def->info.addr.drive.controller = (idx / 15) * 2 + 1; No, controller numbers *must* be contiguous starting from 0.

> @@ -2992,6 +3005,17 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) > } else { > /* For a narrow SCSI bus we define the default mapping to be > * 7 units per bus, 1 bus per controller, many controllers */ > + > + /* assign even number to index of vscsi controller, > + * and odd number to index of scsi controller, which can > + * make vscsi controller and scsi controller work together. > + */ > + > + if (def->bus == VIR_DOMAIN_DISK_BUS_VSCSI) > + def->info.addr.drive.controller = (idx / 7) * 2; > + else > + def->info.addr.drive.controller = (idx / 7 ) * 2 + 1; > + Again NACK to this. Daniel