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

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



On Thu, Jul 26, 2012 at 10:32:00PM +0400, Dmitry Guryanov wrote:
> Parallels Cloud Server is a cloud-ready virtualization
> solution that allows users to simultaneously run multiple virtual
> machines and containers on the same physical server.
> 
> More information can be found here: http://www.parallels.com/products/pcs/
> Also beta version of Parallels Cloud Server can be downloaded there.
> 
> Signed-off-by: Dmitry Guryanov <dguryanov parallels com>
> ---
>  configure.ac                     |   61 ++++++---
>  docs/drvparallels.html.in        |   28 ++++
>  include/libvirt/virterror.h      |    1 +
>  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 |  287 ++++++++++++++++++++++++++++++++++++++
>  src/parallels/parallels_driver.h |   28 ++++
>  src/util/virterror.c             |    3 +-
>  14 files changed, 429 insertions(+), 22 deletions(-)
>  create mode 100644 docs/drvparallels.html.in
>  create mode 100644 src/parallels/parallels_driver.c
>  create mode 100644 src/parallels/parallels_driver.h
> 

> diff --git a/docs/drvparallels.html.in b/docs/drvparallels.html.in
> new file mode 100644
> index 0000000..40a0fe5
> --- /dev/null
> +++ b/docs/drvparallels.html.in
> @@ -0,0 +1,28 @@
> +<html><body>
> +    <h1>Parallels Cloud Server driver</h1>
> +    <ul id="toc"></ul>
> +    <p>
> +        The libvirt Parallels driver can manage Parallels Cloud Server starting 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 Cloud Server</a> Virtualization Solution.
> +      </li>
> +    </ul>
> +
> +
> +    <h2><a name="uri">Connections to the Parallels Cloud 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>

It would be nice to have some examples of the suitable XML for running
a parallels container shown here too. That can be done as a later follow
up patch though. No need to change this patch though - I'm happy to see
it as a later addon patch.



> +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", PARALLELS_DEFAULT_ARCH,
> +                                         64, "parallels",
> +                                         NULL, 0, NULL)) == NULL)
> +        goto no_memory;

Hmm, perhaps I'm misunderstanding, but isn't Parallels a container
based technology like OpenVZ, rather than full machine virtualization ?

For containers we use 'exe' as the os type rather than 'hvm', and I'd
expect the arch + bit count to match the host OS.

> +static virDrvOpenStatus
> +parallelsOpen(virConnectPtr conn,
> +              virConnectAuthPtr auth ATTRIBUTE_UNUSED,
> +              unsigned int flags)
> +{
> +    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' ||
> +        (conn->uri->path[0] == '/' && conn->uri->path[1] == '\0')) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("parallelsOpen: supply a path or use "
> +                         "parallels:///default"));
> +        return VIR_DRV_OPEN_ERROR;
> +    }
> +
> +    if (STREQ(conn->uri->path, "/default"))

How do connections from parallels work from an end user POV ?

Is there is a single parallels service for the whole machine
which all users connect to, or does each user have their own
private service they work with.

If the former, I think it'd be preferable for the URI to be
either simple

  parallels:///

Or

  parallels:///system

to have a bit closer alignment with the naming we use with
LXC / QEMU / OpenBVZ.

If the latter, then I'd suggest parallels:///session to
indicate its tied to a per-user account/session.



> +static int
> +parallelsGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *hvVer)
> +{
> +    /* TODO */
> +    *hvVer = 6;
> +    return 0;
> +}

For this I reckon you basically want to return the version of the parallels
software.

> +int
> +parallelsRegister(void)
> +{
> +    char *prlctl_path;
> +
> +    prlctl_path = virFindFileInPath(PRLCTL);
> +    if (!prlctl_path) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Can't find prlctl command in the PATH env"));
> +        return VIR_DRV_OPEN_ERROR;

You can just return '0' here, and rather than reporting an error,
you want to just log a VIR_DEBUG statement I reckon, since you
don't want to cause initialization of libvirt as a whole to fail
just because parallels isn't installed.

> +    }
> +
> +    VIR_FREE(prlctl_path);
> +
> +    if (virRegisterDriver(&parallelsDriver) < 0)
> +        return -1;
> +
> +    return 0;
> +}


If you can either answer my questions or address them, I'd ACK this now.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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