From patchwork Mon Nov 7 05:08:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Haozhong Zhang X-Patchwork-Id: 9414273 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id AFFB560585 for ; Mon, 7 Nov 2016 05:13:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A414128CEF for ; Mon, 7 Nov 2016 05:13:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 97DBE28D42; Mon, 7 Nov 2016 05:13:21 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9CA1728CEF for ; Mon, 7 Nov 2016 05:13:20 +0000 (UTC) Received: from localhost ([::1]:51197 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c3cFC-0000Td-TN for patchwork-qemu-devel@patchwork.kernel.org; Mon, 07 Nov 2016 00:13:18 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50981) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c3cBj-00064z-13 for qemu-devel@nongnu.org; Mon, 07 Nov 2016 00:09:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c3cBh-0001Y0-7l for qemu-devel@nongnu.org; Mon, 07 Nov 2016 00:09:42 -0500 Received: from mga02.intel.com ([134.134.136.20]:6606) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c3cBg-0001Ul-RM for qemu-devel@nongnu.org; Mon, 07 Nov 2016 00:09:41 -0500 Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 06 Nov 2016 21:09:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,604,1473145200"; d="scan'208";a="458355" Received: from hz-desktop.sh.intel.com (HELO localhost) ([10.239.159.148]) by orsmga002.jf.intel.com with ESMTP; 06 Nov 2016 21:09:38 -0800 From: Haozhong Zhang To: qemu-devel@nongnu.org Date: Mon, 7 Nov 2016 13:08:58 +0800 Message-Id: <20161107050859.31058-3-haozhong.zhang@intel.com> X-Mailer: git-send-email 2.10.1 In-Reply-To: <20161107050859.31058-1-haozhong.zhang@intel.com> References: <20161107050859.31058-1-haozhong.zhang@intel.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 134.134.136.20 Subject: [Qemu-devel] [PATCH 2/3] exec.c: move file operations to qemu_ram_alloc_from_file() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Haozhong Zhang , Eduardo Habkost , Peter Crosthwaite , Paolo Bonzini , Igor Mammedov , Richard Henderson Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Move operations that open/close/get size of the backend file from file_ram_alloc() to its caller qemu_ram_alloc_from_file(). After this change, check of backend file size and memory size can be put in qemu_ram_alloc_from_file(). Signed-off-by: Haozhong Zhang --- exec.c | 218 +++++++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 123 insertions(+), 95 deletions(-) diff --git a/exec.c b/exec.c index 68b0c92..42045aa 100644 --- a/exec.c +++ b/exec.c @@ -1231,27 +1231,13 @@ 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, + int fd, + bool truncate, Error **errp) { - bool unlink_on_error = false; - char *filename; - char *sanitized_name; - char *c; void *area = MAP_FAILED; - int fd = -1; - int64_t file_size; if (kvm_enabled() && !kvm_has_sync_mmu()) { error_setg(errp, @@ -1259,53 +1245,6 @@ static void *file_ram_alloc(RAMBlock *block, return NULL; } - for (;;) { - fd = open(path, O_RDWR); - if (fd >= 0) { - /* @path names an existing file, use it */ - break; - } - if (errno == ENOENT) { - /* @path names a file that doesn't exist, create it */ - fd = open(path, O_RDWR | O_CREAT | O_EXCL, 0644); - if (fd >= 0) { - unlink_on_error = true; - break; - } - } else if (errno == EISDIR) { - /* @path names a directory, create a file there */ - /* Make name safe to use with mkstemp by replacing '/' with '_'. */ - sanitized_name = g_strdup(memory_region_name(block->mr)); - for (c = sanitized_name; *c != '\0'; c++) { - if (*c == '/') { - *c = '_'; - } - } - - filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path, - sanitized_name); - g_free(sanitized_name); - - fd = mkstemp(filename); - if (fd >= 0) { - unlink(filename); - g_free(filename); - break; - } - g_free(filename); - } - if (errno != EEXIST && errno != EINTR) { - error_setg_errno(errp, errno, - "can't open backing store %s for guest RAM", - path); - goto error; - } - /* - * Try again on EINTR and EEXIST. The latter happens when - * something else creates the file between our two open(). - */ - } - block->page_size = qemu_fd_getpagesize(fd); block->mr->align = block->page_size; #if defined(__s390x__) @@ -1314,11 +1253,6 @@ static void *file_ram_alloc(RAMBlock *block, } #endif - /* If QEMU fails to get the backend file size, i.e. file_size < 0, - * it will treat the file as non-empty and not truncate it. - */ - 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", @@ -1326,13 +1260,6 @@ static void *file_ram_alloc(RAMBlock *block, goto error; } - if (file_size > 0 && file_size < memory) { - error_setg(errp, "backing store %s size 0x%" PRIx64 - " does not match 'size' option 0x" RAM_ADDR_FMT, - path, file_size, memory); - goto error; - } - memory = ROUND_UP(memory, block->page_size); /* @@ -1340,16 +1267,8 @@ 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 (!file_size && ftruncate(fd, memory)) { + if (truncate && ftruncate(fd, memory)) { perror("ftruncate"); } @@ -1375,12 +1294,6 @@ error: if (area != MAP_FAILED) { qemu_ram_munmap(area, memory); } - if (unlink_on_error) { - unlink(path); - } - if (fd != -1) { - close(fd); - } return NULL; } #endif @@ -1674,11 +1587,95 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) } #ifdef __linux__ +/* + * Open the backend file @path and return the file descriptor on + * success. + * + * If @path does not exist, it will be created and true will be + * returned via @unlink_on_error. + * + * If @path is a directory, a temporal file will be created under + * @path and its file descriptor will be returned. + * + * @errp will be set on any errors. + */ +static int backend_file_open(const char *path, const char *name, + bool *unlink_on_error, Error **errp) +{ + char *filename; + char *sanitized_name; + char *c; + int fd = -1; + Error *local_err = NULL; + + for (;;) { + fd = open(path, O_RDWR); + if (fd >= 0) { + /* @path names an existing file, use it */ + break; + } + if (errno == ENOENT) { + /* @path names a file that doesn't exist, create it */ + fd = open(path, O_RDWR | O_CREAT | O_EXCL, 0644); + if (fd >= 0) { + *unlink_on_error = true; + break; + } + } else if (errno == EISDIR) { + /* @path names a directory, create a file there */ + /* Make name safe to use with mkstemp by replacing '/' with '_'. */ + sanitized_name = g_strdup(name); + for (c = sanitized_name; *c != '\0'; c++) { + if (*c == '/') { + *c = '_'; + } + } + + filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path, + sanitized_name); + g_free(sanitized_name); + + fd = mkstemp(filename); + if (fd >= 0) { + unlink(filename); + g_free(filename); + break; + } + g_free(filename); + } + if (errno != EEXIST && errno != EINTR) { + error_setg_errno(&local_err, errno, + "can't open backing store %s for guest RAM", + path); + break; + } + /* + * Try again on EINTR and EEXIST. The latter happens when + * something else creates the file between our two open(). + */ + } + + error_propagate(errp, local_err); + return fd; +} + +static int64_t get_file_size(int fd) +{ + int64_t size = lseek(fd, 0, SEEK_END); + if (size < 0) { + return -errno; + } + return size; +} + RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, bool share, const char *mem_path, Error **errp) { - RAMBlock *new_block; + RAMBlock *new_block = NULL; + int fd = -1; + bool unlink_on_error = false; + int64_t file_size; Error *local_err = NULL; if (xen_enabled()) { @@ -1697,22 +1694,53 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, return NULL; } + fd = backend_file_open(mem_path, memory_region_name(mr), &unlink_on_error, + &local_err); + if (fd < 0) { + goto out; + } + /* If QEMU fails to get the backend file size, i.e. file_size < 0, + * it will treat the file as non-empty and not truncate it. + */ + file_size = get_file_size(fd); + size = HOST_PAGE_ALIGN(size); + if (file_size > 0 && file_size < size) { + error_setg(&local_err, "backing store %s size 0x%"PRIx64 + " does not match 'size' option 0x"RAM_ADDR_FMT, + mem_path, file_size, size); + goto out; + } + new_block = g_malloc0(sizeof(*new_block)); new_block->mr = mr; new_block->used_length = size; new_block->max_length = size; new_block->flags = share ? RAM_SHARED : 0; - new_block->host = file_ram_alloc(new_block, size, - mem_path, errp); + /* Do not truncate the non-empty backend file (i.e. file_size != 0) + * 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. + */ + new_block->host = file_ram_alloc(new_block, size, fd, !file_size, errp); if (!new_block->host) { - g_free(new_block); - return NULL; + goto out; } ram_block_add(new_block, &local_err); + + out: if (local_err) { g_free(new_block); + if (unlink_on_error) { + unlink(mem_path); + } + if (fd >= 0) { + close(fd); + } error_propagate(errp, local_err); return NULL; }