From dc2f9620f46c3a70b8e97dae7f9b086824dc90eb Mon Sep 17 00:00:00 2001 From: Mark Syms Date: Tue, 21 Jul 2020 09:29:11 +0100 Subject: [PATCH 1/2] CP-34438: extend tap-ctl unpause to allow adding IO restriction --- control/tap-ctl-ipc.c | 22 +++++++++-- control/tap-ctl-open.c | 2 +- control/tap-ctl-unpause.c | 6 +-- control/tap-ctl.c | 16 +++++--- drivers/tapdisk-control.c | 19 +++++++++ drivers/tapdisk-vbd.c | 83 +++++++++++++++++++++++++++++++++++---- drivers/tapdisk-vbd.h | 3 ++ drivers/tapdisk.h | 1 + include/tap-ctl.h | 12 ++++-- include/tapdisk-message.h | 1 + 10 files changed, 142 insertions(+), 23 deletions(-) diff --git a/control/tap-ctl-ipc.c b/control/tap-ctl-ipc.c index a2d22cb8..5044a63f 100644 --- a/control/tap-ctl-ipc.c +++ b/control/tap-ctl-ipc.c @@ -161,7 +161,8 @@ tap_ctl_send_and_receive(int sfd, tapdisk_message_t *message, int tap_ctl_send_and_receive_ex(int sfd, tapdisk_message_t *message, - const char *logpath, uint8_t key_size, + const char *logpath, const char *sockpath, + uint8_t key_size, const uint8_t *encryption_key, struct timeval *timeout) { @@ -203,6 +204,19 @@ tap_ctl_send_and_receive_ex(int sfd, tapdisk_message_t *message, } } + if (message->u.params.flags & TAPDISK_MESSAGE_FLAG_RATED) { + DPRINTF("Sending socket for td-rated\n"); + char buf[TAPDISK_MESSAGE_MAX_PATH_LENGTH]; + snprintf(buf, TAPDISK_MESSAGE_MAX_PATH_LENGTH - 1, "%s", sockpath); + + ret = write(sfd, &buf, sizeof(buf)); + + if (ret == -1) { + EPRINTF("Failed to send sockpath with '%s' message\n", + tapdisk_message_name(message->type)); + } + } + err = tap_ctl_read_message(sfd, message, timeout); if (err) { EPRINTF("failed to receive '%s' message\n", @@ -309,7 +323,9 @@ tap_ctl_connect_send_and_receive(int id, tapdisk_message_t *message, int tap_ctl_connect_send_receive_ex(int id, tapdisk_message_t *message, - const char *logpath, uint8_t key_size, + const char *logpath, + const char *sockpath, + uint8_t key_size, const uint8_t *encryption_key, struct timeval *timeout) { @@ -319,7 +335,7 @@ tap_ctl_connect_send_receive_ex(int id, tapdisk_message_t *message, if (err) return err; - err = tap_ctl_send_and_receive_ex(sfd, message, logpath, key_size, encryption_key, timeout); + err = tap_ctl_send_and_receive_ex(sfd, message, logpath, sockpath, key_size, encryption_key, timeout); close(sfd); return err; diff --git a/control/tap-ctl-open.c b/control/tap-ctl-open.c index 690d847d..a9e7c86d 100644 --- a/control/tap-ctl-open.c +++ b/control/tap-ctl-open.c @@ -75,7 +75,7 @@ tap_ctl_open(const int id, const int minor, const char *params, int flags, } if (flags & (TAPDISK_MESSAGE_FLAG_ADD_LOG | TAPDISK_MESSAGE_FLAG_OPEN_ENCRYPTED)) { err = tap_ctl_connect_send_receive_ex( - id, &message, logpath, key_size, encryption_key, NULL); + id, &message, logpath, NULL, key_size, encryption_key, NULL); } else { err = tap_ctl_connect_send_and_receive(id, &message, NULL); diff --git a/control/tap-ctl-unpause.c b/control/tap-ctl-unpause.c index f92c9762..a5032b0d 100644 --- a/control/tap-ctl-unpause.c +++ b/control/tap-ctl-unpause.c @@ -43,7 +43,7 @@ int tap_ctl_unpause(const int id, const int minor, const char *params, int flags, - char *secondary, const char *logpath) + char *secondary, const char *logpath, const char *sockpath) { int err; tapdisk_message_t message; @@ -65,8 +65,8 @@ tap_ctl_unpause(const int id, const int minor, const char *params, int flags, return -ENAMETOOLONG; } } - if (logpath) { - err = tap_ctl_connect_send_receive_ex(id, &message, logpath, 0, NULL, NULL); + if (logpath || sockpath) { + err = tap_ctl_connect_send_receive_ex(id, &message, logpath, sockpath, 0, NULL, NULL); } else { err = tap_ctl_connect_send_and_receive(id, &message, NULL); diff --git a/control/tap-ctl.c b/control/tap-ctl.c index a391393d..f56a4cc6 100644 --- a/control/tap-ctl.c +++ b/control/tap-ctl.c @@ -648,13 +648,14 @@ tap_cli_unpause_usage(FILE *stream) { fprintf(stream, "usage: unpause <-p pid> <-m minor> [-a type:/path/to/file] " "[-2 secondary] " - "[-c insert log layer to track changed blocks]\n"); + "[-c insert log layer to track changed blocks] " + "[-l use a td-rated valve for IO limiting]\n"); } int tap_cli_unpause(int argc, char **argv) { - const char *args, *logpath; + const char *args, *logpath, *sockpath; char *secondary; int c, pid, minor, flags; @@ -663,10 +664,11 @@ tap_cli_unpause(int argc, char **argv) args = NULL; secondary = NULL; flags = 0; - logpath = NULL; + logpath = NULL; + sockpath = NULL; optind = 0; - while ((c = getopt(argc, argv, "p:m:a:2:c:h")) != -1) { + while ((c = getopt(argc, argv, "p:m:a:2:c:l:h")) != -1) { switch (c) { case 'p': pid = atoi(optarg); @@ -685,6 +687,10 @@ tap_cli_unpause(int argc, char **argv) logpath = optarg; flags |= TAPDISK_MESSAGE_FLAG_ADD_LOG; break; + case 'l': + sockpath = optarg; + flags |= TAPDISK_MESSAGE_FLAG_RATED; + break; case '?': goto usage; case 'h': @@ -696,7 +702,7 @@ tap_cli_unpause(int argc, char **argv) if (pid == -1 || minor == -1) goto usage; - return tap_ctl_unpause(pid, minor, args, flags, secondary, logpath); + return tap_ctl_unpause(pid, minor, args, flags, secondary, logpath, sockpath); usage: tap_cli_unpause_usage(stderr); diff --git a/drivers/tapdisk-control.c b/drivers/tapdisk-control.c index 9c4416d8..2be7e541 100644 --- a/drivers/tapdisk-control.c +++ b/drivers/tapdisk-control.c @@ -1070,6 +1070,25 @@ tapdisk_control_resume_vbd(struct tapdisk_ctl_conn *conn, vbd->flags &= ~TD_OPEN_ADD_LOG; } + if (request->u.params.flags & TAPDISK_MESSAGE_FLAG_RATED) { + char *sockpath = malloc(TAPDISK_MESSAGE_MAX_PATH_LENGTH + 1); + ret = read(conn->fd, sockpath, TAPDISK_MESSAGE_MAX_PATH_LENGTH); + if (ret < 0) { + err = -EIO; + free(sockpath); + goto out; + } + *(sockpath + TAPDISK_MESSAGE_MAX_PATH_LENGTH) = '\0'; + vbd->rated_sockpath = sockpath; + vbd->flags |= TD_OPEN_RATED; + } else { + if (vbd->rated_sockpath) { + free (vbd->rated_sockpath); + vbd->rated_sockpath = NULL; + } + vbd->flags &= ~TD_OPEN_RATED; + } + if (request->u.params.path[0]) desc = request->u.params.path; diff --git a/drivers/tapdisk-vbd.c b/drivers/tapdisk-vbd.c index 7e09ca70..ff3a3383 100644 --- a/drivers/tapdisk-vbd.c +++ b/drivers/tapdisk-vbd.c @@ -570,6 +570,55 @@ static int tapdisk_vbd_add_dirty_log(td_vbd_t *vbd) return err; } +static int tapdisk_vbd_add_rated(td_vbd_t *vbd) +{ + int err; + td_driver_t *driver; + td_image_t *valve, *parent; + + driver = NULL; + valve = NULL; + + DPRINTF("VALVE:tapdisk_vbd_add_rated called for %s with %s\n", + vbd->name, vbd->rated_sockpath); + + parent = tapdisk_vbd_first_image(vbd); + + if (!parent) + return -EINVAL; + + valve = tapdisk_image_allocate(vbd->rated_sockpath, + DISK_TYPE_VALVE, + parent->flags); + + if (!valve) + return -ENOMEM; + + driver = tapdisk_driver_allocate(valve->type, + valve->name, + valve->flags); + + if (!driver) { + err = -ENOMEM; + goto fail; + } + + driver->info = parent->driver->info; + valve->driver = driver; + + err = td_open(valve, &vbd->encryption); + if (err) + goto fail; + + list_add(&valve->next, parent->next.prev); + tapdisk_vbd_debug(vbd); + return 0; + +fail: + tapdisk_image_free(valve); + return err; +} + int tapdisk_vbd_open_vdi(td_vbd_t *vbd, const char *name, td_flag_t flags, int prt_devnum) { @@ -623,9 +672,25 @@ tapdisk_vbd_open_vdi(td_vbd_t *vbd, const char *name, td_flag_t flags, int prt_d goto fail; } + if (td_flag_test(vbd->flags, TD_OPEN_RATED)) { + if (!vbd->rated_sockpath) { + err = -EINVAL; + goto fail; + } + err = tapdisk_vbd_add_rated(vbd); + if (err) { + EPRINTF("VBD %d Error adding valve, %s\n", + vbd->uuid, strerror(-err)); + goto fail; + } + } + err = tapdisk_vbd_validate_chain(vbd); - if (err) + if (err) { + EPRINTF("VBD: failed to validate chain %s\n", + strerror(-err)); goto fail; + } if (td_flag_test(vbd->flags, TD_OPEN_SECONDARY)) { err = tapdisk_vbd_add_secondary(vbd); @@ -637,13 +702,13 @@ tapdisk_vbd_open_vdi(td_vbd_t *vbd, const char *name, td_flag_t flags, int prt_d } } - err = vbd_stats_create(vbd); - if (err) - goto fail; + err = vbd_stats_create(vbd); + if (err) + goto fail; - err = td_metrics_vdi_start(vbd->tap->minor, &vbd->vdi_stats); - if (err) - goto fail; + err = td_metrics_vdi_start(vbd->tap->minor, &vbd->vdi_stats); + if (err) + goto fail; if (tmp != vbd->name) free(tmp); @@ -764,6 +829,10 @@ tapdisk_vbd_shutdown(td_vbd_t *vbd) tapdisk_vbd_detach(vbd); tapdisk_server_remove_vbd(vbd); free(vbd->name); + if (vbd->logpath) + free(vbd->logpath); + if (vbd->rated_sockpath) + free(vbd->rated_sockpath); free(vbd); return 0; diff --git a/drivers/tapdisk-vbd.h b/drivers/tapdisk-vbd.h index c17bddf7..8dbb417b 100644 --- a/drivers/tapdisk-vbd.h +++ b/drivers/tapdisk-vbd.h @@ -166,6 +166,9 @@ struct td_vbd_handle { struct td_vbd_encryption encryption; bool watchdog_warned; + + /* Socket path for IO rating service */ + char *rated_sockpath; }; #define tapdisk_vbd_for_each_request(vreq, tmp, list) \ diff --git a/drivers/tapdisk.h b/drivers/tapdisk.h index de384ea6..9bbd1322 100644 --- a/drivers/tapdisk.h +++ b/drivers/tapdisk.h @@ -102,6 +102,7 @@ extern unsigned int PAGE_SHIFT; #define TD_OPEN_STANDBY 0x00800 #define TD_IGNORE_ENOSPC 0x01000 #define TD_OPEN_NO_O_DIRECT 0x02000 +#define TD_OPEN_RATED 0x04000 #define TD_CREATE_SPARSE 0x00001 #define TD_CREATE_MULTITYPE 0x00002 diff --git a/include/tap-ctl.h b/include/tap-ctl.h index c9f484aa..2e98d1a8 100644 --- a/include/tap-ctl.h +++ b/include/tap-ctl.h @@ -80,6 +80,7 @@ int tap_ctl_connect_send_and_receive(int id, int tap_ctl_connect_send_receive_ex(int id, tapdisk_message_t *message, const char *logpath, + const char *sockpath, uint8_t key_size, const uint8_t *encryption_key, struct timeval *timeout); @@ -135,14 +136,17 @@ int tap_ctl_pause(const int id, const int minor, struct timeval *timeout); /** * Unpauses the VBD * - * @param pid the process ID of the tapdisk + * @param id the process ID of the tapdisk + * @param minor the minor device number for the tapdisk * @param flags TODO * @param secondary TODO - * @param uuid - * @param new_params the new VDI to use (type:/path/to/file), optional + * @param params the new VDI to use (type:/path/to/file), optional + * @param logpath path for changed block tracking log + * @param sockpath path for socket to connect td-rated valve layer */ int tap_ctl_unpause(const int id, const int minor, const char *params, - int flags, char *secondary, const char *logpath); + int flags, char *secondary, const char *logpath, + const char *sockpath); ssize_t tap_ctl_stats(pid_t pid, int minor, char *buf, size_t size); int tap_ctl_stats_fwrite(pid_t pid, int minor, FILE *out); diff --git a/include/tapdisk-message.h b/include/tapdisk-message.h index dceddbbe..e9a5c371 100644 --- a/include/tapdisk-message.h +++ b/include/tapdisk-message.h @@ -56,6 +56,7 @@ #define TAPDISK_MESSAGE_FLAG_STANDBY 0x100 #define TAPDISK_MESSAGE_FLAG_NO_O_DIRECT 0x200 #define TAPDISK_MESSAGE_FLAG_OPEN_ENCRYPTED 0x400 +#define TAPDISK_MESSAGE_FLAG_RATED 0x800 typedef struct tapdisk_message tapdisk_message_t; typedef uint32_t tapdisk_message_flag_t; From 0020950b8a428e8d6a4121a9953886be30a60e22 Mon Sep 17 00:00:00 2001 From: Mark Syms Date: Tue, 21 Jul 2020 10:44:53 +0100 Subject: [PATCH 2/2] Temp no error from valve validate-parent --- drivers/block-valve.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block-valve.c b/drivers/block-valve.c index 85682b42..031ffd9e 100644 --- a/drivers/block-valve.c +++ b/drivers/block-valve.c @@ -647,7 +647,7 @@ static int td_valve_validate_parent(td_driver_t *driver, td_driver_t *parent_driver, td_flag_t flags) { - return -EINVAL; + return 0; } static void