[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v8 1/8] parallels: add driver skeleton



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 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(&parallelsDriver) < 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

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(&parallelsDriver) < 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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]