[libvirt] [PATCH] Add support for Vendor and Model in Storage Pool XML
Daniel P. Berrange
berrange at redhat.com
Fri Aug 6 08:16:08 UTC 2010
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
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
More information about the libvir-list
mailing list