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

[Eric Blake]

On 08/17/2010 11:44 AM, Patrick Dignan wrote:
>>> +    if (src->product != NULL) {
>>> +        virBufferVSprintf(buf,"<product name='%s'/>\n", src->product);
>>> +    }
>>> +
>>>       virBufferAddLit(buf,"</source>\n");
>> The only change is that these should use virBufferEscapeString
>> since vendor/product strings might contain<,>  or&  characters
>> which would make the XML unhappy
>>
>>
>> Regards,
>> Daniel
> 
> Ok, so I've updated that.  Hopefully all is well with this patch!

Almost.  The .rng file had some whitespace issues.  Also, the
sourcefmtfs reference only occurs as part of the sourcefs element, so
only the latter needed the optional sourceinfovendor.  And 'make
syntax-check' didn't like your use of "the the" in a comment nor your
trailing spaces.

Meanwhile, you missed documentation in docs/formatstorage.html.in. I'm
getting more serious about blocking patches to docs/schemas without the
corresponding changes to docs/format*html.in, because even if it's hard
to do up front, it's much harder to do long after the fact; but until we
have a strong precedence of that action in the git history, I can see
how you overlooked it.

But those were minor enough to have my ACK; I didn't mind touching this
up since it was your first submission, but hopefully don't have to spend
as much cleanup work on future patches from you.  Here's what I squashed
in before pushing (along with an AUTHORS update not listed here).

diff --git i/docs/formatstorage.html.in w/docs/formatstorage.html.in
index 5c1d36c..91f70a3 100644
--- i/docs/formatstorage.html.in
+++ w/docs/formatstorage.html.in
@@ -70,6 +70,8 @@
         <source>
           <host name="iscsi.example.com"/>
           <device path="demo-target"/>
+          <vendor name="Acme"/>
+          <product name="model"/&t;
         </source>
         ...</pre>

@@ -108,6 +110,16 @@
         type, or network filesystem type, or partition table type, or
         LVM metadata type. All drivers are required to have a default
         value for this, so it is optional. <span class="since">Since
0.4.1</span></dd>
+
+      <dt><code>vendor</code></dt>
+      <dd>Provides optional information about the vendor of the
+        storage device. This contains a single
+        attribute <code>name</code> whose value is backend
+        specific. <span class="since">Since 0.8.4</span></dd>
+      <dt><code>product</code></dt>
+      <dd>Provides an optional product name of the storage device.
+        This contains a single attribute <code>name</code> whose value
+        is backend specific.  <span class="since">Since 0.8.4</span></dd>
     </dl>

     <h3><a name="StoragePoolTarget">Target elements</a></h3>
diff --git i/docs/schemas/storagepool.rng w/docs/schemas/storagepool.rng
index a8a3f36..54eb802 100644
--- i/docs/schemas/storagepool.rng
+++ w/docs/schemas/storagepool.rng
@@ -289,9 +289,6 @@
             <value>ocfs2</value>
           </choice>
         </attribute>
-        <optional>
-          <ref name='sourceinfovendor'/>
-        </optional>
       </element>
     </optional>
   </define>
@@ -371,7 +368,7 @@
       <ref name='sourceinfodev'/>
       <ref name='sourcefmtfs'/>
       <optional>
-          <ref name='sourceinfovendor'/>
+        <ref name='sourceinfovendor'/>
       </optional>
     </element>
   </define>
@@ -382,7 +379,7 @@
       <ref name='sourceinfodir'/>
       <ref name='sourcefmtnetfs'/>
       <optional>
-          <ref name='sourceinfovendor'/>
+        <ref name='sourceinfovendor'/>
       </optional>
     </element>
   </define>
@@ -399,7 +396,7 @@
       </oneOrMore>
       <ref name='sourcefmtlogical'/>
       <optional>
-          <ref name='sourceinfovendor'/>
+        <ref name='sourceinfovendor'/>
       </optional>
     </element>
   </define>
@@ -408,8 +405,8 @@
     <element name='source'>
       <ref name='sourceinfodev'/>
       <ref name='sourcefmtdisk'/>
-       <optional>
-          <ref name='sourceinfovendor'/>
+      <optional>
+        <ref name='sourceinfovendor'/>
       </optional>
     </element>
   </define>
@@ -425,7 +422,7 @@
         <ref name='sourceinfoauth'/>
       </optional>
       <optional>
-          <ref name='sourceinfovendor'/>
+        <ref name='sourceinfovendor'/>
       </optional>
     </element>
   </define>
@@ -434,7 +431,7 @@
     <element name='source'>
       <ref name='sourceinfoadapter'/>
       <optional>
-          <ref name='sourceinfovendor'/>
+        <ref name='sourceinfovendor'/>
       </optional>

     </element>
diff --git i/src/conf/storage_conf.h w/src/conf/storage_conf.h
index 5f17b5a..fef0a46 100644
--- i/src/conf/storage_conf.h
+++ w/src/conf/storage_conf.h
@@ -237,7 +237,7 @@ struct _virStoragePoolSource {
         virStoragePoolAuthChap chap;
     } auth;

-    /* Vendor of the the source */
+    /* Vendor of the source */
     char *vendor;

     /* Product name of the source*/
diff --git i/src/conf/storage_conf.c w/src/conf/storage_conf.c
index cf6271e..168243f 100644
--- i/src/conf/storage_conf.c
+++ w/src/conf/storage_conf.c
@@ -844,14 +844,14 @@ virStoragePoolSourceFormat(virBufferPtr buf,
                           src->auth.chap.login,
                           src->auth.chap.passwd);

-    if (src->vendor != NULL) {
+    if (src->vendor != NULL) {
         virBufferEscapeString(buf,"    <vendor name='%s'/>\n",
src->vendor);
     }
-
+
     if (src->product != NULL) {
         virBufferEscapeString(buf,"    <product name='%s'/>\n",
src->product);
-    }
-
+    }
+
     virBufferAddLit(buf,"  </source>\n");

     return 0;


-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100819/1795e6fa/attachment-0001.sig>