Re: [Qemu-devel] [PATCH v6 07/33] util: introduce qemu_file_get_page_size()
On 10/31/2015 05:37 PM, Vladimir Sementsov-Ogievskiy wrote: 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. Yup, it's better, however, i am planning to introduce a Error parameter and error condition can be checked by if Error is NULL as Eduardo suggested. -- 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 07/33] util: introduce qemu_file_get_page_size()
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. 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. -- 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
