[libvirt] [libvirt-glib] gconfig: API for SPICE image compression options
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> This patch adds API to set/get image compression options on domain/graphics[@type='spice'] nodes. Also included are simple tests for this API. --- libvirt-gconfig/Makefile.am | 2 + .../libvirt-gconfig-domain-graphics-spice-image.c | 139 +++++++++++++++++++++ .../libvirt-gconfig-domain-graphics-spice-image.h | 86 +++++++++++++ .../libvirt-gconfig-domain-graphics-spice.c | 34 +++++ .../libvirt-gconfig-domain-graphics-spice.h | 7 ++ libvirt-gconfig/libvirt-gconfig.h | 1 + libvirt-gconfig/libvirt-gconfig.sym | 14 +++ libvirt-gconfig/tests/test-domain-create.c | 14 +++ 8 files changed, 297 insertions(+) create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-spice-image.c create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-spice-image.h diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am index 6b3b2cb..7158bbd 100644 --- a/libvirt-gconfig/Makefile.am +++ b/libvirt-gconfig/Makefile.am @@ -41,6 +41,7 @@ GCONFIG_HEADER_FILES = \ libvirt-gconfig-domain-graphics.h \ libvirt-gconfig-domain-graphics-sdl.h \ libvirt-gconfig-domain-graphics-spice.h \ + libvirt-gconfig-domain-graphics-spice-image.h \ libvirt-gconfig-domain-graphics-vnc.h \ libvirt-gconfig-domain-input.h \ libvirt-gconfig-domain-interface.h \ @@ -118,6 +119,7 @@ GCONFIG_SOURCE_FILES = \ libvirt-gconfig-domain-graphics.c \ libvirt-gconfig-domain-graphics-sdl.c \ libvirt-gconfig-domain-graphics-spice.c \ + libvirt-gconfig-domain-graphics-spice-image.c \ libvirt-gconfig-domain-graphics-vnc.c \ libvirt-gconfig-domain-input.c \ libvirt-gconfig-domain-interface.c \ diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice-image.c b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice-image.c new file mode 100644 index 0000000..e4a964a --- /dev/null +++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice-image.c @@ -0,0 +1,139 @@ +/* + * libvirt-gconfig-domain-graphics-spice-image.c: libvirt domain SPICE image compression configuration + * + * Copyright (C) 2013 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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Zeeshan Ali (Khattak) <zeeshanak@gnome.org> + */ + +#include <config.h> +#include <string.h> + +#include "libvirt-gconfig/libvirt-gconfig.h" +#include "libvirt-gconfig/libvirt-gconfig-private.h" + +#define GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_GET_PRIVATE(obj) \ + (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE, GVirConfigDomainGraphicsSpiceImagePrivate)) + +struct _GVirConfigDomainGraphicsSpiceImagePrivate +{ + gboolean unused; +}; + +G_DEFINE_TYPE(GVirConfigDomainGraphicsSpiceImage, gvir_config_domain_graphics_spice_image, GVIR_CONFIG_TYPE_OBJECT); + + +static void gvir_config_domain_graphics_spice_image_class_init(GVirConfigDomainGraphicsSpiceImageClass *klass) +{ + g_type_class_add_private(klass, sizeof(GVirConfigDomainGraphicsSpiceImagePrivate)); +} + + +static void gvir_config_domain_graphics_spice_image_init(GVirConfigDomainGraphicsSpiceImage *image) +{ + g_debug("Init GVirConfigDomainGraphicsSpiceImage=%p", image); + + image->priv = GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_GET_PRIVATE(image); +} + + +GVirConfigDomainGraphicsSpiceImage *gvir_config_domain_graphics_spice_image_new(void) +{ + GVirConfigObject *object; + + object = gvir_config_object_new(GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE, + "image", NULL); + return GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE(object); +} + +GVirConfigDomainGraphicsSpiceImage * +gvir_config_domain_graphics_spice_image_new_from_xml(const gchar *xml, + GError **error) +{ + GVirConfigObject *object; + + object = gvir_config_object_new_from_xml + (GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE, + "image", NULL, xml, error); + return GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE(object); +} + +void gvir_config_domain_graphics_spice_image_set_compression + (GVirConfigDomainGraphicsSpiceImage *image, + GVirConfigDomainGraphicsSpiceImageCompression compression) +{ + const char *str; + char *value; + guint8 i; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE_IMAGE(image)); + + str = gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION, + compression); + g_return_if_fail(str != NULL); + + value = g_strdup(str); + /* glib-mkenum replaces '_' by default '-' in enum nicks and in this case + * we don't want that as libvirt use '_' rather than '-' for SPICE image + * compression attribute (unlike other attributes). + */ + for (i = 0; i < strlen(str); i++) { + if (value[i] == '-') + value[i] = '_'; + } + + gvir_config_object_set_attribute(GVIR_CONFIG_OBJECT(image), + "compression", value, + NULL); + g_free(value); +} + +int +gvir_config_domain_graphics_spice_image_get_compression + (GVirConfigDomainGraphicsSpiceImage *image) +{ + const char *str; + char *str_value; + int value; + guint8 i; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE_IMAGE(image), + GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_OFF); + + str = gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(image), + NULL, + "compression"); + if (str == NULL) + return GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_AUTO_GLZ; + + str_value = g_strdup(str); + /* See comment in gvir_config_domain_graphics_spice_image_set_compression() + * for why we are doing this. + */ + for (i = 0; i < strlen(str); i++) { + if (str_value[i] == '_') + str_value[i] = '-'; + } + + value = gvir_config_genum_get_value + (GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION, + str_value, + GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_OFF); + g_free(str_value); + + return value; +} diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice-image.h b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice-image.h new file mode 100644 index 0000000..6a28d1f --- /dev/null +++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice-image.h @@ -0,0 +1,86 @@ +/* + * libvirt-gconfig-domain-graphics-spice-image.h: libvirt domain SPICE image compression configuration + * + * Copyright (C) 2013 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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Zeeshan Ali (Khattak) <zeeshanak@gnome.org> + */ + +#if !defined(__LIBVIRT_GCONFIG_H__) && !defined(LIBVIRT_GCONFIG_BUILD) +#error "Only <libvirt-gconfig/libvirt-gconfig.h> can be included directly." +#endif + +#ifndef __LIBVIRT_GCONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_H__ +#define __LIBVIRT_GCONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_H__ + +#include <libvirt-gconfig/libvirt-gconfig-domain-timer.h> + +G_BEGIN_DECLS + +#define GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE (gvir_config_domain_graphics_spice_image_get_type ()) +#define GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE, GVirConfigDomainGraphicsSpiceImage)) +#define GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE, GVirConfigDomainGraphicsSpiceImageClass)) +#define GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE_IMAGE(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE)) +#define GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE_IMAGE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE)) +#define GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE, GVirConfigDomainGraphicsSpiceImageClass)) + +typedef struct _GVirConfigDomainGraphicsSpiceImage GVirConfigDomainGraphicsSpiceImage; +typedef struct _GVirConfigDomainGraphicsSpiceImagePrivate GVirConfigDomainGraphicsSpiceImagePrivate; +typedef struct _GVirConfigDomainGraphicsSpiceImageClass GVirConfigDomainGraphicsSpiceImageClass; + +struct _GVirConfigDomainGraphicsSpiceImage +{ + GVirConfigObject parent; + + GVirConfigDomainGraphicsSpiceImagePrivate *priv; + + /* Do not add fields to this struct */ +}; + +struct _GVirConfigDomainGraphicsSpiceImageClass +{ + GVirConfigObjectClass parent_class; + + gpointer padding[20]; +}; + +typedef enum { + GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_AUTO_GLZ, + GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_AUTO_LZ, + GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_QUIC, + GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_GLZ, + GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_LZ, + GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_OFF +} GVirConfigDomainGraphicsSpiceImageCompression; + +GType gvir_config_domain_graphics_spice_image_get_type(void); + +GVirConfigDomainGraphicsSpiceImage * +gvir_config_domain_graphics_spice_image_new(void); +GVirConfigDomainGraphicsSpiceImage * +gvir_config_domain_graphics_spice_image_new_from_xml(const gchar *xml, + GError **error); +void gvir_config_domain_graphics_spice_image_set_compression + (GVirConfigDomainGraphicsSpiceImage *image, + GVirConfigDomainGraphicsSpiceImageCompression compression); +int +gvir_config_domain_graphics_spice_image_get_compression + (GVirConfigDomainGraphicsSpiceImage *image); + +G_END_DECLS + +#endif /* __LIBVIRT_GCONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_H__ */ diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c index d090a3a..f9e13e5 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c @@ -122,3 +122,37 @@ void gvir_config_domain_graphics_spice_set_tls_port(GVirConfigDomainGraphicsSpic "tlsPort", G_TYPE_INT, port, NULL); } + +/** + * gvir_config_domain_graphics_spice_get_image: + * @graphics: a #GVirConfigDomainGraphicsSpice + * + * Gets the image compression configuration of @graphics + * + * Returns: (transfer full): A #GVirConfigDomainGraphicsSpiceImage. The returned + * object should be unreffed with g_object_unref() when no longer needed. + */ +GVirConfigDomainGraphicsSpiceImage * +gvir_config_domain_graphics_spice_get_image(GVirConfigDomainGraphicsSpice *graphics) +{ + GVirConfigObject *object; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE(graphics), NULL); + + object = gvir_config_object_get_child_with_type(GVIR_CONFIG_OBJECT(graphics), + "image", + GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE); + + return GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE(object); +} + +void gvir_config_domain_graphics_spice_set_image(GVirConfigDomainGraphicsSpice *graphics, + GVirConfigDomainGraphicsSpiceImage *image) +{ + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE(graphics)); + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE_IMAGE(image)); + + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(graphics), + "image", + GVIR_CONFIG_OBJECT(image)); +} diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h index c82615b..7b1596d 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h +++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h @@ -27,6 +27,8 @@ #ifndef __LIBVIRT_GCONFIG_DOMAIN_GRAPHICS_SPICE_H__ #define __LIBVIRT_GCONFIG_DOMAIN_GRAPHICS_SPICE_H__ +#include "libvirt-gconfig-domain-graphics-spice-image.h" + G_BEGIN_DECLS #define GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE (gvir_config_domain_graphics_spice_get_type ()) @@ -75,6 +77,11 @@ void gvir_config_domain_graphics_spice_set_port(GVirConfigDomainGraphicsSpice *g void gvir_config_domain_graphics_spice_set_tls_port(GVirConfigDomainGraphicsSpice *graphics, int port); +GVirConfigDomainGraphicsSpiceImage * +gvir_config_domain_graphics_spice_get_image(GVirConfigDomainGraphicsSpice *graphics); +void gvir_config_domain_graphics_spice_set_image(GVirConfigDomainGraphicsSpice *graphics, + GVirConfigDomainGraphicsSpiceImage *image); + G_END_DECLS #endif /* __LIBVIRT_GCONFIG_DOMAIN_GRAPHICS_SPICE_H__ */ diff --git a/libvirt-gconfig/libvirt-gconfig.h b/libvirt-gconfig/libvirt-gconfig.h index 9feaba2..df9399b 100644 --- a/libvirt-gconfig/libvirt-gconfig.h +++ b/libvirt-gconfig/libvirt-gconfig.h @@ -58,6 +58,7 @@ #include <libvirt-gconfig/libvirt-gconfig-domain-graphics.h> #include <libvirt-gconfig/libvirt-gconfig-domain-graphics-sdl.h> #include <libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h> +#include <libvirt-gconfig/libvirt-gconfig-domain-graphics-spice-image.h> #include <libvirt-gconfig/libvirt-gconfig-domain-graphics-vnc.h> #include <libvirt-gconfig/libvirt-gconfig-domain-input.h> #include <libvirt-gconfig/libvirt-gconfig-domain-interface.h> diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index d9cff90..a9d8066 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -504,4 +504,18 @@ LIBVIRT_GCONFIG_0.1.5 { gvir_config_domain_smartcard_passthrough_set_source; } LIBVIRT_GCONFIG_0.1.4; +LIBVIRT_GCONFIG_0.1.6 { + global: + gvir_config_domain_graphics_spice_get_image; + gvir_config_domain_graphics_spice_set_image; + + gvir_config_domain_graphics_spice_image_get_type; + gvir_config_domain_graphics_spice_image_compression_get_type; + + gvir_config_domain_graphics_spice_image_new; + gvir_config_domain_graphics_spice_image_new_from_xml; + gvir_config_domain_graphics_spice_image_set_compression; + gvir_config_domain_graphics_spice_image_get_compression; +} LIBVIRT_GCONFIG_0.1.5; + # .... define new API here using predicted next version number .... diff --git a/libvirt-gconfig/tests/test-domain-create.c b/libvirt-gconfig/tests/test-domain-create.c index 4c94b2a..d8466c8 100644 --- a/libvirt-gconfig/tests/test-domain-create.c +++ b/libvirt-gconfig/tests/test-domain-create.c @@ -288,6 +288,20 @@ int main(int argc, char **argv) graphics = gvir_config_domain_graphics_spice_new(); gvir_config_domain_graphics_spice_set_port(graphics, 1234); g_assert(gvir_config_domain_graphics_spice_get_port(graphics) == 1234); + + /* SPICE image compression configuration */ + GVirConfigDomainGraphicsSpiceImage *image; + + image = gvir_config_domain_graphics_spice_image_new(); + gvir_config_domain_graphics_spice_image_set_compression + (image, GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_AUTO_GLZ); + gvir_config_domain_graphics_spice_set_image(graphics, image); + g_object_unref(G_OBJECT(image)); + image = gvir_config_domain_graphics_spice_get_image(graphics); + g_assert(gvir_config_domain_graphics_spice_image_get_compression(image) == + GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_AUTO_GLZ); + g_object_unref(G_OBJECT(image)); + devices = g_list_append(devices, GVIR_CONFIG_DOMAIN_DEVICE(graphics)); /* video node */ -- 1.8.1.4
On Sat, Mar 09, 2013 at 04:44:14PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
This patch adds API to set/get image compression options on domain/graphics[@type='spice'] nodes.
Also included are simple tests for this API. --- libvirt-gconfig/Makefile.am | 2 + .../libvirt-gconfig-domain-graphics-spice-image.c | 139 +++++++++++++++++++++ .../libvirt-gconfig-domain-graphics-spice-image.h | 86 +++++++++++++ .../libvirt-gconfig-domain-graphics-spice.c | 34 +++++ .../libvirt-gconfig-domain-graphics-spice.h | 7 ++ libvirt-gconfig/libvirt-gconfig.h | 1 + libvirt-gconfig/libvirt-gconfig.sym | 14 +++ libvirt-gconfig/tests/test-domain-create.c | 14 +++ 8 files changed, 297 insertions(+) create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-spice-image.c create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-graphics-spice-image.h
ACK 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 :|
On Sat, Mar 09, 2013 at 04:44:14PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
This patch adds API to set/get image compression options on domain/graphics[@type='spice'] nodes.
Also included are simple tests for this API.
Do we really need a dedicated gobject for this? I would have gone with gvir_config_domain_graphics_spice_[gs]et_image_compression()
+void gvir_config_domain_graphics_spice_image_set_compression + (GVirConfigDomainGraphicsSpiceImage *image, + GVirConfigDomainGraphicsSpiceImageCompression compression) +{ + const char *str; + char *value; + guint8 i; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE_IMAGE(image)); + + str = gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION, + compression); + g_return_if_fail(str != NULL); + + value = g_strdup(str); + /* glib-mkenum replaces '_' by default '-' in enum nicks and in this case + * we don't want that as libvirt use '_' rather than '-' for SPICE image + * compression attribute (unlike other attributes). + */ + for (i = 0; i < strlen(str); i++) { + if (value[i] == '-') + value[i] = '_'; + }
Did you try something like typedef enum { GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_AUTO_GLZ, /*< nick=auto_glz >*/ GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_AUTO_LZ, /*< nick=auto_lz >*/ GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_QUIC, GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_GLZ, GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_LZ, GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_OFF } GVirConfigDomainGraphicsSpiceImageCompression; as described on https://developer.gnome.org/gobject/stable/glib-mkenums.html rather than doing it by hand at runtime? Christophe
On Tue, Mar 12, 2013 at 3:16 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Sat, Mar 09, 2013 at 04:44:14PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
This patch adds API to set/get image compression options on domain/graphics[@type='spice'] nodes.
Also included are simple tests for this API.
Do we really need a dedicated gobject for this? I would have gone with gvir_config_domain_graphics_spice_[gs]et_image_compression()
It would have been easier to go w/o a separate object but I went this way since 'image' is a separate node in the config and I would assume its for a reason. One reason that comes to mind is that in future there might be some other options being added here?
+void gvir_config_domain_graphics_spice_image_set_compression + (GVirConfigDomainGraphicsSpiceImage *image, + GVirConfigDomainGraphicsSpiceImageCompression compression) +{ + const char *str; + char *value; + guint8 i; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_SPICE_IMAGE(image)); + + str = gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION, + compression); + g_return_if_fail(str != NULL); + + value = g_strdup(str); + /* glib-mkenum replaces '_' by default '-' in enum nicks and in this case + * we don't want that as libvirt use '_' rather than '-' for SPICE image + * compression attribute (unlike other attributes). + */ + for (i = 0; i < strlen(str); i++) { + if (value[i] == '-') + value[i] = '_'; + }
Did you try something like typedef enum { GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_AUTO_GLZ, /*< nick=auto_glz >*/ GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_AUTO_LZ, /*< nick=auto_lz >*/ GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_QUIC, GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_GLZ, GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_LZ, GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_OFF } GVirConfigDomainGraphicsSpiceImageCompression;
as described on https://developer.gnome.org/gobject/stable/glib-mkenums.html rather than doing it by hand at runtime?
I didn't know of this cause one thing i didn't do was to read the docs. :( I'll try and update. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Tue, Mar 12, 2013 at 06:28:36PM +0200, Zeeshan Ali (Khattak) wrote:
On Tue, Mar 12, 2013 at 3:16 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Sat, Mar 09, 2013 at 04:44:14PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
This patch adds API to set/get image compression options on domain/graphics[@type='spice'] nodes.
Also included are simple tests for this API.
Do we really need a dedicated gobject for this? I would have gone with gvir_config_domain_graphics_spice_[gs]et_image_compression()
It would have been easier to go w/o a separate object but I went this way since 'image' is a separate node in the config and I would assume its for a reason. One reason that comes to mind is that in future there might be some other options being added here?
We don't always go with 'one node' = 'one class', see the <os> node for example, or gvir_config_domain_disk_set_startup_policy (various other similar examples in this case). For xml nodes that are very simple, they generally are modelled along with the parent node, we can always deprecate the simple setter in the parent node if the node gets more complex in the future. It's something to decide on a case by case basis, but here I'd tend to go with the simpler approach. Christophe
On Tue, Mar 12, 2013 at 7:07 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Mar 12, 2013 at 06:28:36PM +0200, Zeeshan Ali (Khattak) wrote:
On Tue, Mar 12, 2013 at 3:16 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Sat, Mar 09, 2013 at 04:44:14PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
This patch adds API to set/get image compression options on domain/graphics[@type='spice'] nodes.
Also included are simple tests for this API.
Do we really need a dedicated gobject for this? I would have gone with gvir_config_domain_graphics_spice_[gs]et_image_compression()
It would have been easier to go w/o a separate object but I went this way since 'image' is a separate node in the config and I would assume its for a reason. One reason that comes to mind is that in future there might be some other options being added here?
We don't always go with 'one node' = 'one class', see the <os> node for example, or gvir_config_domain_disk_set_startup_policy (various other similar examples in this case). For xml nodes that are very simple, they generally are modelled along with the parent node, we can always deprecate the simple setter in the parent node if the node gets more complex in the future. It's something to decide on a case by case basis, but here I'd tend to go with the simpler approach.
Ah ok, makes sense. I was under the impression that we always followed 'one node' = 'one class' rule. I'll change this part as well then. Thanks. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Tue, Mar 12, 2013 at 09:30:17PM +0200, Zeeshan Ali (Khattak) wrote:
Ah ok, makes sense. I was under the impression that we always followed 'one node' = 'one class' rule.
Not really, it's more 'one node with more than 2 'parameters'' = 'one class' or 'one node that is similar to another one' = 'one class for each of these nodes', but there is no hard rule. One node = one class is overkill in some situation, and not doing it saves some work (which you unfortunately have already done here :( Christophe
participants (3)
-
Christophe Fergeau -
Daniel P. Berrange -
Zeeshan Ali (Khattak)