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

[Eric Blake]

On 08/05/2010 12:45 PM, Patrick Dignan wrote:
> diff -Naurb libvirt-0.8.2.orig/docs/schemas/storagepool.rng

Ouch - this flattened the whitespace to the point that it makes
reviewing harder to do.  Could you consider resending, either via 'git
send-email' or by attaching the output of 'git format-patch', such that
your indentation is preserved?

> 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>

Would it ever make sense to have only vendor or product, rather than to
always require both elements or neither?  If so, you either need two
<define>s or some more .rng magic around the two <element>s.

> @@ -465,6 +469,18 @@
>              goto cleanup;
>      }
> 
> +    vendor = virXPathString("string(./vendor/@name)", ctxt);
> +
> +    if (vendor != NULL) {
> +        source->vendor = strdup(vendor);

virXPathString returns malloc'd memory.  No need to strdup it; just use
it as is - that will avoid a memory leak, since you didn't call
VIR_FREE(vendor) in the cleanup: label.


> +    }
> +
> +    product = virXPathString("string(./product/@name)", ctxt);
> +
> +    if (product != NULL) {
> +        source->product = strdup(product);
> +    }
> +

Back to the mutually-essential question - per your current .rng, you
should issue an error here if (product==NULL)!=(vendor==NULL).  But is
that really essential, or does it make sense that you could have one and
not the other?

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

More code that assumes both elements will either be present or absent
together.

-- 
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: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100805/64ff8fd6/attachment-0001.sig>