[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [libvirt-glib 1/2] API to get node information about the connection



On Sun, Feb 19, 2012 at 2:50 PM, Christophe Fergeau <cfergeau redhat com> wrote:
> On Sat, Feb 18, 2012 at 07:24:03PM +0200, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" <zeeshanak gnome org>
>>
>> Mostly just a wrapper around virNodeGetInfo() and virNodeInfo struct.
>
> Apart from the 2 small comments below, I'm wondering if we should introduce
> a GVirNode class to wrap the virNode* methods. There are currently 5 to 10
> such methods (forgot the exact numbers), if more are to come, this would
> "bloat" the GVirConnection class. Any thoughts on that?

While I agree with you about the "bloat" part, I think the app
developer experience is more important. Since I don't know why libvirt
has put these functions under a different prefix (you couldn't figure
either IIRC?), I wonder if any app developer will be able to figure
why he is asked to create/get a different object here (especially
after he looks at underlying libvirt API). In this particular case,
he'll be asked to make 3 calls just to get the amount of RAM on the
host.

Looking at the following commented-out declarations in
libvirt-gobject/libvirt-gobject-connection.h:

GList *gvir_connection_get_node_devices(GVirConnection *conn);
GVirNodeDevice *gvir_connection_get_node_device(GVirConnection *conn,
                                                const gchar *name);


I'm guessing Daniel intended to handle it the way my patch is doing.

>> +    ret = g_slice_new(GVirNodeInfo);
>> +    g_memmove (ret->model, info.model, sizeof (ret->model));
>
> Why not g_strncpy here?

Cause I always forget which one is most appropriate. :) Thanks for pointing out.

>>  #endif /* __LIBVIRT_GOBJECT_CONNECTION_H__ */
>> diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym
>> index 7a2f65d..9e63773 100644
>> --- a/libvirt-gobject/libvirt-gobject.sym
>> +++ b/libvirt-gobject/libvirt-gobject.sym
>> @@ -4,6 +4,7 @@ LIBVIRT_GOBJECT_0.0.4 {
>>          gvir_init_object_check;
>>
>>       gvir_connection_get_type;
>> +     gvir_node_info_get_type;
>>       gvir_connection_new;
>>       gvir_connection_open;
>>       gvir_connection_open_async;
>
> gvir_connection_get_node_info is missing here.

Yup.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]