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

On Tue, May 10, 2022 at 12:09:38PM +0200, Laszlo Ersek
wrote:
> Back to the structuring of these patches / patch sets. The generator
> lives and runs solely in libguestfs. However, it generates such code as
> well that is owned by libguestfs-common. Normally we don't notice,
> because the generator overwrites "common" submodule contents with
> identical files; thus, "git" does not complain about the
submodule
> checkout being modified locally.
Right, this was a known compromise to make splitting up the libguestfs
repo tractable.  Originally the generator could write into the common/
subdirectory freely because there was only a single repo.  When
splitting, I chose to commit the generated files, and since they
change infrequently this hasn't been a problem.
> This no longer holds with these
> patches. Therefore:
> 
> - as first step, libguestfs needs to be modified
> 
> - the generator is run as a part of "make", which creates a local
diff
>   in the "common" submodule checkout under the libguestfs
worktree
> 
> - that diff is reflected to, and captured as a commit, in
>   libguestfs-common
> 
> - this returns libguestfs to an "everything in sync" state, but
more
>   importantly
> 
> - it exposes the new stuff to virt-v2v and guestfs-tools,
> 
> - virt-v2v and guestfs-tools need to be updated to consider the
>   disappearance of "--selinux-relabel".
This is the right way to do it.
> The fact that documentation and test cases are shared in various ways
> only makes this more complicated. For example, the virt-builder(1)
> manual speaks words on SELinux in the auto-generated (and shared), and
> the private (non-shared) sections *both*.
> 
> One thing to note is that libguestfs itself does not *consume* the
> particular "common" contents that it generates. Therefore we
don't have
> a reference loop in practice. What we have is this dependency graph:
> 
>   libguestfs (generator)
>             |
>             v
>   libguestfs-common (generated content)
>        /              \
>        v               v
>    guestfs-tools     virt-v2v
> 
> Because of that, the usual "update common submodule" hunk *need
not* be
> squashed into the libguestfs (generator) patches, when merging this.
> However, said "update common submodule" hunk does have to be
squashed
> into the (single) guestfs-tools and virt-v2v patches, when merging.
To be clear, while there isn't a separate "update common
submodule"
commit in libguestfs, the commit hash of libguestfs -> common
submodule *will* still change (in the same commit that changes the
generator)?
> I meticulously tested this stuff:
> 
> - libguestfs:
> 
>   - "make check" and "make check-slow" complete fine
> 
>   - There is no documentation (under the "website/" subdir) that
is
>     updated by the patches.
> 
> - guestfs-tools:
> 
>   - Checked the rendered documentation regarding
"--no-selinux-relabel"
>     that comes from "common":
> 
>       virt-builder.1.html
>       virt-customize.1.html
>       virt-sysprep.1.html
> 
>   - Checked the rendered documentation changes that come from
>     guestfs-tools itself:
> 
>       virt-builder.1.html
> 
>   - Checked the "--help" output of:
> 
>       virt-builder
>       virt-customize
>       virt-sysprep
> 
>   - "make check" completes OK.
> 
>   - "make check-slow" completes OK:
> 
>     - PASS for test-firstboot-*.sh (Linux guests -- Windows guests are
>       SKIPped),
> 
>     - same for test-settings-*.sh
> 
>       - except for "test-settings-ubuntu-18.04.sh". It fails for
an
>         independent reason: "libguestfs: error: download:
>         /etc/sysconfig/network: No such file or directory"
> 
>     - PASS for test-selinuxrelabel.sh
> 
>     - "test-console-ubuntu-20.04.sh" fails for an independent
reason:
>       "didn't see login banner in serial console output" --
but no
>       serial output was actually shown in the log.
> 
> - virt-v2v:
> 
>   - "make check" completes OK.
> 
>   - "make check-slow" completes fine
> 
>     - in particular, PASS for test-v2v-conversion-of-*.sh (Linux guests
>       -- Windows guests are SKIPped)
I looked at the patches through the mailing list archives and
everything looks fine to me so you might as well push it all.  If
there are any problems I'm sure we'll find out soon enough.

For the whole series:

Acked-by: Richard W.M. Jones <rjones at redhat.com>

(You don't need to add this because I guess it will mess up your
carefully curated git commit hashes!)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On 05/10/22 13:10, Richard W.M. Jones wrote:
> On Tue, May 10, 2022 at 12:09:38PM +0200, Laszlo Ersek wrote:
>> One thing to note is that libguestfs itself does not *consume* the
>> particular "common" contents that it generates. Therefore we
don't have
>> a reference loop in practice. What we have is this dependency graph:
>>
>>   libguestfs (generator)
>>             |
>>             v
>>   libguestfs-common (generated content)
>>        /              \
>>        v               v
>>    guestfs-tools     virt-v2v
>>
>> Because of that, the usual "update common submodule" hunk
*need not* be
>> squashed into the libguestfs (generator) patches, when merging this.
>> However, said "update common submodule" hunk does have to be
squashed
>> into the (single) guestfs-tools and virt-v2v patches, when merging.
> 
> To be clear, while there isn't a separate "update common
submodule"
> commit in libguestfs, the commit hash of libguestfs -> common
> submodule *will* still change (in the same commit that changes the
> generator)?
I'll merge the libguestfs-common patch set, and the libguestfs patch
set, in unspecified order.

Then there *will* be a separate commit in libguestfs that updates the
submodule reference. It's just that this change will be an entirely
stand-alone commit -- the submodule update hunk need not be squashed
into any of the other -- actual patch -- libguestfs commits.

In other words, in the git history, there will be two stages where the
generator will output such content under "common" that actually
differs
from the then-checked-out submodule content -- but that's fine, because
libguestfs itself does not "consume back" the same content. So this
(temporary) difference is harmless.

For guestfs-tools and virt-v2v however, the difference is not tolerable.
Therefore, in each of those superprojects, I will extend the one patch
for that superproject, with the submodule update. So that the submodule
checkout and the dependent code advance in sync.
> I looked at the patches through the mailing list archives and
> everything looks fine to me so you might as well push it all.  If
> there are any problems I'm sure we'll find out soon enough.
Thanks!
> 
> For the whole series:
> 
> Acked-by: Richard W.M. Jones <rjones at redhat.com>
> 
> (You don't need to add this because I guess it will mess up your
> carefully curated git commit hashes!)
I will add it -- first because I have to extend the guestfs-tools and
virt-v2v patches for merging, like described above, anyway; second, I
need to be prepared for libguestfs-common moving forward mid-review in
any case. (I shouldn't expect actual conflicts, but the commit hash of
the HEAD could change.)

I'll report back with the actual commit ranges.

Thanks!
Laszlo