[libvirt] [libvirt-glib PATCHv2] Add gvir_domain_open_graphics_fd()

[Christophe Fergeau]

Hey,

On Fri, Nov 21, 2014 at 03:11:36PM +0000, Zeeshan Ali (Khattak) wrote:
> Add binding for virDomainOpenGraphicsFD. If virDomainOpenGraphicsFD is
> not available, it means we are dealing with older libvirt so we create
> the socket pair ourselves if that is the case.
> ---
>  configure.ac                             |  4 ++
>  libvirt-gobject/libvirt-gobject-domain.c | 72 ++++++++++++++++++++++++++++++++
>  libvirt-gobject/libvirt-gobject-domain.h |  4 ++
>  libvirt-gobject/libvirt-gobject.sym      |  5 +++
>  4 files changed, 85 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 8cc3fca..bcb5cda 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -93,6 +93,10 @@ m4_if(m4_version_compare([2.61a.100],
>  LIBVIRT_GLIB_COMPILE_WARNINGS
>  
>  PKG_CHECK_MODULES(LIBVIRT, libvirt >= $LIBVIRT_REQUIRED)
> +# virDomainOpenGraphicsFD was introduced in libvirt 1.2.8
> +AC_CHECK_LIB([virt],
> +             [virDomainOpenGraphicsFD],
> +             [AC_DEFINE([HAVE_VIR_DOMAIN_OPEN_GRAPHICS_FD], 1, [Have virDomainOpenGraphicsFD?])])
>  enable_tests=no
>  PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_TEST_REQUIRED,
>                    [enable_tests=yes],
> diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c
> index 8df30d7..d41dadd 100644
> --- a/libvirt-gobject/libvirt-gobject-domain.c
> +++ b/libvirt-gobject/libvirt-gobject-domain.c
> @@ -25,6 +25,7 @@
>  
>  #include <libvirt/virterror.h>
>  #include <string.h>

> +#include <sys/socket.h>

This could be enclosed in

#ifndef HAVE_VIR_DOMAIN_OPEN_GRAPHICS_FD

but that's not very important

>  
>  #include "libvirt-glib/libvirt-glib.h"
>  #include "libvirt-gobject/libvirt-gobject.h"
> @@ -1222,6 +1223,77 @@ cleanup:
>  }
>  
>  /**
> + * gvir_domain_open_graphics_fd:
> + * @dom: the domain
> + * @idx: the graphics index
> + * @flags: extra flags, currently unused
> + *
> + * This will create a socket pair connected to the graphics backend of @dom. One
> + * end of the socket will be returned on success, and the other end is handed to
> + * the hypervisor. If @dom has multiple graphics backends configured, then @idx
> + * will determine which one is opened, starting from @idx 0.
> + *
> + * Returns: An fd on success, -1 on failure.
> + *
> + * Since: 0.2.0
> + */
> +int gvir_domain_open_graphics_fd(GVirDomain *dom,
> +                                 guint idx,
> +                                 unsigned int flags,
> +                                 GError **err)
> +{
> +    GVirDomainPrivate *priv;
> +    int ret = -1;
> +#ifndef HAVE_VIR_DOMAIN_OPEN_GRAPHICS_FD
> +    int pair[2];
> +#endif
> +
> +    g_return_val_if_fail(GVIR_IS_DOMAIN(dom), -1);
> +    g_return_val_if_fail(err == NULL || *err == NULL, -1);
> +
> +    priv = dom->priv;
> +
> +#ifdef HAVE_VIR_DOMAIN_OPEN_GRAPHICS_FD
> +    ret = virDomainOpenGraphicsFD(priv->handle, idx, flags);
> +    if (ret <= 0) {
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Unable to open graphics");
> +        goto end;
> +    }
> +#else
> +

I'd either get rid of that blank line, or add one before the #else as
well.

> +    if (socketpair(PF_UNIX, SOCK_STREAM, 0, pair) < 0) {
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               "Failed to create socket pair");

gvir_set_error_literal should only be used to report errors from libvirt
API calls as this will virGetLastError() to generate the error message.

> +        goto end;
> +    }
> +
> +    if (virDomainOpenGraphics(priv->handle, idx, pair[0], flags) < 0) {
> +        virErrorPtr vir_err;
> +
> +        vir_err = virGetLastError();
> +        gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
> +                               0,
> +                               vir_err && vir_err->message ?
> +                               vir_err->message :
> +                               "Unable to open graphics");

You don't need to look at vir_err->message here as
gvir_set_error_literal will already append it to the end of the
GError::message.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141125/a1123fc2/attachment-0001.sig>