On 07/04/20 09:29, Liu, Jingqi wrote: > Ping. > > Any comments are appreciated. > > Hi Paolo, Richard, > > Any comments about this ?
I was hoping to get a review from someone else because I have no way to test it. But I've now queued the patch, thanks. Paolo > > Thanks, > > Jingqi > > On 4/1/2020 11:13 AM, Liu, Jingqi wrote: >> If the backend file is devdax pmem character device, the alignment >> specified by the option 'align=NUM' in the '-object memory-backend-file' >> needs to match the alignment requirement of the devdax pmem character >> device. >> >> This patch fetches the devdax pmem file 'align', so that we can compare >> it with the NUM of 'align=NUM'. >> The NUM needs to be larger than or equal to the devdax pmem file 'align'. >> >> It also fixes the problem that mmap() returns failure in qemu_ram_mmap() >> when the NUM of 'align=NUM' is less than the devdax pmem file 'align'. >> >> Cc: Dan Williams <dan.j.willi...@intel.com> >> Signed-off-by: Jingqi Liu <jingqi....@intel.com> >> --- >> exec.c | 46 +++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 45 insertions(+), 1 deletion(-) >> >> diff --git a/exec.c b/exec.c >> index de9d949902..8221abffec 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -1736,6 +1736,42 @@ static int64_t get_file_size(int fd) >> return size; >> } >> +static int64_t get_file_align(int fd) >> +{ >> + int64_t align = -1; >> +#if defined(__linux__) >> + struct stat st; >> + >> + if (fstat(fd, &st) < 0) { >> + return -errno; >> + } >> + >> + /* Special handling for devdax character devices */ >> + if (S_ISCHR(st.st_mode)) { >> + g_autofree char *subsystem_path = NULL; >> + g_autofree char *subsystem = NULL; >> + >> + subsystem_path = >> g_strdup_printf("/sys/dev/char/%d:%d/subsystem", >> + major(st.st_rdev), >> minor(st.st_rdev)); >> + subsystem = g_file_read_link(subsystem_path, NULL); >> + >> + if (subsystem && g_str_has_suffix(subsystem, "/dax")) { >> + g_autofree char *align_path = NULL; >> + g_autofree char *align_str = NULL; >> + >> + align_path = >> g_strdup_printf("/sys/dev/char/%d:%d/device/align", >> + major(st.st_rdev), >> minor(st.st_rdev)); >> + >> + if (g_file_get_contents(align_path, &align_str, NULL, >> NULL)) { >> + return g_ascii_strtoll(align_str, NULL, 0); >> + } >> + } >> + } >> +#endif /* defined(__linux__) */ >> + >> + return align; >> +} >> + >> static int file_ram_open(const char *path, >> const char *region_name, >> bool *created, >> @@ -2275,7 +2311,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t >> size, MemoryRegion *mr, >> { >> RAMBlock *new_block; >> Error *local_err = NULL; >> - int64_t file_size; >> + int64_t file_size, file_align; >> /* Just support these ram flags by now. */ >> assert((ram_flags & ~(RAM_SHARED | RAM_PMEM)) == 0); >> @@ -2311,6 +2347,14 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t >> size, MemoryRegion *mr, >> return NULL; >> } >> + file_align = get_file_align(fd); >> + if (file_align > 0 && mr && file_align > mr->align) { >> + error_setg(errp, "backing store align 0x%" PRIx64 >> + " is larger than 'align' option 0x" RAM_ADDR_FMT, >> + file_align, mr->align); >> + return NULL; >> + } >> + >> new_block = g_malloc0(sizeof(*new_block)); >> new_block->mr = mr; >> new_block->used_length = size; >