| Message ID | 87vbih6u7a.fsf@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 04 Mar 2015 14:00:17 +0530 Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > Markus Armbruster <armbru@redhat.com> writes: > > > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: > > > >> diff --git a/vl.c b/vl.c > >> index eb89d62..dd56754 100644 > >> --- a/vl.c > >> +++ b/vl.c > >> @@ -4053,6 +4053,18 @@ int main(int argc, char **argv, char **envp) > >> exit(1); > >> } > >> > >> + if (machine_class->default_ram_size && ram_size < machine_class->default_ram_size) { > >> + fprintf(stderr, "qemu: %s guest ram size defaulting to %ld MB\n", > >> + machine_class->name, > >> + machine_class->default_ram_size / (1024 * 1024)); > > > > If the user explicitly asks for something, we either provide it > > silently, or we error out. This does neither. Why? > > In case the user has provided memory not enough to boot the machine, I > could error out. My idea was to have a sane default which is provided by > the machine. > > Initially, I had just "ram_size == default_ram_size", but then it was > allowing "-m 128M" to go through. And the VM would not boot. > > This can as well be converted to an error report and fail here to boot > the VM. What does exactly fail with 128MB? Linux? SLOF? IIRC, a couple of years ago, 128MB were enough to boot a Linux guest in the spapr machine... Thomas
Thomas Huth <thuth@linux.vnet.ibm.com> writes: > On Wed, 04 Mar 2015 14:00:17 +0530 > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > >> Markus Armbruster <armbru@redhat.com> writes: >> >> > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: >> > >> >> diff --git a/vl.c b/vl.c >> >> index eb89d62..dd56754 100644 >> >> --- a/vl.c >> >> +++ b/vl.c >> >> @@ -4053,6 +4053,18 @@ int main(int argc, char **argv, char **envp) >> >> exit(1); >> >> } >> >> >> >> + if (machine_class->default_ram_size && ram_size < machine_class->default_ram_size) { >> >> + fprintf(stderr, "qemu: %s guest ram size defaulting to %ld MB\n", >> >> + machine_class->name, >> >> + machine_class->default_ram_size / (1024 * 1024)); >> > >> > If the user explicitly asks for something, we either provide it >> > silently, or we error out. This does neither. Why? >> >> In case the user has provided memory not enough to boot the machine, I >> could error out. My idea was to have a sane default which is provided by >> the machine. >> >> Initially, I had just "ram_size == default_ram_size", but then it was >> allowing "-m 128M" to go through. And the VM would not boot. >> >> This can as well be converted to an error report and fail here to boot >> the VM. > > What does exactly fail with 128MB? Linux? Linux kernel, and not much info as well on the console. Regards Nikunj
On Wed, 04 Mar 2015 14:34:27 +0530 Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > Thomas Huth <thuth@linux.vnet.ibm.com> writes: > > > On Wed, 04 Mar 2015 14:00:17 +0530 > > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > > > >> Markus Armbruster <armbru@redhat.com> writes: > >> > >> > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: > >> > > >> >> diff --git a/vl.c b/vl.c > >> >> index eb89d62..dd56754 100644 > >> >> --- a/vl.c > >> >> +++ b/vl.c > >> >> @@ -4053,6 +4053,18 @@ int main(int argc, char **argv, char **envp) > >> >> exit(1); > >> >> } > >> >> > >> >> + if (machine_class->default_ram_size && ram_size < machine_class->default_ram_size) { > >> >> + fprintf(stderr, "qemu: %s guest ram size defaulting to %ld MB\n", > >> >> + machine_class->name, > >> >> + machine_class->default_ram_size / (1024 * 1024)); > >> > > >> > If the user explicitly asks for something, we either provide it > >> > silently, or we error out. This does neither. Why? > >> > >> In case the user has provided memory not enough to boot the machine, I > >> could error out. My idea was to have a sane default which is provided by > >> the machine. > >> > >> Initially, I had just "ram_size == default_ram_size", but then it was > >> allowing "-m 128M" to go through. And the VM would not boot. > >> > >> This can as well be converted to an error report and fail here to boot > >> the VM. > > > > What does exactly fail with 128MB? Linux? > > Linux kernel, and not much info as well on the console. Ok, but then I think it should still be possible to specify -m 128M on the command line - in case the user wants to run an older Linux which still works fine with that amount of memory. Thomas
Thomas Huth <thuth@linux.vnet.ibm.com> writes: > On Wed, 04 Mar 2015 14:34:27 +0530 > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > >> Thomas Huth <thuth@linux.vnet.ibm.com> writes: >> >> > On Wed, 04 Mar 2015 14:00:17 +0530 >> > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: >> > >> >> Markus Armbruster <armbru@redhat.com> writes: >> >> >> >> > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: >> >> > >> >> >> diff --git a/vl.c b/vl.c >> >> >> index eb89d62..dd56754 100644 >> >> >> --- a/vl.c >> >> >> +++ b/vl.c >> >> >> @@ -4053,6 +4053,18 @@ int main(int argc, char **argv, char **envp) >> >> >> exit(1); >> >> >> } >> >> >> >> >> >> + if (machine_class->default_ram_size && ram_size < machine_class->default_ram_size) { >> >> >> + fprintf(stderr, "qemu: %s guest ram size defaulting to %ld MB\n", >> >> >> + machine_class->name, >> >> >> + machine_class->default_ram_size / (1024 * 1024)); >> >> > >> >> > If the user explicitly asks for something, we either provide it >> >> > silently, or we error out. This does neither. Why? >> >> >> >> In case the user has provided memory not enough to boot the machine, I >> >> could error out. My idea was to have a sane default which is provided by >> >> the machine. >> >> >> >> Initially, I had just "ram_size == default_ram_size", but then it was >> >> allowing "-m 128M" to go through. And the VM would not boot. >> >> >> >> This can as well be converted to an error report and fail here to boot >> >> the VM. >> > >> > What does exactly fail with 128MB? Linux? >> >> Linux kernel, and not much info as well on the console. > > Ok, but then I think it should still be possible to specify -m 128M on > the command line - in case the user wants to run an older Linux which > still works fine with that amount of memory. But how do we distinguish whether its old/new kernel in the distro? And the older kernel will boot with more memory, while the reverse isnt true. Regards Nikunj
On Wed, 04 Mar 2015 14:59:13 +0530 Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > Thomas Huth <thuth@linux.vnet.ibm.com> writes: > > > On Wed, 04 Mar 2015 14:34:27 +0530 > > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > > > >> Thomas Huth <thuth@linux.vnet.ibm.com> writes: > >> > >> > On Wed, 04 Mar 2015 14:00:17 +0530 > >> > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > >> > > >> >> Markus Armbruster <armbru@redhat.com> writes: > >> >> > >> >> > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: > >> >> > > >> >> >> diff --git a/vl.c b/vl.c > >> >> >> index eb89d62..dd56754 100644 > >> >> >> --- a/vl.c > >> >> >> +++ b/vl.c > >> >> >> @@ -4053,6 +4053,18 @@ int main(int argc, char **argv, char **envp) > >> >> >> exit(1); > >> >> >> } > >> >> >> > >> >> >> + if (machine_class->default_ram_size && ram_size < machine_class->default_ram_size) { > >> >> >> + fprintf(stderr, "qemu: %s guest ram size defaulting to %ld MB\n", > >> >> >> + machine_class->name, > >> >> >> + machine_class->default_ram_size / (1024 * 1024)); > >> >> > > >> >> > If the user explicitly asks for something, we either provide it > >> >> > silently, or we error out. This does neither. Why? > >> >> > >> >> In case the user has provided memory not enough to boot the machine, I > >> >> could error out. My idea was to have a sane default which is provided by > >> >> the machine. > >> >> > >> >> Initially, I had just "ram_size == default_ram_size", but then it was > >> >> allowing "-m 128M" to go through. And the VM would not boot. > >> >> > >> >> This can as well be converted to an error report and fail here to boot > >> >> the VM. > >> > > >> > What does exactly fail with 128MB? Linux? > >> > >> Linux kernel, and not much info as well on the console. > > > > Ok, but then I think it should still be possible to specify -m 128M on > > the command line - in case the user wants to run an older Linux which > > still works fine with that amount of memory. > > But how do we distinguish whether its old/new kernel in the distro? > > And the older kernel will boot with more memory, while the reverse isnt > true. True, and it's IMHO certainly ok to increase the default memory size - I just wanted to say that you should not disallow "-m 128M" in case the users know what they are doing. Thomas
Thomas Huth <thuth@linux.vnet.ibm.com> writes: > On Wed, 04 Mar 2015 14:59:13 +0530 > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: > >> Thomas Huth <thuth@linux.vnet.ibm.com> writes: >> >> > On Wed, 04 Mar 2015 14:34:27 +0530 >> > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: >> > >> >> Thomas Huth <thuth@linux.vnet.ibm.com> writes: >> >> >> >> > On Wed, 04 Mar 2015 14:00:17 +0530 >> >> > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> wrote: >> >> > >> >> >> Markus Armbruster <armbru@redhat.com> writes: >> >> >> >> >> >> > Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: >> >> >> > >> >> >> >> diff --git a/vl.c b/vl.c >> >> >> >> index eb89d62..dd56754 100644 >> >> >> >> --- a/vl.c >> >> >> >> +++ b/vl.c >> >> >> >> @@ -4053,6 +4053,18 @@ int main(int argc, char **argv, char **envp) >> >> >> >> exit(1); >> >> >> >> } >> >> >> >> >> >> >> >> + if (machine_class->default_ram_size && ram_size < machine_class->default_ram_size) { >> >> >> >> + fprintf(stderr, "qemu: %s guest ram size defaulting to %ld MB\n", >> >> >> >> + machine_class->name, >> >> >> >> + machine_class->default_ram_size / (1024 * 1024)); >> >> >> > >> >> >> > If the user explicitly asks for something, we either provide it >> >> >> > silently, or we error out. This does neither. Why? >> >> >> >> >> >> In case the user has provided memory not enough to boot the machine, I >> >> >> could error out. My idea was to have a sane default which is provided by >> >> >> the machine. >> >> >> >> >> >> Initially, I had just "ram_size == default_ram_size", but then it was >> >> >> allowing "-m 128M" to go through. And the VM would not boot. >> >> >> >> >> >> This can as well be converted to an error report and fail here to boot >> >> >> the VM. >> >> > >> >> > What does exactly fail with 128MB? Linux? >> >> >> >> Linux kernel, and not much info as well on the console. >> > >> > Ok, but then I think it should still be possible to specify -m 128M on >> > the command line - in case the user wants to run an older Linux which >> > still works fine with that amount of memory. >> >> But how do we distinguish whether its old/new kernel in the distro? >> >> And the older kernel will boot with more memory, while the reverse isnt >> true. > > True, and it's IMHO certainly ok to increase the default memory size - > I just wanted to say that you should not disallow "-m 128M" in case the > users know what they are doing. That becomes a little bit tricky, as 128MB can come from default_ram_size and from the user as well. So here I am only changing the machine class for sPAPR. I have tried few distro installations and none of them boot with qemu default memory(128MB). Moreover, the installer kernel does not tell what went wrong. It silently goes till a point and is stuck. Regards Nikunj
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: > Markus Armbruster <armbru@redhat.com> writes: > >> Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: >> >>> diff --git a/vl.c b/vl.c >>> index eb89d62..dd56754 100644 >>> --- a/vl.c >>> +++ b/vl.c >>> @@ -4053,6 +4053,18 @@ int main(int argc, char **argv, char **envp) >>> exit(1); >>> } >>> >>> + if (machine_class->default_ram_size && ram_size < machine_class->default_ram_size) { >>> + fprintf(stderr, "qemu: %s guest ram size defaulting to %ld MB\n", >>> + machine_class->name, >>> + machine_class->default_ram_size / (1024 * 1024)); >> >> If the user explicitly asks for something, we either provide it >> silently, or we error out. This does neither. Why? > > In case the user has provided memory not enough to boot the machine, I > could error out. My idea was to have a sane default which is provided by > the machine. > > Initially, I had just "ram_size == default_ram_size", but then it was > allowing "-m 128M" to go through. And the VM would not boot. > > This can as well be converted to an error report and fail here to boot > the VM. > >> >>> + ram_size = machine_class->default_ram_size; >>> + >>> + /* if maxram size is not provided in options use machine default */ >>> + if (maxram_size == default_ram_size) { >>> + maxram_size = machine_class->default_ram_size; >>> + } >>> + } >>> + >>> /* store value for the future use */ >>> qemu_opt_set_number(qemu_find_opts_singleton("memory"), "size", ram_size); >> >> Does not apply to master, please name your prerequisite patches or >> rebase. > > > Rebased patch: > > > Introduce machine specific default memory size > > Qemu default memory of 128MB is not enough to boot sPAPR > guest. Introduce a member in the machine class to override the default > memory size enforced by qemu. > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> Ignore this, will be sending a v2
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 23cde20..f6b1137 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1738,6 +1738,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) mc->max_cpus = MAX_CPUS; mc->no_parallel = 1; mc->default_boot_order = NULL; + mc->default_ram_size = SPAPR_DEFAULT_RAM_SIZE; mc->kvm_type = spapr_kvm_type; mc->has_dynamic_sysbus = true; diff --git a/include/hw/boards.h b/include/hw/boards.h index 3ddc449..b2b4698 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -108,6 +108,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/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 716bff4..d401dd0 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -444,6 +444,9 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr, #define SPAPR_VIO_BASE_LIOBN 0x00000000 #define SPAPR_PCI_BASE_LIOBN 0x80000000 +/* Default to 1GB guest ram_size */ +#define SPAPR_DEFAULT_RAM_SIZE (1ULL << 30) + #define RTAS_ERROR_LOG_MAX 2048 typedef struct sPAPRTCETable sPAPRTCETable; diff --git a/vl.c b/vl.c index 801d487..4519ccc 100644 --- a/vl.c +++ b/vl.c @@ -2684,6 +2684,18 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size) exit(EXIT_FAILURE); } + if (machine_class->default_ram_size && ram_size < machine_class->default_ram_size) { + fprintf(stderr, "qemu: %s guest ram size defaulting to %ld MB\n", + machine_class->name, + machine_class->default_ram_size / (1024 * 1024)); + ram_size = machine_class->default_ram_size; + + /* if maxram size is not provided in options use machine default */ + if (maxram_size == default_ram_size) { + maxram_size = machine_class->default_ram_size; + } + } + /* store value for the future use */ qemu_opt_set_number(opts, "size", ram_size, &error_abort); *maxram_size = ram_size;