[libvirt] [PATCH] storage: use valid XML for awkward volume names

[Ján Tomko]

On 11/21/2013 03:00 PM, Eric Blake wrote:
> $ touch /var/lib/libvirt/images/'a<b>c'
> $ virsh pool-refresh default
> $ virsh vol-dumpxml 'a<b>c' default | head -n2
> <volume>
>   <name>a<b>c</name>
> 
> Oops.  That's not valid XML.  And when we fix the XML
> generation, it fails RelaxNG validation.
> 
> I'm also tired of seeing <key>(null)</key> in the example
> output for volume xml; while we used NULLSTR() to avoid
> a NULL deref rather than relying on glibc's printf
> extension behavior, it's even better if we avoid the issue
> in the first place.
> 
> Note that this patch allows pretty much any volume name
> that can appear in a directory (excluding . and .. because
> those are special), but does nothing to change the current
> (unenforced) RelaxNG claim that pool names will consist
> only of letters, numbers, _, -, and +.  Tightening the C
> code to match RelaxNG patterns and/or relaxing the grammar
> to match the C code for pool names is a task for another
> day (but remember, we DID recently tighten C code for
> domain names to exclude a leading '.').
> 
> * src/conf/storage_conf.c (virStoragePoolSourceFormat)
> (virStoragePoolDefFormat, virStorageVolTargetDefFormat)
> (virStorageVolDefFormat): Escape user-controlled strings.
> (virStorageVolDefParseXML): Parse key, for use in unit tests.
> * docs/schemas/basictypes.rng (volName): Relax definition.
> * tests/storagepoolxml2xmltest.c (mymain): Test it.
> * tests/storagevolxml2xmltest.c (mymain): Likewise.
> * tests/storagepoolxml2xmlin/pool-dir-naming.xml: New file.
> * tests/storagepoolxml2xmlout/pool-dir-naming.xml: Likewise.
> * tests/storagevolxml2xmlin/vol-file-naming.xml: Likewise.
> * tests/storagevolxml2xmlout/vol-file-naming.xml: Likewise.
> * tests/storagevolxml2xmlout/vol-*.xml: Fix fallout.
> 

> @@ -1282,8 +1275,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
>          goto error;
>      }
> 
> -    /* Auto-generated so deliberately ignore */
> -    /* ret->key = virXPathString("string(./key)", ctxt); */
> +    /* Normally generated by pool refresh, but useful for unit tests */
> +    ret->key = virXPathString("string(./key)", ctxt);
> 
>      capacity = virXPathString("string(./capacity)", ctxt);
>      unit = virXPathString("string(./capacity/@unit)", ctxt);

This seems unrelated to the rest of the patch and might affect
virStorageBackendDiskCreateVol which doesn't ignore the key.

ACK if you drop this hunk or fix virStorageBackendDiskCreateVol.

Jan