| Message ID | 20200528213946.1636444-5-rvkagan@yandex-team.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | block: enhance handling of size-related BlockConf properties | expand |
On 5/28/20 4:39 PM, Roman Kagan wrote: > Introduce size32 property type which handles size suffixes (k, m) just > like size property, but is uint32_t rather than uint64_t. Does it handle 'g' as well? (even though the set of valid 32-bit sizes with a g suffix is rather small ;) > It's going to > be useful for properties that are byte sizes but are inherently 32bit, > like BlkConf.opt_io_size or .discard_granularity (they are switched to > this new property type in a followup commit). > > The getter for size32 is left out for a separate patch as its benefit is > less obvious, and it affects test output; for now the regular uint32 > getter is used. > > Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru> > --- > > +static void set_size32(Object *obj, Visitor *v, const char *name, void *opaque, > + Error **errp) > +{ > + DeviceState *dev = DEVICE(obj); > + Property *prop = opaque; > + uint32_t *ptr = qdev_get_prop_ptr(dev, prop); > + uint64_t value; > + Error *local_err = NULL; > + > + if (dev->realized) { > + qdev_prop_set_after_realize(dev, name, errp); > + return; > + } > + > + visit_type_size(v, name, &value, &local_err); Yes, it does. Whether or not the commit message is tweaked, Reviewed-by: Eric Blake <eblake@redhat.com>
On Thu, May 28, 2020 at 04:45:19PM -0500, Eric Blake wrote: > On 5/28/20 4:39 PM, Roman Kagan wrote: > > Introduce size32 property type which handles size suffixes (k, m) just > > like size property, but is uint32_t rather than uint64_t. > > Does it handle 'g' as well? (even though the set of valid 32-bit sizes with > a g suffix is rather small ;) > > > It's going to > > be useful for properties that are byte sizes but are inherently 32bit, > > like BlkConf.opt_io_size or .discard_granularity (they are switched to > > this new property type in a followup commit). > > > > The getter for size32 is left out for a separate patch as its benefit is > > less obvious, and it affects test output; for now the regular uint32 > > getter is used. > > > > Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru> > > --- > > > > > +static void set_size32(Object *obj, Visitor *v, const char *name, void *opaque, > > + Error **errp) > > +{ > > + DeviceState *dev = DEVICE(obj); > > + Property *prop = opaque; > > + uint32_t *ptr = qdev_get_prop_ptr(dev, prop); > > + uint64_t value; > > + Error *local_err = NULL; > > + > > + if (dev->realized) { > > + qdev_prop_set_after_realize(dev, name, errp); > > + return; > > + } > > + > > + visit_type_size(v, name, &value, &local_err); > > Yes, it does. > > Whether or not the commit message is tweaked, > Reviewed-by: Eric Blake <eblake@redhat.com> I did this stupid stringify(UINT32_MAX) here too. It's even uglier here, with an 'U' appended to the number in the brackets, but somehow it didn't strike me in the eye while testing. So I'll fix this too in the respin, and drop the r-b. Thanks, Roman.
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index f161604fb6..c03eadfad6 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -29,6 +29,7 @@ extern const PropertyInfo qdev_prop_drive; extern const PropertyInfo qdev_prop_drive_iothread; extern const PropertyInfo qdev_prop_netdev; extern const PropertyInfo qdev_prop_pci_devfn; +extern const PropertyInfo qdev_prop_size32; extern const PropertyInfo qdev_prop_blocksize; extern const PropertyInfo qdev_prop_pci_host_devaddr; extern const PropertyInfo qdev_prop_uuid; @@ -196,6 +197,8 @@ extern const PropertyInfo qdev_prop_pcie_link_width; BlockdevOnError) #define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int) +#define DEFINE_PROP_SIZE32(_n, _s, _f, _d) \ + DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_size32, uint32_t) #define DEFINE_PROP_BLOCKSIZE(_n, _s, _f) \ DEFINE_PROP_UNSIGNED(_n, _s, _f, 0, qdev_prop_blocksize, uint16_t) #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \ diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 249dc69bd8..d943755832 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -727,6 +727,46 @@ const PropertyInfo qdev_prop_pci_devfn = { .set_default_value = set_default_value_int, }; +/* --- 32bit unsigned int 'size' type --- */ + +static void set_size32(Object *obj, Visitor *v, const char *name, void *opaque, + Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + uint32_t *ptr = qdev_get_prop_ptr(dev, prop); + uint64_t value; + Error *local_err = NULL; + + if (dev->realized) { + qdev_prop_set_after_realize(dev, name, errp); + return; + } + + visit_type_size(v, name, &value, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + if (value > UINT32_MAX) { + error_setg(errp, + "Property %s.%s doesn't take value %" PRIu64 + " (maximum: " stringify(UINT32_MAX) ")", + dev->id ? : "", name, value); + return; + } + + *ptr = value; +} + +const PropertyInfo qdev_prop_size32 = { + .name = "size", + .get = get_uint32, + .set = set_size32, + .set_default_value = set_default_value_uint, +}; + /* --- blocksize --- */ /* lower limit is sector size */
Introduce size32 property type which handles size suffixes (k, m) just like size property, but is uint32_t rather than uint64_t. It's going to be useful for properties that are byte sizes but are inherently 32bit, like BlkConf.opt_io_size or .discard_granularity (they are switched to this new property type in a followup commit). The getter for size32 is left out for a separate patch as its benefit is less obvious, and it affects test output; for now the regular uint32 getter is used. Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru> --- v6 -> v7: - split out into separate patch [Eric] include/hw/qdev-properties.h | 3 +++ hw/core/qdev-properties.c | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+)