Hey,
'GVirCofigDomainGraphicsRemote' typo in the subject
On Tue, Mar 22, 2016 at 11:04:46AM +0100, Fabiano Fidêncio wrote:
> Seems that GVirConfigDomainGraphics* were built with a strong focus on
> writing/setting configs, but not reading those.
>
> For instance, considering virt-viewer's case, where the app is just
> consuming an already built xml, for getting the port attribute of a
> GVirDomainConfigGraphis{Spice,Vnc} you have to know, beforehand, the
> type of the connection and then call
> gvir_config_domain_graphics_{sdl,spice}_get_port(). It means creating an
> abstraction on virt-viewer side, that will ended up in some code like:
> if (GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE(graphics))
> _get_whatever_you_want_using_specific_spice_api()
> else
> _get_whatever_you want_using_specific_vnc_api()
>
> In order to avoid this, let's introduce GVirConfigDomainGraphicsRemote
> class that, at least for now, is intended to be a helper for the case
> explained above. It introduces a new hierarchy in the project, where,
> instead of having GVirConfigDomainGraphics{Spice,Vnc,Rdp} inheriting
> from GVirCOnfigDomainGraphics, these classes will inherit from
> GVirConfigDomainGraphicsRemote (see the next patches) which inherits
> from from GVirConfigGraphics (it will cause an ABI breakage, though).
Yes, the ABI breakage will be noticed if people have classes inheriting
from GVirConfigDomainGraphics{Spice,Vnc,Rdp}, or if there is code
expecting direct inheritance between these classes and
GVirConfigDomainGraphics. Imo it's reasonable to assume that at this
point no code is doing that, and that we can avoid raising the soname
for this change.
>
> Signed-off-by: Fabiano Fidêncio <fidencio@xxxxxxxxxx>
> ---
> libvirt-gconfig/Makefile.am | 2 +
> .../libvirt-gconfig-domain-graphics-remote.c | 103 +++++++++++++++++++++
> .../libvirt-gconfig-domain-graphics-remote.h | 70 ++++++++++++++
> libvirt-gconfig/libvirt-gconfig.h | 1 +
> libvirt-gconfig/libvirt-gconfig.sym | 5 +
> po/POTFILES.in | 1 +
> 6 files changed, 182 insertions(+)
> create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.c
> create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.h
>
> diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
> index f308539..45fc559 100644
> --- a/libvirt-gconfig/Makefile.am
> +++ b/libvirt-gconfig/Makefile.am
> @@ -47,6 +47,7 @@ GCONFIG_HEADER_FILES = \
> libvirt-gconfig-domain-graphics.h \
> libvirt-gconfig-domain-graphics-desktop.h \
> libvirt-gconfig-domain-graphics-rdp.h \
> + libvirt-gconfig-domain-graphics-remote.h \
> libvirt-gconfig-domain-graphics-sdl.h \
> libvirt-gconfig-domain-graphics-spice.h \
> libvirt-gconfig-domain-graphics-vnc.h \
> @@ -138,6 +139,7 @@ GCONFIG_SOURCE_FILES = \
> libvirt-gconfig-domain-graphics.c \
> libvirt-gconfig-domain-graphics-desktop.c \
> libvirt-gconfig-domain-graphics-rdp.c \
> + libvirt-gconfig-domain-graphics-remote.c \
> libvirt-gconfig-domain-graphics-sdl.c \
> libvirt-gconfig-domain-graphics-spice.c \
> libvirt-gconfig-domain-graphics-vnc.c \
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.c b/libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.c
> new file mode 100644
> index 0000000..e8b090f
> --- /dev/null
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.c
> @@ -0,0 +1,103 @@
> +/*
> + * libvirt-gconfig-domain-graphics-remote.c: libvirt domain graphics remote configuration
> + *
> + * Copyright (C) 2016 Red Hat, 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, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Fabiano Fidêncio <fidencio@xxxxxxxxxx>
> + */
> +
> +#include <config.h>
> +
> +#include <glib/gi18n-lib.h>
> +
> +#include "libvirt-gconfig/libvirt-gconfig.h"
> +#include "libvirt-gconfig/libvirt-gconfig-private.h"
> +
> +#define GVIR_CONFIG_DOMAIN_GRAPHICS_REMOTE_GET_PRIVATE(obj) \
> + (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_REMOTE, GVirConfigDomainGraphicsRemotePrivate))
> +
> +struct _GVirConfigDomainGraphicsRemotePrivate
> +{
> + gboolean unused;
> +};
> +
> +typedef GVirConfigObject *(*GVirConfigDomainGraphicsRemoteNewFromXml)(const gchar *xml, GError **error);
Maybe this could be local to _new_from_xml ?
> +
> +G_DEFINE_ABSTRACT_TYPE(GVirConfigDomainGraphicsRemote, gvir_config_domain_graphics_remote, GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS);
> +
> +static void gvir_config_domain_graphics_remote_class_init(GVirConfigDomainGraphicsRemoteClass *klass)
> +{
> + g_type_class_add_private(klass, sizeof(GVirConfigDomainGraphicsRemotePrivate));
> +}
> +
> +static void gvir_config_domain_graphics_remote_init(GVirConfigDomainGraphicsRemote *graphics)
> +{
> + graphics->priv = GVIR_CONFIG_DOMAIN_GRAPHICS_REMOTE_GET_PRIVATE(graphics);
> +}
> +
> +GVirConfigDomainGraphicsRemote *
> +gvir_config_domain_graphics_remote_new_from_xml(const gchar *xml,
> + GError **error)
> +{
> + GVirConfigDomainGraphicsRemoteNewFromXml functions[] = {
> + (GVirConfigDomainGraphicsRemoteNewFromXml)gvir_config_domain_graphics_vnc_new_from_xml,
> + (GVirConfigDomainGraphicsRemoteNewFromXml)gvir_config_domain_graphics_spice_new_from_xml,
> + (GVirConfigDomainGraphicsRemoteNewFromXml)gvir_config_domain_graphics_rdp_new_from_xml,
> + };
> + GVirConfigObject *object;
> +
I think I would look at /graphics/type here and directly call the right
function.
However, I'm not sure how much use there is in adding these methods in
all objects as I don't think there are any users of these. dunno if we should
keep adding them, or just stop, and add them all at once the day they
are needed.
> + for (int i = 0; i < G_N_ELEMENTS(functions); i++) {
> + GVirConfigDomainGraphicsRemoteNewFromXml function = functions[i];
> +
> + object = GVIR_CONFIG_OBJECT(function(xml, NULL));
> + if (object != NULL)
> + break;
> + }
> +
> + if (object == NULL) {
> + g_set_error(error,
> + GVIR_CONFIG_OBJECT_ERROR,
> + 0,
> + _("Unable to create a new GraphicRemote object from the XML"));
> + }
> +
> + return GVIR_CONFIG_DOMAIN_GRAPHICS_REMOTE(object);
> +}
> +
> +gboolean gvir_config_domain_graphics_remote_get_autoport(GVirConfigDomainGraphicsRemote *graphics)
> +{
> + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_REMOTE(graphics), FALSE);
> +
> + return gvir_config_object_get_attribute_boolean(GVIR_CONFIG_OBJECT(graphics),
> + NULL, "autoport", FALSE);
> +}
> +
> +const gchar *gvir_config_domain_graphics_remote_get_host(GVirConfigDomainGraphicsRemote *graphics)
> +{
> + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_REMOTE(graphics), NULL);
> +
> + return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(graphics),
> + NULL, "listen");
> +}
> +
> +int gvir_config_domain_graphics_remote_get_port(GVirConfigDomainGraphicsRemote *graphics)
> +{
> + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_REMOTE(graphics), 0);
> +
> + return gvir_config_object_get_attribute_uint64(GVIR_CONFIG_OBJECT(graphics),
> + NULL, "port", 0);
> +}
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.h b/libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.h
> new file mode 100644
> index 0000000..d9de6df
> --- /dev/null
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-remote.h
> @@ -0,0 +1,70 @@
> +/*
> + * libvirt-gconfig-domain-graphics-remote.h: libvirt domain graphics remote configuration
Extra space between 'graphics' and 'remote'
Looks good otherwise.
Christophe
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list