[libvirt] [PATCH v8 1/8] parallels: add driver skeleton
Peter Krempa
pkrempa at redhat.com
Tue Jul 10 14:53:39 UTC 2012
On 07/05/12 12:58, Dmitry Guryanov wrote:
> Parallels Virtuozzo Server is a cloud-ready virtualization
> solution that allows users to simultaneously run multiple virtual
> machines and containers on the same physical server.
>
> Current name of this product is Parallels Server Bare Metal and
> more information about it can be found here -
> http://www.parallels.com/products/server/baremetal/sp/.
>
> This first patch adds driver, which can report node info only.
>
> Signed-off-by: Dmitry Guryanov <dguryanov at parallels.com>
> ---
> cfg.mk | 1 +
> configure.ac | 23 ++++
> docs/drvparallels.html.in | 28 ++++
> include/libvirt/virterror.h | 2 +-
> libvirt.spec.in | 9 +-
> mingw-libvirt.spec.in | 6 +
> po/POTFILES.in | 1 +
> src/Makefile.am | 13 ++
> src/conf/domain_conf.c | 3 +-
> src/conf/domain_conf.h | 1 +
> src/driver.h | 1 +
> src/libvirt.c | 9 ++
> src/parallels/parallels_driver.c | 271 ++++++++++++++++++++++++++++++++++++++
> src/parallels/parallels_driver.h | 51 +++++++
> src/util/virterror.c | 3 +-
> 15 files changed, 418 insertions(+), 4 deletions(-)
> create mode 100644 docs/drvparallels.html.in
> create mode 100644 src/parallels/parallels_driver.c
> create mode 100644 src/parallels/parallels_driver.h
>
> @@ -2805,6 +2827,7 @@ AC_MSG_NOTICE([ LXC: $with_lxc])
> AC_MSG_NOTICE([ PHYP: $with_phyp])
> AC_MSG_NOTICE([ ESX: $with_esx])
> AC_MSG_NOTICE([ Hyper-V: $with_hyperv])
> +AC_MSG_NOTICE([ PARALLELS: $with_parallels])
This line should be aligned with others.
> AC_MSG_NOTICE([ Test: $with_test])
> AC_MSG_NOTICE([ Remote: $with_remote])
> AC_MSG_NOTICE([ Network: $with_network])
> diff --git a/docs/drvparallels.html.in b/docs/drvparallels.html.in
> new file mode 100644
> index 0000000..976dea1
> --- /dev/null
> +++ b/docs/drvparallels.html.in
> @@ -0,0 +1,28 @@
> +<html><body>
> + <h1>Parallels Virtuozzo Server driver</h1>
> + <ul id="toc"></ul>
> + <p>
> + The libvirt PARALLELS driver can manage Parallels Virtuozzo Server starting from 6.0 version.
... from version 6.0.
> + </p>
> +
> +
> + <h2><a name="project">Project Links</a></h2>
> + <ul>
> + <li>
> + The <a href="http://www.parallels.com/products/server/baremetal/sp/">Parallels Virtuozzo Server</a> Virtualization Solution.
> + </li>
> + </ul>
> +
> +
> + <h2><a name="uri">Connections to the Parallels Virtuozzo Server driver</a></h2>
> + <p>
> + The libvirt PARALLELS driver is a single-instance privileged driver, with a driver name of 'parallels'. Some example connection URIs for the libvirt driver are:
> + </p>
> +<pre>
> +parallels:///default (local access)
> +parallels+unix:///default (local access)
> +parallels://example.com/default (remote access, TLS/x509)
> +parallels+tcp://example.com/default (remote access, SASl/Kerberos)
> +parallels+ssh://root@example.com/default (remote access, SSH tunnelled)
> +</pre>
> +</body></html>
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 0e0bc9c..25e8d43 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -97,7 +97,7 @@ typedef enum {
> VIR_FROM_URI = 45, /* Error from URI handling */
> VIR_FROM_AUTH = 46, /* Error from auth handling */
> VIR_FROM_DBUS = 47, /* Error from DBus */
> -
> + VIR_FROM_PARALLELS = 48, /* Error from PARALLELS */
The comment is misaligned and the one empty line should remain here.
> # ifdef VIR_ENUM_SENTINELS
> VIR_ERR_DOMAIN_LAST
> # endif
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index ec2b3b4..21d9bc0 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -67,6 +67,7 @@
> %define with_esx 0%{!?_without_esx:1}
> %define with_hyperv 0%{!?_without_hyperv:1}
> %define with_xenapi 0%{!?_without_xenapi:1}
> +%define with_parallels 0%{!?_without_parallels:1}
Again, misaligned.
>
> # Then the secondary host drivers, which run inside libvirtd
> %define with_network 0%{!?_without_network:%{server_drivers}}
> @@ -131,6 +132,7 @@
> %define with_xenapi 0
> %define with_libxl 0
> %define with_hyperv 0
> +%define with_parallels 0
> %endif
>
> # Fedora 17 / RHEL-7 are first where we use systemd. Although earlier
> @@ -1032,6 +1034,10 @@ of recent versions of Linux (and other OSes).
> %define _without_vmware --without-vmware
> %endif
>
> +%if ! %{with_parallels}
> +%define _without_parallels --without-parallels
> +%endif
> +
> %if ! %{with_polkit}
> %define _without_polkit --without-polkit
> %endif
> @@ -1170,6 +1176,7 @@ autoreconf -if
> %{?_without_esx} \
> %{?_without_hyperv} \
> %{?_without_vmware} \
> + %{?_without_parallels} \
> %{?_without_network} \
> %{?_with_rhel5_api} \
> %{?_without_storage_fs} \
> @@ -1360,7 +1367,7 @@ fi
>
> /sbin/chkconfig --add libvirtd
> if [ "$1" -ge "1" ]; then
> - /sbin/service libvirtd condrestart > /dev/null 2>&1
> + /sbin/service libvirtd condrestart > /dev/null 2>&1
> fi
> %endif
>
> diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
> index d2a8cf3..4695895 100644
> --- a/mingw-libvirt.spec.in
> +++ b/mingw-libvirt.spec.in
> @@ -13,6 +13,7 @@
> # missing libwsman, so can't build hyper-v
> %define with_hyperv 0%{!?_without_hyperv:0}
> %define with_xenapi 0%{!?_without_xenapi:1}
> +%define with_parallels 0%{!?_without_parallels:0}
Missaligned
>
> # RHEL ships ESX but not PowerHypervisor, HyperV, or libxenserver (xenapi)
> %if 0%{?rhel}
> @@ -125,6 +126,10 @@ MinGW Windows libvirt virtualization library, static version.
> %define _without_xenapi --without-xenapi
> %endif
>
> +%if ! %{with_parallels}
> +%define _without_parallels --without-parallels
> +%endif
> +
> %if 0%{?enable_autotools}
> autoreconf -if
> %endif
> @@ -148,6 +153,7 @@ autoreconf -if
> %{?_without_esx} \
> %{?_without_hyperv} \
> --without-vmware \
> + --without-parallels \
> --without-netcf \
> --without-audit \
> --without-dtrace
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 31246f7..1917899 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -63,6 +63,7 @@ src/openvz/openvz_conf.c
> src/openvz/openvz_driver.c
> src/openvz/openvz_util.c
> src/phyp/phyp_driver.c
> +src/parallels/parallels_driver.c
parallels sorts before phyp, as this file should be in alphabetical order
> src/qemu/qemu_agent.c
> src/qemu/qemu_bridge_filter.c
> src/qemu/qemu_capabilities.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 2309984..3aa2a18 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -479,6 +479,10 @@ HYPERV_DRIVER_EXTRA_DIST = \
> hyperv/hyperv_wmi_generator.py \
> $(HYPERV_DRIVER_GENERATED)
>
> +PARALLELS_DRIVER_SOURCES = \
> + parallels/parallels_driver.h \
> + parallels/parallels_driver.c
> +
> NETWORK_DRIVER_SOURCES = \
> network/bridge_driver.h network/bridge_driver.c
>
> @@ -900,6 +904,14 @@ libvirt_driver_hyperv_la_LIBADD = $(OPENWSMAN_LIBS)
> libvirt_driver_hyperv_la_SOURCES = $(HYPERV_DRIVER_SOURCES)
> endif
>
> +if WITH_PARALLELS
> +noinst_LTLIBRARIES += libvirt_driver_parallels.la
> +libvirt_la_BUILT_LIBADD += libvirt_driver_parallels.la
> +libvirt_driver_parallels_la_CFLAGS = \
> + -I$(top_srcdir)/src/conf $(AM_CFLAGS)
> +libvirt_driver_parallels_la_SOURCES = $(PARALLELS_DRIVER_SOURCES)
> +endif
> +
> if WITH_NETWORK
> noinst_LTLIBRARIES += libvirt_driver_network_impl.la
> libvirt_driver_network_la_SOURCES =
> @@ -1107,6 +1119,7 @@ EXTRA_DIST += \
> $(ESX_DRIVER_EXTRA_DIST) \
> $(HYPERV_DRIVER_SOURCES) \
> $(HYPERV_DRIVER_EXTRA_DIST) \
> + $(PARALLELS_DRIVER_SOURCES) \
> $(NETWORK_DRIVER_SOURCES) \
> $(INTERFACE_DRIVER_SOURCES) \
> $(STORAGE_DRIVER_SOURCES) \
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3fb90db..be385a2 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -93,7 +93,8 @@ VIR_ENUM_IMPL(virDomainVirt, VIR_DOMAIN_VIRT_LAST,
> "vmware",
> "hyperv",
> "vbox",
> - "phyp")
> + "phyp",
> + "parallels")
>
> VIR_ENUM_IMPL(virDomainBoot, VIR_DOMAIN_BOOT_LAST,
> "fd",
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 7d5d60b..c9e95f5 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -160,6 +160,7 @@ enum virDomainVirtType {
> VIR_DOMAIN_VIRT_HYPERV,
> VIR_DOMAIN_VIRT_VBOX,
> VIR_DOMAIN_VIRT_PHYP,
> + VIR_DOMAIN_VIRT_PARALLELS,
>
> VIR_DOMAIN_VIRT_LAST,
> };
> diff --git a/src/driver.h b/src/driver.h
> index b3c1740..e8ba99c 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -31,6 +31,7 @@ typedef enum {
> VIR_DRV_VMWARE = 13,
> VIR_DRV_LIBXL = 14,
> VIR_DRV_HYPERV = 15,
> + VIR_DRV_PARALLELS = 16,
> } virDrvNo;
>
>
> diff --git a/src/libvirt.c b/src/libvirt.c
> index db6ba15..69ec94f 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -72,6 +72,9 @@
> #ifdef WITH_XENAPI
> # include "xenapi/xenapi_driver.h"
> #endif
> +#ifdef WITH_PARALLELS
> +# include "parallels/parallels_driver.h"
> +#endif
>
> #define VIR_FROM_THIS VIR_FROM_NONE
>
> @@ -443,6 +446,9 @@ virInitialize(void)
> #ifdef WITH_XENAPI
> if (xenapiRegister() == -1) return -1;
> #endif
> +#ifdef WITH_PARALLELS
> + if (parallelsRegister() == -1) return -1;
> +#endif
> #ifdef WITH_REMOTE
> if (remoteRegister () == -1) return -1;
> #endif
> @@ -1144,6 +1150,9 @@ do_open (const char *name,
> #ifndef WITH_XENAPI
> STRCASEEQ(ret->uri->scheme, "xenapi") ||
> #endif
> +#ifndef WITH_PARALLELS
> + STRCASEEQ(ret->uri->scheme, "parallels") ||
> +#endif
> false)) {
> virReportErrorHelper(VIR_FROM_NONE, VIR_ERR_CONFIG_UNSUPPORTED,
> __FILE__, __FUNCTION__, __LINE__,
> diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
> new file mode 100644
> index 0000000..614f78f
> --- /dev/null
> +++ b/src/parallels/parallels_driver.c
> @@ -0,0 +1,271 @@
> +/*
> + * parallels_driver.c: core driver functions for managing
> + * Parallels Virtuozzo Server hosts
> + *
> + * Copyright (C) 2012 Parallels, Inc.
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + */
> +
> +#include <config.h>
> +
> +#include <sys/types.h>
> +#include <sys/poll.h>
> +#include <limits.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdarg.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sys/utsname.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <paths.h>
> +#include <pwd.h>
> +#include <stdio.h>
> +#include <sys/wait.h>
> +#include <sys/time.h>
> +#include <sys/statvfs.h>
> +
> +#include "datatypes.h"
> +#include "virterror_internal.h"
> +#include "memory.h"
> +#include "util.h"
> +#include "logging.h"
> +#include "command.h"
> +#include "configmake.h"
> +#include "storage_file.h"
> +#include "nodeinfo.h"
> +#include "json.h"
> +
> +#include "parallels_driver.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_PARALLELS
> +
> +static virCapsPtr parallelsBuildCapabilities(void);
This line shouldn't be needed.
> +static int parallelsClose(virConnectPtr conn);
> +
> +static void
> +parallelsDriverLock(parallelsConnPtr driver)
> +{
> + virMutexLock(&driver->lock);
> +}
> +
> +static void
> +parallelsDriverUnlock(parallelsConnPtr driver)
> +{
> + virMutexUnlock(&driver->lock);
> +}
> +
> +static int
> +parallelsDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED)
> +{
> + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
> +}
> +
> +static virCapsPtr
> +parallelsBuildCapabilities(void)
> +{
> + virCapsPtr caps;
> + virCapsGuestPtr guest;
> + struct utsname utsname;
> + uname(&utsname);
> +
> + if ((caps = virCapabilitiesNew(utsname.machine, 0, 0)) == NULL)
> + goto no_memory;
> +
> + if (nodeCapsInitNUMA(caps) < 0)
> + goto no_memory;
> +
> + virCapabilitiesSetMacPrefix(caps, (unsigned char[]) {
> + 0x42, 0x1C, 0x00});
> +
> + if ((guest = virCapabilitiesAddGuest(caps, "hvm", "x86_64",
> + 64, "parallels",
> + NULL, 0, NULL)) == NULL)
> + goto no_memory;
> +
> + if (virCapabilitiesAddGuestDomain(guest,
> + "parallels", NULL, NULL, 0, NULL) == NULL)
> + goto no_memory;
> +
> + caps->defaultConsoleTargetType = parallelsDefaultConsoleType;
> + return caps;
> +
> + no_memory:
> + virReportOOMError();
> + virCapabilitiesFree(caps);
> + return NULL;
> +}
> +
> +static char *
> +parallelsGetCapabilities(virConnectPtr conn)
> +{
> + parallelsConnPtr privconn = conn->privateData;
> + char *xml;
> +
> + parallelsDriverLock(privconn);
> + if ((xml = virCapabilitiesFormatXML(privconn->caps)) == NULL)
> + virReportOOMError();
> + parallelsDriverUnlock(privconn);
> + return xml;
> +}
> +
> +static int
> +parallelsOpenDefault(virConnectPtr conn)
> +{
> + parallelsConnPtr privconn;
> +
> + if (VIR_ALLOC(privconn) < 0) {
> + virReportOOMError();
> + return VIR_DRV_OPEN_ERROR;
> + }
> + if (virMutexInit(&privconn->lock) < 0) {
> + parallelsError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("cannot initialize mutex"));
You can avoid misaligning this line when you move "%s" on the previous line.
> + goto error;
> + }
> +
> + parallelsDriverLock(privconn);
> + conn->privateData = privconn;
> + parallelsDriverUnlock(privconn);
You unlock the private data here, but you're touching them afterwards.
> +
> + if (!(privconn->caps = parallelsBuildCapabilities()))
> + goto error;
> +
> + if (virDomainObjListInit(&privconn->domains) < 0)
> + goto error;
The unlock line should go here.
> +
> + return VIR_DRV_OPEN_SUCCESS;
> +
> + error:
> + virDomainObjListDeinit(&privconn->domains);
> + virCapabilitiesFree(privconn->caps);
> + virStoragePoolObjListFree(&privconn->pools);
> + parallelsDriverUnlock(privconn);
If initialisation of the mutex fails, you jump on error and then you'll
try to unlock a noninitialized mutex.
Thinking it over: you're the only one that's able to access the private
data until you make them public. Moving line:
> + conn->privateData = privconn;
Just before
> + return VIR_DRV_OPEN_SUCCESS;
Guarantees that nobody will access it until you are done initializing it
and you avoid the need to unlock the mutex and un-set the private data
pointer on the error path.
> + conn->privateData = NULL;
> + VIR_FREE(privconn);
> + return VIR_DRV_OPEN_ERROR;
> +}
> +
> +static virDrvOpenStatus
> +parallelsOpen(virConnectPtr conn,
> + virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags)
It's better to have one argument per line and align them.
> +{
> + int ret;
> + parallelsConnPtr privconn;
> + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
> +
> + if (!conn->uri)
> + return VIR_DRV_OPEN_DECLINED;
> +
> + if (!conn->uri->scheme || STRNEQ(conn->uri->scheme, "parallels"))
> + return VIR_DRV_OPEN_DECLINED;
> +
> + /* Remote driver should handle these. */
> + if (conn->uri->server)
> + return VIR_DRV_OPEN_DECLINED;
> +
> + /* From this point on, the connection is for us. */
> + if (!conn->uri->path
> + || conn->uri->path[0] == '\0'
We usualy write logic operators at the end of the line
> + || (conn->uri->path[0] == '/' && conn->uri->path[1] == '\0')) {
> + parallelsError(VIR_ERR_INVALID_ARG,
> + "%s", _("parallelsOpen: supply a path or use parallels:///default"));
Long line and misaligned.
> + return VIR_DRV_OPEN_ERROR;
> + }
> +
> + if (STREQ(conn->uri->path, "/default"))
> + ret = parallelsOpenDefault(conn);
> + else
> + return VIR_DRV_OPEN_DECLINED;
> +
> + if (ret != VIR_DRV_OPEN_SUCCESS)
> + return ret;
> +
> + privconn = conn->privateData;
> + parallelsDriverLock(privconn);
> + privconn->domainEventState = virDomainEventStateNew();
> + if (!privconn->domainEventState) {
> + parallelsDriverUnlock(privconn);
> + parallelsClose(conn);
> + return VIR_DRV_OPEN_ERROR;
> + }
> +
> + parallelsDriverUnlock(privconn);
> + return VIR_DRV_OPEN_SUCCESS;
> +}
> +
> +static int
> +parallelsClose(virConnectPtr conn)
> +{
> + parallelsConnPtr privconn = conn->privateData;
> +
> + parallelsDriverLock(privconn);
> + virCapabilitiesFree(privconn->caps);
> + virDomainObjListDeinit(&privconn->domains);
> + virDomainEventStateFree(privconn->domainEventState);
> + conn->privateData = NULL;
> +
> + parallelsDriverUnlock(privconn);
> + virMutexDestroy(&privconn->lock);
> +
> + VIR_FREE(privconn);
> + return 0;
> +}
> +
> +static int
> +parallelsGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *hvVer)
> +{
> + /* TODO */
> + *hvVer = 6;
I hope this hack gets updated in a patch later on. Also this produces
following output:
virsh # version
Compiled against library: libvir 0.9.13
Using library: libvir 0.9.13
Using API: PARALLELS 0.9.13
Running hypervisor: PARALLELS 0.0.6
> + return 0;
> +}
> +
> +static virDriver parallelsDriver = {
> + .no = VIR_DRV_PARALLELS,
> + .name = "PARALLELS",
> + .open = parallelsOpen, /* 0.10.0 */
> + .close = parallelsClose, /* 0.10.0 */
> + .version = parallelsGetVersion, /* 0.10.0 */
> + .getHostname = virGetHostname, /* 0.10.0 */
> + .nodeGetInfo = nodeGetInfo, /* 0.10.0 */
> + .getCapabilities = parallelsGetCapabilities, /* 0.10.0 */
> +};
> +
> +/**
> + * parallelsRegister:
> + *
> + * Registers the parallels driver
> + */
> +int
> +parallelsRegister(void)
> +{
> + char *prlctl_path;
> +
> + prlctl_path = virFindFileInPath(PRLCTL);
> + if (!prlctl_path) {
> + parallelsError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Can't find prlctl command in the PATH env"));
> + return VIR_DRV_OPEN_ERROR;
> + }
Memory leak: virFindFileInPath states:
/*
* Finds a requested executable file in the PATH env. e.g.:
* "kvm-img" will return "/usr/bin/kvm-img"
*
* You must free the result
*/
char *virFindFileInPath(const char *file)
VIR_FREE(prctl_path)
Also this piece of code is somewhat user-unfriendly. If you don't have
the prlctl command, the driver is not registered and for some reason the
error message is not present in the logs.
> +
> + if (virRegisterDriver(¶llelsDriver) < 0)
> + return -1;
> +
> + return 0;
> +}
> diff --git a/src/parallels/parallels_driver.h b/src/parallels/parallels_driver.h
> new file mode 100644
> index 0000000..c04db35
> --- /dev/null
> +++ b/src/parallels/parallels_driver.h
> @@ -0,0 +1,51 @@
> +/*
> + * parallels_driver.c: core driver functions for managing
> + * Parallels Virtuozzo Server hosts
> + *
> + * Copyright (C) 2012 Parallels, Inc.
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + */
> +
> +#ifndef PARALLELS_DRIVER_H
> +# define PARALLELS_DRIVER_H
> +
> +
> +# include "domain_conf.h"
> +# include "storage_conf.h"
> +# include "domain_event.h"
> +
> +# define parallelsError(code, ...) \
> + virReportErrorHelper(VIR_FROM_TEST, code, __FILE__, \
s/VIR_FROM_TEST/VIR_FROM_THIS/
> + __FUNCTION__, __LINE__, __VA_ARGS__)
> +# define PRLCTL "prlctl"
> +
> +
> +struct _parallelsConn {
> + virMutex lock;
> + virDomainObjList domains;
> + virStoragePoolObjList pools;
> + virCapsPtr caps;
> + virDomainEventStatePtr domainEventState;
> +};
> +
> +typedef struct _parallelsConn parallelsConn;
> +
> +typedef struct _parallelsConn *parallelsConnPtr;
All of the above definitions belong to the driver .c file as we don't
want to expose the internals.
> +
> +int parallelsRegister(void);
> +
> +#endif
> diff --git a/src/util/virterror.c b/src/util/virterror.c
> index cb37be0..7c0119f 100644
> --- a/src/util/virterror.c
> +++ b/src/util/virterror.c
> @@ -99,7 +99,8 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
>
> "URI Utils", /* 45 */
> "Authentication Utils",
> - "DBus Utils"
> + "DBus Utils",
> + "Parallels Cloud Server"
> )
I'm attaching a patch that contains my findings. I'm inclining to giving
an ACK to this patch with the changes I suggested, but I'd be glad if
somebody else could have a quick look. Let's see how the rest of the
series works.
Peter
-------------- next part --------------
diff --git a/configure.ac b/configure.ac
index eff36ce..14d53cd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2815,26 +2815,26 @@ AC_MSG_NOTICE([=====================])
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Drivers])
AC_MSG_NOTICE([])
-AC_MSG_NOTICE([ Xen: $with_xen])
-AC_MSG_NOTICE([ QEMU: $with_qemu])
-AC_MSG_NOTICE([ UML: $with_uml])
-AC_MSG_NOTICE([ OpenVZ: $with_openvz])
-AC_MSG_NOTICE([ VMware: $with_vmware])
-AC_MSG_NOTICE([ VBox: $with_vbox])
-AC_MSG_NOTICE([ XenAPI: $with_xenapi])
-AC_MSG_NOTICE([xenlight: $with_libxl])
-AC_MSG_NOTICE([ LXC: $with_lxc])
-AC_MSG_NOTICE([ PHYP: $with_phyp])
-AC_MSG_NOTICE([ ESX: $with_esx])
-AC_MSG_NOTICE([ Hyper-V: $with_hyperv])
-AC_MSG_NOTICE([ PARALLELS: $with_parallels])
-AC_MSG_NOTICE([ Test: $with_test])
-AC_MSG_NOTICE([ Remote: $with_remote])
-AC_MSG_NOTICE([ Network: $with_network])
-AC_MSG_NOTICE([Libvirtd: $with_libvirtd])
-AC_MSG_NOTICE([ netcf: $with_netcf])
-AC_MSG_NOTICE([ macvtap: $with_macvtap])
-AC_MSG_NOTICE([virtport: $with_virtualport])
+AC_MSG_NOTICE([ Xen: $with_xen])
+AC_MSG_NOTICE([ QEMU: $with_qemu])
+AC_MSG_NOTICE([ UML: $with_uml])
+AC_MSG_NOTICE([ OpenVZ: $with_openvz])
+AC_MSG_NOTICE([ VMware: $with_vmware])
+AC_MSG_NOTICE([ VBox: $with_vbox])
+AC_MSG_NOTICE([ XenAPI: $with_xenapi])
+AC_MSG_NOTICE([ xenlight: $with_libxl])
+AC_MSG_NOTICE([ LXC: $with_lxc])
+AC_MSG_NOTICE([ PHYP: $with_phyp])
+AC_MSG_NOTICE([ ESX: $with_esx])
+AC_MSG_NOTICE([ Hyper-V: $with_hyperv])
+AC_MSG_NOTICE([PARALLELS: $with_parallels])
+AC_MSG_NOTICE([ Test: $with_test])
+AC_MSG_NOTICE([ Remote: $with_remote])
+AC_MSG_NOTICE([ Network: $with_network])
+AC_MSG_NOTICE([ Libvirtd: $with_libvirtd])
+AC_MSG_NOTICE([ netcf: $with_netcf])
+AC_MSG_NOTICE([ macvtap: $with_macvtap])
+AC_MSG_NOTICE([ virtport: $with_virtualport])
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Storage Drivers])
AC_MSG_NOTICE([])
diff --git a/docs/drvparallels.html.in b/docs/drvparallels.html.in
index 976dea1..256faf9 100644
--- a/docs/drvparallels.html.in
+++ b/docs/drvparallels.html.in
@@ -2,7 +2,7 @@
<h1>Parallels Virtuozzo Server driver</h1>
<ul id="toc"></ul>
<p>
- The libvirt PARALLELS driver can manage Parallels Virtuozzo Server starting from 6.0 version.
+ The libvirt PARALLELS driver can manage Parallels Virtuozzo Server starting from version 6.0.
</p>
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 25e8d43..f69de4b 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -97,7 +97,8 @@ typedef enum {
VIR_FROM_URI = 45, /* Error from URI handling */
VIR_FROM_AUTH = 46, /* Error from auth handling */
VIR_FROM_DBUS = 47, /* Error from DBus */
- VIR_FROM_PARALLELS = 48, /* Error from PARALLELS */
+ VIR_FROM_PARALLELS = 48, /* Error from PARALLELS */
+
# ifdef VIR_ENUM_SENTINELS
VIR_ERR_DOMAIN_LAST
# endif
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 21d9bc0..d91d1c6 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -67,7 +67,7 @@
%define with_esx 0%{!?_without_esx:1}
%define with_hyperv 0%{!?_without_hyperv:1}
%define with_xenapi 0%{!?_without_xenapi:1}
-%define with_parallels 0%{!?_without_parallels:1}
+%define with_parallels 0%{!?_without_parallels:1}
# Then the secondary host drivers, which run inside libvirtd
%define with_network 0%{!?_without_network:%{server_drivers}}
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 1917899..1b361f3 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -62,8 +62,8 @@ src/nwfilter/nwfilter_learnipaddr.c
src/openvz/openvz_conf.c
src/openvz/openvz_driver.c
src/openvz/openvz_util.c
-src/phyp/phyp_driver.c
src/parallels/parallels_driver.c
+src/phyp/phyp_driver.c
src/qemu/qemu_agent.c
src/qemu/qemu_bridge_filter.c
src/qemu/qemu_capabilities.c
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 614f78f..a484b6b 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -51,12 +51,30 @@
#include "storage_file.h"
#include "nodeinfo.h"
#include "json.h"
+#include "domain_conf.h"
+#include "storage_conf.h"
+#include "domain_event.h"
#include "parallels_driver.h"
#define VIR_FROM_THIS VIR_FROM_PARALLELS
-static virCapsPtr parallelsBuildCapabilities(void);
+# define parallelsError(code, ...) \
+ virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
+ __FUNCTION__, __LINE__, __VA_ARGS__)
+# define PRLCTL "prlctl"
+
+struct _parallelsConn {
+ virMutex lock;
+ virDomainObjList domains;
+ virStoragePoolObjList pools;
+ virCapsPtr caps;
+ virDomainEventStatePtr domainEventState;
+};
+
+typedef struct _parallelsConn parallelsConn;
+typedef struct _parallelsConn *parallelsConnPtr;
+
static int parallelsClose(virConnectPtr conn);
static void
@@ -135,36 +153,33 @@ parallelsOpenDefault(virConnectPtr conn)
return VIR_DRV_OPEN_ERROR;
}
if (virMutexInit(&privconn->lock) < 0) {
- parallelsError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("cannot initialize mutex"));
+ parallelsError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot initialize mutex"));
goto error;
}
- parallelsDriverLock(privconn);
- conn->privateData = privconn;
- parallelsDriverUnlock(privconn);
-
if (!(privconn->caps = parallelsBuildCapabilities()))
goto error;
if (virDomainObjListInit(&privconn->domains) < 0)
goto error;
+ conn->privateData = privconn;
+
return VIR_DRV_OPEN_SUCCESS;
error:
virDomainObjListDeinit(&privconn->domains);
virCapabilitiesFree(privconn->caps);
virStoragePoolObjListFree(&privconn->pools);
- parallelsDriverUnlock(privconn);
- conn->privateData = NULL;
VIR_FREE(privconn);
return VIR_DRV_OPEN_ERROR;
}
static virDrvOpenStatus
parallelsOpen(virConnectPtr conn,
- virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags)
+ virConnectAuthPtr auth ATTRIBUTE_UNUSED,
+ unsigned int flags)
{
int ret;
parallelsConnPtr privconn;
@@ -181,11 +196,12 @@ parallelsOpen(virConnectPtr conn,
return VIR_DRV_OPEN_DECLINED;
/* From this point on, the connection is for us. */
- if (!conn->uri->path
- || conn->uri->path[0] == '\0'
- || (conn->uri->path[0] == '/' && conn->uri->path[1] == '\0')) {
- parallelsError(VIR_ERR_INVALID_ARG,
- "%s", _("parallelsOpen: supply a path or use parallels:///default"));
+ if (!conn->uri->path ||
+ conn->uri->path[0] == '\0' ||
+ (conn->uri->path[0] == '/' && conn->uri->path[1] == '\0')) {
+ parallelsError(VIR_ERR_INVALID_ARG, "%s",
+ _("parallelsOpen: supply a path or use "
+ "parallels:///default"));
return VIR_DRV_OPEN_ERROR;
}
@@ -260,10 +276,12 @@ parallelsRegister(void)
prlctl_path = virFindFileInPath(PRLCTL);
if (!prlctl_path) {
parallelsError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Can't find prlctl command in the PATH env"));
+ _("Can't find prlctl command in the PATH env"));
return VIR_DRV_OPEN_ERROR;
}
+ VIR_FREE(prlctl_path);
+
if (virRegisterDriver(¶llelsDriver) < 0)
return -1;
diff --git a/src/parallels/parallels_driver.h b/src/parallels/parallels_driver.h
index c04db35..460839f 100644
--- a/src/parallels/parallels_driver.h
+++ b/src/parallels/parallels_driver.h
@@ -23,29 +23,6 @@
#ifndef PARALLELS_DRIVER_H
# define PARALLELS_DRIVER_H
-
-# include "domain_conf.h"
-# include "storage_conf.h"
-# include "domain_event.h"
-
-# define parallelsError(code, ...) \
- virReportErrorHelper(VIR_FROM_TEST, code, __FILE__, \
- __FUNCTION__, __LINE__, __VA_ARGS__)
-# define PRLCTL "prlctl"
-
-
-struct _parallelsConn {
- virMutex lock;
- virDomainObjList domains;
- virStoragePoolObjList pools;
- virCapsPtr caps;
- virDomainEventStatePtr domainEventState;
-};
-
-typedef struct _parallelsConn parallelsConn;
-
-typedef struct _parallelsConn *parallelsConnPtr;
-
int parallelsRegister(void);
#endif
More information about the libvir-list
mailing list