James Bottomley, on 08/24/2010 06:57 PM wrote:
quoted text
> On Tue, 2010-08-24 at 18:41 +0400, Vladislav Bolkhovitin wrote:
>> James Bottomley, on 08/22/2010 12:43 AM wrote:
>>> Interface re-use (or at least ABI compatibility) is the whole point,
>>> it's what makes the solution a drop in replacement.
>>
>> I see now. You want ABI compatibility to keep the "contract" that no
>> kernel changes can break applications binary compatibility for unlimited
>> time.
>>
>> OK, we will write the compatibility module. It shouldn't take much time.
>>
>> But before we start, I'd like to clear 2 related questions:
>>
>> 1. How far the ABI compatibility "contract" goes? Are there cases, where
>> it isn't so strong? I'm asking, because I can recall that open-iscsi at
>> least once has broken ABI compatibility with user space tools. Was it an
>> accidental (but not corrected) mistake or was it deliberate? If the
>> latter, then, I guess, there must be some exceptions defining when ABI
>> compatibility can be not followed.
> 
> I don't think it has to be complete.  As long as the STGT people think
> it's good enough, that's fine by me.

Tomonori, Mike, could you comment on that, please?
 
quoted text
>> 2. Currently STGT in the kernel is just 2 files, scsi_tgt_if.c and
>> scsi_tgt_lib.c (with headers), + ibmvstgt driver. The C files define the
>> STGT interface in question. So, if we keep ABI compatibility with the
>> new target engine, we would have to keep those 2 files included in the
>> kernel,
> 
> This isn't really correct.  The ABI is defined by the headers not the
> implementation.

Yes, but we on the target side would not be able to implement the ABI compatible interface without using library functions provided by those C files. Or, at least, it would be much harder.

So, would it be OK for you to keep those files?
 
quoted text
>> which would effectively mean that STGT would stay in the kernel.
>> This would lead to the situation you are trying to avoid: 2 SCSI target
>> infrastructures in the kernel. Would it be OK?
> 
> If you mean is the marketing solution of wedging two products into the
> kernel and calling it a single one going to fly, the answer is no.

I mean that if we keep those 2 files to ease our ABI compatibility effort, it would effectively mean that we would leave STGT merged. It isn't something we would create, it just would be so itself as a matter of fact. Ultimately, STGT is an user space engine. What it has in the kernel is the interface helper functions to interact with the in-kernel drivers. The simplest way to achieve the ABI compatibility is to make a backend module acting as an STGT in-target driver.

(Actually, I may not ask it, because this is the way how LIO seems[1] implemented that, which was approved on the LSF summit. I only want to make all pros and cons clear from the very beginning.)

Thanks,
Vlad

1. I wrote "seems", because currently LIO has the following code for STGT commands execution: 

int stgt_do_task(se_task_t *task)
{
	stgt_plugin_task_t *st = (stgt_plugin_task_t *) task->transport_req;
	struct Scsi_Host *sh = task->se_dev->se_hba->hba_ptr;
	struct scsi_cmnd *sc;
	int tag = MSG_SIMPLE_TAG;

	sc = scsi_host_get_command(sh, st->stgt_direction, GFP_KERNEL);
	if (!sc) {
		printk(KERN_ERR "Unable to allocate memory for struct"
			" scsi_cmnd\n");
		return PYX_TRANSPORT_LU_COMM_FAILURE;
	}

	memcpy(sc->cmnd, st->stgt_cdb, MAX_COMMAND_SIZE);
	sc->sdb.length = task->task_size;
	sc->sdb.table.sgl = task->task_sg;
	sc->tag = tag;

	BUG();
#warning FIXME: Get struct scsi_lun for scsi_tgt_queue_command()
#if 0
	err = scsi_tgt_queue_command(sc, itn_id, (struct scsi_lun *)&cmd->lun,
			cmd->tag);
	if (err) {
		printk(KERN_INFO "scsi_tgt_queue_command() failed for sc:"
			" %p\n", sc);
		scsi_host_put_command(sh, sc);
	}
#endif
	return PYX_TRANSPORT_SENT_TO_TRANSPORT;
}

which means that this pluging completely not functioning.
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/