Hi,

I just noticed the following in dasd.py:

if status in ["unformatted"] and device not in exclusiveDisks:

This seems wrong, as exclusiveDisks is not a list of disks to filter,
but the opposite it is a list of disks which we are allowed to touch,
so the proper test would be:

if status in ["unformatted"] and (not exclusiveDisks or device in exclusiveDisks):

Note that this code likely currently still works properly as DASD().startup() gets called
for the first time from the filter UI when building the disk list and at this time
exclusiveDisks is empty.

I do find this strange though, what is the use of having an
exclusiveDisks parameter to DASD().startup() to allow filtering out dasd's
from dasd formatting if we call DASD().startup() before the user has been
able to select which disks to filter out (and thus to populate
exclusiveDisks).

Regards,

Hans

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

On 06/30/2010 12:13 PM, Hans de Goede wrote:
> I just noticed the following in dasd.py:
>
> if status in ["unformatted"] and device not in exclusiveDisks:
>
> This seems wrong, as exclusiveDisks is not a list of disks to filter,
> but the opposite it is a list of disks which we are allowed to touch,
> so the proper test would be:
>
> if status in ["unformatted"] and (not exclusiveDisks or
> device in exclusiveDisks):

Cool, thanks, we missed that. If that's wrong it may cause potential
data loss.

> Note that this code likely currently still works properly as
> DASD().startup() gets called
> for the first time from the filter UI when building the disk list and at
> this time
> exclusiveDisks is empty.
>
> I do find this strange though, what is the use of having an
> exclusiveDisks parameter to DASD().startup() to allow filtering out dasd's
> from dasd formatting if we call DASD().startup() before the user has been
> able to select which disks to filter out (and thus to populate
> exclusiveDisks).

Exactly, that seems very much related to what I was referring to in an
earlier discussion I started and which was IMHO not finished:
https://www.redhat.com/archives/anaconda-devel-list/2010-April/msg00236.html

Steffen

Linux on System z Development

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

On Wed, 30 Jun 2010, Hans de Goede wrote:


Hi,

I just noticed the following in dasd.py:

if status in ["unformatted"] and device not in exclusiveDisks:

This seems wrong, as exclusiveDisks is not a list of disks to filter,
but the opposite it is a list of disks which we are allowed to touch,
so the proper test would be:

if status in ["unformatted"] and (not exclusiveDisks or device in
exclusiveDisks):


Yeah, this looks like a bug. Thanks for catching this.


Note that this code likely currently still works properly as
DASD().startup() gets called for the first time from the filter UI when
building the disk list and at this time exclusiveDisks is empty.

I do find this strange though, what is the use of having an
exclusiveDisks parameter to DASD().startup() to allow filtering out dasd's
from dasd formatting if we call DASD().startup() before the user has been
able to select which disks to filter out (and thus to populate
exclusiveDisks).


The dasd.py code came along before the filtering UI we have and, if I'm
remembering correctly, the main reason for the check against exclusiveDisks
was to handle the kickstart use case. If exclusiveDisks is still populated by
kickstart data when we enter DASD().startup(), we should probably still keep
it. Otherwise, I do see your point and we could simplify the test.

--
David Cantrell <dcantrell@redhat.com>
Red Hat / Honolulu, HI

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

Hi,

On 06/30/2010 02:35 PM, David Cantrell wrote:

On Wed, 30 Jun 2010, Hans de Goede wrote:


Hi,

I just noticed the following in dasd.py:

if status in ["unformatted"] and device not in exclusiveDisks:

This seems wrong, as exclusiveDisks is not a list of disks to filter,
but the opposite it is a list of disks which we are allowed to touch,
so the proper test would be:

if status in ["unformatted"] and (not exclusiveDisks or device in
exclusiveDisks):


Yeah, this looks like a bug. Thanks for catching this.


Note that this code likely currently still works properly as
DASD().startup() gets called for the first time from the filter UI when
building the disk list and at this time exclusiveDisks is empty.

I do find this strange though, what is the use of having an
exclusiveDisks parameter to DASD().startup() to allow filtering out
dasd's
from dasd formatting if we call DASD().startup() before the user has been
able to select which disks to filter out (and thus to populate
exclusiveDisks).


The dasd.py code came along before the filtering UI we have and, if I'm
remembering correctly, the main reason for the check against exclusiveDisks
was to handle the kickstart use case. If exclusiveDisks is still
populated by
kickstart data when we enter DASD().startup(), we should probably still
keep
it.


Yes exclusiveDisks can (but does not has to) be populated from kickstart.


Otherwise, I do see your point and we could simplify the test.


My point is that the order when doing an interactive install is wrong IMHO.

Currently what happens is:

1) call DASD().startup() (exclusive disks is empty at this time)
2) DASD().startup() will ask if dasdfmt should be done for *ALL* unformatted
dasd's
3) User can select which DASD's to make available during installation and
which to filter out.

And the order should be IMHO:
1) User can select which DASD's to make available during installation and
which to filter out.
2) call DASD().startup() (exclusive disks is *populated* at this time)
3) DASD().startup() will ask if dasdfmt should be done for *not filtered out*
unformatted dasd's

This assumes that DASD's can be shown for selection in the filter UI before
being dasdfmt-ed

Regards,

Hans

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list