Re: [Qemu-devel] [PATCH 4/5] virtio: Add virtio-rng driver

[Ian Molton <ian.molton@collabora.co.uk>]

Anthony Liguori wrote:

>> +            DEFINE_RNG_PROPERTIES(VirtIOPCIProxy, rng),
>>    
> 
> I don't see any reason to use a define here.

Consistency with the other virtio code (at the time I wrote it)

> Coding style is off here (newline between ) and { ).

Fixed.

> Can't call gettimeofday directly.  You have to use qemu_gettimeofday(). 
> But it would be better to not rely on gettimeofday and instead make use
> of the rt_clock.

Hm, this I fixed before, I'll make sure its right in the next patch.
Must have got an old revision mixed up there.

>> +static void virtio_rng_save(QEMUFile *f, void *opaque)
>> +{
>> +    VirtIORng *s = opaque;
>> +
>> +    virtio_save(&s->vdev, f);
>> +}
>> +
>> +static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
>> +{
>> +    VirtIORng *s = opaque;
>> +
>> +    if (version_id != 1)
>> +        return -EINVAL;
>> +
>> +    virtio_load(&s->vdev, f);
>> +    return 0;
>> +}
>>
>>    
> 
> This doesn't look correct to me.  There is absolutely no state
> maintained by the virtio-rng backend?  I find that hard to believe.

What state needs maintaining? when it runs out of entropy, it simply
reconnects. Unless I misunderstood what those functions are for...

-Ian