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@xxxxxxxx>:
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
From 3a2139c87a1d1c92a8bf947ecc5b431f0f67906c Mon Sep 17 00:00:00 2001
From: Patrick Dignan <pat_dignan@xxxxxxxx>
Date: Fri, 6 Aug 2010 11:08:03 -0500
Subject: [PATCH] Add support for vendor/product
---
docs/schemas/storagepool.rng | 52 ++++++++++++++++++++
src/conf/storage_conf.c | 14 +++++
src/conf/storage_conf.h | 6 ++
.../pool-iscsi-vendor-product.xml | 19 +++++++
.../pool-iscsi-vendor-product.xml | 22 ++++++++
tests/storagepoolxml2xmltest.c | 1 +
6 files changed, 114 insertions(+), 0 deletions(-)
create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-vendor-product.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-vendor-product.xml
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index b911f7c..a8a3f36 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -103,6 +103,23 @@
<ref name='target'/>
</define>
+ <define name='sourceinfovendor'>
+ <optional>
+ <element name='vendor'>
+ <attribute name='name'>
+ <text/>
+ </attribute>
+ </element>
+ </optional>
+ <optional>
+ <element name='product'>
+ <attribute name='name'>
+ <text/>
+ </attribute>
+ </element>
+ </optional>
+ </define>
+
<define name='commonmetadata'>
<element name='name'>
<ref name='name'/>
@@ -272,6 +289,9 @@
<value>ocfs2</value>
</choice>
</attribute>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</optional>
</define>
@@ -286,6 +306,9 @@
<value>nfs</value>
</choice>
</attribute>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</optional>
</define>
@@ -307,6 +330,9 @@
<value>lvm2</value>
</choice>
</attribute>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</optional>
</define>
@@ -321,6 +347,9 @@
<value>lvm2</value>
</choice>
</attribute>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</optional>
</define>
@@ -330,13 +359,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 +381,9 @@
<ref name='sourceinfohost'/>
<ref name='sourceinfodir'/>
<ref name='sourcefmtnetfs'/>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</define>
@@ -359,6 +398,9 @@
</optional>
</oneOrMore>
<ref name='sourcefmtlogical'/>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</define>
@@ -366,6 +408,9 @@
<element name='source'>
<ref name='sourceinfodev'/>
<ref name='sourcefmtdisk'/>
+ <optional>
+ <ref name='sourceinfovendor'/>
+ </optional>
</element>
</define>
@@ -379,12 +424,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 --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index bf86c93..6e3cce6 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -284,6 +284,8 @@ virStoragePoolSourceFree(virStoragePoolSourcePtr source) {
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);
@@ -465,6 +467,9 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
goto cleanup;
}
+ source->vendor = virXPathString("string(./vendor/@name)", ctxt);
+ source->product = virXPathString("string(./product/@name)", ctxt);
+
ret = 0;
cleanup:
ctxt->node = relnode;
@@ -838,6 +843,15 @@ virStoragePoolSourceFormat(virBufferPtr buf,
virBufferVSprintf(buf," <auth type='chap' login='%s' passwd='%s'/>\n",
src->auth.chap.login,
src->auth.chap.passwd);
+
+ if (src->vendor != NULL) {
+ virBufferVSprintf(buf," <vendor name='%s'/>\n", src->vendor);
+ }
+
+ if (src->product != NULL) {
+ virBufferVSprintf(buf," <product name='%s'/>\n", src->product);
+ }
+
virBufferAddLit(buf," </source>\n");
return 0;
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index bedee9e..5f17b5a 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -237,6 +237,12 @@ struct _virStoragePoolSource {
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 --git a/tests/storagepoolxml2xmlin/pool-iscsi-vendor-product.xml b/tests/storagepoolxml2xmlin/pool-iscsi-vendor-product.xml
new file mode 100644
index 0000000..01fbd9b
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-iscsi-vendor-product.xml
@@ -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-product'/>
+ </source>
+ <target>
+ <path>/dev/disk/by-path</path>
+ <permissions>
+ <mode>0700</mode>
+ <owner>0</owner>
+ <group>0</group>
+ </permissions>
+ </target>
+</pool>
diff --git a/tests/storagepoolxml2xmlout/pool-iscsi-vendor-product.xml b/tests/storagepoolxml2xmlout/pool-iscsi-vendor-product.xml
new file mode 100644
index 0000000..baab6c0
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-iscsi-vendor-product.xml
@@ -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-product'/>
+ </source>
+ <target>
+ <path>/dev/disk/by-path</path>
+ <permissions>
+ <mode>0700</mode>
+ <owner>0</owner>
+ <group>0</group>
+ </permissions>
+ </target>
+</pool>
diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
index 33a7343..2ec29e3 100644
--- a/tests/storagepoolxml2xmltest.c
+++ b/tests/storagepoolxml2xmltest.c
@@ -96,6 +96,7 @@ mymain(int argc, char **argv)
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);
}
--
1.7.2
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list