[blivet:f21-branch/master 3/3] Make self.formatClass not a field (#1150147)

Anne Mulhern amulhern at redhat.com
Mon Oct 13 12:14:41 UTC 2014





----- Original Message -----
> From: "Vratislav Podzimek" <vpodzime at redhat.com>
> To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> Sent: Monday, October 13, 2014 3:42:50 AM
> Subject: Re: [blivet:f21-branch/master 3/3] Make self.formatClass not a	field (#1150147)
> 
> On Fri, 2014-10-10 at 12:24 -0400, mulhern wrote:
> > Related: fed#1150147
> > 
> > Remove constructor, because setting formatClass was all it did before
> > calling superconstructor.
> > 
> > self.formatClass field is only used in one method,
> > so might as well make it local to that one method as set it in constructor.
> > 
> > In constructor, it is necessary to set if before superclass constructor
> > call, which is a sign that something is slightly wrong (as superclass
> > state depends on additional subclass fields). Ideally, we could
> > be in a situation where we always call superclass constructor at start
> > of subclass constructor. In reality, that's not happening, but it
> > can here, so that's another plus.
> One big difference is that doing the call in the constructor means the
> exception is raised at that time. Is it okay to gracefully create a
> device with a wrong format and rthen raise an exception later when it is
> used?

There is no creating a device with any format that occurs in the ContainerDevice constructor.
What is calculated is the format that that devices requires its parents to posses.

This format is only used in the situation where a parent is added, in the current code,
and I'm not aware of any plans to use it in any other situation, although there might be.

There are many instances in blivet code where a method raises an exception even though
the constructor did not. This would just be another, except that it is possible to
demonstrate that it will never happen, by the following argument:

_addParent() will always be invoked by superclass constructor OR the number of parents
will be 0, which will cause an exception to be raised in the constructor.

However, this patch got withheld anyway, by the argument that at some
point, _addParent() might no longer check that the format was correct, and therefore not
need to invoke the code to find that format that it requires its members to possess,
and that it would still somehow be desirable to have a field that holds that format
(perhaps because at that time it was used in some other code that checked parents'
format type, maybe by some anaconda sanityCheck() code that needed access to that
information, or something like that).

In any case, this irritating bug brought to light again the fact that
"mdcontainer" devices have to be special cased all over, and that the
real solution is to split these out into a class that is separate
from MDRaidArrayDevice, or maybe a subtype of it. So really, the lighter
the patch set that deals with this current problem, the better,
assuming that that task is accomplished in the reasonably near future.

- mulhern

> 
> --
> Vratislav Podzimek
> 
> Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 


More information about the anaconda-patches mailing list