On 02/25/2013 03:45 AM, Peter Krempa wrote:
On 02/23/13 01:29, Eric Blake wrote:
> On 02/21/2013 07:47 AM, Peter Krempa wrote:
>> Adds XML parsing and qemu commandline tests for the VirtIO RNG device
>> support.
>> ---
>
> Is it worth testing that a filename containing an XML-special character
> is properly escaped? Other than that, this one is still good to go.
>
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
>> @@ -0,0 +1,23 @@
>
>> + <rng model='virtio'>
>> + <backend model='random'>/test/phile</backend>
>
> That is, should this use something like /test/<phile as the XML
> encoded file name?
>
Uh, I'm not following you on this one. You mean that if the user
specifies some characters that are invalid from the perspective of XML
as a source path?
We should not get in the way of a user doing:
ln -s /dev/urandom '/tmp/my<evil>random'
then passing:
<backend model='random'>/tmp/my<evil>random</backend>
in their domain XML. By using virBufferEscape on the output side, we
allow the user to use XML escapes on their input to specify any valid
file name.
Anyways, I fixed the issues you pointed out in 1-6 and provided
explanation for the other stuff. I'm pushing patches 1-7 (the test suite
can be improved at any time) now and will follow up later with a
improved version of 8 as well as with a patch that will allow multiple
RNG devices. That should be better to review as slicing apart the
existing patches.
Sure, doing things as followups is fine, since you already collected
ACKs before my late review.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org