Re: [Qemu-devel] [PULL 4/7] hostmem-file: add the 'pmem' option

[Peter Maydell]

On 20 August 2018 at 21:24, Michael S. Tsirkin <address@hidden> wrote:
> From: Junyan He <address@hidden>
>
> When QEMU emulates vNVDIMM labels and migrates vNVDIMM devices, it
> needs to know whether the backend storage is a real persistent memory,
> in order to decide whether special operations should be performed to
> ensure the data persistence.
>
> This boolean option 'pmem' allows users to specify whether the backend
> storage of memory-backend-file is a real persistent memory. If
> 'pmem=on', QEMU will set the flag RAM_PMEM in the RAM block of the
> corresponding memory region. If 'pmem' is set while lack of libpmem
> support, a error is generated.

Hi; Coverity reports (CID 1395184) that there is a resource leak
in an error-exit path in this function:

> +static void file_memory_backend_set_pmem(Object *o, bool value, Error **errp)
> +{
> +    HostMemoryBackend *backend = MEMORY_BACKEND(o);
> +    HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
> +
> +    if (host_memory_backend_mr_inited(backend)) {
> +        error_setg(errp, "cannot change property 'pmem' of %s '%s'",
> +                   object_get_typename(o),
> +                   object_get_canonical_path_component(o));
> +        return;
> +    }
> +
> +#ifndef CONFIG_LIBPMEM
> +    if (value) {
> +        Error *local_err = NULL;
> +        error_setg(&local_err,
> +                   "Lack of libpmem support while setting the 'pmem=on'"
> +                   " of %s '%s'. We can't ensure data persistence.",
> +                   object_get_typename(o),
> +                   object_get_canonical_path_component(o));

object_get_canonical_path_component() returns a string which
must be freed using g_free().

> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +#endif
> +
> +    fb->is_pmem = value;
> +}

thanks
-- PMM