[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 4/7] hostmem-file: add the 'pmem' option
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PULL 4/7] hostmem-file: add the 'pmem' option |
Date: |
Fri, 24 Aug 2018 19:53:09 +0300 |
On Fri, Aug 24, 2018 at 04:13:18PM +0100, Peter Maydell wrote:
> 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
Like the following? Junyan, could you pls try this one and confirm?
Signed-off-by: Michael S. Tsirkin <address@hidden>
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 2476dcb435..d88125b86e 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -154,11 +154,14 @@ static void file_memory_backend_set_pmem(Object *o, bool
value, Error **errp)
#ifndef CONFIG_LIBPMEM
if (value) {
Error *local_err = NULL;
+ char *path = object_get_canonical_path_component(o);
+
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));
+ );
+ g_free(path);
error_propagate(errp, local_err);
return;
}
- [Qemu-devel] [PULL 0/7] pc: fixes, Michael S. Tsirkin, 2018/08/20
- [Qemu-devel] [PULL 1/7] memory, exec: Expose all memory block related flags., Michael S. Tsirkin, 2018/08/20
- [Qemu-devel] [PULL 3/7] configure: add libpmem support, Michael S. Tsirkin, 2018/08/20
- [Qemu-devel] [PULL 2/7] memory, exec: switch file ram allocation functions to 'flags' parameters, Michael S. Tsirkin, 2018/08/20
- [Qemu-devel] [PULL 4/7] hostmem-file: add the 'pmem' option, Michael S. Tsirkin, 2018/08/20
[Qemu-devel] [PULL 5/7] mem/nvdimm: ensure write persistence to PMEM in label emulation, Michael S. Tsirkin, 2018/08/20
[Qemu-devel] [PULL 6/7] migration/ram: Add check and info message to nvdimm post copy., Michael S. Tsirkin, 2018/08/20
[Qemu-devel] [PULL 7/7] migration/ram: ensure write persistence on loading all data to PMEM., Michael S. Tsirkin, 2018/08/20
Re: [Qemu-devel] [PULL 0/7] pc: fixes, Peter Maydell, 2018/08/21