[libvirt] [PATCH v5 1/9] pvs: add driver skeleton
Eric Blake
eblake at redhat.com
Mon Apr 30 23:27:26 UTC 2012
On 04/20/2012 10:01 AM, Dmitry Guryanov wrote:
> Add driver, which can report node info only.
Since this is the first commit in the series, can you please add more
information about pvs? This content from your 0/9 message would be
useful here:
>> 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 driver will work with PVS version 6.0 , beta version
>> scheduled at 2012 Q2.
I'm still thinking this series might be worth including in 0.9.12, but
I'm worried about getting good reviews (as you can see, I'm only on 1/9
and ran out of time today).
>
> changes:
> * add me to AUTHORS
> * fix indent in preprocessor directives in pvs_driver.h
> * remove unneded include
> * remove pvs_driver.c from po/POTFILES.in
Normally, it helps to list patch version history...
>
> Signed-off-by: Dmitry Guryanov <dguryanov at parallels.com>
> ---
after this line, so that it doesn't become a permanent part of
libvirt.git (that is, it is useful for review purposes, but once the
final patch is accepted, we no longer care how we got to the final patch).
> AUTHORS | 1 +
> cfg.mk | 1 +
> configure.ac | 23 ++++
> docs/drvpvs.html.in | 28 +++++
> include/libvirt/virterror.h | 1 +
> libvirt.spec.in | 7 +
> mingw32-libvirt.spec.in | 6 +
> src/Makefile.am | 21 ++++
> src/conf/domain_conf.c | 3 +-
> src/conf/domain_conf.h | 1 +
> src/driver.h | 1 +
> src/libvirt.c | 12 ++
> src/pvs/pvs_driver.c | 271 +++++++++++++++++++++++++++++++++++++++++++
> src/pvs/pvs_driver.h | 51 ++++++++
> src/util/virterror.c | 3 +
> 15 files changed, 429 insertions(+), 1 deletions(-)
> create mode 100644 docs/drvpvs.html.in
> create mode 100644 src/pvs/pvs_driver.c
> create mode 100644 src/pvs/pvs_driver.h
Run 'make syntax-check' on your source; you missed at least one change:
po_check
--- ./po/POTFILES.in
+++ ./po/POTFILES.in
@@ -61,6 +61,7 @@
src/openvz/openvz_conf.c
src/openvz/openvz_driver.c
src/phyp/phyp_driver.c
+src/pvs/pvs_driver.c
src/qemu/qemu_agent.c
src/qemu/qemu_bridge_filter.c
src/qemu/qemu_capabilities.c
maint.mk: you have changed the set of files with translatable diagnostics;
apply the above patch
make: *** [sc_po_check] Error 1
> dnl
> +dnl Checks for the PVS driver
> +dnl
> +
> +if test "$with_pvs" = "check"; then
> + with_pvs=$with_linux
> + if test ! $(uname -i) = 'x86_64'; then
This will not work when cross-compiling. For that matter, 'uname -i' is
not portable. I'm not sure what better solution there is for deciding
whether to default things to true or false based on whether the $host
will be 64-bit, but it's certainly not right to be probing uname of $build.
> @@ -2706,6 +2728,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([ PVS: $with_pvs])
Line up the ':'.
> +++ b/src/pvs/pvs_driver.c
> @@ -0,0 +1,271 @@
> +/*
> + * pvs_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
Not a problem with your patch, per se, but we really should be using the
FSF-preferred form of LGPLv2+ boilerplate that uses a URL rather than a
street address (that's a global cleanup to all of libvirt).
> +
> +static virDriver pvsDriver = {
> + .no = VIR_DRV_PVS,
> + .name = "PVS",
> + .open = pvsOpen, /* 0.9.12 */
> + .close = pvsClose, /* 0.9.12 */
> + .version = pvsGetVersion, /* 0.9.12 */
> + .getHostname = virGetHostname, /* 0.9.12 */
> + .nodeGetInfo = nodeGetInfo, /* 0.9.12 */
> + .getCapabilities = pvsGetCapabilities, /* 0.9.12 */
> +};
> +
> +/**
> + * pvsRegister:
> + *
> + * Registers the pvs driver
> + */
> +int
> +pvsRegister(void)
> +{
> + if (virRegisterDriver(&pvsDriver) < 0)
> + return -1;
Should we be checking for whether the PRLCTL binary even exists, before
registering this driver?
> +++ b/src/util/virterror.c
> @@ -175,6 +175,9 @@ static const char *virErrorDomainName(virErrorDomain domain) {
> case VIR_FROM_HYPERV:
> dom = "Hyper-V ";
> break;
> + case VIR_FROM_PVS:
> + dom = "Parallels Virtuozzo Server ";
> + break;
> case VIR_FROM_CAPABILITIES:
> dom = "Capabilities ";
> break;
Unusual ordering; typically, it's nice to keep case statements in the
same order as the enum declarations.
--
Eric Blake eblake at redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120430/6fec7773/attachment-0001.sig>
More information about the libvir-list
mailing list