[Libstoragemgmt-devel] [PATCH 1/7] lsm_mgmt.cpp: DRY, use common code for ag add/delete

[Tony Asleson]

Consolidate similar code in one function.

Signed-off-by: Tony Asleson <***@redhat.com>
---
c_binding/lsm_mgmt.cpp | 52 +++++++++++++++++---------------------------------
1 file changed, 18 insertions(+), 34 deletions(-)

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index cb2665a..4aade5e 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -1364,12 +1364,12 @@ int lsm_access_group_delete(lsm_connect *c, lsm_access_group *access_group,
return rpc(c, "access_group_delete", parameters, response);
}

-int lsm_access_group_initiator_add(lsm_connect *c,
+static int _lsm_ag_add_delete(lsm_connect *c,
lsm_access_group *access_group,
const char *init_id,
lsm_access_group_init_type init_type,
lsm_access_group **updated_access_group,
- lsm_flag flags)
+ lsm_flag flags, const char *message)
{
CONN_SETUP(c);

@@ -1393,7 +1393,7 @@ int lsm_access_group_initiator_add(lsm_connect *c,
Value parameters(p);
Value response;

- int rc = rpc(c, "access_group_initiator_add", parameters, response);
+ int rc = rpc(c, message, parameters, response);
if( LSM_ERR_OK == rc ) {
//We should be getting a value back.
if( Value::object_t == response.valueType() ) {
@@ -1404,6 +1404,18 @@ int lsm_access_group_initiator_add(lsm_connect *c,
return rc;
}

+int lsm_access_group_initiator_add(lsm_connect *c,
+ lsm_access_group *access_group,
+ const char *init_id,
+ lsm_access_group_init_type init_type,
+ lsm_access_group **updated_access_group,
+ lsm_flag flags)
+{
+ return _lsm_ag_add_delete(c, access_group, init_id, init_type,
+ updated_access_group, flags,
+ "access_group_initiator_add");
+}
+
int lsm_access_group_initiator_delete(lsm_connect *c,
lsm_access_group *access_group,
const char* init_id,
@@ -1411,37 +1423,9 @@ int lsm_access_group_initiator_delete(lsm_connect *c,
lsm_access_group **updated_access_group,
lsm_flag flags)
{
- CONN_SETUP(c);
-
- if( !LSM_IS_ACCESS_GROUP(access_group) || CHECK_STR(init_id) ||
- LSM_FLAG_UNUSED_CHECK(flags) || CHECK_RP(updated_access_group)) {
- return LSM_ERR_INVALID_ARGUMENT;
- }
-
- Value id;
-
- if( LSM_ERR_OK != verify_initiator_id(init_id, init_type, id) ) {
- return LSM_ERR_INVALID_ARGUMENT;
- }
-
- std::map<std::string, Value> p;
- p["access_group"] = access_group_to_value(access_group);
- p["init_id"] = id;
- p["init_type"] = Value((int32_t)init_type);
- p["flags"] = Value(flags);
-
- Value parameters(p);
- Value response;
-
- int rc = rpc(c, "access_group_initiator_delete", parameters, response);
- if( LSM_ERR_OK == rc ) {
- //We should be getting a value back.
- if( Value::object_t == response.valueType() ) {
- *updated_access_group = value_to_access_group(response);
- }
- }
-
- return rc;
+ return _lsm_ag_add_delete(c, access_group, init_id, init_type,
+ updated_access_group, flags,
+ "access_group_initiator_delete");
}

int lsm_volume_mask(lsm_connect *c, lsm_access_group *access_group,

[Tony Asleson]

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

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index 985a765..08dddd0 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -331,6 +331,13 @@ int lsm_connect_close(lsm_connect *c, lsm_flag flags)
return rc;
}

+static Value _create_flag_param(lsm_flag flags)
+{
+ std::map<std::string, Value> p;
+ p["flags"] = Value(flags);
+ return Value(p);
+}
+
int lsm_plugin_info_get(lsm_connect *c, char **desc,
char **version, lsm_flag flags)
{
@@ -346,9 +353,8 @@ int lsm_plugin_info_get(lsm_connect *c, char **desc,
}

try {
- std::map<std::string, Value> p;
- p["flags"] = Value(flags);
- Value parameters(p);
+
+ Value parameters = _create_flag_param(flags);
Value response;

rc = rpc(c, "plugin_info", parameters, response);
@@ -505,9 +511,8 @@ int lsm_connect_timeout_get(lsm_connect *c, uint32_t *timeout, lsm_flag flags)
}

try {
- std::map<std::string, Value> p;
- p["flags"] = Value(flags);
- Value parameters(p);
+
+ Value parameters = _create_flag_param(flags);
Value response;

rc = rpc(c, "time_out_get", parameters, response);

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
c_binding/lsm_mgmt.cpp | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index 08dddd0..43c65db 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -1133,6 +1133,15 @@ int lsm_volume_replicate_range(lsm_connect *c,
return jobCheck(c, rc, response, job);
}

+static Value _create_volume_flag_param(lsm_volume *volume, lsm_flag flags)
+{
+ std::map<std::string, Value> p;
+ p["volume"] = volume_to_value(volume);
+ p["flags"] = Value(flags);
+
+ return Value(p);
+}
+
int lsm_volume_delete(lsm_connect *c, lsm_volume *volume, char **job,
lsm_flag flags)
{
@@ -1148,11 +1157,8 @@ int lsm_volume_delete(lsm_connect *c, lsm_volume *volume, char **job,
}

try {
- std::map<std::string, Value> p;
- p["volume"] = volume_to_value(volume);
- p["flags"] = Value(flags);

- Value parameters(p);
+ Value parameters = _create_volume_flag_param(volume, flags);
Value response;

rc = rpc(c, "volume_delete", parameters, response);
@@ -1199,11 +1205,8 @@ int lsm_volume_raid_info(lsm_connect *c, lsm_volume *volume,
}

try {
- std::map<std::string, Value> p;
- p["volume"] = volume_to_value(volume);
- p["flags"] = Value(flags);

- Value parameters(p);
+ Value parameters = _create_volume_flag_param(volume, flags);
Value response;

rc = rpc(c, "volume_raid_info", parameters, response);
@@ -1585,11 +1588,8 @@ int lsm_volume_child_dependency(lsm_connect *c, lsm_volume *volume,
}

try {
- std::map<std::string, Value> p;
- p["volume"] = volume_to_value(volume);
- p["flags"] = Value(flags);

- Value parameters(p);
+ Value parameters = _create_volume_flag_param(volume, flags);
Value response;

rc = rpc(c, "volume_child_dependency", parameters, response);
@@ -1610,11 +1610,7 @@ int lsm_volume_child_dependency_delete(lsm_connect *c, lsm_volume *volume,
return LSM_ERR_INVALID_ARGUMENT;
}

- std::map<std::string, Value> p;
- p["volume"] = volume_to_value(volume);
- p["flags"] = Value(flags);
-
- Value parameters(p);
+ Value parameters = _create_volume_flag_param(volume, flags);
Value response;

int rc = rpc(c, "volume_child_dependency_rm", parameters, response);

[Tony Asleson]

Use common code for retrieving a bool response.

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

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index 4aade5e..985a765 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -1546,6 +1546,25 @@ int lsm_access_groups_granted_to_volume(lsm_connect *c,
return getAccessGroups(c, rc, response, groups, groupCount);
}

+static int _retrieve_bool(int rc, Value &response, uint8_t *yes)
+{
+ int rc_out = rc;
+
+ *yes = 0;
+
+ if( LSM_ERR_OK == rc ) {
+ //We should be getting a boolean value back.
+ if( Value::boolean_t == response.valueType() ) {
+ if( response.asBool() ) {
+ *yes = 1;
+ }
+ } else {
+ rc_out = LSM_ERR_LIB_BUG;
+ }
+ }
+ return rc_out;
+}
+
int lsm_volume_child_dependency(lsm_connect *c, lsm_volume *volume,
uint8_t *yes, lsm_flag flags)
{
@@ -1568,19 +1587,8 @@ int lsm_volume_child_dependency(lsm_connect *c, lsm_volume *volume,
Value parameters(p);
Value response;

- *yes = 0;
-
rc = rpc(c, "volume_child_dependency", parameters, response);
- if( LSM_ERR_OK == rc ) {
- //We should be getting a boolean value back.
- if( Value::boolean_t == response.valueType() ) {
- if( response.asBool() ) {
- *yes = 1;
- }
- } else {
- rc = LSM_ERR_LIB_BUG;
- }
- }
+ rc = _retrieve_bool(rc, response, yes);
} catch( const ValueException &ve ) {
rc = logException(c, LSM_ERR_LIB_BUG, "Unexpected type",
ve.what());
@@ -1876,19 +1884,8 @@ int lsm_fs_child_dependency( lsm_connect *c, lsm_fs *fs, lsm_string_list *files,
Value parameters(p);
Value response;

- *yes = 0;
-
rc = rpc(c, "fs_child_dependency", parameters, response);
- if( LSM_ERR_OK == rc ) {
- //We should be getting a boolean value back.
- if( Value::boolean_t == response.valueType() ) {
- if( response.asBool() ) {
- *yes = 1;
- }
- } else {
- rc = LSM_ERR_LIB_BUG;
- }
- }
+ rc = _retrieve_bool(rc, response, yes);
} catch( const ValueException &ve ) {
rc = logException(c, LSM_ERR_LIB_BUG, "Unexpected type",
ve.what());

[Tony Asleson]

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

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index cf7ce30..54a3b2d 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -1744,6 +1744,14 @@ int lsm_fs_create(lsm_connect *c, lsm_pool *pool, const char *name,
return rc;
}

+static Value _create_fs_flag_param(lsm_fs *fs, lsm_flag flags)
+{
+ std::map<std::string, Value> p;
+ p["fs"] = fs_to_value(fs);
+ p["flags"] = Value(flags);
+ return Value(p);
+}
+
int lsm_fs_delete(lsm_connect *c, lsm_fs *fs, char **job, lsm_flag flags)
{
CONN_SETUP(c);
@@ -1752,11 +1760,7 @@ int lsm_fs_delete(lsm_connect *c, lsm_fs *fs, char **job, lsm_flag flags)
return LSM_ERR_INVALID_ARGUMENT;
}

- std::map<std::string, Value> p;
- p["fs"] = fs_to_value(fs);
- p["flags"] = Value(flags);
-
- Value parameters(p);
+ Value parameters = _create_fs_flag_param(fs, flags);
Value response;

int rc = rpc(c, "fs_delete", parameters, response);
@@ -1928,11 +1932,7 @@ int lsm_fs_ss_list(lsm_connect *c, lsm_fs *fs, lsm_fs_ss **ss[],
return LSM_ERR_INVALID_ARGUMENT;
}

- std::map<std::string, Value> p;
- p["fs"] = fs_to_value(fs);
- p["flags"] = Value(flags);
-
- Value parameters(p);
+ Value parameters = _create_fs_flag_param(fs, flags);
Value response;

try {

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
c_binding/lsm_mgmt.cpp | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index 43c65db..cf7ce30 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -1162,18 +1162,7 @@ int lsm_volume_delete(lsm_connect *c, lsm_volume *volume, char **job,
Value response;

rc = rpc(c, "volume_delete", parameters, response);
- if( LSM_ERR_OK == rc ) {
- //We get a value back, either null or job id.
- if( Value::string_t == response.valueType() ) {
- *job = strdup(response.asString().c_str());
-
- if( *job ) {
- rc = LSM_ERR_JOB_STARTED;
- } else {
- rc = LSM_ERR_NO_MEMORY;
- }
- }
- }
+ rc = jobCheck(c, rc, response, job);
} catch( const ValueException &ve ) {
rc = logException(c, LSM_ERR_LIB_BUG, "Unexpected type",
ve.what());

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
c_binding/lsm_mgmt.cpp | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index 54a3b2d..3b91402 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -1849,6 +1849,16 @@ int lsm_fs_file_clone(lsm_connect *c, lsm_fs *fs, const char *src_file_name,
return jobCheck(c, rc, response, job);
}

+static Value _create_fs_file_flag_params(lsm_fs *fs, lsm_string_list *files,
+ lsm_flags flags)
+{
+ std::map<std::string, Value> p;
+ p["fs"] = fs_to_value(fs);
+ p["files"] = string_list_to_value(files);
+ p["flags"] = Value(flags);
+ return Value(p);
+}
+
int lsm_fs_child_dependency( lsm_connect *c, lsm_fs *fs, lsm_string_list *files,
uint8_t *yes, lsm_flag flags)
{
@@ -1870,12 +1880,8 @@ int lsm_fs_child_dependency( lsm_connect *c, lsm_fs *fs, lsm_string_list *files,
}

try {
- std::map<std::string, Value> p;
- p["fs"] = fs_to_value(fs);
- p["files"] = string_list_to_value(files);
- p["flags"] = Value(flags);

- Value parameters(p);
+ Value parameters = _create_fs_file_flag_params(fs, files, flags);
Value response;

rc = rpc(c, "fs_child_dependency", parameters, response);
@@ -1906,12 +1912,7 @@ int lsm_fs_child_dependency_delete( lsm_connect *c, lsm_fs *fs, lsm_string_list
return LSM_ERR_INVALID_ARGUMENT;
}

- std::map<std::string, Value> p;
- p["fs"] = fs_to_value(fs);
- p["files"] = string_list_to_value(files);
- p["flags"] = Value(flags);
-
- Value parameters(p);
+ Value parameters = _create_fs_file_flag_params(fs, files, flags);
Value response;

int rc = rpc(c, "fs_child_dependency_rm", parameters, response);

[Gris Ge]

On Thu, Mar 05, 2015 at 07:04:06PM -0600, Tony Asleson wrote:

Hi Tony,

All other patches looks good. With trivial changes below, it passes
'make rpm' test in all supported OS.

Post by Tony Asleson
---
c_binding/lsm_mgmt.cpp | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index 54a3b2d..3b91402 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -1849,6 +1849,16 @@ int lsm_fs_file_clone(lsm_connect *c, lsm_fs *fs, const char *src_file_name,
return jobCheck(c, rc, response, job);
}
+static Value _create_fs_file_flag_params(lsm_fs *fs, lsm_string_list *files,
+ lsm_flags flags)

Should be 'lsm_flag flags'.

[Tony Asleson]

Post by Gris Ge
Hi Tony,
All other patches looks good. With trivial changes below, it passes
'make rpm' test in all supported OS.

Post by Tony Asleson
---
c_binding/lsm_mgmt.cpp | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index 54a3b2d..3b91402 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -1849,6 +1849,16 @@ int lsm_fs_file_clone(lsm_connect *c, lsm_fs *fs, const char *src_file_name,
return jobCheck(c, rc, response, job);
}
+static Value _create_fs_file_flag_params(lsm_fs *fs, lsm_string_list *files,
+ lsm_flags flags)

Should be 'lsm_flag flags'.

I wonder how this didn't fail for me when I ran the tests...

Thanks for the review!

Regards,
Tony

[Gris Ge]

After I stumbled upon the copy/pasted code in previous patch
submission I looked some more and found some. I just tackled this
one file, but other duplicate code exists in other files too.

Hi Tony,

I lost myself in the battle with SMI-S.
Will review this patch set next Monday.

Thank you.