From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58497) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZzoT2-0005Cf-US for qemu-devel@nongnu.org; Fri, 20 Nov 2015 11:23:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZzoSy-0008Eg-0e for qemu-devel@nongnu.org; Fri, 20 Nov 2015 11:23:20 -0500 Received: from mx6-phx2.redhat.com ([209.132.183.39]:32879) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZzoSx-0008EU-OA for qemu-devel@nongnu.org; Fri, 20 Nov 2015 11:23:15 -0500 Date: Fri, 20 Nov 2015 11:23:08 -0500 (EST) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Message-ID: <2083526024.12459505.1448036588653.JavaMail.zimbra@redhat.com> In-Reply-To: <87si401wpf.fsf@blackfin.pond.sub.org> References: <87si401wpf.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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 , Claudio Fontana , qemu-devel@nongnu.org, Luiz Capitulino Hi ----- Original Message ----- > Everybody's favourite device model has "size" property. It's declared > as *string* > > DEFINE_PROP_STRING("size", IVShmemState, sizearg), > > which gets converted to a size manually in the realize method: > > } else if (s->sizearg == NULL) { > s->ivshmem_size = 4 << 20; /* 4 MB default */ > } else { > char *end; > int64_t size = qemu_strtosz(s->sizearg, &end); > if (size < 0 || *end != '\0' || !is_power_of_2(size)) { > error_setg(errp, "Invalid size %s", s->sizearg); > return; > } > s->ivshmem_size = size; > } > > This is *wrong*. Impact, as far as I can tell: > > * -device ivshmem,help shows the property as 'str' instead of 'size'. > > Unhelpful, but hardly show-stopper. > > * On the command line and in HMP, ivshmem's size is parsed differently > than other size properties. In particular, a number without a suffix > is normally interpreted as bytes, except for ivshmem, where it's > Mebibytes. > > Ugly inconsistency, but hardly the only one. > > * 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. > > 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. > If that's unacceptable, we'll have to provide a new, fixed property, > and deprecate size. Sounds a better alternative to me.