[libvirt] [PATCH] Add support for Vendor and Model in Storage Pool XML

[Patrick Dignan]

  On 08/06/2010 03:16 AM, Daniel P. Berrange wrote:
> On Thu, Aug 05, 2010 at 11:51:48PM +0200, Matthias Bolte wrote:
>> 2010/8/5 Patrick Dignan<pat_dignan at dell.com>:
>>>   On 08/05/2010 07:24 AM, Daniel P. Berrange wrote:
>>>> On Mon, Aug 02, 2010 at 12:44:37PM -0500, Patrick Dignan wrote:
>>>>>   Hi all,
>>>>>
>>>>> I wrote a patch to add support for listing the Vendor and Model of a
>>>>> storage pool in the storage pool XML.  This would allow vendor
>>>>> extensions of specific devices.  The patch includes a test for the new
>>>>> attributes as well.  I'd appreciate some feedback on it, and would like
>>>>> get it merged when it's ready.  Patch added at the bottom of the message.
>>>>> libvirt-0.8.2.ml/tests/storagepoolxml2xmlin/pool-iscsi-vendor-model.xml
>>>>> 2010-08-02 12:00:37.015538583 -0500
>>>>> @@ -0,0 +1,17 @@
>>>>> +<pool type='iscsi' vendor='test-vendor' model='test-model'>
>>>>> +<name>virtimages</name>
>>>>> +<uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
>>>> We should avoid adding attributes to the top level<pool>    tag.
>>>> The vendor/model is a piece of metadata associated with the
>>>> pool source, so it should be under
>>> Makes sense, I originally had it there, but decided it made more sense up
>>> top.  Fixed in the newest patch
>>>>> +<source>
>>>>> +<host name="iscsi.example.com"/>
>>>>> +<device path="demo-target"/>
>>>>> +<auth type='chap' login='foobar' passwd='frobbar'/>
>>>>       <vendor name='test-vendor'/>
>>>>       <product name='test-product'/>
>>>>
>>>> Also NB I called it 'product' instead of 'model', since that
>>>> matches the terminology we use in the node device XML format
>>>>
>>> Switched the naming there.
>>>>> +</source>
>>>>> +<target>
>>>>> +<path>/dev/disk/by-path</path>
>>>>> +<permissions>
>>>>> +<mode>0700</mode>
>>>>> +<owner>0</owner>
>>>>> +<group>0</group>
>>>>> +</permissions>
>>>>> +</target>
>>>>> +</pool>
>>>> REgards,
>>>> Daniel
>>> I've prepped a new patch which should fix the issues brought up by both
>>> Daniels.  It's attached at the bottom.
>>>
>>> Best,
>>>
>>> Patrick Dignan
>>>
>>> diff -Naurb libvirt-0.8.2.orig/include/libvirt/libvirt.h
>>> libvirt-0.8.2.ml/include/libvirt/libvirt.h
>>> --- libvirt-0.8.2.orig/include/libvirt/libvirt.h    2010-07-22
>>> 10:26:05.000000000 -0500
>>> +++ libvirt-0.8.2.ml/include/libvirt/libvirt.h    2010-08-02
>>> 14:57:49.903530315 -0500
>>> @@ -1145,6 +1145,8 @@
>>>    unsigned long long capacity;   /* Logical size bytes */
>>>    unsigned long long allocation; /* Current allocation bytes */
>>>    unsigned long long available;  /* Remaining free space bytes */
>>> +  char *vendor;
>>> +  char *model;
>>>   };
>> Why do you add new members to the virStoragePoolInfo struct? Also
>> libvirt.h is generated from libvirt.h.in.
>>
>> It seems that those members are a leftover from previous versions of
>> this patch and can be removed entirely?
> Yes, those have to be removed as they are changing public ABI.
>
> Regards,
> Daniel
Somehow the git-send-email patch didn't make it to the mailing list, 
though my reply to it did...I've attached the patch here, hopefully this 
one doesn't get flattened.

I decided to allow vendor and product to be set independently of each 
other in order to allow greater flexibility at the possible expense of 
namespace clashes.

Best,

Patrick Dignan
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Add-support-for-vendor-product.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100809/ffc42fd3/attachment-0001.ksh>