[libvirt] [RFC] Memory hotplug naming [Was: [PATCHv2 3/7] conf: Replace access to def->mem.max_balloon with accessor functions]

[Martin Kletzander]

On Wed, Mar 04, 2015 at 03:25:31PM +0100, Peter Krempa wrote:
>Adding Dan && Eric to cc
>

I already did that in my mail, but since the subject didn't start with
"Re:" and they have de-duplication turned on, you can't see them here
(thank the mailman, I guess).

>On Thu, Feb 26, 2015 at 14:06:55 +0100, Martin Kletzander wrote:
>> On Wed, Feb 25, 2015 at 11:22:02AM +0100, Peter Krempa wrote:
>> >As there are two possible approaches to define a domain's memory size -
>> >one used with legacy, non-NUMA VMs configured in the <memory> element
>> >and per-node based approach on NUMA machines - the user needs to make
>> >sure that both are specified correctly in the NUMA case.
>> >
>> >To avoid this burden on the user I'd like to replace the NUMA case with
>> >automatic totaling of the memory size. To achieve this I need to replace
>> >direct access to the virDomainMemtune's 'max_balloon' field with
>> >two separate getters depending on the desired size.
>> >
>> >The two sizes are needed as:
>> >1) Startup memory size doesn't include memory modules in some
>> >hypervisors.
>> >2) After startup these count as the usable memory size.
>> >
>> >Note that the comments for the functions are future aware and document
>> >state that will be present after a few later patches.
>> >---
>> >
>> >Notes:
>> >    Version 2:
>> >    - renamed virDomainDefGetMemoryCurrent to virDomainDefGetMemoryActual
>> >    - changed to the initial memory accessor when starting VMs in LXC, phyp and libxl
>> >
>>
>> When looking at these accessors' names, I'm thinking how not to screw
>> ourselves by naming decisions when memory hot-plug will come.  If we
>> have maxMemory, memory and currentMemory, and then have APIs where
>> setting maximum memory actually changes "memory" and so on, we have to
>> go through the whole documentation and everything that mentions memory
>> just to fix the naming and we still won't be able to change API
>> functions and flags.  We will be stuck with this for fair amount of
>> time, I guess.
>>
>> So I'm thinking about an approach that I haven't thought through
>> completely, but I want to have other opinions as well.
>>
>> What if we used our <memory/> that we have now and started treating it
>> as the complete maximum after all slots are filled?  So instead of:
>>
>>   <maxMemory>2048</maxMemory>
>>   <memory>1024</memory>
>>   <currentMemory>512</currentMemory>
>>
>> we'd have something like:
>>
>>   <memory>2048</memory>
>>   <actualMemory>1024</actualMemory>
>>   <currentMemory>512</currentMemory>
>
>This depends on how you want to interpret the <memory> element.
>Currently it is stored in variable called "max_balloon" so in that case
>my implementation is closer to the existing meaning as the max_balloon
>memory is actually available in the address space of the guest until the
>guest decides to enable the memory balloon.
>

Yes, it is closer, from a developers' point of view.  I was just
trying to make it look nicer and probably a bit more usable from the
users' POV.

>As a fact, both of those approaches have to have NUMA enabled though so
>you can configure the size of the initial memory. If we wanted to change
>the way we are configuring this we might want to consider whether we
>ever want to enable memory hotplug for non-NUMA guests and thus have to
>introduce both <maxMemory> and <actualMemory> or similar.
>
>>
>> where the main difference is the fact that memory will still be
>> maximum memory, so we don't have to rename half of the enums, funcs
>> and vars in the code.  <actualMemory/> would be optional (the name
>> doesn't actually matter at all) and if not specified it would just
>> work like until now.
>
>Since the field is internally called max_ballon, every change will
>probably trigger some refactors.
>
>>
>> Yet another approach comes to mind and that is that we'll still have
>> only <memory/> and <currentMemory/>, <memory/> gains "dimmSlots"
>> attribute and the number of slots that are filled (and with what
>> memory modules) will be decided upon the existence of memory devices
>> (that is <memory/> under the <devices/> attribute).  Default behaviour
>> is still kept until "dimmSlots" (or "slots" or whatever) is not used
>> and then the memory has to be defined by memory devices.
>
>This looks definitely more complex ...
>
>>
>> I'm just trying to make our (as well as mgmt app developers' and
>> libvirt users') lives easier>
>
>... since you want to achieve this ^^
>

Yes, I didn't say it's an easier choice ;)  But anyway, it looks like
nobody expressed their opinions while I was offline for about a week,
so let's just go with your approach, then (unless you've changed your
mind :) ).

>>
>> Thanks in advance for any feedback, if you think that the current idea
>> is OK and this mail is just about donkey balls, let me know as well.
>>
>> Martin
>
>Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150309/2d6eb4bf/attachment-0001.sig>