Re: [libvirt] [PATCH] storage: use valid XML for awkward volume names
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
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)