On 11/06/2015 11:36 PM, Eduardo Habkost wrote:
On Mon, Nov 02, 2015 at 05:13:09PM +0800, Xiao Guangrong wrote:
There are three places use the some logic to get the page size on
the file path or file fd
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
This patch introduces qemu_file_get_page_size() to unify the code
Signed-off-by: Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx>
[...]
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 914cef5..51437ff 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -340,7 +340,7 @@ static void sigbus_handler(int signal)
siglongjmp(sigjump, 1);
}
-static size_t fd_getpagesize(int fd)
+static size_t fd_getpagesize(int fd, Error **errp)
{
#ifdef CONFIG_LINUX
struct statfs fs;
@@ -351,7 +351,12 @@ static size_t fd_getpagesize(int fd)
ret = fstatfs(fd, &fs);
} while (ret != 0 && errno == EINTR);
- if (ret == 0 && fs.f_type == HUGETLBFS_MAGIC) {
+ if (ret) {
+ error_setg_errno(errp, errno, "fstatfs is failed");
+ return 0;
+ }
+
+ if (fs.f_type == HUGETLBFS_MAGIC) {
return fs.f_bsize;
}
You are changing os_mem_prealloc() behavior when fstatfs() fails, here.
Have you ensured there are no cases where fstatfs() fails but this code
is still expected to work?
stat() is supported for all kinds of files, so failed stat() is caused by
file is not exist or kernel internal error (e,g memory is not enough) or
security check is not passed. Whichever we should not do any operation on
the file if stat() failed. The origin code did not check it but it is worth
being fixed i think.
The change looks safe: gethugepagesize() seems to be always called in
the path that would make fd_getpagesize() be called from
os_mem_prealloc(), so allocation would abort much earlier if statfs()
failed. But I haven't confirmed that yet, and I wanted to be sure.
Yes, I am entirely agree with you. :)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
