Skip to content

[WIP] Fix: avoid using deprecated valloc & frequent aligned alloc #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 4, 2021
Merged

[WIP] Fix: avoid using deprecated valloc & frequent aligned alloc #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
53277ef
Fix: avoid using deprecated valloc & frequent aligned alloc
wenningerk Sep 6, 2021
b3b822c
Refactor: sbd-md: open_device, close_device
wenningerk Sep 23, 2021
8d19d37
Fix: sbd-md: properly free one-time-allocations of sector-buffers
wenningerk Sep 8, 2021
9e6cbba
Fix: sbd-md: properly destroy io-context
wenningerk Sep 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 67 additions & 26 deletions src/sbd-md.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,16 @@ char2cmd(const char cmd)
static void
close_device(struct sbd_context *st)
{
close(st->devfd);
if (!st) {
return;
}
if (st->ioctx) {
io_destroy(st->ioctx);
}
if (st->devfd >= 0) {
close(st->devfd);
}
free(st->buffer);
free(st);
}

Expand All @@ -101,15 +110,15 @@ open_device(const char* devname, int loglevel)
if (!devname)
return NULL;

st = malloc(sizeof(struct sbd_context));
if (!st)
st = calloc(1, sizeof(struct sbd_context));
if (!st) {
return NULL;
memset(st, 0, sizeof(struct sbd_context));
}
st->devfd = -1;

if (io_setup(1, &st->ioctx) != 0) {
cl_perror("io_setup failed");
free(st);
return NULL;
goto out;
}

st->devfd = open(devname, O_SYNC|O_RDWR|O_DIRECT);
Expand All @@ -120,31 +129,37 @@ open_device(const char* devname, int loglevel)
} else {
cl_log(loglevel, "Opening device %s failed.", devname);
}
free(st);
return NULL;
goto out;
}

ioctl(st->devfd, BLKSSZGET, &sector_size);

if (sector_size == 0) {
cl_perror("Get sector size failed.\n");
close_device(st);
return NULL;
goto out;
}

if (posix_memalign(&st->buffer, sector_size, sector_size)) {
cl_perror("Couldn't allocate sector-buffer.");
goto out;
}

return st;

out:
close_device(st);
return NULL;
}

static void *
sector_alloc(void)
{
void *x;

x = valloc(sector_size);
x = calloc(1, sector_size);
if (!x) {
exit(1);
}
memset(x, 0, sector_size);

return x;
}
Expand All @@ -162,9 +177,11 @@ sector_io(struct sbd_context *st, int sector, void *data, int rw)

memset(&st->io, 0, sizeof(struct iocb));
if (rw) {
io_prep_pwrite(&st->io, st->devfd, data, sector_size, (long long) sector_size * sector);
memcpy(st->buffer, data, sector_size);
io_prep_pwrite(&st->io, st->devfd, st->buffer, sector_size, (long long) sector_size * sector);
} else {
io_prep_pread(&st->io, st->devfd, data, sector_size, (long long) sector_size * sector);
memset(st->buffer, 0, sector_size);
io_prep_pread(&st->io, st->devfd, st->buffer, sector_size, (long long) sector_size * sector);
}

if (io_submit(st->ioctx, 1, ios) != 1) {
Expand All @@ -179,7 +196,7 @@ sector_io(struct sbd_context *st, int sector, void *data, int rw)
cl_log(LOG_ERR, "Failed to retrieve IO events (rw=%d)", rw);
return -1;
} else if (r < 1L) {
cl_log(LOG_INFO, "Cancelling IO request due to timeout (rw=%d)", rw);
cl_log(LOG_INFO, "Cancelling IO request due to timeout (rw=%d, r=%ld)", rw, r);
r = io_cancel(st->ioctx, ios[0], &event);
if (r) {
DBGLOG(LOG_INFO, "Could not cancel IO request (rw=%d)", rw);
Expand All @@ -195,6 +212,9 @@ sector_io(struct sbd_context *st, int sector, void *data, int rw)

/* IO is happy */
if (event.res == sector_size) {
if (!rw) {
memcpy(data, st->buffer, sector_size);
}
return 0;
} else {
cl_log(LOG_ERR, "Short IO (rw=%d, res=%lu, sector_size=%d)",
Expand Down Expand Up @@ -322,11 +342,13 @@ header_get(struct sbd_context *st)

if (header_read(st, s_header) < 0) {
cl_log(LOG_ERR, "Unable to read header from device %d", st->devfd);
free(s_header);
return NULL;
}

if (valid_header(s_header) < 0) {
cl_log(LOG_ERR, "header on device %d is not valid.", st->devfd);
free(s_header);
return NULL;
}

Expand Down Expand Up @@ -364,6 +386,8 @@ header_dump(struct sbd_context *st)
(unsigned long)s_header->timeout_loop);
printf("Timeout (msgwait) : %lu\n",
(unsigned long)s_header->timeout_msgwait);

free(s_header);
return 0;
}

Expand Down Expand Up @@ -916,6 +940,7 @@ get_first_msgwait(struct servants_list_item *servants)
if (s_header != NULL) {
msgwait = (unsigned long)s_header->timeout_msgwait;
close_device(st);
free(s_header);
return msgwait;
}

Expand Down Expand Up @@ -1081,13 +1106,15 @@ int servant_md(const char *diskname, int mode, const void* argp)
s_header = header_get(st);
if (!s_header) {
cl_log(LOG_ERR, "Not a valid header on %s", diskname);
exit(EXIT_MD_SERVANT_IO_FAIL);
rc = EXIT_MD_SERVANT_IO_FAIL;
goto out;
}

if (servant_check_timeout_inconsistent(s_header) < 0) {
cl_log(LOG_ERR, "Timeouts on %s do not match first device",
diskname);
exit(EXIT_MD_SERVANT_IO_FAIL);
rc = EXIT_MD_SERVANT_IO_FAIL;
goto out;
}

if (s_header->minor_version > 0) {
Expand All @@ -1107,7 +1134,8 @@ int servant_md(const char *diskname, int mode, const void* argp)
if (slot_read(st, mbox, s_node) < 0) {
cl_log(LOG_ERR, "Unable to read node entry on %s",
diskname);
exit(EXIT_MD_SERVANT_IO_FAIL);
rc = EXIT_MD_SERVANT_IO_FAIL;
goto out;
}

cl_log(LOG_NOTICE, "Monitoring slot %d on disk %s", mbox, diskname);
Expand Down Expand Up @@ -1168,28 +1196,36 @@ int servant_md(const char *diskname, int mode, const void* argp)
s_header_retry = header_get(st);
if (!s_header_retry) {
cl_log(LOG_ERR, "No longer found a valid header on %s", diskname);
exit(EXIT_MD_SERVANT_IO_FAIL);
rc = EXIT_MD_SERVANT_IO_FAIL;
goto out;
}
if (memcmp(s_header, s_header_retry, sizeof(*s_header)) != 0) {
cl_log(LOG_ERR, "Header on %s changed since start-up!", diskname);
exit(EXIT_MD_SERVANT_IO_FAIL);
free(s_header_retry);
rc = EXIT_MD_SERVANT_IO_FAIL;
goto out;
}
free(s_header_retry);

s_node_retry = sector_alloc();
if (slot_read(st, mbox, s_node_retry) < 0) {
cl_log(LOG_ERR, "slot read failed in servant.");
exit(EXIT_MD_SERVANT_IO_FAIL);
free(s_node_retry);
rc = EXIT_MD_SERVANT_IO_FAIL;
goto out;
}
if (memcmp(s_node, s_node_retry, sizeof(*s_node)) != 0) {
cl_log(LOG_ERR, "Node entry on %s changed since start-up!", diskname);
exit(EXIT_MD_SERVANT_IO_FAIL);
free(s_node_retry);
rc = EXIT_MD_SERVANT_IO_FAIL;
goto out;
}
free(s_node_retry);

if (mbox_read(st, mbox, s_mbox) < 0) {
cl_log(LOG_ERR, "mbox read failed in servant.");
exit(EXIT_MD_SERVANT_IO_FAIL);
rc = EXIT_MD_SERVANT_IO_FAIL;
goto out;
}

if (s_mbox->cmd > 0) {
Expand All @@ -1204,14 +1240,17 @@ int servant_md(const char *diskname, int mode, const void* argp)
sigqueue(ppid, SIG_TEST, signal_value);
break;
case SBD_MSG_RESET:
exit(EXIT_MD_SERVANT_REQUEST_RESET);
rc = EXIT_MD_SERVANT_REQUEST_RESET;
goto out;
case SBD_MSG_OFF:
exit(EXIT_MD_SERVANT_REQUEST_SHUTOFF);
rc = EXIT_MD_SERVANT_REQUEST_SHUTOFF;
goto out;
case SBD_MSG_EXIT:
sigqueue(ppid, SIG_EXITREQ, signal_value);
break;
case SBD_MSG_CRASHDUMP:
exit(EXIT_MD_SERVANT_REQUEST_CRASHDUMP);
rc = EXIT_MD_SERVANT_REQUEST_CRASHDUMP;
goto out;
default:
/* FIXME:
An "unknown" message might result
Expand Down Expand Up @@ -1240,6 +1279,8 @@ int servant_md(const char *diskname, int mode, const void* argp)
}
}
out:
free(s_header);
free(s_node);
free(s_mbox);
close_device(st);
exit(rc);
Expand Down
1 change: 1 addition & 0 deletions src/sbd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ struct sbd_context {
int devfd;
io_context_t ioctx;
struct iocb io;
void *buffer;
};

enum pcmk_health
Expand Down
16 changes: 16 additions & 0 deletions tests/sbd-testbed.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ typedef int (*orig_close_f_type)(int fd);
typedef FILE *(*orig_fopen_f_type)(const char *pathname, const char *mode);
typedef int (*orig_fclose_f_type)(FILE *fp);
typedef int (*orig_io_setup_f_type)(int nr_events, io_context_t *ctx_idp);
typedef int (*orig_io_destroy_f_type)(io_context_t ctx_id);
typedef int (*orig_io_submit_f_type)(io_context_t ctx_id, long nr, struct iocb *ios[]);
typedef int (*orig_io_getevents_f_type)(io_context_t ctx_id, long min_nr, long nr,
struct io_event *events, struct timespec *timeout);
Expand Down Expand Up @@ -93,6 +94,7 @@ static orig_close_f_type orig_close = NULL;
static orig_fopen_f_type orig_fopen = NULL;
static orig_fclose_f_type orig_fclose = NULL;
static orig_io_setup_f_type orig_io_setup = NULL;
static orig_io_destroy_f_type orig_io_destroy = NULL;
static orig_io_submit_f_type orig_io_submit = NULL;
static orig_io_getevents_f_type orig_io_getevents = NULL;
static orig_io_cancel_f_type orig_io_cancel = NULL;
Expand Down Expand Up @@ -158,6 +160,7 @@ init (void)
exit(1);
}
orig_io_setup = (orig_io_setup_f_type)dlsym_fatal(handle,"io_setup");
orig_io_destroy = (orig_io_destroy_f_type)dlsym_fatal(handle,"io_destroy");
orig_io_submit = (orig_io_submit_f_type)dlsym_fatal(handle,"io_submit");
orig_io_getevents = (orig_io_getevents_f_type)dlsym_fatal(handle,"io_getevents");
orig_io_cancel = (orig_io_cancel_f_type)dlsym_fatal(handle,"io_cancel");
Expand Down Expand Up @@ -625,6 +628,19 @@ int io_setup(int nr_events, io_context_t *ctx_idp)
return 0;
}

int io_destroy(io_context_t ctx_id)
{
init();

if (!translate_aio) {
return orig_io_destroy(ctx_id);
}

if (ctx_id != &our_io_context) {
return EINVAL;
}
return 0;
}

int io_submit(io_context_t ctx_id, long nr, struct iocb *ios[])
{
Expand Down