[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 2/9] storage: gluster: Introduce dummy functions for creating a volume



On 01/27/14 18:49, Eric Blake wrote:
> On 01/16/2014 06:14 AM, Peter Krempa wrote:
>> The temporary pool code will need to initialize some fields for the
>> temporary gluster volumes. This is done by the createVol function of
>> the storage backend. This patch implements only the metadata setting.
>> Attempts to create a regular volume yield an error.
> 
> It will be nice to allow creation of volumes, but I agree that it can
> come later.
> 
>>
>> +
>> +static int
>> +virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state,
>> +                                    virStorageVolDefPtr vol,
>> +                                    const char *name)
> 
> Thanks for the refactor; this looks better than v1.
> 
>> +static int
>> +virStorageBackendGlusterVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
>> +                                  virStoragePoolObjPtr pool,
>> +                                  virStorageVolDefPtr vol,
>> +                                  bool internal)
>> +{
>> +    virStorageBackendGlusterStatePtr state = NULL;
>> +    struct stat st;
>> +    int ret = -1;
>> +
>> +    if (!internal) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       _("gluster pool backend doesn't "
>> +                         "yet support volume creation"));
>> +        goto cleanup;
>> +    }
> ...
>> +
>> +    if (glfs_stat(state->vol, vol->name, &st) == 0 &&
>> +        S_ISDIR(st.st_mode)) {
>> +        vol->type = VIR_STORAGE_VOL_NETDIR;
>> +        vol->target.format = VIR_STORAGE_FILE_DIR;
>> +    }
> 
> Shouldn't we error out if S_ISDIR and internal?  That is, the only time
> we should be using this function internally is for a regular file, not a
> directory.

Well, currently there isn't any planned use-case regarding the use of
this part of code on directories. There may be a need for this in the
future, but it can always be removed.

> 
> ACK with that addressed.
> 


Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]