Patchwork [07/14] ide: Change ide_init_drive() to require valid dinfo argument

login
register
mail settings
Submitter Markus Armbruster
Date May 28, 2010, 1:38 p.m.
Message ID <1275053935-10627-8-git-send-email-armbru@redhat.com>
Download mbox | patch
Permalink /patch/53893/
State New
Headers show

Comments

Markus Armbruster - May 28, 2010, 1:38 p.m.
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 <armbru@redhat.com>
---
 hw/ide/core.c |   47 +++++++++++++++++++++++++----------------------
 1 files changed, 25 insertions(+), 22 deletions(-)
Kevin Wolf - June 1, 2010, 2:57 p.m.
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 <armbru@redhat.com>
> ---
>  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.

Kevin
Markus Armbruster - June 1, 2010, 6:29 p.m.
Kevin Wolf <kwolf@redhat.com> 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 <armbru@redhat.com>
>> ---
>>  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!

Patch

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) {
         snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
                  "QM%05d", s->drive_serial);
+    }
     if (version) {
         pstrcpy(s->version, sizeof(s->version), version);
     } else {
@@ -2646,7 +2645,11 @@  static void ide_init1(IDEBus *bus, int unit, DriveInfo *dinfo)
     s->smart_selftest_data = qemu_blockalign(s->bs, 512);
     s->sector_write_timer = qemu_new_timer(vm_clock,
                                            ide_sector_write_timer_cb, s);
-    ide_init_drive(s, dinfo, NULL);
+    if (dinfo) {
+        ide_init_drive(s, dinfo, NULL);
+    } else {
+        ide_reset(s);
+    }
 }
 
 void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,