[master] & [rhel6-branch] various fixes

[David Cantrell]

These patches are intended for master and rhel6-branch.

1) In execWithCallback(), allow callers to disable the stdout printing.
   Useful when dasdfmt is run because without it all the of the progress
   information from dasdfmt scrolls by on the 3270 console and obscures
   other messages.

2) Stop writing the user-supplied hostname to the localhost line in
   /etc/hosts.  What we do currently is breaking virt systems and other
   programs.

3) Copy over cio_ignore parameter on s390 systems to the /etc/zipl.conf
   file during post installation setup.

[PATCH 1/3] In execWithCallback(), support disabling stdout echo (#528386)

[David Cantrell]

stdout echo is on by default, add a parameter that lets us turn that off
if we want to.
---
 iutil.py        |   14 +++++++++-----
 storage/dasd.py |    2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/iutil.py b/iutil.py
index 0b15bd8..7086f2c 100644
--- a/iutil.py
+++ b/iutil.py
@@ -243,8 +243,8 @@ def execWithCapture(command, argv, stdin = None, stderr = None, root='/'):
     return rc

def execWithCallback(command, argv, stdin = None, stdout = None,
-                     stderr = None, callback = None, callback_data = None,
-                     root = '/'):
+                     stderr = None, echo = True, callback = None,
+                     callback_data = None, root = '/'):
     def chroot():
         os.chroot(root)

@@ -310,8 +310,11 @@ def execWithCallback(command, argv, stdin = None, stdout = None,
             if e.errno != 4:
                 raise IOError, e.args

-        os.write(stdout, s)
+        if echo:
+            os.write(stdout, s)
+
         runningLog.write(s)
+
         if callback:
             callback(s, callback_data=callback_data)

@@ -349,9 +352,10 @@ def _pulseProgressCallback(data, callback_data=None):
         callback_data.pulse()

def execWithPulseProgress(command, argv, stdin = None, stdout = None,
-                          stderr = None, progress = None, root = '/'):
+                          stderr = None, echo = True, progress = None,
+                          root = '/'):
     execWithCallback(command, argv, stdin=stdin, stdout=stdout,
-                     stderr=stderr, callback=_pulseProgressCallback,
+                     stderr=stderr, echo=echo, callback=_pulseProgressCallback,
                      callback_data=progress, root=root)

## Run a shell.
diff --git a/storage/dasd.py b/storage/dasd.py
index 88f93f6..19b8488 100644
--- a/storage/dasd.py
+++ b/storage/dasd.py
@@ -140,7 +140,7 @@ class DASD:
                 iutil.execWithCallback("/sbin/dasdfmt", argv + [dasd],
                                        stdout="/dev/tty5", stderr="/dev/tty5",
                                        callback=self._updateProgressWindow,
-                                       callback_data=pw)
+                                       callback_data=pw, echo=False)

pw.pop()
         else:

[PATCH 1/3] In execWithCallback(), support disabling stdout echo (#528386)

[Hans de Goede]

Hi,

Have you checked all callers that they use keyword args, because if they
don't then adding a new argument in the middle of the list may be a problem?

Regards,

Hans

On 10/31/2009 12:20 AM, David Cantrell wrote:
> stdout echo is on by default, add a parameter that lets us turn that off
> if we want to.
>
> iutil.py        |   14 +++++++++-----
>   storage/dasd.py |    2 +-
>   2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/iutil.py b/iutil.py
> index 0b15bd8..7086f2c 100644
> --- a/iutil.py
> +++ b/iutil.py
> @@ -243,8 +243,8 @@ def execWithCapture(command, argv, stdin = None, stderr = None, root='/'):
>       return rc
>
> def execWithCallback(command, argv, stdin = None, stdout = None,
>                 stderr = None, callback = None, callback_data = None,
>                 root = '/'):
>                 stderr = None, echo = True, callback = None,
>                 callback_data = None, root = '/'):
> def chroot():
>     os.chroot(root)
>
> @@ -310,8 +310,11 @@ def execWithCallback(command, argv, stdin = None, stdout = None,
>               if e.errno != 4:
>                   raise IOError, e.args
>    os.write(stdout, s)
>    if echo:
>        os.write(stdout, s)
>     runningLog.write(s)
>     if callback:
>         callback(s, callback_data=callback_data)
>
> @@ -349,9 +352,10 @@ def _pulseProgressCallback(data, callback_data=None):
>           callback_data.pulse()
>
> def execWithPulseProgress(command, argv, stdin = None, stdout = None,
>                      stderr = None, progress = None, root = '/'):
>                      stderr = None, echo = True, progress = None,
>                      root = '/'):
> execWithCallback(command, argv, stdin=stdin, stdout=stdout,
>                 stderr=stderr, callback=_pulseProgressCallback,
>                 stderr=stderr, echo=echo, callback=_pulseProgressCallback,
>                  callback_data=progress, root=root)
>
> ## Run a shell.
>
> diff --git a/storage/dasd.py b/storage/dasd.py
> index 88f93f6..19b8488 100644
> --- a/storage/dasd.py
> +++ b/storage/dasd.py
> @@ -140,7 +140,7 @@ class DASD:
>                   iutil.execWithCallback("/sbin/dasdfmt", argv + [dasd],
>                                          stdout="/dev/tty5", stderr="/dev/tty5",
>                                          callback=self._updateProgressWindow,
>                                   callback_data=pw)
>                                   callback_data=pw, echo=False)
>
>         pw.pop()
>     else:

[PATCH 1/3] In execWithCallback(), support disabling stdout echo (#528386)

[David Cantrell]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yes, there are three execWithPulseProgress() callers in storage/formats/fs.py
and those are fine with their arguments.  There's only one execWithCallback()
caller at the moment and it's in storage/dasd.py...it's fine.

On Sun, 1 Nov 2009, Hans de Goede wrote:
> Hi,
>
> Have you checked all callers that they use keyword args, because if they
> don't then adding a new argument in the middle of the list may be a problem?
>
> Regards,
>
> Hans
>
> On 10/31/2009 12:20 AM, David Cantrell wrote:
> > stdout echo is on by default, add a parameter that lets us turn that off
> > if we want to.
> >
> > iutil.py        |   14 +++++++++-----
> >   storage/dasd.py |    2 +-
> >   2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/iutil.py b/iutil.py
> > index 0b15bd8..7086f2c 100644
> > --- a/iutil.py
> > +++ b/iutil.py
> > @@ -243,8 +243,8 @@ def execWithCapture(command, argv, stdin = None, stderr 
> > = None, root='/'):
> >       return rc
> >
> > def execWithCallback(command, argv, stdin = None, stdout = None,
> >                 stderr = None, callback = None, callback_data = None,
> >                 root = '/'):
> >                 stderr = None, echo = True, callback = None,
> >                 callback_data = None, root = '/'):
> > def chroot():
> >     os.chroot(root)
> >
> > @@ -310,8 +310,11 @@ def execWithCallback(command, argv, stdin = None, 
> > stdout = None,
> >               if e.errno != 4:
> >                   raise IOError, e.args
> >    os.write(stdout, s)
> >    if echo:
> >        os.write(stdout, s)
> >     runningLog.write(s)
> >     if callback:
> >         callback(s, callback_data=callback_data)
> >
> > @@ -349,9 +352,10 @@ def _pulseProgressCallback(data, callback_data=None):
> >           callback_data.pulse()
> >
> > def execWithPulseProgress(command, argv, stdin = None, stdout = None,
> >                      stderr = None, progress = None, root = '/'):
> >                      stderr = None, echo = True, progress = None,
> >                      root = '/'):
> > execWithCallback(command, argv, stdin=stdin, stdout=stdout,
> >                 stderr=stderr, callback=_pulseProgressCallback,
> >                 stderr=stderr, echo=echo, 
> >
> > callback=_pulseProgressCallback,
> >                        callback_data=progress, root=root)
> >
> > ## Run a shell.
> > diff --git a/storage/dasd.py b/storage/dasd.py
> > index 88f93f6..19b8488 100644
> > --- a/storage/dasd.py
> > +++ b/storage/dasd.py
> > @@ -140,7 +140,7 @@ class DASD:
> >                   iutil.execWithCallback("/sbin/dasdfmt", argv + [dasd],
> >                                          stdout="/dev/tty5", 
> > stderr="/dev/tty5",
> >                                          callback=self._updateProgressWindow,
> >                                   callback_data=pw)
> >                                   callback_data=pw, echo=False)
> >
> >         pw.pop()
> >     else:
>
> Anaconda-devel-list mailing list
> [e-mail]
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list

- -- 
David Cantrell 
Red Hat / Honolulu, HI
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkrvTfgACgkQ5hsjjIy1Vkmq0ACfZu1dVuLzLRw4K5Ke6xysnjEp
> lOkAniEn3yz1+aLt73JMII9U+bo4/iKm
> =35Y6
> -----END PGP SIGNATURE-----

[PATCH 1/3] In execWithCallback(), support disabling stdout echo (#528386)

[Hans de Goede]

Hi,

On 11/02/2009 10:24 PM, David Cantrell wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Yes, there are three execWithPulseProgress() callers in
> storage/formats/fs.py
> and those are fine with their arguments. There's only one
> execWithCallback()
> caller at the moment and it's in storage/dasd.py...it's fine.

Ok, in that case: ack.

Regards,

Hans
> On Sun, 1 Nov 2009, Hans de Goede wrote:
> > Hi,
> >
> > Have you checked all callers that they use keyword args, because if they
> > don't then adding a new argument in the middle of the list may be a
> > problem?
> >
> > Regards,
> >
> > Hans
> >
> > On 10/31/2009 12:20 AM, David Cantrell wrote:
> > > stdout echo is on by default, add a parameter that lets us turn that off
> > > if we want to.
> > >
> > > iutil.py | 14 +++++++++-----
> > > storage/dasd.py | 2 +-
> > > 2 files changed, 10 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/iutil.py b/iutil.py
> > > index 0b15bd8..7086f2c 100644
> > > --- a/iutil.py
> > > +++ b/iutil.py
> > > @@ -243,8 +243,8 @@ def execWithCapture(command, argv, stdin = None,
> > > stderr = None, root='/'):
> > > return rc
> > >
> > > def execWithCallback(command, argv, stdin = None, stdout = None,
> > > stderr = None, callback = None, callback_data = None,
> > > root = '/'):
> > > stderr = None, echo = True, callback = None,
> > > callback_data = None, root = '/'):
> > >
> > > def chroot():
> > > os.chroot(root)
> > >
> > > @@ -310,8 +310,11 @@ def execWithCallback(command, argv, stdin =
> > > None, stdout = None,
> > > if e.errno != 4:
> > > raise IOError, e.args
> > > os.write(stdout, s)
> > > if echo:
> > > os.write(stdout, s)
> > >
> > > runningLog.write(s)
> > >
> > > if callback:
> > > callback(s, callback_data=callback_data)
> > >
> > > @@ -349,9 +352,10 @@ def _pulseProgressCallback(data,
> > > callback_data=None):
> > > callback_data.pulse()
> > >
> > > def execWithPulseProgress(command, argv, stdin = None, stdout = None,
> > > stderr = None, progress = None, root = '/'):
> > > stderr = None, echo = True, progress = None,
> > > root = '/'):
> > >
> > > execWithCallback(command, argv, stdin=stdin, stdout=stdout,
> > > stderr=stderr, callback=_pulseProgressCallback,
> > > stderr=stderr, echo=echo, callback=_pulseProgressCallback,
> > >
> > > callback_data=progress, root=root)
> > >
> > > ## Run a shell.
> > > diff --git a/storage/dasd.py b/storage/dasd.py
> > > index 88f93f6..19b8488 100644
> > > --- a/storage/dasd.py
> > > +++ b/storage/dasd.py
> > > @@ -140,7 +140,7 @@ class DASD:
> > > iutil.execWithCallback("/sbin/dasdfmt", argv + [dasd],
> > > stdout="/dev/tty5", stderr="/dev/tty5",
> > > callback=self._updateProgressWindow,
> > > callback_data=pw)
> > > callback_data=pw, echo=False)
> > >
> > > pw.pop()
> > > else:
> >
> > Anaconda-devel-list mailing list
> > [e-mail]
> > https://www.redhat.com/mailman/listinfo/anaconda-devel-list
> -- David Cantrell
>
> Red Hat / Honolulu, HI
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkrvTfgACgkQ5hsjjIy1Vkmq0ACfZu1dVuLzLRw4K5Ke6xysnjEp
> lOkAniEn3yz1+aLt73JMII9U+bo4/iKm
> =35Y6
> -----END PGP SIGNATURE-----
>
> Anaconda-devel-list mailing list
> [e-mail]
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[PATCH 2/3] Do not modify /etc/hosts from setup package (#530343).

[David Cantrell]

Adding the user-specified hostname to the 127.0.0.1 line is breaking
virt systems, among other things.  I'm not entirely sure we need this
functionality anymore (faking the hostname as 127.0.0.1).

This reverts the changes done in
1aa67d5a2cdacd45f92ba15aa9392ba7191a49c7, but looking at the comments
for #506384, you'll see the same complaints as #530343 has are present.
---
 network.py |   23 -----------------------
 1 files changed, 0 insertions(+), 23 deletions(-)

diff --git a/network.py b/network.py
index 248255a..cb2a8e7 100644
--- a/network.py
+++ b/network.py
@@ -677,29 +677,6 @@ class Network:
             if domainname:
                 self.domains = [domainname]

-        # /etc/hosts
-        # update lines for 127.0.0.1 and ::1 with hostname if the file exists
-        if instPath and os.path.isfile(instPath + "/etc/hosts"):
-            of = open(instPath + "/etc/hosts", "r")
-            updatedlines = []
-            update = False
-            for line in of:
-                line = line.strip()
-                if line.startswith('127.0.0.1') or line.startswith('::1'):
-                    if self.hostname not in line.split():
-                        line += " %s" % self.hostname
-                        update = True
-                updatedlines.append(line)
-            of.close()
-
-            if update:
-                nf = open(instPath + "/etc/hosts", "w")
-                upd_comment = "# hostname %s added to /etc/hosts by anaconda\n" % self.hostname
-                nf.write(upd_comment + '\n'.join(updatedlines) + '\n')
-                nf.close()
-                log.info("/etc/hosts updated with hostname %s" % self.hostname) 
-
-
         # /etc/resolv.conf
         if (not instPath) or (not os.path.isfile(instPath + '/etc/resolv.conf')) or flags.livecdInstall:
             if os.path.isfile('/etc/resolv.conf') and instPath != '':

[PATCH 2/3] Do not modify /etc/hosts from setup package (#530343).

[Hans de Goede]

Hi,

Looks ok

Regards,

Hans

On 10/31/2009 12:20 AM, David Cantrell wrote:
> Adding the user-specified hostname to the 127.0.0.1 line is breaking
> virt systems, among other things.  I'm not entirely sure we need this
> functionality anymore (faking the hostname as 127.0.0.1).
>
> This reverts the changes done in
> 1aa67d5a2cdacd45f92ba15aa9392ba7191a49c7, but looking at the comments
> for #506384, you'll see the same complaints as #530343 has are present.
>
> network.py |   23 -----------------------
>   1 files changed, 0 insertions(+), 23 deletions(-)
>
> diff --git a/network.py b/network.py
> index 248255a..cb2a8e7 100644
> --- a/network.py
> +++ b/network.py
> @@ -677,29 +677,6 @@ class Network:
>               if domainname:
>                   self.domains = [domainname]
>    # /etc/hosts
>    # update lines for 127.0.0.1 and ::1 with hostname if the file exists
>    if instPath and os.path.isfile(instPath + "/etc/hosts"):
>        of = open(instPath + "/etc/hosts", "r")
>        updatedlines = []
>        update = False
>        for line in of:
>            line = line.strip()
>            if line.startswith('127.0.0.1') or line.startswith('::1'):
>                if self.hostname not in line.split():
>                    line += " %s" % self.hostname
>                    update = True
>            updatedlines.append(line)
>        of.close()
>        if update:
>            nf = open(instPath + "/etc/hosts", "w")
>            upd_comment = "# hostname %s added to /etc/hosts by anaconda\n" % self.hostname
>            nf.write(upd_comment + '\n'.join(updatedlines) + '\n')
>            nf.close()
>            log.info("/etc/hosts updated with hostname %s" % self.hostname)
>     # /etc/resolv.conf
>     if (not instPath) or (not os.path.isfile(instPath + '/etc/resolv.conf')) or flags.livecdInstall:
>         if os.path.isfile('/etc/resolv.conf') and instPath != '':

[PATCH 2/3] Do not modify /etc/hosts from setup package (#530343).

[Jeremy Katz]

On Fri, Oct 30, 2009 at 6:20 PM, David Cantrell  wrote:
> Adding the user-specified hostname to the 127.0.0.1 line is breaking
> virt systems, among other things.  I'm not entirely sure we need this
> functionality anymore (faking the hostname as 127.0.0.1).

Can you try booting a system with the hostname set to something which
is unresolvable and see if httpd and/or sendmail hang/take forever on
startup?  I think those were the main things which were unhappy at one
point with not being able to resolve the hostname.

- Jeremy

[PATCH 2/3] Do not modify /etc/hosts from setup package (#530343).

[David Cantrell]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Sun, 1 Nov 2009, Jeremy Katz wrote:
> On Fri, Oct 30, 2009 at 6:20 PM, David Cantrell  wrote:
> > Adding the user-specified hostname to the 127.0.0.1 line is breaking
> > virt systems, among other things.  I'm not entirely sure we need this
> > functionality anymore (faking the hostname as 127.0.0.1).
>
> Can you try booting a system with the hostname set to something which
> is unresolvable and see if httpd and/or sendmail hang/take forever on
> startup?  I think those were the main things which were unhappy at one
> point with not being able to resolve the hostname.

Looks like those hangs are finally gone.  Both sendmail and httpd start up
just fine with the /etc/hosts file as provided by the setup RPM and hostname
set to 'unreachablehost' (which is not configured on my LAN to resolve to
anything).

Tried this with the latest rawhide tree.

- -- 
David Cantrell 
Red Hat / Honolulu, HI
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkrvdMcACgkQ5hsjjIy1VknYHACbBfRh7o4vehfA++aykTefWY3G
> VTsAn3PzOfO9HhG7jLGoHyJt6La+laE9
> =mlcI
> -----END PGP SIGNATURE-----

[PATCH 2/3] Do not modify /etc/hosts from setup package (#530343).

[Jeremy Katz]

On Mon, Nov 2, 2009 at 7:09 PM, David Cantrell  wrote:
> On Sun, 1 Nov 2009, Jeremy Katz wrote:
> > On Fri, Oct 30, 2009 at 6:20 PM, David Cantrell  wrote:
> > > Adding the user-specified hostname to the 127.0.0.1 line is breaking
> > > virt systems, among other things.  I'm not entirely sure we need this
> > > functionality anymore (faking the hostname as 127.0.0.1).
> >
> > Can you try booting a system with the hostname set to something which
> > is unresolvable and see if httpd and/or sendmail hang/take forever on
> > startup?  I think those were the main things which were unhappy at one
> > point with not being able to resolve the hostname.
>
> Looks like those hangs are finally gone.  Both sendmail and httpd start up
> just fine with the /etc/hosts file as provided by the setup RPM and hostname
> set to 'unreachablehost' (which is not configured on my LAN to resolve to
> anything).

Hooray!  And some of the last vestiges of severe "WTF" inspiring code
can go away.

Thanks for double-checking it

- Jeremy

[PATCH 2/3] Do not modify /etc/hosts from setup package (#530343).

[David Cantrell]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 2 Nov 2009, Jeremy Katz wrote:
> On Mon, Nov 2, 2009 at 7:09 PM, David Cantrell  wrote:
> > On Sun, 1 Nov 2009, Jeremy Katz wrote:
> > > On Fri, Oct 30, 2009 at 6:20 PM, David Cantrell  wrote:
> > > > Adding the user-specified hostname to the 127.0.0.1 line is breaking
> > > > virt systems, among other things.  I'm not entirely sure we need this
> > > > functionality anymore (faking the hostname as 127.0.0.1).
> > >
> > > Can you try booting a system with the hostname set to something which
> > > is unresolvable and see if httpd and/or sendmail hang/take forever on
> > > startup?  I think those were the main things which were unhappy at one
> > > point with not being able to resolve the hostname.
> >
> > Looks like those hangs are finally gone.  Both sendmail and httpd start up
> > just fine with the /etc/hosts file as provided by the setup RPM and hostname
> > set to 'unreachablehost' (which is not configured on my LAN to resolve to
> > anything).
>
> Hooray!  And some of the last vestiges of severe "WTF" inspiring code
> can go away.
>
> Thanks for double-checking it

It is always nice to see minus signs in patches.

- -- 
David Cantrell 
Red Hat / Honolulu, HI
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkrwsioACgkQ5hsjjIy1VknhcACfWRp6fOLq31iwjikb4yuTIA9g
> eXkAn3pK/jK8Q67Pa2rPRWD8pAisCRC8
> =+saW
> -----END PGP SIGNATURE-----

[PATCH 3/3] Copy cio_ignore kernel parameter to zipl.conf on s390 (#475675).

[David Cantrell]

Part of the changes necessary for #475675, we need to take the
cio_ignore parameter (if it exists) given during installation and write
it to the parameter list in /etc/zipl.conf on the target system.
---
 booty/bootloaderInfo.py |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py
index bb51294..26f8bcc 100644
--- a/booty/bootloaderInfo.py
+++ b/booty/bootloaderInfo.py
@@ -154,6 +154,10 @@ class KernelArguments:
         ourargs = ["speakup_synth", "apic", "noapic", "apm", "ide", "noht",
                    "acpi", "video", "pci", "nodmraid", "nompath", "nomodeset",
                    "noiswmd"]
+
+        if iutil.isS390():
+            ourargs.append("cio_ignore")
+
         for arg in ourargs:
             if not flags.cmdline.has_key(arg):
                 continue

[PATCH 3/3] Copy cio_ignore kernel parameter to zipl.conf on s390 (#475675).

[Hans de Goede]

ack.

On 10/31/2009 12:20 AM, David Cantrell wrote:
> Part of the changes necessary for #475675, we need to take the
> cio_ignore parameter (if it exists) given during installation and write
> it to the parameter list in /etc/zipl.conf on the target system.
>
> booty/bootloaderInfo.py |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/booty/bootloaderInfo.py b/booty/bootloaderInfo.py
> index bb51294..26f8bcc 100644
> --- a/booty/bootloaderInfo.py
> +++ b/booty/bootloaderInfo.py
> @@ -154,6 +154,10 @@ class KernelArguments:
>           ourargs = ["speakup_synth", "apic", "noapic", "apm", "ide", "noht",
>                      "acpi", "video", "pci", "nodmraid", "nompath", "nomodeset",
>                      "noiswmd"]
>    if iutil.isS390():
>        ourargs.append("cio_ignore")
>     for arg in ourargs:
>         if not flags.cmdline.has_key(arg):
>             continue