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()