On Mon, Nov 8, 2010 at 6:58 AM, John Baldwin <j...@freebsd.org> wrote:
> On Saturday, November 06, 2010 4:13:23 am Garrett Cooper wrote:
>> Similar to r214396, this patch deals with properly capturing error
>> and passing it up to the caller in mptutil just in case the errno
>> value gets stomped on by warn*(3); this patch deals with an improper
>> use of warn(3), and also some malloc(3) errors, as well as shrink down
>> some static buffers to fit the data being output.
>> If someone could review and help me commit this patch it would be
>> much appreciated; all I could do is run negative tests on my local box
>> and minor positive tests on my vmware fusion instance because it
>> doesn't fully emulate a fully working mpt(4) device (the vmware
>> instance consistently crashed with a warning about the mpt
>> controller's unimplemented features after I poked at it enough).
>> I'll submit another patch to fix up style(9) in this app if requested.
>> Thanks!
>
> The explicit 'return (ENOMEM)' calls are fine as-is. I do not think they need
> changing.
I'll changed back all of the other *alloc calls (I was just
thinking about all of the other awesome cases that malloc can fail,
but if we break that assumption a lot more of our programs would break
too unfortunately :(...).
I read up on cam_getccb and I agree based on the description today:
"cam_getccb() allocates a CCB using malloc(3) and sets fields in the CCB
header using values from the cam_device structure."
But if it called something else in cam_getccb other than malloc it
might confuse the end-user if it failed *shrugs*... But whatever, that
would have to be a wholesale change if things changed in this area so
I'll default to being lazy now :) (I'm almost positive Scott did that
because he understands the overall system a lot better than me :D..).
> Having static char arrays of '15' rather than '16' is probably pointless. The
> stack is already at least 4-byte aligned on all the architectures we support,
> so a 15-byte char array will actually be 16 bytes. It was chose to be a good
> enough value, not an exact fit. An exact fit is not important here.
Yeah, that was stupid micro-optimization on my part.
> Moving the 'buf' in mpt_raid_level() is a style bug. It should stay where it
> is. Same with 'buf' in mpt_volstate() and mpt_pdstate().
Same as above.
> IOC_STATUS_SUCCESS() returns a boolean, it is appropriate to test it with !
> rather than == 0. It is also easier for a person to read the code that way.
Good point. I didn't catch that part in style(9) before now.
Thanks for the review!
-Garrett
Index: usr.sbin/mptutil/mpt_evt.c
===================================================================
--- usr.sbin/mptutil/mpt_evt.c (revision 214907)
+++ usr.sbin/mptutil/mpt_evt.c (working copy)
@@ -94,18 +94,20 @@
{
CONFIG_PAGE_LOG_0 *log;
MPI_LOG_0_ENTRY **entries;
- int ch, fd, i, num_events, verbose;
+ int ch, error, fd, i, num_events, verbose;
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
log = mpt_get_events(fd, NULL);
if (log == NULL) {
+ error = errno;
warn("Failed to get event log info");
- return (errno);
+ return (error);
}
/* Default settings. */
@@ -128,6 +130,8 @@
/* Build a list of valid entries and sort them by sequence. */
entries = malloc(sizeof(MPI_LOG_0_ENTRY *) * log->NumLogEntries);
+ if (entries == NULL)
+ return (ENOMEM);
num_events = 0;
for (i = 0; i < log->NumLogEntries; i++) {
if (log->LogEntry[i].LogEntryQualifier ==
Index: usr.sbin/mptutil/mpt_cam.c
===================================================================
--- usr.sbin/mptutil/mpt_cam.c (revision 214907)
+++ usr.sbin/mptutil/mpt_cam.c (working copy)
@@ -63,6 +63,7 @@
struct bus_match_pattern *b;
union ccb ccb;
size_t bufsize;
+ int error;
if (xpt_open() < 0)
return (ENXIO);
@@ -91,9 +92,10 @@
b->flags = BUS_MATCH_NAME | BUS_MATCH_UNIT | BUS_MATCH_BUS_ID;
if (ioctl(xptfd, CAMIOCOMMAND, &ccb) < 0) {
+ error = errno;
free(ccb.cdm.matches);
free(ccb.cdm.patterns);
- return (errno);
+ return (error);
}
free(ccb.cdm.patterns);
@@ -124,7 +126,7 @@
union ccb ccb;
path_id_t path_id;
size_t bufsize;
- int error, i;
+ int error;
/* mpt(4) only handles devices on bus 0. */
if (VolumeBus != 0)
@@ -164,10 +166,10 @@
p->flags = PERIPH_MATCH_PATH | PERIPH_MATCH_NAME | PERIPH_MATCH_TARGET;
if (ioctl(xptfd, CAMIOCOMMAND, &ccb) < 0) {
- i = errno;
+ error = errno;
free(ccb.cdm.matches);
free(ccb.cdm.patterns);
- return (i);
+ return (error);
}
free(ccb.cdm.patterns);
@@ -397,8 +399,8 @@
union ccb ccb;
path_id_t path_id;
size_t bufsize;
- u_int i;
int count, error;
+ uint32_t i;
if (xpt_open() < 0)
return (ENXIO);
@@ -431,10 +433,10 @@
p->flags = PERIPH_MATCH_PATH | PERIPH_MATCH_NAME;
if (ioctl(xptfd, CAMIOCOMMAND, &ccb) < 0) {
- i = errno;
+ error = errno;
free(ccb.cdm.matches);
free(ccb.cdm.patterns);
- return (i);
+ return (error);
}
free(ccb.cdm.patterns);
@@ -481,6 +483,8 @@
* exclude them from the list.
*/
ioc2 = mpt_read_ioc_page(fd, 2, NULL);
+ if (ioc2 == NULL)
+ return (errno);
disks = calloc(ccb.cdm.num_matches, sizeof(*disks));
count = 0;
for (i = 0; i < ccb.cdm.num_matches; i++) {
Index: usr.sbin/mptutil/mpt_show.c
===================================================================
--- usr.sbin/mptutil/mpt_show.c (revision 214907)
+++ usr.sbin/mptutil/mpt_show.c (working copy)
@@ -79,7 +79,7 @@
CONFIG_PAGE_IOC_2 *ioc2;
CONFIG_PAGE_IOC_6 *ioc6;
U16 IOCStatus;
- int fd, comma;
+ int comma, error, fd;
if (ac != 1) {
warnx("show adapter: extra arguments");
@@ -88,17 +88,19 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
man0 = mpt_read_man_page(fd, 0, NULL);
if (man0 == NULL) {
+ error = errno;
warn("Failed to get controller info");
- return (errno);
+ return (error);
}
if (man0->Header.PageLength < sizeof(*man0) / 4) {
- warn("Invalid controller info");
+ warnx("Invalid controller info");
return (EINVAL);
}
printf("mpt%d Adapter:\n", mpt_unit);
@@ -283,7 +285,7 @@
CONFIG_PAGE_RAID_VOL_1 *vnames;
CONFIG_PAGE_RAID_PHYS_DISK_0 *pinfo;
struct mpt_standalone_disk *sdisks;
- int fd, i, j, nsdisks;
+ int error, fd, i, j, nsdisks;
if (ac != 1) {
warnx("show config: extra arguments");
@@ -292,20 +294,23 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
/* Get the config from the controller. */
ioc2 = mpt_read_ioc_page(fd, 2, NULL);
ioc5 = mpt_read_ioc_page(fd, 5, NULL);
if (ioc2 == NULL || ioc5 == NULL) {
+ error = errno;
warn("Failed to get config");
- return (errno);
+ return (error);
}
if (mpt_fetch_disks(fd, &nsdisks, &sdisks) < 0) {
+ error = errno;
warn("Failed to get standalone drive list");
- return (errno);
+ return (error);
}
/* Dump out the configuration. */
@@ -381,7 +386,7 @@
CONFIG_PAGE_IOC_2_RAID_VOL *vol;
CONFIG_PAGE_RAID_VOL_0 **volumes;
CONFIG_PAGE_RAID_VOL_1 *vnames;
- int fd, i, len, state_len;
+ int error, fd, i, len, state_len;
if (ac != 1) {
warnx("show volumes: extra arguments");
@@ -390,15 +395,17 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
/* Get the volume list from the controller. */
ioc2 = mpt_read_ioc_page(fd, 2, NULL);
if (ioc2 == NULL) {
+ error = errno;
warn("Failed to get volume list");
- return (errno);
+ return (error);
}
/*
@@ -466,7 +473,7 @@
{
struct mpt_drive_list *list;
struct mpt_standalone_disk *sdisks;
- int fd, i, len, nsdisks, state_len;
+ int error, fd, i, len, nsdisks, state_len;
if (ac != 1) {
warnx("show drives: extra arguments");
@@ -475,15 +482,17 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
/* Get the drive list. */
list = mpt_pd_list(fd);
if (list == NULL) {
+ error = errno;
warn("Failed to get drive list");
- return (errno);
+ return (error);
}
/* Fetch the list of standalone disks for this controller. */
@@ -538,8 +547,9 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
/* Try to find each possible phys disk page. */
Index: usr.sbin/mptutil/mpt_cmd.c
===================================================================
--- usr.sbin/mptutil/mpt_cmd.c (revision 214907)
+++ usr.sbin/mptutil/mpt_cmd.c (working copy)
@@ -310,18 +310,15 @@
id = strtol(cp + 1, &cp, 0);
if (*cp == '\0') {
if (bus < 0 || bus > 0xff || id < 0 || id > 0xff) {
- errno = EINVAL;
- return (-1);
+ return (EINVAL);
}
*VolumeBus = bus;
*VolumeID = id;
return (0);
}
} else if (*cp == '\0') {
- if (bus < 0 || bus > 0xff) {
- errno = EINVAL;
- return (-1);
- }
+ if (bus < 0 || bus > 0xff)
+ return (EINVAL);
*VolumeBus = 0;
*VolumeID = bus;
return (0);
@@ -329,7 +326,7 @@
ioc2 = mpt_read_ioc_page(fd, 2, NULL);
if (ioc2 == NULL)
- return (-1);
+ return (errno);
vol = ioc2->RaidVolume;
for (i = 0; i < ioc2->NumActiveVolumes; vol++, i++) {
@@ -343,8 +340,7 @@
}
}
free(ioc2);
- errno = EINVAL;
- return (-1);
+ return (EINVAL);
}
int
@@ -360,15 +356,14 @@
req.header.PageNumber = PageNumber;
req.page_address = PageAddress;
if (ioctl(fd, MPTIO_READ_CFG_HEADER, &req) < 0)
- return (-1);
+ return (errno);
if (!IOC_STATUS_SUCCESS(req.ioc_status)) {
if (IOCStatus != NULL)
*IOCStatus = req.ioc_status;
else
warnx("Reading config page header failed: %s",
mpt_ioc_status(req.ioc_status));
- errno = EIO;
- return (-1);
+ return (EIO);
}
*header = req.header;
return (0);
@@ -380,7 +375,7 @@
{
struct mpt_cfg_page_req req;
void *buf;
- int save_errno;
+ int error;
if (IOCStatus != NULL)
*IOCStatus = MPI_IOCSTATUS_SUCCESS;
@@ -404,9 +399,9 @@
req.buf = buf;
bcopy(&req.header, buf, sizeof(req.header));
if (ioctl(fd, MPTIO_READ_CFG_PAGE, &req) < 0) {
- save_errno = errno;
+ error = errno;
free(buf);
- errno = save_errno;
+ errno = error;
return (NULL);
}
if (!IOC_STATUS_SUCCESS(req.ioc_status)) {
@@ -428,7 +423,7 @@
{
struct mpt_ext_cfg_page_req req;
void *buf;
- int save_errno;
+ int error;
if (IOCStatus != NULL)
*IOCStatus = MPI_IOCSTATUS_SUCCESS;
@@ -453,9 +448,9 @@
req.buf = buf;
bcopy(&req.header, buf, sizeof(req.header));
if (ioctl(fd, MPTIO_READ_EXT_CFG_PAGE, &req) < 0) {
- save_errno = errno;
+ error = errno;
free(buf);
- errno = save_errno;
+ errno = error;
return (NULL);
}
if (!IOC_STATUS_SUCCESS(req.ioc_status)) {
@@ -484,7 +479,7 @@
hdr = buf;
req.len = hdr->PageLength * 4;
if (ioctl(fd, MPTIO_WRITE_CFG_PAGE, &req) < 0)
- return (-1);
+ return (errno);
if (!IOC_STATUS_SUCCESS(req.ioc_status)) {
if (IOCStatus != NULL) {
*IOCStatus = req.ioc_status;
@@ -492,8 +487,7 @@
}
warnx("Writing config page failed: %s",
mpt_ioc_status(req.ioc_status));
- errno = EIO;
- return (-1);
+ return (EIO);
}
return (0);
}
@@ -507,10 +501,8 @@
if (IOCStatus != NULL)
*IOCStatus = MPI_IOCSTATUS_SUCCESS;
- if (datalen < 0 || (unsigned)datalen > sizeof(raid_act.action_data)) {
- errno = EINVAL;
- return (-1);
- }
+ if (datalen < 0 || (unsigned)datalen > sizeof(raid_act.action_data))
+ return (EINVAL);
bzero(&raid_act, sizeof(raid_act));
raid_act.action = Action;
raid_act.volume_bus = VolumeBus;
@@ -524,7 +516,7 @@
}
if (ioctl(fd, MPTIO_RAID_ACTION, &raid_act) < 0)
- return (-1);
+ return (errno);
if (!IOC_STATUS_SUCCESS(raid_act.ioc_status)) {
if (IOCStatus != NULL) {
@@ -533,8 +525,7 @@
}
warnx("RAID action failed: %s",
mpt_ioc_status(raid_act.ioc_status));
- errno = EIO;
- return (-1);
+ return (EIO);
}
if (ActionStatus != NULL)
@@ -544,8 +535,7 @@
return (0);
warnx("RAID action failed: %s",
mpt_raid_status(raid_act.action_status));
- errno = EIO;
- return (-1);
+ return (EIO);
}
if (VolumeStatus != NULL)
Index: usr.sbin/mptutil/mpt_config.c
===================================================================
--- usr.sbin/mptutil/mpt_config.c (revision 214907)
+++ usr.sbin/mptutil/mpt_config.c (working copy)
@@ -104,13 +104,14 @@
if (error) {
errno = error;
warn("Unable to lookup volume device name");
- return (-1);
+ return (error);
}
snprintf(path, sizeof(path), "%s%s", _PATH_DEV, qd.devname);
vfd = open(path, O_RDWR);
if (vfd < 0) {
+ error = errno;
warn("Unable to lock volume %s", qd.devname);
- return (-1);
+ return (error);
}
return (0);
}
@@ -119,13 +120,14 @@
mpt_lock_physdisk(struct mpt_standalone_disk *disk)
{
char path[MAXPATHLEN];
- int dfd;
+ int dfd, error;
snprintf(path, sizeof(path), "%s%s", _PATH_DEV, disk->devname);
dfd = open(path, O_RDWR);
if (dfd < 0) {
+ error = errno;
warn("Unable to lock disk %s", disk->devname);
- return (-1);
+ return (error);
}
return (0);
}
@@ -144,8 +146,7 @@
id = strtol(cp + 1, &cp, 0);
if (*cp == '\0') {
if (bus < 0 || bus > 0xff || id < 0 || id > 0xff) {
- errno = EINVAL;
- return (-1);
+ return (EINVAL);
}
for (i = 0; i < ndisks; i++) {
if (disks[i].bus == (U8)bus &&
@@ -154,8 +155,7 @@
return (0);
}
}
- errno = ENOENT;
- return (-1);
+ return (ENOENT);
}
}
@@ -166,12 +166,10 @@
return (0);
}
}
- errno = ENOENT;
- return (-1);
+ return (ENOENT);
}
- errno = EINVAL;
- return (-1);
+ return (EINVAL);
}
/*
@@ -182,16 +180,17 @@
{
CONFIG_PAGE_HEADER header;
CONFIG_PAGE_RAID_PHYS_DISK_0 *config_page;
+ int error;
U32 ActionData;
- if (mpt_read_config_page_header(fd, MPI_CONFIG_PAGETYPE_RAID_PHYSDISK,
- 0, 0, &header, NULL) < 0)
- return (-1);
+ error = mpt_read_config_page_header(fd, MPI_CONFIG_PAGETYPE_RAID_PHYSDISK,
+ 0, 0, &header, NULL);
+ if (error)
+ return (error);
if (header.PageVersion > MPI_RAIDPHYSDISKPAGE0_PAGEVERSION) {
warnx("Unsupported RAID physdisk page 0 version %d",
header.PageVersion);
- errno = EOPNOTSUPP;
- return (-1);
+ return (EOPNOTSUPP);
}
config_page = calloc(1, sizeof(CONFIG_PAGE_RAID_PHYS_DISK_0));
config_page->Header.PageType = MPI_CONFIG_PAGETYPE_RAID_PHYSDISK;
@@ -203,10 +202,11 @@
config_page->PhysDiskID = disk->target;
/* XXX: Enclosure info for PhysDiskSettings? */
- if (mpt_raid_action(fd, MPI_RAID_ACTION_CREATE_PHYSDISK, 0, 0, 0, 0,
+ error = mpt_raid_action(fd, MPI_RAID_ACTION_CREATE_PHYSDISK, 0, 0, 0, 0,
config_page, sizeof(CONFIG_PAGE_RAID_PHYS_DISK_0), NULL,
- &ActionData, sizeof(ActionData), NULL, NULL, 1) < 0)
- return (-1);
+ &ActionData, sizeof(ActionData), NULL, NULL, 1);
+ if (error)
+ return (error);
*PhysDiskNum = ActionData & 0xff;
return (0);
}
@@ -232,18 +232,20 @@
IOC_3_PHYS_DISK *disk;
CONFIG_PAGE_IOC_5 *ioc5;
IOC_5_HOT_SPARE *spare;
- int ch, fd, i;
+ int ch, error, fd, i;
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
ioc2 = mpt_read_ioc_page(fd, 2, NULL);
if (ioc2 == NULL) {
+ error = errno;
warn("Failed to fetch volume list");
- return (errno);
+ return (error);
}
/* Lock all the volumes first. */
@@ -268,13 +270,16 @@
/* Delete all the volumes. */
vol = ioc2->RaidVolume;
for (i = 0; i < ioc2->NumActiveVolumes; vol++, i++)
- if (mpt_raid_action(fd, MPI_RAID_ACTION_DELETE_VOLUME,
+ error = mpt_raid_action(fd, MPI_RAID_ACTION_DELETE_VOLUME,
vol->VolumeBus, vol->VolumeID, 0,
MPI_RAID_ACTION_ADATA_DEL_PHYS_DISKS |
MPI_RAID_ACTION_ADATA_ZERO_LBA0, NULL, 0, NULL, NULL, 0,
- NULL, NULL, 0) < 0)
+ NULL, NULL, 0);
+ if (error) {
+ errno = error;
warn("Failed to delete volume %s",
mpt_volume_name(vol->VolumeBus, vol->VolumeID));
+ }
free(ioc2);
/* Delete all the spares. */
@@ -411,8 +416,9 @@
/* See if it is a standalone disk. */
if (mpt_lookup_standalone_disk(cp, state->sdisks,
state->nsdisks, &i) < 0) {
+ error = errno;
warn("Unable to lookup drive %s", cp);
- return (errno);
+ return (error);
}
dinfo->sdisk = &state->sdisks[i];
@@ -433,17 +439,18 @@
{
struct drive_info *dinfo;
U8 PhysDiskNum;
- int i;
+ int error, i;
for (i = 0, dinfo = info->drives; i < info->drive_count;
i++, dinfo++) {
if (dinfo->info == NULL) {
if (mpt_create_physdisk(fd, dinfo->sdisk,
&PhysDiskNum) < 0) {
+ error = errno;
warn(
"Failed to create physical disk page for %s",
dinfo->sdisk->devname);
- return (errno);
+ return (error);
}
if (verbose)
printf("Added drive %s with PhysDiskNum %u\n",
@@ -500,11 +507,14 @@
U32 MinLBA;
uint64_t MaxLBA;
size_t page_size;
- int i;
+ int error, i;
- if (mpt_read_config_page_header(fd, MPI_CONFIG_PAGETYPE_RAID_VOLUME,
- 0, 0, &header, NULL) < 0)
+ error = mpt_read_config_page_header(fd, MPI_CONFIG_PAGETYPE_RAID_VOLUME,
+ 0, 0, &header, NULL);
+ if (error) {
+ errno = error;
return (NULL);
+ }
if (header.PageVersion > MPI_RAIDVOLPAGE0_PAGEVERSION) {
warnx("Unsupported RAID volume page 0 version %d",
header.PageVersion);
@@ -514,6 +524,8 @@
page_size = sizeof(CONFIG_PAGE_RAID_VOL_0) +
sizeof(RAID_VOL0_PHYS_DISK) * (info->drive_count - 1);
vol = calloc(1, page_size);
+ if (vol == NULL)
+ return (NULL);
/* Header */
vol->Header.PageType = MPI_CONFIG_PAGETYPE_RAID_VOLUME;
@@ -607,8 +619,8 @@
CONFIG_PAGE_RAID_VOL_0 *vol;
struct config_id_state state;
struct volume_info *info;
- int ch, error, fd, i, raid_type, verbose, quick;
long stripe_size;
+ int ch, error, fd, i, quick, raid_type, verbose;
#ifdef DEBUG
int dump;
#endif
@@ -620,8 +632,9 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
/* Lookup the RAID type first. */
@@ -677,8 +690,9 @@
/* Fetch existing config data. */
state.ioc2 = mpt_read_ioc_page(fd, 2, NULL);
if (state.ioc2 == NULL) {
+ error = errno;
warn("Failed to read volume list");
- return (errno);
+ return (error);
}
state.list = mpt_pd_list(fd);
if (state.list == NULL)
@@ -696,6 +710,8 @@
return (EINVAL);
}
info = calloc(1, sizeof(*info));
+ if (info == NULL)
+ return (ENOMEM);
error = parse_volume(fd, raid_type, &state, av[0], info);
if (error)
return (error);
@@ -707,6 +723,8 @@
/* Build the volume. */
vol = build_volume(fd, info, raid_type, stripe_size, &state, verbose);
+ if (vol == NULL)
+ return (errno);
#ifdef DEBUG
if (dump) {
@@ -716,12 +734,13 @@
#endif
/* Send the new volume to the controller. */
- if (mpt_raid_action(fd, MPI_RAID_ACTION_CREATE_VOLUME, vol->VolumeBus,
+ error = mpt_raid_action(fd, MPI_RAID_ACTION_CREATE_VOLUME, vol->VolumeBus,
vol->VolumeID, 0, quick ? MPI_RAID_ACTION_ADATA_DO_NOT_SYNC : 0,
- vol, vol->Header.PageLength * 4, NULL, NULL, 0, NULL, NULL, 1) <
- 0) {
+ vol, vol->Header.PageLength * 4, NULL, NULL, 0, NULL, NULL, 1);
+ if (error) {
+ errno = error;
warn("Failed to add volume");
- return (errno);
+ return (error);
}
#ifdef DEBUG
@@ -745,7 +764,7 @@
delete_volume(int ac, char **av)
{
U8 VolumeBus, VolumeID;
- int fd;
+ int error, fd;
if (ac != 2) {
warnx("delete: volume required");
@@ -754,24 +773,29 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
- if (mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID) < 0) {
+ error = mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID);
+ if (error) {
+ errno = error;
warn("Invalid volume %s", av[1]);
- return (errno);
+ return (error);
}
if (mpt_lock_volume(VolumeBus, VolumeID) < 0)
return (errno);
- if (mpt_raid_action(fd, MPI_RAID_ACTION_DELETE_VOLUME, VolumeBus,
+ error = mpt_raid_action(fd, MPI_RAID_ACTION_DELETE_VOLUME, VolumeBus,
VolumeID, 0, MPI_RAID_ACTION_ADATA_DEL_PHYS_DISKS |
MPI_RAID_ACTION_ADATA_ZERO_LBA0, NULL, 0, NULL, NULL, 0, NULL,
- NULL, 0) < 0) {
+ NULL, 0);
+ if (error) {
+ errno = error;
warn("Failed to delete volume");
- return (errno);
+ return (error);
}
mpt_rescan_bus(-1, -1);
@@ -788,16 +812,18 @@
CONFIG_PAGE_IOC_2 *ioc2;
CONFIG_PAGE_IOC_2_RAID_VOL *vol;
U8 VolumeBus, VolumeID;
- int i, j, new_pool, pool_count[7];
+ int error, i, j, new_pool, pool_count[7];
- if (mpt_lookup_volume(fd, name, &VolumeBus, &VolumeID) < 0) {
+ error = mpt_lookup_volume(fd, name, &VolumeBus, &VolumeID);
+ if (error) {
+ errno = error;
warn("Invalid volume %s", name);
- return (-1);
+ return (error);
}
info = mpt_vol_info(fd, VolumeBus, VolumeID, NULL);
if (info == NULL)
- return (-1);
+ return (errno);
/*
* Check for an existing pool other than pool 0 (used for
@@ -817,15 +843,16 @@
*/
ioc2 = mpt_read_ioc_page(fd, 2, NULL);
if (ioc2 == NULL) {
+ error = errno;
warn("Failed to fetch volume list");
- return (-1);
+ return (error);
}
bzero(pool_count, sizeof(pool_count));
vol = ioc2->RaidVolume;
for (i = 0; i < ioc2->NumActiveVolumes; vol++, i++) {
info = mpt_vol_info(fd, vol->VolumeBus, vol->VolumeID, NULL);
if (info == NULL)
- return (-1);
+ return (errno);
for (j = 0; j < 7; j++)
if (info->VolumeSettings.HotSparePool & (1 << (j + 1)))
pool_count[j]++;
@@ -843,14 +870,15 @@
/* Add this pool to the volume. */
info = mpt_vol_info(fd, VolumeBus, VolumeID, NULL);
if (info == NULL)
- return (-1);
+ return (error);
info->VolumeSettings.HotSparePool |= (1 << new_pool);
- if (mpt_raid_action(fd, MPI_RAID_ACTION_CHANGE_VOLUME_SETTINGS,
+ error = mpt_raid_action(fd, MPI_RAID_ACTION_CHANGE_VOLUME_SETTINGS,
VolumeBus, VolumeID, 0, *(U32 *)&info->VolumeSettings, NULL, 0,
- NULL, NULL, 0, NULL, NULL, 0) < 0) {
+ NULL, NULL, 0, NULL, NULL, 0);
+ if (error) {
warnx("Failed to add spare pool %d to %s", new_pool,
mpt_volume_name(VolumeBus, VolumeID));
- return (-1);
+ return (error);
}
free(info);
@@ -878,13 +906,15 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
if (ac == 3) {
- if (find_volume_spare_pool(fd, av[2], &pool) < 0)
- return (errno);
+ error = find_volume_spare_pool(fd, av[2], &pool);
+ if (error)
+ return (error);
} else
pool = MPI_RAID_HOT_SPARE_POOL_0;
@@ -902,16 +932,18 @@
if (mpt_lookup_standalone_disk(av[1], sdisks, nsdisks, &i) <
0) {
+ error = errno;
warn("Unable to lookup drive %s", av[1]);
- return (errno);
+ return (error);
}
if (mpt_lock_physdisk(&sdisks[i]) < 0)
return (errno);
if (mpt_create_physdisk(fd, &sdisks[i], &PhysDiskNum) < 0) {
+ error = errno;
warn("Failed to create physical disk page");
- return (errno);
+ return (error);
}
free(sdisks);
}
@@ -919,8 +951,9 @@
info = mpt_pd_info(fd, PhysDiskNum, NULL);
if (info == NULL) {
+ error = errno;
warn("Failed to fetch drive info");
- return (errno);
+ return (error);
}
info->PhysDiskSettings.HotSparePool = pool;
@@ -928,8 +961,9 @@
0, PhysDiskNum, *(U32 *)&info->PhysDiskSettings, NULL, 0, NULL,
NULL, 0, NULL, NULL, 0);
if (error) {
+ errno = error;
warn("Failed to assign spare");
- return (errno);
+ return (error);
}
free(info);
@@ -954,8 +988,9 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
list = mpt_pd_list(fd);
@@ -972,8 +1007,9 @@
info = mpt_pd_info(fd, PhysDiskNum, NULL);
if (info == NULL) {
+ error = errno;
warn("Failed to fetch drive info");
- return (errno);
+ return (error);
}
if (info->PhysDiskSettings.HotSparePool == 0) {
@@ -982,11 +1018,14 @@
}
if (mpt_delete_physdisk(fd, PhysDiskNum) < 0) {
+ error = errno;
warn("Failed to delete physical disk page");
- return (errno);
+ return (error);
}
- mpt_rescan_bus(info->PhysDiskBus, info->PhysDiskID);
+ error = mpt_rescan_bus(info->PhysDiskBus, info->PhysDiskID);
+ if (error)
+ return (error);
free(info);
close(fd);
@@ -1011,8 +1050,9 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
error = mpt_fetch_disks(fd, &ndisks, &disks);
@@ -1022,16 +1062,18 @@
}
if (mpt_lookup_standalone_disk(av[1], disks, ndisks, &i) < 0) {
+ error = errno;
warn("Unable to lookup drive");
- return (errno);
+ return (error);
}
if (mpt_lock_physdisk(&disks[i]) < 0)
return (errno);
if (mpt_create_physdisk(fd, &disks[i], &PhysDiskNum) < 0) {
+ error = errno;
warn("Failed to create physical disk page");
- return (errno);
+ return (error);
}
free(disks);
@@ -1048,7 +1090,7 @@
{
CONFIG_PAGE_RAID_PHYS_DISK_0 *info;
struct mpt_drive_list *list;
- int fd;
+ int error, fd;
U8 PhysDiskNum;
if (ac != 2) {
@@ -1058,8 +1100,9 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
list = mpt_pd_list(fd);
@@ -1067,23 +1110,28 @@
return (errno);
if (mpt_lookup_drive(list, av[1], &PhysDiskNum) < 0) {
+ error = errno;
warn("Failed to find drive %s", av[1]);
- return (errno);
+ return (error);
}
mpt_free_pd_list(list);
info = mpt_pd_info(fd, PhysDiskNum, NULL);
if (info == NULL) {
+ error = errno;
warn("Failed to fetch drive info");
- return (errno);
+ return (error);
}
if (mpt_delete_physdisk(fd, PhysDiskNum) < 0) {
+ error = errno;
warn("Failed to delete physical disk page");
- return (errno);
+ return (error);
}
- mpt_rescan_bus(info->PhysDiskBus, info->PhysDiskID);
+ error = mpt_rescan_bus(info->PhysDiskBus, info->PhysDiskID);
+ if (error)
+ return (error);
free(info);
close(fd);
@@ -1126,7 +1174,7 @@
{
CONFIG_PAGE_RAID_VOL_0 *vol;
U8 VolumeBus, VolumeID;
- int fd;
+ int error, fd;
if (ac != 2) {
warnx("debug: volume required");
@@ -1135,19 +1183,23 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
- if (mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID) < 0) {
+ error = mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID);
+ if (error) {
+ errno = error;
warn("Invalid volume: %s", av[1]);
- return (errno);
+ return (error);
}
vol = mpt_vol_info(fd, VolumeBus, VolumeID, NULL);
if (vol == NULL) {
+ error = errno;
warn("Failed to get volume info");
- return (errno);
+ return (error);
}
dump_config(vol);
Index: usr.sbin/mptutil/mpt_volume.c
===================================================================
--- usr.sbin/mptutil/mpt_volume.c (revision 214907)
+++ usr.sbin/mptutil/mpt_volume.c (working copy)
@@ -69,7 +69,7 @@
{
CONFIG_PAGE_RAID_VOL_1 *vnames;
U8 VolumeBus, VolumeID;
- int fd;
+ int error, fd;
if (ac != 3) {
warnx("name: volume and name required");
@@ -83,19 +83,23 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
- if (mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID) < 0) {
+ error = mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID);
+ if (error) {
+ errno = error;
warn("Invalid volume: %s", av[1]);
- return (errno);
+ return (error);
}
vnames = mpt_vol_names(fd, VolumeBus, VolumeID, NULL);
if (vnames == NULL) {
+ error = errno;
warn("Failed to fetch volume names");
- return (errno);
+ return (error);
}
if (vnames->Header.PageType != MPI_CONFIG_PAGEATTR_CHANGEABLE) {
@@ -109,8 +113,9 @@
strcpy(vnames->Name, av[2]);
if (mpt_write_config_page(fd, vnames, NULL) < 0) {
+ error = errno;
warn("Failed to set volume name");
- return (errno);
+ return (error);
}
free(vnames);
@@ -128,7 +133,7 @@
uint64_t total, remaining;
float pct;
U8 VolumeBus, VolumeID;
- int fd;
+ int error, fd;
if (ac != 2) {
warnx("volume status: %s", ac > 2 ? "extra arguments" :
@@ -138,20 +143,25 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
- if (mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID) < 0) {
+ error = mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID);
+ if (error) {
+ errno = error;
warn("Invalid volume: %s", av[1]);
- return (errno);
+ return (error);
}
- if (mpt_raid_action(fd, MPI_RAID_ACTION_INDICATOR_STRUCT, VolumeBus,
+ error = mpt_raid_action(fd, MPI_RAID_ACTION_INDICATOR_STRUCT, VolumeBus,
VolumeID, 0, 0, NULL, 0, &VolumeStatus, (U32 *)&prog, sizeof(prog),
- NULL, NULL, 0) < 0) {
+ NULL, NULL, 0);
+ if (error) {
+ errno = error;
warn("Fetching volume status failed");
- return (errno);
+ return (error);
}
printf("Volume %s status:\n", mpt_volume_name(VolumeBus, VolumeID));
@@ -191,11 +201,11 @@
U32 Settings, NewSettings;
U8 VolumeBus, VolumeID;
char *s1;
- int fd;
+ int error, fd;
if (ac != 3) {
warnx("volume cache: %s", ac > 3 ? "extra arguments" :
- "volume required");
+ "missing arguments");
return (EINVAL);
}
@@ -209,18 +219,21 @@
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
- if (mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID) < 0) {
+ error = mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID);
+ if (error) {
+ errno = error;
warn("Invalid volume: %s", av[1]);
- return (errno);
+ return (error);
}
volume = mpt_vol_info(fd, VolumeBus, VolumeID, NULL);
if (volume == NULL)
- return (-1);
+ return (errno);
Settings = volume->VolumeSettings.Settings;
@@ -231,18 +244,19 @@
NewSettings &= ~0x01;
if (NewSettings == Settings) {
- warnx("volume cache unchanged\n");
+ warnx("volume cache unchanged");
close(fd);
return (0);
}
volume->VolumeSettings.Settings = NewSettings;
- if (mpt_raid_action(fd, MPI_RAID_ACTION_CHANGE_VOLUME_SETTINGS,
+ error = mpt_raid_action(fd, MPI_RAID_ACTION_CHANGE_VOLUME_SETTINGS,
VolumeBus, VolumeID, 0, *(U32 *)&volume->VolumeSettings, NULL, 0,
- NULL, NULL, 0, NULL, NULL, 0) < 0)
- warnx("volume cache change failed, errno= %d\n", errno);
+ NULL, NULL, 0, NULL, NULL, 0);
+ if (error)
+ warn("volume cache change failed");
close(fd);
- return (0);
+ return (error);
}
MPT_COMMAND(volume, cache, volume_cache);
Index: usr.sbin/mptutil/mpt_drive.c
===================================================================
--- usr.sbin/mptutil/mpt_drive.c (revision 214907)
+++ usr.sbin/mptutil/mpt_drive.c (working copy)
@@ -129,7 +129,7 @@
list->drives[j + 1] = list->drives[j];
list->drives[i] = mpt_pd_info(fd, PhysDiskNum, NULL);
if (list->drives[i] == NULL)
- return (-1);
+ return (errno);
list->ndrives++;
return (0);
}
@@ -146,26 +146,32 @@
CONFIG_PAGE_IOC_5 *ioc5;
IOC_5_HOT_SPARE *spare;
struct mpt_drive_list *list;
- int count, i, j;
+ int count, error, i, j;
ioc2 = mpt_read_ioc_page(fd, 2, NULL);
if (ioc2 == NULL) {
+ error = errno;
warn("Failed to fetch volume list");
+ errno = error;
return (NULL);
}
ioc3 = mpt_read_ioc_page(fd, 3, NULL);
if (ioc3 == NULL) {
+ error = errno;
warn("Failed to fetch drive list");
free(ioc2);
+ errno = error;
return (NULL);
}
ioc5 = mpt_read_ioc_page(fd, 5, NULL);
if (ioc5 == NULL) {
+ error = errno;
warn("Failed to fetch spare list");
free(ioc3);
free(ioc2);
+ errno = error;
return (NULL);
}
@@ -180,7 +186,9 @@
volumes[i] = mpt_vol_info(fd, vol->VolumeBus, vol->VolumeID,
NULL);
if (volumes[i] == NULL) {
+ error = errno;
warn("Failed to read volume info");
+ errno = error;
return (NULL);
}
count += volumes[i]->NumPhysDisks;
@@ -264,13 +272,11 @@
return (0);
}
}
- errno = ENOENT;
- return (-1);
+ return (ENOENT);
}
bad:
- errno = EINVAL;
- return (-1);
+ return (EINVAL);
}
/* Borrowed heavily from scsi_all.c:scsi_print_inquiry(). */
@@ -306,12 +312,13 @@
CONFIG_PAGE_RAID_PHYS_DISK_0 *info;
struct mpt_drive_list *list;
U8 PhysDiskNum;
- int fd;
+ int error, fd;
fd = mpt_open(mpt_unit);
if (fd < 0) {
+ error = errno;
warn("mpt_open");
- return (errno);
+ return (error);
}
list = mpt_pd_list(fd);
@@ -319,16 +326,18 @@
return (errno);
if (mpt_lookup_drive(list, drive, &PhysDiskNum) < 0) {
+ error = errno;
warn("Failed to find drive %s", drive);
- return (errno);
+ return (error);
}
mpt_free_pd_list(list);
/* Get the info for this drive. */
info = mpt_pd_info(fd, PhysDiskNum, NULL);
if (info == NULL) {
+ error = errno;
warn("Failed to fetch info for drive %u", PhysDiskNum);
- return (errno);
+ return (error);
}
/* Try to change the state. */
@@ -337,10 +346,12 @@
return (EINVAL);
}
- if (mpt_raid_action(fd, Action, 0, 0, PhysDiskNum, 0, NULL, 0, NULL,
- NULL, 0, NULL, NULL, 0) < 0) {
+ error = mpt_raid_action(fd, Action, 0, 0, PhysDiskNum, 0, NULL, 0, NULL,
+ NULL, 0, NULL, NULL, 0);
+ if (error) {
+ errno = error;
warn("Failed to set drive %u to %s", PhysDiskNum, name);
- return (errno);
+ return (error);
}
free(info);
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
