[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH RFC 2/5] conf: Introduce scsi hostdev



On 2013年03月06日 21:09, Han Cheng wrote:
> On 03/06/2013 02:24 PM, Osier Yang wrote:
>> On 2013年03月04日 14:01, Han Cheng wrote:
>>> Adding scsi hostdev, it should like:
>>>
>>>        <hostdev mode='subsystem' type='scsi'>
>>>          <source>
>>>            <adapter name='scsi_host0'/>
>>>            <address bus='0' target='0' unit='0'/>
>>>          </source>
>>>          <address type='drive' controller='0' bus='0' target='4' unit='8'/>
>>>        </hostdev>
>>> @@ -3893,4 +3921,9 @@
>>>         </element>
>>>         <empty/>
>>>       </define>
>>> +<define name="scsiAdapter">
>>> +<data type="string">
>>> +<param name="pattern">scsi_host[0-9]{1,2}</param>
>>
>> No need to have a duplicate definition. It can reuse what
>> storage pool uses.

I mean you can reuse the schema definition of storage pool.

> This is possible.
> But what make differences is the number. If we don't deal with it and
> storage it as string, we'll have to deal with it when build command line.

Please do it when building command line. The XML should be generic for
all drivers, not specificly for one driver.

> 
> Or, we just change the xml:
>        <source>
>          <adapter name='scsi_host0'/>
>          <address bus='0' target='0' unit='0'/>
>        </source>
> -->
>        <source>
>          <address host='0' bus='0' target='0' unit='0'/>
>        </source>
> 
> And actually 'host' can be 'controller'. Then it is drive address. We
> may reduce redundant codes.

I'm not a fan of this idea. As we will want the adapter name like
"scsi_host0" anyway when associating the scsi storage pool, especially
the scsi pool behind a vHBA, which is represented by wwnn/wwpn, which
is not possible to represented by a number. And on the other hand,
reusing controller makes things confused.

> 
>>> @@ -12997,6 +13119,15 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
>>>         virBufferAdjustIndent(buf, 2);
>>>         switch (def->source.subsys.type)
>>>         {
>>> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
>>> +        virBufferAsprintf(buf, "<adapter name='scsi_host%d'/>\n",
>>
>> This is hard code. Assuming that a scsi host device can have different
>> name with "scsi_host" on platform other than Linux. So again, IMHO
>> we should just storing the adapter name as string.
>>
> Actually I'm quite uncomfortable when writing this hard code. If we
> change xml, this won't be problem.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]