[libvirt] [PATCH 1/1] [RFC] Parallels Server Bare Metal driver stub

[Eric Blake]

On 09/26/2011 05:59 AM, Dmitry Mishin wrote:
> Parallels Server Bare Metal is a virtualization solution which allows to run
> both Containers (OpenVZ-like) and virtual machines (like any other hardware
> hypervisor).

Any web page link so we can familiarize ourselves with the project?

>
> This patch implements initial driver stub with almost no functionality except an
> ability to open and close driver. At the same time it contains initialization of
> Parallels API and clearly demonstrates supposed approach to future driver
> implementation. In particular, I suppose to use parallels-virtualization-sdk
> library provisioned with the PSBM distribution (and separately also) for PSBM's
> virtual servers management.
>
> Signed-off-by: Dmitry Mishin<dim at parallels.com>
> ---
>   autobuild.sh                |    1 +
>   configure.ac                |   21 +++++
>   include/libvirt/virterror.h |    1 +
>   src/Makefile.am             |   22 +++++
>   src/README                  |    1 +
>   src/driver.h                |    1 +
>   src/libvirt.c               |    9 ++
>   src/psbm/psbm_api.c         |  117 ++++++++++++++++++++++++
>   src/psbm/psbm_api.h         |   59 ++++++++++++
>   src/psbm/psbm_driver.c      |  208 +++++++++++++++++++++++++++++++++++++++++++
>   src/psbm/psbm_driver.h      |   32 +++++++
>   src/psbm/psbm_private.h     |   49 ++++++++++
>   src/util/virterror.c        |    3 +

Missing a hypervisor stub page under docs/ (that would be a good place 
to include the link I mentioned above).  Is this driver more like qemu, 
where it requires talking to a daemon like libvirtd (and where remote 
access is provided by libvirt), or more like esx, where it is 
translating things at the client level while talking to an esx protocol 
(and where remote access is provided by the hypervisor)?

Missing changes to libvirt.spec.in and mingw32-libvirt.spec.in for 
making compilation of the new driver conditional when building for 
Fedora and friends.

I'd feel a bit more comfortable reviewing this patch if you also had a 
followon patch that can do some basic APIs, such as start and stop 
guests, or even just list the names of running guests.  Of course, those 
should be separate patches, but it is better to push a patch series that 
makes the hypervisor driver useful, rather than just pushing this patch 
in isolation where the hypervisor driver can't do anything at all.  Look 
at the recent HyperV hypervisor driver addition (around commit 5e3b0f8) 
for an example of what all is needed to make this driver addition 
successful.
> +
> +if test "$with_psbm" = "yes"&&  test "$with_linux" = "no"; then
> +    AC_MSG_ERROR([The PSBM driver can be enabled on Linux only.])

Are there any libraries that have to be linked in?  Is this all calls to 
third-party program(s) (in which case, should configure probe for the 
absolute path of that program)?  Is it really Linux-only, or if those 
programs someday exist on other platforms, have you artificially 
restricted this driver?

> +int psbmApiInit(struct psbm_driver *driver)
> +{
> +    const char *libname = "libprl_sdk.so";

Looks like you are using library calls, so configure needs to probe for 
the existence of this library.

> +
> +static virDriver psbmDriver = {
> +    .no = VIR_DRV_PSBM,
> +    .name = "PSBM",
> +    .open = psbmOpen, /* 0.3.1 */
> +    .close = psbmClose, /* 0.3.1 */
> +    .type = psbmGetType, /* 0.3.1 */
> +    .version = psbmGetVersion, /* 0.3.1 */
> +    .nodeGetInfo = nodeGetInfo, /* 0.3.1 */
> +    .listDomains = psbmListDomains, /* 0.3.1 */
> +    .numOfDomains = psbmNumDomains, /* 0.3.1 */

Wrong version.  You are adding this functions as of 0.9.7, not 0.3.1.

I haven't reviewed this closely, because I'm not familiar enough with 
PSBM yet, but in general, I'm in favor of adding hypervisor drivers, so 
I hope to see this go somewhere.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org