Patchwork [5/8] ide: Split qdev "ide-drive" into "ide-hd" and "ide-cd"

login
register
mail settings
Submitter Markus Armbruster
Date July 6, 2010, 12:37 p.m.
Message ID <1278419869-26126-6-git-send-email-armbru@redhat.com>
Download mbox | patch
Permalink /patch/58020/
State New
Headers show

Comments

Markus Armbruster - July 6, 2010, 12:37 p.m.
Disk vs. CD needs to be in qdev, because it belongs to the drive's
guest part.

Keep ide-drive for backward compatibility.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/core.c     |   11 +++++---
 hw/ide/internal.h |    2 +-
 hw/ide/qdev.c     |   72 ++++++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 66 insertions(+), 19 deletions(-)
Christoph Hellwig - July 7, 2010, 1:35 a.m.
Looks good,


Reviewed-by: Christoph Hellwig <hch@lst.de>
Kevin Wolf - July 7, 2010, 10:19 a.m.
Am 06.07.2010 14:37, schrieb Markus Armbruster:
> Disk vs. CD needs to be in qdev, because it belongs to the drive's
> guest part.
> 
> Keep ide-drive for backward compatibility.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/ide/core.c     |   11 +++++---
>  hw/ide/internal.h |    2 +-
>  hw/ide/qdev.c     |   72 ++++++++++++++++++++++++++++++++++++++++++----------
>  3 files changed, 66 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index e20f2e7..1287f11 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -2595,13 +2595,15 @@ void ide_bus_reset(IDEBus *bus)
>      ide_clear_hob(bus);
>  }
>  
> -int ide_init_drive(IDEState *s, BlockDriverState *bs,
> +int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
>                     const char *version, const char *serial)
>  {
>      int cylinders, heads, secs;
>      uint64_t nb_sectors;
>  
>      s->bs = bs;
> +    s->drive_kind = kind;
> +
>      bdrv_get_geometry(bs, &nb_sectors);
>      bdrv_guess_geometry(bs, &cylinders, &heads, &secs);
>      if (cylinders < 1 || cylinders > 16383) {
> @@ -2626,8 +2628,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs,
>      s->smart_autosave = 1;
>      s->smart_errors = 0;
>      s->smart_selftest_count = 0;
> -    if (bdrv_get_type_hint(bs) == BDRV_TYPE_CDROM) {
> -        s->drive_kind = IDE_CD;
> +    if (kind == IDE_CD) {
>          bdrv_set_change_cb(bs, cdrom_change_cb, s);
>      } else {
>          if (!bdrv_is_inserted(s->bs)) {
> @@ -2692,7 +2693,9 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
>          dinfo = i == 0 ? hd0 : hd1;
>          ide_init1(bus, i);
>          if (dinfo) {
> -            if (ide_init_drive(&bus->ifs[i], dinfo->bdrv, NULL,
> +            if (ide_init_drive(&bus->ifs[i], dinfo->bdrv,
> +                               bdrv_get_type_hint(dinfo->bdrv) == BDRV_TYPE_CDROM ? IDE_CD : IDE_HD,
> +                               NULL,
>                                 *dinfo->serial ? dinfo->serial : NULL) < 0) {
>                  error_report("Can't set up IDE drive %s", dinfo->id);
>                  exit(1);
> diff --git a/hw/ide/internal.h b/hw/ide/internal.h
> index 4165543..d5de33b 100644
> --- a/hw/ide/internal.h
> +++ b/hw/ide/internal.h
> @@ -556,7 +556,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);
>  
> -int ide_init_drive(IDEState *s, BlockDriverState *bs,
> +int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
>                     const char *version, const char *serial);
>  void ide_init2(IDEBus *bus, qemu_irq irq);
>  void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 53468ed..a7f0b22 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -82,7 +82,9 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
>  {
>      DeviceState *dev;
>  
> -    dev = qdev_create(&bus->qbus, "ide-drive");
> +    dev = qdev_create(&bus->qbus,
> +                      bdrv_get_type_hint(drive->bdrv) == BDRV_TYPE_CDROM
> +                      ? "ide-hd" : "ide-cd");
>      qdev_prop_set_uint32(dev, "unit", unit);
>      qdev_prop_set_drive_nofail(dev, "drive", drive->bdrv);
>      qdev_init_nofail(dev);
> @@ -102,7 +104,7 @@ typedef struct IDEDrive {
>      IDEDevice dev;
>  } IDEDrive;
>  
> -static int ide_drive_initfn(IDEDevice *dev)
> +static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
>  {
>      IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
>      IDEState *s = bus->ifs + dev->unit;
> @@ -118,7 +120,7 @@ static int ide_drive_initfn(IDEDevice *dev)
>          }
>      }
>  
> -    if (ide_init_drive(s, dev->conf.bs, dev->version, serial) < 0) {
> +    if (ide_init_drive(s, dev->conf.bs, kind, dev->version, serial) < 0) {
>          return -1;
>      }
>  
> @@ -131,21 +133,63 @@ static int ide_drive_initfn(IDEDevice *dev)
>      return 0;
>  }
>  
> -static IDEDeviceInfo ide_drive_info = {
> -    .qdev.name  = "ide-drive",
> -    .qdev.size  = sizeof(IDEDrive),
> -    .init       = ide_drive_initfn,
> -    .qdev.props = (Property[]) {
> -        DEFINE_PROP_UINT32("unit", IDEDrive, dev.unit, -1),
> -        DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),
> -        DEFINE_PROP_STRING("ver",  IDEDrive, dev.version),
> -        DEFINE_PROP_STRING("serial",  IDEDrive, dev.serial),
> -        DEFINE_PROP_END_OF_LIST(),
> +static int ide_hd_initfn(IDEDevice *dev)
> +{
> +    return ide_dev_initfn(dev, IDE_HD);
> +}
> +
> +static int ide_cd_initfn(IDEDevice *dev)
> +{
> +    return ide_dev_initfn(dev, IDE_CD);
> +}
> +
> +static int ide_drive_initfn(IDEDevice *dev)
> +{
> +    return ide_dev_initfn(dev,
> +                          bdrv_get_type_hint(dev->conf.bs) == BDRV_TYPE_CDROM
> +                          ? IDE_CD : IDE_HD);
> +}
> +
> +#define DEFINE_IDE_DRIVE_PROPERTIES()                           \
> +    DEFINE_PROP_UINT32("unit", IDEDrive, dev.unit, -1),         \
> +    DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),                \
> +    DEFINE_PROP_STRING("ver",  IDEDrive, dev.version),          \
> +    DEFINE_PROP_STRING("serial",  IDEDrive, dev.serial)
> +    

Whitespace error.

If you don't need to respin, I'll fix it when applying the series.

Kevin

Patch

diff --git a/hw/ide/core.c b/hw/ide/core.c
index e20f2e7..1287f11 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2595,13 +2595,15 @@  void ide_bus_reset(IDEBus *bus)
     ide_clear_hob(bus);
 }
 
-int ide_init_drive(IDEState *s, BlockDriverState *bs,
+int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
                    const char *version, const char *serial)
 {
     int cylinders, heads, secs;
     uint64_t nb_sectors;
 
     s->bs = bs;
+    s->drive_kind = kind;
+
     bdrv_get_geometry(bs, &nb_sectors);
     bdrv_guess_geometry(bs, &cylinders, &heads, &secs);
     if (cylinders < 1 || cylinders > 16383) {
@@ -2626,8 +2628,7 @@  int ide_init_drive(IDEState *s, BlockDriverState *bs,
     s->smart_autosave = 1;
     s->smart_errors = 0;
     s->smart_selftest_count = 0;
-    if (bdrv_get_type_hint(bs) == BDRV_TYPE_CDROM) {
-        s->drive_kind = IDE_CD;
+    if (kind == IDE_CD) {
         bdrv_set_change_cb(bs, cdrom_change_cb, s);
     } else {
         if (!bdrv_is_inserted(s->bs)) {
@@ -2692,7 +2693,9 @@  void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
         dinfo = i == 0 ? hd0 : hd1;
         ide_init1(bus, i);
         if (dinfo) {
-            if (ide_init_drive(&bus->ifs[i], dinfo->bdrv, NULL,
+            if (ide_init_drive(&bus->ifs[i], dinfo->bdrv,
+                               bdrv_get_type_hint(dinfo->bdrv) == BDRV_TYPE_CDROM ? IDE_CD : IDE_HD,
+                               NULL,
                                *dinfo->serial ? dinfo->serial : NULL) < 0) {
                 error_report("Can't set up IDE drive %s", dinfo->id);
                 exit(1);
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 4165543..d5de33b 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -556,7 +556,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);
 
-int ide_init_drive(IDEState *s, BlockDriverState *bs,
+int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
                    const char *version, const char *serial);
 void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 53468ed..a7f0b22 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -82,7 +82,9 @@  IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
 {
     DeviceState *dev;
 
-    dev = qdev_create(&bus->qbus, "ide-drive");
+    dev = qdev_create(&bus->qbus,
+                      bdrv_get_type_hint(drive->bdrv) == BDRV_TYPE_CDROM
+                      ? "ide-hd" : "ide-cd");
     qdev_prop_set_uint32(dev, "unit", unit);
     qdev_prop_set_drive_nofail(dev, "drive", drive->bdrv);
     qdev_init_nofail(dev);
@@ -102,7 +104,7 @@  typedef struct IDEDrive {
     IDEDevice dev;
 } IDEDrive;
 
-static int ide_drive_initfn(IDEDevice *dev)
+static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
 {
     IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
     IDEState *s = bus->ifs + dev->unit;
@@ -118,7 +120,7 @@  static int ide_drive_initfn(IDEDevice *dev)
         }
     }
 
-    if (ide_init_drive(s, dev->conf.bs, dev->version, serial) < 0) {
+    if (ide_init_drive(s, dev->conf.bs, kind, dev->version, serial) < 0) {
         return -1;
     }
 
@@ -131,21 +133,63 @@  static int ide_drive_initfn(IDEDevice *dev)
     return 0;
 }
 
-static IDEDeviceInfo ide_drive_info = {
-    .qdev.name  = "ide-drive",
-    .qdev.size  = sizeof(IDEDrive),
-    .init       = ide_drive_initfn,
-    .qdev.props = (Property[]) {
-        DEFINE_PROP_UINT32("unit", IDEDrive, dev.unit, -1),
-        DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),
-        DEFINE_PROP_STRING("ver",  IDEDrive, dev.version),
-        DEFINE_PROP_STRING("serial",  IDEDrive, dev.serial),
-        DEFINE_PROP_END_OF_LIST(),
+static int ide_hd_initfn(IDEDevice *dev)
+{
+    return ide_dev_initfn(dev, IDE_HD);
+}
+
+static int ide_cd_initfn(IDEDevice *dev)
+{
+    return ide_dev_initfn(dev, IDE_CD);
+}
+
+static int ide_drive_initfn(IDEDevice *dev)
+{
+    return ide_dev_initfn(dev,
+                          bdrv_get_type_hint(dev->conf.bs) == BDRV_TYPE_CDROM
+                          ? IDE_CD : IDE_HD);
+}
+
+#define DEFINE_IDE_DRIVE_PROPERTIES()                           \
+    DEFINE_PROP_UINT32("unit", IDEDrive, dev.unit, -1),         \
+    DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),                \
+    DEFINE_PROP_STRING("ver",  IDEDrive, dev.version),          \
+    DEFINE_PROP_STRING("serial",  IDEDrive, dev.serial)
+    
+static IDEDeviceInfo ide_info[] = {
+    {
+        .qdev.name  = "ide-hd",
+        .qdev.size  = sizeof(IDEDrive),
+        .init       = ide_hd_initfn,
+        .qdev.props = (Property[]) {
+            DEFINE_IDE_DRIVE_PROPERTIES(),
+            DEFINE_PROP_END_OF_LIST(),
+        }
+    },{
+        .qdev.name  = "ide-cd",
+        .qdev.size  = sizeof(IDEDrive),
+        .init       = ide_cd_initfn,
+        .qdev.props = (Property[]) {
+            DEFINE_IDE_DRIVE_PROPERTIES(),
+            DEFINE_PROP_END_OF_LIST(),
+        }
+    },{
+        .qdev.name  = "ide-drive", /* legacy -device ide-drive */
+        .qdev.size  = sizeof(IDEDrive),
+        .init       = ide_drive_initfn,
+        .qdev.props = (Property[]) {
+            DEFINE_IDE_DRIVE_PROPERTIES(),
+            DEFINE_PROP_END_OF_LIST(),
+        }
     }
 };
 
 static void ide_drive_register(void)
 {
-    ide_qdev_register(&ide_drive_info);
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(ide_info); i++) {
+        ide_qdev_register(&ide_info[i]);
+    }
 }
 device_init(ide_drive_register);