[libvirt] [PATCH 2/3] bhyve: create capabilities submodule
Roman Bogorodskiy
bogorodskiy at gmail.com
Sat Apr 5 17:58:33 UTC 2014
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 55427ed..c715422 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -780,6 +780,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..b8a9db4
> --- /dev/null
> +++ b/src/bhyve/bhyve_capabilities.c
> @@ -0,0 +1,105 @@
> +/*
> + * bhyve_capabilities.c: bhyve capabilities module
> + *
> + * Copyright (C) 2014 Wojciech Macek <wma at semihalf.com>
> + *
> + * 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..efe496e
> --- /dev/null
> +++ b/src/bhyve/bhyve_capabilities.h
> @@ -0,0 +1,30 @@
> +/*
> + * bhyve_capabilities.h: bhyve capabilities module
> + *
> + * Copyright (C) 2014 Wojciech Macek <wma at semihalf.com>
> + *
> + * 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
Incorrect indentation, it should be:
#ifndef ...
# define ...
# include ...
Please check 'Preprocessor' section in HACKING about that. There's a
'syntax-check' rule to check that. On FreeBSD, it requires installing
devel/cppi port. Also, for the time being, a hack is required to make
'syntax-check' work on FreeBSD: you'd need to s/ sed / $(SED) / in
cfg.mk.
And it'd be nice to keep a consistent style for bhyve headers and use
#ifnded __BHYVE_CAPABILITIES_H__.
> +
> +#include "capabilities.h"
> +
> +virCapsPtr virBhyveCapsBuild(void);
> +
> +#endif
> +
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index 3321a79..ef9192a 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -53,6 +53,7 @@
> #include "bhyve_driver.h"
> #include "bhyve_process.h"
> #include "bhyve_utils.h"
> +#include "bhyve_capabilities.h"
>
> #define VIR_FROM_THIS VIR_FROM_BHYVE
>
> @@ -72,44 +73,49 @@ bhyveDriverUnlock(bhyveConnPtr driver)
> virMutexUnlock(&driver->lock);
> }
>
> +/**
> + * 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
> 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,
> + _("Unable to get Capabilities"));
I might misread the diff as I failed to apply the diff for the reasons
mentioned in the other mail, so sorry of the question is irrelevant,
but..
If we fail at this point, should we return NULL instead of moving
forward?
> +
> + if ((xml = virCapabilitiesFormatXML(privconn->caps)) == NULL) {
^^^ should it be just 'caps'?
> + virObjectUnref(caps);
> virReportOOMError();
> + }
> + virObjectUnref(caps);
Could we end up calling that twice in case previous check fails?
What would be returned as 'xml' in that case?
>
> return xml;
> }
> @@ -326,8 +332,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;
> @@ -350,6 +361,7 @@ bhyveDomainDefineXML(virConnectPtr conn, const char *xml)
> goto cleanup;
>
> cleanup:
> + virObjectUnref(caps);
> virDomainDefFree(def);
> virObjectUnlock(vm);
>
> @@ -704,7 +716,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)))
> --
> 1.9.0
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140405/7a562dc5/attachment-0001.sig>
More information about the libvir-list
mailing list