Patchwork [3/8] split away drive init from ide_init2()

login
register
mail settings
Submitter Gerd Hoffmann
Date Sept. 11, 2009, 12:32 p.m.
Message ID <1252672351-12937-4-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/33458/
State Superseded
Headers show

Comments

Gerd Hoffmann - Sept. 11, 2009, 12:32 p.m.
This allows the ide bus being initialized without drives attached
and the drives being attached and initialization later on as
separate step.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ide/core.c     |   71 +++++++++++++++++++++++++++++------------------------
 hw/ide/internal.h |    1 +
 2 files changed, 40 insertions(+), 32 deletions(-)
Markus Armbruster - Sept. 11, 2009, 2:27 p.m.
Gerd Hoffmann <kraxel@redhat.com> writes:

> This allows the ide bus being initialized without drives attached
> and the drives being attached and initialization later on as
> separate step.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/ide/core.c     |   71 +++++++++++++++++++++++++++++------------------------
>  hw/ide/internal.h |    1 +
>  2 files changed, 40 insertions(+), 32 deletions(-)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index fe5bd17..bac2d5e 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -2505,51 +2505,58 @@ void ide_reset(IDEState *s)
>      s->media_changed = 0;
>  }
>  
> +void ide_init_drive(IDEState *s, DriveInfo *dinfo)
> +{
> +    int cylinders, heads, secs;
> +    uint64_t nb_sectors;
> +
> +    if (dinfo && dinfo->bdrv) {
> +        s->bs = dinfo->bdrv;

Before, this assignement was executed even when !dinfo->bdrv.  Can this
happen?  Does it matter?

> +        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));

Same for this assignment.

> +    }
> +    if (strlen(s->drive_serial_str) == 0)
> +        snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
> +                 "QM%05d", s->drive_serial);
> +    ide_reset(s);
> +}
> +
>  void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
>                 qemu_irq irq)
>  {
>      IDEState *s;
>      static int drive_serial = 1;
> -    int i, cylinders, heads, secs;
> -    uint64_t nb_sectors;
> +    int i;
>  
>      for(i = 0; i < 2; i++) {
>          s = bus->ifs + i;
>          s->bus = bus;
>          s->unit = i;
> -        if (i == 0 && hd0)
> -            s->bs = hd0->bdrv;
> -        if (i == 1 && hd1)
> -            s->bs = hd1->bdrv;
> -        s->io_buffer = qemu_blockalign(s->bs, IDE_DMA_BUF_SECTORS*512 + 4);
> -        if (s->bs) {
> -            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;
> -	    s->smart_selftest_data = qemu_blockalign(s->bs, 512);
> -            if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
> -                s->is_cdrom = 1;
> -		bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
> -            }
> -        }
>          s->drive_serial = drive_serial++;
> -        strncpy(s->drive_serial_str, drive_get_serial(s->bs),
> -                sizeof(s->drive_serial_str));
> -        if (strlen(s->drive_serial_str) == 0)
> -            snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
> -                    "QM%05d", s->drive_serial);
> +        s->io_buffer = qemu_blockalign(s->bs, IDE_DMA_BUF_SECTORS*512 + 4);
> +        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_reset(s);
> +        if (i == 0)
> +            ide_init_drive(s, hd0);
> +        if (i == 1)
> +            ide_init_drive(s, hd1);

I'd write this as

        ide_init_drive(i == 0 ? hd0 : hd1);

because I find it clearer, but it's a matter of taste.

>      }
>      bus->irq = irq;
>  }
> diff --git a/hw/ide/internal.h b/hw/ide/internal.h
> index b9a7c72..62aef1c 100644
> --- a/hw/ide/internal.h
> +++ b/hw/ide/internal.h
> @@ -527,6 +527,7 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr);
>  void ide_data_writel(void *opaque, uint32_t addr, uint32_t val);
>  uint32_t ide_data_readl(void *opaque, uint32_t addr);
>  
> +void ide_init_drive(IDEState *s, DriveInfo *dinfo);
>  void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
>                 qemu_irq irq);
>  void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
Gerd Hoffmann - Sept. 11, 2009, 2:36 p.m.
>> +void ide_init_drive(IDEState *s, DriveInfo *dinfo)
>> +{
>> +    int cylinders, heads, secs;
>> +    uint64_t nb_sectors;
>> +
>> +    if (dinfo&&  dinfo->bdrv) {
>> +        s->bs = dinfo->bdrv;
>
> Before, this assignement was executed even when !dinfo->bdrv.  Can this
> happen?

Running this once with dinfo->bdrv = something, then with dinfo->bdrv = 
NULL (which is the only case where this matters) can't happen.  You 
can't hot-unplut ide drives ;)

The code used to be called once for initialization.  Now it might be 
called twice for ide busses converted to qdev: Once with dinfo = NULL 
(when setting up the bus) and once with dinfo != NULL (when attaching 
the drive).

cheers,
   Gerd

Patch

diff --git a/hw/ide/core.c b/hw/ide/core.c
index fe5bd17..bac2d5e 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2505,51 +2505,58 @@  void ide_reset(IDEState *s)
     s->media_changed = 0;
 }
 
+void ide_init_drive(IDEState *s, DriveInfo *dinfo)
+{
+    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));
+    }
+    if (strlen(s->drive_serial_str) == 0)
+        snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
+                 "QM%05d", s->drive_serial);
+    ide_reset(s);
+}
+
 void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
                qemu_irq irq)
 {
     IDEState *s;
     static int drive_serial = 1;
-    int i, cylinders, heads, secs;
-    uint64_t nb_sectors;
+    int i;
 
     for(i = 0; i < 2; i++) {
         s = bus->ifs + i;
         s->bus = bus;
         s->unit = i;
-        if (i == 0 && hd0)
-            s->bs = hd0->bdrv;
-        if (i == 1 && hd1)
-            s->bs = hd1->bdrv;
-        s->io_buffer = qemu_blockalign(s->bs, IDE_DMA_BUF_SECTORS*512 + 4);
-        if (s->bs) {
-            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;
-	    s->smart_selftest_data = qemu_blockalign(s->bs, 512);
-            if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
-                s->is_cdrom = 1;
-		bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
-            }
-        }
         s->drive_serial = drive_serial++;
-        strncpy(s->drive_serial_str, drive_get_serial(s->bs),
-                sizeof(s->drive_serial_str));
-        if (strlen(s->drive_serial_str) == 0)
-            snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
-                    "QM%05d", s->drive_serial);
+        s->io_buffer = qemu_blockalign(s->bs, IDE_DMA_BUF_SECTORS*512 + 4);
+        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_reset(s);
+        if (i == 0)
+            ide_init_drive(s, hd0);
+        if (i == 1)
+            ide_init_drive(s, hd1);
     }
     bus->irq = irq;
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index b9a7c72..62aef1c 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -527,6 +527,7 @@  uint32_t ide_data_readw(void *opaque, uint32_t addr);
 void ide_data_writel(void *opaque, uint32_t addr, uint32_t val);
 uint32_t ide_data_readl(void *opaque, uint32_t addr);
 
+void ide_init_drive(IDEState *s, DriveInfo *dinfo);
 void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
                qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);