Skip to content

Commit

Permalink
CP-7800: mask conflicting commands instead of retrying
Browse files Browse the repository at this point in the history 
The Storage Manager and tapback might concurrently issue exclusive
commands to tapdisk. tapdisk is not designed to handle concurrent
commands and if this occurs it fails the other command with EBUSY. We
address this issue by masking the event associated with the command that
would be rejecting while tapdisk is busy servicing the first command, and
unmasking it when the first command has finished.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
Reviewed-by: Chandrika Srinivasan <chandrika.srinivasan@citrix.com>
  • Loading branch information
Thanos Makatos committed Apr 8, 2014
1 parent df466b0 commit 9d0ed93
Show file tree
Hide file tree
Showing 11 changed files with 303 additions and 183 deletions.
58 changes: 18 additions & 40 deletions control/tap-ctl-close.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,47 +34,25 @@ tap_ctl_close(const int id, const int minor, const int force,
{
int err;
tapdisk_message_t message;
struct timeval start, now, delta;

/*
* Keep retrying till tapdisk becomes available
* to process the close request
*/
gettimeofday(&start, NULL);
do {
memset(&message, 0, sizeof(message));
message.type = TAPDISK_MESSAGE_CLOSE;
if (force)
message.type = TAPDISK_MESSAGE_FORCE_SHUTDOWN;
message.cookie = minor;

err = tap_ctl_connect_send_and_receive(id, &message, timeout);
if (err)
return err;

if (message.type == TAPDISK_MESSAGE_CLOSE_RSP
|| message.type == TAPDISK_MESSAGE_ERROR) {
err = -message.u.response.error;

if (err != -EBUSY)
break;

sleep(1);

gettimeofday(&now, NULL);
timersub(&now, &start, &delta);
}
else {
EPRINTF("got unexpected result '%s' from %d\n",
tapdisk_message_name(message.type), id);
err = -EINVAL;
return err;
}
/*
* TODO: Can VBDs be accessed here to get
* value of TD_VBD_REQUEST_TIMEOUT
*/
} while(delta.tv_sec < TAPCTL_COMM_RETRY_TIMEOUT);
memset(&message, 0, sizeof(message));
message.type = TAPDISK_MESSAGE_CLOSE;
if (force)
message.type = TAPDISK_MESSAGE_FORCE_SHUTDOWN;
message.cookie = minor;

err = tap_ctl_connect_send_and_receive(id, &message, timeout);
if (err)
return err;

if (message.type == TAPDISK_MESSAGE_CLOSE_RSP
|| message.type == TAPDISK_MESSAGE_ERROR)
err = -message.u.response.error;
else {
EPRINTF("got unexpected result '%s' from %d\n",
tapdisk_message_name(message.type), id);
return err;
}

if (err)
EPRINTF("close failed: %s\n", strerror(-err));
Expand Down
46 changes: 15 additions & 31 deletions control/tap-ctl-pause.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,39 +32,23 @@ tap_ctl_pause(const int id, const int minor, struct timeval *timeout)
{
int err;
tapdisk_message_t message;
struct timeval start, now, delta;

gettimeofday(&start, NULL);
do {
memset(&message, 0, sizeof(message));
message.type = TAPDISK_MESSAGE_PAUSE;
message.cookie = minor;

memset(&message, 0, sizeof(message));
message.type = TAPDISK_MESSAGE_PAUSE;
message.cookie = minor;

err = tap_ctl_connect_send_and_receive(id, &message, timeout);
if (err)
return err;

if (message.type == TAPDISK_MESSAGE_PAUSE_RSP
|| message.type == TAPDISK_MESSAGE_ERROR) {

err = -message.u.response.error;

if (err != -EBUSY)
break;

sleep(1);

gettimeofday(&now, NULL);
timersub(&now, &start, &delta);

} else {
err = -EINVAL;
EPRINTF("got unexpected result '%s' from %d\n",
tapdisk_message_name(message.type), id);
break;
}
} while (delta.tv_sec < TAPCTL_COMM_RETRY_TIMEOUT);
err = tap_ctl_connect_send_and_receive(id, &message, timeout);
if (err)
return err;

if (message.type == TAPDISK_MESSAGE_PAUSE_RSP
|| message.type == TAPDISK_MESSAGE_ERROR)
err = -message.u.response.error;
else {
err = -EINVAL;
EPRINTF("got unexpected result '%s' from %d\n",
tapdisk_message_name(message.type), id);
}

if (err)
EPRINTF("pause failed: %s\n", strerror(-err));
Expand Down
66 changes: 26 additions & 40 deletions control/tap-ctl-unpause.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,51 +34,37 @@ tap_ctl_unpause(const int id, const int minor, const char *params, int flags,
{
int err;
tapdisk_message_t message;
struct timeval start, now, delta;

gettimeofday(&start, NULL);
do {
memset(&message, 0, sizeof(message));
message.type = TAPDISK_MESSAGE_RESUME;
message.cookie = minor;
message.u.params.flags = flags;
memset(&message, 0, sizeof(message));
message.type = TAPDISK_MESSAGE_RESUME;
message.cookie = minor;
message.u.params.flags = flags;

if (params)
strncpy(message.u.params.path, params,
sizeof(message.u.params.path) - 1);
if (secondary) {
err = snprintf(message.u.params.secondary,
sizeof(message.u.params.secondary) - 1, "%s",
secondary);
if (err >= sizeof(message.u.params.secondary)) {
EPRINTF("secondary image name too long\n");
return -ENAMETOOLONG;
}
if (params)
strncpy(message.u.params.path, params,
sizeof(message.u.params.path) - 1);
if (secondary) {
err = snprintf(message.u.params.secondary,
sizeof(message.u.params.secondary) - 1, "%s",
secondary);
if (err >= sizeof(message.u.params.secondary)) {
EPRINTF("secondary image name too long\n");
return -ENAMETOOLONG;
}
}

err = tap_ctl_connect_send_and_receive(id, &message, NULL);
if (err)
return err;

if (message.type == TAPDISK_MESSAGE_RESUME_RSP
|| message.type == TAPDISK_MESSAGE_ERROR) {

err = -message.u.response.error;

if (err != -EBUSY)
break;

sleep(1);
err = tap_ctl_connect_send_and_receive(id, &message, NULL);
if (err)
return err;

gettimeofday(&now, NULL);
timersub(&now, &start, &delta);
} else {
err = -EINVAL;
EPRINTF("got unexpected result '%s' from %d\n",
tapdisk_message_name(message.type), id);
break;
}
} while (delta.tv_sec < TAPCTL_COMM_RETRY_TIMEOUT);
if (message.type == TAPDISK_MESSAGE_RESUME_RSP
|| message.type == TAPDISK_MESSAGE_ERROR)
err = -message.u.response.error;
else {
EPRINTF("got unexpected result '%s' from %d\n",
tapdisk_message_name(message.type), id);
err = -EINVAL;
}

if (err)
EPRINTF("unpause failed: %s\n", strerror(-err));
Expand Down
31 changes: 21 additions & 10 deletions control/tap-ctl-xen.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

#include <stdio.h>
#include <string.h>
#include <unistd.h>

#include "tap-ctl.h"

Expand Down Expand Up @@ -74,21 +75,31 @@ int
tap_ctl_disconnect_xenblkif(const pid_t pid, const domid_t domid,
const int devid, struct timeval *timeout)
{
tapdisk_message_t message;
int err;
tapdisk_message_t message;

memset(&message, 0, sizeof(message));
message.type = TAPDISK_MESSAGE_XENBLKIF_DISCONNECT;
message.u.blkif.domid = domid;
message.u.blkif.devid = devid;
memset(&message, 0, sizeof(message));
message.type = TAPDISK_MESSAGE_XENBLKIF_DISCONNECT;
message.u.blkif.domid = domid;
message.u.blkif.devid = devid;

err = tap_ctl_connect_send_and_receive(pid, &message, timeout);
if (err || message.type == TAPDISK_MESSAGE_ERROR) {
if (!err)
err = -message.u.response.error;
err = tap_ctl_connect_send_and_receive(pid, &message, timeout);
if (err)
goto out;

if (message.type == TAPDISK_MESSAGE_XENBLKIF_DISCONNECT_RSP
|| message.type == TAPDISK_MESSAGE_ERROR)
err = -message.u.response.error;
else {
EPRINTF("got unexpected result '%s' from tapdisk[%d]\n",
tapdisk_message_name(message.type), pid);
err = -EINVAL;
}

out:
if (err)
EPRINTF("failed to disconnect tapdisk[%d] from the ring: %s\n", pid,
strerror(-err));
}

return err;
}
62 changes: 59 additions & 3 deletions drivers/scheduler.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,20 @@ typedef struct event {
event_id_t id;

int fd;

/**
* Timeout in number of seconds, relative to the time of the registration
* of the event. Use the special value (time_t)-1 to indicate infinity.
*/
int timeout;

/**
* Expiration date in number of seconds after Epoch. Once current time
* becomes larger than or equal to this value, the event is considered
* expired and can be run. If event.timeout is set to infinity, this member
* should not be used.
*
*/
int deadline;

event_cb_t cb;
Expand Down Expand Up @@ -99,7 +112,8 @@ scheduler_prepare_events(scheduler_t *s)
s->max_fd = MAX(event->fd, s->max_fd);
}

if (event->mode & SCHEDULER_POLL_TIMEOUT) {
if (event->mode & SCHEDULER_POLL_TIMEOUT
&& event->timeout != (time_t) - 1) {
diff = event->deadline - now.tv_sec;
if (diff > 0)
s->timeout = MIN(s->timeout, diff);
Expand Down Expand Up @@ -148,6 +162,10 @@ scheduler_check_fd_events(scheduler_t *s, int nfds)
return nfds;
}

/**
* Checks all scheduler events whose mode is set to SCHEDULER_POLL_TIMEOUT
* whether their time out has elapsed, and if so it makes them runnable.
*/
static void
scheduler_check_timeouts(scheduler_t *s)
{
Expand All @@ -168,6 +186,9 @@ scheduler_check_timeouts(scheduler_t *s)
if (!(event->mode & SCHEDULER_POLL_TIMEOUT))
continue;

if (event->timeout == (time_t) - 1)
continue;

if (event->deadline > now.tv_sec)
continue;

Expand All @@ -189,7 +210,8 @@ scheduler_check_events(scheduler_t *s, int nfds)
static void
scheduler_event_callback(event_t *event, char mode)
{
if (event->mode & SCHEDULER_POLL_TIMEOUT) {
if (event->mode & SCHEDULER_POLL_TIMEOUT
&& event->timeout != (time_t) - 1) {
struct timeval now;
gettimeofday(&now, NULL);
event->deadline = now.tv_sec + event->timeout;
Expand Down Expand Up @@ -247,7 +269,11 @@ scheduler_register_event(scheduler_t *s, char mode, int fd,
event->mode = mode;
event->fd = fd;
event->timeout = timeout;
event->deadline = now.tv_sec + timeout;
if (event->timeout == (time_t) - 1)
/* initialise it to something meaningful */
event->deadline = (time_t) - 1;
else
event->deadline = now.tv_sec + timeout;
event->cb = cb;
event->private = private;
event->id = s->uuid++;
Expand Down Expand Up @@ -378,3 +404,33 @@ scheduler_initialize(scheduler_t *s)

INIT_LIST_HEAD(&s->events);
}

int
scheduler_event_set_timeout(scheduler_t *sched, event_id_t event_id, int timeo)
{
event_t *event;

ASSERT(sched);

if (!event_id || timeo < 0)
return -EINVAL;

scheduler_for_each_event(sched, event) {
if (event->id == event_id) {
if (!(event->mode & SCHEDULER_POLL_TIMEOUT))
return -EINVAL;
event->timeout = timeo;
if (event->timeout == (time_t) - 1)
event->deadline = (time_t) - 1;
else {
struct timeval now;
gettimeofday(&now, NULL);
event->deadline = now.tv_sec + event->timeout;
}
return 0;
}
}

return -ENOENT;
}

3 changes: 2 additions & 1 deletion drivers/scheduler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,6 @@ void scheduler_unregister_event(scheduler_t *, event_id_t);
void scheduler_mask_event(scheduler_t *, event_id_t, int masked);
void scheduler_set_max_timeout(scheduler_t *, int);
int scheduler_wait_for_events(scheduler_t *);

int scheduler_event_set_timeout(scheduler_t *sched, event_id_t event_id,
int timeo);
#endif
Loading

0 comments on commit 9d0ed93

Please sign in to comment.