qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Marcelo Tosatti <mtosatti@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>
Cc: Igor Mammedov <imammedo@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] mempath: add option to specify minimum huge page size
Date: Fri, 07 Mar 2014 08:53:50 +0100	[thread overview]
Message-ID: <53197B0E.7030906@redhat.com> (raw)
In-Reply-To: <20140307004004.GA6114@amt.cnet>

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?

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.

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;
>
>

  parent reply	other threads:[~2014-03-07  7:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-07  0:40 [Qemu-devel] [PATCH] mempath: add option to specify minimum huge page size Marcelo Tosatti
2014-03-07  4:21 ` Eric Blake
2014-03-07 15:13   ` Marcelo Tosatti
2014-03-07 15:37     ` Eric Blake
2014-03-07  7:53 ` Paolo Bonzini [this message]
2014-03-07 15:23   ` Marcelo Tosatti

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53197B0E.7030906@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).