[libvirt] [PATCH 0/5]qemu: add usb-bot scsi controller support
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=917702 qemu patch:http://lists.gnu.org/archive/html/qemu-devel/2013-01/msg02200.html These patch aims to add usb-bot SCSI controller support for libvirt. As usb-storage libvirt already supported, usb-bot only supports one SCSI target with SCSI ID 0. The difference is that usb-storage creates SCSI HBA and additionaly either a scsi-disk or a scsi-generic device, usb-bot only creates the SCSI HBA. we can attach a SCSI device to it with another -device. usb-bot supports a single target and a number of luns. The limit is 15 luns (0~15 LUN ID) and they must be contiguous(using lun 0 and 2 without 1 doesn't work). Athought usb-bot is a SCSI controller it needs to be attached to a exsiting USB controller for work. So it has to use address of usb type. Libvirt XML: <devices> ... <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/disk.qcow2'/> <target dev='sdc' bus='scsi'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'/> <controller type='scsi' index='0' model='usb-bot'> <address type='usb' bus='0' port='1'/> </controller> ... </devices> The QEMU commandline: qemu ${other_vm_args} \ -device piix3-usb-uhci,id=usb \ -device usb-bot,id=scsi0,bus=usb.0,port=1 \ -drive file=/var/lib/libvirt/images/disk.qcow2,if=none,id=drive-scsi0-0-0-0,format=raw \ -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 As the usb-storage creates scsi disk automatically which doesn't let you set scsi-disk properties such as vendor, product, wwn, channel, scsi-id and lun. So QEMU guys prefer usb-bot to usb-storage. So this is the first part of the whole work. Next step will replace usb-storage with usb-bot when disk in xml uses usb bus like <disk ...> <...> <target bus='usb'> </disk> Guannan Ren(5) qemu: add usb-bot qemu cap flag qemu: add usb-bot model scsi controller support qemu: add usb-bot support from disks points of view qemu: refactor out function to build scsi device qemu commandline tests: add xml2argv test for usb-bot scsi controller docs/formatdomain.html.in | 4 +-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- src/conf/domain_conf.h | 5 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 124 ++++++++++++++++++++++++++++++++++++++++++------------------------------- src/vmx/vmx.c | 3 +- tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args | 10 ++++++ tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml | 33 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 12 files changed, 242 insertions(+), 57 deletions(-)
QEMU_CAPS_DEVICE_USB_BOT /* -device usb-bot */ --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 4 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7888e2d..6a775ee 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -237,6 +237,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "dmi-to-pci-bridge", "i440fx-pci-hole64-size", "q35-pci-hole64-size", + + "usb-bot", /* 155 */ ); struct _virQEMUCaps { @@ -1375,6 +1377,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "vmware-svga", QEMU_CAPS_DEVICE_VMWARE_SVGA }, { "usb-serial", QEMU_CAPS_DEVICE_USB_SERIAL }, { "usb-net", QEMU_CAPS_DEVICE_USB_NET }, + { "usb-bot", QEMU_CAPS_DEVICE_USB_BOT }, { "virtio-rng-pci", QEMU_CAPS_DEVICE_VIRTIO_RNG }, { "virtio-rng-s390", QEMU_CAPS_DEVICE_VIRTIO_RNG }, { "virtio-rng-ccw", QEMU_CAPS_DEVICE_VIRTIO_RNG }, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 69f3395..67746a4 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -193,6 +193,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE = 152, /* -device i82801b11-bridge */ QEMU_CAPS_I440FX_PCI_HOLE64_SIZE = 153, /* i440FX-pcihost.pci-hole64-size */ QEMU_CAPS_Q35_PCI_HOLE64_SIZE = 154, /* q35-pcihost.pci-hole64-size */ + QEMU_CAPS_DEVICE_USB_BOT = 155, /* -device usb-bot */ QEMU_CAPS_LAST, /* this must always be the last item */ }; -- 1.8.3.1
usb-bot is SCSI HBA which support only one SCSI target with ID 0. we can create one or more SCSI devices connected to it with -device as its luns. For usb-bot the limit is 15 luns. The difference from other SCSI controllers is that usb-bot needs usb-bus support. That means usb-bot is required to be attached to a existing USB controller. libvirt xml example: <devices> ... <controller type='usb' index='0'> </controller> <controller type='scsi' index='0' model='usb-bot'> <address type='usb' bus='0' port='1'/> </controller> ... </devices> QEMU commandline should be: -device piix3-usb-uhci,id=usb \ -device usb-bot,id=scsi0,bus=usb.0,port=1 --- docs/formatdomain.html.in | 4 ++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 52 +++++++++++++++++++++++++++++++++++++++++-- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 16 +++++++++++++ src/vmx/vmx.c | 3 ++- 6 files changed, 72 insertions(+), 5 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index cce179d..274acf4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2413,8 +2413,8 @@ control how many devices can be connected through the controller. A "scsi" controller has an optional attribute <code>model</code>, which is one of "auto", "buslogic", - "ibmvscsi", "lsilogic", "lsisas1068", "lsisas1078", "virtio-scsi" or - "vmpvscsi". A "usb" controller has an optional attribute + "ibmvscsi", "lsilogic", "lsisas1068", "lsisas1078", "virtio-scsi", + "vmpvscsi" or "usb-bot". A "usb" controller has an optional attribute <code>model</code>, which is one of "piix3-uhci", "piix4-uhci", "ehci", "ich9-ehci1", "ich9-uhci1", "ich9-uhci2", "ich9-uhci3", "vt82c686b-uhci", "pci-ohci" or "nec-xhci". Additionally, diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6978dc7..d748d68 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1509,6 +1509,7 @@ <value>ibmvscsi</value> <value>virtio-scsi</value> <value>lsisas1078</value> + <value>usb-bot</value> </choice> </attribute> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f8fbf79..545c157 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -324,7 +324,8 @@ VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAS "vmpvscsi", "ibmvscsi", "virtio-scsi", - "lsisas1078"); + "lsisas1078", + "usb-bot"); VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST, "piix3-uhci", @@ -5736,6 +5737,17 @@ virDomainControllerDefParseXML(xmlNodePtr node, goto error; switch (def->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { + if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("usb-bot mode of scsi controller requires " + "address of type 'usb'")); + goto error; + } + } + break; case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: { char *ports = virXMLPropString(node, "ports"); if (ports) { @@ -5826,7 +5838,8 @@ virDomainControllerDefParseXML(xmlNodePtr node, def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Controllers must use the 'pci' address type")); goto error; @@ -13778,6 +13791,38 @@ virDomainDefMaybeAddSmartcardController(virDomainDefPtr def) return 0; } +static int +virDomainDefMaybeAddUSBcontroller(virDomainDefPtr def) +{ + size_t i; + int maxController = -1; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr cont = def->controllers[i]; + + if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) + continue; + + if (cont->model != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) + continue; + + if ((int)cont->info.addr.usb.bus > maxController) + maxController = cont->info.addr.usb.bus; + } + + if (maxController == -1) + return 0; + + for (i = 0; i <= maxController; i++) { + if (virDomainDefMaybeAddController(def, + VIR_DOMAIN_CONTROLLER_TYPE_USB, + i, -1) < 0) + return -1; + } + + return 0; +} + /* * Based on the declared <address/> info for any devices, * add necessary drive controllers which are not already present @@ -13816,6 +13861,9 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) if (virDomainDefMaybeAddHostdevSCSIcontroller(def) < 0) return -1; + if (virDomainDefMaybeAddUSBcontroller(def) < 0) + return -1; + return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 56739b7..3e69f84 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -787,6 +787,7 @@ enum virDomainControllerModelSCSI { VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078, + VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f8fccea..5f08a72 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -684,6 +684,14 @@ qemuSetScsiControllerModel(virDomainDefPtr def, return -1; } break; + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_BOT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support " + "usb-bot scsi controller")); + return -1; + } + break; case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI: /*TODO: need checking work here if necessary */ break; @@ -2692,6 +2700,11 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, def->controllers[i]->idx == 0) continue; + /* SCSI controller model 'usb-bot' needs address of USB type */ + if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI && + def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) + continue; + if (def->controllers[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) continue; if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) @@ -4614,6 +4627,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: virBufferAddLit(&buf, "lsi"); break; + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT: + virBufferAddLit(&buf, "usb-bot"); + break; case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI: virBufferAddLit(&buf, "spapr-vscsi"); break; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 11e8a62..35be3ba 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -514,7 +514,8 @@ VIR_ENUM_IMPL(virVMXControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, "pvscsi", "UNUSED ibmvscsi", "UNUSED virtio-scsi", - "UNUSED lsisas1078"); + "UNUSED lsisas1078", + "UNUSED usb-scsi"); /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -- 1.8.3.1
usb-bot only supports 16 luns(0~15) and they must be contiguous, (using lun 0 and 2 without 1 doesn't work). In this case qemu doesn't throw an error, we can not find the lun 2 in guests. So Adding a checking function in libvirt to prevent from this case. --- src/conf/domain_conf.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 32 ++++++++++++++++++++++++++ 4 files changed, 96 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 545c157..84dfe25 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4287,6 +4287,65 @@ virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, return 0; } +bool +virDomainDiskAttachedToUsbbotLunIsContiguous(virDomainDefPtr def) +{ + size_t i; + int controllerModel; + virBitmapPtr units = NULL; + bool is_set = false; + bool ret = false; + + if (!(units = virBitmapNew(SCSI_CONTROLLER_USB_BOT_MODEL_MAX_LUNS))) + goto cleanup; + + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDefPtr disk = def->disks[i]; + int unitValue = disk->info.addr.drive.unit; + + if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) + continue; + + controllerModel = + virDomainDeviceFindControllerModel(def, &disk->info, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI); + if (controllerModel != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) + continue; + + /* usb-bot only supports 16 luns */ + if (unitValue & ~0xf) { + virReportError(VIR_ERR_XML_ERROR, + _("The address unit value of disk '%s' is too big"), + disk->src); + goto cleanup; + } + + if (virBitmapGetBit(units, unitValue, &is_set) == 0 && is_set) { + virReportError(VIR_ERR_XML_ERROR, + _("The address unit value of disk '%s' is already used"), + disk->src); + goto cleanup; + } + + if (unitValue > 0) { + if (virBitmapGetBit(units, unitValue - 1, &is_set) == 0 && !is_set) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("The address unit value of disk " + "attached to usb-bot controller is not contiguous")); + goto cleanup; + } + } + + ignore_value(virBitmapSetBit(units, unitValue)); + } + + ret = true; + +cleanup: + virBitmapFree(units); + return ret; +} + static virSecurityLabelDefPtr virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, unsigned int flags) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3e69f84..9e2f477 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -778,6 +778,8 @@ typedef enum { VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST } virDomainControllerModelPCI; +# define SCSI_CONTROLLER_USB_BOT_MODEL_MAX_LUNS 16 + enum virDomainControllerModelSCSI { VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC, @@ -2362,6 +2364,8 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def, int virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, virDomainDiskDefPtr def); +bool virDomainDiskAttachedToUsbbotLunIsContiguous(virDomainDefPtr def); + virDomainDiskDefPtr virDomainDiskRemove(virDomainDefPtr def, size_t i); virDomainDiskDefPtr diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35f0f1b..c1f7da5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -179,6 +179,7 @@ virDomainDeviceFindControllerModel; virDomainDeviceInfoCopy; virDomainDeviceInfoIterate; virDomainDeviceTypeToString; +virDomainDiskAttachedToUsbbotLunIsContiguous; virDomainDiskBusTypeToString; virDomainDiskCacheTypeFromString; virDomainDiskCacheTypeToString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5f08a72..f38e98f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4257,6 +4257,32 @@ qemuBuildDriveDevStr(virDomainDefPtr def, disk->info.addr.drive.controller, disk->info.addr.drive.bus, disk->info.addr.drive.unit); + } else if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { + if (disk->info.addr.drive.target != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("target must be 0 for controller " + "model 'usb-bot'")); + goto error; + } + + if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) + virBufferAddLit(&opt, "scsi-cd"); + else + virBufferAddLit(&opt, "scsi-hd"); + } else { + virBufferAddLit(&opt, "scsi-disk"); + } + } else { + virBufferAddLit(&opt, "scsi-block"); + } + + virBufferAsprintf(&opt, ",bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d", + disk->info.addr.drive.controller, + disk->info.addr.drive.bus, + disk->info.addr.drive.target, + disk->info.addr.drive.unit); } else { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { if (disk->info.addr.drive.target > 7) { @@ -8040,6 +8066,12 @@ qemuBuildCommandLine(virConnectPtr conn, } } + /* For disks attached to SCSI usb-bot controller, their + * unit value must be contiguous. + */ + if (!virDomainDiskAttachedToUsbbotLunIsContiguous(def)) + goto error; + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) { for (i = 0; i < def->ncontrollers; i++) { -- 1.8.3.1
On Mon, Sep 02, 2013 at 05:38:42PM +0800, Guannan Ren wrote:
usb-bot only supports 16 luns(0~15) and they must be contiguous, (using lun 0 and 2 without 1 doesn't work). In this case qemu doesn't throw an error, we can not find the lun 2 in guests. So Adding a checking function in libvirt to prevent from this case.
Hmm, this seems like a problematic restriction. How does this work if we start off a guest with 3 disks attached to the usb-bot SCSI controller. Then hot-unplug the 2nd disk. Next time we need will need to start with with LUNs 0 and 2 populated only, otherwise we have ABI change upon migrate or save/restore. I think this restriction about contiguous luns must be removed if this device is to be usable with multiple luns. 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 09/02/2013 08:57 PM, Daniel P. Berrange wrote:
On Mon, Sep 02, 2013 at 05:38:42PM +0800, Guannan Ren wrote:
usb-bot only supports 16 luns(0~15) and they must be contiguous, (using lun 0 and 2 without 1 doesn't work). In this case qemu doesn't throw an error, we can not find the lun 2 in guests. So Adding a checking function in libvirt to prevent from this case. Hmm, this seems like a problematic restriction.
How does this work if we start off a guest with 3 disks attached to the usb-bot SCSI controller. Then hot-unplug the 2nd disk.
Currently, the qemu will throw out an error when I hot-unplug any of disks attached to usb-bot controller. { "execute": "device_del", "arguments": { "id": "scsi1-0-0-1"}} {"error": {"class": "GenericError", "desc": "Bus 'scsi1.0' does not support hotplugging"}} Libvirt will report an error: # virsh detach-disk rhel64qcow2 sdd --current error: Failed to detach disk error: internal error: unable to execute QEMU command 'device_del': Bus 'scsi1.0' does not support hotplugging
Next time we need will need to start with with LUNs 0 and 2 populated only, otherwise we have ABI change upon migrate or save/restore.
I think this restriction about contiguous luns must be removed if this device is to be usable with multiple luns.
Daniel
On Mo, 2013-09-02 at 13:57 +0100, Daniel P. Berrange wrote:
On Mon, Sep 02, 2013 at 05:38:42PM +0800, Guannan Ren wrote:
usb-bot only supports 16 luns(0~15) and they must be contiguous, (using lun 0 and 2 without 1 doesn't work). In this case qemu doesn't throw an error, we can not find the lun 2 in guests. So Adding a checking function in libvirt to prevent from this case.
Hmm, this seems like a problematic restriction.
It's how the hardware works.
How does this work if we start off a guest with 3 disks attached to the usb-bot SCSI controller. Then hot-unplug the 2nd disk.
You can't hotplug individual luns anyway. cheers, Gerd
On Tue, Sep 03, 2013 at 09:51:52AM +0200, Gerd Hoffmann wrote:
On Mo, 2013-09-02 at 13:57 +0100, Daniel P. Berrange wrote:
On Mon, Sep 02, 2013 at 05:38:42PM +0800, Guannan Ren wrote:
usb-bot only supports 16 luns(0~15) and they must be contiguous, (using lun 0 and 2 without 1 doesn't work). In this case qemu doesn't throw an error, we can not find the lun 2 in guests. So Adding a checking function in libvirt to prevent from this case.
Hmm, this seems like a problematic restriction.
It's how the hardware works.
How does this work if we start off a guest with 3 disks attached to the usb-bot SCSI controller. Then hot-unplug the 2nd disk.
You can't hotplug individual luns anyway.
How does hotplug/unplug work in the context of usb-bot ? AFAIK we need to be able to run device_add usb_bot drive_add file... device_add scsi-hd And the reverse, to unplug it, if we're to have feature parity with usb-storage. 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 :|
Hi,
How does this work if we start off a guest with 3 disks attached to the usb-bot SCSI controller. Then hot-unplug the 2nd disk.
You can't hotplug individual luns anyway.
How does hotplug/unplug work in the context of usb-bot ?
AFAIK we need to be able to run
device_add usb_bot drive_add file... device_add scsi-hd
And the reverse, to unplug it, if we're to have feature parity with usb-storage.
Hot-unplug is easy. You can remove the usb-bot device which will also remove all child devices. Hot-plug doesn't work at the moment, and I don't see any obvious way to fix that properly :-( We need some way to hotplug a *group* of devices (usb-bot + all children) as usb-bot itself is hotpluggable but the individual scsi devices connected to it are not. I could allow hotplug on usb-bot as workaround, then you can do stop device_add usb_bot device_add scsi-{hd,cd,whatever} cont but that would be more a gross hack than a solution ... cheers, Gerd
On 09/03/2013 08:06 AM, Gerd Hoffmann wrote:
Hi,
How does this work if we start off a guest with 3 disks attached to the usb-bot SCSI controller. Then hot-unplug the 2nd disk. You can't hotplug individual luns anyway. How does hotplug/unplug work in the context of usb-bot ?
AFAIK we need to be able to run
device_add usb_bot drive_add file... device_add scsi-hd
And the reverse, to unplug it, if we're to have feature parity with usb-storage. Hot-unplug is easy. You can remove the usb-bot device which will also remove all child devices.
Hot-plug doesn't work at the moment, and I don't see any obvious way to fix that properly :-(
We need some way to hotplug a *group* of devices (usb-bot + all children) as usb-bot itself is hotpluggable but the individual scsi devices connected to it are not.
There is another case where hot-plug/unplug of a group of devices would be useful - if you want to hotplug/unplug a pci passthrough device using vfio *and* that device is a part of an iommu group that contains other devices *and* you're okay with having those other devices assigned to the guest as well *AND* you want to use "managed='yes'" (where libvirt automatically unbinds the devices from the host driver and binds them to vfio-pci before passing them off to the guest), then you need to be able to plug and unplug all the devices in that iommu group in a single operation, otherwise the plug fails (due to attempting to assign one device in a group when the other devices haven't been bound to vfio-pci or pci-stub). As it stands now you can only hot-plug/unplug with vfio if a device is in an iommu group all by itself, or if you don't use managed='yes', and instead deal with binding the devices to vfio-pci manually. If none of that makes sense, consider yourself lucky :-)
I could allow hotplug on usb-bot as workaround, then you can do
stop device_add usb_bot device_add scsi-{hd,cd,whatever} cont
but that would be more a gross hack than a solution ...
cheers, Gerd
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
--- src/qemu/qemu_command.c | 124 +++++++++++++++++++----------------------------- 1 file changed, 48 insertions(+), 76 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f38e98f..e46baaf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4094,6 +4094,40 @@ error: return NULL; } +static int +qemuBuildSCSIDriveDevStr(int controllerModel, + virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps, + virBufferPtr opt) +{ + if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) + virBufferAddLit(opt, "scsi-cd"); + else + virBufferAddLit(opt, "scsi-hd"); + } else { + virBufferAddLit(opt, "scsi-disk"); + } + } else { + virBufferAddLit(opt, "scsi-block"); + } + + + if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) + virBufferAsprintf(opt, ",bus=scsi%d.%d,scsi-id=%d", + disk->info.addr.drive.controller, + disk->info.addr.drive.bus, + disk->info.addr.drive.unit); + else + virBufferAsprintf(opt, ",bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d", + disk->info.addr.drive.controller, + disk->info.addr.drive.bus, + disk->info.addr.drive.target, + disk->info.addr.drive.unit); + return 0; +} + char * qemuBuildDriveDevStr(virDomainDefPtr def, virDomainDiskDefPtr disk, @@ -4232,94 +4266,32 @@ qemuBuildDriveDevStr(virDomainDefPtr def, if ((qemuSetScsiControllerModel(def, qemuCaps, &controllerModel)) < 0) goto error; - if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { + if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC || + controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { if (disk->info.addr.drive.target != 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("target must be 0 for controller " "model 'lsilogic'")); goto error; } - - if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - virBufferAddLit(&opt, "scsi-block"); - } else { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) - virBufferAddLit(&opt, "scsi-cd"); - else - virBufferAddLit(&opt, "scsi-hd"); - } else { - virBufferAddLit(&opt, "scsi-disk"); - } - } - - virBufferAsprintf(&opt, ",bus=scsi%d.%d,scsi-id=%d", - disk->info.addr.drive.controller, - disk->info.addr.drive.bus, - disk->info.addr.drive.unit); - } else if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_USB_BOT) { - if (disk->info.addr.drive.target != 0) { + } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { + if (disk->info.addr.drive.target > 7) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("target must be 0 for controller " - "model 'usb-bot'")); + _("This QEMU doesn't support target " + "greater than 7")); goto error; } - if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) - virBufferAddLit(&opt, "scsi-cd"); - else - virBufferAddLit(&opt, "scsi-hd"); - } else { - virBufferAddLit(&opt, "scsi-disk"); - } - } else { - virBufferAddLit(&opt, "scsi-block"); - } - - virBufferAsprintf(&opt, ",bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d", - disk->info.addr.drive.controller, - disk->info.addr.drive.bus, - disk->info.addr.drive.target, - disk->info.addr.drive.unit); - } else { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { - if (disk->info.addr.drive.target > 7) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support target " - "greater than 7")); - goto error; - } - - if ((disk->info.addr.drive.bus != disk->info.addr.drive.unit) && - (disk->info.addr.drive.bus != 0)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU only supports both bus and " - "unit equal to 0")); - goto error; - } - } - - if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD)) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) - virBufferAddLit(&opt, "scsi-cd"); - else - virBufferAddLit(&opt, "scsi-hd"); - } else { - virBufferAddLit(&opt, "scsi-disk"); - } - } else { - virBufferAddLit(&opt, "scsi-block"); + if ((disk->info.addr.drive.bus != disk->info.addr.drive.unit) && + (disk->info.addr.drive.bus != 0)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU only supports both bus and " + "unit equal to 0")); + goto error; } - - virBufferAsprintf(&opt, ",bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d", - disk->info.addr.drive.controller, - disk->info.addr.drive.bus, - disk->info.addr.drive.target, - disk->info.addr.drive.unit); } + + qemuBuildSCSIDriveDevStr(controllerModel, disk, qemuCaps, &opt); break; case VIR_DOMAIN_DISK_BUS_SATA: if (disk->info.addr.drive.bus != 0) { -- 1.8.3.1
--- .../qemuxml2argv-disk-scsi-usbbot.args | 10 +++++++ .../qemuxml2argv-disk-scsi-usbbot.xml | 33 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 46 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args new file mode 100644 index 0000000..09e6d57 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.args @@ -0,0 +1,10 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device usb-bot,id=scsi0 \ +-usb -drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-scsi0-0-0-0 \ +-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ +-drive file=/tmp/scsidisk.img,if=none,id=drive-scsi0-0-0-1 \ +-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=1,\ +drive=drive-scsi0-0-0-1,id=scsi0-0-0-1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml new file mode 100644 index 0000000..4be405f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-usbbot.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='file' device='disk'> + <source file='/tmp/scsidisk.img'/> + <target dev='sdb' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='scsi' index='0' model='usb-bot'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4e3508b..29de992 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -548,6 +548,9 @@ mymain(void) DO_TEST("disk-scsi-device", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_LSI); + DO_TEST("disk-scsi-usbbot", + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_USB_BOT); DO_TEST("disk-scsi-device-auto", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_LSI); -- 1.8.3.1
On Mon, Sep 02, 2013 at 05:38:39PM +0800, Guannan Ren wrote:
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=917702 qemu patch:http://lists.gnu.org/archive/html/qemu-devel/2013-01/msg02200.html
These patch aims to add usb-bot SCSI controller support for libvirt. As usb-storage libvirt already supported, usb-bot only supports one SCSI target with SCSI ID 0. The difference is that usb-storage creates SCSI HBA and additionaly either a scsi-disk or a scsi-generic device, usb-bot only creates the SCSI HBA. we can attach a SCSI device to it with another -device.
usb-bot supports a single target and a number of luns. The limit is 15 luns (0~15 LUN ID) and they must be contiguous(using lun 0 and 2 without 1 doesn't work).
Athought usb-bot is a SCSI controller it needs to be attached to a exsiting USB controller for work. So it has to use address of usb type.
Libvirt XML:
<devices> ... <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/disk.qcow2'/> <target dev='sdc' bus='scsi'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'/> <controller type='scsi' index='0' model='usb-bot'> <address type='usb' bus='0' port='1'/> </controller> ... </devices>
How does this work from a hotplug POV. With usb-storage you could just hotplug the <disk> device. Now it seems we need two separate hotplug calls one of the <controller> and one for the <disk> and the reverse.
The QEMU commandline:
qemu ${other_vm_args} \ -device piix3-usb-uhci,id=usb \ -device usb-bot,id=scsi0,bus=usb.0,port=1 \ -drive file=/var/lib/libvirt/images/disk.qcow2,if=none,id=drive-scsi0-0-0-0,format=raw \ -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0
As the usb-storage creates scsi disk automatically which doesn't let you set scsi-disk properties such as vendor, product, wwn, channel, scsi-id and lun. So QEMU guys prefer usb-bot to usb-storage. So this is the first part of the whole work. Next step will replace usb-storage with usb-bot when disk in xml uses usb bus like <disk ...> <...> <target bus='usb'> </disk>
I'm not really a fan of introducing 2 different ways to configure the exact same device. 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 :|
----- Original Message ----- From: "Daniel P. Berrange" <berrange@redhat.com> To: "Guannan Ren" <gren@redhat.com> Cc: libvir-list@redhat.com Sent: Tuesday, September 3, 2013 5:38:08 AM Subject: Re: [libvirt] [PATCH 0/5]qemu: add usb-bot scsi controller support On Mon, Sep 02, 2013 at 05:38:39PM +0800, Guannan Ren wrote:
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=917702 qemu patch:http://lists.gnu.org/archive/html/qemu-devel/2013-01/msg02200.html
These patch aims to add usb-bot SCSI controller support for libvirt. As usb-storage libvirt already supported, usb-bot only supports one SCSI target with SCSI ID 0. The difference is that usb-storage creates SCSI HBA and additionaly either a scsi-disk or a scsi-generic device, usb-bot only creates the SCSI HBA. we can attach a SCSI device to it with another -device.
usb-bot supports a single target and a number of luns. The limit is 15 luns (0~15 LUN ID) and they must be contiguous(using lun 0 and 2 without 1 doesn't work).
Athought usb-bot is a SCSI controller it needs to be attached to a exsiting USB controller for work. So it has to use address of usb type.
Libvirt XML:
<devices> ... <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/disk.qcow2'/> <target dev='sdc' bus='scsi'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'/> <controller type='scsi' index='0' model='usb-bot'> <address type='usb' bus='0' port='1'/> </controller> ... </devices>
How does this work from a hotplug POV. With usb-storage you could just hotplug the <disk> device. Now it seems we need two separate hotplug calls one of the <controller> and one for the <disk> and the reverse.
Yes, I will think about the hotplug.
The QEMU commandline:
qemu ${other_vm_args} \ -device piix3-usb-uhci,id=usb \ -device usb-bot,id=scsi0,bus=usb.0,port=1 \ -drive file=/var/lib/libvirt/images/disk.qcow2,if=none,id=drive-scsi0-0-0-0,format=raw \ -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0
As the usb-storage creates scsi disk automatically which doesn't let you set scsi-disk properties such as vendor, product, wwn, channel, scsi-id and lun. So QEMU guys prefer usb-bot to usb-storage. So this is the first part of the whole work. Next step will replace usb-storage with usb-bot when disk in xml uses usb bus like <disk ...> <...> <target bus='usb'> </disk>
I'm not really a fan of introducing 2 different ways to configure the exact same device.
The idea is to use usb-bot instead of usb-storage in this case if usb-bot capability is available. usb-storage automatically expands into two devices, a SCSI controller and a SCSI device. And user cannot set any property values to this SCSI device such as vendor, product, wwn, channel, scsi-id and lun. usb-bot gives the more flexible configurations. I think it is to use usb-bot if usb-bot is supported, otherwise still use usb-storage. The xml will not change at all. Guannan
participants (5)
-
Daniel P. Berrange -
Gerd Hoffmann -
Guan Nan Ren -
Guannan Ren
-
Laine Stump