The code by itself is fine, but I'm not sure about the design
If we go this way, we'll have GVirConfigDomainConsolePty,
GVirConfigDomainConsoleStdio, GVirConfigDomainConsoleFile,
and then GVirConfigDomainSerialPty, GVirConfigDomainSerialStdio,
GVirConfigDomainSerialFile, and the same for channels and parallel ports.
Something that might work is having GVirConfigDomainChardev as a base class
for GVirConfigDomainConsole, GVirConfigDomainSerial, ...
Then we'd also have a GVirConfigDomainChardevSource from which
GVirConfigDomainChardevSourcePty, GVirConfigDomainChardevSourceStdio,
GVirConfigDomainChardevSourceFile, ... would derive.
And finally we'd have some kind of gvir_config_domain_chardev_add_source
API to combine the 2 (GVirConfigDomainConsole +
GVirConfigDomainChardevSourcePty).
There's the added complication that sometimes you have multiple source XML
nodes, sometimes you have a protocol node, so maybe
GVirConfigDomainChardevSourcePty would not even derive from
GVirConfigObject to allow handling that, or it would have a fake <chardev>
toplevel node.
To be honest I'm not really sure in which direction we should go, from
reading the doc, I've got the feeling that GVirConfigDomainConsolePty is
too simplistic and that we need something more sophisticated to avoid too
much duplication of code, but maybe I'm wrong. I had patches to handle
chardevs sent to the mailing list at some point, but withdrew them since I
wanted to spend more time thinking about this.
Christophe
On Thu, Dec 01, 2011 at 02:05:30PM +0000, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Add an abstract GVirConfigDomainConsole object for <console>
> devices and an impl GVirConfigDomainConsolePty for the
> <console type=pty> variant
> ---
> libvirt-gconfig/Makefile.am | 4 +
> .../libvirt-gconfig-domain-console-pty.c | 74 ++++++++++++++++++++
> .../libvirt-gconfig-domain-console-pty.h | 67 ++++++++++++++++++
> libvirt-gconfig/libvirt-gconfig-domain-console.c | 49 +++++++++++++
> libvirt-gconfig/libvirt-gconfig-domain-console.h | 64 +++++++++++++++++
> libvirt-gconfig/libvirt-gconfig.h | 2 +
> libvirt-gconfig/libvirt-gconfig.sym | 8 ++
> 7 files changed, 268 insertions(+), 0 deletions(-)
> create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-console-pty.c
> create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-console-pty.h
> create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-console.c
> create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-console.h
>
> diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
> index 6612445..43f719d 100644
> --- a/libvirt-gconfig/Makefile.am
> +++ b/libvirt-gconfig/Makefile.am
> @@ -13,6 +13,8 @@ GCONFIG_HEADER_FILES = \
> libvirt-gconfig-object.h \
> libvirt-gconfig-capabilities.h \
> libvirt-gconfig-domain.h \
> + libvirt-gconfig-domain-console.h \
> + libvirt-gconfig-domain-console-pty.h \
> libvirt-gconfig-domain-clock.h \
> libvirt-gconfig-domain-device.h \
> libvirt-gconfig-domain-disk.h \
> @@ -46,6 +48,8 @@ GCONFIG_SOURCE_FILES = \
> libvirt-gconfig-main.c \
> libvirt-gconfig-capabilities.c \
> libvirt-gconfig-domain.c \
> + libvirt-gconfig-domain-console.c \
> + libvirt-gconfig-domain-console-pty.c \
> libvirt-gconfig-domain-clock.c \
> libvirt-gconfig-domain-device.c \
> libvirt-gconfig-domain-disk.c \
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-console-pty.c b/libvirt-gconfig/libvirt-gconfig-domain-console-pty.c
> new file mode 100644
> index 0000000..f132d6c
> --- /dev/null
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-console-pty.c
> @@ -0,0 +1,74 @@
> +/*
> + * libvirt-gconfig-domain-console-pty.c: libvirt domain console configuration
> + *
> + * Copyright (C) 2011 Red Hat
> + *
> + * 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: Daniel P. Berrange <berrange@xxxxxxxxxx>
> + */
> +
> +#include <config.h>
> +
> +#include "libvirt-gconfig/libvirt-gconfig.h"
> +#include "libvirt-gconfig/libvirt-gconfig-helpers-private.h"
> +#include "libvirt-gconfig/libvirt-gconfig-object-private.h"
> +
> +#define GVIR_CONFIG_DOMAIN_CONSOLE_PTY_GET_PRIVATE(obj) \
> + (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE_PTY, GVirConfigDomainConsolePtyPrivate))
> +
> +struct _GVirConfigDomainConsolePtyPrivate
> +{
> + gboolean unused;
> +};
> +
> +G_DEFINE_TYPE(GVirConfigDomainConsolePty, gvir_config_domain_console_pty, GVIR_TYPE_CONFIG_DOMAIN_CONSOLE);
> +
> +
> +static void gvir_config_domain_console_pty_class_init(GVirConfigDomainConsolePtyClass *klass)
> +{
> + g_type_class_add_private(klass, sizeof(GVirConfigDomainConsolePtyPrivate));
> +}
> +
> +
> +static void gvir_config_domain_console_pty_init(GVirConfigDomainConsolePty *conn)
> +{
> + g_debug("Init GVirConfigDomainConsolePty=%p", conn);
> +
> + conn->priv = GVIR_CONFIG_DOMAIN_CONSOLE_PTY_GET_PRIVATE(conn);
> +}
> +
> +
> +GVirConfigDomainConsolePty *gvir_config_domain_console_pty_new(void)
> +{
> + GVirConfigObject *object;
> +
> + object = gvir_config_object_new(GVIR_TYPE_CONFIG_DOMAIN_CONSOLE_PTY,
> + "console", NULL);
> + gvir_config_object_set_attribute(object, "type", "pty", NULL);
> + return GVIR_CONFIG_DOMAIN_CONSOLE_PTY(object);
> +}
> +
> +GVirConfigDomainConsolePty *gvir_config_domain_console_pty_new_from_xml(const gchar *xml,
> + GError **error)
> +{
> + GVirConfigObject *object;
> +
> + object = gvir_config_object_new_from_xml(GVIR_TYPE_CONFIG_DOMAIN_CONSOLE_PTY,
> + "console", NULL, xml, error);
> + if (object == NULL)
> + return NULL;
> + return GVIR_CONFIG_DOMAIN_CONSOLE_PTY(object);
> +}
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-console-pty.h b/libvirt-gconfig/libvirt-gconfig-domain-console-pty.h
> new file mode 100644
> index 0000000..56640ab
> --- /dev/null
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-console-pty.h
> @@ -0,0 +1,67 @@
> +/*
> + * libvirt-gconfig-domain-console-pty.c: libvirt domain console configuration
> + *
> + * Copyright (C) 2011 Red Hat
> + *
> + * 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: Daniel P. Berrange <berrange@xxxxxxxxxx>
> + */
> +
> +#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_CONSOLE_PTY_H__
> +#define __LIBVIRT_GCONFIG_DOMAIN_CONSOLE_PTY_H__
> +
> +G_BEGIN_DECLS
> +
> +#define GVIR_TYPE_CONFIG_DOMAIN_CONSOLE_PTY (gvir_config_domain_console_pty_get_type ())
> +#define GVIR_CONFIG_DOMAIN_CONSOLE_PTY(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE, GVirConfigDomainConsolePty))
> +#define GVIR_CONFIG_DOMAIN_CONSOLE_PTY_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE, GVirConfigDomainConsolePtyClass))
> +#define GVIR_IS_CONFIG_DOMAIN_CONSOLE_PTY(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE))
> +#define GVIR_IS_CONFIG_DOMAIN_CONSOLE_PTY_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE))
> +#define GVIR_CONFIG_DOMAIN_CONSOLE_PTY_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE, GVirConfigDomainConsolePtyClass))
> +
> +typedef struct _GVirConfigDomainConsolePty GVirConfigDomainConsolePty;
> +typedef struct _GVirConfigDomainConsolePtyPrivate GVirConfigDomainConsolePtyPrivate;
> +typedef struct _GVirConfigDomainConsolePtyClass GVirConfigDomainConsolePtyClass;
> +
> +struct _GVirConfigDomainConsolePty
> +{
> + GVirConfigDomainConsole parent;
> +
> + GVirConfigDomainConsolePtyPrivate *priv;
> +
> + /* Do not add fields to this struct */
> +};
> +
> +struct _GVirConfigDomainConsolePtyClass
> +{
> + GVirConfigDomainConsoleClass parent_class;
> +
> + gpointer padding[20];
> +};
> +
> +GType gvir_config_domain_console_pty_get_type(void);
> +
> +GVirConfigDomainConsolePty *gvir_config_domain_console_pty_new(void);
> +GVirConfigDomainConsolePty *gvir_config_domain_console_pty_new_from_xml(const gchar *xml,
> + GError **error);
> +
> +G_END_DECLS
> +
> +#endif /* __LIBVIRT_GCONFIG_DOMAIN_CONSOLE_PTY_H__ */
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-console.c b/libvirt-gconfig/libvirt-gconfig-domain-console.c
> new file mode 100644
> index 0000000..225a587
> --- /dev/null
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-console.c
> @@ -0,0 +1,49 @@
> +/*
> + * libvirt-gconfig-domain-console.c: libvirt domain console configuration
> + *
> + * Copyright (C) 2011 Red Hat
> + *
> + * 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: Daniel P. Berrange <berrange@xxxxxxxxxx>
> + */
> +
> +#include <config.h>
> +
> +#include "libvirt-gconfig/libvirt-gconfig.h"
> +
> +#define GVIR_CONFIG_DOMAIN_CONSOLE_GET_PRIVATE(obj) \
> + (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE, GVirConfigDomainConsolePrivate))
> +
> +struct _GVirConfigDomainConsolePrivate
> +{
> + gboolean unused;
> +};
> +
> +G_DEFINE_ABSTRACT_TYPE(GVirConfigDomainConsole, gvir_config_domain_console, GVIR_TYPE_CONFIG_DOMAIN_DEVICE);
> +
> +
> +static void gvir_config_domain_console_class_init(GVirConfigDomainConsoleClass *klass)
> +{
> + g_type_class_add_private(klass, sizeof(GVirConfigDomainConsolePrivate));
> +}
> +
> +
> +static void gvir_config_domain_console_init(GVirConfigDomainConsole *console)
> +{
> + g_debug("Init GVirConfigDomainConsole=%p", console);
> +
> + console->priv = GVIR_CONFIG_DOMAIN_CONSOLE_GET_PRIVATE(console);
> +}
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-console.h b/libvirt-gconfig/libvirt-gconfig-domain-console.h
> new file mode 100644
> index 0000000..9221e6d
> --- /dev/null
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-console.h
> @@ -0,0 +1,64 @@
> +/*
> + * libvirt-gconfig-domain-console.h: libvirt domain console configuration
> + *
> + * Copyright (C) 2011 Red Hat
> + *
> + * 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: Daniel P. Berrange <berrange@xxxxxxxxxx>
> + */
> +
> +#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_CONSOLE_H__
> +#define __LIBVIRT_GCONFIG_DOMAIN_CONSOLE_H__
> +
> +G_BEGIN_DECLS
> +
> +#define GVIR_TYPE_CONFIG_DOMAIN_CONSOLE (gvir_config_domain_console_get_type ())
> +#define GVIR_CONFIG_DOMAIN_CONSOLE(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE, GVirConfigDomainConsole))
> +#define GVIR_CONFIG_DOMAIN_CONSOLE_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE, GVirConfigDomainConsoleClass))
> +#define GVIR_IS_CONFIG_DOMAIN_CONSOLE(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE))
> +#define GVIR_IS_CONFIG_DOMAIN_CONSOLE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE))
> +#define GVIR_CONFIG_DOMAIN_CONSOLE_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GVIR_TYPE_CONFIG_DOMAIN_CONSOLE, GVirConfigDomainConsoleClass))
> +
> +typedef struct _GVirConfigDomainConsole GVirConfigDomainConsole;
> +typedef struct _GVirConfigDomainConsolePrivate GVirConfigDomainConsolePrivate;
> +typedef struct _GVirConfigDomainConsoleClass GVirConfigDomainConsoleClass;
> +
> +struct _GVirConfigDomainConsole
> +{
> + GVirConfigDomainDevice parent;
> +
> + GVirConfigDomainConsolePrivate *priv;
> +
> + /* Do not add fields to this struct */
> +};
> +
> +struct _GVirConfigDomainConsoleClass
> +{
> + GVirConfigDomainDeviceClass parent_class;
> +
> + gpointer padding[20];
> +};
> +
> +
> +GType gvir_config_domain_console_get_type(void);
> +
> +G_END_DECLS
> +
> +#endif /* __LIBVIRT_GCONFIG_DOMAIN_CONSOLE_H__ */
> diff --git a/libvirt-gconfig/libvirt-gconfig.h b/libvirt-gconfig/libvirt-gconfig.h
> index be4b4dd..0afe75a 100644
> --- a/libvirt-gconfig/libvirt-gconfig.h
> +++ b/libvirt-gconfig/libvirt-gconfig.h
> @@ -31,6 +31,8 @@
> #include <libvirt-gconfig/libvirt-gconfig-capabilities.h>
> #include <libvirt-gconfig/libvirt-gconfig-domain.h>
> #include <libvirt-gconfig/libvirt-gconfig-domain-clock.h>
> +#include <libvirt-gconfig/libvirt-gconfig-domain-console.h>
> +#include <libvirt-gconfig/libvirt-gconfig-domain-console-pty.h>
> #include <libvirt-gconfig/libvirt-gconfig-domain-device.h>
> #include <libvirt-gconfig/libvirt-gconfig-domain-disk.h>
> #include <libvirt-gconfig/libvirt-gconfig-domain-filesys.h>
> diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym
> index 6f3593d..ce29414 100644
> --- a/libvirt-gconfig/libvirt-gconfig.sym
> +++ b/libvirt-gconfig/libvirt-gconfig.sym
> @@ -34,6 +34,14 @@ LIBVIRT_GCONFIG_0.0.1 {
> gvir_config_domain_clock_set_timezone;
> gvir_config_domain_clock_set_variable_offset;
>
> + gvir_config_domain_console_get_type;
> + gvir_config_domain_console_new;
> + gvir_config_domain_console_new_from_xml;
> +
> + gvir_config_domain_console_pty_get_type;
> + gvir_config_domain_console_pty_new;
> + gvir_config_domain_console_pty_new_from_xml;
> +
> gvir_config_domain_device_get_type;
>
> gvir_config_domain_disk_get_type;
> --
> 1.7.6.4
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
pgphcFiO4NKou.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list