[libvirt] [PATCH 1/2] Add VSCSI bus type and VSCSI controller type for pseries guest.

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 <zhlcindy@linux.vnet.ibm.com> --- 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"); -- 1.7.9.5

On 2012年05月04日 10:05, 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.
Not sure if I understood correctly, but we have a scsi controller model type 'ibmvscsi'. Isn't it enough to tell the difference? And 'ibmvsci' is choosed as the default scsi controller model if the guest arch is ppc64. Also qemu driver will assign the default 'reg' for spapr-vio address type when building the qemu command line (if reg is not specified). So it doesn't need to specify the spapr-vio address type explicitly if I'm right. And thus I can't see any inconvenience. :-)
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<zhlcindy@linux.vnet.ibm.com> --- 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 05/04/2012 02:45 PM, Osier Yang wrote:
On 2012年05月04日 10:05, 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.
Not sure if I understood correctly, but we have a scsi controller model type 'ibmvscsi'. Isn't it enough to tell the difference?
I think it's not enough.
And 'ibmvsci' is choosed as the default scsi controller model if the guest arch is ppc64. Also qemu driver will assign the default 'reg' for spapr-vio address type when building the qemu command line (if reg is not specified). So it doesn't need to specify the spapr-vio address type explicitly if I'm right. And thus I can't see any inconvenience. :-)
In fact, for pseries guest, scsi controller based on pci bus can also work, which can't use ibmvscsi model. In this condition, it needs to specify the controller model and pci bus address in XML file. For virt-manager, to make it work correctly, we need to specify the model and address type in the controller configuration. So I think it is inconvenient. This patch adds the VSCSI bus type and VSCSI controller type. And then the default model will be selected according to the controller type. And then disk can find the right controller to work without specifying the model and address type explicitly. In a word, some times, there should be two kinds of SCSI controllers with different address type and model on pseries. Just using the default model 'ibmvscsi' can't make it work correctly. Thanks. Li
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<zhlcindy@linux.vnet.ibm.com> --- 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");
-- Best Regards Li IBM LTC, China System&Technology Lab, Beijing

On Fri, May 04, 2012 at 05:23:55PM +0800, Li Zhang wrote:
On 05/04/2012 02:45 PM, Osier Yang wrote:
On 2012年05月04日 10:05, 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.
Not sure if I understood correctly, but we have a scsi controller model type 'ibmvscsi'. Isn't it enough to tell the difference?
I think it's not enough.
And 'ibmvsci' is choosed as the default scsi controller model if the guest arch is ppc64. Also qemu driver will assign the default 'reg' for spapr-vio address type when building the qemu command line (if reg is not specified). So it doesn't need to specify the spapr-vio address type explicitly if I'm right. And thus I can't see any inconvenience. :-)
In fact, for pseries guest, scsi controller based on pci bus can also work, which can't use ibmvscsi model. In this condition, it needs to specify the controller model and pci bus address in XML file.
For virt-manager, to make it work correctly, we need to specify the model and address type in the controller configuration. So I think it is inconvenient.
This patch adds the VSCSI bus type and VSCSI controller type. And then the default model will be selected according to the controller type. And then disk can find the right controller to work without specifying the model and address type explicitly.
In a word, some times, there should be two kinds of SCSI controllers with different address type and model on pseries. Just using the default model 'ibmvscsi' can't make it work correctly.
To be honest "inconvenient" isn't really relevant here. From your description, what we already have supported will work, if applications learn how to configure s390 properly. So I don't think we should be changing this again. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

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 -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 05/04/2012 05:04 PM, Daniel P. Berrange wrote:
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.
The problem is that when using two kinds of SCSI controller, vscsi and scsi, which are based on different address type and model at the same time, it needs to specify the model and address type externally. And it's hard to find the right controller for disk to use, only by the 'sdx'.
--- 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.
If two kinds of SCSI controllers work together, it will be a problem. For example, there are 2 disks, sda and sdb. sda is on vscsi, and sdb is on scsi. sda and sdb can't be on the same controller. With the original method, sda will be on controller 0, unit 0. sdb will be controller 0, unit 1. If controller 0 is based on spapr-vio address type, sdb can't work at all. Do you have any suggestion to fix this problem? Thanks. Li
@@ -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
-- Best Regards Li IBM LTC, China System&Technology Lab, Beijing

On Fri, May 04, 2012 at 05:49:54PM +0800, Li Zhang wrote:
On 05/04/2012 05:04 PM, Daniel P. Berrange wrote:
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.
The problem is that when using two kinds of SCSI controller, vscsi and scsi, which are based on different address type and model at the same time, it needs to specify the model and address type externally. And it's hard to find the right controller for disk to use, only by the 'sdx'.
Sure, but there's nothing special about s390 / vSCSI in that respect. If you have a guest with a SATA controller and a SCSI controller you have the same problem. If you have multiple disk controllers assigned to the guest, and are adding a disk, then you are responsible for providing an <address> element for the disk to indicate which controller it should be added to. The automagic guessing of controllers is only ever intended to work in the simple case where you have 1 kind of controller Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Fri 04 May 2012 05:55:46 PM CST, Daniel P. Berrange wrote:
On Fri, May 04, 2012 at 05:49:54PM +0800, Li Zhang wrote:
On 05/04/2012 05:04 PM, Daniel P. Berrange wrote:
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.
The problem is that when using two kinds of SCSI controller, vscsi and scsi, which are based on different address type and model at the same time, it needs to specify the model and address type externally. And it's hard to find the right controller for disk to use, only by the 'sdx'.
Sure, but there's nothing special about s390 / vSCSI in that respect. If you have a guest with a SATA controller and a SCSI controller you have the same problem. If you have multiple disk controllers assigned to the guest, and are adding a disk, then you are responsible for providing an<address> element for the disk to indicate which controller it should be added to.
OK. I see. I will try that way.
The automagic guessing of controllers is only ever intended to work in the simple case where you have 1 kind of controller
Got it. Thanks. :)
Daniel
-- Best Regards Li IBM LTC, China System&Technology Lab, Beijing

On 05/03/2012 10:05 PM, 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.
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.
First off, sorry for taking so long to respond. This isn't exactly what I had in mind when I recommended a libvirt patch. Change libvirt to add: <controller type='scsi' index='0'> <address type='spapr-vio'/> </controller> when the guest XML has <disk type='file' device='disk' > <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi'/> <address type='spapr-vio'/> </disk> and the guest XML doesn't already have spapr-vio <controller>. Then virt-manager etc. only needs to know that for pseries guests, the default disk should use bus='scsi' and <address type='spapr-vio'/>, and libvirt will auto add the correct <controller> for us, like it does for ide, sata, etc. already. The patch has obviously weird logic with the even/odd approach, so I think this approach is better. Thanks, Cole

On 05/30/2012 07:25 AM, Cole Robinson wrote:
On 05/03/2012 10:05 PM, 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.
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.
First off, sorry for taking so long to respond.
This isn't exactly what I had in mind when I recommended a libvirt patch. Change libvirt to add:
<controller type='scsi' index='0'> <address type='spapr-vio'/> </controller>
when the guest XML has
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi'/> <address type='spapr-vio'/> </disk>
and the guest XML doesn't already have spapr-vio<controller>.
Thanks a lot for your suggestion. :) I see the libvirt will always set the address type of the disk as "drive". There is one drive address structure in libvirt, which records the controller, bus and unit. the address can be specified in the target. <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi' address='spapr-vio'/> <address type='drive' controller='0' bus='0' unit='0' /> </disk>
Then virt-manager etc. only needs to know that for pseries guests, the default disk should use bus='scsi' and<address type='spapr-vio'/>, and libvirt will auto add the correct<controller> for us, like it does for ide, sata, etc. already.
The patch has obviously weird logic with the even/odd approach, so I think this approach is better.
Yes, I see. Thanks. :)
Thanks, Cole
-- Best Regards Li IBM LTC, China System&Technology Lab, Beijing

On 05/30/2012 02:57 AM, Li Zhang wrote:
On 05/30/2012 07:25 AM, Cole Robinson wrote:
On 05/03/2012 10:05 PM, 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.
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.
First off, sorry for taking so long to respond.
This isn't exactly what I had in mind when I recommended a libvirt patch. Change libvirt to add:
<controller type='scsi' index='0'> <address type='spapr-vio'/> </controller>
when the guest XML has
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi'/> <address type='spapr-vio'/> </disk>
and the guest XML doesn't already have spapr-vio<controller>.
Thanks a lot for your suggestion. :)
I see the libvirt will always set the address type of the disk as "drive". There is one drive address structure in libvirt, which records the controller, bus and unit.
the address can be specified in the target.
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi' address='spapr-vio'/> <address type='drive' controller='0' bus='0' unit='0' /> </disk>
That sounds okay to me, bug maybe eblake or danpb should sign off on it conceptually, since it's a new XML element. - Cole

On 06/04/2012 11:16 PM, Cole Robinson wrote:
On 05/30/2012 02:57 AM, Li Zhang wrote:
On 05/30/2012 07:25 AM, Cole Robinson wrote:
On 05/03/2012 10:05 PM, 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.
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.
First off, sorry for taking so long to respond.
This isn't exactly what I had in mind when I recommended a libvirt patch. Change libvirt to add:
<controller type='scsi' index='0'> <address type='spapr-vio'/> </controller>
when the guest XML has
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi'/> <address type='spapr-vio'/> </disk>
and the guest XML doesn't already have spapr-vio<controller>.
Thanks a lot for your suggestion. :)
I see the libvirt will always set the address type of the disk as "drive". There is one drive address structure in libvirt, which records the controller, bus and unit.
the address can be specified in the target.
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi' address='spapr-vio'/> <address type='drive' controller='0' bus='0' unit='0' /> </disk>
That sounds okay to me, bug maybe eblake or danpb should sign off on it conceptually, since it's a new XML element.
Got it. Eric, Daniel Any comments ? Thanks. :)
- Cole
-- Best Regards Li IBM LTC, China System&Technology Lab, Beijing

On 06/04/2012 09:16 AM, Cole Robinson wrote:
This isn't exactly what I had in mind when I recommended a libvirt patch. Change libvirt to add:
<controller type='scsi' index='0'> <address type='spapr-vio'/> </controller>
when the guest XML has
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi'/> <address type='spapr-vio'/> </disk>
and the guest XML doesn't already have spapr-vio<controller>.
Thanks a lot for your suggestion. :)
I see the libvirt will always set the address type of the disk as "drive". There is one drive address structure in libvirt, which records the controller, bus and unit.
It sounds like we need <disk> address structure to support more than just drive addressing, but to instead support a union of addressing based on the type='' attribute.
the address can be specified in the target.
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi' address='spapr-vio'/>
No, I don't think you want address added to <target>, but...
<address type='drive' controller='0' bus='0' unit='0' />
you instead want <address type='spapr-vio'/> to be something you add to your XML to force libvirt to then populate the remaining attributes and auto-instantiate the controller.
</disk>
That sounds okay to me, bug maybe eblake or danpb should sign off on it conceptually, since it's a new XML element.
- Cole
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Il 07/06/2012 00:45, Eric Blake ha scritto:
It sounds like we need <disk> address structure to support more than just drive addressing, but to instead support a union of addressing based on the type='' attribute.
No, we don't. The disk has a normal bus/target/unit address, like any other SCSI disk.
the address can be specified in the target.
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi' address='spapr-vio'/>
No, I don't think you want address added to <target>, but...
<address type='drive' controller='0' bus='0' unit='0' />
you instead want <address type='spapr-vio'/> to be something you add to your XML to force libvirt to then populate the remaining attributes and auto-instantiate the controller.
A simple <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi'/> <address type='drive' controller='0' bus='0' unit='0' /> </disk> will do exactly what you want and add a default ibmvscsi controller. More complex cases are handled with the controller element: - giving a specific address to the controller: <controller type='scsi' model='ibmvscsi'> <address type='spapr-vio' ... /> </disk> - using a PCI controller even with an sPAPR machine: <controller type='scsi' model='virtio-scsi'> <address type='pci' ... /> </disk> - using both a vscsi and a PCI controller: <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi'/> <address type='drive' controller='0' bus='0' unit='0' /> </disk> <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sdb' bus='scsi'/> <address type='drive' controller='1' bus='0' unit='0' /> </disk> <controller type='scsi' model='ibmvscsi'/> <controller type='scsi' model='virtio-scsi'/> (possibly with an spapr-vio and a pci address in the respective controller elements). Paolo

On 06/07/2012 11:42 PM, Paolo Bonzini wrote:
Il 07/06/2012 00:45, Eric Blake ha scritto:
It sounds like we need<disk> address structure to support more than just drive addressing, but to instead support a union of addressing based on the type='' attribute.
No, we don't. The disk has a normal bus/target/unit address, like any other SCSI disk.
the address can be specified in the target.
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi' address='spapr-vio'/>
No, I don't think you want address added to<target>, but...
<address type='drive' controller='0' bus='0' unit='0' />
you instead want<address type='spapr-vio'/> to be something you add to your XML to force libvirt to then populate the remaining attributes and auto-instantiate the controller.
A simple
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi'/> <address type='drive' controller='0' bus='0' unit='0' /> </disk>
will do exactly what you want and add a default ibmvscsi controller. More complex cases are handled with the controller element:
It's a little hard to know if the disk is on a ibmvscsi controller. If it is only support one type of scsi, it's OK with this configuration. On sPAPR platform, it can support vscsi and pci scsi. There will be some problems in this situation. For example, user wants 2 disks: sda is vscsi, sdb is scsi. It's hard to know sda is a vscsi with this configuration. So when creating one controller for this disk automatically, the address of controller may be not right. It may need users to select right type of SCSI controller manually.
- giving a specific address to the controller:
<controller type='scsi' model='ibmvscsi'> <address type='spapr-vio' ... /> </disk>
- using a PCI controller even with an sPAPR machine:
<controller type='scsi' model='virtio-scsi'> <address type='pci' ... /> </disk>
- using both a vscsi and a PCI controller:
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi'/> <address type='drive' controller='0' bus='0' unit='0' /> </disk> <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sdb' bus='scsi'/> <address type='drive' controller='1' bus='0' unit='0' /> </disk> <controller type='scsi' model='ibmvscsi'/> <controller type='scsi' model='virtio-scsi'/>
(possibly with an spapr-vio and a pci address in the respective controller elements).
Paolo
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Best Regards Li IBM LTC, China System&Technology Lab, Beijing

Il 08/06/2012 04:08, Li Zhang ha scritto:
A simple
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi'/> <address type='drive' controller='0' bus='0' unit='0' /> </disk>
will do exactly what you want and add a default ibmvscsi controller. More complex cases are handled with the controller element:
It's a little hard to know if the disk is on a ibmvscsi controller. It may need users to select right type of SCSI controller manually.
Of course. Defaults are there so that people can override them. :) If people find defaults confusing, they can just avoid using them and write down everything. Paolo

On 06/07/2012 06:45 AM, Eric Blake wrote:
On 06/04/2012 09:16 AM, Cole Robinson wrote:
This isn't exactly what I had in mind when I recommended a libvirt patch. Change libvirt to add:
<controller type='scsi' index='0'> <address type='spapr-vio'/> </controller>
when the guest XML has
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi'/> <address type='spapr-vio'/> </disk>
and the guest XML doesn't already have spapr-vio<controller>.
Thanks a lot for your suggestion. :)
I see the libvirt will always set the address type of the disk as "drive". There is one drive address structure in libvirt, which records the controller, bus and unit.
It sounds like we need<disk> address structure to support more than just drive addressing, but to instead support a union of addressing based on the type='' attribute.
I think if we add the type of "spapr-vio", it has the almost same of "drive".
the address can be specified in the target.
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/path/file'/> <target dev='sda' bus='scsi' address='spapr-vio'/>
No, I don't think you want address added to<target>, but...
<address type='drive' controller='0' bus='0' unit='0' />
you instead want<address type='spapr-vio'/> to be something you add to your XML to force libvirt to then populate the remaining attributes and auto-instantiate the controller. In fact, this address is used to find right controller. The disk is selected by users, libvirt needs to help find right controller. And also, this type of disk can be reconfigured after it is created.
</disk>
That sounds okay to me, bug maybe eblake or danpb should sign off on it conceptually, since it's a new XML element.
- Cole
-- Best Regards Li IBM LTC, China System&Technology Lab, Beijing
participants (6)
-
Cole Robinson
-
Daniel P. Berrange
-
Eric Blake
-
Li Zhang
-
Osier Yang
-
Paolo Bonzini