On Sun, 15 Apr 2012 13:48:12 +1000
ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote:
> An updated patch to add UNMAP. Here we just try to unmap the region as
> it was given to us by the initiator
> and rely on the initiator to make sure that it only try to unmap data
> on physical block boundaries.
>
> Second is a patch to automatically set
> LogicalBlocksPerPhysicalBlockExponent anytime we open a backing file.
> This is used in READCAPACITY16 to tell the initiators the relations
> between logical blocks and physical blocks
> on the target, and should guide the initiator to do the right thing
> when using the UNMAP CDB
>
> From 8c1c0becbb91e2b2346c7261dbbd6be028f5ff32 Mon Sep 17 00:00:00 2001
> From: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
> Date: Sun, 15 Apr 2012 13:41:18 +1000
> Subject: [PATCH 2/2] SBC: When opening the backing file, check the filesystem blocksize and update LogicalBlocksPerPhysicalBlockExponent accodingly.
>
> This is used to tell the initiator what the allignment between LBAs and the underlying storage blocksize is.
>
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
> ---
> usr/bs_aio.c | 19 +++++++++++++------
> usr/bs_rdwr.c | 13 +++++++++++--
> usr/bs_ssc.c | 2 +-
> usr/util.c | 8 +++++---
> usr/util.h | 3 ++-
> 5 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/usr/bs_aio.c b/usr/bs_aio.c
> index cbd91ba..4db5beb 100644
> --- a/usr/bs_aio.c
> +++ b/usr/bs_aio.c
> @@ -304,6 +304,7 @@ static int bs_aio_open(struct scsi_lu *lu, char *path, int *fd, uint64_t *size)
> {
> struct bs_aio_info *info = BS_AIO_I(lu);
> int ret, afd;
> + uint32_t blksize = 0;
>
> info->iodepth = AIO_MAX_IODEPTH;
> eprintf("create aio context for tgt:%d lun:%"PRId64 ", max iodepth:%d\n",
> @@ -331,13 +332,13 @@ static int bs_aio_open(struct scsi_lu *lu, char *path, int *fd, uint64_t *size)
>
> eprintf("open %s, RW, O_DIRECT for tgt:%d lun:%"PRId64 "\n",
> path, info->lu->tgt->tid, info->lu->lun);
> - *fd = backed_file_open(path, O_RDWR|O_LARGEFILE|O_DIRECT, size);
> + *fd = backed_file_open(path, O_RDWR|O_LARGEFILE|O_DIRECT, size, &blksize);
> /* If we get access denied, try opening the file in readonly mode */
> if (*fd == -1 && (errno == EACCES || errno == EROFS)) {
> eprintf("open %s, READONLY, O_DIRECT for tgt:%d lun:%"PRId64 "\n",
> path, info->lu->tgt->tid, info->lu->lun);
> *fd = backed_file_open(path, O_RDONLY|O_LARGEFILE|O_DIRECT,
> - size);
> + size, &blksize);
> lu->attrs.readonly = 1;
> }
> if (*fd < 0) {
> @@ -346,12 +347,18 @@ static int bs_aio_open(struct scsi_lu *lu, char *path, int *fd, uint64_t *size)
> ret = *fd;
> goto remove_tgt_evt;
> }
> - else {
> - eprintf("%s opened successfully for tgt:%d lun:%"PRId64 "\n",
> - path, info->lu->tgt->tid, info->lu->lun);
> - return 0;
> +
> + eprintf("%s opened successfully for tgt:%d lun:%"PRId64 "\n",
> + path, info->lu->tgt->tid, info->lu->lun);
> +
> + lu->attrs.lbppbe = 0;
lbppbe should be already zero'ed.
> + while (blksize > (1U << lu->blk_shift)) {
> + lu->attrs.lbppbe++;
> + blksize >>= 1;
> }
I think that we should not set lbppbe blindly. Only when users don't
specify lbppbe via tgtadm.
Also can we have a helper function to avoid this duplicated code?
> + return 0;
> +
> remove_tgt_evt:
> tgt_event_del(afd);
> close_eventfd:
> diff --git a/usr/bs_rdwr.c b/usr/bs_rdwr.c
> index 9e15603..a917fab 100644
> --- a/usr/bs_rdwr.c
> +++ b/usr/bs_rdwr.c
> @@ -231,16 +231,25 @@ static void bs_rdwr_request(struct scsi_cmd *cmd)
>
> static int bs_rdwr_open(struct scsi_lu *lu, char *path, int *fd, uint64_t *size)
> {
> - *fd = backed_file_open(path, O_RDWR|O_LARGEFILE|lu->bsoflags, size);
> + uint32_t blksize = 0;
> +
> + *fd = backed_file_open(path, O_RDWR|O_LARGEFILE|lu->bsoflags, size,
> + &blksize);
> /* If we get access denied, try opening the file in readonly mode */
> if (*fd == -1 && (errno == EACCES || errno == EROFS)) {
> *fd = backed_file_open(path, O_RDONLY|O_LARGEFILE|lu->bsoflags,
> - size);
> + size, &blksize);
> lu->attrs.readonly = 1;
> }
> if (*fd < 0)
> return *fd;
>
> + lu->attrs.lbppbe = 0;
> + while (blksize > (1U << lu->blk_shift)) {
> + lu->attrs.lbppbe++;
> + blksize >>= 1;
> + }
> +
> return 0;
> }
>
> diff --git a/usr/bs_ssc.c b/usr/bs_ssc.c
> index 6412972..93d08a6 100644
> --- a/usr/bs_ssc.c
> +++ b/usr/bs_ssc.c
> @@ -619,7 +619,7 @@ static int bs_ssc_open(struct scsi_lu *lu, char *path, int *fd, uint64_t *size)
>
> ssc = dtype_priv(lu);
>
> - *fd = backed_file_open(path, O_RDWR | O_LARGEFILE, size);
> + *fd = backed_file_open(path, O_RDWR | O_LARGEFILE, size, NULL);
> if (*fd < 0) {
> eprintf("Could not open %s %m\n", path);
> return *fd;
> diff --git a/usr/util.c b/usr/util.c
> index c78a999..4cf8291 100644
> --- a/usr/util.c
> +++ b/usr/util.c
> @@ -82,7 +82,7 @@ int chrdev_open(char *modname, char *devpath, uint8_t minor, int *fd)
> return 0;
> }
>
> -int backed_file_open(char *path, int oflag, uint64_t *size)
> +int backed_file_open(char *path, int oflag, uint64_t *size, uint32_t *blksize)
> {
> int fd, err;
> struct stat64 st;
> @@ -99,9 +99,11 @@ int backed_file_open(char *path, int oflag, uint64_t *size)
> goto close_fd;
> }
>
> - if (S_ISREG(st.st_mode))
> + if (S_ISREG(st.st_mode)) {
> *size = st.st_size;
> - else if (S_ISBLK(st.st_mode)) {
> + if (blksize)
> + *blksize = st.st_blksize;
> + } else if (S_ISBLK(st.st_mode)) {
> err = ioctl(fd, BLKGETSIZE64, size);
> if (err < 0) {
> eprintf("Cannot get size, %m\n");
> diff --git a/usr/util.h b/usr/util.h
> index 4bf157d..c016e5a 100644
> --- a/usr/util.h
> +++ b/usr/util.h
> @@ -62,7 +62,8 @@
>
> extern int get_blk_shift(unsigned int size);
> extern int chrdev_open(char *modname, char *devpath, uint8_t minor, int *fd);
> -extern int backed_file_open(char *path, int oflag, uint64_t *size);
> +extern int backed_file_open(char *path, int oflag, uint64_t *size,
> + uint32_t *blksize);
> extern int set_non_blocking(int fd);
> extern int str_to_open_flags(char *buf);
> extern char *open_flags_to_str(char *dest, int flags);
> --
> 1.7.3.1
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
