diff mbox series

[14/15] null-machine: do not create a default memdev

Message ID 20201202081854.4126071-15-pbonzini@redhat.com
State New
Headers show
Series Finish cleaning up qemu_init | expand

Commit Message

Paolo Bonzini Dec. 2, 2020, 8:18 a.m. UTC
The default RAM size is 0, so no RAM will be created anyway.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/null-machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Igor Mammedov Dec. 7, 2020, 4:43 p.m. UTC | #1
On Wed,  2 Dec 2020 03:18:53 -0500
Paolo Bonzini <pbonzini@redhat.com> wrote:

> The default RAM size is 0, so no RAM will be created anyway.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/null-machine.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
> index 7e693523d7..c40badf5bc 100644
> --- a/hw/core/null-machine.c
> +++ b/hw/core/null-machine.c
> @@ -49,7 +49,7 @@ static void machine_none_machine_init(MachineClass *mc)
>      mc->init = machine_none_init;
>      mc->max_cpus = 1;
>      mc->default_ram_size = 0;
> -    mc->default_ram_id = "ram";
> +    mc->default_ram_id = NULL;

probably that will break:

 QEMU -m X -M none


maybe there is  a bug over there but
    "mc->default_ram_size = 0"
above, should result in
    current_machine->ram_size == 0
in case user hasn't provided "-m"
and hence memdev shouldn't be created

>      mc->no_serial = 1;
>      mc->no_parallel = 1;
>      mc->no_floppy = 1;
Paolo Bonzini Dec. 11, 2020, 11:24 p.m. UTC | #2
On 07/12/20 17:43, Igor Mammedov wrote:
>>       mc->default_ram_size = 0;
>> -    mc->default_ram_id = "ram";
>> +    mc->default_ram_id = NULL;
> probably that will break:
> 
>   QEMU -m X -M none

No, it works.  "-m" is simply ignored, because the default memdev is 
created here:

     if (machine_class->default_ram_id && current_machine->ram_size &&
         numa_uses_legacy_mem() && !current_machine->ram_memdev_id) {
         create_default_memdev(current_machine, mem_path);
     }

and is thus skipped for -M none.

Paolo

> 
> maybe there is  a bug over there but
>      "mc->default_ram_size = 0"
> above, should result in
>      current_machine->ram_size == 0
> in case user hasn't provided "-m"
> and hence memdev shouldn't be created
>
Igor Mammedov Dec. 14, 2020, 11:53 a.m. UTC | #3
On Sat, 12 Dec 2020 00:24:25 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 07/12/20 17:43, Igor Mammedov wrote:
> >>       mc->default_ram_size = 0;
> >> -    mc->default_ram_id = "ram";
> >> +    mc->default_ram_id = NULL;  
> > probably that will break:
> > 
> >   QEMU -m X -M none  
> 
> No, it works.  "-m" is simply ignored, because the default memdev is 
> created here:
you mean it doesn't explode.
By default with current code memdev should not be created, 
but it is created if users asks for it with -m.
This patch breaks the later.

I'm not sure that any RAM on null machine is of any use at all
but the if we decide to get rid of it completely, then we need
to clean up
    /* RAM at address zero */                                                    
    if (mch->ram) {                                                              
        memory_region_add_subregion(get_system_memory(), 0, mch->ram);           
    }   
and commit title/message should match what patch does and why.


>      if (machine_class->default_ram_id && current_machine->ram_size &&
>          numa_uses_legacy_mem() && !current_machine->ram_memdev_id) {
>          create_default_memdev(current_machine, mem_path);
>      }
> 
> and is thus skipped for -M none.
> 
> Paolo
> 
> > 
> > maybe there is  a bug over there but
> >      "mc->default_ram_size = 0"
> > above, should result in
> >      current_machine->ram_size == 0
> > in case user hasn't provided "-m"
> > and hence memdev shouldn't be created
> >   
>
Paolo Bonzini Dec. 14, 2020, 1:24 p.m. UTC | #4
On 14/12/20 12:53, Igor Mammedov wrote:
> On Sat, 12 Dec 2020 00:24:25 +0100
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> On 07/12/20 17:43, Igor Mammedov wrote:
>>>>        mc->default_ram_size = 0;
>>>> -    mc->default_ram_id = "ram";
>>>> +    mc->default_ram_id = NULL;
>>> probably that will break:
>>>
>>>    QEMU -m X -M none
>>
>> No, it works.  "-m" is simply ignored, because the default memdev is
>> created here:
> you mean it doesn't explode.
> By default with current code memdev should not be created,
> but it is created if users asks for it with -m.

Oh ok, got it.  I'll leave this patch aside for now and later, when I 
get round to posting the ram_memdev_id cleanup, try to remember why I 
added it.

> I'm not sure that any RAM on null machine is of any use at all
> but the if we decide to get rid of it completely, then we need
> to clean up
>      /* RAM at address zero */
>      if (mch->ram) {
>          memory_region_add_subregion(get_system_memory(), 0, mch->ram);
>      }

This would still be needed because you _can_ get a non-null mch->ram 
(via "-M memdev") even if mc->default_ram_id == NULL.

Paolo

> and commit title/message should match what patch does and why.
> 
> 
>>       if (machine_class->default_ram_id && current_machine->ram_size &&
>>           numa_uses_legacy_mem() && !current_machine->ram_memdev_id) {
>>           create_default_memdev(current_machine, mem_path);
>>       }
>>
>> and is thus skipped for -M none.
>>
>> Paolo
>>
>>>
>>> maybe there is  a bug over there but
>>>       "mc->default_ram_size = 0"
>>> above, should result in
>>>       current_machine->ram_size == 0
>>> in case user hasn't provided "-m"
>>> and hence memdev shouldn't be created
>>>    
>>
>
diff mbox series

Patch

diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
index 7e693523d7..c40badf5bc 100644
--- a/hw/core/null-machine.c
+++ b/hw/core/null-machine.c
@@ -49,7 +49,7 @@  static void machine_none_machine_init(MachineClass *mc)
     mc->init = machine_none_init;
     mc->max_cpus = 1;
     mc->default_ram_size = 0;
-    mc->default_ram_id = "ram";
+    mc->default_ram_id = NULL;
     mc->no_serial = 1;
     mc->no_parallel = 1;
     mc->no_floppy = 1;