[Libguestfs] SELinux relabeling: do it by default

[Laszlo Ersek]

On 05/25/22 07:24, Laszlo Ersek wrote:
> On 05/24/22 15:59, Laszlo Ersek wrote:
>> On 05/24/22 12:04, Richard W.M. Jones wrote:
>>> So I didn't realise that these commits break --selinux-relabel, eg:
>>>
>>> $ rpm -q guestfs-tools
>>> guestfs-tools-1.49.1-1.fc37.x86_64
>>> $ virt-builder fedora-36 --selinux-relabel
>>> virt-builder: unrecognized option '--selinux-relabel'
>>> Try ‘virt-builder --help’ or consult virt-builder(1) for more
>>> information.
>>>
>>> This is kind of bad since it breaks existing scripts.
>>
>> I carefully highlighted it in the cover letter:
>>
>> https://listman.redhat.com/archives/libguestfs/2022-May/028826.html
>>
>>> I've intentionally avoided introducing "--no-selinux-relabel" *in
>>> addition* to "--selinux-relabel". While some utilities support a
>>> similar dual form (such as virt-builder's "--network" and
>>> "--no-network"), with one being the default, those options are special
>>> in that they are *not shared* between different utilities, and they
>>> are not generated by the generator in libguestfs. The key difference
>>> is that the *non-shared* options use Getopt.Set and Getopt.Clear on
>>> the *same* boolean reference cell, whereas the generator introduces a
>>> *separate* boolean reference cell for each option it generates (and
>>> then it uses *either* Getopt.Clear *or* Getopt.Set when the option is
>>> passed on the command line, dependent on the default value of the
>>> reference cell). This means that "--no-selinux-relabel" and
>>> "--selinux-relabel", if they both existed, would work on different
>>> booleans, and that would be the source of a lot of fun (priority?
>>> command line order? documentation? etc etc). So, nope to that.
>>
>> On 05/24/22 12:04, Richard W.M. Jones wrote:
>>> We just had a bug report about this from someone who is testing CentOS
>>> Stream 9.1 (where I backported the patches already).
>>>
>>> This option should continue to exist, but do nothing.
>>
>> The problems I outlined above remain.
>>
>> What happens if someone passes both options?
>>
>> Are we supposed to continue documenting "--selinux-relabel"?
> 
> ... is it perhaps enough if I I do not reintroduce "--selinux-relabel"
> to the generator (in libguestfs), only add it to guestfs-tools, namely in:
> 
> - "basic_args" in "sysprep/main.ml",
> - to the "--ignore=" option in "sysprep/test-virt-sysprep-docs.sh"?
> 
> Basically:
> 
> git show --color 19de3d1c8d4e -- \
>   sysprep/main.ml \
>   sysprep/test-virt-sysprep-docs.sh
> 
> And restore those hunks, except replace "--no-selinux-relabel" with
> "--selinux-relabel"?
> 
> Does that suffice?

Sigh, I don't think that this will work. :(

If I put the compat option in the generator, then both "virt-sysprep"
and "virt-customize" will get the option.

If I only put the compat option into "sysprep/main.ml", as described
above, then virt-customize will *not* get the option.

This can easily be tested with virt-customize and virt-sysprep binaries
that *predate* this particular patch set (that is, binaries that predate
guestfs-tools commit 19de3d1c8d4e):

$ virt-sysprep --no-selinux-relabel
virt-sysprep: error: you must give either -a or -d options.  Read
virt-sysprep(1) man page for further information.
...

$ virt-customize --no-selinux-relabel
virt-customize: unrecognized option '--no-selinux-relabel'

That is, "virt-sysprep" recognizes the historical compat option
"--no-selinux-relabel", but "virt-customize" doesn't. This means that I
cannot put the now-necessary "--selinux-relabel" option into just
"sysprep/main.ml", it needs to go into the generator. Dang.

Laszlo