Ovirt devel - Oct 2008 - [PATCH node-image] avoid image-minimizer fall-out

>From 8e45eb82fb7ed450de78e48aeaa8bd34b04313e1 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 8 Oct 2008 14:59:31 +0200
Subject: [PATCH node-image] ovirt-node-image.ks (%post): Include common-post.ks
last

I noticed the following error go by today:

    Removing blacklisted files and directories
    Cleanup empty directory structures in /usr/share
    Running image-minimizer...
==> Unknown Command: touch
    e2fsck 1.41.0 (10-Jul-2008)
    Pass 1: Checking inodes, blocks, and sizes

That's due to the new use of image-minimizer in common-post.ks (cool, btw!).
It appears at the end of that file:

    echo "Cleanup empty directory structures in /usr/share"
    find /usr/share -type d -exec rmdir {} \; > /dev/null 2>&1

    echo "Running image-minimizer..."
    %post --nochroot --interpreter image-minimizer
    drop /usr/lib/libboost*
    keep /usr/lib/libboost_program_options.so*
    keep /usr/lib/libboost_filesystem.so*
    drop /usr/lib64/libboost*
    keep /usr/lib64/libboost_program_options.so*
    keep /usr/lib64/libboost_filesystem.so*

However, common-post.ks is included from ovirt-node-image.ks like this:

    ...
    %post
    %include common-post.ks

    touch /.autorelabel

    %end

Which means the "touch" command is interpreted by image-minimizer.
Whoops.

The patch below fixes that by moving the "touch" to precede the
inclusion of common-post.ks:
---
 ovirt-node-image.ks |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/ovirt-node-image.ks b/ovirt-node-image.ks
index f5695d8..36b5da9 100644
--- a/ovirt-node-image.ks
+++ b/ovirt-node-image.ks
@@ -8,10 +8,13 @@
 %end

 %post
-%include common-post.ks

 touch /.autorelabel

+%include common-post.ks
+# CAUTION: don't include anything between the inclusion of common-post.ks
+# and the "%end" directive below.  Since the tail of common-post.ks
invokes
+# image-minimizer, anything here would be interpreted as commands to it.
 %end

 %post --nochroot
--
1.6.0.2.307.gc427

On Wed, Oct 08, 2008 at 03:08:50PM +0200, Jim Meyering
wrote:
> 
> >From 8e45eb82fb7ed450de78e48aeaa8bd34b04313e1 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at redhat.com>
> Date: Wed, 8 Oct 2008 14:59:31 +0200
> Subject: [PATCH node-image] ovirt-node-image.ks (%post): Include
common-post.ks last
> 
> I noticed the following error go by today:
> 
>     Removing blacklisted files and directories
>     Cleanup empty directory structures in /usr/share
>     Running image-minimizer...
> ==> Unknown Command: touch
The image minimiser should immediately exit with non-zero status
upon errors, not merely print a warning which could be missed in
the line noise for months.
>     e2fsck 1.41.0 (10-Jul-2008)
>     Pass 1: Checking inodes, blocks, and sizes
> 
> That's due to the new use of image-minimizer in common-post.ks (cool,
btw!).
> It appears at the end of that file:
> 
>     echo "Cleanup empty directory structures in /usr/share"
>     find /usr/share -type d -exec rmdir {} \; > /dev/null 2>&1
> 
>     echo "Running image-minimizer..."
>     %post --nochroot --interpreter image-minimizer
>     drop /usr/lib/libboost*
>     keep /usr/lib/libboost_program_options.so*
>     keep /usr/lib/libboost_filesystem.so*
>     drop /usr/lib64/libboost*
>     keep /usr/lib64/libboost_program_options.so*
>     keep /usr/lib64/libboost_filesystem.so*
> 
> However, common-post.ks is included from ovirt-node-image.ks like this:
> 
>     ...
>     %post
>     %include common-post.ks
> 
>     touch /.autorelabel
> 
>     %end
> 
> Which means the "touch" command is interpreted by
image-minimizer.
> Whoops.
> 
> The patch below fixes that by moving the "touch" to precede the
> inclusion of common-post.ks:
> ---
>  ovirt-node-image.ks |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/ovirt-node-image.ks b/ovirt-node-image.ks
> index f5695d8..36b5da9 100644
> --- a/ovirt-node-image.ks
> +++ b/ovirt-node-image.ks
> @@ -8,10 +8,13 @@
>  %end
> 
>  %post
> -%include common-post.ks
> 
>  touch /.autorelabel
> 
> +%include common-post.ks
> +# CAUTION: don't include anything between the inclusion of
common-post.ks
> +# and the "%end" directive below.  Since the tail of
common-post.ks invokes
> +# image-minimizer, anything here would be interpreted as commands to it.

How about instead, adding another  '%post' at the end of the
'common-post.ks'
script, so that it resets things back to being processed by 'sh',
instead
of the image-minimizer. 

If an include file changes semantics for a period, it is good practice to
restore the original semantics to avoid these kind of surprises.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Jim Meyering wrote:
> ==> Unknown Command: touch
Good catch, I missed that in my build log!
Fix is good, but better is to move all blacklisting into separate ks include.
Patch attached. Eventually, the whole blacklist will be implemented using
image-minimizer, but I just found that some improvements are needed, at least
support for /dir1/*/dir2/ patterns (patch for that posted to thincrust-devel)

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-move-blacklisting-into-separate-kickstart-include.patch
URL:
<http://listman.redhat.com/archives/ovirt-devel/attachments/20081008/39a375f5/attachment.ksh>

Jim Meyering wrote:
> ==> Unknown Command: touch
Good catch, I missed that in my build log!
Fix is good, but better is to move all blacklisting into separate ks include.
Patch post: [PATCH ovirt-node-image] move blacklisting into separate kickstart
include
Eventually, the whole blacklist will be implemented using image-minimizer, but I
just found that some improvements are needed, at least support for /dir1/*/dir2/
patterns (patch for that posted to thincrust-devel)