Re: [libvirt] [libvirt-glib 2/3] Add host capabilities API

On Tue, May 01, 2012 at 08:30:39PM +0300, Zeeshan Ali (Khattak) wrote: I'd keep this one ordered alphabetically This is not really useful as -private files will never be included from libvirt-gconfig.h, just drop it (and we'll have to fix libvirt-gconfig-helpers-private.h as well). For what it's worth, for the device/... _new_from_tree methods I've been lazy and I've grouped them in the same -private file, but I'm fine with a separate file per class. I've spelt 3 letter acronyms "Xml" instead of XML when using camel case spelling, I'd rather use Cpu instead of cpu here too s/conn/feature (not your fault, this c&p error is present everywhere) This "if" is needed because we can have <features><foo/></features> VS <feature name="foo">? This deserves a comment explaining why we do this test. Having a per-feature GVirConfigObject seems overkill since it will only be a string wrapper, and a GVirConfigObject wrapping just a string with no node name identifying the type of the node is unusual. Not having this class, and doing the parsing directly from gvir_config_capabilities_cpu_get_features would allow to remove it. GVirConfigDomain can also have a <features> node, the strings mentioned above can be wrapped in a GVirConfigCpuFeatures object (or just a typedef?) instead of in a raw GList, and then we can factor the GVirConfigDomain and GVirConfigCapabilitiesCpu code somehow. If we keep this, Cpu 2012 Not needed Cpu s/conn/cpu I don't think many _new_from_tree functions are currently doing that, but I'd add a g_return_val_if_fail(!GVIR_CONFIG_IS_XML_DOC(doc), NULL); g_return_val_if_fail(g_strcmp0(tree->name, "cpu") == 0, NULL); This is gvir_config_object_get_node_content Is it expected that "features" nodes are ignored? Are the 2 kind of nodes (feature/features) two different things that we want to expose differently in the API? gvir_config_object_foreach_child > + > + g_clear_object(&data.doc); > + > + return data.features; > +} > + > diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h > new file mode 100644 > index 0000000..abad676 > --- /dev/null > +++ b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h > @@ -0,0 +1,69 @@ > +/* > + * libvirt-gconfig-capabilities-cpu.h: libvirt CPU capabilities > + * > + * Copyright (C) 2010-2012 Red Hat, Inc. > + > +#ifndef __LIBVIRT_GCONFIG_CAPABILITIES_CPU_H__ > +#define __LIBVIRT_GCONFIG_CAPABILITIES_CPU_H__ > + > +G_BEGIN_DECLS > + > +#define GVIR_CONFIG_TYPE_CAPABILITIES_CPU (gvir_config_capabilities_cpu_get_type ()) > +#define GVIR_CONFIG_CAPABILITIES_CPU(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_CONFIG_TYPE_CAPABILITIES_CPU, GVirConfigCapabilitiesCPU)) > +#define GVIR_CONFIG_CAPABILITIES_CPU_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GVIR_CONFIG_TYPE_CAPABILITIES_CPU, GVirConfigCapabilitiesCPUClass)) > +#define GVIR_CONFIG_IS_CAPABILITIES_CPU(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GVIR_CONFIG_TYPE_CAPABILITIES_CPU)) > +#define GVIR_CONFIG_IS_CAPABILITIES_CPU_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GVIR_CONFIG_TYPE_CAPABILITIES_CPU)) > +#define GVIR_CONFIG_CAPABILITIES_CPU_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GVIR_CONFIG_TYPE_CAPABILITIES_CPU, GVirConfigCapabilitiesCPUClass)) > + > +typedef struct _GVirConfigCapabilitiesCPU GVirConfigCapabilitiesCPU; Cpu > +typedef struct _GVirConfigCapabilitiesCPUPrivate GVirConfigCapabilitiesCPUPrivate; > +typedef struct _GVirConfigCapabilitiesCPUClass GVirConfigCapabilitiesCPUClass; > + > +struct _GVirConfigCapabilitiesCPU > +{ > + GVirConfigObject parent; > + > + GVirConfigCapabilitiesCPUPrivate *priv; > + > + /* Do not add fields to this struct */ > +}; > + > +struct _GVirConfigCapabilitiesCPUClass > +{ > + GVirConfigObjectClass parent_class; > + > + gpointer padding[20]; > +}; > + > +GType gvir_config_capabilities_cpu_get_type(void); > + > +const gchar * > +gvir_config_capabilities_cpu_get_arch(GVirConfigCapabilitiesCPU *caps); > +GList * > +gvir_config_capabilities_cpu_get_features(GVirConfigCapabilitiesCPU *caps); > + > +G_END_DECLS > + > +#endif /* __LIBVIRT_GCONFIG_CAPABILITIES_CPU_H__ */ > diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities-host-private.h b/libvirt-gconfig/libvirt-gconfig-capabilities-host-private.h > new file mode 100644 > index 0000000..732b64c > --- /dev/null > +++ b/libvirt-gconfig/libvirt-gconfig-capabilities-host-private.h > @@ -0,0 +1,39 @@ > +/* > + * libvirt-gconfig-capabilities-host-private.h: libvirt host capabilities > + * > + * Copyright (C) 2010-2012 Red Hat, Inc. > + > +#ifndef __LIBVIRT_GCONFIG_CAPABILITIES_HOST_PRIVATE_H__ > +#define __LIBVIRT_GCONFIG_CAPABILITIES_HOST_PRIVATE_H__ > + > +G_BEGIN_DECLS > + > +GVirConfigCapabilitiesHost * > +gvir_config_capabilities_host_new_from_tree(GVirConfigXmlDoc *doc, > + xmlNodePtr tree); > + > +G_END_DECLS > + > +#endif /* __LIBVIRT_GCONFIG_CAPABILITIES_HOST_PRIVATE_H__ */ > diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities-host.c b/libvirt-gconfig/libvirt-gconfig-capabilities-host.c > new file mode 100644 > index 0000000..becaf6d > --- /dev/null > +++ b/libvirt-gconfig/libvirt-gconfig-capabilities-host.c > @@ -0,0 +1,98 @@ > +/* > + * libvirt-gconfig-capabilities-host.c: libvirt host capabilities > + * > + * Copyright (C) 2008 Daniel P. Berrange > + * Copyright (C) 2010-2012 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 > + * > + * Authors: Zeeshan Ali <zeenix(a)redhat.com&gt; > + * Daniel P. Berrange <berrange(a)redhat.com&gt; > + */ > + > +#include <config.h> > + > +#include "libvirt-gconfig/libvirt-gconfig.h" > +#include "libvirt-gconfig/libvirt-gconfig-private.h" > + > +#define GVIR_CONFIG_CAPABILITIES_HOST_GET_PRIVATE(obj) \ > + (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_CONFIG_TYPE_CAPABILITIES_HOST, GVirConfigCapabilitiesHostPrivate)) > + > +struct _GVirConfigCapabilitiesHostPrivate > +{ > + gboolean unused; > +}; > + > +G_DEFINE_TYPE(GVirConfigCapabilitiesHost, gvir_config_capabilities_host, GVIR_CONFIG_TYPE_OBJECT); > + > + > +static void gvir_config_capabilities_host_class_init(GVirConfigCapabilitiesHostClass *klass) > +{ > + g_type_class_add_private(klass, sizeof(GVirConfigCapabilitiesHostPrivate)); > +} > + > +static void gvir_config_capabilities_host_init(GVirConfigCapabilitiesHost *conn) > +{ > + g_debug("Init GVirConfigCapabilitiesHost=%p", conn); > + > + conn->priv = GVIR_CONFIG_CAPABILITIES_HOST_GET_PRIVATE(conn); > +} > + > +G_GNUC_INTERNAL GVirConfigCapabilitiesHost * > +gvir_config_capabilities_host_new_from_tree(GVirConfigXmlDoc *doc, > + xmlNodePtr tree) > +{ > + GVirConfigObject *object; > + > + object = gvir_config_object_new_from_tree(GVIR_CONFIG_TYPE_CAPABILITIES_HOST, > + doc, > + NULL, > + tree); > + > + return GVIR_CONFIG_CAPABILITIES_HOST(object); > +} > + > +const gchar * > +gvir_config_capabilities_host_get_uuid(GVirConfigCapabilitiesHost *caps) > +{ > + return gvir_config_object_get_node_content(GVIR_CONFIG_OBJECT(caps), "uuid"); > +} > + > +/** > + * gvir_config_capabilities_host_get_cpu: > + * > + * Gets the CPU capabilities of the host. > + * > + * Returns: (transfer full): a new #GVirConfigCapabilitiesCPU. > + */ > +GVirConfigCapabilitiesCPU * > +gvir_config_capabilities_host_get_cpu(GVirConfigCapabilitiesHost *caps) > +{ > + GVirConfigXmlDoc *doc; > + xmlNodePtr node; > + > + g_return_val_if_fail(GVIR_CONFIG_IS_CAPABILITIES_HOST(caps), NULL); > + > + g_object_get(G_OBJECT(caps), "doc", &doc, NULL); > + > + node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(caps)); > + g_return_val_if_fail(node != NULL, NULL); > + > + node = gvir_config_xml_get_element(node, "cpu", NULL); > + g_return_val_if_fail(node != NULL, NULL); Is it a mandatory element in the capabilities XML? If not, a if (node == NULL) { return NULL; } is better > + > + return gvir_config_capabilities_cpu_new_from_tree(doc, node); > +} > + > diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities-host.h b/libvirt-gconfig/libvirt-gconfig-capabilities-host.h > new file mode 100644 > index 0000000..34ee19e > --- /dev/null > +++ b/libvirt-gconfig/libvirt-gconfig-capabilities-host.h > @@ -0,0 +1,70 @@ > +/* > + * libvirt-gconfig-capabilities-host.h: libvirt host capabilities > + * > + * Copyright (C) 2010-2012 Red Hat, Inc. > + > +#ifndef __LIBVIRT_GCONFIG_CAPABILITIES_HOST_H__ > +#define __LIBVIRT_GCONFIG_CAPABILITIES_HOST_H__ > + > +G_BEGIN_DECLS > + > +#define GVIR_CONFIG_TYPE_CAPABILITIES_HOST (gvir_config_capabilities_host_get_type ()) > +#define GVIR_CONFIG_CAPABILITIES_HOST(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_CONFIG_TYPE_CAPABILITIES_HOST, GVirConfigCapabilitiesHost)) > +#define GVIR_CONFIG_CAPABILITIES_HOST_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GVIR_CONFIG_TYPE_CAPABILITIES_HOST, GVirConfigCapabilitiesHostClass)) > +#define GVIR_CONFIG_IS_CAPABILITIES_HOST(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GVIR_CONFIG_TYPE_CAPABILITIES_HOST)) > +#define GVIR_CONFIG_IS_CAPABILITIES_HOST_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GVIR_CONFIG_TYPE_CAPABILITIES_HOST)) > +#define GVIR_CONFIG_CAPABILITIES_HOST_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GVIR_CONFIG_TYPE_CAPABILITIES_HOST, GVirConfigCapabilitiesHostClass)) > + > +typedef struct _GVirConfigCapabilitiesHost GVirConfigCapabilitiesHost; > +typedef struct _GVirConfigCapabilitiesHostPrivate GVirConfigCapabilitiesHostPrivate; > +typedef struct _GVirConfigCapabilitiesHostClass GVirConfigCapabilitiesHostClass; > + > +struct _GVirConfigCapabilitiesHost > +{ > + GVirConfigObject parent; > + > + GVirConfigCapabilitiesHostPrivate *priv; > + > + /* Do not add fields to this struct */ > +}; > + > +struct _GVirConfigCapabilitiesHostClass > +{ > + GVirConfigObjectClass parent_class; > + > + gpointer padding[20]; > +}; > + > +GType gvir_config_capabilities_host_get_type(void); > + > +const gchar * > +gvir_config_capabilities_host_get_uuid(GVirConfigCapabilitiesHost *caps); > + > +GVirConfigCapabilitiesCPU * > +gvir_config_capabilities_host_get_cpu(GVirConfigCapabilitiesHost *caps); > + > +G_END_DECLS > + > +#endif /* __LIBVIRT_GCONFIG_CAPABILITIES_HOST_H__ */ > diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities.c b/libvirt-gconfig/libvirt-gconfig-capabilities.c > index 3d9d036..9518a30 100644 > --- a/libvirt-gconfig/libvirt-gconfig-capabilities.c > +++ b/libvirt-gconfig/libvirt-gconfig-capabilities.c > @@ -24,6 +24,7 @@ > #include <config.h> > > #include "libvirt-gconfig/libvirt-gconfig.h" > +#include "libvirt-gconfig/libvirt-gconfig-private.h" > > #define GVIR_CONFIG_CAPABILITIES_GET_PRIVATE(obj) \ > (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_CONFIG_TYPE_CAPABILITIES, GVirConfigCapabilitiesPrivate)) > @@ -71,3 +72,29 @@ GVirConfigCapabilities *gvir_config_capabilities_new_from_xml(const gchar *xml, > xml, error); > return GVIR_CONFIG_CAPABILITIES(object); > } > + > +/** > + * gvir_config_capabilities_get_host: > + * > + * Gets the host capabilities. > + * > + * Returns: (transfer full): a new #GVirConfigCapabilitiesHost. > + */ > +GVirConfigCapabilitiesHost * > +gvir_config_capabilities_get_host(GVirConfigCapabilities *caps) > +{ > + GVirConfigXmlDoc *doc; > + xmlNodePtr node; > + > + g_return_val_if_fail(GVIR_CONFIG_IS_CAPABILITIES(caps), NULL); > + > + g_object_get(G_OBJECT(caps), "doc", &doc, NULL); > + > + node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(caps)); > + g_return_val_if_fail(node != NULL, NULL); > + > + node = gvir_config_xml_get_element(node, "host", NULL); > + g_return_val_if_fail(node != NULL, NULL); > + > + return gvir_config_capabilities_host_new_from_tree(doc, node); > +} This is the same code as gvir_config_capabilities_host_get_cpu, if this pattern gets repeated often, it will be worth trying to factor it in a helper function. -capabilities.h comes before -capabilities-cpu-feature.h This should not be exported. Christophe