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

[Markus Armbruster]

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.

  If that's unacceptable, we'll have to provide a new, fixed property,
  and deprecate size.

  Opinions?