Re: [Qemu-devel] [PATCH for 2.0] ide: Correct improper smart self test counter reset in ide core.

[Peter Maydell]

On 14 April 2014 13:31, Kevin Wolf <address@hidden> wrote:
> Am 14.04.2014 um 14:10 hat Markus Armbruster geschrieben:
>> Markus Armbruster <address@hidden> writes:
>>
>> > Benoît Canet <address@hidden> writes:
>> >
>> >> The counter being reseted to zero make the array index negative.
>> >> Reset it to 1.
>> >>
>> >> Signed-off-by: Benoit Canet <address@hidden>
>> >> ---
>> >>  hw/ide/core.c |    2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> >> index e1dfe54..c943a4d 100644
>> >> --- a/hw/ide/core.c
>> >> +++ b/hw/ide/core.c
>> >> @@ -1602,7 +1602,7 @@ static bool cmd_smart(IDEState *s, uint8_t cmd)
>> >>          case 2: /* extended self test */
>> >>              s->smart_selftest_count++;
>> >>              if (s->smart_selftest_count > 21) {
>> >> -                s->smart_selftest_count = 0;
>> >> +                s->smart_selftest_count = 1;
>> >>              }
>> >>              n = 2 + (s->smart_selftest_count - 1) * 24;
>> >>              s->smart_selftest_data[n] = s->sector;
>> >
>> > Good catch.
>> >
>> > Commit message could use some love, though.  On every 21st SMART EXECUTE
>> > OFFLINE:
>> >
>> > * We write before a dynamically allocated buffer
>> >
>> >   Your diff's context has one of the writes.
>> >
>> > * We forget SMART history
>> >
>> >   See the s->smart_selftest_count == 0 special cases in SMART READ DATA
>> >   and SMART READ LOG.
>>
>> Peter offered to improve the commit message on merge.  Even without
>> that, it would be better to get this into 2.0 with a sub-optimal commit
>> message than not to get it, so:
>>
>> Reviewed-by: Markus Armbruster <address@hidden>
>
> Should also be fixed in the stable branch of earlier releases. The bug
> is present since SMART emulation was added in 2009.
>
> Cc: address@hidden
> Acked-by: Kevin Wolf <address@hidden>

Applied to master with a tweaked commit message and the various
r-b/cc/etc tags.

thanks
-- PMM