Changeset View
Standalone View
stand/common/disk.c
Show First 20 Lines • Show All 123 Lines • ▼ Show 20 Lines | ptable_print(void *arg, const char *pname, const struct ptable_entry *part) | ||||
if (pager_output(line)) | if (pager_output(line)) | ||||
return 1; | return 1; | ||||
res = 0; | res = 0; | ||||
if (part->type == PART_FREEBSD) { | if (part->type == PART_FREEBSD) { | ||||
/* Open slice with BSD label */ | /* Open slice with BSD label */ | ||||
dev.dd.d_dev = pa->dev->dd.d_dev; | dev.dd.d_dev = pa->dev->dd.d_dev; | ||||
dev.dd.d_unit = pa->dev->dd.d_unit; | dev.dd.d_unit = pa->dev->dd.d_unit; | ||||
dev.d_slice = part->index; | dev.d_slice = part->index; | ||||
dev.d_partition = -1; | dev.d_partition = D_PARTNONE; | ||||
if (disk_open(&dev, partsize, sectsize) == 0) { | if (disk_open(&dev, partsize, sectsize) == 0) { | ||||
/* | |||||
* disk_open() for partition -1 on a bsd slice assumes | |||||
* you want the first bsd partition. Reset things so | |||||
* that we're looking at the start of the raw slice. | |||||
*/ | |||||
dev.d_partition = -1; | |||||
dev.d_offset = part->start; | |||||
table = ptable_open(&dev, partsize, sectsize, ptblread); | table = ptable_open(&dev, partsize, sectsize, ptblread); | ||||
if (table != NULL) { | if (table != NULL) { | ||||
sprintf(line, " %s%s", pa->prefix, pname); | sprintf(line, " %s%s", pa->prefix, pname); | ||||
bsd.dev = pa->dev; | bsd.dev = pa->dev; | ||||
tsoome: We did disk_open() and should be using handle from it. pa->dev is working here only because we… | |||||
bsd.prefix = line; | bsd.prefix = line; | ||||
bsd.verbose = pa->verbose; | bsd.verbose = pa->verbose; | ||||
res = ptable_iterate(table, &bsd, ptable_print); | res = ptable_iterate(table, &bsd, ptable_print); | ||||
ptable_close(table); | ptable_close(table); | ||||
} | } | ||||
disk_close(&dev); | disk_close(&dev); | ||||
} | } | ||||
} | } | ||||
▲ Show 20 Lines • Show All 85 Lines • ▼ Show 20 Lines | disk_open(struct disk_devdesc *dev, uint64_t mediasize, u_int sectorsize) | ||||
od->entrysize = 0; | od->entrysize = 0; | ||||
od->mediasize = mediasize; | od->mediasize = mediasize; | ||||
od->sectorsize = sectorsize; | od->sectorsize = sectorsize; | ||||
/* | /* | ||||
* While we are reading disk metadata, make sure we do it relative | * While we are reading disk metadata, make sure we do it relative | ||||
* to the start of the disk | * to the start of the disk | ||||
*/ | */ | ||||
memcpy(&partdev, dev, sizeof(partdev)); | memcpy(&partdev, dev, sizeof(partdev)); | ||||
partdev.d_offset = 0; | partdev.d_offset = 0; | ||||
partdev.d_slice = -1; | partdev.d_slice = D_SLICENONE; | ||||
Not Done Inline ActionsActually this whole partdev variable is not needed. We only need to save the original slice and partition values (which we do anyhow). On successful disk_open() we end up dev having the correct slice and partition set, and d_offset pointing to either disk or partition/slice start. tsoome: Actually this whole partdev variable is not needed. We only need to save the original slice and… | |||||
partdev.d_partition = -1; | partdev.d_partition = D_PARTNONE; | ||||
dev->d_offset = 0; | dev->d_offset = 0; | ||||
table = NULL; | table = NULL; | ||||
slice = dev->d_slice; | slice = dev->d_slice; | ||||
partition = dev->d_partition; | partition = dev->d_partition; | ||||
DEBUG("%s unit %d, slice %d, partition %d => %p", | DEBUG("%s unit %d, slice %d, partition %d => %p", | ||||
disk_fmtdev(dev), dev->dd.d_unit, dev->d_slice, dev->d_partition, od); | disk_fmtdev(dev), dev->dd.d_unit, dev->d_slice, dev->d_partition, od); | ||||
/* Determine disk layout. */ | /* Determine disk layout. */ | ||||
od->table = ptable_open(&partdev, mediasize / sectorsize, sectorsize, | od->table = ptable_open(&partdev, mediasize / sectorsize, sectorsize, | ||||
ptblread); | ptblread); | ||||
if (od->table == NULL) { | if (od->table == NULL) { | ||||
DEBUG("Can't read partition table"); | DEBUG("Can't read partition table"); | ||||
rc = ENXIO; | rc = ENXIO; | ||||
goto out; | goto out; | ||||
} | } | ||||
if (ptable_getsize(od->table, &mediasize) != 0) { | if (ptable_getsize(od->table, &mediasize) != 0) { | ||||
rc = ENXIO; | rc = ENXIO; | ||||
goto out; | goto out; | ||||
} | } | ||||
od->mediasize = mediasize; | od->mediasize = mediasize; | ||||
if (ptable_gettype(od->table) == PTABLE_BSD && | if (ptable_gettype(od->table) == PTABLE_BSD && | ||||
Not Done Inline ActionsThere and at line 277 we still continue to use the original hack and we do not check for the D_PARTWILD. Perhaps something like this will give better overview what we want to see and will avoid getting lost in the if statement spaghetti: switch (ptable_gettype(od->table)) { if (partition == D_PARTWILD) .. else .. break; case PTABLE_ISO9660: ... break; case PTABLE_GPT; ... break; case PTABLE_MBR: ... } tsoome: There and at line 277 we still continue to use the original hack and we do not check for the… | |||||
Done Inline ActionsI specifically didn't change anything in disk_open() because I don't want to change the logic and change all the names at the same time. I'd rather commit the number->name changes and then commit a rewrite of disk_open() along the lines you suggest here in a week or so as a separate thing. That way if something goes wrong we don't have to guess whether it was a bad number->name change somewhere or bad logic in the disk_open() rewrite. ian: I specifically didn't change anything in disk_open() because I don't want to change the logic… | |||||
partition >= 0) { | partition >= 0) { | ||||
/* It doesn't matter what value has d_slice */ | /* It doesn't matter what value has d_slice */ | ||||
rc = ptable_getpart(od->table, &part, partition); | rc = ptable_getpart(od->table, &part, partition); | ||||
if (rc == 0) { | if (rc == 0) { | ||||
dev->d_offset = part.start; | dev->d_offset = part.start; | ||||
od->entrysize = part.end - part.start + 1; | od->entrysize = part.end - part.start + 1; | ||||
} | } | ||||
} else if (ptable_gettype(od->table) == PTABLE_ISO9660) { | } else if (ptable_gettype(od->table) == PTABLE_ISO9660) { | ||||
▲ Show 20 Lines • Show All 86 Lines • ▼ Show 20 Lines | |||||
char* | char* | ||||
disk_fmtdev(struct disk_devdesc *dev) | disk_fmtdev(struct disk_devdesc *dev) | ||||
{ | { | ||||
static char buf[128]; | static char buf[128]; | ||||
char *cp; | char *cp; | ||||
cp = buf + sprintf(buf, "%s%d", dev->dd.d_dev->dv_name, dev->dd.d_unit); | cp = buf + sprintf(buf, "%s%d", dev->dd.d_dev->dv_name, dev->dd.d_unit); | ||||
if (dev->d_slice >= 0) { | if (dev->d_slice > D_SLICENONE) { | ||||
Not Done Inline ActionsSuch constructs are now bad; it depends on specific value setup and is assuming particular ordering. If someone will change the values, all the related code will suddenly be wrong. tsoome: Such constructs are now bad; it depends on specific value setup and is assuming particular… | |||||
Not Done Inline ActionsSame as disk_open(), I'd rather leave the logic changes for the next round. ian: Same as disk_open(), I'd rather leave the logic changes for the next round. | |||||
#ifdef LOADER_GPT_SUPPORT | #ifdef LOADER_GPT_SUPPORT | ||||
if (dev->d_partition == 255) { | if (dev->d_partition == D_PARTISGPT) { | ||||
sprintf(cp, "p%d:", dev->d_slice); | sprintf(cp, "p%d:", dev->d_slice); | ||||
return (buf); | return (buf); | ||||
} else | } else | ||||
#endif | #endif | ||||
#ifdef LOADER_MBR_SUPPORT | #ifdef LOADER_MBR_SUPPORT | ||||
cp += sprintf(cp, "s%d", dev->d_slice); | cp += sprintf(cp, "s%d", dev->d_slice); | ||||
#endif | #endif | ||||
} | } | ||||
if (dev->d_partition >= 0) | if (dev->d_partition > D_PARTNONE) | ||||
cp += sprintf(cp, "%c", dev->d_partition + 'a'); | cp += sprintf(cp, "%c", dev->d_partition + 'a'); | ||||
strcat(cp, ":"); | strcat(cp, ":"); | ||||
return (buf); | return (buf); | ||||
} | } | ||||
int | int | ||||
disk_parsedev(struct disk_devdesc *dev, const char *devspec, const char **path) | disk_parsedev(struct disk_devdesc *dev, const char *devspec, const char **path) | ||||
{ | { | ||||
int unit, slice, partition; | int unit, slice, partition; | ||||
const char *np; | const char *np; | ||||
char *cp; | char *cp; | ||||
np = devspec; | np = devspec; | ||||
unit = slice = partition = -1; | unit = -1; | ||||
slice = D_SLICEWILD; | |||||
Not Done Inline ActionsD_SLICENONE tsoome: D_SLICENONE | |||||
Not Done Inline ActionsGood catch, I missed this one somehow. But, it looks like what we actually want is D_SLICEWILD, which is a behavior change, but a sensible one. I would expect "ls disk0:" to work and find the best slice and partition, which it will only do with D_SLICEWILD. This is the one place where I do think we should change the logic by using WILD instead of NONE. ian: Good catch, I missed this one somehow. But, it looks like what we actually want is D_SLICEWILD… | |||||
partition = D_PARTWILD; | |||||
if (*np != '\0' && *np != ':') { | if (*np != '\0' && *np != ':') { | ||||
unit = strtol(np, &cp, 10); | unit = strtol(np, &cp, 10); | ||||
if (cp == np) | if (cp == np) | ||||
return (EUNIT); | return (EUNIT); | ||||
#ifdef LOADER_GPT_SUPPORT | #ifdef LOADER_GPT_SUPPORT | ||||
if (*cp == 'p') { | if (*cp == 'p') { | ||||
np = cp + 1; | np = cp + 1; | ||||
slice = strtol(np, &cp, 10); | slice = strtol(np, &cp, 10); | ||||
Show All 34 Lines |
We did disk_open() and should be using handle from it. pa->dev is working here only because we *usually* do not have second level of nesting and we only do use sectorsize from this structure.