On 31/05/13 22:14, Osier Yang wrote:
On 31/05/13 22:09, Eric Blake wrote:
> On 05/31/2013 04:09 AM, 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 not used
>> by any disk or scsi host device yet), 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.
> ACK.
>
>> src/conf/domain_conf.c | 202
>> ++++++++++++++++++---
>> .../qemuxml2argv-hostdev-scsi-autogen-address.xml | 95 ++++++++++
>> ...qemuxml2xmlout-hostdev-scsi-autogen-address.xml | 106 +++++++++++
>> tests/qemuxml2xmltest.c | 2 +
>> 4 files changed, 375 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
>>
> Relatively big to be pushing this late after freeze, but it can be
> argued that this is a bug fix and worth including in 1.0.6.
>
I hope it could be included in 1.0.6. The scsi host device can work
without
it, but the address should be specified manually, which is not quite
good,
and better to have them together in one release.
Pushed as-is, John is a bit not fine with the comment, but it's trivial, and
I do think it's not the time to change it to produce an inconsistent comment
in virDomainDiskDefAssignAddress. As I said, I have to ensure the 7 is
used for the controller itself indeed for narrow scsi bus, and it should be
a independant patch, as it's for different purpose.
Thanks.
Osier