Patchwork [v2,2/3] qdev: add blocksize property type

login
register
mail settings
Submitter Stefan Hajnoczi
Date March 14, 2012, 3:57 p.m.
Message ID <1331740626-5053-3-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/146656/
State New
Headers show

Comments

Stefan Hajnoczi - March 14, 2012, 3:57 p.m.
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 <stefanha@linux.vnet.ibm.com>
---
 hw/qdev-properties.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h            |    3 +++
 qerror.c             |    5 +++++
 qerror.h             |    4 ++++
 4 files changed, 58 insertions(+), 0 deletions(-)
Paolo Bonzini - March 14, 2012, 4:04 p.m.
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 <stefanha@linux.vnet.ibm.com>
> ---
>  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 <pbonzini@redhat.com>

Paolo

Patch

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" } }"