Libguestfs - May 2022 - SELinux relabeling: do it by default

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"?

Thanks
Laszlo

On Tue, May 24, 2022 at 03:59:24PM +0200, 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 didn't carefully read it enough :-(
> > 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"?
AIUI this option should remain in the code, but do nothing.  If it can
never do anything[0] then we are allowed to drop it from
documentation.

[0] = I'm assuming here that even for a non-SELinux guest this option
would do nothing.  It wouldn't do some doomed attempt to relabel it.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

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?

Thanks
Laszlo