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

Re: [libvirt] [PATCH] Support configuration of huge pages in guests



On Thu, Sep 03, 2009 at 11:48:53AM +0100, Mark McLoughlin wrote:
> > @@ -290,10 +305,22 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
> >          }
> >      }
> >  
> > +     p = virConfGetValue (conf, "hugetlbfs_mount");
> > +     CHECK_TYPE ("hugetlbfs_mount", VIR_CONF_STRING);
> > +     if (p && p->str) {
> > +         VIR_FREE(driver->hugetlbfs_mount);
> > +         if (!(driver->hugetlbfs_mount = strdup(p->str))) {
> > +             virReportOOMError(NULL);
> > +             virConfFree(conf);
> > +             return -1;
> > +         }
> > +     }
> > +
> 
> How come you probe for a hugetlbfs mount even when the config file
> contains it?

That's just the most convenient way with the way the config file
loading is structured. The previous patch of John's had the
probing down in this part of the code in the else {} clause
here. The trouble with that is that if there is no config file
on disk at all, it'll never be run. Moving it to the top
ensures its always got a sensible default.

> > diff --git a/src/util.c b/src/util.c
> > index 0d4f3fa..35efee2 100644
> > --- a/src/util.c
> > +++ b/src/util.c
> > @@ -60,7 +60,9 @@
> >  #if HAVE_CAPNG
> >  #include <cap-ng.h>
> >  #endif
> > -
> > +#ifdef HAVE_MNTENT_H
> > +#include <mntent.h>
> > +#endif
> >  
> >  #include "virterror_internal.h"
> >  #include "logging.h"
> > @@ -1983,3 +1985,36 @@ int virGetGroupID(virConnectPtr conn,
> >      return 0;
> >  }
> >  #endif
> > +
> > +
> > +#ifdef HAVE_MNTENT_H
> 
> Hmm, if mntent.h isn't found, the qemu driver will fail to link
> 
> Is the idea here that anywhere mntent.h isn't available, the qemu driver
> won't be built so we don't need to check HAVE_MNTENT_H in qemu_conf.c?

That's an oversight - should have conditionalized the caller, though
I wouldn't be surprisd if QEMU driver is already broken on non-UNIX
since I don't think anyone's ever really tested it. Not that I ever
really expect people to use Windows for the QEMU driver itself

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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