Skip to content

Commit 4988fb0

Browse files
author
Robert Breker
committed
Have tapback announce the discard capability for vbds
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 <[email protected]>
1 parent 442ee66 commit 4988fb0

File tree

8 files changed

+84
-15
lines changed

8 files changed

+84
-15
lines changed

control/tap-ctl-info.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
#include "tap-ctl.h"
2626

2727
int tap_ctl_info(pid_t pid, unsigned long long *sectors,
28-
unsigned int *sector_size, unsigned int *info, const int minor)
28+
unsigned int *sector_size, unsigned int *info,
29+
bool * discard_supported, const int minor)
2930
{
3031
tapdisk_message_t message;
3132
int err;
@@ -49,6 +50,7 @@ int tap_ctl_info(pid_t pid, unsigned long long *sectors,
4950
*sectors = message.u.image.sectors;
5051
*sector_size = message.u.image.sector_size;
5152
*info = message.u.image.info;
53+
*discard_supported = message.u.image.discard_supported;
5254
return 0;
5355
} else if (TAPDISK_MESSAGE_ERROR == message.type) {
5456
return -message.u.response.error;

drivers/tapdisk-control.c

+1
Original file line numberDiff line numberDiff line change
@@ -1193,6 +1193,7 @@ tapdisk_control_disk_info(
11931193
image->sectors = vbd->disk_info.size;
11941194
image->sector_size = vbd->disk_info.sector_size;
11951195
image->info = vbd->disk_info.info;
1196+
image->discard_supported = vbd->disk_info.discard_supported;
11961197
}
11971198
return err;
11981199
}

include/tap-ctl.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,8 @@ int tap_ctl_disconnect_xenblkif(const pid_t pid, const domid_t domid,
176176
*
177177
*/
178178
int tap_ctl_info(pid_t pid, unsigned long long *sectors, unsigned int
179-
*sector_size, unsigned int *info, const int minor);
179+
*sector_size, unsigned int *info, bool *discard_supported,
180+
const int minor);
180181

181182
/**
182183
* Parses a type:/path/to/file string, storing the type and path to the output

include/tapdisk-message.h

+2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#define _TAPDISK_MESSAGE_H_
2020

2121
#include <inttypes.h>
22+
#include <stdbool.h>
2223
#include <sys/types.h>
2324

2425
/*
@@ -70,6 +71,7 @@ struct tapdisk_message_image {
7071
uint64_t sectors;
7172
uint32_t sector_size;
7273
uint32_t info;
74+
bool discard_supported;
7375
};
7476

7577
struct tapdisk_message_string {

tapback/backend.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ physical_device_changed(vbd_t *device) {
344344
* get the VBD parameters from the tapdisk
345345
*/
346346
if ((err = tap_ctl_info(device->tap->pid, &device->sectors,
347-
&device->sector_size, &info,
347+
&device->sector_size, &info, &device->discard_supported,
348348
device->minor))) {
349349
WARN(device, "error retrieving disk characteristics: %s\n",
350350
strerror(-err));

tapback/frontend.c

+44-2
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,10 @@ connect_frontend(vbd_t *device) {
259259
int err = 0;
260260
xs_transaction_t xst = XBT_NULL;
261261
bool abort_transaction = false;
262+
unsigned int discard_granularity = 0;
263+
unsigned int discard_alignment = 0;
264+
int discard_secure = 0;
265+
int feature_discard = 0;
262266

263267
ASSERT(device);
264268

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

274278
/*
275-
* FIXME blkback writes discard-granularity, discard-alignment,
276-
* discard-secure, feature-discard but we don't.
279+
* Prepare for discard
277280
*/
281+
if (device->discard_supported == true && /* backing driver supports */
282+
device->backend->discard == true && /* discard enabled */
283+
device->mode == true && /* writable vbd */
284+
device->cdrom == false) { /* not a CD */
285+
INFO(device, "Discard enabled.");
286+
discard_granularity = device->sector_size;
287+
discard_alignment = 0;
288+
discard_secure = 0;
289+
feature_discard = 1;
290+
} else
291+
INFO(device, "Discard disabled.");
292+
if ((err = tapback_device_printf(device, xst, "discard-granularity",
293+
true, "%u", discard_granularity ))) {
294+
WARN(device, "failed to write discard_granularity: %s\n",
295+
strerror(-err));
296+
break;
297+
}
298+
299+
if ((err = tapback_device_printf(device, xst, "discard-alignment",
300+
true, "%u", discard_alignment ))) {
301+
WARN(device, "failed to write discard_alignment: %s\n",
302+
strerror(-err));
303+
break;
304+
}
305+
306+
if ((err = tapback_device_printf(device, xst, "discard-secure",
307+
true, "%d", discard_secure ))) {
308+
WARN(device, "failed to write discard-secure: %s\n",
309+
strerror(-err));
310+
break;
311+
}
312+
313+
if ((err = tapback_device_printf(device, xst, "feature-discard",
314+
true, "%d", feature_discard ))) {
315+
WARN(device, "failed to write feature_discard: %s\n",
316+
strerror(-err));
317+
break;
318+
}
319+
278320

279321
/*
280322
* Write the number of sectors, sector size, info, and barrier support

tapback/tapback.c

+20-10
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,8 @@ tapback_write_pid(const char *pidfile)
219219
*/
220220
static inline backend_t *
221221
tapback_backend_create(const char *name, const char *pidfile,
222-
const domid_t domid, const bool barrier)
222+
const domid_t domid, const bool barrier,
223+
const bool discard)
223224
{
224225
int err;
225226
int len;
@@ -254,7 +255,8 @@ tapback_backend_create(const char *name, const char *pidfile,
254255
goto out;
255256
}
256257

257-
backend->barrier = barrier;
258+
backend->barrier = barrier;
259+
backend->discard = discard;
258260

259261
backend->path = NULL;
260262

@@ -501,7 +503,8 @@ usage(FILE * const stream, const char * const prog)
501503
"\t[-d|--debug]\n"
502504
"\t[-h|--help]\n"
503505
"\t[-v|--verbose]\n"
504-
"\t[-b]--nobarrier]\n"
506+
"\t[-b]--nobarrier]\n"
507+
"\t[-s]--nodiscard]\n"
505508
"\t[-n|--name]\n", prog);
506509
}
507510

@@ -567,7 +570,8 @@ int main(int argc, char **argv)
567570
int err = 0;
568571
backend_t *backend = NULL;
569572
domid_t opt_domid = 0;
570-
bool opt_barrier = true;
573+
bool opt_barrier = true;
574+
bool opt_discard = true;
571575

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

599604
};
600605
int c;
601606

602-
c = getopt_long(argc, argv, "hdvn:p:x:b", longopts, NULL);
607+
c = getopt_long(argc, argv, "hdvn:p:x:b:s", longopts, NULL);
603608
if (c < 0)
604609
break;
605610

@@ -636,9 +641,14 @@ int main(int argc, char **argv)
636641
}
637642
INFO(NULL, "only serving domain %d\n", opt_domid);
638643
break;
639-
case 'b':
640-
opt_barrier = false;
641-
break;
644+
case 'b':
645+
/* nobarrier */
646+
opt_barrier = false;
647+
break;
648+
case 's':
649+
/* nodiscard */
650+
opt_discard = false;
651+
break;
642652
case '?':
643653
goto usage;
644654
}
@@ -673,7 +683,7 @@ int main(int argc, char **argv)
673683
}
674684

675685
backend = tapback_backend_create(opt_name, opt_pidfile, opt_domid,
676-
opt_barrier);
686+
opt_barrier, opt_discard);
677687
if (!backend) {
678688
err = errno;
679689
WARN(NULL, "error creating back-end: %s\n", strerror(err));

tapback/tapback.h

+11
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,12 @@ typedef struct backend {
195195
* Tells whether we support write I/O barriers.
196196
*/
197197
bool barrier;
198+
199+
/**
200+
* Whether discard is supposed to be enabled if supported by the
201+
* tapdisk.
202+
*/
203+
bool discard;
198204
} backend_t;
199205

200206
/**
@@ -271,6 +277,11 @@ typedef struct vbd {
271277
*/
272278
unsigned long long sectors;
273279

280+
/**
281+
* Whether the backing driver supports discard, supplied by the tapdisk.
282+
*/
283+
bool discard_supported;
284+
274285
/**
275286
* VDISK_???, defined in include/xen/interface/io/blkif.h.
276287
*/

0 commit comments

Comments
 (0)