From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59458) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZzqJ1-0003cV-Dg for qemu-devel@nongnu.org; Fri, 20 Nov 2015 13:21:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZzqIy-00063I-7o for qemu-devel@nongnu.org; Fri, 20 Nov 2015 13:21:07 -0500 Received: from mx5-phx2.redhat.com ([209.132.183.37]:55437) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZzqIx-00063E-Ui for qemu-devel@nongnu.org; Fri, 20 Nov 2015 13:21:04 -0500 Date: Fri, 20 Nov 2015 13:20:57 -0500 (EST) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Message-ID: <430569618.12530858.1448043657890.JavaMail.zimbra@redhat.com> In-Reply-To: <87si40sfzh.fsf@blackfin.pond.sub.org> References: <87si401wpf.fsf@blackfin.pond.sub.org> <2083526024.12459505.1448036588653.JavaMail.zimbra@redhat.com> <564F4E68.8090903@redhat.com> <87si40sfzh.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] ivshmem property size should be a size, not a string List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: marcandre lureau , Luiz Capitulino , Claudio Fontana , qemu-devel@nongnu.org ----- Original Message ----- > Eric Blake writes: >=20 > > On 11/20/2015 09:23 AM, Marc-Andr=C3=A9 Lureau wrote: > >> Hi > >>=20 > >> ----- Original Message ----- > >>> Everybody's favourite device model has "size" property. It's declare= d > >>> as *string* > >>> > >>> DEFINE_PROP_STRING("size", IVShmemState, sizearg), > >>> > >>> > >>> * In QMP, the size must be given as JSON string instead of JSON numbe= r, > >>> and size suffixes are accepted. Example: "size": "512k" instead of > >>> "size": 524288. > >>> > >>> Right now, this violation of QMP rules is tolerable (barely), becau= se > >>> device_add breaks some of these rules already. However, one goal o= f > >>> the current work on QAPI is to support a QMP command to plug device= s > >>> that doesn't break QMP rules, and then this violation will stand ou= t. > >>> > >>> 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. >=20 > Technically, we already have a compatibility break: >=20 > $ qemu-system-x86_64 -nodefaults -S -display none -chardev > socket,path=3D/work/armbru/images/ivshmem-socket,id=3Divshmem-chr -device > ivshmem,size=3D4,chardev=3Divshmem-chr,shm=3Dfoo >=20 > In 2.3.0, this ignores shm with warning "do not specify both 'chardev' > and 'shm' with ivshmem". >=20 > In current upstream, it's rejected. That's a regression, either we should: - keep accepting it - or keep rejection and remove the "WARNING:"=20 Imho in this case, it was an undefined behaviour, warned, so it's fine to r= eject it now. > I suspect we'd find more if we cared to look. >=20 > Of course, the only thing that *really* matters is *actual* usage in > anger: something of value that breaks. >=20 > Hash ivshmem been used in anger? If yes, how? >=20 > >>> A straight fix of size isn't fully backwards compatible: suffixes n= o > >>> longer work in QMP, and you need to use an 'M' suffix to get Mebiby= tes > >>> on command line and HMP. > >>=20 > >> 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 withou= t > > 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. >=20 > Indeed. >=20 > >>> If that's unacceptable, we'll have to provide a new, fixed property= , > >>> and deprecate size. > >>=20 > >> 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? >=20 > Working on one. >=20