qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: Andrey Korolyov <andrey@xdel.ru>
Cc: peter.maydell@linaro.org, alex@alex.org.uk,
	"Michael S. Tsirkin" <mst@redhat.com>,
	aik@ozlabs.ru, hutao@cn.fujitsu.com,
	Michael Tokarev <mjt@tls.msk.ru>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	agraf@suse.de, kraxel@redhat.com, pasteka@kabsi.at,
	Stefan Priebe - Profihost AG <s.priebe@profihost.ag>,
	agarcia@igalia.com, armbru@redhat.com, aliguori@amazon.com,
	david@gibson.dropbear.id.au, lersek@redhat.com,
	ehabkost@redhat.com, marcel.a@redhat.com, stefanha@redhat.com,
	cornelia.huck@de.ibm.com, tangchen@cn.fujitsu.com,
	rth@twiddle.net, vasilis.liaskovitis@profitbricks.com,
	Paolo Bonzini <pbonzini@redhat.com>,
	afaerber@suse.de, aurelien@aurel32.net
Subject: Re: [Qemu-devel] [PATCH v2 05/31] vl.c: extend -m option to support options for memory hotplug
Date: Wed, 21 May 2014 10:55:27 +0200	[thread overview]
Message-ID: <20140521105527.2018bec4@nial.usersys.redhat.com> (raw)
In-Reply-To: <CABYiri_OK0eE_BRdZEwjQsWUA=r3FCsz9OjonYC9Qiz-kR0qZw@mail.gmail.com>

On Wed, 21 May 2014 12:27:05 +0400
Andrey Korolyov <andrey@xdel.ru> wrote:

> On Wed, May 21, 2014 at 12:10 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, May 20, 2014 at 05:15:08PM +0200, Igor Mammedov wrote:
> >> Add following parameters:
> >>   "slots" - total number of hotplug memory slots
> >>   "maxmem" - maximum possible memory
> >>
> >> "slots" and "maxmem" should go in pair and "maxmem" should be greater
> >> than "mem" for memory hotplug to be enabled.
> >>
> >> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >
> > Also, it's a bug to mix this with a compat machine type, right?
> > Maybe best to fail initialization if users try this.
> >
> >> ---
> >> v4:
> >>  - store maxmem & slots values in MachineState
> >> v3:
> >>  - store maxmem & slots values in QEMUMachineInitArgs
> >> v2:
> >>  - rebased on top of the latest "vl: convert -m to QemuOpts"
> >> ---
> >>  include/hw/boards.h |    3 ++-
> >>  qemu-options.hx     |    9 ++++++---
> >>  vl.c                |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 59 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/include/hw/boards.h b/include/hw/boards.h
> >> index b62de4a..f6fbbe1 100644
> >> --- a/include/hw/boards.h
> >> +++ b/include/hw/boards.h
> >> @@ -8,7 +8,6 @@
> >>  #include "hw/qdev.h"
> >>  #include "qom/object.h"
> >>
> >> -
> >>  typedef struct MachineState MachineState;
> >>
> >>  typedef void QEMUMachineInitFunc(MachineState *ms);
> >> @@ -113,6 +112,8 @@ struct MachineState {
> >>      char *firmware;
> >>
> >>      ram_addr_t ram_size;
> >> +    ram_addr_t maxram_size;
> >> +    uint64_t   ram_slots;
> >>      const char *boot_order;
> >>      const char *kernel_filename;
> >>      const char *kernel_cmdline;
> >> diff --git a/qemu-options.hx b/qemu-options.hx
> >> index 781af14..c6411c4 100644
> >> --- a/qemu-options.hx
> >> +++ b/qemu-options.hx
> >> @@ -210,17 +210,20 @@ use is discouraged as it may be removed from future versions.
> >>  ETEXI
> >>
> >>  DEF("m", HAS_ARG, QEMU_OPTION_m,
> >> -    "-m [size=]megs\n"
> >> +    "-m[emory] [size=]megs[,slots=n,maxmem=size]\n"
> >>      "                configure guest RAM\n"
> >>      "                size: initial amount of guest memory (default: "
> >> -    stringify(DEFAULT_RAM_SIZE) "MiB)\n",
> >> +    stringify(DEFAULT_RAM_SIZE) "MiB)\n"
> >> +    "                slots: number of hotplug slots (default: none)\n"
> >> +    "                maxmem: maximum amount of guest memory (default: none)\n",
> >>      QEMU_ARCH_ALL)
> >>  STEXI
> >>  @item -m [size=]@var{megs}
> >>  @findex -m
> >>  Set virtual RAM size to @var{megs} megabytes. Default is 128 MiB.  Optionally,
> >>  a suffix of ``M'' or ``G'' can be used to signify a value in megabytes or
> >> -gigabytes respectively.
> >> +gigabytes respectively. Optional pair @var{slots}, @var{maxmem} could be used
> >> +to set amount of hotluggable memory slots and possible maximum amount of memory.
> >>  ETEXI
> >>
> >>  DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
> >> diff --git a/vl.c b/vl.c
> >> index 8fd4ed9..9fb6fa4 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -520,6 +520,14 @@ static QemuOptsList qemu_mem_opts = {
> >>              .name = "size",
> >>              .type = QEMU_OPT_SIZE,
> >>          },
> >> +        {
> >> +            .name = "slots",
> >> +            .type = QEMU_OPT_NUMBER,
> >> +        },
> >> +        {
> >> +            .name = "maxmem",
> >> +            .type = QEMU_OPT_SIZE,
> >> +        },
> >>          { /* end of list */ }
> >>      },
> >>  };
> >> @@ -2989,6 +2997,8 @@ int main(int argc, char **argv, char **envp)
> >>      const char *trace_file = NULL;
> >>      const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
> >>                                          1024 * 1024;
> >> +    ram_addr_t maxram_size = default_ram_size;
> >> +    uint64_t ram_slots = 0;
> >>
> >>      atexit(qemu_run_exit_notifiers);
> >>      error_set_progname(argv[0]);
> >> @@ -3324,6 +3334,7 @@ int main(int argc, char **argv, char **envp)
> >>              case QEMU_OPTION_m: {
> >>                  uint64_t sz;
> >>                  const char *mem_str;
> >> +                const char *maxmem_str, *slots_str;
> >>
> >>                  opts = qemu_opts_parse(qemu_find_opts("memory"),
> >>                                         optarg, 1);
> >> @@ -3365,6 +3376,44 @@ int main(int argc, char **argv, char **envp)
> >>                      error_report("ram size too large");
> >>                      exit(EXIT_FAILURE);
> >>                  }
> >> +
> >> +                maxmem_str = qemu_opt_get(opts, "maxmem");
> >> +                slots_str = qemu_opt_get(opts, "slots");
> >> +                if (maxmem_str && slots_str) {
> >> +                    uint64_t slots;
> >> +
> >> +                    sz = qemu_opt_get_size(opts, "maxmem", 0);
> >> +                    if (sz < ram_size) {
> >> +                        fprintf(stderr, "qemu: invalid -m option value: maxmem "
> >> +                                "(%" PRIu64 ") <= initial memory (%"
> >> +                                PRIu64 ")\n", sz, ram_size);
> >> +                        exit(EXIT_FAILURE);
> >> +                    }
> >> +
> >> +                    slots = qemu_opt_get_number(opts, "slots", 0);
> >> +                    if ((sz > ram_size) && !slots) {
> >> +                        fprintf(stderr, "qemu: invalid -m option value: maxmem "
> >> +                                "(%" PRIu64 ") more than initial memory (%"
> >> +                                PRIu64 ") but no hotplug slots where "
> >> +                                "specified\n", sz, ram_size);
> >> +                        exit(EXIT_FAILURE);
> >> +                    }
> >> +
> >> +                    if ((sz <= ram_size) && slots) {
> >> +                        fprintf(stderr, "qemu: invalid -m option value:  %"
> >> +                                PRIu64 " hotplug slots where specified but "
> >> +                                "maxmem (%" PRIu64 ") <= initial memory (%"
> >> +                                PRIu64 ")\n", slots, sz, ram_size);
> >> +                        exit(EXIT_FAILURE);
> >> +                    }
> >> +                    maxram_size = sz;
> >> +                    ram_slots = slots;
> >> +                } else if ((!maxmem_str && slots_str) ||
> >> +                           (maxmem_str && !slots_str)) {
> >> +                    fprintf(stderr, "qemu: invalid -m option value: missing "
> >> +                            "'%s' option\n", slots_str ? "maxmem" : "slots");
> >> +                    exit(EXIT_FAILURE);
> >> +                }
> >>                  break;
> >>              }
> >>  #ifdef CONFIG_TPM
> >> @@ -4422,6 +4471,8 @@ int main(int argc, char **argv, char **envp)
> >>      qdev_machine_init();
> >>
> >>      current_machine->ram_size = ram_size;
> >> +    current_machine->maxram_size = maxram_size;
> >> +    current_machine->ram_slots = ram_slots;
> >>      current_machine->boot_order = boot_order;
> >>      current_machine->kernel_filename = kernel_filename;
> >>      current_machine->kernel_cmdline = kernel_cmdline;
> >> --
> >> 1.7.1
> 
> May be I am adding very userish opinion, but ability to specify slot
> state via cmdline explicitly, like in
> https://github.com/vliaskov/qemu-kvm/tree/memhp-v5-wip, looks better
> in sight of thoughts of future integration in libvirt and for unplug
> option (where knowledge of which regions are offlined is necessary to
> do the job).
slots are 'not populated' by default, and if specific slot should be
populated then it should have corresponding "-device dimm,slot=XXX"
on QEMU CLI.
There is not much point to specify on CLI not present DIMMs, that way
it would be less confusing and user won't have to worry about not
present DIMMs options at startup (slot/size/addr/node). That makes
VM configuration flexible and allows user to decide parameters at runtime.

  reply	other threads:[~2014-05-21  8:56 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-20 15:15 [Qemu-devel] [PATCH v2 00/31] pc: ACPI memory hotplug Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 01/31] pc: ACPI BIOS: use enum for defining memory affinity flags Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 02/31] object_add: allow completion handler to get canonical path Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 03/31] vl.c: daemonize before guest memory allocation Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 04/31] add memdev backend infrastructure Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 05/31] vl.c: extend -m option to support options for memory hotplug Igor Mammedov
2014-05-21  8:10   ` Michael S. Tsirkin
2014-05-21  8:26     ` Igor Mammedov
2014-05-21  8:27     ` Andrey Korolyov
2014-05-21  8:55       ` Igor Mammedov [this message]
2014-05-21  9:12         ` Andrey Korolyov
2014-05-21  9:52           ` Igor Mammedov
2014-05-21 10:04             ` Andrey Korolyov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 06/31] pc: create custom generic PC machine type Igor Mammedov
2014-05-20 15:55   ` Marcel Apfelbaum
2014-05-21  7:30     ` Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 07/31] qdev: hotplug for buss-less devices Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 08/31] qdev: expose DeviceState.hotplugged field as a property Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 09/31] dimm: implement dimm device abstraction Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 10/31] memory: add memory_region_is_mapped() API Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 11/31] dimm: do not allow to set already used memdev Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 12/31] pc: initialize memory hotplug address space Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 13/31] pc: exit QEMU if number of slots more than supported 256 Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 14/31] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 15/31] pc: add memory hotplug handler to PC_MACHINE Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 16/31] dimm: add busy address check and address auto-allocation Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 17/31] dimm: add busy slot check and slot auto-allocation Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 18/31] acpi: rename cpu_hotplug_defs.h to acpi_defs.h Igor Mammedov
2014-05-20 15:35   ` Michael S. Tsirkin
2014-05-20 16:03     ` Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 19/31] acpi: memory hotplug ACPI hardware implementation Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 20/31] trace: add acpi memory hotplug IO region events Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 21/31] trace: pc: add DIMM slot & address allocation Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 24/31] pc: ich9 lpc: make it work with global/compat properties Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 29/31] pc: ACPI BIOS: implement memory hotplug interface Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 30/31] pc: ACPI BIOS: reserve SRAT entry for hotplug mem hole Igor Mammedov
2014-05-20 15:38   ` Michael S. Tsirkin
2014-05-21  7:56     ` Igor Mammedov
2014-05-21  8:02       ` Michael S. Tsirkin
2014-05-21  8:05   ` Michael S. Tsirkin
2014-05-21 11:22     ` Igor Mammedov
2014-05-21 12:44       ` Michael S. Tsirkin
2014-05-21 13:56         ` Igor Mammedov
2014-05-21 15:01           ` Michael S. Tsirkin
2014-05-21 15:17             ` Igor Mammedov
2014-05-20 15:15 ` [Qemu-devel] [PATCH v2 31/31] pc: ACPI BIOS: make GPE.3 handle memory hotplug event on PIIX and Q35 machines Igor Mammedov
2014-05-21 11:29 ` [Qemu-devel] [PATCH v2 22/31] acpi:piix4: allow plug/unlug callbacks handle not only PCI devices Igor Mammedov
2014-05-21 11:29 ` [Qemu-devel] [PATCH v2 23/31] acpi:piix4: add memory hotplug handling Igor Mammedov
2014-05-21 11:29 ` [Qemu-devel] [PATCH v2 25/31] acpi:ich9: " Igor Mammedov
2014-05-21 11:29 ` [Qemu-devel] [PATCH v2 26/31] pc: migrate piix4 & ich9 MemHotplugState Igor Mammedov
2014-05-23 15:11   ` Andrey Korolyov
2014-05-23 15:41     ` Igor Mammedov
2014-05-21 11:29 ` [Qemu-devel] [PATCH v2 27/31] pc: add acpi-device link to PCMachineState Igor Mammedov
2014-05-21 11:29 ` [Qemu-devel] [PATCH v2 28/31] pc: propagate memory hotplug event to ACPI device Igor Mammedov

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=20140521105527.2018bec4@nial.usersys.redhat.com \
    --to=imammedo@redhat.com \
    --cc=afaerber@suse.de \
    --cc=agarcia@igalia.com \
    --cc=agraf@suse.de \
    --cc=aik@ozlabs.ru \
    --cc=alex@alex.org.uk \
    --cc=aliguori@amazon.com \
    --cc=andrey@xdel.ru \
    --cc=armbru@redhat.com \
    --cc=aurelien@aurel32.net \
    --cc=cornelia.huck@de.ibm.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=ehabkost@redhat.com \
    --cc=hutao@cn.fujitsu.com \
    --cc=kraxel@redhat.com \
    --cc=lersek@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=mjt@tls.msk.ru \
    --cc=mst@redhat.com \
    --cc=pasteka@kabsi.at \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=s.priebe@profihost.ag \
    --cc=stefanha@redhat.com \
    --cc=tangchen@cn.fujitsu.com \
    --cc=vasilis.liaskovitis@profitbricks.com \
    /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).