From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ftFOa-00076f-NO for qemu-devel@nongnu.org; Fri, 24 Aug 2018 12:57:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ftFKk-0002Lz-AW for qemu-devel@nongnu.org; Fri, 24 Aug 2018 12:53:19 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:51392 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ftFKk-0002K7-2r for qemu-devel@nongnu.org; Fri, 24 Aug 2018 12:53:14 -0400 Date: Fri, 24 Aug 2018 19:53:09 +0300 From: "Michael S. Tsirkin" Message-ID: <20180824195231-mutt-send-email-mst@kernel.org> References: <20180820104045.133968-1-mst@redhat.com> <20180820104045.133968-5-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PULL 4/7] hostmem-file: add the 'pmem' option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers , Junyan He , Haozhong Zhang , Stefan Hajnoczi , Igor Mammedov , Richard Henderson , Eduardo Habkost , Paolo Bonzini , Peter Crosthwaite , Richard Henderson On Fri, Aug 24, 2018 at 04:13:18PM +0100, Peter Maydell wrote: > On 20 August 2018 at 21:24, Michael S. Tsirkin wrote: > > From: Junyan He > > > > 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 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; }