[libvirt] [PATCH v2] conf: Generate address for scsi host device automatically
Osier Yang
jyang at redhat.com
Fri May 31 10:05:31 UTC 2013
On 28/05/13 23:59, Osier Yang wrote:
> On 28/05/13 23:09, John Ferlan wrote:
>> On 05/23/2013 12:48 PM, Osier Yang wrote:
>>> With unknown good reasons, the attribute "bus" of scsi device
>>> address is always set to 0, same for attribute "target". (See
>>> virDomainDiskDefAssignAddress).
>>>
>>> Though we might need to change the algorithm to honor "bus"
>>> and "target" too, that's a different issue. The address generator
>>>
>>> for scsi host device in this patch just follows the unknown
>>> good reasons, only considering the "controller" and "unit".
>>> It walks through all scsi controllers and their units, to see
>>> if the address $controller:0:0:$unit can be used, if found
>>> one, it sits on it, otherwise, it creates a new controller
>>> (actually the controller is implicitly created by someone
>>> else), and sits on $new_controller:0:0:0 instead.
>>>
>>> ---
>>> v1 - v2:
>>> * A new helper virDomainSCSIDriveAddressIsUsed
>>> * Move virDomainDefMaybeAddHostdevSCSIcontroller
>>> * More comments
>>> * Add testing.
>>> * problems fixed with the testing:
>>> 1) s/nscsi_controllers + 1/nscsi_controllers/,
>>> 2) Add the controller implicitly after a scsi hostdev def
>>> parsing, as it can use a new scsi controller index
>>> (nscsi_controllers), which should be added implicitly.
>> First pass of code is found at:
>>
>> https://www.redhat.com/archives/libvir-list/2013-May/msg00340.html
>>
>> The previous/initial review and reply is found at:
>>
>> https://www.redhat.com/archives/libvir-list/2013-May/msg00506.html
>>
>> https://www.redhat.com/archives/libvir-list/2013-May/msg00526.html
>>
>>> ---
>>> src/conf/domain_conf.c | 209
>>> ++++++++++++++++++---
>>> .../qemuxml2argv-hostdev-scsi-autogen-address.xml | 95 ++++++++++
>>> ...qemuxml2xmlout-hostdev-scsi-autogen-address.xml | 106 +++++++++++
>>> tests/qemuxml2xmltest.c | 2 +
>>> 4 files changed, 382 insertions(+), 30 deletions(-)
>>> create mode 100644
>>> tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-autogen-address.xml
>>> create mode 100644
>>> tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-autogen-address.xml
>>>
>>>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index ad5550c..86d743b 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -3782,6 +3782,148 @@ cleanup:
>>> return ret;
>>> }
>>> +/* Check if a drive type address $controller:0:0:$unit is already
>>> + * taken by a disk or not.
>>> + */
>>> +static bool
>>> +virDomainDriveAddressIsUsedByDisk(virDomainDefPtr def,
>>> + enum virDomainDiskBus type,
>>> + unsigned int controller,
>>> + unsigned int unit)
>>> +{
>>> + virDomainDiskDefPtr disk;
>>> + int i;
>>> +
>>> + for (i = 0; i < def->ndisks; i++) {
>>> + disk = def->disks[i];
>>> +
>>> + if (disk->bus != type ||
>>> + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
>>> + continue;
>>> +
>>> + if (disk->info.addr.drive.controller == controller &&
>>> + disk->info.addr.drive.unit == unit &&
>>> + disk->info.addr.drive.bus == 0 &&
>>> + disk->info.addr.drive.target == 0)
>>> + return true;
>>> + }
>>> +
>>> + return false;
>>> +}
>>> +
>>> +/* Check if a drive type address $controller:0:0:$unit is already
>>> + * taken by a host device or not.
>>> + */
>>> +static bool
>>> +virDomainDriveAddressIsUsedByHostdev(virDomainDefPtr def,
>>> + enum
>>> virDomainHostdevSubsysType type,
>>> + unsigned int controller,
>>> + unsigned int unit)
>>> +{
>>> + virDomainHostdevDefPtr hostdev;
>>> + int i;
>>> +
>>> + for (i = 0; i < def->nhostdevs; i++) {
>>> + hostdev = def->hostdevs[i];
>>> +
>>> + if (hostdev->source.subsys.type != type)
>>> + continue;
>>> +
>>> + if (hostdev->info->addr.drive.controller == controller &&
>>> + hostdev->info->addr.drive.unit == unit &&
>>> + hostdev->info->addr.drive.bus == 0 &&
>>> + hostdev->info->addr.drive.target == 0)
>>> + return true;
>>> + }
>>> +
>>> + return false;
>>> +}
>>> +
>>> +static bool
>>> +virDomainSCSIDriveAddressIsUsed(virDomainDefPtr def,
>>> + unsigned int controller,
>>> + unsigned int unit)
>>> +{
>>> + /* In current implementation, the maximum unit number of a
>>> controller
>>> + * is either 16 or 7 (narrow SCSI bus), and if the maximum unit
>>> number
>>> + * is 16, the controller itself is on unit 7 */
>>> + if (unit == 7)
>>
>> FYI: The previous code cared that a wide address was being used when
>> returning true - this code doesn't. Just making sure that change is
>> intentional. Essentially, 7 cannot be used. The comments above the
>> code talk about wide/narrow which it seems becomes irrelevant. The
>> reality seems to be that you are avoiding using 7 altogether because
>> it's reserved for the controller itself, correct? Regardless of whether
>> a wide or narrow. The controller itself uses the unit number of the
>> maximum value of the narrow band. At least that's how I'm reading it.
>
> You are right, will update the comment.
Actually no, I was confused. For the narrow scsi bus, the unit can't be 7,
so I just shortened the statement by removing the checking for max_unit
== 16
I don't know why the units on the narrow scsi bus is 7 though, it should
be 8
instead. See:
http://en.wikipedia.org/wiki/SCSI
>
>>
>>> + return true;
>>> +
>>> + if (virDomainDriveAddressIsUsedByDisk(def,
>>> VIR_DOMAIN_DISK_BUS_SCSI,
>>> + controller, unit) ||
>>> + virDomainDriveAddressIsUsedByHostdev(def,
>>> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI,
>>> + controller, unit))
>>> +}
>>> +
>>> +/* Find out the next usable "unit" of a specific controller */
>>> +static int
>>> +virDomainControllerSCSINextUnit(virDomainDefPtr def,
>>> + unsigned int max_unit,
>>> + unsigned int controller)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < max_unit; i++) {
>>> + if (!virDomainSCSIDriveAddressIsUsed(def, controller, i))
>>> + return i;
>>> + }
>>> +
>>> + return -1;
>>> +}
>>> +
>>> +#define SCSI_WIDE_BUS_MAX_CONT_UNIT 16
>>> +#define SCSI_NARROW_BUS_MAX_CONT_UNIT 7
>>> +
>>> +static int
>>> +virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt,
>>> + virDomainDefPtr def,
>>> + virDomainHostdevDefPtr hostdev)
>>> +{
>>> + int next_unit;
>>> + unsigned nscsi_controllers = 0;
>>> + bool found = false;
>>> + int i;
>>> +
>>> + if (hostdev->source.subsys.type !=
>>> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)
>>> + return -1;
>>> +
>>> + for (i = 0; i < def->ncontrollers; i++) {
>> As a for loop exit condition, consider either using
>>
>> && !found
>> or
>>
>> && next_unit == -1
>>
>> as long as you initialize next_unit = -1 above...
>>
>>> + if (def->controllers[i]->type !=
>>> VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
>>> + continue;
>>> +
>>> + nscsi_controllers++;
>>> + next_unit = virDomainControllerSCSINextUnit(def,
>>> + xmlopt->config.hasWideScsiBus ?
>>> + SCSI_WIDE_BUS_MAX_CONT_UNIT :
>>> + SCSI_NARROW_BUS_MAX_CONT_UNIT,
>>> + def->controllers[i]->idx);
>>> + if (next_unit >= 0) {
>>> + found = true;
>>> + break;
>> break; is irrelevant using !found in loop exit or the whole statement is
>> irrelevent if using next_unit == -1 for loop exit...
>>
>>> + }
>>> + }
>>> +
>>> + hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
>>> +
>>> + if (found) {
>>> + hostdev->info->addr.drive.controller =
>>> def->controllers[i]->idx;
>>> + hostdev->info->addr.drive.bus = 0;
>>> + hostdev->info->addr.drive.target = 0;
>>> + hostdev->info->addr.drive.unit = next_unit;
>>> + } else {
>>> + hostdev->info->addr.drive.controller = nscsi_controllers;
>>> + hostdev->info->addr.drive.bus = 0;
>>> + hostdev->info->addr.drive.target = 0;
>>> + hostdev->info->addr.drive.unit = 0;
>>> + }
>>
>> Although it looked odd initially, I think I convinced myself the logic
>> is OK. Initially, nscsi_controllers = 0;
>>
>> If none exist, then 'controller = 0' and 'units' are assigned until
>> full, right?
>
> Right.
>
>> If one exists and is full, then 'controller = 1' and is
>> placed at unit = 0.
>
> Right.
>
>>
>> NOTE:
>> Using next_unit to break the loop means this code could be rewritten as:
>>
>> hostdev->info->addr.drive.controller = (next_unit >= 0 ?
>> def->controllers[i]->idx :
>> nscsi_controllers)
>> hostdev->info->addr.drive.bus = 0;
>> hostdev->info->addr.drive.target = 0;
>> hostdev->info->addr.drive.unit = (next_unit >= 0 ? next_unit : 0);
>>
>> Although one could argue setting "= 0;" is redundant due to allocation.
>
> Compact. I will consider your suggestion for the loop in v3.
>
>>
>>
>>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static int
>>> virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
>>> xmlXPathContextPtr ctxt,
>>> @@ -8802,7 +8944,9 @@ error:
>>> }
>>> static virDomainHostdevDefPtr
>>> -virDomainHostdevDefParseXML(const xmlNodePtr node,
>>> +virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt,
>>> + virDomainDefPtr vmdef,
>>> + const xmlNodePtr node,
>>> xmlXPathContextPtr ctxt,
>>> virHashTablePtr bootHash,
>>> unsigned int flags)
>>> @@ -8862,7 +9006,9 @@ virDomainHostdevDefParseXML(const xmlNodePtr
>>> node,
>>> }
>>> break;
>>> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
>>> - if (def->info->type ==
>>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
>>> + if (def->info->type ==
>>> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
>>> + virDomainHostdevAssignAddress(xmlopt, vmdef, def) <
>>> 0) {
>>> +
>>> virReportError(VIR_ERR_XML_ERROR, "%s",
>>> _("SCSI host devices must have
>>> address specified"));
>>> goto error;
>>> @@ -9271,8 +9417,8 @@ virDomainDeviceDefParse(const char *xmlStr,
>>> goto error;
>>> } else if (xmlStrEqual(node->name, BAD_CAST "hostdev")) {
>>> dev->type = VIR_DOMAIN_DEVICE_HOSTDEV;
>>> - if (!(dev->data.hostdev = virDomainHostdevDefParseXML(node,
>>> ctxt, NULL,
>>> - flags)))
>>> + if (!(dev->data.hostdev =
>>> virDomainHostdevDefParseXML(xmlopt, def, node,
>>> + ctxt, NULL, flags)))
>>> goto error;
>>> } else if (xmlStrEqual(node->name, BAD_CAST "controller")) {
>>> dev->type = VIR_DOMAIN_DEVICE_CONTROLLER;
>>> @@ -10255,6 +10401,30 @@ error:
>>> return NULL;
>>> }
>>> +static int
>>> +virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def)
>>> +{
>>> + /* Look for any hostdev scsi dev */
>>> + int i;
>>> + int maxController = -1;
>>> + virDomainHostdevDefPtr hostdev;
>>> +
>>> + for (i = 0; i < def->nhostdevs; i++) {
>>> + hostdev = def->hostdevs[i];
>>> + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
>>> + hostdev->source.subsys.type ==
>>> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
>>> + (int)hostdev->info->addr.drive.controller >
>>> maxController) {
>>> + maxController = hostdev->info->addr.drive.controller;
>>> + }
>>> + }
>>> +
>>> + for (i = 0; i <= maxController; i++) {
>>> + if (virDomainDefMaybeAddController(def,
>>> VIR_DOMAIN_CONTROLLER_TYPE_SCSI, i, -1) < 0)
>>> + return -1;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> static virDomainDefPtr
>>> virDomainDefParseXML(xmlDocPtr xml,
>>> @@ -11618,7 +11788,8 @@ virDomainDefParseXML(xmlDocPtr xml,
>>> for (i = 0; i < n; i++) {
>>> virDomainHostdevDefPtr hostdev;
>>> - hostdev = virDomainHostdevDefParseXML(nodes[i], ctxt,
>>> bootHash, flags);
>>> + hostdev = virDomainHostdevDefParseXML(xmlopt, def, nodes[i],
>>> + ctxt, bootHash, flags);
>>> if (!hostdev)
>>> goto error;
>>> @@ -11631,6 +11802,9 @@ virDomainDefParseXML(xmlDocPtr xml,
>>> }
>>> def->hostdevs[def->nhostdevs++] = hostdev;
>>> +
>>> + if (virDomainDefMaybeAddHostdevSCSIcontroller(def) < 0)
>>> + goto error;
>>> }
>>> VIR_FREE(nodes);
>>> @@ -13232,31 +13406,6 @@
>>> virDomainDefMaybeAddSmartcardController(virDomainDefPtr def)
>>> return 0;
>>> }
>>> -static int
>>> -virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def)
>>> -{
>>> - /* Look for any hostdev scsi dev */
>>> - int i;
>>> - int maxController = -1;
>>> - virDomainHostdevDefPtr hostdev;
>>> -
>>> - for (i = 0; i < def->nhostdevs; i++) {
>>> - hostdev = def->hostdevs[i];
>>> - if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
>>> - hostdev->source.subsys.type ==
>>> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
>>> - (int)hostdev->info->addr.drive.controller >
>>> maxController) {
>>> - maxController = hostdev->info->addr.drive.controller;
>>> - }
>>> - }
>>> -
>>> - for (i = 0; i <= maxController; i++) {
>>> - if (virDomainDefMaybeAddController(def,
>>> VIR_DOMAIN_CONTROLLER_TYPE_SCSI, 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
>>> diff --git
>>> a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-autogen-address.xml
>>> b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-autogen-address.xml
>>> new file mode 100644
>>> index 0000000..21f112b
>>> --- /dev/null
>>> +++
>>> b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-autogen-address.xml
>>> @@ -0,0 +1,95 @@
>>> +<domain type='qemu'>
>>> + <name>QEMUGuest2</name>
>>> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
>>> + <memory unit='KiB'>219100</memory>
>>> + <currentMemory unit='KiB'>219100</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/QEMUGuest2'/>
>>> + <target dev='hda' bus='ide'/>
>>> + <address type='drive' controller='0' bus='0' target='0'
>>> unit='0'/>
>>> + </disk>
>>> + <controller type='scsi' index='0' model='virtio-scsi'/>
>>> + <controller type='usb' index='0'/>
>>> + <controller type='ide' index='0'/>
>>> + <controller type='pci' index='0' model='pci-root'/>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host0'/>
>>> + <address bus='0' target='0' unit='0'/>
>>> + </source>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host1'/>
>>> + <address bus='0' target='0' unit='1'/>
>>> + </source>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host2'/>
>>> + <address bus='0' target='0' unit='2'/>
>>> + </source>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host3'/>
>>> + <address bus='0' target='0' unit='3'/>
>>> + </source>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host4'/>
>>> + <address bus='0' target='0' unit='4'/>
>>> + </source>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host5'/>
>>> + <address bus='0' target='0' unit='5'/>
>>> + </source>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host6'/>
>>> + <address bus='0' target='0' unit='6'/>
>>> + </source>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host7'/>
>>> + <address bus='0' target='0' unit='7'/>
>>> + </source>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host8'/>
>>> + <address bus='0' target='0' unit='8'/>
>>> + </source>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host9'/>
>>> + <address bus='0' target='0' unit='9'/>
>>> + </source>
>>> + <address type='drive' controller='1' bus='0' target='0'
>>> unit='5'/>
>> [1] see below
>>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host10'/>
>>> + <address bus='0' target='0' unit='10'/>
>>> + </source>
>>> + </hostdev>
>>> + <memballoon model='virtio'/>
>>> + </devices>
>>> +</domain>
>>> diff --git
>>> a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-autogen-address.xml
>>> b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-autogen-address.xml
>>>
>>> new file mode 100644
>>> index 0000000..e416654
>>> --- /dev/null
>>> +++
>>> b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-autogen-address.xml
>>> @@ -0,0 +1,106 @@
>>> +<domain type='qemu'>
>>> + <name>QEMUGuest2</name>
>>> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
>>> + <memory unit='KiB'>219100</memory>
>>> + <currentMemory unit='KiB'>219100</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/QEMUGuest2'/>
>>> + <target dev='hda' bus='ide'/>
>>> + <address type='drive' controller='0' bus='0' target='0'
>>> unit='0'/>
>>> + </disk>
>>> + <controller type='scsi' index='0' model='virtio-scsi'/>
>>> + <controller type='usb' index='0'/>
>>> + <controller type='ide' index='0'/>
>>> + <controller type='pci' index='0' model='pci-root'/>
>>> + <controller type='scsi' index='1'/>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host0'/>
>>> + <address bus='0' target='0' unit='0'/>
>>> + </source>
>>> + <address type='drive' controller='0' bus='0' target='0'
>>> unit='0'/>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host1'/>
>>> + <address bus='0' target='0' unit='1'/>
>>> + </source>
>>> + <address type='drive' controller='0' bus='0' target='0'
>>> unit='1'/>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host2'/>
>>> + <address bus='0' target='0' unit='2'/>
>>> + </source>
>>> + <address type='drive' controller='0' bus='0' target='0'
>>> unit='2'/>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host3'/>
>>> + <address bus='0' target='0' unit='3'/>
>>> + </source>
>>> + <address type='drive' controller='0' bus='0' target='0'
>>> unit='3'/>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host4'/>
>>> + <address bus='0' target='0' unit='4'/>
>>> + </source>
>>> + <address type='drive' controller='0' bus='0' target='0'
>>> unit='4'/>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host5'/>
>>> + <address bus='0' target='0' unit='5'/>
>>> + </source>
>>> + <address type='drive' controller='0' bus='0' target='0'
>>> unit='5'/>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host6'/>
>>> + <address bus='0' target='0' unit='6'/>
>>> + </source>
>>> + <address type='drive' controller='0' bus='0' target='0'
>>> unit='6'/>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host7'/>
>>> + <address bus='0' target='0' unit='7'/>
>>> + </source>
>>> + <address type='drive' controller='1' bus='0' target='0'
>>> unit='0'/>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host8'/>
>>> + <address bus='0' target='0' unit='8'/>
>>> + </source>
>>> + <address type='drive' controller='1' bus='0' target='0'
>>> unit='1'/>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host9'/>
>>> + <address bus='0' target='0' unit='9'/>
>>> + </source>
>>> + <address type='drive' controller='1' bus='0' target='0'
>>> unit='5'/>
>>> + </hostdev>
>>> + <hostdev mode='subsystem' type='scsi' managed='yes'>
>>> + <source>
>>> + <adapter name='scsi_host10'/>
>>> + <address bus='0' target='0' unit='10'/>
>>> + </source>
>>> + <address type='drive' controller='1' bus='0' target='0'
>>> unit='2'/>
>>> + </hostdev>
>>> + <memballoon model='virtio'/>
>>> + </devices>
>>> +</domain>
>>> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
>>> index 64a271c..06e4206 100644
>>> --- a/tests/qemuxml2xmltest.c
>>> +++ b/tests/qemuxml2xmltest.c
>>> @@ -298,6 +298,8 @@ mymain(void)
>>> DO_TEST("hostdev-scsi-shareable");
>>> DO_TEST("hostdev-scsi-sgio");
>>> + DO_TEST_DIFFERENT("hostdev-scsi-autogen-address");
>>> +
>> [1]
>> Still don't understand the test setup 100%, but I think for the purposes
>> of what you're trying to prove that the defined unit=5 above should be 1
>> or 2 and then ensure that '3' gets created to prove the algorithm will
>> find one at '1' or '2'. As it stands all that seems to be provided is
>> the scsi10 will use '3' even though '5' exists.
>
> Yes, I want to prove it will allocate the smallest available unit number,
> instead of increasing on top of the current largest unit number.
>
> /me should document this in the commit log.
>
>>
>> Curious to understand why the 2nd controller gets created without the
>> "model='virtio-scsi'"? Does it matter? Does creating a controller on
>> the fly like this would seem to imply that the controllers would be
>> alike.
>
> It's a problem actually, I intended to write a RFC for it, but not posted
> yet. Assuming that one wants to hotplug a virtio-scsi disk with XML:
>
> <disk type='file' device='disk'>
> <driver name='qemu' type='raw'/>
> <source file='/var/lib/libvirt/images/f18_sdb.img'/>
> <target dev='sda' bus='scsi'/>
> <address type='drive' controller='1' bus='0' target='0' unit='0'/>
> </disk>
>
> If there is no scsi controller with index "1", the implicitly added
> scsi controller
> will use the default scsi controller model "lsilogic" (true for
> qemu/kvm). That's
> obviously not what the user wants.
>
> Though we provide the way to hotplug a controller, one can hotplug a
> controller first, and then hotplug the disk. But it's just a bit
> better, as it still
> needs some manual work.
>
> There are many other problems, I'm going to write them out when posting
> the RFC to solve the problem (I don't have a good thought yet though).
>
>>
>> Also what is the delineation between a narrow vs. wide bus in the xml?
>> It seems you'd want to have a test that ensures that a wide scsi bus
>> skips unit=7 when adding 10 devices, right?
>>
> I wanted to add test for the wide scsi bus too, but the test qemu only
> supports
> narrow scsi bus, I didn't dig it more, as I thought it may not be
> deserved to
> do. The 10 devices in this test is to prove a new controller can be
> added, and the
> address can be allocated as expected.
QEMU driver never supported wide SCSI bus yet. The "wide scsi bus" support
was added by Matthias for ESX driver. See:
http://www.redhat.com/archives/libvir-list/2010-June/msg00452.html
I do think qemu driver should support too, but I have to figure out how to
detect it from qemu first, and will be a later patch...
Will post v3 with your suggestion on the for loop. Thanks.
Osier
More information about the libvir-list
mailing list