pvchange: use process_each_pv
PATCH 1/1 pvchange: use process_each_pv
The summary output at the end has been removed. A similar counter method could be added to the various process_each functions if we want to retain this sort of summary reporting.
--- tools/pvchange.c | 112 +++++++++++-------------------------------------------- 1 file changed, 22 insertions(+), 90 deletions(-)
tools/pvchange.c +22 -90
@@ -40,20 +40,20 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,if (tagargs && !(vg->fid->fmt->features &FMT_TAGS )) {log_error ("Volume group containing %s does not " "support tags" , pv_name); -return 0 ; +return ECMD_FAILED ; }if (arg_count (cmd, uuid_ARG) &&lvs_in_vg_activated (vg)) {log_error ("Volume group containing %s has active " "logical volumes" , pv_name); -return 0 ; +return ECMD_FAILED ; }if (!archive (vg)) -return 0 ; +return ECMD_FAILED ; }else {if (tagargs) {log_error ("Can't change tag on Physical Volume %s not " "in volume group" , pv_name); -return 0 ; +return ECMD_FAILED ; } }
@@ -62,20 +62,20 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg, !(pv->fmt->features & FMT_ORPHAN_ALLOCATABLE)) {log_error ("Allocatability not supported by orphan " "%s format PV %s" , pv->fmt->name, pv_name); -return 0 ; +return ECMD_FAILED ; }/* change allocatability for a PV */ if (allocatable && (pv_status (pv) &ALLOCATABLE_PV )) {log_warn ("Physical volume \"%s\" is already " "allocatable." , pv_name); -return 1 ; +return ECMD_PROCESSED ; }if (!allocatable && !(pv_status (pv) &ALLOCATABLE_PV )) {log_warn ("Physical volume \"%s\" is already " "unallocatable." , pv_name); -return 1 ; +return ECMD_PROCESSED ; }if (allocatable) {
@@ -92,10 +92,10 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,if (tagargs) {/* tag or deltag */ if (arg_count (cmd, addtag_ARG) && !change_tag (cmd,NULL ,NULL , pv, addtag_ARG)) - return_0; + return_ECMD_FAILED;if (arg_count (cmd, deltag_ARG) && !change_tag (cmd,NULL ,NULL , pv, deltag_ARG)) - return_0; + return_ECMD_FAILED; }
@@ -106,10 +106,10 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg, "of VG %s metadata? [y/n]: ", pv_vg_name(pv)) == 'n') {log_error ("Physical volume %s not changed" , pv_name); -return 0 ; +return ECMD_FAILED ; }if (!pv_change_metadataignore (pv, mda_ignore)) - return_0; + return_ECMD_FAILED; }if (arg_count (cmd, uuid_ARG)) {
@@ -118,15 +118,15 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,if (!id_create (&pv->id)) {log_error ("Failed to generate new random UUID for %s." , pv_name); -return 0 ; +return ECMD_FAILED ; }if (!id_write_format (&pv->id, uuid,sizeof (uuid))) -return 0 ; +return ECMD_FAILED ;log_verbose ("Changing uuid of %s to %s." , pv_name, uuid);if (!is_orphan (pv) && (!pv_write (cmd, pv,1 ))) {log_error ("pv_write with new uuid failed " "for %s." , pv_name); -return 0 ; +return ECMD_FAILED ; } }
@@ -135,33 +135,23 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,if (!vg_write (vg) || !vg_commit (vg)) {log_error ("Failed to store physical volume \"%s\" in " "volume group \"%s\"" , pv_name, vg->name); -return 0 ; +return ECMD_FAILED ; }backup (vg); }else if (!(pv_write (cmd, pv,0 ))) {log_error ("Failed to store physical volume \"%s\"" , pv_name); -return 0 ; +return ECMD_FAILED ; }log_print_unless_silent ("Physical volume \"%s\" changed" , pv_name); -return 1 ; +return ECMD_PROCESSED ; }int pvchange (struct cmd_context *cmd,int argc,char **argv) { -int opt =0 ; -int done =0 ; -int total =0 ; - -struct volume_group *vg; -const char *vg_name; -char *pv_name; - -struct pv_list *pvl; -struct dm_list *vgnames; -struct dm_str_list *sll; +int ret; if (!(arg_count(cmd, allocatable_ARG) + arg_is_set(cmd, addtag_ARG) + arg_is_set(cmd, deltag_ARG) + arg_count(cmd, uuid_ARG) +
@@ -181,39 +171,7 @@ int pvchange(struct cmd_context *cmd, int argc, char **argv)return EINVALID_CMD_LINE ; } -if (argc) { -log_verbose ("Using physical volume(s) on command line" ); -for (; opt < argc; opt++) { - total++; - pv_name = argv[opt]; -dm_unescape_colons_and_at_signs (pv_name,NULL ,NULL ); - vg_name =find_vgname_from_pvname (cmd, pv_name); -if (!vg_name) { -log_error ("Failed to read physical volume %s" , - pv_name); -continue ; - } - vg =vg_read_for_update (cmd, vg_name,NULL ,0 ); -if (vg_read_error (vg)) { -release_vg (vg); - stack; -continue ; - } - pvl =find_pv_in_vg (vg, pv_name); -if (!pvl || !pvl->pv) { -unlock_and_release_vg (cmd, vg, vg_name); -log_error ("Unable to find %s in %s" , - pv_name, vg_name); -continue ; - } - - done +=_pvchange_single (cmd, vg, - pvl->pv,NULL ); -unlock_and_release_vg (cmd, vg, vg_name); - } - }else { -log_verbose ("Scanning for physical volume names" ); -/* FIXME: share code with toollib */ +if (!argc) {/* * Take the global lock here so the lvmcache remains * consistent across orphan/non-orphan vg locks. If we don't
@@ -224,36 +182,10 @@ int pvchange(struct cmd_context *cmd, int argc, char **argv)log_error ("Unable to obtain global lock." );return ECMD_FAILED ; } - -/* populate lvmcache */ -if (!lvmetad_vg_list_to_lvmcache (cmd)) - stack; - -if ((vgnames =get_vgnames (cmd,1 )) && - !dm_list_empty (vgnames)) { - dm_list_iterate_items(sll, vgnames) { - vg =vg_read_for_update (cmd, sll->str,NULL ,0 ); -if (vg_read_error (vg)) { -release_vg (vg); - stack; -continue ; - } - dm_list_iterate_items(pvl, &vg->pvs) { - total++; - done +=_pvchange_single (cmd, vg, - pvl->pv, -NULL ); - } -unlock_and_release_vg (cmd, vg, sll->str); - } - } -unlock_vg (cmd,VG_GLOBAL ); } -log_print_unless_silent ("%d physical volume%s changed / %d physical volume%s " -"not changed" , - done, done ==1 ?"" :"s" , - total - done, (total - done) ==1 ?"" :"s" ); + ret =process_each_pv (cmd, argc, argv,NULL ,READ_FOR_UPDATE ,NULL , + _pvchange_single); -return (total == done) ?ECMD_PROCESSED :ECMD_FAILED ; +return ret; }
Signature · 1 line
1.8.3.1
PATCH 1/1 pvchange: use process_each_pv
Dne 16.10.2014 v 23:41 David Teigland napsal(a):
Quoted 14 lines
The summary output at the end has been removed. A similar counter method could be added to the various
process_eachfunctions if we want to retain this sort of summary reporting.--- tools/pvchange.c | 112 +++++++++++-------------------------------------------- 1 file changed, 22 insertions(+), 90 deletions(-)tools/pvchange.c +0
-0
tools/pvchange.c static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg, · line 44
Quoted 6 lines
tools/pvchange.c +1
-1@@ -40,20 +40,20 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,if (tagargs && !(vg->fid->fmt->features &FMT_TAGS )) {log_error ("Volume group containing %s does not " "support tags" , pv_name); -return 0 ; +return ECMD_FAILED ; }
I'd prefer to not switch internal functions that are called after the VG is opened to anything else then 0/1 return code.
Logic is:
When command starts - it validates options - any error detected before the VG is opened means EINVALID_CMD_LINE == 3 return code (but if the error is result of i.e. failing memory alloc it should be ECMD_FAIL)
Once you open VG - error is always ECMD_FAIL since VG was needed to resolve it. We do not make any more differences on cmd return code - more info goes from error message.
So until we figure out reasonable good interface for error passing form library function for error reporting I'd keep any function to stay with 0 - error
1 - ok
Once we start mixing ECMD_PROCESSED == 0 with 0/1 functions we get into troubles (I've already fixing these errors).
Zdenek
PATCH 1/1 pvchange: use process_each_pv
Quoted 38 lines · Zdenek Kabelac
Dne 16.10.2014 v 23:41 David Teigland napsal(a):
Quoted 14 lines
The summary output at the end has been removed. A similar counter method could be added to the various
process_eachfunctions if we want to retain this sort of summary reporting.--- tools/pvchange.c | 112 +++++++++++-------------------------------------------- 1 file changed, 22 insertions(+), 90 deletions(-)tools/pvchange.c +0
-0tools/pvchange.c static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg, · line 44
Quoted 6 lines
tools/pvchange.c +1
-1@@ -40,20 +40,20 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,if (tagargs && !(vg->fid->fmt->features &FMT_TAGS )) {log_error ("Volume group containing %s does not " "support tags" , pv_name); -return 0 ; +return ECMD_FAILED ; }I'd prefer to not switch internal functions that are called after the VG is opened to anything else then 0/1 return code.
Logic is:
When command starts - it validates options - any error detected before the VG is opened means
EINVALID_CMD_LINE== 3 return code (but if the error is result of i.e. failing memory alloc it should beECMD_FAIL)Once you open VG - error is always
ECMD_FAILsince VG was needed to resolve it. We do not make any more differences on cmd return code - more info goes from error message.
Quoted 8 lines
So until we figure out reasonable good interface for error passing form library function for error reporting I'd keep any function to stay with
0 - error
1 - okOnce we start mixing
ECMD_PROCESSED== 0 with 0/1 functions we get into troubles (I've already fixing these errors).
Returning 0/1 is not compatible with process_each, so I simply switched it to the ECMD standard used by process_each. The old 0/1 appears to be no more than a method to count objects for the summary output. Whether process_each ought to change from ECMD to 0/1 is another question.