Hi Bruno, Sorry it has taken my longer than announce but here is finally my response:
On Sun, 2006-04-30 at 22:41 +0200, Bruno Cornec wrote: > Andree Leidenfrost said on Sat, Apr 29, 2006 at 05:32:55PM +1000: > > > Please find attached a patch that implements mdadm support whilst fully > > retaining raidtools2 support. > > Many thanks for your work in this Andree ! No worries. Remind me to thank you for all your work at times! ;-) > > With the changes detailed in the patch, I > > have just successfully archived and restored a RAID1 and a RAID0 array > > consisting of two drives each on Debian sid amd64, but I expect all > > Linux distribution with mdadm to work as well (famous last words ;-) ). > > Great ! > > > After much going back and forth I have decided to extend what is already > > there rather than recode much of the RAID functionality. The reasons are > > mainly that I have only limited time and wanted results rather sooner > > than later but even more so that I didn't want to change the raidtools2 > > side of things because I can't/won't test this and I didn't want to > > spend any effort on raidtools2 because it's officially deprecated (cf. > > http://people.redhat.com/mingo/raidtools/). > > No problem. That's a wise decision I think. Cool. > > With this said, the patch utilises existing capabilities for > > parsing /proc/mdstat and for reading and writing /etc/raidtab. mdadm > > works without a config file and even 'mdadm --detail --scan --verbose' > > does not give all the (relevant) information that can be gathered from > > mdstat. I could have come up with a new file (format), but I think > > raidtab is fine (it shouldn't be in /etc really). I am introducing a new > > function create_raid_device_via_mdadm() that will create a single RAID > > array using mdadm and that gets called in format_device() in case mkraid > > cannot be found. Same goes for 'mdadm -S' versus raidstop. An equivalent > > for raidstart is not really required as 'mdadm --create' starts the > > array straight away. > > > > There are limitations (pre-existing, not introduced by my changes): > > - chunk size is hard coded to 4k > > Onc my set of conf files work, please remind us to put that in it. As a side note, the system default if you don't specify anything is 64k. > > - spare devices are ignored > > - the parity algorithm is ignored > > - multipath is not supported > > Ok. > > > To overcome these limitations, I will need to change some data > > structures and expand the mdstat parser - in fact, I have already > > written an experimental stand-alone parser for this purpose. I'm not > > sure whether multipath is worth it; it is not even really a RAID level. > > No, but it's extremely useful for HA solutions. So if it's not a big > effort, I'd like to see it one day. Ok, I'll include multipath support than. It's already in parse_mdstat() and shouldn;t be too hard to add to the rest. > > Further, I had to include a few more modules for RAID in mindi and init > > needed a little change. > > No pb. I'll commit those changes right now, as they are obvious Cool. > > Finally, device mapper is not supported. I understand you are working on > > this, so I haven't looked into it. > > Well I'll work on it, when I'm done with all the other patches I have to > integrate (internationalization !!! done by an old relative, and also > Labeled swap fs). It won't be in 2.0.8, but in the next, I hope, which > should be 2.2.0 Ok, no worries. Internationalisation sounds great by the way! > > It would be great if you could let me know what you think, in particular > > whether you'd be sufficiently happy for this to go into the next stable > > version. > > patch mindi: taken and applied. > patch init: I have a question: > You're testing first raidstart, and then assume that it's lvmv1 Erm, I'm testing for raidstart and in its absence use mdrun. I have not touches any lvm related bits, so if lvm1 is assumed than that's unchanged from before. (RAID and LVM are independent, no?) > What about if both tools were installed ? Couldn't we get it more surely > from another parameter ? Shouldn't we store it and pass it throught our > conf file to init ? Well, if both tools were installed, raidtools2 would be used as in earlier versions to minimise the impact of the changes made - the new code only gets used if raidtools2 are not present. If we store it in a conf file, that would have to be read by mindi, mondoarchive, mondorestore and init. Can we maybe phase it in? I.e. could we use the 'look for raidtools2 programs and only use mdadm in their absence'-approach for now and revisit inclusion into the conf file later on? > No problem, OTOH with the patch itself > > patch mondo-prep.c : when you use asprintf, the devices string is > allocated. So before re-using it, you need to call paranoid_free on it. > If not, we're creating memory leaks. In your case, you need 2 variables. > Take an example in trunk to see how to deal with it. > You also have the same problem with program. > > That's why it takes time to do that in trunk :-)) <me, slapping my forehead> Of course, memory needs to be freed! Thanks a lot for pointing that out! I hope the updated attached version is correct in regards to that (please tell me if it's still not good!)... > Maybe the sync could also be kept for mdadm ? Just to let time to the > system to flush al the buffers. ...and it also inlcude the sync and wait and implements most of the missing bits. > I would take the opportunity that you test mkraid vs mdadm to create an > option somewhere that we could use in init. WDYT ? Yes above: In general yes, but I'd prefer if we could do this a t a slightly later stage. > Greetings, > Bruno. Cheers, Andree > -- > Linux Profession Lead EMEA / Open Source Evangelist \ HP C&I EMEA IET > http://www.mondorescue.org / HP/Intel Solution Center \ http://hpintelco.net > Des infos sur Linux? http://www.HyPer-Linux.org http://www.hp.com/linux > La musique ancienne? http://www.musique-ancienne.org http://www.medieval.org -- Andree Leidenfrost Sydney - Australia
/**
* Create @p RAID device using information from @p structure.
* This will create the specified RAID devive using information provided in
* raidlist by means of the mdadm tool.
* @param raidlist The structure containing all RAID information
* @param device The RAID device to create.
* @return 0 for success, nonzero for failure.
*/
int create_raid_device_via_mdadm(struct raidlist_itself *raidlist, char *device)
{
/** int **************************************************************/
int i = 0;
int j = 0;
int res = 0;
/** buffers ***********************************************************/
char *devices = NULL;
char *strtmp = NULL;
char *level = NULL;
char *program = NULL;
// leave straight away if raidlist is initial or has no entries
if (!raidlist || raidlist->entries == 0) {
log_msg(1, "No RAID arrays found.");
return 1;
} else {
log_msg(1, "%d RAID arrays found.", raidlist->entries);
}
// find raidlist entry for requested device
for (i = 0; i < raidlist->entries; i++) {
if (!strcmp(raidlist->el[i].raid_device, device)) break;
}
// check whether RAID device was found in raidlist
if (i == raidlist->entries) {
log_msg(1, "RAID device %s not found in list.", device);
return 1;
}
// create device list from normal disks followed by spare ones
asprintf(&devices, raidlist->el[i].data_disks.el[0].device);
for (j = 1; j < raidlist->el[i].data_disks.entries; j++) {
asprintf(&strtmp, "%s", devices);
paranoid_free(devices);
asprintf(&devices, "%s %s", strtmp,
raidlist->el[i].data_disks.el[j].device);
paranoid_free(strtmp);
}
for (j = 0; j < raidlist->el[i].spare_disks.entries; j++) {
asprintf(&strtmp, "%s", devices);
paranoid_free(devices);
asprintf(&devices, "%s %s", devices,
raidlist->el[i].spare_disks.el[j].device);
paranoid_free(strtmp);
}
// translate RAID level
if (raidlist->el[i].raid_level == -2) {
asprintf(&level, "multipath");
} else if (raidlist->el[i].raid_level == -1) {
asprintf(&level, "linear");
} else {
asprintf(&level, "raid%d", raidlist->el[i].raid_level);
}
// create RAID device:
// - RAID device, number of devices and devices mandatory
// - parity algorithm, chunk size and spare devices optional
// - faulty devices ignored
// - persistent superblock always used as this is recommended
asprintf(&program,
"mdadm --create --force --run %s --level=%s --raid-devices=%d",
raidlist->el[i].raid_device, level,
raidlist->el[i].data_disks.entries);
// parity commented out because it currently defaults to 0 which is valid
value
//if (raidlist->el[i].parity != -1) {
// asprintf(&strtmp, "%s", program);
// paranoid_free(program);
// asprintf(&program, "%s --parity=%d", strtmp, raidlist->el[i].parity);
// paranoid_free(strtmp);
// }
if (raidlist->el[i].chunk_size != -1) {
asprintf(&strtmp, "%s", program);
paranoid_free(program);
asprintf(&program, "%s --chunk=%d", strtmp, raidlist->el[i].chunk_size);
paranoid_free(strtmp);
}
if (raidlist->el[i].spare_disks.entries > 0) {
asprintf(&strtmp, "%s", program);
paranoid_free(program);
asprintf(&program, "%s --spare-devices=%d", strtmp,
raidlist->el[i].spare_disks.entries);
paranoid_free(strtmp);
}
asprintf(&strtmp, "%s", program);
paranoid_free(program);
asprintf(&program, "%s %s", strtmp, devices);
paranoid_free(strtmp);
log_msg(1, "About to run: %s", program);
res = run_program_and_log_output(program, 1);
log_msg(1, "%s returned %d", program, res);
// give the system the opportunity to adjust
system("sync");
sleep(3);
// free memory
paranoid_free(devices);
paranoid_free(level);
paranoid_free(program);
// return to calling instance
return res;
}
signature.asc
Description: This is a digitally signed message part
