Re: [PATCH 10/13] conf: Don't always recalculate initial memory size from NUMA size totals

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 22, 2015 at 14:29:02 +0200, Michal Privoznik wrote:
> On 21.09.2015 19:21, Peter Krempa wrote:
> > When implementing memory hotplug I've opted to recalculate the initial
> > memory size (contents of the <memory> element) as a sum of the sizes of
> > NUMA nodes when NUMA was enabled. This was based on an assumption that
> > qemu did not allow starting when the NUMA node size total didn't equal
> > to the initial memory size. Unfortunately the check was introduced to
> > qemu just lately.
> > 
> > This patch uses the new XML parser flag to decide whether it's safe to
> > update the memory size total from the NUMA cell sizes or not.
> > 
> > As an additional improvement we now report an error in case when the
> > size of hotplug memory would exceed the total memory size.
> > 
> > The rest of the changes assures that the function is called with correct
> > flags.
> > ---
> >  src/conf/domain_conf.c  | 37 ++++++++++++++++++++++++++++++-------
> >  src/conf/domain_conf.h  |  1 +
> >  src/qemu/qemu_command.c |  3 ++-
> >  3 files changed, 33 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index d2a13ca..64cfd8c 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -3726,15 +3726,36 @@ virDomainDefRemoveDuplicateMetadata(virDomainDefPtr def)
> > 
> > 
> >  static int
> > -virDomainDefPostParseMemory(virDomainDefPtr def)
> > +virDomainDefPostParseMemory(virDomainDefPtr def,
> > +                            unsigned int parseFlags)
> >  {
> >      size_t i;
> > +    unsigned long long numaMemory = 0;
> > +    unsigned long long hotplugMemory = 0;
> > 
> > -    if ((def->mem.initial_memory = virDomainNumaGetMemorySize(def->numa)) == 0) {
> > +    /* Attempt to infer the initial memory size from the sum NUMA memory sizes
> > +     * in case ABI updates are allowed or the <memory> element wasn't specified */
> > +    if (def->mem.total_memory == 0 ||
> > +        parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE)
> > +        numaMemory = virDomainNumaGetMemorySize(def->numa);
> > +
> > +    if (numaMemory) {
> > +        def->mem.initial_memory = numaMemory;
> 
> Is there a reason to not use the setter function you've introduced in
> previous patch?
> 

Not really, it even saves a few lines of code.

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]