Re: [Qemu-devel] [PATCH 03/11] pc: check if KVM has enough memory slots for DIMM devices

[Igor Mammedov]

On Mon, 03 Nov 2014 12:51:50 +0100
Paolo Bonzini <address@hidden> wrote:

> On 31/10/2014 17:38, Igor Mammedov wrote:
> > check amount of available KVM memory slots after all
> > devices were initialized and exit with error if
> > there isn't enough free memory slots for DIMMs.
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >  hw/i386/pc.c | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index f6dfd9b..41d91fb 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1125,6 +1125,36 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t
> > below_4g_mem_size, return guest_info;
> >  }
> >  
> > +static int pc_dimm_count(Object *obj, void *opaque)
> > +{
> > +    int *count = opaque;
> > +
> > +    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
> > +        (*count)++;
> > +    }
> > +
> > +    object_child_foreach(obj, pc_dimm_count, opaque);
> > +    return 0;
> > +}
> > +
> > +static void pc_kvm_slot_check(Notifier *notifier, void *data)
> > +{
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> > +    int free_slots = kvm_free_slot_count(ms);
> > +    int used_ram_slots = 0;
> > +
> > +    pc_dimm_count(OBJECT(ms), &used_ram_slots);
> > +    if ((ms->ram_slots - used_ram_slots) > free_slots) {
> > +        error_report("KVM doesn't support more than %d memory
> > slots",
> > +                     kvm_free_slot_count(ms));
> > +        exit(EXIT_FAILURE);
> > +    }
> > +}
> > +
> > +static Notifier kvm_slot_check_on_machine_done = {
> > +                        .notify = pc_kvm_slot_check
> > +                     };
> > +
> >  /* setup pci memory address space mapping into system address
> > space */ void pc_pci_as_mapping_init(Object *owner, MemoryRegion
> > *system_memory, MemoryRegion *pci_address_space)
> > @@ -1269,6 +1299,8 @@ FWCfgState *pc_memory_init(MachineState
> > *machine, "hotplug-memory", hotplug_mem_size);
> >          memory_region_add_subregion(system_memory,
> > pcms->hotplug_memory_base, &pcms->hotplug_memory);
> > +
> > +
> > qemu_add_machine_init_done_notifier(&kvm_slot_check_on_machine_done); }
> >  
> >      /* Initialize PC system firmware */
> > 
> 
> This will always be off by 4 or so due to system RAM and ROM slots.  I
> think patch 1 is enough.
The goal of this patch is to prevent starting guest with not supported
amount of lots requested on CLI with -m slots=too_much, patch 1 won't
prevent user from it.

As for "always be off by 4 or so", unfortunately consumed slots are not
constant and depend on CLI options.
If guest is started 1Gb then 5 slots are consumed, with 4Gb 5 slots are
consumed. Devices might also consume slots, for example vga does. I
would bet that some another device won't consume it. Hence machine done
notifier is suggested.

I wouldn't try to assume/make default consumed slots as constant
sinceit's  quite fragile and easy to break. Runtime check looks like
more stable approach in general.



> 
> Paolo