Re: [Qemu-devel] ivshmem property size should be a size, not a string

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1975 bytes --]

On 11/20/2015 09:23 AM, Marc-André Lureau wrote:
> Hi
> 
> ----- Original Message -----
>> Everybody's favourite device model has "size" property.  It's declared
>> as *string*
>>
>>     DEFINE_PROP_STRING("size", IVShmemState, sizearg),
>>
>>
>> * In QMP, the size must be given as JSON string instead of JSON number,
>>   and size suffixes are accepted.  Example: "size": "512k" instead of
>>   "size": 524288.
>>
>>   Right now, this violation of QMP rules is tolerable (barely), because
>>   device_add breaks some of these rules already.  However, one goal of
>>   the current work on QAPI is to support a QMP command to plug devices
>>   that doesn't break QMP rules, and then this violation will stand out.
>>
>>   Therefore, I want it fixed now, before ivshmem gets used in anger.

Was ivshmem even usable in 2.4?  I'd argue that if we are going to
change it, changing it for 2.5 is the ideal time.

>>
>>   A straight fix of size isn't fully backwards compatible: suffixes no
>>   longer work in QMP, and you need to use an 'M' suffix to get Mebibytes
>>   on command line and HMP.
> 
> I don't know the rules to break properties in qemu, but I would prefer to avoid it if possible.

It's possible to use an alternate to accept both a string and an
integer.  But in general, QMP _wants_ to use byte-count integers without
suffix (the convenience of '512k' is only for the command line and HMP),
so letting QMP expose a string of "512k" instead of an integer 524288
feels wrong.

> 
>>   If that's unacceptable, we'll have to provide a new, fixed property,
>>   and deprecate size.
> 
> Sounds a better alternative to me.

I'd rather fix the interface rather than catering to ugliness,
particularly since 2.5 already fixes so much else that was ugly about
ivshmem.

Markus, did you have a patch to propose?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]