Joe, Upon testing, I noticed a problem when the Solaris partition is deleted and re-created at another location. In that case, the slice information collected at TD is no longer valid and should be invalidated. Added code in disk_slices.c to invalidate the slice information.
I also decided to include the fix for 6628 installation shouldn't fail when trying to delete nonexistent slice. http://defect.opensolaris.org/bz/show_bug.cgi?id=6628 I extended this to include partitions, so that if you specify a partition or slice for deletion and it is not there, only a warning is issued to the log file and the installation proceeds. Retested on SPARC and x86, deleting and recreating partitions and slices. Responses to your comments are below. Joseph J VLcek wrote: > William Schumann wrote: >> The AI manifest can specify partitions existing on a disk to be >> deleted. Currently there is only support for deleting partitions if >> you know the starting offset sector and length, which must match >> those for the partition in fdisk(1m), (which you can find from what >> 'fdisk -W - /dev/rdsk/cxtxdxp0' dumps). >> >> This provides support for deleting partitions using the manifest if >> you know only the partition ID - (1-4) >> >> Affects x86-only >> >> http://defect.opensolaris.org/bz/show_bug.cgi?id=5654 >> http://cr.opensolaris.org/~wmsch/bug-5654/ >> >> Tested: >> - deleted multiple partitions and added new partitions >> - tried to delete partition that doesn't exist >> - regression tested deleting by starting sector + length >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > William, > > Things look good. I only have some nits... > > Hope this helps! Joe > > > lib/liborchestrator/test_driver.c > --------------------------------- > > Invocation to om_delete_partition needs to be updated to include the > new parameter. We are no longer maintaining test_driver.c officially. I have not yet removed it from the Makefile, but it is neither built or included in any packages at the moment. > > usr/src/lib/liborchestrator/disk_parts.c > ---------------------------------------- > > Issue 1: to be consistent FD_NUMPART should be be OM_NUMPART on lines: > > 190 for (ipart = 0; ipart < FD_NUMPART; ipart++, pinfo++) { > 1165 for (ipart = 0; ipart < FD_NUMPART; ipart++) { > 1191 "%d (0-%d) for deletion\n", ipart, FD_NUMPART - 1); > 1194 (FD_NUMPART - ipart - 1) * sizeof (*pinfo)); > 1196 set_partition_unused(&pinfo[FD_NUMPART - 1]); > Changed. > Issue 2: Question/Idea > > Wouldn't the old post-delete om_debug_print loop be valuable prior to > returning from function om_delete_partition? > > 1144 for (ip = 0; ip < OM_NUMPART; ip++) > 1145 om_debug_print(OM_DBGLVL_INFO, > 1146 "post-delete dump: part_id=%d size=%d\n", > 1147 pinfo[ip].partition_id, > 1148 pinfo[ip].partition_size); > > Making this block of code a macro could keep the code clean... Just an > idea... When you look at the log, there seems to be enough debugging information, and the complexity of the code does not warrant lots of debugging info in the log, I think. Joe, Would you please review the additional changes? Thank you, William
