[Mageia-dev] NEED REVIEW: drakdisk refresh feature

[AL13N]

Op maandag 15 oktober 2012 15:05:28 schreef Pascal Terjan:
> On Sat, Oct 13, 2012 at 6:52 PM, AL13N <alien at rmail.be> wrote:
> > Hi all,
> > 
> > This is a first patch, concerning the drakdisk feature
> > https://wiki.mageia.org/en/Feature:DiskDrakeRedesign
> > 
> > the patch does the following:
> > - group the functionality to actually get all the disks in a function
> > - pass a ref to the function as the do_force_reload parameter to
> > interactive - make a Refresh function that returns "force_reload"
> > - add a button in the gtk
> > 
> > what it still needs for the first stage of this feature:
> > - a button for the curses one
> > - remove the abort message if no disks are found, but add a text in place
> > of the disk tabs to say there are no disks and telling you can refresh
> > after plugging in a disk.
> > 
> > 
> > attached is a small patch for /soft/drakx
> > 
> > can *anyone* tell me if i'm on the right track, or if i'm doing ugly
> > stuff, or if i'm forgetting important parts?
> 
> Sorry for not looing at it faster but after one month travelling my
> home was burgled 10 days ago which mean I could not yet come back to a
> normal life...

that is no worries, i hope you'll be better soon-ish.

> How is it different from the "Reload partition table" action hidden
> behind "More" (which for some reason was per disk even if it reloads
> them all) and only displayed during install?
> There is nothing obviously wrong, most of the work will be to make
> sure every part of the code handles this correctly (mostly
> raid/lvm/...) and interface also recreates tabs correctly etc...
> It has been broken/fixed various times and if this is to be displayed
> more visibly it should be properly tested

i realized this afterwards, and i think you're right, perhaps i should handle 
it a bit better:

-> actively look for new devices (or size-changed devices) and handle those; 
and thus not change all the changes you've made already.

The point of this is more like stepping stone towards the real things: ie: not 
aborting if there are no disks. and later adding installable network-based 
disks (for diskless systems).

And also, if your one of your hotplugged disks isn't correctly in it, i'd like 
to refresh it, so the 4th one is detected and so i can add it to the raid, for 
instance. instead of exiting the installer, etc...

in any case, it seems like someone found that it showed gray for a all disks, 
so it might not be that good after all...

but it would be nice for me though if i could pick your brain about the code 
and the way of programming though... it's not always easy to get around this 
kind of code.

Can i ask you in the future to look at this from time to time?

also, i fail to correctly grasp the purpose of the $do_force_reload variable. 
in some forms, it's used, and some it doesn't. but the function that is 
supposed to be parsed into that, it not clear what it should return. (to me)

Thanks for the feedback already.

AL13N