[Libstoragemgmt-devel] [PATCH 1/4] lsm_convert.cpp: Remove duplicate code

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
c_binding/lsm_convert.cpp | 40 +++++++++++++++++++---------------------
1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/c_binding/lsm_convert.cpp b/c_binding/lsm_convert.cpp
index a971f15..02621c7 100644
--- a/c_binding/lsm_convert.cpp
+++ b/c_binding/lsm_convert.cpp
@@ -35,15 +35,21 @@ static bool is_expected_object(Value &obj, std::string class_name)
return false;
}

+static lsm_optional_data *get_optional(std::map<std::string, Value> &v)
+{
+ lsm_optional_data *op = NULL;
+ Value opv = v["optional_data"];
+ op = value_to_optional_data(opv);
+ return op;
+}
+
lsm_volume *value_to_volume(Value &vol)
{
lsm_volume *rc = NULL;

if (is_expected_object(vol, "Volume")) {
std::map<std::string, Value> v = vol.asObject();
- lsm_optional_data *op = NULL;
- Value opv = v["optional_data"];
- op = value_to_optional_data(opv);
+ lsm_optional_data *op = get_optional(v);

rc = lsm_volume_record_alloc(v["id"].asString().c_str(),
v["name"].asString().c_str(),
@@ -123,11 +129,9 @@ lsm_disk *value_to_disk(Value &disk)
{
lsm_disk *rc = NULL;
if (is_expected_object(disk, "Disk")) {
- lsm_optional_data *op = NULL;
- Value opv = disk["optional_data"];
- op = value_to_optional_data(opv);
-
std::map<std::string, Value> d = disk.asObject();
+ lsm_optional_data *op = get_optional(d);
+
rc = lsm_disk_record_alloc(
d["id"].asString().c_str(),
d["name"].asString().c_str(),
@@ -233,10 +237,8 @@ lsm_pool *value_to_pool(Value &pool)
lsm_pool *rc = NULL;

if (is_expected_object(pool, "Pool")) {
- lsm_optional_data *op = NULL;
std::map<std::string, Value> i = pool.asObject();
- Value opv = i["optional_data"];
- op = value_to_optional_data(opv);
+ lsm_optional_data *op = get_optional(i);

rc = lsm_pool_record_alloc(i["id"].asString().c_str(),
i["name"].asString().c_str(),
@@ -247,6 +249,8 @@ lsm_pool *value_to_pool(Value &pool)
i["system_id"].asString().c_str(),
op,
i["plugin_data"].asC_str());
+
+ lsm_optional_data_record_free(op);
}
return rc;
}
@@ -275,9 +279,7 @@ lsm_system *value_to_system(Value &system)
lsm_system *rc = NULL;
if (is_expected_object(system, "System")) {
std::map<std::string, Value> i = system.asObject();
- lsm_optional_data *op = NULL;
- Value opv = i["optional_data"];
- op = value_to_optional_data(opv);
+ lsm_optional_data *op = get_optional(i);

rc = lsm_system_record_alloc(i["id"].asString().c_str(),
i["name"].asString().c_str(),
@@ -351,8 +353,7 @@ lsm_access_group *value_to_access_group( Value &group )
if( is_expected_object(group, "AccessGroup")) {
std::map<std::string, Value> vAg = group.asObject();
il = value_to_string_list(vAg["init_ids"]);
- Value opv = group["optional_data"];
- op = value_to_optional_data(opv);
+ op = get_optional(vAg);

if( il ) {
ag = lsm_access_group_record_alloc(
@@ -489,9 +490,7 @@ lsm_fs *value_to_fs(Value &fs)
lsm_fs *rc = NULL;
if( is_expected_object(fs, "FileSystem") ) {
std::map<std::string, Value> f = fs.asObject();
- lsm_optional_data *op = NULL;
- Value opv = f["optional_data"];
- op = value_to_optional_data(opv);
+ lsm_optional_data *op = get_optional(f);

rc = lsm_fs_record_alloc(f["id"].asString().c_str(),
f["name"].asString().c_str(),
@@ -583,9 +582,7 @@ lsm_nfs_export *value_to_nfs_export(Value &exp)
}

if( ok ) {
- lsm_optional_data *op = NULL;
- Value opv = i["optional_data"];
- op = value_to_optional_data(opv);
+ lsm_optional_data *op = get_optional(i);

rc = lsm_nfs_export_record_alloc(i["id"].asC_str(),
i["fs_id"].asC_str(),
@@ -603,6 +600,7 @@ lsm_nfs_export *value_to_nfs_export(Value &exp)
lsm_string_list_free(root);
lsm_string_list_free(rw);
lsm_string_list_free(ro);
+ lsm_optional_data_record_free(op);
}
}
return rc;

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
c_binding/lsm_plugin_ipc.cpp | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/c_binding/lsm_plugin_ipc.cpp b/c_binding/lsm_plugin_ipc.cpp
index aba8e01..90e4e30 100644
--- a/c_binding/lsm_plugin_ipc.cpp
+++ b/c_binding/lsm_plugin_ipc.cpp
@@ -2638,13 +2638,17 @@ int LSM_DLL_EXPORT lsm_uri_parse(const char *uri, char **scheme, char **user,
struct qparam_set *qp = NULL;
qp = qparam_query_parse(u->query_raw);

- for( i = 0; i < qp->n; ++i ) {
- rc = lsm_optional_data_string_set(*query_params,
- qp->p[i].name,
- qp->p[i].value);
- if( LSM_ERR_OK != rc ) {
- goto bail;
+ if( qp ) {
+ for( i = 0; i < qp->n; ++i ) {
+ rc = lsm_optional_data_string_set(*query_params,
+ qp->p[i].name,
+ qp->p[i].value);
+ if( LSM_ERR_OK != rc ) {
+ free_qparam_set(qp);
+ goto bail;
+ }
}
+ free_qparam_set(qp);
}
} else {
rc = LSM_ERR_NO_MEMORY;

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
.../libstoragemgmt/libstoragemgmt_plug_interface.h | 16 +++++++++--
.../libstoragemgmt/libstoragemgmt_snapshot.h | 7 +++++
c_binding/lsm_convert.cpp | 9 ++++++-
c_binding/lsm_datatypes.cpp | 31 +++++++++++++++++-----
c_binding/lsm_datatypes.hpp | 2 ++
plugin/simc/simc_lsmplugin.c | 2 +-
python_binding/lsm/_data.py | 8 +++++-
7 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h b/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h
index 777fba7..e762685 100644
--- a/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h
+++ b/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h
@@ -1284,10 +1284,15 @@ const char *lsm_fs_plugin_data_get(lsm_fs *fs);
* @param id ID
* @param name Name
* @param ts Epoch time stamp when snapshot was created
+ * @param optional_data Optional data
+ * @param plugin_data Private plugin data
* @return Allocated memory, NULL on error
*/
-lsm_fs_ss LSM_DLL_EXPORT *lsm_fs_ss_record_alloc( const char *id, const char *name,
- uint64_t ts);
+lsm_fs_ss LSM_DLL_EXPORT *lsm_fs_ss_record_alloc(const char *id,
+ const char *name,
+ uint64_t ts,
+ lsm_optional_data *optional_data,
+ const char * plugin_data);

/**
* Allocates the memory for an array of snapshot records.
@@ -1297,6 +1302,13 @@ lsm_fs_ss LSM_DLL_EXPORT *lsm_fs_ss_record_alloc( const char *id, const char *na
lsm_fs_ss LSM_DLL_EXPORT **lsm_fs_ss_record_array_alloc( uint32_t size );

/**
+ * Retrieve private data from fs_ss.
+ * @param exp Valid fs_ss record
+ * @return Private data, else NULL
+ */
+const char LSM_DLL_EXPORT *lsm_fs_ss_plugin_data_get( lsm_fs_ss *fs_ss );
+
+/**
* Set a capability
* @param cap Valid capability pointer
* @param t Which capability to set
diff --git a/c_binding/include/libstoragemgmt/libstoragemgmt_snapshot.h b/c_binding/include/libstoragemgmt/libstoragemgmt_snapshot.h
index 393596b..144ed0b 100644
--- a/c_binding/include/libstoragemgmt/libstoragemgmt_snapshot.h
+++ b/c_binding/include/libstoragemgmt/libstoragemgmt_snapshot.h
@@ -69,6 +69,13 @@ const char LSM_DLL_EXPORT *lsm_fs_ss_name_get(lsm_fs_ss *ss);
*/
uint64_t LSM_DLL_EXPORT lsm_fs_ss_time_stamp_get(lsm_fs_ss *ss);

+/**
+ * Retrieve the optional data
+ * @param ss The file system snapshot record
+ * @return optional data, else NULL if it doesn't exist.
+ */
+lsm_optional_data LSM_DLL_EXPORT *lsm_fs_ss_optional_data_get(lsm_fs_ss *ss);
+
#ifdef __cplusplus
}
#endif
diff --git a/c_binding/lsm_convert.cpp b/c_binding/lsm_convert.cpp
index 02621c7..86e9e8f 100644
--- a/c_binding/lsm_convert.cpp
+++ b/c_binding/lsm_convert.cpp
@@ -530,9 +530,14 @@ lsm_fs_ss *value_to_ss(Value &ss)
lsm_fs_ss *rc = NULL;
if( is_expected_object(ss, "FsSnapshot") ) {
std::map<std::string, Value> f = ss.asObject();
+ lsm_optional_data *op = get_optional(f);
+
rc = lsm_fs_ss_record_alloc(f["id"].asString().c_str(),
f["name"].asString().c_str(),
- f["ts"].asUint64_t());
+ f["ts"].asUint64_t(), op,
+ f["plugin_data"].asC_str());
+
+ lsm_optional_data_record_free(op);
}
return rc;
}
@@ -545,6 +550,8 @@ Value ss_to_value(lsm_fs_ss *ss)
f["id"] = Value(ss->id);
f["name"] = Value(ss->name);
f["ts"] = Value(ss->ts);
+ f["optional_data"] = optional_data_to_value(ss->optional_data);
+ f["plugin_data"] = Value(ss->plugin_data);
return Value(f);
}
return Value();
diff --git a/c_binding/lsm_datatypes.cpp b/c_binding/lsm_datatypes.cpp
index 6791127..3ee76d0 100644
--- a/c_binding/lsm_datatypes.cpp
+++ b/c_binding/lsm_datatypes.cpp
@@ -1373,17 +1373,24 @@ MEMBER_FUNC_GET(lsm_optional_data *, lsm_fs_optional_data_get, lsm_fs *fs,
MEMBER_FUNC_GET(const char *, lsm_fs_plugin_data_get, lsm_fs *fs,
fs, LSM_IS_POOL, plugin_data, NULL)

-lsm_fs_ss * lsm_fs_ss_record_alloc( const char *id, const char *name,
- uint64_t ts)
+lsm_fs_ss * lsm_fs_ss_record_alloc(const char *id, const char *name,
+ uint64_t ts,
+ lsm_optional_data *optional_data,
+ const char * plugin_data)
{
- lsm_fs_ss *rc = (lsm_fs_ss *) malloc(sizeof(lsm_fs_ss));
+ lsm_fs_ss *rc = (lsm_fs_ss *) calloc(1, sizeof(lsm_fs_ss));
if( rc ) {
rc->magic = LSM_SS_MAGIC;
rc->id = strdup(id);
rc->name = strdup(name);
rc->ts = ts;
+ rc->optional_data = lsm_optional_data_record_copy(optional_data);
+ if( plugin_data ) {
+ rc->plugin_data = strdup(plugin_data);
+ }

- if( !rc->id || ! rc->name ) {
+ if( !rc->id || ! rc->name || (optional_data && !rc->optional_data) ||
+ (plugin_data && !rc->plugin_data)) {
lsm_fs_ss_record_free(rc);
rc = NULL;
}
@@ -1395,9 +1402,10 @@ lsm_fs_ss *lsm_fs_ss_record_copy(lsm_fs_ss *source)
{
lsm_fs_ss *rc = NULL;
if( LSM_IS_SS(source) ) {
- rc = lsm_fs_ss_record_alloc( lsm_fs_ss_id_get(source),
- lsm_fs_ss_name_get(source),
- lsm_fs_ss_time_stamp_get(source));
+ rc = lsm_fs_ss_record_alloc(source->id,
+ source->name,
+ source->ts,
+ source->optional_data, source->plugin_data);
}
return rc;
}
@@ -1408,6 +1416,9 @@ int lsm_fs_ss_record_free( lsm_fs_ss *ss)
ss->magic = LSM_DEL_MAGIC(LSM_SS_MAGIC);
free(ss->id);
free(ss->name);
+ lsm_optional_data_record_free(ss->optional_data);
+ free(ss->plugin_data);
+
free(ss);
return LSM_ERR_OK;
}
@@ -1433,6 +1444,12 @@ uint64_t lsm_fs_ss_time_stamp_get(lsm_fs_ss *ss)
MEMBER_GET(ss, LSM_IS_SS, ts, 0);
}

+MEMBER_FUNC_GET(lsm_optional_data *, lsm_fs_ss_optional_data_get, lsm_fs_ss *ss,
+ ss, LSM_IS_SS, optional_data, NULL)
+
+MEMBER_FUNC_GET(const char *, lsm_fs_ss_plugin_data_get, lsm_fs_ss *ss,
+ ss, LSM_IS_SS, plugin_data, NULL)
+
lsm_nfs_export *lsm_nfs_export_record_alloc(const char *id,
const char *fs_id,
const char *export_path,
diff --git a/c_binding/lsm_datatypes.hpp b/c_binding/lsm_datatypes.hpp
index b863f5d..8bae8de 100644
--- a/c_binding/lsm_datatypes.hpp
+++ b/c_binding/lsm_datatypes.hpp
@@ -291,6 +291,8 @@ struct LSM_DLL_LOCAL _lsm_fs_ss {
char *id;
char *name;
uint64_t ts;
+ lsm_optional_data *optional_data; /**< Optional data */
+ char *plugin_data; /**< Reserved for the plugin to use */
};

#define LSM_DISK_MAGIC 0xAA7A0010
diff --git a/plugin/simc/simc_lsmplugin.c b/plugin/simc/simc_lsmplugin.c
index 0632efc..a9321d2 100644
--- a/plugin/simc/simc_lsmplugin.c
+++ b/plugin/simc/simc_lsmplugin.c
@@ -2179,7 +2179,7 @@ static int ss_create(lsm_plugin_ptr c, lsm_fs *fs,
if( !g_hash_table_lookup(find->ss, md5(name)) ) {
char *id = strdup(md5(name));
if( id ) {
- lsm_fs_ss *ss = lsm_fs_ss_record_alloc(id, name, time(NULL));
+ lsm_fs_ss *ss = lsm_fs_ss_record_alloc(id, name, time(NULL), NULL, NULL);
lsm_fs_ss *new_shot = lsm_fs_ss_record_copy(ss);
if( ss && new_shot ) {
g_hash_table_insert(find->ss, (gpointer)id, (gpointer)ss);
diff --git a/python_binding/lsm/_data.py b/python_binding/lsm/_data.py
index e10b982..f7929f1 100644
--- a/python_binding/lsm/_data.py
+++ b/python_binding/lsm/_data.py
@@ -744,11 +744,17 @@ class FileSystem(IData):
@default_property('id', doc="Unique identifier")
@default_property('name', doc="Snapshot name")
@default_property('ts', doc="Time stamp the snapshot was created")
+@default_property("optional_data", "Optional data")
+@default_property("plugin_data", "Private plugin data")
class FsSnapshot(IData):
- def __init__(self, _id, _name, _ts):
+ def __init__(self, _id, _name, _ts, _optional_data=None,
+ _plugin_data=None):
self._id = _id
self._name = _name
self._ts = int(_ts)
+ self._optional_data = _check_opt_data(_optional_data,
+ self.OPT_PROPERTIES)
+ self._plugin_data = _plugin_data


@default_property('id', doc="Unique identifier")

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
c_binding/lsm_datatypes.hpp | 13 -------------
1 file changed, 13 deletions(-)

diff --git a/c_binding/lsm_datatypes.hpp b/c_binding/lsm_datatypes.hpp
index 8bae8de..cc6f65f 100644
--- a/c_binding/lsm_datatypes.hpp
+++ b/c_binding/lsm_datatypes.hpp
@@ -120,19 +120,6 @@ struct _lsm_access_group {
char *plugin_data; /**< Reserved for the plugin to use */
};

-#define LSM_SNAP_SHOT_MAGIC 0xAA7A0005
-#define LSM_IS_SNAP_SHOT(obj) MAGIC_CHECK(obj, LSM_SNAP_SHOT_MAGIC)
-
-/**
- * Structure for snapshots.
- */
-struct _lsm_snap_shot {
- uint32_t magic; /**< Used for verification */
- char *id; /**< Id */
- char *name; /**< Name */
- uint64_t ts; /**< Time stamp */
-};
-
#define LSM_NFS_EXPORT_MAGIC 0xAA7A0006
#define LSM_IS_NFS_EXPORT(obj) MAGIC_CHECK(obj, LSM_NFS_EXPORT_MAGIC)