[Qemu-devel] [PATCH 4/4] hw/block-common: Factor out fall back to legacy -drive cyls=...

[Markus Armbruster]

Signed-off-by: Markus Armbruster <address@hidden>
---
 hw/block-common.c |   40 ++++++++++++++++++++++++++++++++++++++++
 hw/block-common.h |    2 ++
 hw/ide/core.c     |   24 ++++++++++++------------
 hw/ide/qdev.c     |   19 ++-----------------
 hw/scsi-disk.c    |   31 +++----------------------------
 hw/virtio-blk.c   |   31 +++----------------------------
 6 files changed, 62 insertions(+), 85 deletions(-)

diff --git a/hw/block-common.c b/hw/block-common.c
index 0a0542a..f0196d7 100644
--- a/hw/block-common.c
+++ b/hw/block-common.c
@@ -9,6 +9,7 @@
 
 #include "blockdev.h"
 #include "hw/block-common.h"
+#include "qemu-error.h"
 
 void blkconf_serial(BlockConf *conf, char **serial)
 {
@@ -22,3 +23,42 @@ void blkconf_serial(BlockConf *conf, char **serial)
         }
     }
 }
+
+int blkconf_geometry(BlockConf *conf, int *ptrans,
+                     unsigned cyls_max, unsigned heads_max, unsigned secs_max)
+{
+    DriveInfo *dinfo;
+
+    if (!conf->cyls && !conf->heads && !conf->secs) {
+        /* try to fall back to value set with legacy -drive cyls=... */
+        dinfo = drive_get_by_blockdev(conf->bs);
+        conf->cyls  = dinfo->cyls;
+        conf->heads = dinfo->heads;
+        conf->secs  = dinfo->secs;
+        if (ptrans) {
+            *ptrans = dinfo->trans;
+        }
+    }
+    if (!conf->cyls && !conf->heads && !conf->secs) {
+        hd_geometry_guess(conf->bs,
+                          &conf->cyls, &conf->heads, &conf->secs,
+                          ptrans);
+    } else if (ptrans && *ptrans == BIOS_ATA_TRANSLATION_AUTO) {
+        *ptrans = hd_bios_chs_auto_trans(conf->cyls, conf->heads, conf->secs);
+    }
+    if (conf->cyls || conf->heads || conf->secs) {
+        if (conf->cyls < 1 || conf->cyls > cyls_max) {
+            error_report("cyls must be between 1 and %u", cyls_max);
+            return -1;
+        }
+        if (conf->heads < 1 || conf->heads > heads_max) {
+            error_report("heads must be between 1 and %u", heads_max);
+            return -1;
+        }
+        if (conf->secs < 1 || conf->secs > secs_max) {
+            error_report("secs must be between 1 and %u", secs_max);
+            return -1;
+        }
+    }
+    return 0;
+}
diff --git a/hw/block-common.h b/hw/block-common.h
index 52bddda..bb808f7 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -60,6 +60,8 @@ static inline unsigned int get_physical_block_exp(BlockConf 
*conf)
 /* Configuration helpers */
 
 void blkconf_serial(BlockConf *conf, char **serial);
+int blkconf_geometry(BlockConf *conf, int *trans,
+                     unsigned cyls_max, unsigned heads_max, unsigned secs_max);
 
 /* Hard disk geometry */
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 5378fc3..d65ef3d 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1935,18 +1935,6 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, 
IDEDriveKind kind,
     s->drive_kind = kind;
 
     bdrv_get_geometry(bs, &nb_sectors);
-    if (cylinders < 1 || cylinders > 65535) {
-        error_report("cyls must be between 1 and 65535");
-        return -1;
-    }
-    if (heads < 1 || heads > 16) {
-        error_report("heads must be between 1 and 16");
-        return -1;
-    }
-    if (secs < 1 || secs > 255) {
-        error_report("secs must be between 1 and 255");
-        return -1;
-    }
     s->cylinders = cylinders;
     s->heads = heads;
     s->sectors = secs;
@@ -2094,6 +2082,18 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, 
DriveInfo *hd0,
             } else if (trans == BIOS_ATA_TRANSLATION_AUTO) {
                 trans = hd_bios_chs_auto_trans(cyls, heads, secs);
             }
+            if (cyls < 1 || cyls > 65535) {
+                error_report("cyls must be between 1 and 65535");
+                exit(1);
+            }
+            if (heads < 1 || heads > 16) {
+                error_report("heads must be between 1 and 16");
+                exit(1);
+            }
+            if (secs < 1 || secs > 255) {
+                error_report("secs must be between 1 and 255");
+                exit(1);
+            }
             if (ide_init_drive(&bus->ifs[i], dinfo->bdrv,
                                dinfo->media_cd ? IDE_CD : IDE_HD,
                                NULL, dinfo->serial, NULL, 0,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 7fe803c..22e58df 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -142,7 +142,6 @@ 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;
-    DriveInfo *dinfo;
 
     if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) 
{
         error_report("discard_granularity must be 512 for ide");
@@ -150,22 +149,8 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind 
kind)
     }
 
     blkconf_serial(&dev->conf, &dev->serial);
-
-    if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
-        /* try to fall back to value set with legacy -drive cyls=... */
-        dinfo = drive_get_by_blockdev(dev->conf.bs);
-        dev->conf.cyls  = dinfo->cyls;
-        dev->conf.heads = dinfo->heads;
-        dev->conf.secs  = dinfo->secs;
-        dev->chs_trans  = dinfo->trans;
-    }
-    if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
-        hd_geometry_guess(dev->conf.bs,
-                          &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs,
-                          &dev->chs_trans);
-    } else if (dev->chs_trans == BIOS_ATA_TRANSLATION_AUTO) {
-        dev->chs_trans = hd_bios_chs_auto_trans(dev->conf.cyls,
-                                        dev->conf.heads, dev->conf.secs);
+    if (blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255) < 0) {
+        return -1;
     }
 
     if (ide_init_drive(s, dev->conf.bs, kind,
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 39a07d7..525816c 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1737,7 +1737,6 @@ static void scsi_disk_unit_attention_reported(SCSIDevice 
*dev)
 static int scsi_initfn(SCSIDevice *dev)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
-    DriveInfo *dinfo;
 
     if (!s->qdev.conf.bs) {
         error_report("drive property not set");
@@ -1750,34 +1749,10 @@ static int scsi_initfn(SCSIDevice *dev)
         return -1;
     }
 
-    if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
-        /* try to fall back to value set with legacy -drive cyls=... */
-        dinfo = drive_get_by_blockdev(s->qdev.conf.bs);
-        dev->conf.cyls = dinfo->cyls;
-        dev->conf.heads = dinfo->heads;
-        dev->conf.secs = dinfo->secs;
-    }
-    if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
-        hd_geometry_guess(s->qdev.conf.bs,
-                          &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs,
-                          NULL);
-    }
-    if (dev->conf.cyls || dev->conf.heads || dev->conf.secs) {
-        if (dev->conf.cyls < 1 || dev->conf.cyls > 65535) {
-            error_report("cyls must be between 1 and 65535");
-            return -1;
-        }
-        if (dev->conf.heads < 1 || dev->conf.heads > 255) {
-            error_report("heads must be between 1 and 255");
-            return -1;
-        }
-        if (dev->conf.secs < 1 || dev->conf.secs > 255) {
-            error_report("secs must be between 1 and 255");
-            return -1;
-        }
-    }
-
     blkconf_serial(&s->qdev.conf, &s->serial);
+    if (blkconf_geometry(&dev->conf, NULL, 65535, 255, 255) < 0) {
+        return -1;
+    }
 
     if (!s->version) {
         s->version = g_strdup(qemu_get_version());
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index ba087bc..f21757e 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -589,7 +589,6 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, 
VirtIOBlkConf *blk)
 {
     VirtIOBlock *s;
     static int virtio_blk_id;
-    DriveInfo *dinfo;
 
     if (!blk->conf.bs) {
         error_report("drive property not set");
@@ -601,6 +600,9 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, 
VirtIOBlkConf *blk)
     }
 
     blkconf_serial(&blk->conf, &blk->serial);
+    if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {
+        return NULL;
+    }
 
     s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
                                           sizeof(struct virtio_blk_config),
@@ -615,33 +617,6 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, 
VirtIOBlkConf *blk)
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
 
-    if (!blk->conf.cyls && !blk->conf.heads && !blk->conf.secs) {
-        /* try to fall back to value set with legacy -drive cyls=... */
-        dinfo = drive_get_by_blockdev(blk->conf.bs);
-        blk->conf.cyls = dinfo->cyls;
-        blk->conf.heads = dinfo->heads;
-        blk->conf.secs = dinfo->secs;
-    }
-    if (!blk->conf.cyls && !blk->conf.heads && !blk->conf.secs) {
-        hd_geometry_guess(s->bs,
-                          &blk->conf.cyls, &blk->conf.heads, &blk->conf.secs,
-                          NULL);
-    }
-    if (blk->conf.cyls || blk->conf.heads || blk->conf.secs) {
-        if (blk->conf.cyls < 1 || blk->conf.cyls > 65535) {
-            error_report("cyls must be between 1 and 65535");
-            return NULL;
-        }
-        if (blk->conf.heads < 1 || blk->conf.heads > 255) {
-            error_report("heads must be between 1 and 255");
-            return NULL;
-        }
-        if (blk->conf.secs < 1 || blk->conf.secs > 255) {
-            error_report("secs must be between 1 and 255");
-            return NULL;
-        }
-    }
-
     s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
 
     qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
-- 
1.7.6.5