This patch effectively limits the number of cases when we would want to do a complete SELinux relabeling on Linux guest conversion. This was brought to my attention as we've recently had a support case when the conversion was taking too much time mostly because of relabeling performed with "setfiles -F". Although this patch might be worthy of taking as it is, I'd also like to clarify, do we really need relabeling of the entire file system during conversion? What exactly might go wrong here if we don't do that? Since this process might take hours on VMs big enough, it would be beneficial to be able to limit number of such cases even further, if possible. Unfortunately I couldn't find any hints in the libguestfs commit history as the relabeling code goes back to 0131d6f666 ("New tool: virt-v2v.").

On 4/24/23 10:46, Richard W.M. Jones wrote: > On Fri, Apr 21, 2023 at 09:01:40PM +0300, Andrey Drobyshev wrote: >> This patch effectively limits the number of cases when we would want to >> do a complete SELinux relabeling on Linux guest conversion. >> >> This was brought to my attention as we've recently had a support case >> when the conversion was taking too much time mostly because of >> relabeling performed with "setfiles -F". >> >> Although this patch might be worthy of taking as it is, I'd also like to >> clarify, do we really need relabeling of the entire file system during >> conversion? What exactly might go wrong here if we don't do that? >> Since this process might take hours on VMs big enough, it would be >> beneficial to be able to limit number of such cases even further, if >> possible. Unfortunately I couldn't find any hints in the libguestfs commit >> history as the relabeling code goes back to 0131d6f666 ("New tool: virt-v2v."). > > Relabelling is generally needed because we may have modified or > created files during the conversion. These will not be labelled > correctly by the libguestfs appliance (as would happen when the guest > runs normally) because whatever SELinux mechanism that does this isn't > running. > > If SELinux is enforcing this can and will stop services from starting > up at boot (you will see permission errors), and can even prevent a > guest from booting at all. > > Note we don't even have a list of possible files affected because we > run stuff like dracut & rpm. > > We should probably only need to relabel over "system directories" > (whatever that means), but we currently relabel over everything > mounted (basically everything mentioned in /etc/fstab) because that's > easier. The alternate path if setfiles doesn't work touches > /.autorelabel, but that just moves the same work to boot time. > > I don't think we've seen a case of labelling taking a long time, but > it could happen. (1) In "daemon/selinux-relabel.c", we have this comment: /* If setfiles takes an excessively long time to run (but still * completes) then removing .../contexts/files/file_contexts.bin * appears to help. If you find any such cases, please add * observations to the bug report: * https://bugzilla.redhat.com/show_bug.cgi?id=1396297 */ Perhaps it still applies. (2) We've touched on passing "--smp N" with N>1 to virt-customize. Recent versions of the selinux library can utilize multiple threads for relabeling. For example, the "restorecon" and "setfiles" utilities gained the "-T nthreads" option a while ago. Unfortunately, the default is "-T 1"; we should modify libguestfs to pass "-T 0", whenever "-T" is recognized. http://mid.mail-archive.com/20220719192112.GO21552@redhat.com AFAICT, virt-v2v would immediately benefit from this (even without an --smp switch); see commit d2b64ecc6701 ("v2v: Set the number of vCPUs to same as host number of pCPUs.", 2020-12-01). Andrey, how do you feel about contributing the "-T 0" extension to libguestfs? :) In "daemon/selinux-relabel.c", the setfiles_has_option() function should be usable for detecting whether "-T" is supported in the appliance.

On 4/24/23 10:46, Richard W.M. Jones wrote: >On Fri, Apr 21, 2023 at 09:01:40PM +0300, Andrey Drobyshev wrote: >>This patch effectively limits the number of cases when we would want to >>do a complete SELinux relabeling on Linux guest conversion. >> >>This was brought to my attention as we've recently had a support case >>when the conversion was taking too much time mostly because of >>relabeling performed with "setfiles -F". >> >>Although this patch might be worthy of taking as it is, I'd also like to >>clarify, do we really need relabeling of the entire file system during >>conversion? What exactly might go wrong here if we don't do that? >>Since this process might take hours on VMs big enough, it would be >>beneficial to be able to limit number of such cases even further, if >>possible. Unfortunately I couldn't find any hints in the libguestfs commit >>history as the relabeling code goes back to 0131d6f666 ("New tool: virt-v2v."). >Relabelling is generally needed because we may have modified or >created files during the conversion. These will not be labelled >correctly by the libguestfs appliance (as would happen when the guest >runs normally) because whatever SELinux mechanism that does this isn't >running. > >If SELinux is enforcing this can and will stop services from starting >up at boot (you will see permission errors), and can even prevent a >guest from booting at all. > >Note we don't even have a list of possible files affected because we >run stuff like dracut & rpm. > >We should probably only need to relabel over "system directories" >(whatever that means), but we currently relabel over everything >mounted (basically everything mentioned in /etc/fstab) because that's >easier. The alternate path if setfiles doesn't work touches >/.autorelabel, but that just moves the same work to boot time. > >I don't think we've seen a case of labelling taking a long time, but >it could happen. > >The patch you posted is fine because if SELinux is disabled then file >labels naturally get out of synch over time, as they won't be set on >newly created files. This is why setting SELinux to disabled isn't >really a "reversible" operation. You cannot reenable SELinux >afterwards without first doing a full filesystem relabel and reboot. >(Permissive doesn't have this problem.) > >Rich. > Would it be possible to relabel only modified files, at the first glance this should be faster.

Andrey is going to measure the performance difference and write a bit of the code. At the moment with 1 Tb image the process of relabeling taken more than 1 hour and exceed our reasonable timeout for the conversion.