From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40328) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxE82-0003DK-1E for qemu-devel@nongnu.org; Thu, 20 Oct 2016 10:15:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxE7y-0006wN-1e for qemu-devel@nongnu.org; Thu, 20 Oct 2016 10:15:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48876) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bxE7x-0006w8-QU for qemu-devel@nongnu.org; Thu, 20 Oct 2016 10:15:25 -0400 Date: Thu, 20 Oct 2016 16:15:21 +0200 From: Igor Mammedov Message-ID: <20161020161521.6e20ce2a@nial.brq.redhat.com> In-Reply-To: <20161020135610.GY5057@thinpad.lan.raisama.net> References: <20161020061301.31372-1-haozhong.zhang@intel.com> <20161020143412.5ea6b564@nial.brq.redhat.com> <20161020131138.4gzxk5ekltkiqtq2@hz-desktop> <20161020154215.4b765284@nial.brq.redhat.com> <20161020135610.GY5057@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hostmem-file: add a property 'notrunc' to avoid data corruption List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Haozhong Zhang , kwolf@redhat.com, Xiao Guangrong , Peter Crosthwaite , qemu-devel@nongnu.org, mreitz@redhat.com, Paolo Bonzini , Richard Henderson On Thu, 20 Oct 2016 11:56:10 -0200 Eduardo Habkost wrote: > On Thu, Oct 20, 2016 at 03:42:15PM +0200, Igor Mammedov wrote: > > On Thu, 20 Oct 2016 21:11:38 +0800 > > Haozhong Zhang wrote: > > > > > On 10/20/16 14:34 +0200, Igor Mammedov wrote: > > > >On Thu, 20 Oct 2016 14:13:01 +0800 > > > >Haozhong Zhang wrote: > > > > > > > >> If a file is used as the backend of memory-backend-file and its size is > > > >> not identical to the property 'size', the file will be truncated. For a > > > >> file used as the backend of vNVDIMM, its data is expected to be > > > >> persistent and the truncation may corrupt the existing data. > > > >I wonder if it's possible just skip 'size' property in your case instead > > > >'notrunc' property. That way if size is not present one'd get actual size > > > >using get_file_size() and set 'size' to it? > > > >And if 'size' is provided and 'size' != file_size then error out. > > > > > > > > > > I don't know how this can be implemented in QEMU. Specially, how does > > > the memory-backend-file know it's used for vNVDIMM, so that it can > > > skip the 'size' property? > > Does memory-backend-file needs to know that it's used by NVDIMM? > > Looking at nvdimm_realize it doesn't as it's assumes > > hostemem_size == pmem_size + label_size > > > > I'd make hostmem_file.size optional and take size from file > > and if 'size' is specified explictly require it to mach file size. > > It's generic and has nothing to do with nvdimm. > > We can take size from file, or take size from the > host_memory_backend_get_memory() callers. > > Enumerating all sizes that QEMU can use as input: > > A) Backend file size > B) memory backend "size" option > C) frontend-provided size (-numa size, -m, or pc-dimm "size" > property) -numa size affect only anon memory not backend backed one, for backend baked memory we use memdev where size comes from backend pc-dimm.size is readonly and isn't supposed to influence backend.size I'd drop C option > > My suggestion is: > * B should be optional. > * If B is omitted, we should never truncate the file to a smaller > size. i.e. derive backend.size from filesize if possible (i.e. not hugepages) > * If B is omitted, we can use C as the size when mapping the > file. frontend size is the size that's mapped into guest address space. it should not influence backend's size in backward direction. You may notice pc-dimm.size is not user settable (readonly) property. > * If B is omitted, and C > A, maybe we could use ftruncate() to > extend the file to make users happy. But I'm not sure we > should (I think B should be the only option that cause > truncation). > * If we want to make C optional on some cases, we could use A if > B is omitted. we shouldn't use C to manage backends behavior But I have a question why do we have truncation at all in place now. > > Does that make sense? >