On Wed, Apr 18, 2012 at 9:30 PM, FUJITA Tomonori
<fujita.tomonori@xxxxxxxxxxxxx> wrote:
> On Wed, 18 Apr 2012 19:26:47 +1000
> Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx> wrote:
>
>> Unless the user has set an explicit value through tgtadm in which case we leave the system using the value the user has set.
>>
>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
>> ---
>> doc/targets.conf.5.xml | 7 ++++++-
>> usr/bs_aio.c | 19 ++++++++++++-------
>> usr/bs_rdwr.c | 10 ++++++++--
>> usr/bs_ssc.c | 2 +-
>> usr/spc.c | 1 +
>> usr/target.c | 9 +++++++++
>> usr/tgtd.h | 5 +++++
>> usr/util.c | 8 +++++---
>> usr/util.h | 3 ++-
>> 9 files changed, 49 insertions(+), 15 deletions(-)
>>
>> diff --git a/doc/targets.conf.5.xml b/doc/targets.conf.5.xml
>> index f754df3..833e40a 100644
>> --- a/doc/targets.conf.5.xml
>> +++ b/doc/targets.conf.5.xml
>> @@ -336,7 +336,12 @@
>> <listitem>
>> <para>
>> Specify the Logical blocks per physical block
>> - exponent. This is an internal option that should not be
>> + exponent. By default TGTD will set the lbppbe to automatically
>> + match the underlying filesystem. Use this parameter to override
>> + that setting.
>> + </para>
>> + <para>
>> + This is an internal option that should not be
>> set directly.
>> </para>
>> </listitem>
>> diff --git a/usr/bs_aio.c b/usr/bs_aio.c
>> index cbd91ba..75ae66f 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,14 @@ 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,11 +348,14 @@ 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);
>> +
>> + if (!lu->attrs.no_auto_lbppbe)
>> + update_lbppbe(lu, blksize);
>
> Why can't we simply do:
>
>> + if (!lu->lbppbe)
>> + update_lbppbe(lu, blksize);
>
> ?
>
> If an user doesn't specify lbppbe via tgtadm, lu->lbppbe should be
> zero?
>
It can still be non-zero.
It is only zero first time we mount the backing file.
Since we support removable devices, a user could make a LUN/disk "offline"
which means we close the backing file.
Then a little while later the user swaps the media and makes the file
online again and we open a new file as the backing-store.
I.e.
1, create lun and open backing file /fs1/foo.img (lbppbe is
update automatically)
2, user sets lbppbe manually via tgtadm
3, user makes the file offline and close() the backing file :
tgtadm --tid 1 --lun 1 --op update --mode logicalunit --name online --value No
4, user connects the LUN to a different backing file and makes it online again :
tgtadm --tid 1 --lun 1 --op update --mode logicalunit --name path
--value /fs2/bar.img
At this stage we open() a new backing store file and lbppbe is no
longer zero. It is onlt zero for the first time we open a backing
store file for a specific LUN.
At this stage, the LUN already exist but we just open a new backing store file.
At this stage lbppbe is not zero when we open the file, so we can
not from lbppbe itself
determine
"is lbppbe non-zero because it was set for the previous backing file
we used and we swapped to a different one
or is lbppbe non-zero because a user forced a value using tgtadm?"
regards
ronnie sahlberg
--
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
