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

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



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.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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