Re: [Qemu-devel] [PATCH] mempath: add option to specify minimum huge page size

[Marcelo Tosatti <mtosatti@redhat.com>]

On Fri, Mar 07, 2014 at 08:53:50AM +0100, Paolo Bonzini wrote:
> Il 07/03/2014 01:40, Marcelo Tosatti ha scritto:
> >
> >Failing initialization in case hugepage path has
> >hugepage smaller than specified.
> >
> >Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> 
> Why is this needed?  Isn't it just operator error?

Libvirt can be responsible for setting up the hugetlbfs mount.

Are you suggesting that enforcement of this property be moved to 
the software on top of libvirt?

> Perhaps libvirt could add an attribute to its <hugepages/> XML
> element, and could use it to find the appropriate hugetlbfs mount.
> But I don't think this check belongs in QEMU.

http://www.spinics.net/linux/fedora/libvir/msg92526.html

> Also, see the series I posted recently for a complete (and more
> powerful + more extensible) replacement of -mem-path and
> -mem-prealloc.
>
> Paolo
> 
> >diff --git a/exec.c b/exec.c
> >index b69fd29..c95a0f3 100644
> >--- a/exec.c
> >+++ b/exec.c
> >@@ -1034,6 +1034,13 @@ static void *file_ram_alloc(RAMBlock *block,
> >         return NULL;
> >     }
> >
> >+    if (mem_path_min_hpagesize && hpagesize < mem_path_min_hpagesize) {
> >+        fprintf(stderr, "mount point (%s) has page size "
> >+                "(%ld) < (%ld) = min_hpagesize\n", path, hpagesize,
> >+                mem_path_min_hpagesize);
> >+        exit(1);
> >+    }
> >+
> >     if (memory < hpagesize) {
> >         return NULL;
> >     }
> >diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> >index 4cb4b4a..cc9e28a 100644
> >--- a/include/exec/cpu-all.h
> >+++ b/include/exec/cpu-all.h
> >@@ -470,6 +470,7 @@ extern RAMList ram_list;
> >
> > extern const char *mem_path;
> > extern int mem_prealloc;
> >+extern unsigned long int mem_path_min_hpagesize;
> >
> > /* Flags stored in the low bits of the TLB virtual address.  These are
> >    defined so that fast path ram access is all zeros.  */
> >diff --git a/qemu-options.hx b/qemu-options.hx
> >index 56e5fdf..36743e1 100644
> >--- a/qemu-options.hx
> >+++ b/qemu-options.hx
> >@@ -221,9 +221,9 @@ gigabytes respectively.
> > ETEXI
> >
> > DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
> >-    "-mem-path FILE  provide backing storage for guest RAM\n", QEMU_ARCH_ALL)
> >+    "-mem-path [mem-path=]file[,min-hpage-size=value]  provide backing storage for guest RAM\n", QEMU_ARCH_ALL)
> > STEXI
> >-@item -mem-path @var{path}
> >+@item -mem-path [mem-path=]@var{path}[,min-hpage-size=@var{min-hpage-size}]
> > @findex -mem-path
> > Allocate guest RAM from a temporarily created file in @var{path}.
> > ETEXI
> >diff --git a/vl.c b/vl.c
> >index 1d27b34..08f9bee 100644
> >--- a/vl.c
> >+++ b/vl.c
> >@@ -136,6 +136,7 @@ static int display_remote;
> > const char* keyboard_layout = NULL;
> > ram_addr_t ram_size;
> > const char *mem_path = NULL;
> >+unsigned long int mem_path_min_hpagesize;
> > int mem_prealloc = 0; /* force preallocation of physical target memory */
> > int nb_nics;
> > NICInfo nd_table[MAX_NICS];
> >@@ -479,6 +480,22 @@ static QemuOptsList qemu_msg_opts = {
> >     },
> > };
> >
> >+static QemuOptsList qemu_mempath_opts = {
> >+    .name = "mem-path",
> >+    .implied_opt_name = "mem-path",
> >+    .head = QTAILQ_HEAD_INITIALIZER(qemu_mempath_opts.head),
> >+    .desc = {
> >+        {
> >+            .name = "mem-path",
> >+            .type = QEMU_OPT_STRING,
> >+        },{
> >+            .name = "min-hpage-size",
> >+            .type = QEMU_OPT_SIZE,
> >+        },
> >+        { /* end of list */ }
> >+    },
> >+};
> >+
> > /**
> >  * Get machine options
> >  *
> >@@ -2863,6 +2880,7 @@ int main(int argc, char **argv, char **envp)
> >     qemu_add_opts(&qemu_tpmdev_opts);
> >     qemu_add_opts(&qemu_realtime_opts);
> >     qemu_add_opts(&qemu_msg_opts);
> >+    qemu_add_opts(&qemu_mempath_opts);
> >
> >     runstate_init();
> >
> >@@ -3189,9 +3207,16 @@ int main(int argc, char **argv, char **envp)
> >                 }
> >                 break;
> > #endif
> >-            case QEMU_OPTION_mempath:
> >-                mem_path = optarg;
> >+            case QEMU_OPTION_mempath: {
> >+                opts = qemu_opts_parse(qemu_find_opts("mem-path"), optarg, 1);
> >+                if (!opts) {
> >+                    exit(1);
> >+                }
> >+                mem_path = qemu_opt_get(opts, "mem-path");
> >+                mem_path_min_hpagesize = qemu_opt_get_size(opts,
> >+                                                           "min-hpage-size", 0);
> >                 break;
> >+            }
> >             case QEMU_OPTION_mem_prealloc:
> >                 mem_prealloc = 1;
> >                 break;
> >
> >