On Wed, Feb 29, 2012 at 4:24 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Feb 29, 2012 at 04:01:40PM +0200, Zeeshan Ali (Khattak) wrote:
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? :)
I assumed this because of the lack of g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), ...)
Ah, should fix that.
It still kind of sounds better to me to say "let's create a device associated with this GVirDomain", ie to have this method operating on a GVirDomain.
Hmm.. OK, i'll change the order.
+ + 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?
You can always do a g_list_reverse after the fact, and I'd rather we avoid to make such guarantees anyway. If gvir_config_domain_get_devices used g_list_prepend (which I just noticed it's not doing), you'd need to prepend to return things in the order of the XML :) Not that important here, it's just a better practice to use g_list_prepend.
OK. Will change this too then.. -- Regards, Zeeshan Ali (Khattak) FSF member#5124