[libvirt] [PATCH v2 1/4] virSecurityLabelDef: substitute 'norelabel' with 'relabel'
Eric Blake
eblake at redhat.com
Thu Jul 10 16:42:25 UTC 2014
On 07/10/2014 09:02 AM, Ján Tomko wrote:
> 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>
>> ---
>> +++ 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.
Actually, that would argue that virSecurityLabelDefNew() (and any other
allocation function) should set the relabel=true as PART of the
allocation. Any time that we rely on a default state, if the state is
all 0 then VIR_ALLOC is okay, and if the default state requires
non-zero, then all clients should use an allocation function and the
allocation function should take care of the default.
For example, see the commit pair bc3f5f1 (where I ensured that all
clients allocate via a common function) and c123ef7 (where I used that
common allocation function to initialize a non-zero member, rather than
having to touch each caller).
>> +++ 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?
or even "true (default) for allowing relabels"
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140710/51034b18/attachment-0001.sig>
More information about the libvir-list
mailing list