Re: [libvirt] [PATCH] Implement vol delete for disk pools

[Cole Robinson <crobinso@xxxxxxxxxx>]

Daniel Veillard wrote:
> On Fri, Sep 05, 2008 at 11:17:27PM -0400, Cole Robinson wrote:
>>  static int
>>  virStorageBackendDiskDeleteVol(virConnectPtr conn,
>> -                               virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
>> -                               virStorageVolDefPtr vol ATTRIBUTE_UNUSED,
>> +                               virStoragePoolObjPtr pool,
>> +                               virStorageVolDefPtr vol,
>>                                 unsigned int flags ATTRIBUTE_UNUSED)
>>  {
>> -    /* delete a partition */
>> -    virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
>> -                          _("Disk pools are not yet supported"));
>> -    return -1;
>> +    char *part_num = NULL;
>> +    int n;
>> +    char devpath[PATH_MAX];
>> +    char *devname, *srcname;
>> +
>> +    if ((n = readlink(vol->target.path, devpath, sizeof(devpath))) < 0 &&
>> +        errno != EINVAL) {
>> +        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
>> +                              _("Couldn't read volume target path '%s'. %s"),
>> +                              vol->target.path, strerror(errno));
>> +        return -1;
>> +    } else if (n <= 0) {
>> +        strncpy(devpath, vol->target.path, PATH_MAX);
>> +    } else {
>> +        devpath[n] = '\0';
>> +    }
>> +
>> +    devname = basename(devpath);
>> +    srcname = basename(pool->def->source.devices[0].path);
> 
>   This seems to leak the two strings and not check for errors. That
> would need to be fixed before being commited IMHO
> 

glibc defines basename in string.h as:

char *
basename (filename)
     const char *filename;
{
  char *p = strrchr (filename, '/');
  return p ? p + 1 : (char *) filename;
}

So there doesn't seem to be any cleanup or failure catching
necessary.

Thanks,
Cole

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list