[lvm-devel] [PATCH 17/19] Rename internal vg_get_property to more generic lvm_get_property.
Dave Wysochanski
dwysocha at redhat.com
Mon Sep 20 16:49:56 UTC 2010
On Thu, 2010-09-16 at 10:57 +0200, Zdenek Kabelac wrote:
> Dne 16.9.2010 10:41, Zdenek Kabelac napsal(a):
> > Dne 15.9.2010 17:36, Dave Wysochanski napsal(a):
> >> lib/report/properties.c | 4 ++--
> >> lib/report/properties.h | 2 +-
> >> liblvm/lvm_vg.c | 2 +-
> >> 3 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/lib/report/properties.c b/lib/report/properties.c
> >> index 0b80593..95da8ab 100644
> >> --- a/lib/report/properties.c
> >> +++ b/lib/report/properties.c
> >> @@ -262,7 +262,7 @@ struct lvm_property_type _properties[] = {
> >> #undef FIELD
> >>
> >>
> >> -int vg_get_property(struct volume_group *vg, struct lvm_property_type *prop)
> >> +int lvm_get_property(void *obj, struct lvm_property_type *prop)
> >
> > Hope vg_get_property has been marked as unstable API ?
> >
> >
> >> {
> >> struct lvm_property_type *p;
> >>
> >> @@ -278,7 +278,7 @@ int vg_get_property(struct volume_group *vg, struct lvm_property_type *prop)
> >> }
> >>
> >> *prop = *p;
> >> - if (!p->get((void *)vg, prop)) {
> >> + if (!p->get(obj, prop)) {
> >> return 0;
> >> }
> >> return 1;
> >> diff --git a/lib/report/properties.h b/lib/report/properties.h
> >> index 2e1381d..0a13f39 100644
> >> --- a/lib/report/properties.h
> >> +++ b/lib/report/properties.h
> >> @@ -32,6 +32,6 @@ struct lvm_property_type {
> >> int (*set) (void *obj, struct lvm_property_type *prop);
> >> };
> >>
> >> -int vg_get_property(struct volume_group *vg, struct lvm_property_type *prop);
> >> +int lvm_get_property(void *obj, struct lvm_property_type *prop);
> >>
> >> #endif
> >> diff --git a/liblvm/lvm_vg.c b/liblvm/lvm_vg.c
> >> index 9a72bec..98070dd 100644
> >> --- a/liblvm/lvm_vg.c
> >> +++ b/liblvm/lvm_vg.c
> >> @@ -343,7 +343,7 @@ int lvm_vg_get_property(vg_t vg, const char *name,
> >> struct lvm_property_type prop;
> >>
> >> strncpy(prop.id, name, LVM_PROPERTY_NAME_LEN);
> >> - if (!vg_get_property(vg, &prop))
> >> + if (!lvm_get_property((void *)vg, &prop))
> >
> > No need to add cast to (void*) - it's C not C++...
> >
>
> In fact - do we really need this functionality ?? It's quite dangerous to
> allow passing any pointer type to such global API function.
>
> I would vote for only using separate functionality and using right types and
> right function names - this C++ polymorphism in C code leads to error which
> are hard to catch as compiler will not give any warning about missused typed
> structure pointers.
>
How about the attached patch in place of this one. I've left
vg_get_property() alone and added pv_get_property(). Also, I add the
report 'type' to lvm_property_type and then check inside the static
_get_property() function to ensure the correct function is called for
the given 'id' string.
>From 63e74cff2f9db5606b48d64e6ecf53935dd66839 Mon Sep 17 00:00:00 2001
From: Dave Wysochanski <dwysocha at redhat.com>
Date: Fri, 17 Sep 2010 13:16:53 -0400
Subject: [PATCH] Add pv_get_property and create generic internal _get_property function.
We need to use a similar function for pv and lv properties, so just make
a generic _get_property() function that contains most of the required
functionality. Also, add a check to ensure the field name matches the
object passed in by re-using report_type_t enum. For pv properties,
the report_type might be either PVS or LABEL.
---
lib/report/properties.c | 23 +++++++++++++++++++----
lib/report/properties.h | 3 +++
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/lib/report/properties.c b/lib/report/properties.c
index 0b80593..52657c0 100644
--- a/lib/report/properties.c
+++ b/lib/report/properties.c
@@ -250,11 +250,11 @@ GET_VG_NUM_PROPERTY_FN(vg_mda_copies, (vg_mda_copies(vg)))
#define STR DM_REPORT_FIELD_TYPE_STRING
#define NUM DM_REPORT_FIELD_TYPE_NUMBER
#define FIELD(type, strct, sorttype, head, field, width, fn, id, desc, writeable) \
- { #id, writeable, sorttype == STR, { .n_val = 0 }, _ ## id ## _get, _ ## id ## _set },
+ { type, #id, writeable, sorttype == STR, { .n_val = 0 }, _ ## id ## _get, _ ## id ## _set },
struct lvm_property_type _properties[] = {
#include "columns.h"
- { "", 0, 0, { .n_val = 0 }, _not_implemented, _not_implemented },
+ { 0, "", 0, 0, { .n_val = 0 }, _not_implemented, _not_implemented },
};
#undef STR
@@ -262,7 +262,7 @@ struct lvm_property_type _properties[] = {
#undef FIELD
-int vg_get_property(struct volume_group *vg, struct lvm_property_type *prop)
+static int _get_property(void *obj, struct lvm_property_type *prop, report_type_t type)
{
struct lvm_property_type *p;
@@ -276,10 +276,25 @@ int vg_get_property(struct volume_group *vg, struct lvm_property_type *prop)
log_errno(EINVAL, "Invalid property name %s", prop->id);
return 0;
}
+ if (!(p->type & type)) {
+ log_errno(EINVAL, "Property name %s does not match type %d",
+ prop->id, p->type);
+ return 0;
+ }
*prop = *p;
- if (!p->get((void *)vg, prop)) {
+ if (!p->get(obj, prop)) {
return 0;
}
return 1;
}
+
+int vg_get_property(struct volume_group *vg, struct lvm_property_type *prop)
+{
+ return _get_property(vg, prop, VGS);
+}
+
+int pv_get_property(struct physical_volume *pv, struct lvm_property_type *prop)
+{
+ return _get_property(pv, prop, PVS | LABEL);
+}
diff --git a/lib/report/properties.h b/lib/report/properties.h
index 2e1381d..1a1a439 100644
--- a/lib/report/properties.h
+++ b/lib/report/properties.h
@@ -17,10 +17,12 @@
#include "libdevmapper.h"
#include "lvm-types.h"
#include "metadata.h"
+#include "report.h"
#define LVM_PROPERTY_NAME_LEN DM_REPORT_FIELD_TYPE_ID_LEN
struct lvm_property_type {
+ report_type_t type;
char id[LVM_PROPERTY_NAME_LEN];
unsigned is_writeable;
unsigned is_string;
@@ -33,5 +35,6 @@ struct lvm_property_type {
};
int vg_get_property(struct volume_group *vg, struct lvm_property_type *prop);
+int pv_get_property(struct physical_volume *pv, struct lvm_property_type *prop);
#endif
--
1.7.2.2
More information about the lvm-devel
mailing list