On Sun, Feb 19, 2012 at 2:50 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> On Sat, Feb 18, 2012 at 07:24:03PM +0200, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" <zeeshanak@xxxxxxxxx>
>>
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list