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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list