On 11/21/2013 07:55 AM, Ján Tomko wrote:
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.
@@ -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.
It's related, in that several of the tests/*out/*.xml files now have a <key> that was parsed from then counterpart *in/*.xml file (whereas they previously had a <key>(null)</key>). Most of the time, we generate volume defs during pool refresh and not from xml reparsing; but you do have a point that on CreateVol, we need to be careful to ignore any input key and use the one that the storage volume would have generated normally; likewise, we must ensure we don't leak memory. I'm still checking...
ACK if you drop this hunk or fix virStorageBackendDiskCreateVol.
I'll reply again with what I squash in after auditing all paths where the user passes in volume XML to make sure we aren't leaking or using the wrong key. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org