On 4/15/20 4:35 AM, Jingqi Liu 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 uses the interfaces of libdaxctl to fetch 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'.
>
> Suggested-by: Dan Williams <dan.j.willi...@intel.com>
> Signed-off-by: Jingqi Liu <jingqi....@intel.com>
Reviewed-by: Joao Martins <joao.m.mart...@oracle.com>
I also no longer see the previous problem we talked in v1 wrt to ABI differences
between dax_pmem_compat (your case) vs dax_pmem|dax_hmem (my case). The @align
validation also occurs as expected.
> ---
> exec.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/exec.c b/exec.c
> index de9d949902..2c3444e47e 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -77,6 +77,10 @@
>
> #include "monitor/monitor.h"
>
> +#ifdef CONFIG_LIBDAXCTL
> +#include <daxctl/libdaxctl.h>
> +#endif
> +
> //#define DEBUG_SUBPAGE
>
> #if !defined(CONFIG_USER_ONLY)
> @@ -1736,6 +1740,46 @@ 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__) && defined(CONFIG_LIBDAXCTL)
> + 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 *path = NULL;
> + g_autofree char *rpath = NULL;
> + struct daxctl_ctx *ctx;
> + struct daxctl_region *region;
> + int rc = 0;
> +
> + path = g_strdup_printf("/sys/dev/char/%d:%d",
> + major(st.st_rdev), minor(st.st_rdev));
> + rpath = realpath(path, NULL);
> +
> + rc = daxctl_new(&ctx);
> + if (rc) {
> + return -1;
> + }
> +
> + daxctl_region_foreach(ctx, region) {
> + if (strstr(rpath, daxctl_region_get_path(region))) {
> + align = daxctl_region_get_align(region);
> + break;
> + }
> + }
> + daxctl_unref(ctx);
> + }
> +#endif /* defined(__linux__) && defined(CONFIG_LIBDAXCTL) */
> +
> + return align;
> +}
> +
> static int file_ram_open(const char *path,
> const char *region_name,
> bool *created,
> @@ -2275,7 +2319,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 +2355,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;
>
