From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56278) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bzlir-00008G-HA for qemu-devel@nongnu.org; Thu, 27 Oct 2016 10:32:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bzlim-0001AE-HY for qemu-devel@nongnu.org; Thu, 27 Oct 2016 10:32:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42358) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bzlim-00019z-A6 for qemu-devel@nongnu.org; Thu, 27 Oct 2016 10:31:56 -0400 Date: Thu, 27 Oct 2016 12:31:53 -0200 From: Eduardo Habkost Message-ID: <20161027143153.GI5057@thinpad.lan.raisama.net> References: <20161027042300.5929-1-haozhong.zhang@intel.com> <20161027042300.5929-2-haozhong.zhang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161027042300.5929-2-haozhong.zhang@intel.com> Subject: Re: [Qemu-devel] [PATCH v2 1/3] exec.c: do not truncate non-empty memory backend file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Haozhong Zhang Cc: qemu-devel@nongnu.org, Igor Mammedov , Paolo Bonzini , Peter Crosthwaite , Richard Henderson On Thu, Oct 27, 2016 at 12:22:58PM +0800, Haozhong Zhang wrote: > For '-object memory-backend-file,mem-path=foo,size=xyz', if the size of > file 'foo' does not match the given size 'xyz', the current QEMU will > truncate the file to the given size, which may corrupt the existing data > in that file. To avoid such data corruption, this patch disables > truncating non-empty backend files. > > Signed-off-by: Haozhong Zhang Reviewed-by: Eduardo Habkost But I would add comment near the get_file_size() call to indicate that not stopping on get_file_size() errors is on purpose and not a mistake. > --- > exec.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/exec.c b/exec.c > index 587b489..a2b371a 100644 > --- a/exec.c > +++ b/exec.c > @@ -1224,6 +1224,15 @@ void qemu_mutex_unlock_ramlist(void) > } > > #ifdef __linux__ > +static int64_t get_file_size(int fd) > +{ > + int64_t size = lseek(fd, 0, SEEK_END); > + if (size < 0) { > + return -errno; > + } > + return size; > +} > + > static void *file_ram_alloc(RAMBlock *block, > ram_addr_t memory, > const char *path, > @@ -1235,6 +1244,7 @@ static void *file_ram_alloc(RAMBlock *block, > char *c; > void *area = MAP_FAILED; > int fd = -1; > + int64_t file_size; > > if (kvm_enabled() && !kvm_has_sync_mmu()) { > error_setg(errp, > @@ -1297,6 +1307,8 @@ static void *file_ram_alloc(RAMBlock *block, > } > #endif > > + file_size = get_file_size(fd); > + > if (memory < block->page_size) { > error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " > "or larger than page size 0x%zx", > @@ -1311,8 +1323,16 @@ static void *file_ram_alloc(RAMBlock *block, > * hosts, so don't bother bailing out on errors. > * If anything goes wrong with it under other filesystems, > * mmap will fail. > + * > + * Do not truncate the non-empty backend file to avoid corrupting > + * the existing data in the file. Disabling shrinking is not > + * enough. For example, the current vNVDIMM implementation stores > + * the guest NVDIMM labels at the end of the backend file. If the > + * backend file is later extended, QEMU will not be able to find > + * those labels. Therefore, extending the non-empty backend file > + * is disabled as well. > */ > - if (ftruncate(fd, memory)) { > + if (!file_size && ftruncate(fd, memory)) { > perror("ftruncate"); > } > > -- > 2.10.1 > -- Eduardo