Hi Christophe,
I have another patch for you to consider. This one removes some duplicated code by creating and using a utility function to ovirt-utils.c. I also switched out the two G_MAXUINT's in OvirtVmPool for G_MAXUINT32's.
We can make use of it any time an integer attribute needs to be parsed. Maybe it should be in the OvirtResource class, but I'll leave that for you to decide. Also, feel free to change naming or anything else you don't like.I have another patch for you to consider. This one removes some duplicated code by creating and using a utility function to ovirt-utils.c. I also switched out the two G_MAXUINT's in OvirtVmPool for G_MAXUINT32's.
iordan
On Thu, Dec 5, 2013 at 4:05 AM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
On Wed, Dec 04, 2013 at 11:12:09PM -0500, i iordanov wrote:Cool, thanks for the testing, I can push it upstream then!
> On Wed, Dec 4, 2013 at 10:30 AM, Christophe Fergeau <cfergeau@xxxxxxxxxx>wrote:
> > It would be nice if you could check I did not break anything for you (I
> > haven't tested ovirt_vm_pool_activate_vm() at all). Then I can push all of
> > it to git master ;)
> >
>
> I tested and it allocates VMs just as well as the before your changes. It
> also reports the parsed attributes correctly for me.
Christophe
--
The conscious mind has only one thread of execution.
From 0da8920996800ce271cbe3fff1f554a9c0367e0e Mon Sep 17 00:00:00 2001
From: Iordan Iordanov <iiordanov@xxxxxxxxx>
Date: Fri, 6 Dec 2013 13:26:07 -0500
Subject: [PATCH] Reduced code duplication in OvirtVmPool by adding a utility
function.
The functions to get size, prestarted_vms, and user_max_vms were all almost
the same code because each one was getting an integer from an XML node.
To deal with the code duplication, a utility function was added which
given a g_object and an XML node, sets a specified g_object property
from a specified XML sub-node. This patch removes the separate functions
and replaces them with calls to the new utility function, called
g_object_set_guint_property_from_xml().
---
govirt/ovirt-utils.c | 20 +++++++++++++++
govirt/ovirt-utils.h | 4 +++
govirt/ovirt-vm-pool.c | 63 +++++-------------------------------------------
3 files changed, 30 insertions(+), 57 deletions(-)
diff --git a/govirt/ovirt-utils.c b/govirt/ovirt-utils.c
index 3b9593a..8601cfe 100644
--- a/govirt/ovirt-utils.c
+++ b/govirt/ovirt-utils.c
@@ -212,3 +212,23 @@ G_GNUC_INTERNAL gboolean ovirt_utils_gerror_from_xml_fault(RestXmlNode *root, GE
return TRUE;
}
+
+
+G_GNUC_INTERNAL gboolean g_object_set_guint_property_from_xml(GObject *g_object,
+ RestXmlNode *node,
+ const gchar *node_name,
+ const gchar *prop_name)
+{
+ RestXmlNode *sub_node;
+ sub_node = rest_xml_node_find(node, node_name);
+ if (sub_node != NULL) {
+ guint value;
+ g_return_val_if_fail(sub_node->content != NULL, FALSE);
+ if (!ovirt_utils_guint_from_string(sub_node->content, &value)) {
+ return FALSE;
+ }
+ g_object_set(g_object, prop_name, value, NULL);
+ return TRUE;
+ }
+ return FALSE;
+}
diff --git a/govirt/ovirt-utils.h b/govirt/ovirt-utils.h
index f627c13..e92a2ec 100644
--- a/govirt/ovirt-utils.h
+++ b/govirt/ovirt-utils.h
@@ -32,6 +32,10 @@ G_BEGIN_DECLS
RestXmlNode *ovirt_rest_xml_node_from_call(RestProxyCall *call);
const char *ovirt_rest_xml_node_get_content(RestXmlNode *node, ...);
gboolean ovirt_utils_gerror_from_xml_fault(RestXmlNode *root, GError **error);
+gboolean g_object_set_guint_property_from_xml(GObject *g_object,
+ RestXmlNode *node,
+ const gchar *node_name,
+ const gchar *prop_name);
const char *ovirt_utils_genum_get_nick (GType enum_type, gint value);
int ovirt_utils_genum_get_value (GType enum_type, const char *nick,
diff --git a/govirt/ovirt-vm-pool.c b/govirt/ovirt-vm-pool.c
index ebbf3ae..b2000dc 100644
--- a/govirt/ovirt-vm-pool.c
+++ b/govirt/ovirt-vm-pool.c
@@ -138,7 +138,7 @@ static void ovirt_vm_pool_class_init(OvirtVmPoolClass *klass)
"Size of pool",
"The number of VMs in the pool",
0,
- G_MAXUINT,
+ G_MAXUINT32,
0,
G_PARAM_READWRITE));
g_object_class_install_property(object_class,
@@ -147,7 +147,7 @@ static void ovirt_vm_pool_class_init(OvirtVmPoolClass *klass)
"Prestarted VMs",
"The number of VMs prestarted in the pool",
0,
- G_MAXUINT,
+ G_MAXUINT32,
0,
G_PARAM_READWRITE));
g_object_class_install_property(object_class,
@@ -156,7 +156,7 @@ static void ovirt_vm_pool_class_init(OvirtVmPoolClass *klass)
"Max VMs per user",
"The number of VMs a user can allocate from the pool",
0,
- G_MAXUINT,
+ G_MAXUINT32,
0,
G_PARAM_READWRITE));
}
@@ -196,61 +196,10 @@ gboolean ovirt_vm_pool_allocate_vm_finish(OvirtVmPool *vm_pool,
}
-static gboolean vm_pool_set_size_from_xml(OvirtVmPool *vm_pool, RestXmlNode *node)
-{
- RestXmlNode *size_node;
- size_node = rest_xml_node_find(node, "size");
- if (size_node != NULL) {
- guint size;
- g_return_val_if_fail(size_node->content != NULL, FALSE);
- if (!ovirt_utils_guint_from_string(size_node->content, &size)) {
- return FALSE;
- }
- g_object_set(G_OBJECT(vm_pool), "size", size, NULL);
- return TRUE;
- }
- return FALSE;
-}
-
-
-static gboolean vm_pool_set_prestarted_vms_from_xml(OvirtVmPool *vm_pool, RestXmlNode *node)
-{
- RestXmlNode *prestarted_vms_node;
- prestarted_vms_node = rest_xml_node_find(node, "prestarted_vms");
- if (prestarted_vms_node != NULL) {
- guint prestarted_vms;
- g_return_val_if_fail(prestarted_vms_node->content != NULL, FALSE);
- if (!ovirt_utils_guint_from_string(prestarted_vms_node->content, &prestarted_vms)) {
- return FALSE;
- }
- g_object_set(G_OBJECT(vm_pool), "prestarted_vms", prestarted_vms, NULL);
- return TRUE;
- }
- return FALSE;
-}
-
-
-static gboolean vm_pool_set_max_user_vms_from_xml(OvirtVmPool *vm_pool, RestXmlNode *node)
-{
- RestXmlNode *max_user_vms_node;
- max_user_vms_node = rest_xml_node_find(node, "max_user_vms");
- if (max_user_vms_node != NULL) {
- guint max_user_vms;
- g_return_val_if_fail(max_user_vms_node->content != NULL, FALSE);
- if (!ovirt_utils_guint_from_string(max_user_vms_node->content, &max_user_vms)) {
- return FALSE;
- }
- g_object_set(G_OBJECT(vm_pool), "max_user_vms", max_user_vms, NULL);
- return TRUE;
- }
- return FALSE;
-}
-
-
static gboolean ovirt_vm_pool_refresh_from_xml(OvirtVmPool *vm_pool, RestXmlNode *node)
{
- vm_pool_set_size_from_xml(vm_pool, node);
- vm_pool_set_prestarted_vms_from_xml(vm_pool, node);
- vm_pool_set_max_user_vms_from_xml(vm_pool, node);
+ g_object_set_guint_property_from_xml(G_OBJECT(vm_pool), node, "size", "size");
+ g_object_set_guint_property_from_xml(G_OBJECT(vm_pool), node, "prestarted_vms", "prestarted_vms");
+ g_object_set_guint_property_from_xml(G_OBJECT(vm_pool), node, "max_user_vms", "max_user_vms");
return TRUE;
}
--
1.7.9.5
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel