* master - cleanup: move verbose message to lv_activation_skip
@ 2014-02-18 20:28 Zdenek Kabelac
2014-02-19 6:54 ` Marian Csontos
0 siblings, 1 reply; 3+ messages in thread
From: Zdenek Kabelac @ 2014-02-18 20:28 UTC (permalink / raw)
To: lvm-devel
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=fb519c35bbfae24a26346faf2b90d2a4189e8562
Commit: fb519c35bbfae24a26346faf2b90d2a4189e8562
Parent: fdcd95a3b3dd92ed219571467fdc235e1a6cb0b6
Author: Zdenek Kabelac <zkabelac@redhat.com>
AuthorDate: Tue Feb 18 20:49:32 2014 +0100
Committer: Zdenek Kabelac <zkabelac@redhat.com>
CommitterDate: Tue Feb 18 20:49:32 2014 +0100
cleanup: move verbose message to lv_activation_skip
Simplify code and put verbose message into a single place.
---
lib/metadata/lv_manip.c | 12 ++++++------
tools/lvchange.c | 5 +----
tools/vgchange.c | 5 +----
3 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
index 6b4c9a8..c2cc590 100644
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -5675,14 +5675,14 @@ void lv_set_activation_skip(struct logical_volume *lv, int override_default,
int lv_activation_skip(struct logical_volume *lv, activation_change_t activate,
int override_lv_skip_flag, int skip)
{
- /* Do not skip deactivation! */
- if ((activate == CHANGE_AN) || (activate == CHANGE_ALN))
+ if (!(lv->status & LV_ACTIVATION_SKIP) ||
+ !is_change_activating(activate) || /* Do not skip deactivation */
+ (override_lv_skip_flag && !skip))
return 0;
- if (override_lv_skip_flag)
- return skip;
-
- return (lv->status & LV_ACTIVATION_SKIP) ? 1 : 0;
+ log_verbose("ACTIVATON_SKIP flag set for LV %s/%s, skipping activation.",
+ lv->vg->name, lv->name);
+ return 1;
}
/* Greatest common divisor */
diff --git a/tools/lvchange.c b/tools/lvchange.c
index a0e350f..5b6da56 100644
--- a/tools/lvchange.c
+++ b/tools/lvchange.c
@@ -215,11 +215,8 @@ static int _lvchange_activate(struct cmd_context *cmd, struct logical_volume *lv
activate = (activation_change_t) arg_uint_value(cmd, activate_ARG, CHANGE_AY);
- if (lv_activation_skip(lv, activate, arg_count(cmd, ignoreactivationskip_ARG), 0)) {
- log_verbose("ACTIVATON_SKIP flag set for LV %s/%s, skipping activation.",
- lv->vg->name, lv->name);
+ if (lv_activation_skip(lv, activate, arg_count(cmd, ignoreactivationskip_ARG), 0))
return 1;
- }
if (lv_is_cow(lv) && !lv_is_virtual_origin(origin_from_cow(lv)))
lv = origin_from_cow(lv);
diff --git a/tools/vgchange.c b/tools/vgchange.c
index af4b002..d862ec2 100644
--- a/tools/vgchange.c
+++ b/tools/vgchange.c
@@ -121,11 +121,8 @@ static int _activate_lvs_in_vg(struct cmd_context *cmd, struct volume_group *vg,
((lv->status & PVMOVE) ))
continue;
- if (lv_activation_skip(lv, activate, arg_count(cmd, ignoreactivationskip_ARG), 0)) {
- log_verbose("ACTIVATION_SKIP flag set for LV %s/%s, skipping activation.",
- lv->vg->name, lv->name);
+ if (lv_activation_skip(lv, activate, arg_count(cmd, ignoreactivationskip_ARG), 0))
continue;
- }
if ((activate == CHANGE_AAY) &&
!lv_passes_auto_activation_filter(cmd, lv))
^ permalink raw reply related [flat|nested] 3+ messages in thread
* master - cleanup: move verbose message to lv_activation_skip
2014-02-18 20:28 master - cleanup: move verbose message to lv_activation_skip Zdenek Kabelac
@ 2014-02-19 6:54 ` Marian Csontos
2014-02-19 10:53 ` Zdenek Kabelac
0 siblings, 1 reply; 3+ messages in thread
From: Marian Csontos @ 2014-02-19 6:54 UTC (permalink / raw)
To: lvm-devel
On 02/18/2014 09:28 PM, Zdenek Kabelac wrote:
> Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=fb519c35bbfae24a26346faf2b90d2a4189e8562
> Commit: fb519c35bbfae24a26346faf2b90d2a4189e8562
> Parent: fdcd95a3b3dd92ed219571467fdc235e1a6cb0b6
> Author: Zdenek Kabelac <zkabelac@redhat.com>
> AuthorDate: Tue Feb 18 20:49:32 2014 +0100
> Committer: Zdenek Kabelac <zkabelac@redhat.com>
> CommitterDate: Tue Feb 18 20:49:32 2014 +0100
>
> cleanup: move verbose message to lv_activation_skip
>
> Simplify code and put verbose message into a single place.
> ---
> lib/metadata/lv_manip.c | 12 ++++++------
> tools/lvchange.c | 5 +----
> tools/vgchange.c | 5 +----
> 3 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
> index 6b4c9a8..c2cc590 100644
> --- a/lib/metadata/lv_manip.c
> +++ b/lib/metadata/lv_manip.c
> @@ -5675,14 +5675,14 @@ void lv_set_activation_skip(struct logical_volume *lv, int override_default,
> int lv_activation_skip(struct logical_volume *lv, activation_change_t activate,
> int override_lv_skip_flag, int skip)
> {
> - /* Do not skip deactivation! */
> - if ((activate == CHANGE_AN) || (activate == CHANGE_ALN))
> + if (!(lv->status & LV_ACTIVATION_SKIP) ||
> + !is_change_activating(activate) || /* Do not skip deactivation */
> + (override_lv_skip_flag && !skip))
> return 0;
>
> - if (override_lv_skip_flag)
> - return skip;
> -
> - return (lv->status & LV_ACTIVATION_SKIP) ? 1 : 0;
> + log_verbose("ACTIVATON_SKIP flag set for LV %s/%s, skipping activation.",
> + lv->vg->name, lv->name);
> + return 1;
I am not convinced this is pure "cleanup".
When `!(lv->status & LV_ACTIVATION_SKIP)` this returns 0 and ignores
`override_lv_skip_flag == 1 && skip == 1`.
-- Martian
> }
>
> /* Greatest common divisor */
> diff --git a/tools/lvchange.c b/tools/lvchange.c
> index a0e350f..5b6da56 100644
> --- a/tools/lvchange.c
> +++ b/tools/lvchange.c
> @@ -215,11 +215,8 @@ static int _lvchange_activate(struct cmd_context *cmd, struct logical_volume *lv
>
> activate = (activation_change_t) arg_uint_value(cmd, activate_ARG, CHANGE_AY);
>
> - if (lv_activation_skip(lv, activate, arg_count(cmd, ignoreactivationskip_ARG), 0)) {
> - log_verbose("ACTIVATON_SKIP flag set for LV %s/%s, skipping activation.",
> - lv->vg->name, lv->name);
> + if (lv_activation_skip(lv, activate, arg_count(cmd, ignoreactivationskip_ARG), 0))
> return 1;
> - }
>
> if (lv_is_cow(lv) && !lv_is_virtual_origin(origin_from_cow(lv)))
> lv = origin_from_cow(lv);
> diff --git a/tools/vgchange.c b/tools/vgchange.c
> index af4b002..d862ec2 100644
> --- a/tools/vgchange.c
> +++ b/tools/vgchange.c
> @@ -121,11 +121,8 @@ static int _activate_lvs_in_vg(struct cmd_context *cmd, struct volume_group *vg,
> ((lv->status & PVMOVE) ))
> continue;
>
> - if (lv_activation_skip(lv, activate, arg_count(cmd, ignoreactivationskip_ARG), 0)) {
> - log_verbose("ACTIVATION_SKIP flag set for LV %s/%s, skipping activation.",
> - lv->vg->name, lv->name);
> + if (lv_activation_skip(lv, activate, arg_count(cmd, ignoreactivationskip_ARG), 0))
> continue;
> - }
>
> if ((activate == CHANGE_AAY) &&
> !lv_passes_auto_activation_filter(cmd, lv))
>
> --
> lvm-devel mailing list
> lvm-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/lvm-devel
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* master - cleanup: move verbose message to lv_activation_skip
2014-02-19 6:54 ` Marian Csontos
@ 2014-02-19 10:53 ` Zdenek Kabelac
0 siblings, 0 replies; 3+ messages in thread
From: Zdenek Kabelac @ 2014-02-19 10:53 UTC (permalink / raw)
To: lvm-devel
Dne 19.2.2014 07:54, Marian Csontos napsal(a):
> On 02/18/2014 09:28 PM, Zdenek Kabelac wrote:
>> Gitweb:
>> http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=fb519c35bbfae24a26346faf2b90d2a4189e8562
>>
>> Commit: fb519c35bbfae24a26346faf2b90d2a4189e8562
>> Parent: fdcd95a3b3dd92ed219571467fdc235e1a6cb0b6
>> Author: Zdenek Kabelac <zkabelac@redhat.com>
>> AuthorDate: Tue Feb 18 20:49:32 2014 +0100
>> Committer: Zdenek Kabelac <zkabelac@redhat.com>
>> CommitterDate: Tue Feb 18 20:49:32 2014 +0100
>>
>> cleanup: move verbose message to lv_activation_skip
>>
>> Simplify code and put verbose message into a single place.
>> ---
>> lib/metadata/lv_manip.c | 12 ++++++------
>> tools/lvchange.c | 5 +----
>> tools/vgchange.c | 5 +----
>> 3 files changed, 8 insertions(+), 14 deletions(-)
>>
>> diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
>> index 6b4c9a8..c2cc590 100644
>> --- a/lib/metadata/lv_manip.c
>> +++ b/lib/metadata/lv_manip.c
>> @@ -5675,14 +5675,14 @@ void lv_set_activation_skip(struct logical_volume
>> *lv, int override_default,
>> int lv_activation_skip(struct logical_volume *lv, activation_change_t
>> activate,
>> int override_lv_skip_flag, int skip)
>> {
>> - /* Do not skip deactivation! */
>> - if ((activate == CHANGE_AN) || (activate == CHANGE_ALN))
>> + if (!(lv->status & LV_ACTIVATION_SKIP) ||
>> + !is_change_activating(activate) || /* Do not skip deactivation */
>> + (override_lv_skip_flag && !skip))
>> return 0;
>>
>> - if (override_lv_skip_flag)
>> - return skip;
>> -
>> - return (lv->status & LV_ACTIVATION_SKIP) ? 1 : 0;
>> + log_verbose("ACTIVATON_SKIP flag set for LV %s/%s, skipping activation.",
>> + lv->vg->name, lv->name);
>> + return 1;
>
> I am not convinced this is pure "cleanup".
> When `!(lv->status & LV_ACTIVATION_SKIP)` this returns 0 and ignores
> `override_lv_skip_flag == 1 && skip == 1`.
Yep - however we never call this function with 'skip == 1' - I've committed
second patch which takes this argument away so it now should end with better
readable code.
Zdenek
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-02-19 10:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-18 20:28 master - cleanup: move verbose message to lv_activation_skip Zdenek Kabelac
2014-02-19 6:54 ` Marian Csontos
2014-02-19 10:53 ` Zdenek Kabelac
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.