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

[Eric Blake]

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?


> +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.

> +        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?

> +        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.

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

signature.asc
Description: OpenPGP digital signature