On Wed, Feb 29, 2012 at 3:30 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Currently we only support existing DomainDevice implementations: DomainDisk and DomainInterface. --- .../libvirt-gobject-domain-device-private.h | 2 + libvirt-gobject/libvirt-gobject-domain-device.c | 21 ++++++++++ libvirt-gobject/libvirt-gobject-domain.c | 42 ++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 3 + libvirt-gobject/libvirt-gobject.sym | 1 + 5 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h b/libvirt-gobject/libvirt-gobject-domain-device-private.h index 72c660e..292a2ac 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device-private.h +++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h @@ -24,6 +24,8 @@
G_BEGIN_DECLS
+G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain); virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self);
G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c b/libvirt-gobject/libvirt-gobject-domain-device.c index 85879d2..0ec5dad 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.c +++ b/libvirt-gobject/libvirt-gobject-domain-device.c @@ -176,3 +176,24 @@ GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) { GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice *device) { return g_object_ref (device->priv->config); } + +G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain)
gvir_domain_new_device(GVirDomain *domain, GVirConfigDomainDevice *config) maybe ? Having the argument that is always non-NULL first feels better to me.
Who said config is nullable? :)
+{ + GType type; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), NULL); + + if (GVIR_CONFIG_IS_DOMAIN_DISK(config)) + type = GVIR_TYPE_DOMAIN_DISK; + else if (GVIR_CONFIG_IS_DOMAIN_INTERFACE(config)) + type = GVIR_TYPE_DOMAIN_INTERFACE; + else { + g_debug("Unknown device type: %s", G_OBJECT_TYPE_NAME(config)); + return NULL; + }
I'd have a slight preference for something like
switch (G_OBJECT_TYPE(config)) { case GVIR_CONFIG_TYPE_DOMAIN_DISK: type = GVIR_TYPE_DOMAIN_DISK; break; ...
but I'm fine with either version.
I would have prefered it too but if any of these classes are subclassed tomorrow and we forget to update this code, we'll start loosing config instances we can actually handle.
+ + device_config = GVIR_CONFIG_DOMAIN_DEVICE(node->data); + device = gvir_domain_device_new(device_config, domain); + if (device != NULL) + ret = g_list_append(ret, device);
slight preference for g_list_prepend (much more efficient when the list is big).
Good point but wouldn't you expect devices to be sorted in the order they are sorted in the XML?
+ + g_object_unref (device_config); + } + g_list_free (config_devices);
The individual elements need to be unref'ed too as well as config.
Thats what g_object_unref is doing just above? I think I should name this 'device_configs' to be consistent with 'device_config' and therefore clear..
diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index d6999dc..b7b95cb 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -72,6 +72,7 @@ LIBVIRT_GOBJECT_0.0.4 { gvir_domain_get_persistent; gvir_domain_get_saved; gvir_domain_screenshot; + gvir_domain_get_devices;
Try to get the list sorted/consistent with the other entries.
K. -- Regards, Zeeshan Ali (Khattak) FSF member#5124