Re: [Qemu-devel] [PATCH 01/10] qdev: add qdev property for bool type

Re: [Qemu-devel] [PATCH 01/10] qdev: add qdev property for bool type

[Andreas Färber]

Am 25.02.2013 02:03, schrieb Igor Mammedov:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

I vaguely remember having written something like this long time ago for
ISA and for Vasilis... looks good except for minor nits.

> ---
>  hw/qdev-properties.c |   33 +++++++++++++++++++++++++++++++++
>  hw/qdev-properties.h |   10 ++++++++++
>  2 files changed, 43 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index a8a31f5..16ac814 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -106,6 +106,39 @@ PropertyInfo qdev_prop_bit = {
>      .set   = set_bit,
>  };
>  
> +/* --- bool --- */
> +
> +static void get_bool(Object *obj, Visitor *v, void *opaque,
> +                     const char *name, Error **errp)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    Property *prop = opaque;
> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
> +
> +    visit_type_bool(v, ptr, name, errp);
> +}
> +
> +static void set_bool(Object *obj, Visitor *v, void *opaque,
> +                       const char *name, Error **errp)

Indentation is off.

> +{
> +    DeviceState *dev = DEVICE(obj);
> +    Property *prop = opaque;
> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
> +
> +    if (dev->realized) {
> +        error_setg(errp, "Insufficient permission to perform this operation");

Should we be more specific here?
E.g., "Property '%s' cannot be changed once realized."

Andreas

> +        return;
> +    }
> +
> +    visit_type_bool(v, ptr, name, errp);
> +}
> +
> +PropertyInfo qdev_prop_bool = {
> +    .name  = "boolean",
> +    .get   = get_bool,
> +    .set   = set_bool,
> +};
> +
>  /* --- 8bit integer --- */
>  
>  static void get_uint8(Object *obj, Visitor *v, void *opaque,
> diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h
> index 20c67f3..3915f7c 100644
> --- a/hw/qdev-properties.h
> +++ b/hw/qdev-properties.h
> @@ -6,6 +6,7 @@
>  /*** qdev-properties.c ***/
>  
>  extern PropertyInfo qdev_prop_bit;
> +extern PropertyInfo qdev_prop_bool;
>  extern PropertyInfo qdev_prop_uint8;
>  extern PropertyInfo qdev_prop_uint16;
>  extern PropertyInfo qdev_prop_uint32;
> @@ -51,6 +52,15 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
>          .defval    = (bool)_defval,                              \
>          }
>  
> +#define DEFINE_PROP_BOOL(_name, _state, _field, _defval) {       \
> +        .name      = (_name),                                    \
> +        .info      = &(qdev_prop_bool),                          \
> +        .offset    = offsetof(_state, _field)                    \
> +            + type_check(bool, typeof_field(_state, _field)),    \
> +        .qtype     = QTYPE_QBOOL,                                \
> +        .defval    = (bool)_defval,                              \
> +        }
> +
>  #define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
>      DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
>  #define DEFINE_PROP_UINT16(_n, _s, _f, _d)                      \
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 01/10 v2] qdev: add qdev property for bool type

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  - fixed indentation
  - replaced error message. Suggested-By: Andreas Färber <afaerber@suse.de>
---
 hw/qdev-properties.c |   33 +++++++++++++++++++++++++++++++++
 hw/qdev-properties.h |   10 ++++++++++
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index a8a31f5..b0f2333 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -106,6 +106,39 @@ PropertyInfo qdev_prop_bit = {
     .set   = set_bit,
 };
 
+/* --- bool --- */
+
+static void get_bool(Object *obj, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    bool *ptr = qdev_get_prop_ptr(dev, prop);
+
+    visit_type_bool(v, ptr, name, errp);
+}
+
+static void set_bool(Object *obj, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    bool *ptr = qdev_get_prop_ptr(dev, prop);
+
+    if (dev->realized) {
+        error_setg(errp, "Property '%s' cannot be changed once realized", name);
+        return;
+    }
+
+    visit_type_bool(v, ptr, name, errp);
+}
+
+PropertyInfo qdev_prop_bool = {
+    .name  = "boolean",
+    .get   = get_bool,
+    .set   = set_bool,
+};
+
 /* --- 8bit integer --- */
 
 static void get_uint8(Object *obj, Visitor *v, void *opaque,
diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h
index 20c67f3..3915f7c 100644
--- a/hw/qdev-properties.h
+++ b/hw/qdev-properties.h
@@ -6,6 +6,7 @@
 /*** qdev-properties.c ***/
 
 extern PropertyInfo qdev_prop_bit;
+extern PropertyInfo qdev_prop_bool;
 extern PropertyInfo qdev_prop_uint8;
 extern PropertyInfo qdev_prop_uint16;
 extern PropertyInfo qdev_prop_uint32;
@@ -51,6 +52,15 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
         .defval    = (bool)_defval,                              \
         }
 
+#define DEFINE_PROP_BOOL(_name, _state, _field, _defval) {       \
+        .name      = (_name),                                    \
+        .info      = &(qdev_prop_bool),                          \
+        .offset    = offsetof(_state, _field)                    \
+            + type_check(bool, typeof_field(_state, _field)),    \
+        .qtype     = QTYPE_QBOOL,                                \
+        .defval    = (bool)_defval,                              \
+        }
+
 #define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
 #define DEFINE_PROP_UINT16(_n, _s, _f, _d)                      \
-- 
1.7.1

Re: [Qemu-devel] [PATCH 01/10 v2] qdev: add qdev property for bool type

[Andreas Färber]

Am 07.03.2013 17:16, schrieb Igor Mammedov:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
>   - fixed indentation
>   - replaced error message. Suggested-By: Andreas Färber <afaerber@suse.de>

Thanks, rebased onto PMM's array properties and applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 01/10 v2] qdev: add qdev property for bool type

[Igor Mammedov]

On Tue, 09 Apr 2013 15:56:57 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 07.03.2013 17:16, schrieb Igor Mammedov:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v2:
> >   - fixed indentation
> >   - replaced error message. Suggested-By: Andreas Färber
> > <afaerber@suse.de>
> 
> Thanks, rebased onto PMM's array properties and applied to qom-cpu:
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu
> 
> Andreas
> 

Thanks,

one more note could you fixup setter like Peter did in b000dfbd4,
or would you like me send a patch?

Re: [Qemu-devel] [PATCH 01/10 v2] qdev: add qdev property for bool type

[Andreas Färber]

Am 09.04.2013 16:13, schrieb Igor Mammedov:
> On Tue, 09 Apr 2013 15:56:57 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Am 07.03.2013 17:16, schrieb Igor Mammedov:
>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>> ---
>>> v2:
>>>   - fixed indentation
>>>   - replaced error message. Suggested-By: Andreas Färber
>>> <afaerber@suse.de>
>>
>> Thanks, rebased onto PMM's array properties and applied to qom-cpu:
>> https://github.com/afaerber/qemu-cpu/commits/qom-cpu
>>
>> Andreas
>>
> 
> Thanks,
> 
> one more note could you fixup setter like Peter did in b000dfbd4,
> or would you like me send a patch?

Thanks for pointing that out! Updated to use
qdev_prop_set_after_realized() and pushed.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH qom-cpu-next 00/10 v7] target-i386: convert CPU features into properties

[Igor Mammedov]

ping

Re: [Qemu-devel] [PATCH qom-cpu-next 00/10 v7] target-i386: convert CPU features into properties

[Andreas Färber]

Am 18.03.2013 13:31, schrieb Igor Mammedov:
> ping

Thanks. First day back at work here. :)

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 02/10] target-i386: cpu: convert existing dynamic properties into static properties

[Andreas Färber]

Am 25.02.2013 02:03, schrieb Igor Mammedov:
> Following properties are converted:
>     * vendor
>     * xlevel
>         * custom setter/getter replaced by qdev's DEFINE_PROP_UINT32
>     * level
>         * custom setter/getter replaced by qdev's DEFINE_PROP_UINT32
>     * tsc-frequency
>     * stepping
>     * model
>     * family
>     * model-id
>         * check "if (model_id == NULL)" looks unnecessary now, since all
>         builtin model-ids are not NULL and user shouldn't be able to set
>         it NULL (cpumodel string parsing code takes care of it, if feature
>         is specified as "model-id=" on command line, its parsing will
>         result in an empty string as value).
> 
> Common changes to all properties:
>     * string properties: changed function signature to conform to form used in
>       qdev-properties.c
> 
> * extra change is addition of feat2prop() helper to deal with properties
>   naming using '-' instead of '_'. Used in this patch for 'model_id' name
>   conversion and converting along the way 'hv-spinlocks', but will be
>   reused in following patches for "hv_*" and +-foo conversions as well.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> v2:
>   - removed s/error_set/error_setg/;s/QERR*/with similar message/
>   - made PropertyInfo static
>   - Left setters/getters atwher they have been, only signature change
>     + usin visitors where needed
>   - use g_malloc0() instead of g_malloc() in x86_cpuid_get_model_id()
> ---
>  target-i386/cpu.c |  174 ++++++++++++++++++++++++++++++++---------------------
>  1 files changed, 105 insertions(+), 69 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index dfcf86e..5626931 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
[...]
> @@ -1297,6 +1345,17 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
>      return -1;
>  }
>  
> +/* It converts all '_' in a feature string option name to '-', to make
> + * feature name to conform property naming rule which uses '-' instead of '_'
> + */
> +static inline void feat2prop(char *s)
> +{
> +    char *delimiter = strchr(s, '=');
> +    while ((s = strchr(s, '_')) && ((delimiter == NULL) || (s < delimiter))) {
> +        *s = '-';
> +    }
> +}
> +
>  /* Parse "+feature,-feature,feature=foo" CPU feature string
>   */
>  static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
> @@ -1318,6 +1377,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
>          } else if (featurestr[0] == '-') {
>              add_flagname_to_bitmaps(featurestr + 1, minus_features);
>          } else if ((val = strchr(featurestr, '='))) {
> +            feat2prop(featurestr);
>              *val = 0; val++;
>              if (!strcmp(featurestr, "family")) {
>                  object_property_parse(OBJECT(cpu), val, featurestr, errp);
> @@ -1345,9 +1405,9 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
>                  object_property_parse(OBJECT(cpu), num, featurestr, errp);
>              } else if (!strcmp(featurestr, "vendor")) {
>                  object_property_parse(OBJECT(cpu), val, featurestr, errp);
> -            } else if (!strcmp(featurestr, "model_id")) {
> -                object_property_parse(OBJECT(cpu), val, "model-id", errp);
> -            } else if (!strcmp(featurestr, "tsc_freq")) {
> +            } else if (!strcmp(featurestr, "model-id")) {
> +                object_property_parse(OBJECT(cpu), val, featurestr, errp);
> +            } else if (!strcmp(featurestr, "tsc-freq")) {
>                  int64_t tsc_freq;
>                  char *err;
>                  char num[32];
> @@ -1360,7 +1420,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
>                  }
>                  snprintf(num, sizeof(num), "%" PRId64, tsc_freq);
>                  object_property_parse(OBJECT(cpu), num, "tsc-frequency", errp);
> -            } else if (!strcmp(featurestr, "hv_spinlocks")) {
> +            } else if (!strcmp(featurestr, "hv-spinlocks")) {
>                  char *err;
>                  numvalue = strtoul(val, &err, 0);
>                  if (!*val || *err) {
[snip]

Thanks, split this part off and applied to qom-cpu (modified as below):

https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f34ba23..697848d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1307,6 +1307,16 @@ static int cpu_x86_find_by_name(x86_def_t
*x86_cpu_def, c
onst char *name)
     return -1;
 }

+/* Convert all '_' in a feature string option name to '-', to make feature
+ * name conform to QOM property naming rule, which uses '-' instead of '_'.
+ */
+static inline void feat2prop(char *s)
+{
+    while ((s = strchr(s, '_'))) {
+        *s = '-';
+    }
+}
+
 /* Parse "+feature,-feature,feature=foo" CPU feature string
  */
 static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error
**errp)
@@ -1329,6 +1339,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu,
char *fe
atures, Error **errp)
             add_flagname_to_bitmaps(featurestr + 1, minus_features);
         } else if ((val = strchr(featurestr, '='))) {
             *val = 0; val++;
+            feat2prop(featurestr);
             if (!strcmp(featurestr, "family")) {
                 object_property_parse(OBJECT(cpu), val, featurestr, errp);
             } else if (!strcmp(featurestr, "model")) {
@@ -1355,9 +1366,9 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu,
char *fe
atures, Error **errp)
                 object_property_parse(OBJECT(cpu), num, featurestr, errp);
             } else if (!strcmp(featurestr, "vendor")) {
                 object_property_parse(OBJECT(cpu), val, featurestr, errp);
-            } else if (!strcmp(featurestr, "model_id")) {
-                object_property_parse(OBJECT(cpu), val, "model-id", errp);
-            } else if (!strcmp(featurestr, "tsc_freq")) {
+            } else if (!strcmp(featurestr, "model-id")) {
+                object_property_parse(OBJECT(cpu), val, featurestr, errp);
+            } else if (!strcmp(featurestr, "tsc-freq")) {
                 int64_t tsc_freq;
                 char *err;
                 char num[32];
@@ -1370,7 +1381,7 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu,
char *features, Error **errp)
                 }
                 snprintf(num, sizeof(num), "%" PRId64, tsc_freq);
                 object_property_parse(OBJECT(cpu), num,
"tsc-frequency", errp);
-            } else if (!strcmp(featurestr, "hv_spinlocks")) {
+            } else if (!strcmp(featurestr, "hv-spinlocks")) {
                 char *err;
                 numvalue = strtoul(val, &err, 0);
                 if (!*val || *err) {


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg