[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



 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);
 }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]