Skip to content

Commit

Permalink
Have tapback announce the discard capability for vbds
Browse files Browse the repository at this point in the history
The discard capability is announced for vbds if the following two conditions
apply:
* The backing tapdisk indicates discard support for the image that is
  attached to the vbd (i.e. block-aio is used in combination with a capable
  filesystem)
* tapback is not run with the --nodiscard overwrite

If discard is enabled, the discard granularity (discard_granularity) is
statically announced as the sector size of the vbd. The alignment
(discard_alignment) is statically announced as 0.
Secure discard (discard_secure) is never guaranteed.

This commit has been dev-tested using:
* v8 Windows PV drivers that include XenDisk and thereby implement discard
* Linux xen-blkfront that implements discard

Signed-off-by: Robert Breker <robert.breker@citrix.com>
  • Loading branch information
Robert Breker committed Oct 7, 2015
1 parent 442ee66 commit 4988fb0
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 15 deletions.
4 changes: 3 additions & 1 deletion control/tap-ctl-info.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
#include "tap-ctl.h"

int tap_ctl_info(pid_t pid, unsigned long long *sectors,
unsigned int *sector_size, unsigned int *info, const int minor)
unsigned int *sector_size, unsigned int *info,
bool * discard_supported, const int minor)
{
tapdisk_message_t message;
int err;
Expand All @@ -49,6 +50,7 @@ int tap_ctl_info(pid_t pid, unsigned long long *sectors,
*sectors = message.u.image.sectors;
*sector_size = message.u.image.sector_size;
*info = message.u.image.info;
*discard_supported = message.u.image.discard_supported;
return 0;
} else if (TAPDISK_MESSAGE_ERROR == message.type) {
return -message.u.response.error;
Expand Down
1 change: 1 addition & 0 deletions drivers/tapdisk-control.c
Original file line number Diff line number Diff line change
Expand Up @@ -1193,6 +1193,7 @@ tapdisk_control_disk_info(
image->sectors = vbd->disk_info.size;
image->sector_size = vbd->disk_info.sector_size;
image->info = vbd->disk_info.info;
image->discard_supported = vbd->disk_info.discard_supported;
}
return err;
}
Expand Down
3 changes: 2 additions & 1 deletion include/tap-ctl.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ int tap_ctl_disconnect_xenblkif(const pid_t pid, const domid_t domid,
*
*/
int tap_ctl_info(pid_t pid, unsigned long long *sectors, unsigned int
*sector_size, unsigned int *info, const int minor);
*sector_size, unsigned int *info, bool *discard_supported,
const int minor);

/**
* Parses a type:/path/to/file string, storing the type and path to the output
Expand Down
2 changes: 2 additions & 0 deletions include/tapdisk-message.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#define _TAPDISK_MESSAGE_H_

#include <inttypes.h>
#include <stdbool.h>
#include <sys/types.h>

/*
Expand Down Expand Up @@ -70,6 +71,7 @@ struct tapdisk_message_image {
uint64_t sectors;
uint32_t sector_size;
uint32_t info;
bool discard_supported;
};

struct tapdisk_message_string {
Expand Down
2 changes: 1 addition & 1 deletion tapback/backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ physical_device_changed(vbd_t *device) {
* get the VBD parameters from the tapdisk
*/
if ((err = tap_ctl_info(device->tap->pid, &device->sectors,
&device->sector_size, &info,
&device->sector_size, &info, &device->discard_supported,
device->minor))) {
WARN(device, "error retrieving disk characteristics: %s\n",
strerror(-err));
Expand Down
46 changes: 44 additions & 2 deletions tapback/frontend.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,10 @@ connect_frontend(vbd_t *device) {
int err = 0;
xs_transaction_t xst = XBT_NULL;
bool abort_transaction = false;
unsigned int discard_granularity = 0;
unsigned int discard_alignment = 0;
int discard_secure = 0;
int feature_discard = 0;

ASSERT(device);

Expand All @@ -272,9 +276,47 @@ connect_frontend(vbd_t *device) {
abort_transaction = true;

/*
* FIXME blkback writes discard-granularity, discard-alignment,
* discard-secure, feature-discard but we don't.
* Prepare for discard
*/
if (device->discard_supported == true && /* backing driver supports */
device->backend->discard == true && /* discard enabled */
device->mode == true && /* writable vbd */
device->cdrom == false) { /* not a CD */
INFO(device, "Discard enabled.");
discard_granularity = device->sector_size;
discard_alignment = 0;
discard_secure = 0;
feature_discard = 1;
} else
INFO(device, "Discard disabled.");
if ((err = tapback_device_printf(device, xst, "discard-granularity",
true, "%u", discard_granularity ))) {
WARN(device, "failed to write discard_granularity: %s\n",
strerror(-err));
break;
}

if ((err = tapback_device_printf(device, xst, "discard-alignment",
true, "%u", discard_alignment ))) {
WARN(device, "failed to write discard_alignment: %s\n",
strerror(-err));
break;
}

if ((err = tapback_device_printf(device, xst, "discard-secure",
true, "%d", discard_secure ))) {
WARN(device, "failed to write discard-secure: %s\n",
strerror(-err));
break;
}

if ((err = tapback_device_printf(device, xst, "feature-discard",
true, "%d", feature_discard ))) {
WARN(device, "failed to write feature_discard: %s\n",
strerror(-err));
break;
}


/*
* Write the number of sectors, sector size, info, and barrier support
Expand Down
30 changes: 20 additions & 10 deletions tapback/tapback.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ tapback_write_pid(const char *pidfile)
*/
static inline backend_t *
tapback_backend_create(const char *name, const char *pidfile,
const domid_t domid, const bool barrier)
const domid_t domid, const bool barrier,
const bool discard)
{
int err;
int len;
Expand Down Expand Up @@ -254,7 +255,8 @@ tapback_backend_create(const char *name, const char *pidfile,
goto out;
}

backend->barrier = barrier;
backend->barrier = barrier;
backend->discard = discard;

backend->path = NULL;

Expand Down Expand Up @@ -501,7 +503,8 @@ usage(FILE * const stream, const char * const prog)
"\t[-d|--debug]\n"
"\t[-h|--help]\n"
"\t[-v|--verbose]\n"
"\t[-b]--nobarrier]\n"
"\t[-b]--nobarrier]\n"
"\t[-s]--nodiscard]\n"
"\t[-n|--name]\n", prog);
}

Expand Down Expand Up @@ -567,7 +570,8 @@ int main(int argc, char **argv)
int err = 0;
backend_t *backend = NULL;
domid_t opt_domid = 0;
bool opt_barrier = true;
bool opt_barrier = true;
bool opt_discard = true;

if (access("/dev/xen/gntdev", F_OK ) == -1) {
WARN(NULL, "grant device does not exist\n");
Expand All @@ -594,12 +598,13 @@ int main(int argc, char **argv)
{"name", 0, NULL, 'n'},
{"pidfile", 0, NULL, 'p'},
{"domain", 0, NULL, 'x'},
{"nobarrier", 0, NULL, 'b'},
{"nobarrier", 0, NULL, 'b'},
{"nodiscard", 0, NULL, 's'},

};
int c;

c = getopt_long(argc, argv, "hdvn:p:x:b", longopts, NULL);
c = getopt_long(argc, argv, "hdvn:p:x:b:s", longopts, NULL);
if (c < 0)
break;

Expand Down Expand Up @@ -636,9 +641,14 @@ int main(int argc, char **argv)
}
INFO(NULL, "only serving domain %d\n", opt_domid);
break;
case 'b':
opt_barrier = false;
break;
case 'b':
/* nobarrier */
opt_barrier = false;
break;
case 's':
/* nodiscard */
opt_discard = false;
break;
case '?':
goto usage;
}
Expand Down Expand Up @@ -673,7 +683,7 @@ int main(int argc, char **argv)
}

backend = tapback_backend_create(opt_name, opt_pidfile, opt_domid,
opt_barrier);
opt_barrier, opt_discard);
if (!backend) {
err = errno;
WARN(NULL, "error creating back-end: %s\n", strerror(err));
Expand Down
11 changes: 11 additions & 0 deletions tapback/tapback.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ typedef struct backend {
* Tells whether we support write I/O barriers.
*/
bool barrier;

/**
* Whether discard is supposed to be enabled if supported by the
* tapdisk.
*/
bool discard;
} backend_t;

/**
Expand Down Expand Up @@ -271,6 +277,11 @@ typedef struct vbd {
*/
unsigned long long sectors;

/**
* Whether the backing driver supports discard, supplied by the tapdisk.
*/
bool discard_supported;

/**
* VDISK_???, defined in include/xen/interface/io/blkif.h.
*/
Expand Down

0 comments on commit 4988fb0

Please sign in to comment.