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()
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list