Eric Blake <ebl...@redhat.com> writes:
> 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.
Technically, we already have a compatibility break:
$ qemu-system-x86_64 -nodefaults -S -display none -chardev
socket,path=/work/armbru/images/ivshmem-socket,id=ivshmem-chr -device
ivshmem,size=4,chardev=ivshmem-chr,shm=foo
In 2.3.0, this ignores shm with warning "do not specify both 'chardev'
and 'shm' with ivshmem".
In current upstream, it's rejected.
I suspect we'd find more if we cared to look.
Of course, the only thing that *really* matters is *actual* usage in
anger: something of value that breaks.
Hash ivshmem been used in anger? If yes, how?
>>> 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.
Indeed.
>>> 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?
Working on one.
