[libvirt] PATCH: 3/7: Main HAL & DeviceKit impl

[Mark McLoughlin]

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