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 _______________________________________________ Libguestfs mailing list [email protected] https://listman.redhat.com/mailman/listinfo/libguestfs
