Re: [Qemu-devel] [PATCH v6 07/33] util: introduce qemu_file_get_page_size()

[Xiao Guangrong]

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()

[Vladimir Sementsov-Ogievskiy]

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()

[Xiao Guangrong]

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()

[Vladimir Sementsov-Ogievskiy]

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, );
-} 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