On Thu, 25 Apr 2013 17:36:45 -0300 Eduardo Habkost <ehabk...@redhat.com> wrote:
> On Thu, Apr 25, 2013 at 04:05:26PM +0200, Igor Mammedov wrote: > [...] > > +static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque, > > + const char *name, Error **errp) > > +{ > > + X86CPU *cpu = X86_CPU(obj); > > + const int64_t min = 0; > > + const int64_t max = UINT32_MAX; > > + Error *error = NULL; > > + int64_t value; > > + > > + visit_type_int(v, &value, name, &error); > > + if (error) { > > + error_propagate(errp, error); > > + return; > > + } > > + if (value < min || value > max) { > > + error_setg(&error, "Property %s.%s doesn't take value %" PRId64 > > + " (minimum: %" PRId64 ", maximum: %" PRId64 ")" , > > + object_get_typename(obj), name, value, min, max); > > + error_propagate(errp, error); > > + return; > > + } > > Why you copied and pasted the string from > QERR_PROPERTY_VALUE_OUT_OF_RANGE, instead of simply using the define > like in the other property setters? it's designed to work with error_set(), not with error_setg(). > > The rest of the patch looks good to me. > > -- > Eduardo > -- Regards, Igor