qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, blauwirbel@gmail.com,
	stefanha@linux.vnet.ibm.com, pbonzini@redhat.com
Subject: [Qemu-devel] [PATCH 27/32] block: Geometry and translation hints are now useless, purge them
Date: Fri,  6 Jul 2012 08:58:05 +0200	[thread overview]
Message-ID: <1341557890-17464-28-git-send-email-armbru@redhat.com> (raw)
In-Reply-To: <1341557890-17464-1-git-send-email-armbru@redhat.com>

There are two producers of these hints: drive_init() on behalf of
-drive, and hd_geometry_guess().

The only consumer of the hint is hd_geometry_guess().

The callers of hd_geometry_guess() call it only when drive_init()
didn't set the hints.  Therefore, drive_init()'s hints are never used.

Thus, hd_geometry_guess() only ever sees hints it produced itself in a
prior call.  Only the first call computes something, subsequent calls
just repeat the first call's results.  However, hd_geometry_guess() is
never called more than once: the device models don't, and the block
device is destroyed on unplug.  Thus, dropping the repeat feature
doesn't break anything now.

If a block device wasn't destroyed on unplug and could be reused with
a new device, then repeating old results would be wrong.  Thus,
dropping the repeat feature prevents future breakage.

This renders the hints unused.  Purge them from the block layer.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c              |   32 --------------------------------
 block.h              |   12 ------------
 block_int.h          |    1 -
 blockdev.c           |   14 ++------------
 hw/block-common.h    |    6 ++++++
 hw/hd-geometry.c     |   20 +-------------------
 hw/pc.c              |    1 +
 hw/qdev-properties.c |    1 +
 vl.c                 |    2 +-
 9 files changed, 12 insertions(+), 77 deletions(-)

diff --git a/block.c b/block.c
index d021fd0..1eb5897 100644
--- a/block.c
+++ b/block.c
@@ -1013,12 +1013,6 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
     tmp.block_timer       = bs_top->block_timer;
     tmp.io_limits_enabled = bs_top->io_limits_enabled;
 
-    /* geometry */
-    tmp.cyls              = bs_top->cyls;
-    tmp.heads             = bs_top->heads;
-    tmp.secs              = bs_top->secs;
-    tmp.translation       = bs_top->translation;
-
     /* r/w error */
     tmp.on_read_error     = bs_top->on_read_error;
     tmp.on_write_error    = bs_top->on_write_error;
@@ -2103,27 +2097,6 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr)
     *nb_sectors_ptr = length;
 }
 
-void bdrv_set_geometry_hint(BlockDriverState *bs,
-                            int cyls, int heads, int secs)
-{
-    bs->cyls = cyls;
-    bs->heads = heads;
-    bs->secs = secs;
-}
-
-void bdrv_set_translation_hint(BlockDriverState *bs, int translation)
-{
-    bs->translation = translation;
-}
-
-void bdrv_get_geometry_hint(BlockDriverState *bs,
-                            int *pcyls, int *pheads, int *psecs)
-{
-    *pcyls = bs->cyls;
-    *pheads = bs->heads;
-    *psecs = bs->secs;
-}
-
 /* throttling disk io limits */
 void bdrv_set_io_limits(BlockDriverState *bs,
                         BlockIOLimit *io_limits)
@@ -2132,11 +2105,6 @@ void bdrv_set_io_limits(BlockDriverState *bs,
     bs->io_limits_enabled = bdrv_io_limits_enabled(bs);
 }
 
-int bdrv_get_translation_hint(BlockDriverState *bs)
-{
-    return bs->translation;
-}
-
 void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
                        BlockErrorAction on_write_error)
 {
diff --git a/block.h b/block.h
index e0121d5..f02316d 100644
--- a/block.h
+++ b/block.h
@@ -256,18 +256,6 @@ int bdrv_has_zero_init(BlockDriverState *bs);
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
                       int *pnum);
 
-#define BIOS_ATA_TRANSLATION_AUTO   0
-#define BIOS_ATA_TRANSLATION_NONE   1
-#define BIOS_ATA_TRANSLATION_LBA    2
-#define BIOS_ATA_TRANSLATION_LARGE  3
-#define BIOS_ATA_TRANSLATION_RECHS  4
-
-void bdrv_set_geometry_hint(BlockDriverState *bs,
-                            int cyls, int heads, int secs);
-void bdrv_set_translation_hint(BlockDriverState *bs, int translation);
-void bdrv_get_geometry_hint(BlockDriverState *bs,
-                            int *pcyls, int *pheads, int *psecs);
-int bdrv_get_translation_hint(BlockDriverState *bs);
 void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
                        BlockErrorAction on_write_error);
 BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read);
diff --git a/block_int.h b/block_int.h
index 1fb5352..d72317f 100644
--- a/block_int.h
+++ b/block_int.h
@@ -320,7 +320,6 @@ struct BlockDriverState {
 
     /* NOTE: the following infos are only hints for real hardware
        drivers. They are not used by the block driver */
-    int cyls, heads, secs, translation;
     BlockErrorAction on_read_error, on_write_error;
     bool iostatus_enabled;
     BlockDeviceIoStatus iostatus;
diff --git a/blockdev.c b/blockdev.c
index 4d3b707..3274923 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -7,8 +7,8 @@
  * later.  See the COPYING file in the top-level directory.
  */
 
-#include "block.h"
 #include "blockdev.h"
+#include "hw/block-common.h"
 #include "monitor.h"
 #include "qerror.h"
 #include "qemu-option.h"
@@ -551,17 +551,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
     case IF_SCSI:
     case IF_XEN:
     case IF_NONE:
-        switch(media) {
-	case MEDIA_DISK:
-            if (cyls != 0) {
-                bdrv_set_geometry_hint(dinfo->bdrv, cyls, heads, secs);
-                bdrv_set_translation_hint(dinfo->bdrv, translation);
-            }
-	    break;
-	case MEDIA_CDROM:
-            dinfo->media_cd = 1;
-	    break;
-	}
+        dinfo->media_cd = media == MEDIA_CDROM;
         break;
     case IF_SD:
     case IF_FLOPPY:
diff --git a/hw/block-common.h b/hw/block-common.h
index 2f65186..ec7810d 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -15,6 +15,12 @@
 
 /* Hard disk geometry */
 
+#define BIOS_ATA_TRANSLATION_AUTO   0
+#define BIOS_ATA_TRANSLATION_NONE   1
+#define BIOS_ATA_TRANSLATION_LBA    2
+#define BIOS_ATA_TRANSLATION_LARGE  3
+#define BIOS_ATA_TRANSLATION_RECHS  4
+
 void hd_geometry_guess(BlockDriverState *bs,
                        uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
                        int *ptrans);
diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 7626cbb..74678a6 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -122,25 +122,10 @@ void hd_geometry_guess(BlockDriverState *bs,
 {
     int cylinders, heads, secs, translation;
 
-    bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs);
-    translation = bdrv_get_translation_hint(bs);
-
-    if (cylinders != 0) {
-        /* already got a geometry hint: use it */
-        *pcyls = cylinders;
-        *pheads = heads;
-        *psecs = secs;
-        if (ptrans) {
-            *ptrans = translation;
-        }
-        return;
-    }
-
-    assert(translation == BIOS_ATA_TRANSLATION_AUTO);
-
     if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) {
         /* no LCHS guess: use a standard physical disk geometry  */
         guess_chs_for_size(bs, pcyls, pheads, psecs);
+        translation = BIOS_ATA_TRANSLATION_AUTO;
     } else if (heads > 16) {
         /* LCHS guess with heads > 16 means that a BIOS LBA
            translation was active, so a standard physical disk
@@ -149,7 +134,6 @@ void hd_geometry_guess(BlockDriverState *bs,
         translation = *pcyls * *pheads <= 131072
             ? BIOS_ATA_TRANSLATION_LARGE
             : BIOS_ATA_TRANSLATION_LBA;
-        bdrv_set_translation_hint(bs, translation);
     } else {
         /* LCHS guess with heads <= 16: use as physical geometry */
         *pcyls = cylinders;
@@ -158,11 +142,9 @@ void hd_geometry_guess(BlockDriverState *bs,
         /* disable any translation to be in sync with
            the logical geometry */
         translation = BIOS_ATA_TRANSLATION_NONE;
-        bdrv_set_translation_hint(bs, translation);
     }
     if (ptrans) {
         *ptrans = translation;
     }
-    bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
     trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation);
 }
diff --git a/hw/pc.c b/hw/pc.c
index 5fc64f7..968376e 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -44,6 +44,7 @@
 #include "kvm.h"
 #include "xen.h"
 #include "blockdev.h"
+#include "hw/block-common.h"
 #include "ui/qemu-spice.h"
 #include "memory.h"
 #include "exec-memory.h"
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 0b18f8c..01c378f 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -2,6 +2,7 @@
 #include "qdev.h"
 #include "qerror.h"
 #include "blockdev.h"
+#include "hw/block-common.h"
 
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
 {
diff --git a/vl.c b/vl.c
index 1329c30..a7b707c 100644
--- a/vl.c
+++ b/vl.c
@@ -130,8 +130,8 @@ int main(int argc, char **argv)
 #include "qemu-timer.h"
 #include "qemu-char.h"
 #include "cache-utils.h"
-#include "block.h"
 #include "blockdev.h"
+#include "hw/block-common.h"
 #include "block-migration.h"
 #include "dma.h"
 #include "audio/audio.h"
-- 
1.7.6.5

  parent reply	other threads:[~2012-07-06  7:04 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-06  6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 01/32] fdc: Drop broken code for user-defined floppy geometry Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 02/32] fdc: Move floppy geometry guessing back from block.c Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 03/32] vvfat: Fix partition table Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 04/32] vvfat: Do not clobber the user's geometry Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 05/32] qtest: Tidy up temporary files properly Markus Armbruster
2012-07-07  7:39   ` Blue Swirl
2012-07-09  7:56     ` Markus Armbruster
2012-07-09  9:09       ` Kevin Wolf
2012-07-06  6:57 ` [Qemu-devel] [PATCH 06/32] qtest: Add hard disk geometry test Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 07/32] block: Factor bdrv_read_unthrottled() out of guess_disk_lchs() Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 08/32] hd-geometry: Move disk geometry guessing back from block.c Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 09/32] hd-geometry: Add tracepoints Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 10/32] hd-geometry: Unnest conditional in hd_geometry_guess() Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 11/32] hd-geometry: Factor out guess_chs_for_size() Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 12/32] hd-geometry: Clean up gratuitous goto in hd_geometry_guess() Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 13/32] hd-geometry: Clean up confusing use of prior translation hint Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 14/32] hd-geometry: Cut out block layer translation middleman Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 15/32] ide pc: Cut out the block layer geometry middleman Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 16/32] blockdev: Save geometry in DriveInfo Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 17/32] qdev: Introduce block geometry properties Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 18/32] hd-geometry: Switch to uint32_t to match BlockConf Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 19/32] scsi-hd: qdev properties for disk geometry Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 20/32] virtio-blk: " Markus Armbruster
2012-07-06  6:57 ` [Qemu-devel] [PATCH 21/32] ide: " Markus Armbruster
2012-07-06  6:58 ` [Qemu-devel] [PATCH 22/32] qtest: Cover " Markus Armbruster
2012-07-06  6:58 ` [Qemu-devel] [PATCH 23/32] qdev: Collect private helpers in one place Markus Armbruster
2012-07-06  6:58 ` [Qemu-devel] [PATCH 24/32] qdev: New property type chs-translation Markus Armbruster
2012-07-06  6:58 ` [Qemu-devel] [PATCH 25/32] ide: qdev property for BIOS CHS translation Markus Armbruster
2012-07-06  6:58 ` [Qemu-devel] [PATCH 26/32] qtest: Cover " Markus Armbruster
2012-07-06  6:58 ` Markus Armbruster [this message]
2012-07-06  6:58 ` [Qemu-devel] [PATCH 28/32] ide pc: Put hard disk info into CMOS only for hard disks Markus Armbruster
2012-07-06  6:58 ` [Qemu-devel] [PATCH 29/32] qtest: Test we don't put hard disk info into CMOS for a CD-ROM Markus Armbruster
2012-07-06  6:58 ` [Qemu-devel] [PATCH 30/32] hd-geometry: Compute BIOS CHS translation in one place Markus Armbruster
2012-07-06  6:58 ` [Qemu-devel] [PATCH 31/32] blockdev: Drop redundant CHS validation for if=ide Markus Armbruster
2012-07-06  6:58 ` [Qemu-devel] [PATCH 32/32] Relax IDE CHS limits from 16383, 16, 63 to 65535, 16, 255 Markus Armbruster
2012-07-06  7:28 ` [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1341557890-17464-28-git-send-email-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).