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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list