[libvirt] [PATCH] bhyve: add config file support

[Roman Bogorodskiy]

  Michal Privoznik wrote:

> On 03/13/2017 07:02 PM, Roman Bogorodskiy wrote:
> > Introduce config file support for the bhyve driver. The only available
> > setting at present is 'firmware_dir' for specifying a directory with
> > UEFI firmware files.
> > ---
> >  src/Makefile.am                      |  25 +++++++-
> >  src/bhyve/bhyve.conf                 |   7 +++
> >  src/bhyve/bhyve_capabilities.c       |   9 ++-
> >  src/bhyve/bhyve_capabilities.h       |   5 +-
> >  src/bhyve/bhyve_conf.c               | 112 +++++++++++++++++++++++++++++++++++
> >  src/bhyve/bhyve_conf.h               |  32 ++++++++++
> >  src/bhyve/bhyve_driver.c             |  11 +++-
> >  src/bhyve/bhyve_utils.h              |  12 ++++
> >  src/bhyve/libvirtd_bhyve.aug         |  39 ++++++++++++
> >  src/bhyve/test_libvirtd_bhyve.aug.in |   5 ++
> >  10 files changed, 252 insertions(+), 5 deletions(-)
> >  create mode 100644 src/bhyve/bhyve.conf
> >  create mode 100644 src/bhyve/bhyve_conf.c
> >  create mode 100644 src/bhyve/bhyve_conf.h
> >  create mode 100644 src/bhyve/libvirtd_bhyve.aug
> >  create mode 100644 src/bhyve/test_libvirtd_bhyve.aug.in
> > 
> > diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c
> > index 5e6094e3c..da06ba711 100644
> > --- a/src/bhyve/bhyve_capabilities.c
> > +++ b/src/bhyve/bhyve_capabilities.c
> > @@ -111,7 +111,8 @@ virBhyveCapsBuild(void)
> >  }
> >  
> >  virDomainCapsPtr
> > -virBhyveDomainCapsBuild(const char *emulatorbin,
> > +virBhyveDomainCapsBuild(bhyveConnPtr conn,
> > +                        const char *emulatorbin,
> >                          const char *machine,
> >                          virArch arch,
> >                          virDomainVirtType virttype)
> > @@ -120,8 +121,9 @@ virBhyveDomainCapsBuild(const char *emulatorbin,
> >      unsigned int bhyve_caps = 0;
> >      DIR *dir;
> >      struct dirent *entry;
> > -    const char *firmware_dir = "/usr/local/share/uefi-firmware";
> >      size_t firmwares_alloc = 0;
> > +    virBhyveDriverConfigPtr cfg = virBhyveDriverGetConfig(conn);
> 
> You need to unref @cfg.

Fixed.

> > +    const char *firmware_dir = cfg->firmwareDir;
> >  
> >      if (!(caps = virDomainCapsNew(emulatorbin, machine, arch, virttype)))
> >          goto cleanup;
> > @@ -152,7 +154,10 @@ virBhyveDomainCapsBuild(const char *emulatorbin,
> >  
> >             caps->os.loader.values.nvalues++;
> >          }
> > +    } else {
> > +        VIR_WARN("Cannot open firmware directory %s", firmware_dir);
> >      }
> > +
> >      caps->disk.supported = true;
> >      VIR_DOMAIN_CAPS_ENUM_SET(caps->disk.diskDevice,
> >                               VIR_DOMAIN_DISK_DEVICE_DISK,
> > diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h
> > index 8fb97d730..3d8edb490 100644
> > --- a/src/bhyve/bhyve_capabilities.h
> > +++ b/src/bhyve/bhyve_capabilities.h
> > @@ -25,8 +25,11 @@
> >  # include "capabilities.h"
> >  # include "conf/domain_capabilities.h"
> >  
> > +# include "bhyve_conf.h"
> > +
> 
> This include is because of bhyveConnPtr type I assume. Well, that one is defined in bhyve_utils.h which is the file you should be including.

Actually it's simpler: initially I planned to pass
virBhyveDriverConfigPtr instead of bhyveConnPtr to virBhyveDomainCapsBuild(),
but then decided to call virBhyveDriverGetConfig() in the place where it's
actually used and made a last minute change forgetting to change
includes (which unfortunately didn't trigger a warning).

> After you've done that you'll find that bhyve_capabilities needs to include bhyve_conf.h" because of virBhyveDriverGetConfig call. But that's okay - you can replace include of bhyve_utils.h there with bhyve_conf.h:
> 
> >  virCapsPtr virBhyveCapsBuild(void);
> > -virDomainCapsPtr virBhyveDomainCapsBuild(const char *emulatorbin,
> > +virDomainCapsPtr virBhyveDomainCapsBuild(bhyveConnPtr,
> > +                                         const char *emulatorbin,
> >                                           const char *machine,
> >                                           virArch arch,
> >                                           virDomainVirtType virttype);
> 
> With that squashed in, and unrefing @cfg you have my ACK.
> 
> Michal

Pushed with the fixes applied, thanks!

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170314/8d1e65d2/attachment-0001.sig>