Re: [libvirt] [PATCHv2 2/3] bhyve: create capabilities submodule

On 07.04.2014 07:06, Wojciech Macek wrote: > - Move all capabilities functions to separate file > - Add initCPU > --- > src/Makefile.am | 2 + > src/bhyve/bhyve_capabilities.c | 105 ++++++++++++++++++++++++++++++ > +++++++++++ > src/bhyve/bhyve_capabilities.h | 30 ++++++++++++ > src/bhyve/bhyve_driver.c | 56 +++++++++++++--------- > 4 files changed, 171 insertions(+), 22 deletions(-) > create mode 100644 src/bhyve/bhyve_capabilities.c > create mode 100644 src/bhyve/bhyve_capabilities.h > > diff --git a/src/Makefile.am b/src/Makefile.am > index f6690b6..21d56fc 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -778,6 +778,8 @@ PARALLELS_DRIVER_SOURCES = > \ > parallels/parallels_network.c > > BHYVE_DRIVER_SOURCES = \ > + bhyve/bhyve_capabilities.c \ > + bhyve/bhyve_capabilities.h \ > bhyve/bhyve_command.c \ > bhyve/bhyve_command.h \ > bhyve/bhyve_driver.h \ > diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_ > capabilities.c > new file mode 100644 > index 0000000..b591357 > --- /dev/null > +++ b/src/bhyve/bhyve_capabilities.c > @@ -0,0 +1,105 @@ > +/* > + * bhyve_capabilities.c: bhyve capabilities module > + * > + * Copyright (C) 2014 Roman Bogorodskiy > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>;. > + * > + */ > +#include <config.h> > +#include <sys/utsname.h> > + > +#include "viralloc.h" > +#include "virlog.h" > +#include "virstring.h" > +#include "cpu/cpu.h" > +#include "nodeinfo.h" > +#include "bhyve_utils.h" > +#include "domain_conf.h" > +#include "vircommand.h" > +#include "bhyve_capabilities.h" > + > +#define VIR_FROM_THIS VIR_FROM_BHYVE > + > +VIR_LOG_INIT("bhyve.bhyve_capabilities"); > + > +static int > +virBhyveCapsInitCPU(virCapsPtr caps, > + virArch arch) > +{ > + virCPUDefPtr cpu = NULL; > + virCPUDataPtr data = NULL; > + virNodeInfo nodeinfo; > + int ret = -1; > + > + if (VIR_ALLOC(cpu) < 0) > + goto error; > + > + cpu->arch = arch; > + > + if (nodeGetInfo(&nodeinfo)) > + goto error; > + > + cpu->type = VIR_CPU_TYPE_HOST; > + cpu->sockets = nodeinfo.sockets; > + cpu->cores = nodeinfo.cores; > + cpu->threads = nodeinfo.threads; > + caps->host.cpu = cpu; > + > + if (!(data = cpuNodeData(arch)) > + || cpuDecode(cpu, data, NULL, 0, NULL) < 0) > + goto cleanup; > + > + ret = 0; > + > + cleanup: > + cpuDataFree(data); > + > + return ret; > + > + error: > + virCPUDefFree(cpu); > + goto cleanup; > +} > + > +virCapsPtr > +virBhyveCapsBuild(void) > +{ > + virCapsPtr caps; > + virCapsGuestPtr guest; > + > + if ((caps = virCapabilitiesNew(virArchFromHost(), > + 0, 0)) == NULL) > + return NULL; > + > + if ((guest = virCapabilitiesAddGuest(caps, "hvm", > + VIR_ARCH_X86_64, > + "bhyve", > + NULL, 0, NULL)) == NULL) > + goto error; > + > + if (virCapabilitiesAddGuestDomain(guest, > + "bhyve", NULL, NULL, 0, NULL) == > NULL) > + goto error; > + > + if (virBhyveCapsInitCPU(caps, virArchFromHost()) < 0) > + VIR_WARN("Failed to get host CPU"); > + > + return caps; > + > + error: > + virObjectUnref(caps); > + return NULL; > +} > diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_ > capabilities.h > new file mode 100644 > index 0000000..741c631 > --- /dev/null > +++ b/src/bhyve/bhyve_capabilities.h > @@ -0,0 +1,30 @@ > +/* > + * bhyve_capabilities.h: bhyve capabilities module > + * > + * Copyright (C) 2014 Semihalf > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>;. > + * > + */ > + > +#ifndef _BHYVE_CAPABILITIES > +#define _BHYVE_CAPABILITIES > + > +#include "capabilities.h" > There need to be a space after hash tag. + > +virCapsPtr virBhyveCapsBuild(void); > + > +#endif > + > Empty line at EOF. diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c > index f70eff5..728ab2b 100644 > --- a/src/bhyve/bhyve_driver.c > +++ b/src/bhyve/bhyve_driver.c > @@ -54,6 +54,7 @@ > #include "bhyve_driver.h" > #include "bhyve_process.h" > #include "bhyve_utils.h" > +#include "bhyve_capabilities.h" > > #define VIR_FROM_THIS VIR_FROM_BHYVE > > @@ -111,44 +112,49 @@ bhyveAutostartDomains(bhyveConnPtr driver) > virObjectUnref(conn); > } > > +/** > + * bhyveDriverGetCapabilities: > + * > + * Get a reference to the virCapsPtr instance for the > + * driver. > + * > + * The caller must release the reference with virObjetUnref > + * > + * Returns: a reference to a virCapsPtr instance or NULL > + */ > static virCapsPtr > -bhyveBuildCapabilities(void) > +bhyveDriverGetCapabilities(bhyveConnPtr driver) > { > - virCapsPtr caps; > - virCapsGuestPtr guest; > + virCapsPtr ret = NULL; > > - if ((caps = virCapabilitiesNew(virArchFromHost(), > - 0, 0)) == NULL) > + if(driver == NULL) > missing space between 'if' and bracket. return NULL; > > - if ((guest = virCapabilitiesAddGuest(caps, "hvm", > - VIR_ARCH_X86_64, > - "bhyve", > - NULL, 0, NULL)) == NULL) > - goto error; > + ret = virObjectRef(driver->caps); > > - if (virCapabilitiesAddGuestDomain(guest, > - "bhyve", NULL, NULL, 0, NULL) == > NULL) > - goto error; > - > - return caps; > - > - error: > - virObjectUnref(caps); > - return NULL; > + return ret; > } > > static char * > bhyveConnectGetCapabilities(virConnectPtr conn) > { > bhyveConnPtr privconn = conn->privateData; > + virCapsPtr caps; > char *xml; > > if (virConnectGetCapabilitiesEnsureACL(conn) < 0) > return NULL; > > - if ((xml = virCapabilitiesFormatXML(privconn->caps)) == NULL) > + caps = bhyveDriverGetCapabilities(privconn); > + if (!caps) > + virReportError(VIR_ERR_INTERNAL_ERROR, > s/,/, "%s"/ + _("Unable to get Capabilities")); > + > + if ((xml = virCapabilitiesFormatXML(privconn->caps)) == NULL) { > + virObjectUnref(caps); > virReportOOMError(); > + } > + virObjectUnref(caps); > > return xml; > } > @@ -448,8 +454,13 @@ bhyveDomainDefineXML(virConnectPtr conn, const char > *xml) > virDomainDefPtr def = NULL; > virDomainDefPtr oldDef = NULL; > virDomainObjPtr vm = NULL; > + virCapsPtr caps = NULL; > + > + caps = bhyveDriverGetCapabilities(privconn); > + if (!caps) > + return NULL; > > - if ((def = virDomainDefParseString(xml, privconn->caps, > privconn->xmlopt, > + if ((def = virDomainDefParseString(xml, caps, privconn->xmlopt, > 1 << VIR_DOMAIN_VIRT_BHYVE, > VIR_DOMAIN_XML_INACTIVE)) == > NULL) > goto cleanup; > @@ -472,6 +483,7 @@ bhyveDomainDefineXML(virConnectPtr conn, const char > *xml) > goto cleanup; > > cleanup: > + virObjectUnref(caps); > virDomainDefFree(def); > virObjectUnlock(vm); > > @@ -869,7 +881,7 @@ bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED, > if (!(bhyve_driver->closeCallbacks = virCloseCallbacksNew())) > goto cleanup; > > - if (!(bhyve_driver->caps = bhyveBuildCapabilities())) > + if (!(bhyve_driver->caps = virBhyveCapsBuild())) > goto cleanup; > > if (!(bhyve_driver->xmlopt = virDomainXMLOptionNew(NULL, NULL, > NULL))) > > You can catch many of these by running 'make syntax-check'. Michal