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

On 08/17/2010 11:44 AM, Patrick Dignan wrote: 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 @@ &lt;source&gt; &lt;host name="iscsi.example.com"/&gt; &lt;device path="demo-target"/&gt; + &lt;vendor name="Acme"/&gt; + &lt;product name="model"/&t; &lt;/source&gt; ...</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(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org