Re: [Qemu-devel] [PATCH v3 05/34] add memdev backend infrastructure

[Igor Mammedov]

On Thu, 29 May 2014 10:41:32 -0600
Eric Blake <address@hidden> wrote:

> On 05/27/2014 07:01 AM, Igor Mammedov wrote:
> > Provides framework for splitting host RAM allocation/
> > policies into a separate backend that could be used
> > by devices.
> > 
> > Initially only legacy RAM backend is provided, which
> > uses memory_region_init_ram() allocator and compatible
> > with every CLI option that affects memory_region_init_ram().
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > Signed-off-by: Paolo Bonzini <address@hidden>
> > ---
> > v4:
> >  - don't use nonexisting anymore error_is_set()
> > v3:
> >  - fix path leak & use object_get_canonical_path_component()
> >    for getting object name
> > v2:
> >  - reuse UserCreatable interface instead of custom callbacks
> > ---
> 
> > +++ b/backends/hostmem-ram.c
> > @@ -0,0 +1,54 @@
> > +/*
> > + * QEMU Host Memory Backend
> > + *
> > + * Copyright (C) 2013 Red Hat Inc
> 
> This patch has been in the queue for a while.  Is it worth listing
> 2013-2014 here and in other new files?
ok

> 
> > +hostmemory_backend_set_size(Object *obj, Visitor *v, void *opaque,
> > +                            const char *name, Error **errp)
> > +{
> > +    HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> > +    Error *local_err = NULL;
> > +    uint64_t value;
> > +
> > +    if (memory_region_size(&backend->mr)) {
> > +        error_setg(&local_err, "cannot change property value\n");
> 
> Error messages should not include trailing newline.
ok

> 
> > +        goto out;
> > +    }
> > +
> > +    visit_type_size(v, &value, name, errp);
> > +    if (local_err) {
> 
> local_err is guaranteed NULL at this point.  Or did you mean to pass
> &local_err instead of errp to visit_type_size?
yep, visit_type_size() should take &local_err

> 
> > +        goto out;
> > +    }
> > +    if (!value) {
> > +        error_setg(&local_err, "Property '%s.%s' doesn't take value '%"
> > +                   PRIu64 "'", object_get_typename(obj), name , value);
> 
> No space before comma.
there is no space before comma and there shouldn't be one since it's an end of
an argument. 

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


-- 
Regards,
  Igor