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
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
