[libvirt] [PATCH v2 1/4] virSecurityLabelDef: substitute 'norelabel' with 'relabel'

[Ján Tomko]

On 07/10/2014 04:04 PM, Michal Privoznik wrote:
> This negation in names of boolean variables is driving me insane. The
> code is much more readable if we drop the 'no-' prefix. Well, at least
> for me.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/conf/domain_conf.c           | 20 ++++++++++----------
>  src/security/security_apparmor.c | 10 +++++-----
>  src/security/security_dac.c      | 14 +++++++-------
>  src/security/security_manager.c  |  2 +-
>  src/security/security_selinux.c  | 24 ++++++++++--------------
>  src/util/virseclabel.h           |  2 +-
>  6 files changed, 34 insertions(+), 38 deletions(-)
> 

> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index 16bec5c..8a45e04 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -616,7 +616,7 @@ virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
>                  seclabel->type = VIR_DOMAIN_SECLABEL_DYNAMIC;

seclabel->relabel = true;
is needed here now, since the code was relying on norelabel being false by
default to enable relabeling (and I agree with your comment about readability
now :))

The new default also affects the other caller of virSecurityLabelDefNew:
In qemuProcessAttach where we generate a new label:

        if (seclabeldef == NULL) {
            if (!(seclabeldef = virSecurityLabelDefNew(model)))
                goto error;
            seclabelgen = true;
        }

I'd set relabel to true here, to make this commit a no-op.


>              } else {
>                  seclabel->type = VIR_DOMAIN_SECLABEL_NONE;
> -                seclabel->norelabel = true;
> +                seclabel->relabel = false;
>              }
>          }
>  

> diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h
> index b90d212..8d671fd 100644
> --- a/src/util/virseclabel.h
> +++ b/src/util/virseclabel.h
> @@ -40,7 +40,7 @@ struct _virSecurityLabelDef {
>      char *imagelabel;   /* security image label string */
>      char *baselabel;    /* base name of label string */
>      int type;           /* virDomainSeclabelType */
> -    bool norelabel;
> +    bool relabel;       /* should try labeling attempts? */

I can't parse that. How about "whether we relabel files", or just leaving it
without a comment?

ACK with the two callers of virSecurityLabelDefNew fixed.

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140710/58245f56/attachment-0001.sig>