[libvirt PATCH v2 1/3] scripts: Check spelling

[Jim Fehlig]

On 1/10/22 11:21, Andrea Bolognani wrote:
> On Mon, Jan 10, 2022 at 04:41:25PM +0100, Tim Wiederhake wrote:
>> +    ("/docs/glib-adoption.rst", "preferrable"),
> 
> This is an actual typo, isn't it?
> 
>> +    ("/docs/js/main.js", "whats"),
>> +    ("/src/libxl/libxl_logger.c", "purposedly"),
>> +    ("/src/qemu/qemu_process.c", "wee"),
>> +    ("/tests/storagepoolxml2xml", "cant"),
> 
> These are a few cases where I feel that rewording the existing
> comment or piece of code, even if it wouldn't strictly speaking count
> as fixing a typo, would still be preferable to having to list it as
> an exception.
> 
>> +    ("/src/util/virnetdevmacvlan.c", "calld"),
> 
> Same for this one, but I appreciate that others might consider
> renaming the variable as unnecessary churn and not worth the effort.
> 
>> +    ("/src/security/apparmor/libvirt-lxc", "devic"),
> 
> Looking at the context where this appears:
> 
>    deny /sys/d[^e]*{,/**} wklx,
>    deny /sys/de[^v]*{,/**} wklx,
>    deny /sys/dev[^i]*{,/**} wklx,
>    deny /sys/devi[^c]*{,/**} wklx,
>    deny /sys/devic[^e]*{,/**} wklx,
>    deny /sys/device[^s]*{,/**} wklx,
>    deny /sys/devices/[^v]*{,/**} wklx,
>    deny /sys/devices/v[^i]*{,/**} wklx,
>    deny /sys/devices/vi[^r]*{,/**} wklx,
>    deny /sys/devices/vir[^t]*{,/**} wklx,
>    deny /sys/devices/virt[^u]*{,/**} wklx,
>    deny /sys/devices/virtu[^a]*{,/**} wklx,
>    deny /sys/devices/virtua[^l]*{,/**} wklx,
>    deny /sys/devices/virtual/[^n]*{,/**} wklx,
>    deny /sys/devices/virtual/n[^e]*{,/**} wklx,
>    deny /sys/devices/virtual/ne[^t]*{,/**} wklx,
>    deny /sys/devices/virtual/net?*{,/**} wklx,
>    deny /sys/devices/virtual?*{,/**} wklx,
>    deny /sys/devices?*{,/**} wklx,
> 
> I mean, I don't speak AppArmor but this can't be right, can it? :D

It's valid apparmor. At least the apparmor parser doesn't complain :-). ISTM the 
last rule should cover the others.

> Jim, do you think we actually need such a slippery slope of deny
> rules, or can we simplify things a bit?

I don't know why all of these deny rules are defined in this manner. /sys/class, 
/proc/sys/kernel, and others are defined similarly. They were added by Cedric in 
commit 9265f8ab67d. Cedric, do you recall the purpose of defining the rules in 
this way?

>> +    ("/src/security/apparmor/libvirt-qemu", "readby"),
> 
> This should probably be made to apply to all libvirt-* files in that
> directory, as it's apparently part of the format specification.

Agree, it's a valid permissions value

https://gitlab.com/apparmor/apparmor/-/wikis/TechnicalDoc_Proc_and_ptrace

Regards,
Jim