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/docs/schemas/storagepool.rng
libvirt-0.8.2.ml/docs/schemas/storagepool.rng
--- libvirt-0.8.2.orig/docs/schemas/storagepool.rng 2010-06-16
17:27:22.000000000 -0500
+++ libvirt-0.8.2.ml/docs/schemas/storagepool.rng 2010-08-05
12:16:31.703536682 -0500
@@ -103,6 +103,19 @@
<ref name='target'/>
</define>
+ <define name='sourceinfovendor'>
+ <element name='vendor'>
+ <attribute name='name'>
+ <text/>
+ </attribute>
+ </element>
+ <element name='product'>
+ <attribute name='name'>
+ <text/>
+ </attribute>
+ </element>
+ </define>
+
<define name='commonmetadata'>
<element name='name'>
<ref name='name'/>
@@ -272,6 +285,9 @@
<value>ocfs2</value>
</choice>
</attribute>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</optional>
</define>
@@ -286,6 +302,9 @@
<value>nfs</value>
</choice>
</attribute>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</optional>
</define>
@@ -307,6 +326,9 @@
<value>lvm2</value>
</choice>
</attribute>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</optional>
</define>
@@ -321,6 +343,9 @@
<value>lvm2</value>
</choice>
</attribute>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</optional>
</define>
@@ -330,13 +355,20 @@
<optional>
<element name='source'>
<empty/>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</optional>
</define>
+
<define name='sourcefs'>
<element name='source'>
<ref name='sourceinfodev'/>
<ref name='sourcefmtfs'/>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</define>
@@ -345,6 +377,9 @@
<ref name='sourceinfohost'/>
<ref name='sourceinfodir'/>
<ref name='sourcefmtnetfs'/>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</define>
@@ -359,6 +394,9 @@
</optional>
</oneOrMore>
<ref name='sourcefmtlogical'/>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</define>
@@ -366,6 +404,9 @@
<element name='source'>
<ref name='sourceinfodev'/>
<ref name='sourcefmtdisk'/>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</define>
@@ -379,12 +420,19 @@
<optional>
<ref name='sourceinfoauth'/>
</optional>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</define>
<define name='sourcescsi'>
<element name='source'>
<ref name='sourceinfoadapter'/>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
+
</element>
</define>
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;
};
typedef virStoragePoolInfo *virStoragePoolInfoPtr;
diff -Naurb libvirt-0.8.2.orig/src/conf/storage_conf.c
libvirt-0.8.2.ml/src/conf/storage_conf.c
--- libvirt-0.8.2.orig/src/conf/storage_conf.c 2010-06-16
17:27:22.000000000 -0500
+++ libvirt-0.8.2.ml/src/conf/storage_conf.c 2010-08-05
13:31:32.469533098 -0500
@@ -284,6 +284,8 @@
VIR_FREE(source->name);
VIR_FREE(source->adapter);
VIR_FREE(source->initiator.iqn);
+ VIR_FREE(source->vendor);
+ VIR_FREE(source->product);
if (source->authType == VIR_STORAGE_POOL_AUTH_CHAP) {
VIR_FREE(source->auth.chap.login);
@@ -393,6 +395,8 @@
char *authType = NULL;
int nsource, i;
virStoragePoolOptionsPtr options;
+ char *vendor;
+ char *product;
relnode = ctxt->node;
ctxt->node = node;
@@ -465,6 +469,18 @@
goto cleanup;
}
+ vendor = virXPathString("string(./vendor/@name)", ctxt);
+
+ if (vendor != NULL) {
+ source->vendor = strdup(vendor);
+ }
+
+ product = virXPathString("string(./product/@name)", ctxt);
+
+ if (product != NULL) {
+ source->product = strdup(product);
+ }
+
ret = 0;
cleanup:
ctxt->node = relnode;
@@ -838,6 +854,12 @@
virBufferVSprintf(buf," <auth type='chap' login='%s'
passwd='%s'/>\n",
src->auth.chap.login,
src->auth.chap.passwd);
+
+ if (src->vendor != NULL && src->product != NULL) {
+ virBufferVSprintf(buf," <vendor name='%s'/>\n", src->vendor);
+ virBufferVSprintf(buf," <product name='%s'/>\n", src->product);
+ }
+
virBufferAddLit(buf," </source>\n");
return 0;
diff -Naurb libvirt-0.8.2.orig/src/conf/storage_conf.h
libvirt-0.8.2.ml/src/conf/storage_conf.h
--- libvirt-0.8.2.orig/src/conf/storage_conf.h 2010-06-16
17:27:22.000000000 -0500
+++ libvirt-0.8.2.ml/src/conf/storage_conf.h 2010-08-05
10:53:41.034539304 -0500
@@ -237,6 +237,12 @@
virStoragePoolAuthChap chap;
} auth;
+ /* Vendor of the the source */
+ char *vendor;
+
+ /* Product name of the source*/
+ char *product;
+
int format; /* Pool type specific format such as filesystem type,
or lvm version, etc */
};
diff -Naurb
libvirt-0.8.2.orig/tests/storagepoolxml2xmlin/pool-iscsi-vendor-product.xml
libvirt-0.8.2.ml/tests/storagepoolxml2xmlin/pool-iscsi-vendor-product.xml
---
libvirt-0.8.2.orig/tests/storagepoolxml2xmlin/pool-iscsi-vendor-product.xml
1969-12-31 18:00:00.000000000 -0600
+++
libvirt-0.8.2.ml/tests/storagepoolxml2xmlin/pool-iscsi-vendor-product.xml
2010-08-05 10:55:29.097539286 -0500
@@ -0,0 +1,19 @@
+<pool type='iscsi'>
+ <name>virtimages</name>
+ <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
+ <source>
+ <host name="iscsi.example.com"/>
+ <device path="demo-target"/>
+ <auth type='chap' login='foobar' passwd='frobbar'/>
+ <vendor name='test-vendor'/>
+ <product name='test-model'/>
+ </source>
+ <target>
+ <path>/dev/disk/by-path</path>
+ <permissions>
+ <mode>0700</mode>
+ <owner>0</owner>
+ <group>0</group>
+ </permissions>
+ </target>
+</pool>
diff -Naurb
libvirt-0.8.2.orig/tests/storagepoolxml2xmlout/pool-iscsi-vendor-product.xml
libvirt-0.8.2.ml/tests/storagepoolxml2xmlout/pool-iscsi-vendor-product.xml
---
libvirt-0.8.2.orig/tests/storagepoolxml2xmlout/pool-iscsi-vendor-product.xml
1969-12-31 18:00:00.000000000 -0600
+++
libvirt-0.8.2.ml/tests/storagepoolxml2xmlout/pool-iscsi-vendor-product.xml
2010-08-05 10:56:50.137535457 -0500
@@ -0,0 +1,22 @@
+<pool type='iscsi'>
+ <name>virtimages</name>
+ <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
+ <capacity>0</capacity>
+ <allocation>0</allocation>
+ <available>0</available>
+ <source>
+ <host name='iscsi.example.com'/>
+ <device path='demo-target'/>
+ <auth type='chap' login='foobar' passwd='frobbar'/>
+ <vendor name='test-vendor'/>
+ <product name='test-model'/>
+ </source>
+ <target>
+ <path>/dev/disk/by-path</path>
+ <permissions>
+ <mode>0700</mode>
+ <owner>0</owner>
+ <group>0</group>
+ </permissions>
+ </target>
+</pool>
diff -Naurb libvirt-0.8.2.orig/tests/storagepoolxml2xmltest.c
libvirt-0.8.2.ml/tests/storagepoolxml2xmltest.c
--- libvirt-0.8.2.orig/tests/storagepoolxml2xmltest.c 2010-06-16
17:27:22.000000000 -0500
+++ libvirt-0.8.2.ml/tests/storagepoolxml2xmltest.c 2010-08-05
11:11:20.823541487 -0500
@@ -96,6 +96,7 @@
DO_TEST("pool-scsi");
DO_TEST("pool-mpath");
DO_TEST("pool-iscsi-multiiqn");
+ DO_TEST("pool-iscsi-vendor-product");
return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list