Re: [Qemu-devel] [PATCH v8 0/5] implement vNVDIMM
On 16.11.2015 13:50, Xiao Guangrong wrote: NVDIMM (A Non-Volatile Dual In-line Memory Module) is going to be supported on Intel's platform. Hi. One question: do this mean, that your qemu emulated nvidimm - pmem solution will work only on Intel host? -- Best regards, Vladimir * now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 23/35] nvdimm: implement NVDIMM device abstract
On 02.11.2015 12:13, Xiao Guangrong wrote:
Introduce "nvdimm" device which is based on dimm device type
128K memory region which is the minimum namespace label size
required by NVDIMM Namespace Spec locates at the end of
backend memory device is reserved for label data
We can use "-m 1G,maxmem=100G,slots=10 -object memory-backend-file,
id=mem1,size=1G,mem-path=/dev/pmem0 -device nvdimm,memdev=mem1" to
create NVDIMM device for guest
Signed-off-by: Xiao Guangrong
---
default-configs/i386-softmmu.mak | 1 +
default-configs/x86_64-softmmu.mak | 1 +
hw/acpi/memory_hotplug.c | 6 ++
hw/mem/Makefile.objs | 1 +
hw/mem/nvdimm.c| 116 +
include/hw/mem/nvdimm.h| 83 ++
6 files changed, 208 insertions(+)
create mode 100644 hw/mem/nvdimm.c
create mode 100644 include/hw/mem/nvdimm.h
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 3ece8bb..4e84a1c 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -47,6 +47,7 @@ CONFIG_APIC=y
CONFIG_IOAPIC=y
CONFIG_PVPANIC=y
CONFIG_MEM_HOTPLUG=y
+CONFIG_NVDIMM=y
CONFIG_XIO3130=y
CONFIG_IOH3420=y
CONFIG_I82801B11=y
diff --git a/default-configs/x86_64-softmmu.mak
b/default-configs/x86_64-softmmu.mak
index 92ea7c1..e877a86 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -47,6 +47,7 @@ CONFIG_APIC=y
CONFIG_IOAPIC=y
CONFIG_PVPANIC=y
CONFIG_MEM_HOTPLUG=y
+CONFIG_NVDIMM=y
CONFIG_XIO3130=y
CONFIG_IOH3420=y
CONFIG_I82801B11=y
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 20d3093..bb5a29f 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -1,6 +1,7 @@
#include "hw/acpi/memory_hotplug.h"
#include "hw/acpi/pc-hotplug.h"
#include "hw/mem/dimm.h"
+#include "hw/mem/nvdimm.h"
#include "hw/boards.h"
#include "hw/qdev-core.h"
#include "trace.h"
@@ -231,6 +232,11 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq,
MemHotplugState *mem_st,
{
MemStatus *mdev;
+/* Currently, NVDIMM hotplug has not been supported yet. */
+if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
+return;
+}
+
hmm, should not check for DIMM_GET_CLASS(dev)->hotpluggable be used here
instead for more common case covering?
mdev = acpi_memory_slot_status(mem_st, dev, errp);
if (!mdev) {
return;
diff --git a/hw/mem/Makefile.objs b/hw/mem/Makefile.objs
index cebb4b1..12d9b72 100644
--- a/hw/mem/Makefile.objs
+++ b/hw/mem/Makefile.objs
@@ -1,2 +1,3 @@
common-obj-$(CONFIG_DIMM) += dimm.o
common-obj-$(CONFIG_MEM_HOTPLUG) += pc-dimm.o
+common-obj-$(CONFIG_NVDIMM) += nvdimm.o
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
new file mode 100644
index 000..c310887
--- /dev/null
+++ b/hw/mem/nvdimm.c
@@ -0,0 +1,116 @@
+/*
+ * Non-Volatile Dual In-line Memory Module Virtualization Implementation
+ *
+ * Copyright(C) 2015 Intel Corporation.
+ *
+ * Author:
+ * Xiao Guangrong
+ *
+ * Currently, it only supports PMEM Virtualization.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
Re: [PATCH v7 20/35] dimm: get mapped memory region from DIMMDeviceClass->get_memory_region
On 03.11.2015 17:47, Xiao Guangrong wrote: On 11/03/2015 12:16 AM, Vladimir Sementsov-Ogievskiy wrote: On 02.11.2015 18:06, Xiao Guangrong wrote: On 11/02/2015 10:26 PM, Vladimir Sementsov-Ogievskiy wrote: On 02.11.2015 16:08, Xiao Guangrong wrote: On 11/02/2015 08:19 PM, Vladimir Sementsov-Ogievskiy wrote: On 02.11.2015 12:13, Xiao Guangrong wrote: Curretly, the memory region of backed memory is directly mapped to guest's address space, however, it is not true for nvdimm device This patch let dimm device realize this fact and use DIMMDeviceClass->get_memory_region method to get the mapped memory region Current code did not check the return value of get_memory_region as it assumed the backend memory of pc-dimm is always properly initialized, we make get_memory_region internally catch the case if something is wrong but here you call not pc-dimm's get_memory_region, but common ddc->get_memory_region, which may be nvdimm or possibly other future dimm, so, why not check it here? And than pc_dimm_get_memory_region may be left untouched (error_abort is ok, because errp is unused). Hmm, because 'here' is not the only place calling ->get_memory_region, this method has multiple callers: $ git grep "\->get_memory_region" hw/i386/pc.c:MemoryRegion *mr = ddc->get_memory_region(dimm); hw/i386/pc.c:MemoryRegion *mr = ddc->get_memory_region(dimm); hw/mem/dimm.c:mr = ddc->get_memory_region(dimm); hw/mem/nvdimm.c:ddc->get_memory_region = nvdimm_get_memory_region; hw/mem/pc-dimm.c:ddc->get_memory_region = pc_dimm_get_memory_region; hw/ppc/spapr.c:MemoryRegion *mr = ddc->get_memory_region(dimm); memory region validation is also done for NVDIMM in nvdimm device. Ok, then it should be documented by a comment in dimm.h, where DIMMDeviceClass is defined, that this function should not fail Okay, how about this comment: /* * get the memory region which will be mapped into guest's address * space. It is called after dimm device realized so it is never * failed. */ MemoryRegion *(*get_memory_region)(DIMMDevice *dimm); if you don't mind: s/it is never failed/it should never fail and assumed to return valid not-NULL address I'll ok with this if others don't mind, but personally I prefer explicit error handling for such functions. -- Best regards, Vladimir * now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 13/35] hostmem-file: use whole file size if possible
On 02.11.2015 12:13, Xiao Guangrong wrote:
Use the whole file size if @size is not specified which is useful
if we want to directly pass a file to guest
Signed-off-by: Xiao Guangrong
---
backends/hostmem-file.c | 22 ++
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 9097a57..ea355c1 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -38,15 +38,29 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error
**errp)
{
HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
-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;
}
+if (!backend->size) {
+Error *local_err = NULL;
+
+/*
+ * use the whole file size if @size is not specified.
+ */
+backend->size = qemu_file_getlength(fb->mem_path, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+}
+
+if (!backend->size) {
+error_setg(errp, "can't create backend on the file whose size is 0");
+return;
+}
+
backend->force_prealloc = mem_prealloc;
memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
object_get_canonical_path(OBJECT(backend)),
why not just
+if (!backend->size) {
+/*
+ * use the whole file size if @size is not specified.
+ */
+backend->size = qemu_file_getlength(fb->mem_path, errp);
+if (*errp) {
+return;
+}
+}
what the purpose of propagating?
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 20/35] dimm: get mapped memory region from DIMMDeviceClass->get_memory_region
On 02.11.2015 18:06, Xiao Guangrong wrote: On 11/02/2015 10:26 PM, Vladimir Sementsov-Ogievskiy wrote: On 02.11.2015 16:08, Xiao Guangrong wrote: On 11/02/2015 08:19 PM, Vladimir Sementsov-Ogievskiy wrote: On 02.11.2015 12:13, Xiao Guangrong wrote: Curretly, the memory region of backed memory is directly mapped to guest's address space, however, it is not true for nvdimm device This patch let dimm device realize this fact and use DIMMDeviceClass->get_memory_region method to get the mapped memory region Current code did not check the return value of get_memory_region as it assumed the backend memory of pc-dimm is always properly initialized, we make get_memory_region internally catch the case if something is wrong but here you call not pc-dimm's get_memory_region, but common ddc->get_memory_region, which may be nvdimm or possibly other future dimm, so, why not check it here? And than pc_dimm_get_memory_region may be left untouched (error_abort is ok, because errp is unused). Hmm, because 'here' is not the only place calling ->get_memory_region, this method has multiple callers: $ git grep "\->get_memory_region" hw/i386/pc.c:MemoryRegion *mr = ddc->get_memory_region(dimm); hw/i386/pc.c:MemoryRegion *mr = ddc->get_memory_region(dimm); hw/mem/dimm.c:mr = ddc->get_memory_region(dimm); hw/mem/nvdimm.c:ddc->get_memory_region = nvdimm_get_memory_region; hw/mem/pc-dimm.c:ddc->get_memory_region = pc_dimm_get_memory_region; hw/ppc/spapr.c:MemoryRegion *mr = ddc->get_memory_region(dimm); memory region validation is also done for NVDIMM in nvdimm device. Ok, then it should be documented by a comment in dimm.h, where DIMMDeviceClass is defined, that this function should not fail -- Best regards, Vladimir * now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 12/35] util: let qemu_fd_getlength support block device
On 02.11.2015 12:13, Xiao Guangrong wrote:
lseek can not work for all block devices as the man page says:
| Some devices are incapable of seeking and POSIX does not specify
| which devices must support lseek().
This patch tries to add the support on Linux by using BLKGETSIZE64
ioctl
Signed-off-by: Xiao Guangrong
---
util/osdep.c | 20
1 file changed, 20 insertions(+)
diff --git a/util/osdep.c b/util/osdep.c
index 5a61e19..b20c793 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -45,6 +45,11 @@
extern int madvise(caddr_t, size_t, int);
#endif
+#ifdef CONFIG_LINUX
+#include
+#include
+#endif
+
#include "qemu-common.h"
#include "qemu/sockets.h"
#include "qemu/error-report.h"
@@ -433,6 +438,21 @@ int64_t qemu_fd_getlength(int fd)
{
int64_t size;
+#ifdef CONFIG_LINUX
+struct stat stat_buf;
+if (fstat(fd, &stat_buf) < 0) {
+return -errno;
+}
+
+if ((S_ISBLK(stat_buf.st_mode)) && !ioctl(fd, BLKGETSIZE64, &size)) {
+/* The size of block device is larger than max int64_t? */
+if (size < 0) {
+return -EOVERFLOW;
+}
+return size;
+}
+#endif
+
size = lseek(fd, 0, SEEK_END);
if (size < 0) {
return -errno;
Reviewed-by: Vladimir Sementsov-Ogievskiy
just a question: is there any use for stat.st_size ? Is it always worse
then lseek? Does it work for blk?
also, "This patch tries to add..". Hmm. It looks like this patch is not
sure about will it success. I'd prefer "This patch adds", but this is
not important
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v7 09/35] exec: allow file_ram_alloc to work on file
On 02.11.2015 18:25, Xiao Guangrong wrote:
On 11/02/2015 11:12 PM, Vladimir Sementsov-Ogievskiy wrote:
On 02.11.2015 12:13, Xiao Guangrong wrote:
Currently, file_ram_alloc() only works on directory - it creates a file
under @path and do mmap on it
This patch tries to allow it to work on file directly, if @path is a
It isn't try to allow, it allows, as I understand)...
Err... Sorry for my English, but what is the different between:
”This patch tries to allow it to work on file directly“ and
"This patch allows it to work on file directly"
:(
Not sure that everyone is interested in this nit-picking discussion..
A allows B: if A then B
A tries to allow B: if A then _may be_ B
In any way it doesn't matter.
directory it works as before, otherwise it treats @path as the target
file then directly allocate memory from it
Signed-off-by: Xiao Guangrong
---
exec.c | 80
++
1 file changed, 51 insertions(+), 29 deletions(-)
diff --git a/exec.c b/exec.c
index 9075f4d..db0fdaf 100644
--- a/exec.c
+++ b/exec.c
@@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void)
}
#ifdef __linux__
+static bool path_is_dir(const char *path)
+{
+struct stat fs;
+
+return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode);
+}
+
+static int open_ram_file_path(RAMBlock *block, const char *path,
size_t size)
+{
+char *filename;
+char *sanitized_name;
+char *c;
+int fd;
+
+if (!path_is_dir(path)) {
+int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY;
+
+flags |= O_EXCL;
+return open(path, flags);
+}
+
+/* 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.XX", path,
+ sanitized_name);
+g_free(sanitized_name);
one empty line will be very nice here, and it was in master branch
+fd = mkstemp(filename);
+if (fd >= 0) {
+unlink(filename);
+/*
+ * ftruncate is not supported by hugetlbfs in older
+ * hosts, so don't bother bailing out on errors.
+ * If anything goes wrong with it under other filesystems,
+ * mmap will fail.
+ */
+if (ftruncate(fd, size)) {
+perror("ftruncate");
+}
+}
+g_free(filename);
+
+return fd;
+}
+
static void *file_ram_alloc(RAMBlock *block,
ram_addr_t memory,
const char *path,
Error **errp)
{
-char *filename;
-char *sanitized_name;
-char *c;
void *area;
int fd;
uint64_t pagesize;
@@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block,
goto error;
}
-/* 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.XX", path,
- sanitized_name);
-g_free(sanitized_name);
+memory = ROUND_UP(memory, pagesize);
-fd = mkstemp(filename);
+fd = open_ram_file_path(block, path, memory);
if (fd < 0) {
error_setg_errno(errp, errno,
"unable to create backing store for path
%s", path);
-g_free(filename);
goto error;
}
-unlink(filename);
-g_free(filename);
-
-memory = ROUND_UP(memory, pagesize);
-
-/*
- * ftruncate is not supported by hugetlbfs in older
- * hosts, so don't bother bailing out on errors.
- * If anything goes wrong with it under other filesystems,
- * mmap will fail.
- */
-if (ftruncate(fd, memory)) {
-perror("ftruncate");
-}
area = qemu_ram_mmap(fd, memory, pagesize, block->flags &
RAM_SHARED);
if (area == MAP_FAILED) {
Reviewed-by: Vladimir Sementsov-Ogievskiy
Thanks for your review.
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 08/35] exec: allow memory to be allocated from any kind of path
On 02.11.2015 18:22, Xiao Guangrong wrote:
On 11/02/2015 10:51 PM, Vladimir Sementsov-Ogievskiy wrote:
On 02.11.2015 12:13, Xiao Guangrong wrote:
Currently file_ram_alloc() is designed for hugetlbfs, however, the
memory
of nvdimm can come from either raw pmem device eg, /dev/pmem, or the
file
locates at DAX enabled filesystem
So this patch let it work on any kind of path
No, this patch doesn't change any logic, but only fix variable name
and some error messages.
Yes, it is.
'let it work' in my thought exactly was "fix variable name and some
error messages"... okay,
if it confused you, how about change it to:
"This patch fixes variable name and some error messages to let it be
aware of normal
path"
My english is not very good, I don't know figures of speech. For me
"patch let it work" means that without this patch it will not work))
Your new variant is ok for me, or better (imo) "This patch fixes
variable name and some error messages to be suitable for any kind of path"
Signed-off-by: Xiao Guangrong
---
exec.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/exec.c b/exec.c
index 9de38be..9075f4d 100644
--- a/exec.c
+++ b/exec.c
@@ -1184,25 +1184,25 @@ static void *file_ram_alloc(RAMBlock *block,
char *c;
void *area;
int fd;
-uint64_t hpagesize;
+uint64_t pagesize;
Error *local_err = NULL;
-hpagesize = qemu_file_get_page_size(path, &local_err);
+pagesize = qemu_file_get_page_size(path, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto error;
}
-if (hpagesize == getpagesize()) {
-fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path);
+if (pagesize == getpagesize()) {
+fprintf(stderr, "Memory is not allocated from HugeTlbfs.\n");
Why do you remove path from error message? It is good additional
information.. What if we have
several memory file backends?
Good catch, will change it to:
fprintf(stderr, "Memory is not allocated from HugeTlbfs on path
%s.\n", path);
BTW, if no other big change in the further, i will post the new
version just for of this patch,
}
-block->mr->align = hpagesize;
+block->mr->align = pagesize;
-if (memory < hpagesize) {
+if (memory < pagesize) {
error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be
equal to "
- "or larger than huge page size 0x%" PRIx64,
- memory, hpagesize);
+ "or larger than page size 0x%" PRIx64,
+ memory, pagesize);
goto error;
}
@@ -1226,14 +1226,14 @@ static void *file_ram_alloc(RAMBlock *block,
fd = mkstemp(filename);
if (fd < 0) {
error_setg_errno(errp, errno,
- "unable to create backing store for
hugepages");
+ "unable to create backing store for path
%s", path);
g_free(filename);
goto error;
}
unlink(filename);
g_free(filename);
-memory = ROUND_UP(memory, hpagesize);
+memory = ROUND_UP(memory, pagesize);
/*
* ftruncate is not supported by hugetlbfs in older
@@ -1245,10 +1245,10 @@ static void *file_ram_alloc(RAMBlock *block,
perror("ftruncate");
}
-area = qemu_ram_mmap(fd, memory, hpagesize, block->flags &
RAM_SHARED);
+area = qemu_ram_mmap(fd, memory, pagesize, block->flags &
RAM_SHARED);
if (area == MAP_FAILED) {
error_setg_errno(errp, errno,
- "unable to map backing store for hugepages");
+ "unable to map backing store for path %s",
path);
close(fd);
goto error;
}
With these two fixes (any commit message variant):
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 11/35] util: introduce qemu_file_getlength()
On 02.11.2015 12:13, Xiao Guangrong wrote:
It is used to get the size of the specified file, also qemu_fd_getlength()
is introduced to unify the code with raw_getlength() in block/raw-posix.c
Signed-off-by: Xiao Guangrong
---
block/raw-posix.c| 7 +--
include/qemu/osdep.h | 2 ++
util/osdep.c | 31 +++
3 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 918c756..734e6dd 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1592,18 +1592,13 @@ static int64_t raw_getlength(BlockDriverState *bs)
{
BDRVRawState *s = bs->opaque;
int ret;
-int64_t size;
ret = fd_open(bs);
if (ret < 0) {
return ret;
}
-size = lseek(s->fd, 0, SEEK_END);
-if (size < 0) {
-return -errno;
-}
-return size;
+return qemu_fd_getlength(s->fd);
}
#endif
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index dbc17dc..ca4c3fa 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -303,4 +303,6 @@ int qemu_read_password(char *buf, int buf_size);
pid_t qemu_fork(Error **errp);
size_t qemu_file_get_page_size(const char *mem_path, Error **errp);
+int64_t qemu_fd_getlength(int fd);
+size_t qemu_file_getlength(const char *file, Error **errp);
#endif
diff --git a/util/osdep.c b/util/osdep.c
index 0092bb6..5a61e19 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -428,3 +428,34 @@ writev(int fd, const struct iovec *iov, int iov_cnt)
return readv_writev(fd, iov, iov_cnt, true);
}
#endif
+
+int64_t qemu_fd_getlength(int fd)
+{
+int64_t size;
+
+size = lseek(fd, 0, SEEK_END);
+if (size < 0) {
+return -errno;
+}
+return size;
+}
+
+size_t qemu_file_getlength(const char *file, Error **errp)
+{
+int64_t size;
+int fd = qemu_open(file, O_RDONLY);
+
+if (fd < 0) {
+error_setg_file_open(errp, errno, file);
+return 0;
+}
+
+size = qemu_fd_getlength(fd);
+if (size < 0) {
+error_setg_errno(errp, -size, "can't get size of file %s", file);
+size = 0;
+}
+
+qemu_close(fd);
+return size;
+}
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 09/35] exec: allow file_ram_alloc to work on file
On 02.11.2015 12:13, Xiao Guangrong wrote:
Currently, file_ram_alloc() only works on directory - it creates a file
under @path and do mmap on it
This patch tries to allow it to work on file directly, if @path is a
It isn't try to allow, it allows, as I understand)
directory it works as before, otherwise it treats @path as the target
file then directly allocate memory from it
Signed-off-by: Xiao Guangrong
---
exec.c | 80 ++
1 file changed, 51 insertions(+), 29 deletions(-)
diff --git a/exec.c b/exec.c
index 9075f4d..db0fdaf 100644
--- a/exec.c
+++ b/exec.c
@@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void)
}
#ifdef __linux__
+static bool path_is_dir(const char *path)
+{
+struct stat fs;
+
+return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode);
+}
+
+static int open_ram_file_path(RAMBlock *block, const char *path, size_t size)
+{
+char *filename;
+char *sanitized_name;
+char *c;
+int fd;
+
+if (!path_is_dir(path)) {
+int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY;
+
+flags |= O_EXCL;
+return open(path, flags);
+}
+
+/* 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.XX", path,
+ sanitized_name);
+g_free(sanitized_name);
one empty line will be very nice here, and it was in master branch
+fd = mkstemp(filename);
+if (fd >= 0) {
+unlink(filename);
+/*
+ * ftruncate is not supported by hugetlbfs in older
+ * hosts, so don't bother bailing out on errors.
+ * If anything goes wrong with it under other filesystems,
+ * mmap will fail.
+ */
+if (ftruncate(fd, size)) {
+perror("ftruncate");
+}
+}
+g_free(filename);
+
+return fd;
+}
+
static void *file_ram_alloc(RAMBlock *block,
ram_addr_t memory,
const char *path,
Error **errp)
{
-char *filename;
-char *sanitized_name;
-char *c;
void *area;
int fd;
uint64_t pagesize;
@@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block,
goto error;
}
-/* 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.XX", path,
- sanitized_name);
-g_free(sanitized_name);
+memory = ROUND_UP(memory, pagesize);
-fd = mkstemp(filename);
+fd = open_ram_file_path(block, path, memory);
if (fd < 0) {
error_setg_errno(errp, errno,
"unable to create backing store for path %s", path);
-g_free(filename);
goto error;
}
-unlink(filename);
-g_free(filename);
-
-memory = ROUND_UP(memory, pagesize);
-
-/*
- * ftruncate is not supported by hugetlbfs in older
- * hosts, so don't bother bailing out on errors.
- * If anything goes wrong with it under other filesystems,
- * mmap will fail.
- */
-if (ftruncate(fd, memory)) {
- perror("ftruncate");
-}
area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED);
if (area == MAP_FAILED) {
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 08/35] exec: allow memory to be allocated from any kind of path
On 02.11.2015 12:13, Xiao Guangrong wrote:
Currently file_ram_alloc() is designed for hugetlbfs, however, the memory
of nvdimm can come from either raw pmem device eg, /dev/pmem, or the file
locates at DAX enabled filesystem
So this patch let it work on any kind of path
No, this patch doesn't change any logic, but only fix variable name and
some error messages.
Signed-off-by: Xiao Guangrong
---
exec.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/exec.c b/exec.c
index 9de38be..9075f4d 100644
--- a/exec.c
+++ b/exec.c
@@ -1184,25 +1184,25 @@ static void *file_ram_alloc(RAMBlock *block,
char *c;
void *area;
int fd;
-uint64_t hpagesize;
+uint64_t pagesize;
Error *local_err = NULL;
-hpagesize = qemu_file_get_page_size(path, &local_err);
+pagesize = qemu_file_get_page_size(path, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto error;
}
-if (hpagesize == getpagesize()) {
-fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path);
+if (pagesize == getpagesize()) {
+fprintf(stderr, "Memory is not allocated from HugeTlbfs.\n");
Why do you remove path from error message? It is good additional
information.. What if we have several memory file backends?
}
-block->mr->align = hpagesize;
+block->mr->align = pagesize;
-if (memory < hpagesize) {
+if (memory < pagesize) {
error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
- "or larger than huge page size 0x%" PRIx64,
- memory, hpagesize);
+ "or larger than page size 0x%" PRIx64,
+ memory, pagesize);
goto error;
}
@@ -1226,14 +1226,14 @@ static void *file_ram_alloc(RAMBlock *block,
fd = mkstemp(filename);
if (fd < 0) {
error_setg_errno(errp, errno,
- "unable to create backing store for hugepages");
+ "unable to create backing store for path %s", path);
g_free(filename);
goto error;
}
unlink(filename);
g_free(filename);
-memory = ROUND_UP(memory, hpagesize);
+memory = ROUND_UP(memory, pagesize);
/*
* ftruncate is not supported by hugetlbfs in older
@@ -1245,10 +1245,10 @@ static void *file_ram_alloc(RAMBlock *block,
perror("ftruncate");
}
-area = qemu_ram_mmap(fd, memory, hpagesize, block->flags & RAM_SHARED);
+area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED);
if (area == MAP_FAILED) {
error_setg_errno(errp, errno,
- "unable to map backing store for hugepages");
+ "unable to map backing store for path %s", path);
close(fd);
goto error;
}
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 20/35] dimm: get mapped memory region from DIMMDeviceClass->get_memory_region
On 02.11.2015 16:08, Xiao Guangrong wrote:
On 11/02/2015 08:19 PM, Vladimir Sementsov-Ogievskiy wrote:
On 02.11.2015 12:13, Xiao Guangrong wrote:
Curretly, the memory region of backed memory is directly mapped to
guest's address space, however, it is not true for nvdimm device
This patch let dimm device realize this fact and use
DIMMDeviceClass->get_memory_region method to get the mapped memory
region
Current code did not check the return value of get_memory_region as it
assumed the backend memory of pc-dimm is always properly initialized,
we make get_memory_region internally catch the case if something is
wrong
but here you call not pc-dimm's get_memory_region, but common
ddc->get_memory_region, which may be nvdimm or possibly other future
dimm, so, why not check it here? And than pc_dimm_get_memory_region may
be left untouched (error_abort is ok, because errp is unused).
Signed-off-by: Xiao Guangrong
---
hw/mem/dimm.c| 3 ++-
hw/mem/pc-dimm.c | 12 +++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/hw/mem/dimm.c b/hw/mem/dimm.c
index 4a63409..498d380 100644
--- a/hw/mem/dimm.c
+++ b/hw/mem/dimm.c
@@ -377,8 +377,9 @@ static void dimm_get_size(Object *obj, Visitor
*v, void *opaque,
int64_t value;
MemoryRegion *mr;
DIMMDevice *dimm = DIMM(obj);
+DIMMDeviceClass *ddc = DIMM_GET_CLASS(obj);
-mr = host_memory_backend_get_memory(dimm->hostmem, errp);
+mr = ddc->get_memory_region(dimm);
value = memory_region_size(mr);
visit_type_int(v, &value, name, errp);
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 38323e9..e6b6a9f 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -22,7 +22,17 @@
static MemoryRegion *pc_dimm_get_memory_region(DIMMDevice *dimm)
{
-return host_memory_backend_get_memory(dimm->hostmem,
&error_abort);
+Error *local_err = NULL;
+MemoryRegion *mr;
+
+mr = host_memory_backend_get_memory(dimm->hostmem, &local_err);
+
+/*
+ * plug a pc-dimm device whose backend memory was not properly
+ * initialized?
+ */
+assert(!local_err && mr);
+return mr;
}
this should squashed into previous patch, I think
You mean merger this patch with 19/35 (dimm: abstract dimm device from
pc-dimm)?
The 19/35 mostly ‘moves’ the things, this one changes the core logic,
it is not
a big deal. :D
stupid me, you are right)
static void pc_dimm_class_init(ObjectClass *oc, void *data)
I've discovered suddenly, that
MemoryRegion *
host_memory_backend_get_memory(HostMemoryBackend *backend, Error **errp)
{
return memory_region_size(&backend->mr) ? &backend->mr : NULL;
}
- it doesn't use errp at all. In my opinion, this should be fixed
globally, by deleting useless
parameter in separate patch. Or just squash your function into
previous patch.
Yup, this is a globally interface so i prefer to make a separate patch
to do the
cleanup after this patchset.
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 07/35] util: introduce qemu_file_get_page_size()
);
} else {
int i;
-size_t hpagesize = fd_getpagesize(fd);
-size_t numpages = DIV_ROUND_UP(memory, hpagesize);
+Error *local_err = NULL;
+size_t hpagesize = fd_getpagesize(fd, &local_err);
+size_t numpages;
+
+if (local_err) {
+error_report_err(local_err);
+exit(1);
+}
+
+numpages = DIV_ROUND_UP(memory, hpagesize);
/* MAP_POPULATE silently ignores failures */
for (i = 0; i < numpages; i++) {
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 09f9e98..dada6b6 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -462,6 +462,11 @@ size_t getpagesize(void)
return system_info.dwPageSize;
}
+size_t qemu_file_get_page_size(const char *path, Error **errp)
+{
+return getpagesize();
+}
+
void os_mem_prealloc(int fd, char *area, size_t memory)
{
int i;
Ok for me. The only thing: some functions about pagesize return size_t,
when others return long. long is more common practice here, but this
doesn't really matter, so with or without size_t <-> long changes:
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 20/35] dimm: get mapped memory region from DIMMDeviceClass->get_memory_region
On 02.11.2015 12:13, Xiao Guangrong wrote:
Curretly, the memory region of backed memory is directly mapped to
guest's address space, however, it is not true for nvdimm device
This patch let dimm device realize this fact and use
DIMMDeviceClass->get_memory_region method to get the mapped memory
region
Current code did not check the return value of get_memory_region as it
assumed the backend memory of pc-dimm is always properly initialized,
we make get_memory_region internally catch the case if something is
wrong
Signed-off-by: Xiao Guangrong
---
hw/mem/dimm.c| 3 ++-
hw/mem/pc-dimm.c | 12 +++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/hw/mem/dimm.c b/hw/mem/dimm.c
index 4a63409..498d380 100644
--- a/hw/mem/dimm.c
+++ b/hw/mem/dimm.c
@@ -377,8 +377,9 @@ static void dimm_get_size(Object *obj, Visitor *v, void
*opaque,
int64_t value;
MemoryRegion *mr;
DIMMDevice *dimm = DIMM(obj);
+DIMMDeviceClass *ddc = DIMM_GET_CLASS(obj);
-mr = host_memory_backend_get_memory(dimm->hostmem, errp);
+mr = ddc->get_memory_region(dimm);
value = memory_region_size(mr);
visit_type_int(v, &value, name, errp);
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 38323e9..e6b6a9f 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -22,7 +22,17 @@
static MemoryRegion *pc_dimm_get_memory_region(DIMMDevice *dimm)
{
-return host_memory_backend_get_memory(dimm->hostmem, &error_abort);
+Error *local_err = NULL;
+MemoryRegion *mr;
+
+mr = host_memory_backend_get_memory(dimm->hostmem, &local_err);
+
+/*
+ * plug a pc-dimm device whose backend memory was not properly
+ * initialized?
+ */
+assert(!local_err && mr);
+return mr;
}
this should squashed into previous patch, I think
static void pc_dimm_class_init(ObjectClass *oc, void *data)
I've discovered suddenly, that
MemoryRegion *
host_memory_backend_get_memory(HostMemoryBackend *backend, Error **errp)
{
return memory_region_size(&backend->mr) ? &backend->mr : NULL;
}
- it doesn't use errp at all. In my opinion, this should be fixed
globally, by deleting useless parameter in separate patch. Or just
squash your function into previous patch.
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 20/33] dimm: introduce realize callback
On 30.10.2015 08:56, Xiao Guangrong wrote:
nvdimm need check if the backend memory is large enough to contain label
data and init its memory region when the device is realized, so introduce
realize callback which is called after common dimm has been realize
Signed-off-by: Xiao Guangrong
---
hw/mem/dimm.c | 5 +
include/hw/mem/dimm.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/hw/mem/dimm.c b/hw/mem/dimm.c
index 7d1..0ae23ce 100644
--- a/hw/mem/dimm.c
+++ b/hw/mem/dimm.c
@@ -426,6 +426,7 @@ static void dimm_init(Object *obj)
static void dimm_realize(DeviceState *dev, Error **errp)
{
DIMMDevice *dimm = DIMM(dev);
+DIMMDeviceClass *ddc = DIMM_GET_CLASS(dimm);
if (!dimm->hostmem) {
error_setg(errp, "'" DIMM_MEMDEV_PROP "' property is not set");
@@ -438,6 +439,10 @@ static void dimm_realize(DeviceState *dev, Error **errp)
dimm->node, nb_numa_nodes ? nb_numa_nodes : 1);
return;
}
+
+if (ddc->realize) {
+ddc->realize(dimm, errp);
+}
}
static void dimm_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/mem/dimm.h b/include/hw/mem/dimm.h
index 50f768a..72ec24c 100644
--- a/include/hw/mem/dimm.h
+++ b/include/hw/mem/dimm.h
@@ -65,6 +65,7 @@ typedef struct DIMMDeviceClass {
DeviceClass parent_class;
/* public */
+void (*realize)(DIMMDevice *dimm, Error **errp);
MemoryRegion *(*get_memory_region)(DIMMDevice *dimm);
} DIMMDeviceClass;
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 19/33] dimm: keep the state of the whole backend memory
On 30.10.2015 08:56, Xiao Guangrong wrote:
QEMU keeps the state of memory of dimm device during live migration,
however, it is not enough for nvdimm device as its memory does not
contain its label data, so that we should protect the whole backend
memory instead
Signed-off-by: Xiao Guangrong
---
hw/mem/dimm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/hw/mem/dimm.c b/hw/mem/dimm.c
index 498d380..7d1 100644
--- a/hw/mem/dimm.c
+++ b/hw/mem/dimm.c
@@ -134,9 +134,16 @@ void dimm_memory_plug(DeviceState *dev, MemoryHotplugState
*hpms,
}
memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr);
-vmstate_register_ram(mr, dev);
numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node);
+/*
+ * save the state only for @mr is not enough as it does not contain
+ * the label data of NVDIMM device, so that we keep the state of
+ * whole hostmem instead.
+ */
+vmstate_register_ram(host_memory_backend_get_memory(dimm->hostmem, errp),
+ dev);
+
out:
error_propagate(errp, local_err);
}
@@ -145,10 +152,13 @@ void dimm_memory_unplug(DeviceState *dev,
MemoryHotplugState *hpms,
MemoryRegion *mr)
{
DIMMDevice *dimm = DIMM(dev);
+MemoryRegion *backend_mr;
+
+backend_mr = host_memory_backend_get_memory(dimm->hostmem, &error_abort);
numa_unset_mem_node_id(dimm->addr, memory_region_size(mr), dimm->node);
memory_region_del_subregion(&hpms->mr, mr);
-vmstate_unregister_ram(mr, dev);
+vmstate_unregister_ram(backend_mr, dev);
}
int qmp_dimm_device_list(Object *obj, void *opaque)
should get_memory_region be used here like in previous patch?
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 18/33] dimm: get mapped memory region from DIMMDeviceClass->get_memory_region
On 30.10.2015 08:56, Xiao Guangrong wrote: Curretly, the memory region of backed memory is directly mapped to guest's address space, however, it is not true for nvdimm device This patch let dimm device realize this fact and use DIMMDeviceClass->get_memory_region method to get the mapped memory region Signed-off-by: Xiao Guangrong --- hw/mem/dimm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/mem/dimm.c b/hw/mem/dimm.c index 4a63409..498d380 100644 --- a/hw/mem/dimm.c +++ b/hw/mem/dimm.c @@ -377,8 +377,9 @@ static void dimm_get_size(Object *obj, Visitor *v, void *opaque, int64_t value; MemoryRegion *mr; DIMMDevice *dimm = DIMM(obj); +DIMMDeviceClass *ddc = DIMM_GET_CLASS(obj); -mr = host_memory_backend_get_memory(dimm->hostmem, errp); +mr = ddc->get_memory_region(dimm); value = memory_region_size(mr); visit_type_int(v, &value, name, errp); Isn't it better to add Error** parameter to get_memory_region and not mix returning error in errp parameter and aborting through &error_abort? -- Best regards, Vladimir * now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 17/33] dimm: abstract dimm device from pc-dimm
m.h +++ b/include/hw/mem/dimm.h @@ -1,5 +1,5 @@ /* - * PC DIMM device + * Dimm device abstraction * * Copyright ProfitBricks GmbH 2012 * Copyright (C) 2013-2014 Red Hat Inc @@ -20,7 +20,7 @@ #include "sysemu/hostmem.h" #include "hw/qdev.h" -#define TYPE_DIMM "pc-dimm" +#define TYPE_DIMM "dimm" #define DIMM(obj) \ OBJECT_CHECK(DIMMDevice, (obj), TYPE_DIMM) #define DIMM_CLASS(oc) \ diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h new file mode 100644 index 000..50818c2 --- /dev/null +++ b/include/hw/mem/pc-dimm.h @@ -0,0 +1,7 @@ +#ifndef QEMU_PC_DIMM_H +#define QEMU_PC_DIMM_H + +#include "hw/mem/dimm.h" + +#define TYPE_PC_DIMM "pc-dimm" +#endif Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir * now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 16/33] pc-dimm: rename pc-dimm.c and pc-dimm.h
ds in the right node.
* This appears as if some memory moved from one node to another.
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 606dbc2..62e8fb5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -16,7 +16,7 @@
#include "hw/pci/pci.h"
#include "hw/boards.h"
#include "hw/compat.h"
-#include "hw/mem/pc-dimm.h"
+#include "hw/mem/dimm.h"
#define HPET_INTCAP "hpet-intcap"
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/dimm.h
similarity index 100%
rename from include/hw/mem/pc-dimm.h
rename to include/hw/mem/dimm.h
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 5baa906..0ae3abe 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -5,7 +5,7 @@
#include "hw/boards.h"
#include "hw/ppc/xics.h"
#include "hw/ppc/spapr_drc.h"
-#include "hw/mem/pc-dimm.h"
+#include "hw/mem/dimm.h"
struct VIOsPAPRBus;
struct sPAPRPHBState;
diff --git a/numa.c b/numa.c
index cb69965..34c6d6b 100644
--- a/numa.c
+++ b/numa.c
@@ -34,7 +34,7 @@
#include "hw/boards.h"
#include "sysemu/hostmem.h"
#include "qmp-commands.h"
-#include "hw/mem/pc-dimm.h"
+#include "hw/mem/dimm.h"
#include "qemu/option.h"
#include "qemu/config-file.h"
diff --git a/qmp.c b/qmp.c
index 169b981..5ae37f7 100644
--- a/qmp.c
+++ b/qmp.c
@@ -31,7 +31,7 @@
#include "qapi/qmp-input-visitor.h"
#include "hw/boards.h"
#include "qom/object_interfaces.h"
-#include "hw/mem/pc-dimm.h"
+#include "hw/mem/dimm.h"
#include "hw/acpi/acpi_dev_interface.h"
NameInfo *qmp_query_name(Error **errp)
diff --git a/stubs/qmp_dimm_device_list.c b/stubs/qmp_dimm_device_list.c
index b2704c6..fb66400 100644
--- a/stubs/qmp_dimm_device_list.c
+++ b/stubs/qmp_dimm_device_list.c
@@ -1,5 +1,5 @@
#include "qom/object.h"
-#include "hw/mem/pc-dimm.h"
+#include "hw/mem/dimm.h"
int qmp_dimm_device_list(Object *obj, void *opaque)
{
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 07/33] util: introduce qemu_file_get_page_size()
On 31.10.2015 10:26, Xiao Guangrong wrote: On 10/30/2015 09:26 PM, Vladimir Sementsov-Ogievskiy wrote: logic is changed: in old version gethugepagesize on statfs error generates exit(1) in new it returns getpagesize() in this case (through fd_getpagesize) (I think, fd_getpagesize should be fixed to handle error) Indeed. I will let fd_getpagesize() return 0 if statfs is failed, then the caller handle the error properly. Why not use classic error codes < 0? Just change size_t to ssize_t and return exactly fstatfs return value. also, in new version for windows we have getpagesize(), when in old version there was no difference (how did it work?). May be it's ok, but should be mentioned in commit message Windows did not support file hugepage, so it will return normal page for this case. And this interface has not been used on windows so far. I will document it in the commit message as your suggestion. -- Best regards, Vladimir * now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 14/33] pc-dimm: drop the prefix of pc-dimm
On 30.10.2015 08:56, Xiao Guangrong wrote:
This patch is generated by this script:
find ./ -name "*.[ch]" -o -name "*.json" -o -name "trace-events" -type f \
| xargs sed -i "s/PC_DIMM/DIMM/g"
find ./ -name "*.[ch]" -o -name "*.json" -o -name "trace-events" -type f \
| xargs sed -i "s/PCDIMM/DIMM/g"
find ./ -name "*.[ch]" -o -name "*.json" -o -name "trace-events" -type f \
| xargs sed -i "s/pc_dimm/dimm/g"
find ./ -name "trace-events" -type f | xargs sed -i "s/pc-dimm/dimm/g"
It prepares the work which abstracts dimm device type for both pc-dimm and
nvdimm
Signed-off-by: Xiao Guangrong
---
hmp.c | 2 +-
hw/acpi/ich9.c | 6 +-
hw/acpi/memory_hotplug.c| 16 ++---
hw/acpi/piix4.c | 6 +-
hw/i386/pc.c| 32 -
hw/mem/pc-dimm.c| 148
hw/ppc/spapr.c | 18 ++---
include/hw/mem/pc-dimm.h| 62 -
numa.c | 2 +-
qapi-schema.json| 8 +--
qmp.c | 2 +-
stubs/qmp_pc_dimm_device_list.c | 2 +-
trace-events| 8 +--
13 files changed, 156 insertions(+), 156 deletions(-)
In the following patches, dimm is a parent for nv-dimm and pc-dimm, so
dimm is more abstract when nv-dimm and pc-dimm are more concrete. So for
me it is strange, that all these files, all old staff will use an
abstract dimm. What the purpose of pc-dimm in this case (which appeared
in the following patches)?
diff --git a/hmp.c b/hmp.c
index 5048eee..5c617d2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1952,7 +1952,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict
*qdict)
MemoryDeviceInfoList *info_list = qmp_query_memory_devices(&err);
MemoryDeviceInfoList *info;
MemoryDeviceInfo *value;
-PCDIMMDeviceInfo *di;
+DIMMDeviceInfo *di;
for (info = info_list; info; info = info->next) {
value = info->value;
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 1c7fcfa..b0d6a67 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -440,7 +440,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm,
Error **errp)
void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error **errp)
{
if (pm->acpi_memory_hotplug.is_enabled &&
-object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+object_dynamic_cast(OBJECT(dev), TYPE_DIMM)) {
acpi_memory_plug_cb(&pm->acpi_regs, pm->irq, &pm->acpi_memory_hotplug,
dev, errp);
} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
@@ -455,7 +455,7 @@ void ich9_pm_device_unplug_request_cb(ICH9LPCPMRegs *pm,
DeviceState *dev,
Error **errp)
{
if (pm->acpi_memory_hotplug.is_enabled &&
-object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+object_dynamic_cast(OBJECT(dev), TYPE_DIMM)) {
acpi_memory_unplug_request_cb(&pm->acpi_regs, pm->irq,
&pm->acpi_memory_hotplug, dev, errp);
} else {
@@ -468,7 +468,7 @@ void ich9_pm_device_unplug_cb(ICH9LPCPMRegs *pm,
DeviceState *dev,
Error **errp)
{
if (pm->acpi_memory_hotplug.is_enabled &&
-object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+object_dynamic_cast(OBJECT(dev), TYPE_DIMM)) {
acpi_memory_unplug_cb(&pm->acpi_memory_hotplug, dev, errp);
} else {
error_setg(errp, "acpi: device unplug for not supported device"
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index ce428df..e687852 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -54,23 +54,23 @@ static uint64_t acpi_memory_hotplug_read(void *opaque,
hwaddr addr,
o = OBJECT(mdev->dimm);
switch (addr) {
case 0x0: /* Lo part of phys address where DIMM is mapped */
-val = o ? object_property_get_int(o, PC_DIMM_ADDR_PROP, NULL) : 0;
+val = o ? object_property_get_int(o, DIMM_ADDR_PROP, NULL) : 0;
trace_mhp_acpi_read_addr_lo(mem_st->selector, val);
break;
case 0x4: /* Hi part of phys address where DIMM is mapped */
-val = o ? object_property_get_int(o, PC_DIMM_ADDR_PROP, NULL) >> 32 :
0;
+val = o ? object_property_get_int(o, DIMM_ADDR_PROP, NULL) >> 32 : 0;
trace_mhp_acpi_read_addr_hi(mem_st->selector, val);
break;
case 0x8: /* Lo part of DIMM size */
-val = o ? object_property_get_int(o, PC_DIMM_SIZE_PROP, NULL) : 0;
+val = o ? object_property_get_int(o, DIMM_SIZE_PROP, NULL) : 0;
trace_mhp_acpi_read_size_lo(mem_st->selector, val);
break;
case 0xc: /* Hi part of DIMM size */
-val = o ? object_property_get_int(o, PC_DIMM_SIZE_PROP, NULL) >> 32 :
0;
+val = o ? object_property_get_int(o, DIMM_SIZE_PROP, N
Re: [Qemu-devel] [PATCH v6 13/33] pc-dimm: make pc_existing_dimms_capacity static and rename it
On 30.10.2015 08:56, Xiao Guangrong wrote:
pc_existing_dimms_capacity() can be static since it is not used out of
pc-dimm.c and drop the pc_ prefix to prepare the work which abstracts
dimm device type from pc-dimm
Signed-off-by: Xiao Guangrong
---
hw/mem/pc-dimm.c | 73
include/hw/mem/pc-dimm.h | 1 -
2 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 80f424b..2dcbbcd 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -32,6 +32,38 @@ typedef struct pc_dimms_capacity {
Error**errp;
} pc_dimms_capacity;
+static int existing_dimms_capacity_internal(Object *obj, void *opaque)
+{
+pc_dimms_capacity *cap = opaque;
+uint64_t *size = &cap->size;
+
+if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
+DeviceState *dev = DEVICE(obj);
+
+if (dev->realized) {
+(*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
+cap->errp);
+}
+
+if (cap->errp && *cap->errp) {
+return 1;
+}
+}
+object_child_foreach(obj, existing_dimms_capacity_internal, opaque);
+return 0;
+}
+
+static uint64_t existing_dimms_capacity(Error **errp)
+{
+pc_dimms_capacity cap;
+
+cap.size = 0;
+cap.errp = errp;
+
+existing_dimms_capacity_internal(qdev_get_machine(), &cap);
+return cap.size;
+}
+
void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
MemoryRegion *mr, uint64_t align, Error **errp)
{
@@ -39,7 +71,7 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState
*hpms,
MachineState *machine = MACHINE(qdev_get_machine());
PCDIMMDevice *dimm = PC_DIMM(dev);
Error *local_err = NULL;
-uint64_t existing_dimms_capacity = 0;
+uint64_t dimms_capacity = 0;
uint64_t addr;
addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP, &local_err);
@@ -55,17 +87,16 @@ void pc_dimm_memory_plug(DeviceState *dev,
MemoryHotplugState *hpms,
goto out;
}
-existing_dimms_capacity = pc_existing_dimms_capacity(&local_err);
+dimms_capacity = existing_dimms_capacity(&local_err);
if (local_err) {
goto out;
}
-if (existing_dimms_capacity + memory_region_size(mr) >
+if (dimms_capacity + memory_region_size(mr) >
machine->maxram_size - machine->ram_size) {
error_setg(&local_err, "not enough space, currently 0x%" PRIx64
" in use of total hot pluggable 0x" RAM_ADDR_FMT,
- existing_dimms_capacity,
- machine->maxram_size - machine->ram_size);
+ dimms_capacity, machine->maxram_size - machine->ram_size);
goto out;
}
@@ -120,38 +151,6 @@ void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
vmstate_unregister_ram(mr, dev);
}
-static int pc_existing_dimms_capacity_internal(Object *obj, void *opaque)
-{
-pc_dimms_capacity *cap = opaque;
-uint64_t *size = &cap->size;
-
-if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
-DeviceState *dev = DEVICE(obj);
-
-if (dev->realized) {
-(*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
-cap->errp);
-}
-
-if (cap->errp && *cap->errp) {
-return 1;
-}
-}
-object_child_foreach(obj, pc_existing_dimms_capacity_internal, opaque);
-return 0;
-}
-
-uint64_t pc_existing_dimms_capacity(Error **errp)
-{
-pc_dimms_capacity cap;
-
-cap.size = 0;
-cap.errp = errp;
-
-pc_existing_dimms_capacity_internal(qdev_get_machine(), &cap);
-return cap.size;
-}
-
int qmp_pc_dimm_device_list(Object *obj, void *opaque)
{
MemoryDeviceInfoList ***prev = opaque;
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 11a8937..8a43548 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -87,7 +87,6 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
int qmp_pc_dimm_device_list(Object *obj, void *opaque);
-uint64_t pc_existing_dimms_capacity(Error **errp);
void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
MemoryRegion *mr, uint64_t align, Error **errp);
void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 11/33] hostmem-file: use whole file size if possible
On 30.10.2015 08:56, Xiao Guangrong wrote:
Use the whole file size if @size is not specified which is useful
if we want to directly pass a file to guest
Signed-off-by: Xiao Guangrong
---
backends/hostmem-file.c | 48
1 file changed, 44 insertions(+), 4 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 9097a57..e1bc9ff 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -9,6 +9,9 @@
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
*/
+#include
+#include
+
#include "qemu-common.h"
#include "sysemu/hostmem.h"
#include "sysemu/sysemu.h"
@@ -33,20 +36,57 @@ struct HostMemoryBackendFile {
char *mem_path;
};
+static uint64_t get_file_size(const char *file)
+{
+struct stat stat_buf;
+uint64_t size = 0;
+int fd;
+
+fd = open(file, O_RDONLY);
+if (fd < 0) {
+return 0;
+}
+
+if (stat(file, &stat_buf) < 0) {
+goto exit;
+}
+
+if ((S_ISBLK(stat_buf.st_mode)) && !ioctl(fd, BLKGETSIZE64, &size)) {
+goto exit;
+}
+
+size = lseek(fd, 0, SEEK_END);
+if (size == -1) {
+size = 0;
+}
+exit:
+close(fd);
+return size;
+}
+
static void
file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
{
HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
-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;
}
+if (!backend->size) {
+/*
+ * use the whole file size if @size is not specified.
+ */
+backend->size = get_file_size(fb->mem_path);
+}
+
+if (!backend->size) {
+error_setg(errp, "failed to get file size for %s, can't create "
+ "backend on it", mem_path);
+return;
+}
+
backend->force_prealloc = mem_prealloc;
memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
object_get_canonical_path(OBJECT(backend)),
You can use error_setg_errno to get an error descriptioin for free, as
all possible errors from get_file_size comes with errno. Just zero size
should be separated for this, for example by usiing int64_t as return
type and -1 for an error.
Sorry for this nit-picking)
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 10/33] hostmem-file: clean up memory allocation
On 30.10.2015 08:56, Xiao Guangrong wrote:
- hostmem-file.c is compiled only if CONFIG_LINUX is enabled so that is
unnecessary to do the same check in the source file
- the interface, HostMemoryBackendClass->alloc(), is not called many
times, do not need to check if the memory-region is initialized
Signed-off-by: Xiao Guangrong
---
backends/hostmem-file.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index e9b6d21..9097a57 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -46,17 +46,12 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error
**errp)
error_setg(errp, "mem-path property not set");
return;
}
-#ifndef CONFIG_LINUX
-error_setg(errp, "-mem-path not supported on this host");
-#else
-if (!memory_region_size(&backend->mr)) {
-backend->force_prealloc = mem_prealloc;
-memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
+
+backend->force_prealloc = mem_prealloc;
+memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
object_get_canonical_path(OBJECT(backend)),
backend->size, fb->share,
fb->mem_path, errp);
-}
-#endif
}
static void
Similar function for memory backend (the only other
HostMemoryBackendClass) - ram_backend_memory_alloc - has not such check
too. It's ok..
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 09/33] exec: allow file_ram_alloc to work on file
On 30.10.2015 08:56, Xiao Guangrong wrote:
Currently, file_ram_alloc() only works on directory - it creates a file
under @path and do mmap on it
This patch tries to allow it to work on file directly, if @path is a
directory it works as before, otherwise it treats @path as the target
file then directly allocate memory from it
Signed-off-by: Xiao Guangrong
---
exec.c | 80 ++
1 file changed, 51 insertions(+), 29 deletions(-)
diff --git a/exec.c b/exec.c
index 3ca7e50..f219010 100644
--- a/exec.c
+++ b/exec.c
@@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void)
}
#ifdef __linux__
+static bool path_is_dir(const char *path)
+{
+struct stat fs;
+
+return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode);
+}
+
+static int open_file_path(RAMBlock *block, const char *path, size_t size)
I think the name should be more descriptive, as it is very special
function in common exec.c and it doesn't "just open file path'.. May be
open_ram_file_path
+{
+char *filename;
+char *sanitized_name;
+char *c;
+int fd;
+
+if (!path_is_dir(path)) {
+int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY;
+
+flags |= O_EXCL;
+return open(path, flags);
+}
Was not there old scenarios when path is file? statfs will success for
any file withing mounted filesystem.
Also, may be we shouldn't warn about "Memory is not allocated from
HugeTlbfs.\n" in case of !path_is_dir ?
+
+/* 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.XX", path,
+ sanitized_name);
+g_free(sanitized_name);
+fd = mkstemp(filename);
+if (fd >= 0) {
+unlink(filename);
+/*
+ * ftruncate is not supported by hugetlbfs in older
+ * hosts, so don't bother bailing out on errors.
+ * If anything goes wrong with it under other filesystems,
+ * mmap will fail.
+ */
+if (ftruncate(fd, size)) {
+perror("ftruncate");
+}
+}
+g_free(filename);
+
+return fd;
+}
+
static void *file_ram_alloc(RAMBlock *block,
ram_addr_t memory,
const char *path,
Error **errp)
{
-char *filename;
-char *sanitized_name;
-char *c;
void *area;
int fd;
uint64_t pagesize;
@@ -1211,38 +1257,14 @@ static void *file_ram_alloc(RAMBlock *block,
goto error;
}
-/* 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.XX", path,
- sanitized_name);
-g_free(sanitized_name);
+memory = ROUND_UP(memory, pagesize);
-fd = mkstemp(filename);
+fd = open_file_path(block, path, memory);
if (fd < 0) {
error_setg_errno(errp, errno,
"unable to create backing store for path %s", path);
-g_free(filename);
goto error;
}
-unlink(filename);
-g_free(filename);
-
-memory = ROUND_UP(memory, pagesize);
-
-/*
- * ftruncate is not supported by hugetlbfs in older
- * hosts, so don't bother bailing out on errors.
- * If anything goes wrong with it under other filesystems,
- * mmap will fail.
- */
-if (ftruncate(fd, memory)) {
-perror("ftruncate");
-}
area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED);
if (area == MAP_FAILED) {
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 08/33] exec: allow memory to be allocated from any kind of path
On 30.10.2015 08:56, Xiao Guangrong wrote:
Currently file_ram_alloc() is designed for hugetlbfs, however, the memory
of nvdimm can come from either raw pmem device eg, /dev/pmem, or the file
locates at DAX enabled filesystem
So this patch let it work on any kind of path
Signed-off-by: Xiao Guangrong
---
exec.c | 56 +---
1 file changed, 17 insertions(+), 39 deletions(-)
diff --git a/exec.c b/exec.c
index 8af2570..3ca7e50 100644
--- a/exec.c
+++ b/exec.c
@@ -1174,32 +1174,6 @@ void qemu_mutex_unlock_ramlist(void)
}
#ifdef __linux__
-
-#include
-
-#define HUGETLBFS_MAGIC 0x958458f6
-
-static long gethugepagesize(const char *path, Error **errp)
-{
-struct statfs fs;
-int ret;
-
-do {
-ret = statfs(path, &fs);
-} while (ret != 0 && errno == EINTR);
-
-if (ret != 0) {
-error_setg_errno(errp, errno, "failed to get page size of file %s",
- path);
-return 0;
-}
-
-if (fs.f_type != HUGETLBFS_MAGIC)
-fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path);
-
-return fs.f_bsize;
-}
-
static void *file_ram_alloc(RAMBlock *block,
ram_addr_t memory,
const char *path,
@@ -1210,20 +1184,24 @@ static void *file_ram_alloc(RAMBlock *block,
char *c;
void *area;
int fd;
-uint64_t hpagesize;
-Error *local_err = NULL;
+uint64_t pagesize;
-hpagesize = gethugepagesize(path, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
+pagesize = qemu_file_get_page_size(path);
+if (!pagesize) {
+error_setg(errp, "can't get page size for %s", path);
goto error;
}
-block->mr->align = hpagesize;
-if (memory < hpagesize) {
+if (pagesize == getpagesize()) {
+fprintf(stderr, "Memory is not allocated from HugeTlbfs.\n");
+}
It is strange to see this warning every time.
Shouldn't the differentiation be done explicitly in command line? May be
separate option mem-tlb, or separate flag tlbfs=on, or for new feature -
new option mem-file, or prefixes for paths (tlbfs://, file://).. Or the
other way to not mix things but split them.
+
+block->mr->align = pagesize;
+
+if (memory < pagesize) {
error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
- "or larger than huge page size 0x%" PRIx64,
- memory, hpagesize);
+ "or larger than page size 0x%" PRIx64,
+ memory, pagesize);
goto error;
}
@@ -1247,14 +1225,14 @@ static void *file_ram_alloc(RAMBlock *block,
fd = mkstemp(filename);
if (fd < 0) {
error_setg_errno(errp, errno,
- "unable to create backing store for hugepages");
+ "unable to create backing store for path %s", path);
g_free(filename);
goto error;
}
unlink(filename);
g_free(filename);
-memory = ROUND_UP(memory, hpagesize);
+memory = ROUND_UP(memory, pagesize);
/*
* ftruncate is not supported by hugetlbfs in older
@@ -1266,10 +1244,10 @@ static void *file_ram_alloc(RAMBlock *block,
perror("ftruncate");
}
-area = qemu_ram_mmap(fd, memory, hpagesize, block->flags & RAM_SHARED);
+area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED);
if (area == MAP_FAILED) {
error_setg_errno(errp, errno,
- "unable to map backing store for hugepages");
+ "unable to map backing store for path %s", path);
close(fd);
goto error;
}
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v6 07/33] util: introduce qemu_file_get_page_size()
logic is changed:
in old version gethugepagesize on statfs error generates exit(1)
in new it returns getpagesize() in this case (through fd_getpagesize)
(I think, fd_getpagesize should be fixed to handle error)
also, in new version for windows we have getpagesize(), when in old
version there was no difference (how did it work?). May be it's ok, but
should be mentioned in commit message
On 30.10.2015 08:56, Xiao Guangrong wrote:
There are three places use the some logic to get the page size on
the file path or file fd
This patch introduces qemu_file_get_page_size() to unify the code
Signed-off-by: Xiao Guangrong
---
include/qemu/osdep.h | 1 +
target-ppc/kvm.c | 21 +++--
util/oslib-posix.c | 16
util/oslib-win32.c | 5 +
4 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index b568424..d4dde02 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -302,4 +302,5 @@ int qemu_read_password(char *buf, int buf_size);
*/
pid_t qemu_fork(Error **errp);
+size_t qemu_file_get_page_size(const char *mem_path);
#endif
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index ac70f08..c661f1c 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -308,28 +308,13 @@ static void kvm_get_smmu_info(PowerPCCPU *cpu, struct
kvm_ppc_smmu_info *info)
static long gethugepagesize(const char *mem_path)
{
-struct statfs fs;
-int ret;
-
-do {
-ret = statfs(mem_path, &fs);
-} while (ret != 0 && errno == EINTR);
+long size = qemu_file_get_page_size(mem_path);
-if (ret != 0) {
-fprintf(stderr, "Couldn't statfs() memory path: %s\n",
-strerror(errno));
+if (!size) {
exit(1);
}
-#define HUGETLBFS_MAGIC 0x958458f6
-
-if (fs.f_type != HUGETLBFS_MAGIC) {
-/* Explicit mempath, but it's ordinary pages */
-return getpagesize();
-}
-
-/* It's hugepage, return the huge page size */
-return fs.f_bsize;
+return size;
}
static int find_max_supported_pagesize(Object *obj, void *opaque)
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 914cef5..ad94c5a 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -360,6 +360,22 @@ static size_t fd_getpagesize(int fd)
return getpagesize();
}
+size_t qemu_file_get_page_size(const char *path)
+{
+size_t size = 0;
+int fd = qemu_open(path, O_RDONLY);
+
+if (fd < 0) {
+fprintf(stderr, "Could not open %s.\n", path);
+goto exit;
+}
+
+size = fd_getpagesize(fd);
+qemu_close(fd);
+exit:
+return size;
+}
+
void os_mem_prealloc(int fd, char *area, size_t memory)
{
int ret;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 09f9e98..a18aa87 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -462,6 +462,11 @@ size_t getpagesize(void)
return system_info.dwPageSize;
}
+size_t qemu_file_get_page_size(const char *path)
+{
+return getpagesize();
+}
+
void os_mem_prealloc(int fd, char *area, size_t memory)
{
int i;
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 11/32] hostmem-file: use whole file size if possible
On 11.10.2015 06:52, Xiao Guangrong wrote:
Use the whole file size if @size is not specified which is useful
if we want to directly pass a file to guest
Signed-off-by: Xiao Guangrong
---
backends/hostmem-file.c | 47 +++
1 file changed, 43 insertions(+), 4 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 9097a57..adf2835 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -9,6 +9,9 @@
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
*/
+#include
+#include
+
#include "qemu-common.h"
#include "sysemu/hostmem.h"
#include "sysemu/sysemu.h"
@@ -33,20 +36,56 @@ struct HostMemoryBackendFile {
char *mem_path;
};
+static uint64_t get_file_size(const char *file)
+{
+struct stat stat_buf;
+uint64_t size = 0;
+int fd;
+
+fd = open(file, O_RDONLY);
+if (fd < 0) {
+return 0;
+}
+
+if (stat(file, &stat_buf) < 0) {
+goto exit;
+}
+
+if ((S_ISBLK(stat_buf.st_mode)) && !ioctl(fd, BLKGETSIZE64, &size)) {
+goto exit;
+}
+
+size = lseek(fd, 0, SEEK_END);
+if (size == -1) {
+size = 0;
+}
+exit:
+close(fd);
+return size;
+}
+
static void
file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
{
HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
-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;
}
+if (!backend->size) {
+/*
+ * use the whole file size if @size is not specified.
+ */
+backend->size = get_file_size(fb->mem_path);
+}
+
+if (!backend->size) {
+error_setg(errp, "can't create backend with size 0");
+return;
+}
in case of any error in get_file_size (open, stat, lseek) it will write
about "backend with size 0" which may be not appropriate..
+
backend->force_prealloc = mem_prealloc;
memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
object_get_canonical_path(OBJECT(backend)),
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
