[Qemu-devel] [REBASE PATCH v5 0/2] Introduce default ram size in MachineClass

[Qemu-devel] [REBASE PATCH v5 0/2] Introduce default ram size in MachineClass

[Nikunj A Dadhania]

Current DEFAULT_RAM_SIZE(128MB) enforced by QEMU would not work for
all machines. Introduce a default_ram_size as part of MachineClass.

The below patches has following behaviour:

1) If the user does not provide "-m" option, machine's default ram
   size will be picked.

2) The default behaviour of "-m" is not changed 

On the side note, there are other cleanup of removing ram_size, slots
and maxmem from vl.c. All these are being parsed by generic code. This
can be moved to machine specific property. I will take a stab at it
next.

Nikunj A Dadhania (2):
  machine: add default_ram_size to machine class
  spapr: override default ram size to 1GB

 hw/core/machine.c     |  9 +++++++++
 hw/ppc/spapr.c        |  1 +
 include/hw/boards.h   |  1 +
 include/qemu-common.h |  6 ++++++
 vl.c                  | 30 ++++++++++++++++--------------
 5 files changed, 33 insertions(+), 14 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Nikunj A Dadhania]

Machines types can have different requirement for default ram
size. Introduce a member in the machine class and set the current
default_ram_size to 128MB.

For QEMUMachine types override the value during the registration of
the machine and for MachineClass introduce the generic class init
setting the default_ram_size.

Add helpers [K,M,G,T,P,E]_BYTE for better readability and easy usage

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/core/machine.c     |  9 +++++++++
 include/hw/boards.h   |  1 +
 include/qemu-common.h |  6 ++++++
 vl.c                  | 30 ++++++++++++++++--------------
 4 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 25c45e6..ac4654e 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -294,6 +294,14 @@ static void machine_init_notify(Notifier *notifier, void *data)
     foreach_dynamic_sysbus_device(error_on_sysbus_device, NULL);
 }
 
+static void machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    /* Default 128 MB as guest ram size */
+    mc->default_ram_size = 128 * M_BYTE;
+}
+
 static void machine_initfn(Object *obj)
 {
     MachineState *ms = MACHINE(obj);
@@ -463,6 +471,7 @@ static const TypeInfo machine_info = {
     .parent = TYPE_OBJECT,
     .abstract = true,
     .class_size = sizeof(MachineClass),
+    .class_init    = machine_class_init,
     .instance_size = sizeof(MachineState),
     .instance_init = machine_initfn,
     .instance_finalize = machine_finalize,
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 1f11881..48a604b 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -119,6 +119,7 @@ struct MachineClass {
     const char *default_display;
     GlobalProperty *compat_props;
     const char *hw_version;
+    ram_addr_t default_ram_size;
 
     HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
                                            DeviceState *dev);
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 1b5cffb..b8eb7cb 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -186,6 +186,12 @@ int64_t strtosz(const char *nptr, char **end);
 int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
 int64_t strtosz_suffix_unit(const char *nptr, char **end,
                             const char default_suffix, int64_t unit);
+#define K_BYTE     (1ULL << 10)
+#define M_BYTE     (1ULL << 20)
+#define G_BYTE     (1ULL << 30)
+#define T_BYTE     (1ULL << 40)
+#define P_BYTE     (1ULL << 50)
+#define E_BYTE     (1ULL << 60)
 
 /* used to print char* safely */
 #define STR_OR_NULL(str) ((str) ? (str) : "null")
diff --git a/vl.c b/vl.c
index 74c2681..e18c78e 100644
--- a/vl.c
+++ b/vl.c
@@ -120,8 +120,6 @@ int main(int argc, char **argv)
 #include "qom/object_interfaces.h"
 #include "qapi-event.h"
 
-#define DEFAULT_RAM_SIZE 128
-
 #define MAX_VIRTIO_CONSOLES 1
 #define MAX_SCLP_CONSOLES 1
 
@@ -1309,7 +1307,11 @@ void hmp_usb_del(Monitor *mon, const QDict *qdict)
 
 MachineState *current_machine;
 
-static void machine_class_init(ObjectClass *oc, void *data)
+/*
+ * Transitional class registration/init used for converting from
+ * legacy QEMUMachine to MachineClass.
+ */
+static void qemu_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
     QEMUMachine *qm = data;
@@ -1347,7 +1349,7 @@ int qemu_register_machine(QEMUMachine *m)
     TypeInfo ti = {
         .name       = name,
         .parent     = TYPE_MACHINE,
-        .class_init = machine_class_init,
+        .class_init = qemu_machine_class_init,
         .class_data = (void *)m,
     };
 
@@ -2643,13 +2645,13 @@ out:
     return 0;
 }
 
-static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size)
+static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
+                               MachineClass *mc)
 {
     uint64_t sz;
     const char *mem_str;
     const char *maxmem_str, *slots_str;
-    const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
-                                        1024 * 1024;
+    const ram_addr_t default_ram_size = mc->default_ram_size;
     QemuOpts *opts = qemu_find_opts_singleton("memory");
 
     sz = 0;
@@ -3765,7 +3767,13 @@ int main(int argc, char **argv, char **envp)
         machine_class = machine_parse(optarg);
     }
 
-    set_memory_options(&ram_slots, &maxram_size);
+    if (machine_class == NULL) {
+        fprintf(stderr, "No machine specified, and there is no default.\n"
+                "Use -machine help to list supported machines!\n");
+        exit(1);
+    }
+
+    set_memory_options(&ram_slots, &maxram_size, machine_class);
 
     loc_set_none();
 
@@ -3794,12 +3802,6 @@ int main(int argc, char **argv, char **envp)
     }
 #endif
 
-    if (machine_class == NULL) {
-        fprintf(stderr, "No machine specified, and there is no default.\n"
-                "Use -machine help to list supported machines!\n");
-        exit(1);
-    }
-
     current_machine = MACHINE(object_new(object_class_get_name(
                           OBJECT_CLASS(machine_class))));
     if (machine_help_func(qemu_get_machine_opts(), current_machine)) {
-- 
1.8.3.1

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Thomas Huth]

On Tue, 28 Apr 2015 12:23:25 +0530
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:

> Machines types can have different requirement for default ram
> size. Introduce a member in the machine class and set the current
> default_ram_size to 128MB.
> 
> For QEMUMachine types override the value during the registration of
> the machine and for MachineClass introduce the generic class init
> setting the default_ram_size.
> 
> Add helpers [K,M,G,T,P,E]_BYTE for better readability and easy usage
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  hw/core/machine.c     |  9 +++++++++
>  include/hw/boards.h   |  1 +
>  include/qemu-common.h |  6 ++++++
>  vl.c                  | 30 ++++++++++++++++--------------
>  4 files changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 25c45e6..ac4654e 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -294,6 +294,14 @@ static void machine_init_notify(Notifier *notifier, void *data)
>      foreach_dynamic_sysbus_device(error_on_sysbus_device, NULL);
>  }
>  
> +static void machine_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    /* Default 128 MB as guest ram size */
> +    mc->default_ram_size = 128 * M_BYTE;
> +}
> +
>  static void machine_initfn(Object *obj)
>  {
>      MachineState *ms = MACHINE(obj);
> @@ -463,6 +471,7 @@ static const TypeInfo machine_info = {
>      .parent = TYPE_OBJECT,
>      .abstract = true,
>      .class_size = sizeof(MachineClass),
> +    .class_init    = machine_class_init,
>      .instance_size = sizeof(MachineState),
>      .instance_init = machine_initfn,
>      .instance_finalize = machine_finalize,
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 1f11881..48a604b 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -119,6 +119,7 @@ struct MachineClass {
>      const char *default_display;
>      GlobalProperty *compat_props;
>      const char *hw_version;
> +    ram_addr_t default_ram_size;
>  
>      HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>                                             DeviceState *dev);
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 1b5cffb..b8eb7cb 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -186,6 +186,12 @@ int64_t strtosz(const char *nptr, char **end);
>  int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
>  int64_t strtosz_suffix_unit(const char *nptr, char **end,
>                              const char default_suffix, int64_t unit);
> +#define K_BYTE     (1ULL << 10)
> +#define M_BYTE     (1ULL << 20)
> +#define G_BYTE     (1ULL << 30)
> +#define T_BYTE     (1ULL << 40)
> +#define P_BYTE     (1ULL << 50)
> +#define E_BYTE     (1ULL << 60)
>  
>  /* used to print char* safely */
>  #define STR_OR_NULL(str) ((str) ? (str) : "null")
> diff --git a/vl.c b/vl.c
> index 74c2681..e18c78e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -120,8 +120,6 @@ int main(int argc, char **argv)
>  #include "qom/object_interfaces.h"
>  #include "qapi-event.h"
>  
> -#define DEFAULT_RAM_SIZE 128
> -
>  #define MAX_VIRTIO_CONSOLES 1
>  #define MAX_SCLP_CONSOLES 1
>  
> @@ -1309,7 +1307,11 @@ void hmp_usb_del(Monitor *mon, const QDict *qdict)
>  
>  MachineState *current_machine;
>  
> -static void machine_class_init(ObjectClass *oc, void *data)
> +/*
> + * Transitional class registration/init used for converting from
> + * legacy QEMUMachine to MachineClass.
> + */
> +static void qemu_machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
>      QEMUMachine *qm = data;
> @@ -1347,7 +1349,7 @@ int qemu_register_machine(QEMUMachine *m)
>      TypeInfo ti = {
>          .name       = name,
>          .parent     = TYPE_MACHINE,
> -        .class_init = machine_class_init,
> +        .class_init = qemu_machine_class_init,
>          .class_data = (void *)m,
>      };
>  
> @@ -2643,13 +2645,13 @@ out:
>      return 0;
>  }
>  
> -static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size)
> +static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
> +                               MachineClass *mc)
>  {
>      uint64_t sz;
>      const char *mem_str;
>      const char *maxmem_str, *slots_str;
> -    const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
> -                                        1024 * 1024;
> +    const ram_addr_t default_ram_size = mc->default_ram_size;
>      QemuOpts *opts = qemu_find_opts_singleton("memory");
>  
>      sz = 0;
> @@ -3765,7 +3767,13 @@ int main(int argc, char **argv, char **envp)
>          machine_class = machine_parse(optarg);
>      }
>  
> -    set_memory_options(&ram_slots, &maxram_size);
> +    if (machine_class == NULL) {
> +        fprintf(stderr, "No machine specified, and there is no default.\n"
> +                "Use -machine help to list supported machines!\n");
> +        exit(1);
> +    }
> +
> +    set_memory_options(&ram_slots, &maxram_size, machine_class);
>  
>      loc_set_none();
>  
> @@ -3794,12 +3802,6 @@ int main(int argc, char **argv, char **envp)
>      }
>  #endif
>  
> -    if (machine_class == NULL) {
> -        fprintf(stderr, "No machine specified, and there is no default.\n"
> -                "Use -machine help to list supported machines!\n");
> -        exit(1);
> -    }
> -
>      current_machine = MACHINE(object_new(object_class_get_name(
>                            OBJECT_CLASS(machine_class))));
>      if (machine_help_func(qemu_get_machine_opts(), current_machine)) {

Looks good to me.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

On Tue, Apr 28, 2015 at 01:30:29PM +0200, Thomas Huth wrote:
> On Tue, 28 Apr 2015 12:23:25 +0530
> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:
[snip]
> Looks good to me.
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Same here,

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Nikunj A Dadhania]

Hi Paolo,

David Gibson <david@gibson.dropbear.id.au> writes:
> On Tue, Apr 28, 2015 at 01:30:29PM +0200, Thomas Huth wrote:
>> On Tue, 28 Apr 2015 12:23:25 +0530
>> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:
> [snip]
>> Looks good to me.
>> 
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
> Same here,
>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Can you push this patch via you tree?

David will push the spapr changes dependent on this patch through his
tree.

https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg04006.html

Regards,
Nikunj

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Paolo Bonzini]

On 29/04/2015 10:14, Nikunj A Dadhania wrote:
> 
> Hi Paolo,
> 
> David Gibson <david@gibson.dropbear.id.au> writes:
>> On Tue, Apr 28, 2015 at 01:30:29PM +0200, Thomas Huth wrote:
>>> On Tue, 28 Apr 2015 12:23:25 +0530
>>> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:
>> [snip]
>>> Looks good to me.
>>>
>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>
>> Same here,
>>
>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Can you push this patch via you tree?
> 
> David will push the spapr changes dependent on this patch through his
> tree.
> 
> https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg04006.html
> 
> Regards,
> Nikunj
> 
> 
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

so David can push both patches.

But isn't 1G a bit too much?  At least on x86 you can easily boot with 512M.

Paolo

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Nikunj A Dadhania]

Hi Paolo,

Paolo Bonzini <pbonzini@redhat.com> writes:
> On 29/04/2015 10:14, Nikunj A Dadhania wrote:
>> 
>> Hi Paolo,
>> 
>> David Gibson <david@gibson.dropbear.id.au> writes:
>>> On Tue, Apr 28, 2015 at 01:30:29PM +0200, Thomas Huth wrote:
>>>> On Tue, 28 Apr 2015 12:23:25 +0530
>>>> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:
>>> [snip]
>>>> Looks good to me.
>>>>
>>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>
>>> Same here,
>>>
>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>> 
>> Can you push this patch via you tree?
>> 
>> David will push the spapr changes dependent on this patch through his
>> tree.
>> 
>> https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg04006.html
>> 
>> Regards,
>> Nikunj
>> 
>
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>
> so David can push both patches.
>
> But isn't 1G a bit too much?  At least on x86 you can easily boot with 512M.

I understood this number as not the _minimum memory_ to boot the
VM. And this will only come in picture when the user has not specified
any memory.

I have seen the PPC64 VM booting with 256M.

Regards,
Nikunj

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Paolo Bonzini]

On 29/04/2015 11:06, Nikunj A Dadhania wrote:
> > so David can push both patches.
> >
> > But isn't 1G a bit too much?  At least on x86 you can easily boot with 512M.
> 
> I understood this number as not the _minimum memory_ to boot the
> VM. And this will only come in picture when the user has not specified
> any memory.

This in turn will basically only happen for QEMU developers.  So keeping
the default on the low side would make sense.

On my (4G memory) laptop I might not even be able to boot a PPC64 VM
with 1G and TCG, but I can do that nicely with 256M.

Paolo

> I have seen the PPC64 VM booting with 256M.

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Nikunj A Dadhania]

Hi Paolo,

Paolo Bonzini <pbonzini@redhat.com> writes:
> On 29/04/2015 11:06, Nikunj A Dadhania wrote:
>> > so David can push both patches.
>> >
>> > But isn't 1G a bit too much?  At least on x86 you can easily boot with 512M.
>> 
>> I understood this number as not the _minimum memory_ to boot the
>> VM. And this will only come in picture when the user has not specified
>> any memory.
>
> This in turn will basically only happen for QEMU developers.  So keeping
> the default on the low side would make sense.
>
> On my (4G memory) laptop I might not even be able to boot a PPC64 VM
> with 1G and TCG, but I can do that nicely with 256M.

That will be fine with me as well, i.e. 256M

David/Alex, Do you have comments on this before we change it?

Regards
Nikunj

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Alexander Graf]

On 30.04.15 06:41, Nikunj A Dadhania wrote:
> 
> Hi Paolo,
> 
> Paolo Bonzini <pbonzini@redhat.com> writes:
>> On 29/04/2015 11:06, Nikunj A Dadhania wrote:
>>>> so David can push both patches.
>>>>
>>>> But isn't 1G a bit too much?  At least on x86 you can easily boot with 512M.
>>>
>>> I understood this number as not the _minimum memory_ to boot the
>>> VM. And this will only come in picture when the user has not specified
>>> any memory.
>>
>> This in turn will basically only happen for QEMU developers.  So keeping
>> the default on the low side would make sense.
>>
>> On my (4G memory) laptop I might not even be able to boot a PPC64 VM
>> with 1G and TCG, but I can do that nicely with 256M.
> 
> That will be fine with me as well, i.e. 256M
> 
> David/Alex, Do you have comments on this before we change it?

I've seen RAM size combinations that seemed to work ok, but then failed
during grub2 execution for example. Please verify with all reasonably
realistically executed distributions that 256MB is enough.


Alex

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Thomas Huth]

On Thu, 30 Apr 2015 11:18:05 +0200
Alexander Graf <agraf@suse.de> wrote:

> 
> 
> On 30.04.15 06:41, Nikunj A Dadhania wrote:
> > 
> > Hi Paolo,
> > 
> > Paolo Bonzini <pbonzini@redhat.com> writes:
> >> On 29/04/2015 11:06, Nikunj A Dadhania wrote:
> >>>> so David can push both patches.
> >>>>
> >>>> But isn't 1G a bit too much?  At least on x86 you can easily boot with 512M.
> >>>
> >>> I understood this number as not the _minimum memory_ to boot the
> >>> VM. And this will only come in picture when the user has not specified
> >>> any memory.
> >>
> >> This in turn will basically only happen for QEMU developers.  So keeping
> >> the default on the low side would make sense.
> >>
> >> On my (4G memory) laptop I might not even be able to boot a PPC64 VM
> >> with 1G and TCG, but I can do that nicely with 256M.
> > 
> > That will be fine with me as well, i.e. 256M
> > 
> > David/Alex, Do you have comments on this before we change it?
> 
> I've seen RAM size combinations that seemed to work ok, but then failed
> during grub2 execution for example. Please verify with all reasonably
> realistically executed distributions that 256MB is enough.

Since this default value will likely be there for the next couple of
years, it's maybe better to use a slightly higher value than one that
is too low - the amount of RAM that a guest requires likely rather
increases in the next years instead of going down again. So I think
using 512 MB instead is maybe a good compromise?

 Thomas

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Alexander Graf]

> Am 30.04.2015 um 11:40 schrieb Thomas Huth <thuth@redhat.com>:
> 
> On Thu, 30 Apr 2015 11:18:05 +0200
> Alexander Graf <agraf@suse.de> wrote:
> 
>> 
>> 
>>> On 30.04.15 06:41, Nikunj A Dadhania wrote:
>>> 
>>> Hi Paolo,
>>> 
>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>> On 29/04/2015 11:06, Nikunj A Dadhania wrote:
>>>>>> so David can push both patches.
>>>>>> 
>>>>>> But isn't 1G a bit too much?  At least on x86 you can easily boot with 512M.
>>>>> 
>>>>> I understood this number as not the _minimum memory_ to boot the
>>>>> VM. And this will only come in picture when the user has not specified
>>>>> any memory.
>>>> 
>>>> This in turn will basically only happen for QEMU developers.  So keeping
>>>> the default on the low side would make sense.
>>>> 
>>>> On my (4G memory) laptop I might not even be able to boot a PPC64 VM
>>>> with 1G and TCG, but I can do that nicely with 256M.
>>> 
>>> That will be fine with me as well, i.e. 256M
>>> 
>>> David/Alex, Do you have comments on this before we change it?
>> 
>> I've seen RAM size combinations that seemed to work ok, but then failed
>> during grub2 execution for example. Please verify with all reasonably
>> realistically executed distributions that 256MB is enough.
> 
> Since this default value will likely be there for the next couple of
> years, it's maybe better to use a slightly higher value than one that
> is too low - the amount of RAM that a guest requires likely rather
> increases in the next years instead of going down again. So I think
> using 512 MB instead is maybe a good compromise?

Again, even with 512, please verify a few different distros and check that they run.

Alex

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Nikunj A Dadhania]

Alexander Graf <agraf@suse.de> writes:
>> Am 30.04.2015 um 11:40 schrieb Thomas Huth <thuth@redhat.com>:
>> On Thu, 30 Apr 2015 11:18:05 +0200
>> Alexander Graf <agraf@suse.de> wrote:
>>>> On 30.04.15 06:41, Nikunj A Dadhania wrote:
>>>> 
>>>> Hi Paolo,
>>>> 
>>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>>> On 29/04/2015 11:06, Nikunj A Dadhania wrote:
>>>>>>> so David can push both patches.
>>>>>>> 
>>>>>>> But isn't 1G a bit too much?  At least on x86 you can easily boot with 512M.
>>>>>> 
>>>>>> I understood this number as not the _minimum memory_ to boot the
>>>>>> VM. And this will only come in picture when the user has not specified
>>>>>> any memory.
>>>>> 
>>>>> This in turn will basically only happen for QEMU developers.  So keeping
>>>>> the default on the low side would make sense.
>>>>> 
>>>>> On my (4G memory) laptop I might not even be able to boot a PPC64 VM
>>>>> with 1G and TCG, but I can do that nicely with 256M.
>>>> 
>>>> That will be fine with me as well, i.e. 256M
>>>> 
>>>> David/Alex, Do you have comments on this before we change it?
>>> 
>>> I've seen RAM size combinations that seemed to work ok, but then failed
>>> during grub2 execution for example. Please verify with all reasonably
>>> realistically executed distributions that 256MB is enough.
>> 
>> Since this default value will likely be there for the next couple of
>> years, it's maybe better to use a slightly higher value than one that
>> is too low - the amount of RAM that a guest requires likely rather
>> increases in the next years instead of going down again. So I think
>> using 512 MB instead is maybe a good compromise?
>
> Again, even with 512, please verify a few different distros and check that they run.

Verified the few distro images available in my virt-test setup, we boot
fine with 512MB memory. Will send an updated patch. 

Regards
Nikunj

Re: [Qemu-devel] [REBASE PATCH v5 1/2] machine: add default_ram_size to machine class

[Paolo Bonzini]

On 30/04/2015 11:40, Thomas Huth wrote:
>>>> On 29/04/2015 11:06, Nikunj A Dadhania wrote:
>>>>>> so David can push both patches.
>>>>>>
>>>>>> But isn't 1G a bit too much?  At least on x86 you can easily boot with 512M.
>>>>>
>>>>> I understood this number as not the _minimum memory_ to boot the
>>>>> VM. And this will only come in picture when the user has not specified
>>>>> any memory.
>>>>
>>>> This in turn will basically only happen for QEMU developers.  So keeping
>>>> the default on the low side would make sense.
>>>>
>>>> On my (4G memory) laptop I might not even be able to boot a PPC64 VM
>>>> with 1G and TCG, but I can do that nicely with 256M.
>>>
>>> That will be fine with me as well, i.e. 256M
>>>
>>> David/Alex, Do you have comments on this before we change it?
>>
>> I've seen RAM size combinations that seemed to work ok, but then failed
>> during grub2 execution for example. Please verify with all reasonably
>> realistically executed distributions that 256MB is enough.
> 
> Since this default value will likely be there for the next couple of
> years, it's maybe better to use a slightly higher value than one that
> is too low - the amount of RAM that a guest requires likely rather
> increases in the next years instead of going down again. So I think
> using 512 MB instead is maybe a good compromise?

Sure, 512 is okay with me.

Paolo

[Qemu-devel] [REBASE PATCH v5 2/2] spapr: override default ram size to 1GB

[Nikunj A Dadhania]

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/ppc/spapr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 61ddc79..6616650 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1794,6 +1794,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = MAX_CPUS;
     mc->no_parallel = 1;
     mc->default_boot_order = "";
+    mc->default_ram_size = 1 * G_BYTE;
     mc->kvm_type = spapr_kvm_type;
     mc->has_dynamic_sysbus = true;
 
-- 
1.8.3.1

Re: [Qemu-devel] [REBASE PATCH v5 2/2] spapr: override default ram size to 1GB

[Thomas Huth]

On Tue, 28 Apr 2015 12:23:26 +0530
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:

> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/ppc/spapr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 61ddc79..6616650 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1794,6 +1794,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      mc->max_cpus = MAX_CPUS;
>      mc->no_parallel = 1;
>      mc->default_boot_order = "";
> +    mc->default_ram_size = 1 * G_BYTE;
>      mc->kvm_type = spapr_kvm_type;
>      mc->has_dynamic_sysbus = true;
>  

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [REBASE PATCH v5 2/2] spapr: override default ram size to 1GB

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

On Tue, Apr 28, 2015 at 01:32:15PM +0200, Thomas Huth wrote:
> On Tue, 28 Apr 2015 12:23:26 +0530
> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote:
> 
> > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  hw/ppc/spapr.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 61ddc79..6616650 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1794,6 +1794,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> >      mc->max_cpus = MAX_CPUS;
> >      mc->no_parallel = 1;
> >      mc->default_boot_order = "";
> > +    mc->default_ram_size = 1 * G_BYTE;
> >      mc->kvm_type = spapr_kvm_type;
> >      mc->has_dynamic_sysbus = true;
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

If patch 1/2 is merged I'm happy to take this one through the
spapr-next tree.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]