[Qemu-devel] Re: [PATCH 07/14] ide: Change ide_init_drive() to require valid dinfo argument

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 28.05.2010 15:38, schrieb Markus Armbruster:
>> IDEState members drive_serial_str and version are now left empty until
>> an actual drive is connected.  Before, they got a default value that
>> was overwritten when a drive got connected.  Doesn't matter, because
>> they're used only while a drive is connected.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  hw/ide/core.c |   47 +++++++++++++++++++++++++----------------------
>>  1 files changed, 25 insertions(+), 22 deletions(-)
>> 
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index 443ff10..f72d37f 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -2601,30 +2601,29 @@ void ide_init_drive(IDEState *s, DriveInfo *dinfo, 
>> const char *version)
>>      int cylinders, heads, secs;
>>      uint64_t nb_sectors;
>>  
>> -    if (dinfo && dinfo->bdrv) {
>> -        s->bs = dinfo->bdrv;
>> -        bdrv_get_geometry(s->bs, &nb_sectors);
>> -        bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
>> -        s->cylinders = cylinders;
>> -        s->heads = heads;
>> -        s->sectors = secs;
>> -        s->nb_sectors = nb_sectors;
>> -        /* The SMART values should be preserved across power cycles
>> -           but they aren't.  */
>> -        s->smart_enabled = 1;
>> -        s->smart_autosave = 1;
>> -        s->smart_errors = 0;
>> -        s->smart_selftest_count = 0;
>> -        if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
>> -            s->is_cdrom = 1;
>> -            bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
>> -        }
>> -        strncpy(s->drive_serial_str, drive_get_serial(s->bs),
>> -                sizeof(s->drive_serial_str));
>> +    s->bs = dinfo->bdrv;
>> +    bdrv_get_geometry(s->bs, &nb_sectors);
>> +    bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
>> +    s->cylinders = cylinders;
>> +    s->heads = heads;
>> +    s->sectors = secs;
>> +    s->nb_sectors = nb_sectors;
>> +    /* The SMART values should be preserved across power cycles
>> +       but they aren't.  */
>> +    s->smart_enabled = 1;
>> +    s->smart_autosave = 1;
>> +    s->smart_errors = 0;
>> +    s->smart_selftest_count = 0;
>> +    if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
>> +        s->is_cdrom = 1;
>> +        bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
>>      }
>> -    if (strlen(s->drive_serial_str) == 0)
>> +    strncpy(s->drive_serial_str, drive_get_serial(s->bs),
>> +            sizeof(s->drive_serial_str));
>> +    if (*s->drive_serial_str) {
>
> This should be the other way round. It breaks the serial number in both
> cases.

I have no idea how that could get past my testing.  I'll respin.
Thanks!