[anaconda][rhel7-branch/master][PATCH] Don't crash if escrow certificate is requested without network access (#1085265)

Vratislav Podzimek vpodzime at redhat.com
Tue Sep 30 11:21:42 UTC 2014


On Fri, 2014-09-26 at 18:00 +0200, Martin Kolman wrote:
> On Fri, 2014-09-26 at 11:36 -0400, Anne Mulhern wrote:
> > 
> > 
> > 
> > ----- Original Message -----
> > > From: "Martin Kolman" <mkolman at redhat.com>
> > > To: anaconda-patches at lists.fedorahosted.org
> > > Sent: Friday, September 26, 2014 10:36:43 AM
> > > Subject: [anaconda][rhel7-branch/master][PATCH] Don't crash if escrow	certificate is requested without network access
> > > (#1085265)
> > > 
> > > Turn the KickstartError to KickstartValueError. The KickstartValueError
> > > is caught when the Storage spoke executes kickstart storage and user
> > > can inspect the error in the Storage spoke info bar and fix it.
> > > 
> > > Also improve the error message a bit.
> > > 
> > > Resolves: rhbz#1085265
> > > Signed-off-by: Martin Kolman <mkolman at redhat.com>
> > > ---
> > >  pyanaconda/kickstart.py | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
> > > index 7d25840..9f93ce9 100644
> > > --- a/pyanaconda/kickstart.py
> > > +++ b/pyanaconda/kickstart.py
> > > @@ -153,8 +153,8 @@ def getEscrowCertificate(escrowCerts, url):
> > >  
> > >      needs_net = not url.startswith("/") and not url.startswith("file:")
> > >      if needs_net and not nm.nm_is_connected():
> > > -        msg = _("Escrow certificate %s requires the network.") % url
> > > -        raise KickstartError(msg)
> > > +        msg = _("Escrow certificate %s requires network access.") % url
> > > +        raise KickstartValueError(msg)
> > >  
> > >      log.info("escrow: downloading %s", url)
> > >  
> > > --
> > > 1.9.3
> > > 
> > > _______________________________________________
> > > anaconda-patches mailing list
> > > anaconda-patches at lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > > 
> > 
> > There's nothing wrong with the value, it's the context, so KickstartError
> > is the most appropriate to raise.
> I've checked how we use KickstartValueError and based on the current
> usage I think the idea is that it is raised if the value is incorrect
> for the current environment, not just by itself.
> 
> For example this:
> raise KickstartValueError(formatErrorMsg(self.lineno, msg="Specified
> nonexistent disk %s in partition command" % self.disk))
> 
> This reports that the value of the given kickstart command is incorrect
> for the given environment - but it might very well be correct in a
> different one. And I think it the same applies here - requesting an
> escrow certificate without making sure network is available makes the
> value incorrect for the given environment.
> 
> Therefore in my opinion using KickstartValueError is correct + it makes
> the patch less invasive, which is good for RHEL7 branch.
I must agree with Martin here, it's not a perfect design and pattern,
but it is a design and pattern we use in many other places. No harm in
adding one more on rhel7-branch, I think.

-- 
Vratislav Podzimek

Anaconda Rider | RHCE | Red Hat, Inc. | Brno - Czech Republic




More information about the anaconda-patches mailing list