On 07/10/2012 06:53 PM, Peter Krempa wrote:
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
....
> +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
Yes, I'll fix this soon.
> + 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.
Eric Blake suggested moving this check from parallelsOpen to
parallelsRegister,
http://www.redhat.com/archives/libvir-list/2012-May/msg00026.html.
> +
> + 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
Thanks, I agree with all your comments and fixes.
--
Dmitry Guryanov