From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47353) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1HAQ-00079K-H2 for qemu-devel@nongnu.org; Mon, 31 Oct 2016 14:18:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1HAL-0006mh-F9 for qemu-devel@nongnu.org; Mon, 31 Oct 2016 14:18:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37264) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1HAL-0006lV-9m for qemu-devel@nongnu.org; Mon, 31 Oct 2016 14:18:37 -0400 Date: Mon, 31 Oct 2016 16:18:34 -0200 From: Eduardo Habkost Message-ID: <20161031181834.GA25480@thinpad.lan.raisama.net> References: <20161027042300.5929-1-haozhong.zhang@intel.com> <20161027042300.5929-4-haozhong.zhang@intel.com> <20161027145522.GK5057@thinpad.lan.raisama.net> <20161028020640.6dlpah37fkyxpr3u@hz-desktop> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161028020640.6dlpah37fkyxpr3u@hz-desktop> Subject: Re: [Qemu-devel] [PATCH v2 3/3] hostmem-file: make option 'size' optional List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, Igor Mammedov , Paolo Bonzini , Peter Crosthwaite , Richard Henderson On Fri, Oct 28, 2016 at 10:06:40AM +0800, Haozhong Zhang wrote: [...] > > > diff --git a/exec.c b/exec.c > > > index 264a25f..89065bd 100644 > > > --- a/exec.c > > > +++ b/exec.c > > > @@ -1234,7 +1234,7 @@ static int64_t get_file_size(int fd) > > > } > > > > > > static void *file_ram_alloc(RAMBlock *block, > > > - ram_addr_t memory, > > > + ram_addr_t *memory, > > > const char *path, > > > Error **errp) > > > { > > > @@ -1245,6 +1245,7 @@ static void *file_ram_alloc(RAMBlock *block, > > > void *area = MAP_FAILED; > > > int fd = -1; > > > int64_t file_size; > > > + ram_addr_t mem_size = *memory; > > > > > > if (kvm_enabled() && !kvm_has_sync_mmu()) { > > > error_setg(errp, > > > @@ -1309,21 +1310,27 @@ static void *file_ram_alloc(RAMBlock *block, > > > > > > file_size = get_file_size(fd); > > > > > > - if (memory < block->page_size) { > > > + if (!mem_size && file_size > 0) { > > > + mem_size = file_size; > > > > Maybe we should set *memory here and not below? > > > > Qemu currently sets the memory region size to the file size, and block > length to the aligned file size, so the code here can be changed as below: > > memory_region_set_size(block->mr, mem_size); > mem_size = HOST_PAGE_ALIGN(mem_size); > *memory = mem_size; > > The second line is necessary because Qemu currently passes the aligned > file size to file_ram_alloc(). That would duplicate the existing HOST_PAGE_ALIGN logic from qemu_ram_alloc_from_file(), won't it? I believe that's yet another reason to check file size before initializing the memory region, instead of initializing it first, and fixing up its size later. -- Eduardo