[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: |
Yi Zhang |
Subject: |
Re: [Qemu-devel] [PULL 4/7] hostmem-file: add the 'pmem' option |
Date: |
Tue, 28 Aug 2018 23:42:57 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On 2018-08-24 at 20:14:37 +0300, Michael S. Tsirkin wrote:
> On Fri, Aug 24, 2018 at 05:57:06PM +0100, Peter Maydell wrote:
> > On 24 August 2018 at 17:53, Michael S. Tsirkin <address@hidden> wrote:
> > > On Fri, Aug 24, 2018 at 04:13:18PM +0100, Peter Maydell wrote:
> > >> object_get_canonical_path_component() returns a string which
> > >> must be freed using g_free().
> >
> > > 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;
> >
> > Yep, like that (though I would put the closing ");" on
> > the line with object_get_typename() and delete the trailing comma).
> >
> > thanks
> > -- PMM
>a
Ah.. Thanks Michael/Peter to identify/prepare this fix.
> oh i forgot to add in "path".
> I didn't build with libpmem installed
Maybe you already have libpmem installed yet,
it is *ifndef* CONFIG_LIBPMEM ^_^
> Should have been (still untested):
>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
>
>
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 2476dcb435..72e7055ee7 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));
> + path);
> + g_free(path);
> error_propagate(errp, local_err);
> return;
> }
>
Ah.. I saw another place still have the seem leak, on the previous few lines in
file_memory_backend_set_pmem, I will prepare another patch to fix the
leaks in these two place.
Regards
Yi.
- [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