Hi Dan,
All looks good in general - just want to call out the couple of devkit
leaks again. No problem leaving them 'til later to fix, of course.
Cheers,
Mark.
On Thu, 2008-11-20 at 17:55 +0000, Daniel P. Berrange wrote:
+
+static void dev_create(void *_dkdev, void *_dkclient ATTRIBUTE_UNUSED)
+{
+ DevkitDevice *dkdev = _dkdev;
+ const char *sysfs_path = devkit_device_get_native_path(dkdev);
+ virNodeDeviceObjPtr dev = NULL;
+ const char *name;
+ int rv;
+
+ if (sysfs_path == NULL)
+ /* Currently using basename(sysfs_path) as device name (key) */
+ return;
+
+ name = strrchr(sysfs_path, '/');
+ if (name == NULL)
+ name = sysfs_path;
+ else
+ ++name;
+
+ if (VIR_ALLOC(dev) < 0 || VIR_ALLOC(dev->def) < 0)
+ goto failure;
+
+ dev->privateData = dkdev;
Still leaking the GObject ref here, I think
+static int devkitDeviceMonitorStartup(void)
+{
+ size_t caps_tbl_len = sizeof(caps_tbl) / sizeof(caps_tbl[0]);
+ DevkitClient *devkit_client = NULL;
+ GError *err = NULL;
+ GList *devs;
+ int i;
+
+ /* Ensure caps_tbl is sorted by capability name */
+ qsort(caps_tbl, caps_tbl_len, sizeof(caps_tbl[0]), cmpstringp);
+
+ if (VIR_ALLOC(driverState) < 0)
+ return -1;
+
+ // TODO: Is it really ok to call this multiple times??
+ // Is there something analogous to call on close?
+ g_type_init();
+
+ /* Get new devkit_client and connect to daemon */
+ devkit_client = devkit_client_new(NULL);
+ if (devkit_client == NULL) {
+ DEBUG0("devkit_client_new returned NULL");
+ goto failure;
+ }
+ if (!devkit_client_connect(devkit_client, &err)) {
+ DEBUG0("devkit_client_connect failed");
+ goto failure;
+ }
+
+ /* Populate with known devices.
+ *
+ * This really should be:
+ devs = devkit_client_enumerate_by_subsystem(devkit_client, NULL, &err);
+ if (err) {
+ DEBUG0("devkit_client_enumerate_by_subsystem failed");
+ devs = NULL;
+ goto failure;
+ }
+ g_list_foreach(devs, dev_create, devkit_client);
+ * but devkit_client_enumerate_by_subsystem currently fails when the second
+ * arg is null (contrary to the API documentation). So the following code
+ * (from Dan B) works around this by listing devices per handled subsystem.
+ */
+
+ for (i = 0 ; i < ARRAY_CARDINALITY(caps_tbl) ; i++) {
+ const char *caps[] = { caps_tbl[i].cap_name, NULL };
+ devs = devkit_client_enumerate_by_subsystem(devkit_client,
+ caps,
+ &err);
+ if (err) {
+ DEBUG0("devkit_client_enumerate_by_subsystem failed");
+ devs = NULL;
+ goto failure;
+ }
+ g_list_foreach(devs, dev_create, devkit_client);
g_list_free()