[libvirt] [PATCH v4] storage: Check for invalid storage mode before opening

[Cole Robinson]

On 05/28/2010 03:38 PM, Eric Blake wrote:
> On 05/28/2010 01:26 PM, Cole Robinson wrote:
>> v4: Make second VolOpen function more extensible. Didn't opt to change
>>     FS backend defaults, this can just be to fix the original bug.
> 
> Good - you were able to take my pseudocode and flush it into something
> that looks even better.
> 
>> +
>> +    if (S_ISREG(sb.st_mode))
>> +        mode = VIR_STORAGE_VOL_OPEN_REG;
>> +    else if (S_ISCHR(sb.st_mode))
>> +        mode = VIR_STORAGE_VOL_OPEN_CHAR;
>> +    else if (S_ISBLK(sb.st_mode))
>> +        mode = VIR_STORAGE_VOL_OPEN_BLOCK;
>> +
> 
> Easy enough to extend, but as you say, extensions come later.
> 
>> diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
>> index 907c4bc..d0f40da 100644
>> --- a/src/storage/storage_backend.h
>> +++ b/src/storage/storage_backend.h
>> @@ -80,6 +80,25 @@ struct _virStorageBackend {
>>  
>>  virStorageBackendPtr virStorageBackendForType(int type);
>>  
>> +int virStorageBackendVolOpen(const char *path)
>> +ATTRIBUTE_NONNULL(1);
>> +
>> +/* VolOpenCheckMode flags */
>> +enum {
>> +    VIR_STORAGE_VOL_OPEN_ERROR  = 1 << 0, /* warn if unexpected type
>> +                                           * encountered */
>> +    VIR_STORAGE_VOL_OPEN_REG    = 1 << 1, /* regular files okay */
>> +    VIR_STORAGE_VOL_OPEN_BLOCK  = 1 << 2, /* block files okay */
>> +    VIR_STORAGE_VOL_OPEN_CHAR   = 1 << 3, /* char okay */
> 
> s/char /char files /
> 
>> +};
>> +
>> +#define DEFAULT_VOL_OPEN_FLAGS (VIR_STORAGE_VOL_OPEN_ERROR  |\
>> +                                VIR_STORAGE_VOL_OPEN_REG    |\
>> +                                VIR_STORAGE_VOL_OPEN_CHAR   |\
>> +                                VIR_STORAGE_VOL_OPEN_BLOCK)
> 
> Namespace pollution, since the macro doesn't start with VIR_.
> 
> s/DEFAULT_VOL_OPEN_FLAGS/VIR_STORAGE_VOL_OPEN_DEFAULT/
> 
>> +
>> +int virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags)
>> +ATTRIBUTE_NONNULL(1);
> 
> Also add ATTRIBUTE_RETURN_CHECK.
> 
> ACK with those minor changes.
> 

Thanks! I made those changes and pushed.

- Cole