Re: [libvirt] [libvirt-glib 03/20] gconfig: Introduce GVirConfigDomainGraphicsListenAddress

2016-10-06 Thread Daniel P. Berrange 
On Wed, Oct 05, 2016 at 09:37:52AM +0200, Christophe Fergeau wrote:
> hey,
> 
> On Tue, Oct 04, 2016 at 04:30:14PM +0100, Daniel P. Berrange wrote:
> > > +
> > > +/**
> > > + * gvir_config_domain_graphics_listen_address_get_inet_address:
> > > + *
> > > + * Returns the #GInetAddress associated with the 
> > > #GVirConfigDomainGraphicsListenAddress.
> > > + *
> > > + * Returns: (transfer full): a #GInetAddress.
> > > + *
> > > + */
> > > +GInetAddress *
> > > +gvir_config_domain_graphics_listen_address_get_inet_address(GVirConfigDomainGraphicsListenAddress
> > >  *listen)
> > > +{
> > > +
> > > g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_LISTEN_ADDRESS(listen),
> > >  NULL);
> > > +
> > > +const gchar *address = 
> > > gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(listen),
> > > +NULL,
> > > +"address");
> > > +return g_inet_address_new_from_string(address);
> > > +}
> > 
> > IIUC GInetAddress only supports numeric IP addresses, where as libvirt
> > also allows hostnames anywhere that an IP address is used. So apps
> > using get_inet_address are liable to get NULL returned if we use
> > GInetAddress.
> > 
> > So IMHO we should not use GInetAddress as it'd lead to apps which
> > blindly use this API not realising they'll break if someone put a
> > hostname in the XML until it is too late.
> 
> Good point. Should we keep _set_inet_address() which would limit what we
> can set, but would not lead to unexpected results depending on the XML
> content, or should we just drop the GInetAddress altogether, and only
> keep the string based API (which is what Visarion initially proposed).

I think I'd just drop GInetAddress completely to avoid confusion.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-glib 03/20] gconfig: Introduce GVirConfigDomainGraphicsListenAddress

2016-10-05 Thread Christophe Fergeau 
hey,

On Tue, Oct 04, 2016 at 04:30:14PM +0100, Daniel P. Berrange wrote:
> > +
> > +/**
> > + * gvir_config_domain_graphics_listen_address_get_inet_address:
> > + *
> > + * Returns the #GInetAddress associated with the 
> > #GVirConfigDomainGraphicsListenAddress.
> > + *
> > + * Returns: (transfer full): a #GInetAddress.
> > + *
> > + */
> > +GInetAddress *
> > +gvir_config_domain_graphics_listen_address_get_inet_address(GVirConfigDomainGraphicsListenAddress
> >  *listen)
> > +{
> > +
> > g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_LISTEN_ADDRESS(listen), 
> > NULL);
> > +
> > +const gchar *address = 
> > gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(listen),
> > +NULL,
> > +"address");
> > +return g_inet_address_new_from_string(address);
> > +}
> 
> IIUC GInetAddress only supports numeric IP addresses, where as libvirt
> also allows hostnames anywhere that an IP address is used. So apps
> using get_inet_address are liable to get NULL returned if we use
> GInetAddress.
> 
> So IMHO we should not use GInetAddress as it'd lead to apps which
> blindly use this API not realising they'll break if someone put a
> hostname in the XML until it is too late.

Good point. Should we keep _set_inet_address() which would limit what we
can set, but would not lead to unexpected results depending on the XML
content, or should we just drop the GInetAddress altogether, and only
keep the string based API (which is what Visarion initially proposed).

Christophe


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-glib 03/20] gconfig: Introduce GVirConfigDomainGraphicsListenAddress

2016-10-04 Thread Daniel P. Berrange 
On Tue, Oct 04, 2016 at 05:23:41PM +0200, Christophe Fergeau wrote:
> From: Visarion Alexandru 
> 
> This is needed to be able to change the address a graphics
> device is listening for.
> ---
>  libvirt-gconfig/Makefile.am|   2 +
>  ...ibvirt-gconfig-domain-graphics-listen-address.c | 131 
> +
>  ...ibvirt-gconfig-domain-graphics-listen-address.h |  79 +
>  libvirt-gconfig/libvirt-gconfig.h  |   1 +
>  libvirt-gconfig/libvirt-gconfig.sym|   7 ++
>  5 files changed, 220 insertions(+)
>  create mode 100644 
> libvirt-gconfig/libvirt-gconfig-domain-graphics-listen-address.c
>  create mode 100644 
> libvirt-gconfig/libvirt-gconfig-domain-graphics-listen-address.h

> +void 
> gvir_config_domain_graphics_listen_address_set_address(GVirConfigDomainGraphicsListenAddress
>  *listen,
> +const gchar 
> *address)
> +{
> +g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_LISTEN_ADDRESS(listen));
> +
> +gvir_config_object_set_attribute(GVIR_CONFIG_OBJECT(listen),
> + "address", address,
> + NULL);
> +}
> +
> +const gchar *
> +gvir_config_domain_graphics_listen_address_get_address(GVirConfigDomainGraphicsListenAddress
>  *listen)
> +{
> +
> g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_LISTEN_ADDRESS(listen), 
> NULL);
> +
> +return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(listen),
> +NULL,
> +"address");
> +}
> +
> +void 
> gvir_config_domain_graphics_listen_address_set_inet_address(GVirConfigDomainGraphicsListenAddress
>  *listen,
> + 
> GInetAddress *address)
> +{
> +char *address_str;
> +g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_LISTEN_ADDRESS(listen));
> +
> +address_str = g_inet_address_to_string(address);
> +gvir_config_object_set_attribute(GVIR_CONFIG_OBJECT(listen),
> + "address", address_str,
> + NULL);
> +g_free(address_str);
> +}
> +
> +/**
> + * gvir_config_domain_graphics_listen_address_get_inet_address:
> + *
> + * Returns the #GInetAddress associated with the 
> #GVirConfigDomainGraphicsListenAddress.
> + *
> + * Returns: (transfer full): a #GInetAddress.
> + *
> + */
> +GInetAddress *
> +gvir_config_domain_graphics_listen_address_get_inet_address(GVirConfigDomainGraphicsListenAddress
>  *listen)
> +{
> +
> g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_GRAPHICS_LISTEN_ADDRESS(listen), 
> NULL);
> +
> +const gchar *address = 
> gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(listen),
> +NULL,
> +"address");
> +return g_inet_address_new_from_string(address);
> +}

IIUC GInetAddress only supports numeric IP addresses, where as libvirt
also allows hostnames anywhere that an IP address is used. So apps
using get_inet_address are liable to get NULL returned if we use
GInetAddress.

So IMHO we should not use GInetAddress as it'd lead to apps which
blindly use this API not realising they'll break if someone put a
hostname in the XML until it is too late.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [libvirt-glib 03/20] gconfig: Introduce GVirConfigDomainGraphicsListenAddress

2016-10-04 Thread Christophe Fergeau 
From: Visarion Alexandru 

This is needed to be able to change the address a graphics
device is listening for.
---
 libvirt-gconfig/Makefile.am|   2 +
 ...ibvirt-gconfig-domain-graphics-listen-address.c | 131 +
 ...ibvirt-gconfig-domain-graphics-listen-address.h |  79 +
 libvirt-gconfig/libvirt-gconfig.h  |   1 +
 libvirt-gconfig/libvirt-gconfig.sym|   7 ++
 5 files changed, 220 insertions(+)
 create mode 100644 
libvirt-gconfig/libvirt-gconfig-domain-graphics-listen-address.c
 create mode 100644 
libvirt-gconfig/libvirt-gconfig-domain-graphics-listen-address.h

diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
index ec26ae7..045f570 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-listen.h \
+   libvirt-gconfig-domain-graphics-listen-address.h \
libvirt-gconfig-domain-graphics-rdp.h \
libvirt-gconfig-domain-graphics-sdl.h \
libvirt-gconfig-domain-graphics-spice.h \
@@ -141,6 +142,7 @@ GCONFIG_SOURCE_FILES = \
libvirt-gconfig-domain-graphics.c \
libvirt-gconfig-domain-graphics-desktop.c \
libvirt-gconfig-domain-graphics-listen.c \
+   libvirt-gconfig-domain-graphics-listen-address.c \
libvirt-gconfig-domain-graphics-rdp.c \
libvirt-gconfig-domain-graphics-sdl.c \
libvirt-gconfig-domain-graphics-spice.c \
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-listen-address.c 
b/libvirt-gconfig/libvirt-gconfig-domain-graphics-listen-address.c
new file mode 100644
index 000..2489327
--- /dev/null
+++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-listen-address.c
@@ -0,0 +1,131 @@
+/*
+ * libvirt-gconfig-domain-graphics-listen-address.c: libvirt domain graphics 
listen address configuration
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ * Copyright (C) 2016 Visarion Alexandru 
+ *
+ * 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
+ * .
+ *
+ * Author: Visarion Alexandru 
+ */
+
+#include 
+
+#include 
+
+#include "libvirt-gconfig/libvirt-gconfig.h"
+#include "libvirt-gconfig/libvirt-gconfig-private.h"
+
+#define GVIR_CONFIG_DOMAIN_GRAPHICS_LISTEN_ADDRESS_GET_PRIVATE(obj)
 \
+(G_TYPE_INSTANCE_GET_PRIVATE((obj), 
GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_LISTEN_ADDRESS, 
GVirConfigDomainGraphicsListenAddressPrivate))
+
+struct _GVirConfigDomainGraphicsListenAddressPrivate
+{
+gboolean unused;
+};
+
+G_DEFINE_TYPE(GVirConfigDomainGraphicsListenAddress, 
gvir_config_domain_graphics_listen_address, 
GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_LISTEN);
+
+
+static void 
gvir_config_domain_graphics_listen_address_class_init(GVirConfigDomainGraphicsListenAddressClass
 *klass)
+{
+g_type_class_add_private(klass, 
sizeof(GVirConfigDomainGraphicsListenAddressPrivate));
+}
+
+
+static void 
gvir_config_domain_graphics_listen_address_init(GVirConfigDomainGraphicsListenAddress
 *address)
+{
+address->priv = 
GVIR_CONFIG_DOMAIN_GRAPHICS_LISTEN_ADDRESS_GET_PRIVATE(address);
+}
+
+
+GVirConfigDomainGraphicsListenAddress 
*gvir_config_domain_graphics_listen_address_new(void)
+{
+GVirConfigObject *object;
+
+object = 
gvir_config_object_new(GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_LISTEN_ADDRESS, 
"listen", NULL);
+gvir_config_object_set_attribute(object,
+ "type", "address",
+ NULL);
+
+return GVIR_CONFIG_DOMAIN_GRAPHICS_LISTEN_ADDRESS(object);
+}
+
+
+GVirConfigDomainGraphicsListenAddress 
*gvir_config_domain_graphics_listen_address_new_from_xml(const gchar *xml,
+   
GError **error)
+{
+GVirConfigObject *object;
+
+object = 
gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_DOMAIN_GRAPHICS_LISTEN_ADDRESS,
+