| Message ID | 1430204006-10160-2-git-send-email-nikunj@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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>
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>
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
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
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
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.
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
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
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
> 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
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
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
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)) {
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(-)