qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2] virtio-rng: Add human-readable error message


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v2] virtio-rng: Add human-readable error message for negative max-bytes parameter
Date: Fri, 18 Jul 2014 13:15:18 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

Amit Shah <address@hidden> writes:

> On (Fri) 18 Jul 2014 [08:27:59], Markus Armbruster wrote:
>> John Snow <address@hidden> writes:
>> 
>> > If a negative integer is used for the max_bytes parameter, QEMU currently
>> > calls abort() and leaves behind a core dump. This patch adds a simple
>> > error message to make the reason for the termination clearer.
>> >
>> > Signed-off-by: John Snow <address@hidden>
>> > ---
>> > v2: Changed 0L constant to (uint64_t)0 constant to match PRId64 format code
>> >     on both 32bit and 64bit systems. Tested via -m32 flag.
>> >
>> >  hw/virtio/virtio-rng.c | 6 +++++-
>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
>> > index 1356aca..64c7d23 100644
>> > --- a/hw/virtio/virtio-rng.c
>> > +++ b/hw/virtio/virtio-rng.c
>> > @@ -181,7 +181,11 @@ static void virtio_rng_device_realize(DeviceState 
>> > *dev, Error **errp)
>> >  
>> >      vrng->vq = virtio_add_queue(vdev, 8, handle_input);
>> >  
>> > -    assert(vrng->conf.max_bytes <= INT64_MAX);
>> > +    if (vrng->conf.max_bytes > INT64_MAX) {
>> > +        error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "virtio-rng",
>> > +                  "max_bytes", vrng->conf.max_bytes, (uint64_t)0, 
>> > INT64_MAX);
>> > +        return;
>> > +    }
>> >      vrng->quota_remaining = vrng->conf.max_bytes;
>> >  
>> >      vrng->rate_limit_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
>> 
>> Elsewhere in this function, we use
>> 
>>         error_set(errp, QERR_INVALID_PARAMETER_VALUE, "period",
>>                   "a positive number");
>> 
>> Existing uses of QERR_PROPERTY_VALUE_OUT_OF_RANGE are all for intervals
>> with small bounds.
>
> That's suggestion for a 2.2 patch, right?

This *is* a 2.2 patch, isn't it?

> Do you think the usage as in this patch is fine?

It's not wrong, just inconsistent with existing usage.  I'd prefer
consistency.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]