[Stgt-devel] [PATCH] tgt-admin: support persistent device binding

[Tomasz Chmielewski]

FUJITA Tomonori schrieb:
> On Tue, 29 Jul 2008 13:21:45 +0200
> Tomasz Chmielewski <mangoo at wpkg.org> wrote:
> 
>> FUJITA Tomonori schrieb:
>>> On Tue, 29 Jul 2008 13:51:55 +0300
>>> Doron Shoham <dorons at Voltaire.COM> wrote:
>>>
>>>> Eli Dorfman wrote:
>>>>>>> maybe use the following format :
>>>>>>> <target xyz>
>>>>>>>     <backing-store /dev/sdb>
>>>>>>>           properties  custom # use properties specified below or
>>>>>>> default (generate the deadbeaf id) or direct (get real serial number)
>>>>>>>           serial-number 12345678
>>>>>>>           vendor myvendor
>>>>>>>     </backing-store>
>>>>>>> </target>
>>>>>>>
>>>>>>> what do you think?
>>>>>> Hmm, 'properties custom' looks unnecessary for me. If a device file of
>>>>>> a disk drive is used as backing-store and none of the properties are
>>>>>> specified, we can use the physical properties.
>>>>> we need something like properties to differentiate between disk (read
>>>>> real props), file (generate unique serial number
>>>>> disk-sn-partition-filename) and custom (applicable to any case).
>>>>> we can call this attribute: "type" which can accept 3 values disk,
>>>>> file or custom (default).
>>>> This is what I suggest:
>>>>
>>>> <target iqn.2007-04.com.example:san.monitoring>
>>>>    <backing-store /tmp/ramdisk/lun1>
>>>>       type default
>>>>       serial 12345678
>>>>       vendor my_vendor
>>>>       product_id my_id
>>>>       product_rev my_rev
>>>>    </backing-store>
>>>>
>>>>    <backing-store /dev/sdc>
>>>>       type disk
>>>>    </backing-store>
>>>> </target>
>>>>
>>>> The reason we use XML is that the original implementation used XML.
>>>>
>>>> We suggest two types:
>>>> 1. disk: "real" physical devices, where we read the properties from the device itself - i.e. storage disks.
>>>> 2. default - use default tgt values. 
>>>>    We can also override the properties with user defined values (optional).
>>>>
>>>> The reason for the type attribute is to differentiate between these two types.
>>> I don't know anything about XML but should it be something like?
>>>
>>> <target iqn.2007-04.com.example:san.monitoring>
>>>    <lun 0>
>>>      type sbc
>>>      backing-store
>> What we use currently in tgt-admin is a mix of XML and a flat config file, I would say.
>> With the benefits of both (easy hand editing; it is clear where one target definition starts and where it ends).
>>
>>
>> If it was "proper XML", it should look like:
>>
>> <target iqn.2007-04.com.example:san.monitoring>
>> 	<lun 0>
>> 		<type sbc />
>> 		<backing-store />
>> 	</lun>
>> </target>
>>
>> Which is not that nice for hand editing anymore - forget one slash
>> in one place (i.e., after "type sbc"), and your whole config file is
>> invalid.
> 
> But it would be better to let administrators not to edit a
> configuration file by hand?

Why not? I prefer hand editing than "wizards" etc. (unless I have really lots of things to change; in that case, I would probably run some awk/perl script anyway).


> And I guess that using a pure XML enables
> us to use perl XML libraries? I thought that that's the reason why
> people want to use XML.

I chose the apache-style format we have now because:
- it was easy for hand or scripted editing,
- perl provides a parser for it -> no need to write an own parser
- everyone agreed to it ;)

> IMHO, it's much easier to edit a plain text file rather than a XML
> file.

The problem is the definition of a plain text file, and what happens when we have "nested" things with different values (i.e., different luns should have different parameters), detecting simple typos (see what will happen in "other types" below - target vs tagret - some other target will probably get luns which don't belong to it) in the text file etc.


- apache-style plain text configuration - this is what we use now

include /some/other/*.conf

<target iqn.2007-02.com.example:blah.blah>
backing-store /dev/blah/blah
</target>


- INI files (used i.e. by Samba)

[global]
include = /some/other/*.conf

[iqn.2007-02.com.example:blah.blah]
backing-store = /dev/blah/blah



- other types

target iqn.2007-02.com.example:blah.blah
backing-store /dev/blah/blah

tagret iqn.2008-11.com.example:blah2.blah2
backing-store /dev/some/other


-- 
Tomasz Chmielewski 
http://wpkg.org