* [Qemu-devel] [PATCH 2/3] exec.c: move file operations to qemu_ram_alloc_from_file()
2016-11-07 5:08 [Qemu-devel] [PATCH 0/3] hostmem-file: make option 'size' optional Haozhong Zhang
2016-11-07 5:08 ` [Qemu-devel] [PATCH 1/3] exec.c: add comment for errors of get_file_size() Haozhong Zhang
@ 2016-11-07 5:08 ` Haozhong Zhang
2016-11-07 5:08 ` [Qemu-devel] [PATCH 3/3] hostmem-file: make option 'size' optional Haozhong Zhang
2016-11-07 7:52 ` [Qemu-devel] [PATCH 0/3] " Markus Armbruster
3 siblings, 0 replies; 6+ messages in thread
From: Haozhong Zhang @ 2016-11-07 5:08 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Igor Mammedov, Paolo Bonzini, Peter Crosthwaite,
Richard Henderson, Haozhong Zhang
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 <haozhong.zhang@intel.com>
---
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;
}
--
2.10.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH 3/3] hostmem-file: make option 'size' optional
2016-11-07 5:08 [Qemu-devel] [PATCH 0/3] hostmem-file: make option 'size' optional Haozhong Zhang
2016-11-07 5:08 ` [Qemu-devel] [PATCH 1/3] exec.c: add comment for errors of get_file_size() Haozhong Zhang
2016-11-07 5:08 ` [Qemu-devel] [PATCH 2/3] exec.c: move file operations to qemu_ram_alloc_from_file() Haozhong Zhang
@ 2016-11-07 5:08 ` Haozhong Zhang
2016-11-07 7:52 ` [Qemu-devel] [PATCH 0/3] " Markus Armbruster
3 siblings, 0 replies; 6+ messages in thread
From: Haozhong Zhang @ 2016-11-07 5:08 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Igor Mammedov, Paolo Bonzini, Peter Crosthwaite,
Richard Henderson, Haozhong Zhang
If 'size' option is not specified, QEMU will use the file size of
'mem-path' instead. If an empty file, a non-existing file or a directory
is specified by option 'mem-path', a non-zero option 'size' is still
needed.
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
backends/hostmem-file.c | 28 ++++++++++++++++++++--------
exec.c | 27 +++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 42efb2f..56cc96b 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -39,17 +39,14 @@ static void
file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
{
HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
+ Error *local_err = NULL;
- if (!backend->size) {
- error_setg(errp, "can't create backend with size 0");
- return;
- }
if (!fb->mem_path) {
- error_setg(errp, "mem-path property not set");
- return;
+ error_setg(&local_err, "mem-path property not set");
+ goto out;
}
#ifndef CONFIG_LINUX
- error_setg(errp, "-mem-path not supported on this host");
+ error_setg(&local_err, "-mem-path not supported on this host");
#else
if (!memory_region_size(&backend->mr)) {
gchar *path;
@@ -58,10 +55,25 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
path,
backend->size, fb->share,
- fb->mem_path, errp);
+ fb->mem_path, &local_err);
g_free(path);
+
+ if (local_err) {
+ goto out;
+ }
+
+ if (!backend->size) {
+ backend->size = memory_region_size(&backend->mr);
+ }
+ }
+
+ if (!backend->size) {
+ error_setg(&local_err, "can't create backend with size 0");
}
#endif
+
+ out:
+ error_propagate(errp, local_err);
}
static char *get_mem_path(Object *o, Error **errp)
diff --git a/exec.c b/exec.c
index 42045aa..b81c9a8 100644
--- a/exec.c
+++ b/exec.c
@@ -1676,6 +1676,7 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
int fd = -1;
bool unlink_on_error = false;
int64_t file_size;
+ uint64_t mr_size;
Error *local_err = NULL;
if (xen_enabled()) {
@@ -1704,6 +1705,32 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
*/
file_size = get_file_size(fd);
+ if (!size && file_size >= 0) {
+ if (!file_size) {
+ error_setg(&local_err,
+ "%s is empty or a directory, 'size' option must be specified",
+ mem_path);
+ goto out;
+ }
+
+ mr_size = memory_region_size(mr);
+ if (mr_size && mr_size != size) {
+ error_setg(&local_err, "cannot resize non-empty memory region");
+ goto out;
+ }
+ if (!mr_size) {
+ memory_region_set_size(mr, file_size);
+ }
+ size = file_size;
+ }
+
+ if (!size) {
+ error_setg(&local_err,
+ "cannot get size of %s, 'size' option must be specified",
+ mem_path);
+ goto out;
+ }
+
size = HOST_PAGE_ALIGN(size);
if (file_size > 0 && file_size < size) {
error_setg(&local_err, "backing store %s size 0x%"PRIx64
--
2.10.1
^ permalink raw reply related [flat|nested] 6+ messages in thread