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.