On Thu, Aug 05, 2010 at 11:51:48PM +0200, Matthias Bolte wrote:
On 08/05/2010 07:24 AM, Daniel P. Berrange 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
On Mon, Aug 02, 2010 at 12:44:37PM -0500, Patrick Dignan wrote: 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
2010/8/5 Patrick Dignan<pat_dignan@dell.com>: 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,
On 08/06/2010 03:16 AM, Daniel P. Berrange wrote: 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