Re: [PATCH] openvz: determine kb/pages only once

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

 



On Thu, May 10, 2012 at 04:54:59PM -0600, Eric Blake wrote:
> On 05/08/2012 02:12 PM, Guido Günther wrote:
> > to safe some syscalls (as suggested by Eric Blake)
> 
> s/safe/save/
> 
> > ---
> > I've moved the code into an extra module since there's some more code to
> > come that should be shared between openvz_conf.c and openvz_driver.c
> > Cheers,
> >  -- Guido
> > 
> >  src/Makefile.am            |    3 ++-
> >  src/openvz/openvz_conf.c   |   12 ++++-------
> >  src/openvz/openvz_driver.c |   19 +++++------------
> >  src/openvz/openvz_util.c   |   51 ++++++++++++++++++++++++++++++++++++++++++++
> >  src/openvz/openvz_util.h   |   28 ++++++++++++++++++++++++
> >  5 files changed, 90 insertions(+), 23 deletions(-)
> >  create mode 100644 src/openvz/openvz_util.c
> >  create mode 100644 src/openvz/openvz_util.h
> 
> This is quite a bit of churn for a micro-optimization, since it is not
> fixing any actual bugs.  I'll review the code, but let's avoid applying
> it until after 0.9.12 is released.
> 
> 
> > +++ b/src/openvz/openvz_util.c
> > @@ -0,0 +1,51 @@
> > +/*
> > + * openvz_driver.c: core driver methods for managing OpenVZ VEs
> > + *
> > + * Copyright (C) Guido Günther
> 
> Year?
> 
> 
> > +    static long kb_per_pages = 0;
> > +
> > +    if (kb_per_pages == 0) {
> > +        kb_per_pages = sysconf(_SC_PAGESIZE);
> > +        if (kb_per_pages > 0) {
> > +            kb_per_pages /= 1024;
> > +        } else {
> > +            openvzError(VIR_ERR_INTERNAL_ERROR,
> > +                        _("Can't determine page size"));
> > +            kb_per_pages = 0;
> > +            return -1;
> 
> This says that in an error situation, every caller will repeat the
> sysconf() call and issue a new error.  Seems reasonable.
> 
> ACK with copyright nit fixed, post-release.

Pushed now that 0.9.12 is out. Thanks!
 -- Guido

--
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]