From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52442) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7qlo-0002IO-QS for qemu-devel@nongnu.org; Wed, 14 Mar 2012 12:09:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S7qli-0003v2-3V for qemu-devel@nongnu.org; Wed, 14 Mar 2012 12:09:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:4495) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7qlh-0003ux-RC for qemu-devel@nongnu.org; Wed, 14 Mar 2012 12:09:42 -0400 Message-ID: <4F60C18C.2010704@redhat.com> Date: Wed, 14 Mar 2012 17:04:28 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1331740626-5053-1-git-send-email-stefanha@linux.vnet.ibm.com> <1331740626-5053-3-git-send-email-stefanha@linux.vnet.ibm.com> In-Reply-To: <1331740626-5053-3-git-send-email-stefanha@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 2/3] qdev: add blocksize property type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , qemu-devel@nongnu.org Il 14/03/2012 16:57, Stefan Hajnoczi ha scritto: > Storage interfaces like virtio-blk can be configured with block size > information so that the guest can take advantage of efficient I/O > request sizes. > > According to the SCSI Block Commands (SBC) standard a device's block > size is "almost always greater than one byte and may be a multiple of > 512 bytes". QEMU currently has a 512 byte minimum block size because > the block layer functions work at that granularity. Furthermore, the > block size should be a power of 2 because QEMU calculates bitmasks from > the value. > > Introduce a "blocksize" property type so devices can enforce these > constraints on block size values. If the constraints are relaxed in the > future then this property can be updated. > > Introduce the new PropertyValueNotPowerOf2 QError so QMP clients know > exactly why a block size value was rejected. > > Signed-off-by: Stefan Hajnoczi > --- > hw/qdev-properties.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > hw/qdev.h | 3 +++ > qerror.c | 5 +++++ > qerror.h | 4 ++++ > 4 files changed, 58 insertions(+), 0 deletions(-) > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index bff9152..98dd06a 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -877,6 +877,52 @@ PropertyInfo qdev_prop_pci_devfn = { > .max = 0xFFFFFFFFULL, > }; > > +/* --- blocksize --- */ > + > +static void set_blocksize(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > + DeviceState *dev = DEVICE(obj); > + Property *prop = opaque; > + int16_t *ptr = qdev_get_prop_ptr(dev, prop); > + Error *local_err = NULL; > + int64_t value; > + > + if (dev->state != DEV_STATE_CREATED) { > + error_set(errp, QERR_PERMISSION_DENIED); > + return; > + } > + > + visit_type_int(v, &value, name, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + if (value < prop->info->min || value > prop->info->max) { > + error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, > + dev->id?:"", name, value, prop->info->min, > + prop->info->max); > + return; > + } > + > + /* We rely on power-of-2 blocksizes for bitmasks */ > + if ((value & (value - 1)) != 0) { > + error_set(errp, QERR_PROPERTY_VALUE_NOT_POWER_OF_2, > + dev->id?:"", name, value); > + return; > + } > + > + *ptr = value; > +} > + > +PropertyInfo qdev_prop_blocksize = { > + .name = "blocksize", > + .get = get_int16, > + .set = set_blocksize, > + .min = 512, > + .max = 65024, > +}; > + > /* --- public helpers --- */ > > static Property *qdev_prop_walk(Property *props, const char *name) > diff --git a/hw/qdev.h b/hw/qdev.h > index 9cc3f98..f8a5ddf 100644 > --- a/hw/qdev.h > +++ b/hw/qdev.h > @@ -223,6 +223,7 @@ extern PropertyInfo qdev_prop_drive; > extern PropertyInfo qdev_prop_netdev; > extern PropertyInfo qdev_prop_vlan; > extern PropertyInfo qdev_prop_pci_devfn; > +extern PropertyInfo qdev_prop_blocksize; > > #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \ > .name = (_name), \ > @@ -284,6 +285,8 @@ extern PropertyInfo qdev_prop_pci_devfn; > #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \ > DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_losttickpolicy, \ > LostTickPolicy) > +#define DEFINE_PROP_BLOCKSIZE(_n, _s, _f, _d) \ > + DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t) > > #define DEFINE_PROP_END_OF_LIST() \ > {} > diff --git a/qerror.c b/qerror.c > index ecc5259..75c9a5c 100644 > --- a/qerror.c > +++ b/qerror.c > @@ -233,6 +233,11 @@ static const QErrorStringTable qerror_table[] = { > .desc = "Property '%(device).%(property)' can't find value '%(value)'", > }, > { > + .error_fmt = QERR_PROPERTY_VALUE_NOT_POWER_OF_2, > + .desc = "Property '%(device).%(property)' doesn't take " > + "value '%(value)', it's not a power of 2", > + }, > + { > .error_fmt = QERR_PROPERTY_VALUE_OUT_OF_RANGE, > .desc = "Property '%(device).%(property)' doesn't take " > "value %(value) (minimum: %(min), maximum: %(max))", > diff --git a/qerror.h b/qerror.h > index e26c635..a3b9a59 100644 > --- a/qerror.h > +++ b/qerror.h > @@ -196,6 +196,10 @@ QError *qobject_to_qerror(const QObject *obj); > #define QERR_PROPERTY_VALUE_NOT_FOUND \ > "{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }" > > +#define QERR_PROPERTY_VALUE_NOT_POWER_OF_2 \ > + "{ 'class': 'PropertyValueNotPowerOf2', 'data': { " \ > + "'device': %s, 'property': %s, 'value': %"PRId64" } }" > + > #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \ > "{ 'class': 'PropertyValueOutOfRange', 'data': { 'device': %s, 'property': %s, 'value': %"PRId64", 'min': %"PRId64", 'max': %"PRId64" } }" > Reviewed-by: Paolo Bonzini Paolo