[stgt] 0.9.11 released

[Mark Harvey]

On Wed, Nov 25, 2009 at 12:00 PM, FUJITA Tomonori
<fujita.tomonori at lab.ntt.co.jp> wrote:
> On Tue, 24 Nov 2009 11:14:27 +0100
> Albert Pauw <albert.pauw at gmail.com> wrote:
>
>> Tomasz Chmielewski wrote:
>> > FUJITA Tomonori wrote:
>> >> The tar is also available:
>> >>
>> >> http://stgt.berlios.de/releases/tgt-0.9.11.tar.bz2
>> >>
>> >> As I wrote before, I plan to release the next version as 1.0.0. Let me
>> >> know if you think that we need (or fix) something before releasing
>> >> 1.0.0.
>> >
>> > I have a pending patch for tgt-admin, which adds support for different
>> > "params", to be used with tape drives.
>> >
>> > Before I submit it "correctly" - there were some issues with tgtd core
>> > dumping with certain configurations - Mark, Albert: what's the latest
>> > status of it?
>> >
>> >
>> Hi Tomasz,
>>
>> as far as I know Mark is trying to fix the problem, which he was able to
>> reproduce, but I don't know the status of it.
>> Yes, this would be a good thing to have in the 1.0.0 release (Mark's fix
>> and yours).
>
> I've not followed this. You said that tgtd works if you configure by
> hand but it doesn't if you use targets.conf, right? If so, please find
> out how commands to set up targets are different.
>
> Anyway, your log (2009/10/28) says that the medium changer driver
> crashes during READ_ELEMENT_STATUS due to some memory issue. Looks
> like smc doesn't check memory overwrite. Can you try this?
>
> diff --git a/usr/smc.c b/usr/smc.c
> index 6430882..e7f0177 100644
> --- a/usr/smc.c
> +++ b/usr/smc.c
> @@ -304,7 +304,7 @@ static int smc_read_element_status(int host_no, struct scsi_cmd *cmd)
>                }
>        }
>
> -       data = zalloc(alloc_len);
> +       data = zalloc(alloc_len * 16);
>        if (!data) {
>                dprintf("Can't allocate enough memory for cmd\n");
>                key = HARDWARE_ERROR;
> @@ -369,6 +369,7 @@ static int smc_read_element_status(int host_no, struct scsi_cmd *cmd)
>
>        /* Lastly, fill in data header */
>        len = element_status_data_hdr(data, dvcid, voltag, first, count);
> +       eprintf("%d %d %d\n", alloc_len, len, count);
>        memcpy(scsi_get_in_buffer(cmd), data, min(len, alloc_len));
>        scsi_set_in_resid_by_actual(cmd, len);
>        free(data);
> --
> To unsubscribe from this list: send the line "unsubscribe stgt" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

The actual fix requires quite a re-write of the ELEMENT STATUS.

It is valid for the 'application client' to allocate a very small
buffer (8 bytes minimum), and the 'device server' should return the
Element status data indicating 'number of elements available' & 'byte
count of report available'. The 'application client' can then re-issue
the READ ELEMENT STATUS with an appropriate sized buffer.

Unfortunately, I assume the 'application client' has allocated enough
of a buffer to hold all the data.

To quote smp3r12 : "An insufficient allocation length shall not be
considered an error" & "The BYTE COUNT OF REPORT AVAILABLE field
indicates the number of bytes of element status page data available
for all elements meeting the request in the CDB. This value shall not
be adjusted to match the allocation length available"


A quick 'hack' would be to run a quick calculation of how big the
'BYTE COUNT' is to start with, build the structure within this buffer,
and only return 'allocation len' amount of data.

Cheers
Mark
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html