All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup
@ 2012-07-10  9:12 Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 01/29] fdc: Move floppy geometry guessing back from block.c Markus Armbruster
                   ` (29 more replies)
  0 siblings, 30 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

29 patches may look discouraging, but most patches are small, and the
ones that aren't just move code around.

Goals of this series:

1. One more step towards a clean separation block device host and
   guest part.

2. Purge CHS geometry from the block layer

Part I    [PATCH    01/29]: Floppy geometry

Part II   [PATCH 02-03/29]: vvfat geometry bug fixes

Part III  [PATCH 04-10/29]: Clean up hard disk geometry guessing code

Part IV   [PATCH 11-12/29]: Clean up CMOS hard disk info setup

Part V    [PATCH 13-24/29]: qdev properties for disk geometry

Part VI   [PATCH 25-29/29]: A few more fixes and cleanups

A few more cleanups are in the works, in particular geometry checking
code duplication pointed out by Kevin.

This patch series is also available at
git://repo.or.cz/qemu/armbru.git
tag geo-v3

v3: Rebase; drop the three patches that have been committed already
    Fix uninitialized variable in PATCH 02/29 (Anthony)

v2: New hw/block-common.h (Blue & Kevin)
    Coding style here & there (Blue)
    Tracepoint parameter types (Stefan)

Markus Armbruster (29):
  fdc: Move floppy geometry guessing back from block.c
  vvfat: Fix partition table
  vvfat: Do not clobber the user's geometry
  qtest: Add hard disk geometry test
  hd-geometry: Move disk geometry guessing back from block.c
  hd-geometry: Add tracepoints
  hd-geometry: Unnest conditional in hd_geometry_guess()
  hd-geometry: Factor out guess_chs_for_size()
  hd-geometry: Clean up gratuitous goto in hd_geometry_guess()
  hd-geometry: Clean up confusing use of prior translation hint
  hd-geometry: Cut out block layer translation middleman
  ide pc: Cut out the block layer geometry middleman
  blockdev: Save geometry in DriveInfo
  qdev: Introduce block geometry properties
  hd-geometry: Switch to uint32_t to match BlockConf
  scsi-hd: qdev properties for disk geometry
  virtio-blk: qdev properties for disk geometry
  ide: qdev properties for disk geometry
  qtest: Cover qdev properties for disk geometry
  qdev: Collect private helpers in one place
  qdev: New property type chs-translation
  ide: qdev property for BIOS CHS translation
  qtest: Cover qdev property for BIOS CHS translation
  block: Geometry and translation hints are now useless, purge them
  ide pc: Put hard disk info into CMOS only for hard disks
  qtest: Test we don't put hard disk info into CMOS for a CD-ROM
  hd-geometry: Compute BIOS CHS translation in one place
  blockdev: Drop redundant CHS validation for if=ide
  Relax IDE CHS limits from 16383,16,63 to 65535,16,255

 block.c              |  254 ------------------------------
 block.h              |   39 +----
 block/vvfat.c        |   57 ++++---
 block_int.h          |    1 -
 blockdev.c           |   24 +--
 blockdev.h           |    2 +
 hw/Makefile.objs     |    2 +-
 hw/block-common.h    |   29 ++++
 hw/fdc.c             |  122 +++++++++++++--
 hw/fdc.h             |   10 +-
 hw/hd-geometry.c     |  157 ++++++++++++++++++
 hw/ide.h             |    4 +-
 hw/ide/core.c        |   30 +++-
 hw/ide/internal.h    |    7 +-
 hw/ide/qdev.c        |   46 +++++-
 hw/pc.c              |   78 ++++------
 hw/qdev-properties.c |  160 ++++++++++---------
 hw/qdev.h            |    3 +
 hw/s390-virtio-bus.c |    1 +
 hw/scsi-disk.c       |   70 ++++++---
 hw/virtio-blk.c      |   42 ++++-
 hw/virtio-pci.c      |    1 +
 tests/Makefile       |    2 +
 tests/hd-geo-test.c  |  428 ++++++++++++++++++++++++++++++++++++++++++++++++++
 trace-events         |    4 +
 vl.c                 |    2 +-
 26 files changed, 1067 insertions(+), 508 deletions(-)
 create mode 100644 hw/block-common.h
 create mode 100644 hw/hd-geometry.c
 create mode 100644 tests/hd-geo-test.c

-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 01/29] fdc: Move floppy geometry guessing back from block.c
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 02/29] vvfat: Fix partition table Markus Armbruster
                   ` (28 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

Commit 5bbdbb46 moved it to block.c because "other geometry guessing
functions already reside in block.c".  Device-specific functionality
should be kept in device code, not the block layer.  Move it back.

Disk geometry guessing is still in block.c.  To be moved out in a
later patch series.

Bonus: the floppy type used in pc_cmos_init() now obviously matches
the one in the FDrive.  Before, we relied on
bdrv_get_floppy_geometry_hint() picking the same type both in
fd_revalidate() and in pc_cmos_init().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c  |  101 ---------------------------------------------------
 block.h  |   18 ---------
 hw/fdc.c |  122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/fdc.h |   10 +++++-
 hw/pc.c  |   11 +-----
 5 files changed, 123 insertions(+), 139 deletions(-)

diff --git a/block.c b/block.c
index 0c923f2..ffda1c2 100644
--- a/block.c
+++ b/block.c
@@ -2282,107 +2282,6 @@ void bdrv_set_io_limits(BlockDriverState *bs,
     bs->io_limits_enabled = bdrv_io_limits_enabled(bs);
 }
 
-/* Recognize floppy formats */
-typedef struct FDFormat {
-    FDriveType drive;
-    uint8_t last_sect;
-    uint8_t max_track;
-    uint8_t max_head;
-    FDriveRate rate;
-} FDFormat;
-
-static const FDFormat fd_formats[] = {
-    /* First entry is default format */
-    /* 1.44 MB 3"1/2 floppy disks */
-    { FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, },
-    /* 2.88 MB 3"1/2 floppy disks */
-    { FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, },
-    { FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, },
-    { FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, },
-    { FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, },
-    { FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, },
-    /* 720 kB 3"1/2 floppy disks */
-    { FDRIVE_DRV_144,  9, 80, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, },
-    /* 1.2 MB 5"1/4 floppy disks */
-    { FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, },
-    /* 720 kB 5"1/4 floppy disks */
-    { FDRIVE_DRV_120,  9, 80, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, },
-    /* 360 kB 5"1/4 floppy disks */
-    { FDRIVE_DRV_120,  9, 40, 1, FDRIVE_RATE_300K, },
-    { FDRIVE_DRV_120,  9, 40, 0, FDRIVE_RATE_300K, },
-    { FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, },
-    { FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, },
-    /* 320 kB 5"1/4 floppy disks */
-    { FDRIVE_DRV_120,  8, 40, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_120,  8, 40, 0, FDRIVE_RATE_250K, },
-    /* 360 kB must match 5"1/4 better than 3"1/2... */
-    { FDRIVE_DRV_144,  9, 80, 0, FDRIVE_RATE_250K, },
-    /* end */
-    { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
-};
-
-void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads,
-                                   int *max_track, int *last_sect,
-                                   FDriveType drive_in, FDriveType *drive,
-                                   FDriveRate *rate)
-{
-    const FDFormat *parse;
-    uint64_t nb_sectors, size;
-    int i, first_match, match;
-
-    bdrv_get_geometry(bs, &nb_sectors);
-    match = -1;
-    first_match = -1;
-    for (i = 0; ; i++) {
-        parse = &fd_formats[i];
-        if (parse->drive == FDRIVE_DRV_NONE) {
-            break;
-        }
-        if (drive_in == parse->drive ||
-            drive_in == FDRIVE_DRV_NONE) {
-            size = (parse->max_head + 1) * parse->max_track *
-                parse->last_sect;
-            if (nb_sectors == size) {
-                match = i;
-                break;
-            }
-            if (first_match == -1) {
-                first_match = i;
-            }
-        }
-    }
-    if (match == -1) {
-        if (first_match == -1) {
-            match = 1;
-        } else {
-            match = first_match;
-        }
-        parse = &fd_formats[match];
-    }
-    *nb_heads = parse->max_head + 1;
-    *max_track = parse->max_track;
-    *last_sect = parse->last_sect;
-    *drive = parse->drive;
-    *rate = parse->rate;
-}
-
 int bdrv_get_translation_hint(BlockDriverState *bs)
 {
     return bs->translation;
diff --git a/block.h b/block.h
index e34d942..b24f664 100644
--- a/block.h
+++ b/block.h
@@ -269,24 +269,6 @@ void bdrv_set_geometry_hint(BlockDriverState *bs,
 void bdrv_set_translation_hint(BlockDriverState *bs, int translation);
 void bdrv_get_geometry_hint(BlockDriverState *bs,
                             int *pcyls, int *pheads, int *psecs);
-typedef enum FDriveType {
-    FDRIVE_DRV_144  = 0x00,   /* 1.44 MB 3"5 drive      */
-    FDRIVE_DRV_288  = 0x01,   /* 2.88 MB 3"5 drive      */
-    FDRIVE_DRV_120  = 0x02,   /* 1.2  MB 5"25 drive     */
-    FDRIVE_DRV_NONE = 0x03,   /* No drive connected     */
-} FDriveType;
-
-typedef enum FDriveRate {
-    FDRIVE_RATE_500K = 0x00,  /* 500 Kbps */
-    FDRIVE_RATE_300K = 0x01,  /* 300 Kbps */
-    FDRIVE_RATE_250K = 0x02,  /* 250 Kbps */
-    FDRIVE_RATE_1M   = 0x03,  /*   1 Mbps */
-} FDriveRate;
-
-void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads,
-                                   int *max_track, int *last_sect,
-                                   FDriveType drive_in, FDriveType *drive,
-                                   FDriveRate *rate);
 int bdrv_get_translation_hint(BlockDriverState *bs);
 void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
                        BlockErrorAction on_write_error);
diff --git a/hw/fdc.c b/hw/fdc.c
index edf0706..41191c7 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -52,6 +52,113 @@
 /********************************************************/
 /* Floppy drive emulation                               */
 
+typedef enum FDriveRate {
+    FDRIVE_RATE_500K = 0x00,  /* 500 Kbps */
+    FDRIVE_RATE_300K = 0x01,  /* 300 Kbps */
+    FDRIVE_RATE_250K = 0x02,  /* 250 Kbps */
+    FDRIVE_RATE_1M   = 0x03,  /*   1 Mbps */
+} FDriveRate;
+
+typedef struct FDFormat {
+    FDriveType drive;
+    uint8_t last_sect;
+    uint8_t max_track;
+    uint8_t max_head;
+    FDriveRate rate;
+} FDFormat;
+
+static const FDFormat fd_formats[] = {
+    /* First entry is default format */
+    /* 1.44 MB 3"1/2 floppy disks */
+    { FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, },
+    { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
+    { FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, },
+    { FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, },
+    { FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, },
+    { FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, },
+    { FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, },
+    { FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, },
+    /* 2.88 MB 3"1/2 floppy disks */
+    { FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, },
+    { FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, },
+    { FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, },
+    { FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, },
+    { FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, },
+    /* 720 kB 3"1/2 floppy disks */
+    { FDRIVE_DRV_144,  9, 80, 1, FDRIVE_RATE_250K, },
+    { FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, },
+    { FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, },
+    { FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, },
+    { FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, },
+    { FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, },
+    /* 1.2 MB 5"1/4 floppy disks */
+    { FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, },
+    { FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, },
+    { FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, },
+    { FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, },
+    { FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, },
+    /* 720 kB 5"1/4 floppy disks */
+    { FDRIVE_DRV_120,  9, 80, 1, FDRIVE_RATE_250K, },
+    { FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, },
+    /* 360 kB 5"1/4 floppy disks */
+    { FDRIVE_DRV_120,  9, 40, 1, FDRIVE_RATE_300K, },
+    { FDRIVE_DRV_120,  9, 40, 0, FDRIVE_RATE_300K, },
+    { FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, },
+    { FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, },
+    /* 320 kB 5"1/4 floppy disks */
+    { FDRIVE_DRV_120,  8, 40, 1, FDRIVE_RATE_250K, },
+    { FDRIVE_DRV_120,  8, 40, 0, FDRIVE_RATE_250K, },
+    /* 360 kB must match 5"1/4 better than 3"1/2... */
+    { FDRIVE_DRV_144,  9, 80, 0, FDRIVE_RATE_250K, },
+    /* end */
+    { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
+};
+
+static void pick_geometry(BlockDriverState *bs, int *nb_heads,
+                          int *max_track, int *last_sect,
+                          FDriveType drive_in, FDriveType *drive,
+                          FDriveRate *rate)
+{
+    const FDFormat *parse;
+    uint64_t nb_sectors, size;
+    int i, first_match, match;
+
+    bdrv_get_geometry(bs, &nb_sectors);
+    match = -1;
+    first_match = -1;
+    for (i = 0; ; i++) {
+        parse = &fd_formats[i];
+        if (parse->drive == FDRIVE_DRV_NONE) {
+            break;
+        }
+        if (drive_in == parse->drive ||
+            drive_in == FDRIVE_DRV_NONE) {
+            size = (parse->max_head + 1) * parse->max_track *
+                parse->last_sect;
+            if (nb_sectors == size) {
+                match = i;
+                break;
+            }
+            if (first_match == -1) {
+                first_match = i;
+            }
+        }
+    }
+    if (match == -1) {
+        if (first_match == -1) {
+            match = 1;
+        } else {
+            match = first_match;
+        }
+        parse = &fd_formats[match];
+    }
+    *nb_heads = parse->max_head + 1;
+    *max_track = parse->max_track;
+    *last_sect = parse->last_sect;
+    *drive = parse->drive;
+    *rate = parse->rate;
+}
+
 #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
 #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive))
 
@@ -187,8 +294,8 @@ static void fd_revalidate(FDrive *drv)
     FLOPPY_DPRINTF("revalidate\n");
     if (drv->bs != NULL) {
         ro = bdrv_is_read_only(drv->bs);
-        bdrv_get_floppy_geometry_hint(drv->bs, &nb_heads, &max_track,
-                                      &last_sect, drv->drive, &drive, &rate);
+        pick_geometry(drv->bs, &nb_heads, &max_track,
+                      &last_sect, drv->drive, &drive, &rate);
         if (!bdrv_is_inserted(drv->bs)) {
             FLOPPY_DPRINTF("No disk in drive\n");
         } else {
@@ -2054,18 +2161,13 @@ static int sun4m_fdc_init1(SysBusDevice *dev)
     return fdctrl_init_common(fdctrl);
 }
 
-void fdc_get_bs(BlockDriverState *bs[], ISADevice *dev)
+FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
 {
-    FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
-    FDCtrl *fdctrl = &isa->state;
-    int i;
+    FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, fdc);
 
-    for (i = 0; i < MAX_FD; i++) {
-        bs[i] = fdctrl->drives[i].bs;
-    }
+    return isa->state.drives[i].drive;
 }
 
-
 static const VMStateDescription vmstate_isa_fdc ={
     .name = "fdc",
     .version_id = 2,
diff --git a/hw/fdc.h b/hw/fdc.h
index 1b32b17..b5c9f31 100644
--- a/hw/fdc.h
+++ b/hw/fdc.h
@@ -6,11 +6,19 @@
 /* fdc.c */
 #define MAX_FD 2
 
+typedef enum FDriveType {
+    FDRIVE_DRV_144  = 0x00,   /* 1.44 MB 3"5 drive      */
+    FDRIVE_DRV_288  = 0x01,   /* 2.88 MB 3"5 drive      */
+    FDRIVE_DRV_120  = 0x02,   /* 1.2  MB 5"25 drive     */
+    FDRIVE_DRV_NONE = 0x03,   /* No drive connected     */
+} FDriveType;
+
 ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
 void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
                         target_phys_addr_t mmio_base, DriveInfo **fds);
 void sun4m_fdctrl_init(qemu_irq irq, target_phys_addr_t io_base,
                        DriveInfo **fds, qemu_irq *fdc_tc);
-void fdc_get_bs(BlockDriverState *bs[], ISADevice *dev);
+
+FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
 
 #endif
diff --git a/hw/pc.c b/hw/pc.c
index c7e9ab3..91cf77d 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -335,10 +335,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
                   ISADevice *floppy, BusState *idebus0, BusState *idebus1,
                   ISADevice *s)
 {
-    int val, nb, nb_heads, max_track, last_sect, i;
+    int val, nb, i;
     FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
-    FDriveRate rate;
-    BlockDriverState *fd[MAX_FD];
     static pc_cmos_init_late_arg arg;
 
     /* various important CMOS locations needed by PC/Bochs bios */
@@ -381,13 +379,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
 
     /* floppy type */
     if (floppy) {
-        fdc_get_bs(fd, floppy);
         for (i = 0; i < 2; i++) {
-            if (fd[i]) {
-                bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
-                                              &last_sect, FDRIVE_DRV_NONE,
-                                              &fd_type[i], &rate);
-            }
+            fd_type[i] = isa_fdc_get_drive_type(floppy, i);
         }
     }
     val = (cmos_get_fd_drive_type(fd_type[0]) << 4) |
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 02/29] vvfat: Fix partition table
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 01/29] fdc: Move floppy geometry guessing back from block.c Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 03/29] vvfat: Do not clobber the user's geometry Markus Armbruster
                   ` (27 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

Unless parameter ":floppy:" is given, vvfat creates a virtual image
with DOS MBR defining a single partition which holds the FAT file
system.  The size of the virtual image depends on the width of the
FAT: 32 MiB (CHS 64, 16, 63) for 12 bit FAT, 504 MiB (CHS 1024, 16,
63) for 16 and 32 bit FAT, leaving (64*16-1)*63 = 64449 and
(1024*16-1)*64 = 1032129 sectors for the partition.

However, it screws up the end of the partition in the MBR:

    FAT width param.  start CHS  end CHS     start LBA  size
        :32:          0,1,1      1023,14,63       63    1032065
        :16:          0,1,1      1023,14,55       63    1032057
        :12:          0,1,1        63,14,55       63      64377

The actual FAT file system nevertheless assumes the partition has
1032129 or 64449 sectors.  Oops.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/vvfat.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 0fd3367..e2b83a2 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -394,11 +394,12 @@ static void init_mbr(BDRVVVFATState* s)
 
     /* LBA is used when partition is outside the CHS geometry */
     lba = sector2CHS(s->bs, &partition->start_CHS, s->first_sectors_number-1);
-    lba|= sector2CHS(s->bs, &partition->end_CHS,   s->sector_count);
+    lba |= sector2CHS(s->bs, &partition->end_CHS, s->bs->total_sectors - 1);
 
     /*LBA partitions are identified only by start/length_sector_long not by CHS*/
-    partition->start_sector_long =cpu_to_le32(s->first_sectors_number-1);
-    partition->length_sector_long=cpu_to_le32(s->sector_count - s->first_sectors_number+1);
+    partition->start_sector_long  = cpu_to_le32(s->first_sectors_number - 1);
+    partition->length_sector_long = cpu_to_le32(s->bs->total_sectors
+                                                - s->first_sectors_number + 1);
 
     /* FAT12/FAT16/FAT32 */
     /* DOS uses different types when partition is LBA,
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 03/29] vvfat: Do not clobber the user's geometry
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 01/29] fdc: Move floppy geometry guessing back from block.c Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 02/29] vvfat: Fix partition table Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 04/29] qtest: Add hard disk geometry test Markus Armbruster
                   ` (26 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

vvfat creates a virtual VFAT filesystem with a certain logical
geometry that depends on its options.  It sets the "geometry hint" to
this geometry.  It is the only block driver to do this.

The geometry hint is about about *physical* geometry, and used only by
certain hard disk device models.

vvfat's hint is normally invisible for device models, because
bdrv_open() puts a raw format on top of vvfat's fat protocol.  That
raw format is where drive_init() puts the user's geometry (if any),
and where the device model gets it from.

Nobody complained, because the default physical geometry is the same
as vvfat's logical geometry:

    opts        LCHS        def. PCHS
                1024,16,63  same
    :32:        1024,16,63  same
    :16:        1024,16,63  same
    :12:          64,16,63  same

Except when you specify :floppy:

    opts        LCHS        def. PCHS
       :floppy:   80, 2,36  5,16,63
    :32:floppy:   80, 2,36  5,16,63
    :16:floppy:   80, 2,36  5,16,63
    :12:floppy:   80, 2,18  2,16,63

Silly thing to do for use with a hard disk.

However, the "raw" format can be suppressed by adding an
redundant-looking "format=vvfat" to "file=fat:FOO".  Then, vvfat's
hint clobbers the user's geometry, i.e. -drive options cyls, heads,
secs get silently ignored.  Don't do that.

No change without format=vvfat.  With it, the user's hard disk
geometry (-drive options cyls, heads, secs) is now obeyed, and the
default hard disk geometry with :floppy: now matches the one without
format=vvfat.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/vvfat.c |   52 ++++++++++++++++++++++++++++------------------------
 1 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index e2b83a2..07d637b 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -359,11 +359,12 @@ typedef struct BDRVVVFATState {
  * if the position is outside the specified geometry, fill maximum value for CHS
  * and return 1 to signal overflow.
  */
-static int sector2CHS(BlockDriverState* bs, mbr_chs_t * chs, int spos){
+static int sector2CHS(mbr_chs_t *chs, int spos, int cyls, int heads, int secs)
+{
     int head,sector;
-    sector   = spos % (bs->secs);  spos/= bs->secs;
-    head     = spos % (bs->heads); spos/= bs->heads;
-    if(spos >= bs->cyls){
+    sector   = spos % secs;  spos /= secs;
+    head     = spos % heads; spos /= heads;
+    if (spos >= cyls) {
         /* Overflow,
         it happens if 32bit sector positions are used, while CHS is only 24bit.
         Windows/Dos is said to take 1023/255/63 as nonrepresentable CHS */
@@ -378,7 +379,7 @@ static int sector2CHS(BlockDriverState* bs, mbr_chs_t * chs, int spos){
     return 0;
 }
 
-static void init_mbr(BDRVVVFATState* s)
+static void init_mbr(BDRVVVFATState *s, int cyls, int heads, int secs)
 {
     /* TODO: if the files mbr.img and bootsect.img exist, use them */
     mbr_t* real_mbr=(mbr_t*)s->first_sectors;
@@ -393,8 +394,10 @@ static void init_mbr(BDRVVVFATState* s)
     partition->attributes=0x80; /* bootable */
 
     /* LBA is used when partition is outside the CHS geometry */
-    lba = sector2CHS(s->bs, &partition->start_CHS, s->first_sectors_number-1);
-    lba |= sector2CHS(s->bs, &partition->end_CHS, s->bs->total_sectors - 1);
+    lba  = sector2CHS(&partition->start_CHS, s->first_sectors_number - 1,
+                     cyls, heads, secs);
+    lba |= sector2CHS(&partition->end_CHS,   s->bs->total_sectors - 1,
+                     cyls, heads, secs);
 
     /*LBA partitions are identified only by start/length_sector_long not by CHS*/
     partition->start_sector_long  = cpu_to_le32(s->first_sectors_number - 1);
@@ -831,7 +834,7 @@ static inline off_t cluster2sector(BDRVVVFATState* s, uint32_t cluster_num)
 }
 
 static int init_directories(BDRVVVFATState* s,
-	const char* dirname)
+                            const char *dirname, int heads, int secs)
 {
     bootsector_t* bootsector;
     mapping_t* mapping;
@@ -958,8 +961,8 @@ static int init_directories(BDRVVVFATState* s,
     bootsector->media_type=(s->first_sectors_number>1?0xf8:0xf0); /* media descriptor (f8=hd, f0=3.5 fd)*/
     s->fat.pointer[0] = bootsector->media_type;
     bootsector->sectors_per_fat=cpu_to_le16(s->sectors_per_fat);
-    bootsector->sectors_per_track=cpu_to_le16(s->bs->secs);
-    bootsector->number_of_heads=cpu_to_le16(s->bs->heads);
+    bootsector->sectors_per_track = cpu_to_le16(secs);
+    bootsector->number_of_heads = cpu_to_le16(heads);
     bootsector->hidden_sectors=cpu_to_le32(s->first_sectors_number==1?0:0x3f);
     bootsector->total_sectors=cpu_to_le32(s->sector_count>0xffff?s->sector_count:0);
 
@@ -992,7 +995,7 @@ static void vvfat_rebind(BlockDriverState *bs)
 static int vvfat_open(BlockDriverState *bs, const char* dirname, int flags)
 {
     BDRVVVFATState *s = bs->opaque;
-    int i;
+    int i, cyls, heads, secs;
 
 #ifdef DEBUG
     vvv = s;
@@ -1034,24 +1037,28 @@ DLOG(if (stderr == NULL) {
 	/* 1.44MB or 2.88MB floppy.  2.88MB can be FAT12 (default) or FAT16. */
 	if (!s->fat_type) {
 	    s->fat_type = 12;
-	    bs->secs = 36;
+            secs = 36;
 	    s->sectors_per_cluster=2;
 	} else {
-	    bs->secs=(s->fat_type == 12 ? 18 : 36);
+            secs = s->fat_type == 12 ? 18 : 36;
 	    s->sectors_per_cluster=1;
 	}
 	s->first_sectors_number = 1;
-	bs->cyls=80; bs->heads=2;
+        cyls = 80;
+        heads = 2;
     } else {
 	/* 32MB or 504MB disk*/
 	if (!s->fat_type) {
 	    s->fat_type = 16;
 	}
-	bs->cyls=(s->fat_type == 12 ? 64 : 1024);
-	bs->heads=16; bs->secs=63;
+        cyls = s->fat_type == 12 ? 64 : 1024;
+        heads = 16;
+        secs = 63;
     }
+    fprintf(stderr, "vvfat %s chs %d,%d,%d\n",
+            dirname, cyls, heads, secs);
 
-    s->sector_count=bs->cyls*bs->heads*bs->secs-(s->first_sectors_number-1);
+    s->sector_count = cyls * heads * secs - (s->first_sectors_number - 1);
 
     if (strstr(dirname, ":rw:")) {
 	if (enable_write_target(s))
@@ -1067,19 +1074,16 @@ DLOG(if (stderr == NULL) {
     else
 	dirname += i+1;
 
-    bs->total_sectors=bs->cyls*bs->heads*bs->secs;
+    bs->total_sectors = cyls * heads * secs;
 
-    if(init_directories(s, dirname))
+    if (init_directories(s, dirname, heads, secs)) {
 	return -1;
+    }
 
     s->sector_count = s->faked_sectors + s->sectors_per_cluster*s->cluster_count;
 
     if(s->first_sectors_number==0x40)
-	init_mbr(s);
-    else {
-        /* MS-DOS does not like to know about CHS (?). */
-	bs->heads = bs->cyls = bs->secs = 0;
-    }
+        init_mbr(s, cyls, heads, secs);
 
     //    assert(is_consistent(s));
     qemu_co_mutex_init(&s->lock);
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 04/29] qtest: Add hard disk geometry test
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (2 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 03/29] vvfat: Do not clobber the user's geometry Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 05/29] hd-geometry: Move disk geometry guessing back from block.c Markus Armbruster
                   ` (25 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

So far covers only IDE and tests only CMOS contents.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/Makefile      |    2 +
 tests/hd-geo-test.c |  403 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 405 insertions(+), 0 deletions(-)
 create mode 100644 tests/hd-geo-test.c

diff --git a/tests/Makefile b/tests/Makefile
index d687ecc..9675ba7 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -21,6 +21,7 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 # All QTests for now are POSIX-only, but the dependencies are
 # really in libqtest, not in the testcases themselves.
 check-qtest-i386-y = tests/fdc-test$(EXESUF)
+check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
 check-qtest-i386-y += tests/rtc-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
@@ -72,6 +73,7 @@ tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(
 tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y)
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
 tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y)
+tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y)
 
 # QTest rules
 
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
new file mode 100644
index 0000000..cc447a2
--- /dev/null
+++ b/tests/hd-geo-test.c
@@ -0,0 +1,403 @@
+/*
+ * Hard disk geometry test cases.
+ *
+ * Copyright (c) 2012 Red Hat Inc.
+ *
+ * Authors:
+ *  Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+/*
+ * Covers only IDE and tests only CMOS contents.  Better than nothing.
+ * Improvements welcome.
+ */
+
+#include <glib.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include "qemu-common.h"
+#include "libqtest.h"
+
+static const char test_image[] = "/tmp/qtest.XXXXXX";
+
+static char *create_test_img(int secs)
+{
+    char *template = strdup("/tmp/qtest.XXXXXX");
+    int fd, ret;
+
+    fd = mkstemp(template);
+    g_assert(fd >= 0);
+    ret = ftruncate(fd, (off_t)secs * 512);
+    g_assert(ret == 0);
+    close(fd);
+    return template;
+}
+
+typedef struct {
+    int cyls, heads, secs, trans;
+} CHST;
+
+typedef enum {
+    mbr_blank, mbr_lba, mbr_chs,
+    mbr_last
+} MBRcontents;
+
+typedef enum {
+    /* order is relevant */
+    backend_small, backend_large, backend_empty,
+    backend_last
+} Backend;
+
+static const int img_secs[backend_last] = {
+    [backend_small] = 61440,
+    [backend_large] = 8388608,
+    [backend_empty] = -1,
+};
+
+static const CHST hd_chst[backend_last][mbr_last] = {
+    [backend_small] = {
+        [mbr_blank] = { 60, 16, 63, 0 },
+        [mbr_lba]   = { 60, 16, 63, 2 },
+        [mbr_chs]   = { 60, 16, 63, 0 }
+    },
+    [backend_large] = {
+        [mbr_blank] = { 8322, 16, 63, 1 },
+        [mbr_lba]   = { 8322, 16, 63, 1 },
+        [mbr_chs]   = { 8322, 16, 63, 0 }
+    },
+};
+
+static const char *img_file_name[backend_last];
+
+static const CHST *cur_ide[4];
+
+static bool is_hd(const CHST *expected_chst)
+{
+    return expected_chst && expected_chst->cyls;
+}
+
+static void test_cmos_byte(int reg, int expected)
+{
+    enum { cmos_base = 0x70 };
+    int actual;
+
+    outb(cmos_base + 0, reg);
+    actual = inb(cmos_base + 1);
+    g_assert(actual == expected);
+}
+
+static void test_cmos_bytes(int reg0, int n, uint8_t expected[])
+{
+    int i;
+
+    for (i = 0; i < 9; i++) {
+        test_cmos_byte(reg0 + i, expected[i]);
+    }
+}
+
+static void test_cmos_disk_data(void)
+{
+    test_cmos_byte(0x12,
+                   (is_hd(cur_ide[0]) ? 0xf0 : 0) |
+                   (is_hd(cur_ide[1]) ? 0x0f : 0));
+}
+
+static void test_cmos_drive_cyl(int reg0, const CHST *expected_chst)
+{
+    if (is_hd(expected_chst)) {
+        int c = expected_chst->cyls;
+        int h = expected_chst->heads;
+        int s = expected_chst->secs;
+        uint8_t expected_bytes[9] = {
+            c & 0xff, c >> 8, h, 0xff, 0xff, 0xc0 | ((h > 8) << 3),
+            c & 0xff, c >> 8, s
+        };
+        test_cmos_bytes(reg0, 9, expected_bytes);
+    } else {
+        int i;
+
+        for (i = 0; i < 9; i++) {
+            test_cmos_byte(reg0 + i, 0);
+        }
+    }
+}
+
+static void test_cmos_drive1(void)
+{
+    test_cmos_byte(0x19, is_hd(cur_ide[0]) ? 47 : 0);
+    test_cmos_drive_cyl(0x1b, cur_ide[0]);
+}
+
+static void test_cmos_drive2(void)
+{
+    test_cmos_byte(0x1a, is_hd(cur_ide[1]) ? 47 : 0);
+    test_cmos_drive_cyl(0x24, cur_ide[1]);
+}
+
+static void test_cmos_disktransflag(void)
+{
+    int val, i;
+
+    val = 0;
+    for (i = 0; i < ARRAY_SIZE(cur_ide); i++) {
+        if (is_hd(cur_ide[i])) {
+            val |= cur_ide[i]->trans << (2 * i);
+        }
+    }
+    test_cmos_byte(0x39, val);
+}
+
+static void test_cmos(void)
+{
+    test_cmos_disk_data();
+    test_cmos_drive1();
+    test_cmos_drive2();
+    test_cmos_disktransflag();
+}
+
+static int append_arg(int argc, char *argv[], int argv_sz, char *arg)
+{
+    g_assert(argc + 1 < argv_sz);
+    argv[argc++] = arg;
+    argv[argc] = NULL;
+    return argc;
+}
+
+static int setup_common(char *argv[], int argv_sz)
+{
+    memset(cur_ide, 0, sizeof(cur_ide));
+    return append_arg(0, argv, argv_sz,
+                      g_strdup("-nodefaults -display none"));
+}
+
+static void setup_mbr(int img_idx, MBRcontents mbr)
+{
+    static const uint8_t part_lba[16] = {
+        /* chs 0,1,1 (lba 63) to chs 0,127,63 (8001 sectors) */
+        0x80, 1, 1, 0, 6, 127, 63, 0, 63, 0, 0, 0, 0x41, 0x1F, 0, 0,
+    };
+    static const uint8_t part_chs[16] = {
+        /* chs 0,1,1 (lba 63) to chs 7,15,63 (8001 sectors) */
+        0x80, 1, 1, 0, 6,  15, 63, 7, 63, 0, 0, 0, 0x41, 0x1F, 0, 0,
+    };
+    uint8_t buf[512];
+    int fd, ret;
+
+    memset(buf, 0, sizeof(buf));
+
+    if (mbr != mbr_blank) {
+        buf[0x1fe] = 0x55;
+        buf[0x1ff] = 0xAA;
+        memcpy(buf + 0x1BE, mbr == mbr_lba ? part_lba : part_chs, 16);
+    }
+
+    fd = open(img_file_name[img_idx], O_WRONLY);
+    g_assert(fd >= 0);
+    ret = write(fd, buf, sizeof(buf));
+    g_assert(ret == sizeof(buf));
+    close(fd);
+}
+
+static int setup_ide(int argc, char *argv[], int argv_sz,
+                     int ide_idx, const char *dev, int img_idx,
+                     MBRcontents mbr, const char *opts)
+{
+    char *s1, *s2, *s3;
+
+    s1 = g_strdup_printf("-drive id=drive%d,if=%s",
+                         ide_idx, dev ? "none" : "ide");
+    s2 = dev ? g_strdup("") : g_strdup_printf(",index=%d", ide_idx);
+
+    if (img_secs[img_idx] >= 0) {
+        setup_mbr(img_idx, mbr);
+        s3 = g_strdup_printf(",file=%s", img_file_name[img_idx]);
+    } else {
+        s3 = g_strdup(",media=cdrom");
+    }
+    argc = append_arg(argc, argv, argv_sz,
+                      g_strdup_printf("%s%s%s%s", s1, s2, s3, opts));
+    g_free(s1);
+    g_free(s2);
+    g_free(s3);
+
+    if (dev) {
+        argc = append_arg(argc, argv, argv_sz,
+                          g_strdup_printf("-device %s,drive=drive%d,"
+                                          "bus=ide.%d,unit=%d",
+                                          dev, ide_idx,
+                                          ide_idx / 2, ide_idx % 2));
+    }
+    return argc;
+}
+
+/*
+ * Test case: no IDE devices
+ */
+static void test_ide_none(void)
+{
+    char *argv[256];
+
+    setup_common(argv, ARRAY_SIZE(argv));
+    qtest_start(g_strjoinv(" ", argv));
+    test_cmos();
+    qtest_quit(global_qtest);
+}
+
+static void test_ide_mbr(bool use_device, MBRcontents mbr)
+{
+    char *argv[256];
+    int argc;
+    Backend i;
+    const char *dev;
+
+    argc = setup_common(argv, ARRAY_SIZE(argv));
+    for (i = 0; i < backend_last; i++) {
+        cur_ide[i] = &hd_chst[i][mbr];
+        dev = use_device ? (is_hd(cur_ide[i]) ? "ide-hd" : "ide-cd") : NULL;
+        argc = setup_ide(argc, argv, ARRAY_SIZE(argv), i, dev, i, mbr, "");
+    }
+    qtest_start(g_strjoinv(" ", argv));
+    test_cmos();
+    qtest_quit(global_qtest);
+}
+
+/*
+ * Test case: IDE devices (if=ide) with blank MBRs
+ */
+static void test_ide_drive_mbr_blank(void)
+{
+    test_ide_mbr(false, mbr_blank);
+}
+
+/*
+ * Test case: IDE devices (if=ide) with MBRs indicating LBA is in use
+ */
+static void test_ide_drive_mbr_lba(void)
+{
+    test_ide_mbr(false, mbr_lba);
+}
+
+/*
+ * Test case: IDE devices (if=ide) with MBRs indicating CHS is in use
+ */
+static void test_ide_drive_mbr_chs(void)
+{
+    test_ide_mbr(false, mbr_chs);
+}
+
+/*
+ * Test case: IDE devices (if=none) with blank MBRs
+ */
+static void test_ide_device_mbr_blank(void)
+{
+    test_ide_mbr(true, mbr_blank);
+}
+
+/*
+ * Test case: IDE devices (if=none) with MBRs indicating LBA is in use
+ */
+static void test_ide_device_mbr_lba(void)
+{
+    test_ide_mbr(true, mbr_lba);
+}
+
+/*
+ * Test case: IDE devices (if=none) with MBRs indicating CHS is in use
+ */
+static void test_ide_device_mbr_chs(void)
+{
+    test_ide_mbr(true, mbr_chs);
+}
+
+static void test_ide_drive_user(const char *dev, bool trans)
+{
+    char *argv[256], *opts;
+    int argc;
+    int secs = img_secs[backend_small];
+    const CHST expected_chst = { secs / (4 * 32) , 4, 32, trans };
+
+    argc = setup_common(argv, ARRAY_SIZE(argv));
+    opts = g_strdup_printf(",cyls=%d,heads=%d,secs=%d%s",
+                           expected_chst.cyls, expected_chst.heads,
+                           expected_chst.secs,
+                           trans ? ",trans=lba" : "");
+    cur_ide[0] = &expected_chst;
+    argc = setup_ide(argc, argv, ARRAY_SIZE(argv),
+                     0, dev, backend_small, mbr_chs, opts);
+    g_free(opts);
+    qtest_start(g_strjoinv(" ", argv));
+    test_cmos();
+    qtest_quit(global_qtest);
+}
+
+/*
+ * Test case: IDE device (if=ide) with explicit CHS
+ */
+static void test_ide_drive_user_chs(void)
+{
+    test_ide_drive_user(NULL, false);
+}
+
+/*
+ * Test case: IDE device (if=ide) with explicit CHS and translation
+ */
+static void test_ide_drive_user_chst(void)
+{
+    test_ide_drive_user(NULL, true);
+}
+
+/*
+ * Test case: IDE device (if=none) with explicit CHS
+ */
+static void test_ide_device_user_chs(void)
+{
+    test_ide_drive_user("ide-hd", false);
+}
+
+/*
+ * Test case: IDE device (if=none) with explicit CHS and translation
+ */
+static void test_ide_device_user_chst(void)
+{
+    test_ide_drive_user("ide-hd", true);
+}
+
+int main(int argc, char **argv)
+{
+    Backend i;
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    for (i = 0; i < backend_last; i++) {
+        if (img_secs[i] >= 0) {
+            img_file_name[i] = create_test_img(img_secs[i]);
+        } else {
+            img_file_name[i] = NULL;
+        }
+    }
+
+    qtest_add_func("hd-geo/ide/none", test_ide_none);
+    qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
+    qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
+    qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
+    qtest_add_func("hd-geo/ide/drive/user/chs", test_ide_drive_user_chs);
+    qtest_add_func("hd-geo/ide/drive/user/chst", test_ide_drive_user_chst);
+    qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank);
+    qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
+    qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs);
+    qtest_add_func("hd-geo/ide/device/user/chs", test_ide_device_user_chs);
+    qtest_add_func("hd-geo/ide/device/user/chst", test_ide_device_user_chst);
+
+    ret = g_test_run();
+
+    for (i = 0; i < backend_last; i++) {
+        unlink(img_file_name[i]);
+    }
+
+    return ret;
+}
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 05/29] hd-geometry: Move disk geometry guessing back from block.c
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (3 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 04/29] qtest: Add hard disk geometry test Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-11 13:54   ` Kevin Wolf
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 06/29] hd-geometry: Add tracepoints Markus Armbruster
                   ` (24 subsequent siblings)
  29 siblings, 1 reply; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

Commit f3d54fc4 factored it out of hw/ide.c for reuse.  Sensible,
except it was put into block.c.  Device-specific functionality should
be kept in device code, not the block layer.  Move it to
hw/hd-geometry.c, and make stylistic changes required to keep
checkpatch.pl happy.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.c           |  121 ---------------------------------------
 block.h           |    1 -
 blockdev.h        |    1 +
 hw/Makefile.objs  |    2 +-
 hw/block-common.h |   21 +++++++
 hw/hd-geometry.c  |  162 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/ide/core.c     |    3 +-
 hw/scsi-disk.c    |    5 +-
 hw/virtio-blk.c   |    3 +-
 9 files changed, 192 insertions(+), 127 deletions(-)
 create mode 100644 hw/block-common.h
 create mode 100644 hw/hd-geometry.c

diff --git a/block.c b/block.c
index ffda1c2..06323cf 100644
--- a/block.c
+++ b/block.c
@@ -2132,127 +2132,6 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr)
     *nb_sectors_ptr = length;
 }
 
-struct partition {
-        uint8_t boot_ind;           /* 0x80 - active */
-        uint8_t head;               /* starting head */
-        uint8_t sector;             /* starting sector */
-        uint8_t cyl;                /* starting cylinder */
-        uint8_t sys_ind;            /* What partition type */
-        uint8_t end_head;           /* end head */
-        uint8_t end_sector;         /* end sector */
-        uint8_t end_cyl;            /* end cylinder */
-        uint32_t start_sect;        /* starting sector counting from 0 */
-        uint32_t nr_sects;          /* nr of sectors in partition */
-} QEMU_PACKED;
-
-/* try to guess the disk logical geometry from the MSDOS partition table. Return 0 if OK, -1 if could not guess */
-static int guess_disk_lchs(BlockDriverState *bs,
-                           int *pcylinders, int *pheads, int *psectors)
-{
-    uint8_t buf[BDRV_SECTOR_SIZE];
-    int i, heads, sectors, cylinders;
-    struct partition *p;
-    uint32_t nr_sects;
-    uint64_t nb_sectors;
-
-    bdrv_get_geometry(bs, &nb_sectors);
-
-    /**
-     * The function will be invoked during startup not only in sync I/O mode,
-     * but also in async I/O mode. So the I/O throttling function has to
-     * be disabled temporarily here, not permanently.
-     */
-    if (bdrv_read_unthrottled(bs, 0, buf, 1) < 0) {
-        return -1;
-    }
-    /* test msdos magic */
-    if (buf[510] != 0x55 || buf[511] != 0xaa)
-        return -1;
-    for(i = 0; i < 4; i++) {
-        p = ((struct partition *)(buf + 0x1be)) + i;
-        nr_sects = le32_to_cpu(p->nr_sects);
-        if (nr_sects && p->end_head) {
-            /* We make the assumption that the partition terminates on
-               a cylinder boundary */
-            heads = p->end_head + 1;
-            sectors = p->end_sector & 63;
-            if (sectors == 0)
-                continue;
-            cylinders = nb_sectors / (heads * sectors);
-            if (cylinders < 1 || cylinders > 16383)
-                continue;
-            *pheads = heads;
-            *psectors = sectors;
-            *pcylinders = cylinders;
-#if 0
-            printf("guessed geometry: LCHS=%d %d %d\n",
-                   cylinders, heads, sectors);
-#endif
-            return 0;
-        }
-    }
-    return -1;
-}
-
-void bdrv_guess_geometry(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs)
-{
-    int translation, lba_detected = 0;
-    int cylinders, heads, secs;
-    uint64_t nb_sectors;
-
-    /* if a geometry hint is available, use it */
-    bdrv_get_geometry(bs, &nb_sectors);
-    bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs);
-    translation = bdrv_get_translation_hint(bs);
-    if (cylinders != 0) {
-        *pcyls = cylinders;
-        *pheads = heads;
-        *psecs = secs;
-    } else {
-        if (guess_disk_lchs(bs, &cylinders, &heads, &secs) == 0) {
-            if (heads > 16) {
-                /* if heads > 16, it means that a BIOS LBA
-                   translation was active, so the default
-                   hardware geometry is OK */
-                lba_detected = 1;
-                goto default_geometry;
-            } else {
-                *pcyls = cylinders;
-                *pheads = heads;
-                *psecs = secs;
-                /* disable any translation to be in sync with
-                   the logical geometry */
-                if (translation == BIOS_ATA_TRANSLATION_AUTO) {
-                    bdrv_set_translation_hint(bs,
-                                              BIOS_ATA_TRANSLATION_NONE);
-                }
-            }
-        } else {
-        default_geometry:
-            /* if no geometry, use a standard physical disk geometry */
-            cylinders = nb_sectors / (16 * 63);
-
-            if (cylinders > 16383)
-                cylinders = 16383;
-            else if (cylinders < 2)
-                cylinders = 2;
-            *pcyls = cylinders;
-            *pheads = 16;
-            *psecs = 63;
-            if ((lba_detected == 1) && (translation == BIOS_ATA_TRANSLATION_AUTO)) {
-                if ((*pcyls * *pheads) <= 131072) {
-                    bdrv_set_translation_hint(bs,
-                                              BIOS_ATA_TRANSLATION_LARGE);
-                } else {
-                    bdrv_set_translation_hint(bs,
-                                              BIOS_ATA_TRANSLATION_LBA);
-                }
-            }
-        }
-        bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
-    }
-}
-
 void bdrv_set_geometry_hint(BlockDriverState *bs,
                             int cyls, int heads, int secs)
 {
diff --git a/block.h b/block.h
index b24f664..993894e 100644
--- a/block.h
+++ b/block.h
@@ -178,7 +178,6 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset);
 int64_t bdrv_getlength(BlockDriverState *bs);
 int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
 void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
-void bdrv_guess_geometry(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs);
 int bdrv_commit(BlockDriverState *bs);
 int bdrv_commit_all(void);
 int bdrv_change_backing_file(BlockDriverState *bs,
diff --git a/blockdev.h b/blockdev.h
index 260e16b..26454c9 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -62,4 +62,5 @@ void qmp_change_blockdev(const char *device, const char *filename,
                          bool has_format, const char *format, Error **errp);
 void do_commit(Monitor *mon, const QDict *qdict);
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
+
 #endif
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 9a350de..c3bdedc 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -138,7 +138,7 @@ common-obj-$(CONFIG_MAX111X) += max111x.o
 common-obj-$(CONFIG_DS1338) += ds1338.o
 common-obj-y += i2c.o smbus.o smbus_eeprom.o
 common-obj-y += eeprom93xx.o
-common-obj-y += scsi-disk.o cdrom.o
+common-obj-y += scsi-disk.o cdrom.o hd-geometry.o
 common-obj-y += scsi-generic.o scsi-bus.o
 common-obj-y += hid.o
 common-obj-$(CONFIG_SSI) += ssi.o
diff --git a/hw/block-common.h b/hw/block-common.h
new file mode 100644
index 0000000..3a4d4c6
--- /dev/null
+++ b/hw/block-common.h
@@ -0,0 +1,21 @@
+/*
+ * Common code for block device models
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#ifndef HW_BLOCK_COMMON_H
+#define HW_BLOCK_COMMON_H
+
+#include "qemu-common.h"
+
+/* Hard disk geometry */
+
+void hd_geometry_guess(BlockDriverState *bs,
+                       int *pcyls, int *pheads, int *psecs);
+
+#endif
diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
new file mode 100644
index 0000000..c45eafd
--- /dev/null
+++ b/hw/hd-geometry.c
@@ -0,0 +1,162 @@
+/*
+ * Hard disk geometry utilities
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * This file incorporates work covered by the following copyright and
+ * permission notice:
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "block.h"
+#include "hw/block-common.h"
+
+struct partition {
+        uint8_t boot_ind;           /* 0x80 - active */
+        uint8_t head;               /* starting head */
+        uint8_t sector;             /* starting sector */
+        uint8_t cyl;                /* starting cylinder */
+        uint8_t sys_ind;            /* What partition type */
+        uint8_t end_head;           /* end head */
+        uint8_t end_sector;         /* end sector */
+        uint8_t end_cyl;            /* end cylinder */
+        uint32_t start_sect;        /* starting sector counting from 0 */
+        uint32_t nr_sects;          /* nr of sectors in partition */
+} QEMU_PACKED;
+
+/* try to guess the disk logical geometry from the MSDOS partition table.
+   Return 0 if OK, -1 if could not guess */
+static int guess_disk_lchs(BlockDriverState *bs,
+                           int *pcylinders, int *pheads, int *psectors)
+{
+    uint8_t buf[BDRV_SECTOR_SIZE];
+    int i, heads, sectors, cylinders;
+    struct partition *p;
+    uint32_t nr_sects;
+    uint64_t nb_sectors;
+
+    bdrv_get_geometry(bs, &nb_sectors);
+
+    /**
+     * The function will be invoked during startup not only in sync I/O mode,
+     * but also in async I/O mode. So the I/O throttling function has to
+     * be disabled temporarily here, not permanently.
+     */
+    if (bdrv_read_unthrottled(bs, 0, buf, 1) < 0) {
+        return -1;
+    }
+    /* test msdos magic */
+    if (buf[510] != 0x55 || buf[511] != 0xaa) {
+        return -1;
+    }
+    for (i = 0; i < 4; i++) {
+        p = ((struct partition *)(buf + 0x1be)) + i;
+        nr_sects = le32_to_cpu(p->nr_sects);
+        if (nr_sects && p->end_head) {
+            /* We make the assumption that the partition terminates on
+               a cylinder boundary */
+            heads = p->end_head + 1;
+            sectors = p->end_sector & 63;
+            if (sectors == 0) {
+                continue;
+            }
+            cylinders = nb_sectors / (heads * sectors);
+            if (cylinders < 1 || cylinders > 16383) {
+                continue;
+            }
+            *pheads = heads;
+            *psectors = sectors;
+            *pcylinders = cylinders;
+#if 0
+            printf("guessed geometry: LCHS=%d %d %d\n",
+                   cylinders, heads, sectors);
+#endif
+            return 0;
+        }
+    }
+    return -1;
+}
+
+void hd_geometry_guess(BlockDriverState *bs,
+                       int *pcyls, int *pheads, int *psecs)
+{
+    int translation, lba_detected = 0;
+    int cylinders, heads, secs;
+    uint64_t nb_sectors;
+
+    /* if a geometry hint is available, use it */
+    bdrv_get_geometry(bs, &nb_sectors);
+    bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs);
+    translation = bdrv_get_translation_hint(bs);
+    if (cylinders != 0) {
+        *pcyls = cylinders;
+        *pheads = heads;
+        *psecs = secs;
+    } else {
+        if (guess_disk_lchs(bs, &cylinders, &heads, &secs) == 0) {
+            if (heads > 16) {
+                /* if heads > 16, it means that a BIOS LBA
+                   translation was active, so the default
+                   hardware geometry is OK */
+                lba_detected = 1;
+                goto default_geometry;
+            } else {
+                *pcyls = cylinders;
+                *pheads = heads;
+                *psecs = secs;
+                /* disable any translation to be in sync with
+                   the logical geometry */
+                if (translation == BIOS_ATA_TRANSLATION_AUTO) {
+                    bdrv_set_translation_hint(bs,
+                                              BIOS_ATA_TRANSLATION_NONE);
+                }
+            }
+        } else {
+        default_geometry:
+            /* if no geometry, use a standard physical disk geometry */
+            cylinders = nb_sectors / (16 * 63);
+
+            if (cylinders > 16383) {
+                cylinders = 16383;
+            } else if (cylinders < 2) {
+                cylinders = 2;
+            }
+            *pcyls = cylinders;
+            *pheads = 16;
+            *psecs = 63;
+            if ((lba_detected == 1)
+                && (translation == BIOS_ATA_TRANSLATION_AUTO)) {
+                if ((*pcyls * *pheads) <= 131072) {
+                    bdrv_set_translation_hint(bs,
+                                              BIOS_ATA_TRANSLATION_LARGE);
+                } else {
+                    bdrv_set_translation_hint(bs,
+                                              BIOS_ATA_TRANSLATION_LBA);
+                }
+            }
+        }
+        bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
+    }
+}
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 71d4d77..0d1bf10 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -30,6 +30,7 @@
 #include "qemu-timer.h"
 #include "sysemu.h"
 #include "dma.h"
+#include "hw/block-common.h"
 #include "blockdev.h"
 
 #include <hw/ide/internal.h>
@@ -1933,7 +1934,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
     s->drive_kind = kind;
 
     bdrv_get_geometry(bs, &nb_sectors);
-    bdrv_guess_geometry(bs, &cylinders, &heads, &secs);
+    hd_geometry_guess(bs, &cylinders, &heads, &secs);
     if (cylinders < 1 || cylinders > 16383) {
         error_report("cyls must be between 1 and 16383");
         return -1;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 34336b1..5339c2e 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -34,6 +34,7 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 #include "scsi-defs.h"
 #include "sysemu.h"
 #include "blockdev.h"
+#include "hw/block-common.h"
 #include "dma.h"
 
 #ifdef __linux
@@ -989,7 +990,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
             break;
         }
         /* if a geometry hint is available, use it */
-        bdrv_guess_geometry(bdrv, &cylinders, &heads, &secs);
+        hd_geometry_guess(bdrv, &cylinders, &heads, &secs);
         p[2] = (cylinders >> 16) & 0xff;
         p[3] = (cylinders >> 8) & 0xff;
         p[4] = cylinders & 0xff;
@@ -1023,7 +1024,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         p[2] = 5000 >> 8;
         p[3] = 5000 & 0xff;
         /* if a geometry hint is available, use it */
-        bdrv_guess_geometry(bdrv, &cylinders, &heads, &secs);
+        hd_geometry_guess(bdrv, &cylinders, &heads, &secs);
         p[4] = heads & 0xff;
         p[5] = secs & 0xff;
         p[6] = s->qdev.blocksize >> 8;
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index fe07746..f16c5ce 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -14,6 +14,7 @@
 #include "qemu-common.h"
 #include "qemu-error.h"
 #include "trace.h"
+#include "hw/block-common.h"
 #include "blockdev.h"
 #include "virtio-blk.h"
 #include "scsi-defs.h"
@@ -622,7 +623,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     s->blk = blk;
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
-    bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
+    hd_geometry_guess(s->bs, &cylinders, &heads, &secs);
 
     s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
 
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 06/29] hd-geometry: Add tracepoints
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (4 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 05/29] hd-geometry: Move disk geometry guessing back from block.c Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 07/29] hd-geometry: Unnest conditional in hd_geometry_guess() Markus Armbruster
                   ` (23 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/hd-geometry.c |    7 +++----
 trace-events     |    4 ++++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index c45eafd..f0dd021 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -32,6 +32,7 @@
 
 #include "block.h"
 #include "hw/block-common.h"
+#include "trace.h"
 
 struct partition {
         uint8_t boot_ind;           /* 0x80 - active */
@@ -89,10 +90,7 @@ static int guess_disk_lchs(BlockDriverState *bs,
             *pheads = heads;
             *psectors = sectors;
             *pcylinders = cylinders;
-#if 0
-            printf("guessed geometry: LCHS=%d %d %d\n",
-                   cylinders, heads, sectors);
-#endif
+            trace_hd_geometry_lchs_guess(bs, cylinders, heads, sectors);
             return 0;
         }
     }
@@ -159,4 +157,5 @@ void hd_geometry_guess(BlockDriverState *bs,
         }
         bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
     }
+    trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation);
 }
diff --git a/trace-events b/trace-events
index 1f9fc98..acef082 100644
--- a/trace-events
+++ b/trace-events
@@ -141,6 +141,10 @@ ecc_mem_readl_ecr1(uint32_t ret) "Read event count 2 %08x"
 ecc_diag_mem_writeb(uint64_t addr, uint32_t val) "Write diagnostic %"PRId64" = %02x"
 ecc_diag_mem_readb(uint64_t addr, uint32_t ret) "Read diagnostic %"PRId64"= %02x"
 
+# hw/hd-geometry.c
+hd_geometry_lchs_guess(void *bs, int cyls, int heads, int secs) "bs %p LCHS %d %d %d"
+hd_geometry_guess(void *bs, int cyls, int heads, int secs, int trans) "bs %p CHS %d %d %d trans %d"
+
 # hw/jazz-led.c
 jazz_led_read(uint64_t addr, uint8_t val) "read addr=0x%"PRIx64": 0x%x"
 jazz_led_write(uint64_t addr, uint8_t new) "write addr=0x%"PRIx64": 0x%x"
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 07/29] hd-geometry: Unnest conditional in hd_geometry_guess()
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (5 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 06/29] hd-geometry: Add tracepoints Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 08/29] hd-geometry: Factor out guess_chs_for_size() Markus Armbruster
                   ` (22 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/hd-geometry.c |   84 +++++++++++++++++++++++++++---------------------------
 1 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index f0dd021..db47846 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -104,58 +104,58 @@ void hd_geometry_guess(BlockDriverState *bs,
     int cylinders, heads, secs;
     uint64_t nb_sectors;
 
-    /* if a geometry hint is available, use it */
     bdrv_get_geometry(bs, &nb_sectors);
     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;
-    } else {
-        if (guess_disk_lchs(bs, &cylinders, &heads, &secs) == 0) {
-            if (heads > 16) {
-                /* if heads > 16, it means that a BIOS LBA
-                   translation was active, so the default
-                   hardware geometry is OK */
-                lba_detected = 1;
-                goto default_geometry;
-            } else {
-                *pcyls = cylinders;
-                *pheads = heads;
-                *psecs = secs;
-                /* disable any translation to be in sync with
-                   the logical geometry */
-                if (translation == BIOS_ATA_TRANSLATION_AUTO) {
-                    bdrv_set_translation_hint(bs,
-                                              BIOS_ATA_TRANSLATION_NONE);
-                }
-            }
-        } else {
-        default_geometry:
-            /* if no geometry, use a standard physical disk geometry */
-            cylinders = nb_sectors / (16 * 63);
+        return;
+    }
 
-            if (cylinders > 16383) {
-                cylinders = 16383;
-            } else if (cylinders < 2) {
-                cylinders = 2;
-            }
-            *pcyls = cylinders;
-            *pheads = 16;
-            *psecs = 63;
-            if ((lba_detected == 1)
-                && (translation == BIOS_ATA_TRANSLATION_AUTO)) {
-                if ((*pcyls * *pheads) <= 131072) {
-                    bdrv_set_translation_hint(bs,
-                                              BIOS_ATA_TRANSLATION_LARGE);
-                } else {
-                    bdrv_set_translation_hint(bs,
-                                              BIOS_ATA_TRANSLATION_LBA);
-                }
+    if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) {
+        /* no LCHS guess: use a standard physical disk geometry  */
+    default_geometry:
+        cylinders = nb_sectors / (16 * 63);
+
+        if (cylinders > 16383) {
+            cylinders = 16383;
+        } else if (cylinders < 2) {
+            cylinders = 2;
+        }
+        *pcyls = cylinders;
+        *pheads = 16;
+        *psecs = 63;
+        if ((lba_detected == 1) && (translation == BIOS_ATA_TRANSLATION_AUTO)) {
+            if ((*pcyls * *pheads) <= 131072) {
+                bdrv_set_translation_hint(bs,
+                                          BIOS_ATA_TRANSLATION_LARGE);
+            } else {
+                bdrv_set_translation_hint(bs,
+                                          BIOS_ATA_TRANSLATION_LBA);
             }
         }
-        bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
+    } else if (heads > 16) {
+        /* LCHS guess with heads > 16 means that a BIOS LBA
+           translation was active, so a standard physical disk
+           geometry is OK */
+        lba_detected = 1;
+        goto default_geometry;
+    } else {
+        /* LCHS guess with heads <= 16: use as physical geometry */
+        *pcyls = cylinders;
+        *pheads = heads;
+        *psecs = secs;
+        /* disable any translation to be in sync with
+           the logical geometry */
+        if (translation == BIOS_ATA_TRANSLATION_AUTO) {
+            bdrv_set_translation_hint(bs,
+                                      BIOS_ATA_TRANSLATION_NONE);
+        }
     }
+    bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
     trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation);
 }
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 08/29] hd-geometry: Factor out guess_chs_for_size()
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (6 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 07/29] hd-geometry: Unnest conditional in hd_geometry_guess() Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 09/29] hd-geometry: Clean up gratuitous goto in hd_geometry_guess() Markus Armbruster
                   ` (21 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/hd-geometry.c |   32 ++++++++++++++++++++------------
 1 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index db47846..1a58894 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -97,14 +97,31 @@ static int guess_disk_lchs(BlockDriverState *bs,
     return -1;
 }
 
+static void guess_chs_for_size(BlockDriverState *bs,
+                               int *pcyls, int *pheads, int *psecs)
+{
+    uint64_t nb_sectors;
+    int cylinders;
+
+    bdrv_get_geometry(bs, &nb_sectors);
+
+    cylinders = nb_sectors / (16 * 63);
+    if (cylinders > 16383) {
+        cylinders = 16383;
+    } else if (cylinders < 2) {
+        cylinders = 2;
+    }
+    *pcyls = cylinders;
+    *pheads = 16;
+    *psecs = 63;
+}
+
 void hd_geometry_guess(BlockDriverState *bs,
                        int *pcyls, int *pheads, int *psecs)
 {
     int translation, lba_detected = 0;
     int cylinders, heads, secs;
-    uint64_t nb_sectors;
 
-    bdrv_get_geometry(bs, &nb_sectors);
     bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs);
     translation = bdrv_get_translation_hint(bs);
 
@@ -119,16 +136,7 @@ void hd_geometry_guess(BlockDriverState *bs,
     if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) {
         /* no LCHS guess: use a standard physical disk geometry  */
     default_geometry:
-        cylinders = nb_sectors / (16 * 63);
-
-        if (cylinders > 16383) {
-            cylinders = 16383;
-        } else if (cylinders < 2) {
-            cylinders = 2;
-        }
-        *pcyls = cylinders;
-        *pheads = 16;
-        *psecs = 63;
+        guess_chs_for_size(bs, pcyls, pheads, psecs);
         if ((lba_detected == 1) && (translation == BIOS_ATA_TRANSLATION_AUTO)) {
             if ((*pcyls * *pheads) <= 131072) {
                 bdrv_set_translation_hint(bs,
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 09/29] hd-geometry: Clean up gratuitous goto in hd_geometry_guess()
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (7 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 08/29] hd-geometry: Factor out guess_chs_for_size() Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 10/29] hd-geometry: Clean up confusing use of prior translation hint Markus Armbruster
                   ` (20 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/hd-geometry.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 1a58894..fb849a3 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -119,8 +119,7 @@ static void guess_chs_for_size(BlockDriverState *bs,
 void hd_geometry_guess(BlockDriverState *bs,
                        int *pcyls, int *pheads, int *psecs)
 {
-    int translation, lba_detected = 0;
-    int cylinders, heads, secs;
+    int cylinders, heads, secs, translation;
 
     bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs);
     translation = bdrv_get_translation_hint(bs);
@@ -135,23 +134,18 @@ void hd_geometry_guess(BlockDriverState *bs,
 
     if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) {
         /* no LCHS guess: use a standard physical disk geometry  */
-    default_geometry:
         guess_chs_for_size(bs, pcyls, pheads, psecs);
-        if ((lba_detected == 1) && (translation == BIOS_ATA_TRANSLATION_AUTO)) {
-            if ((*pcyls * *pheads) <= 131072) {
-                bdrv_set_translation_hint(bs,
-                                          BIOS_ATA_TRANSLATION_LARGE);
-            } else {
-                bdrv_set_translation_hint(bs,
-                                          BIOS_ATA_TRANSLATION_LBA);
-            }
-        }
     } else if (heads > 16) {
         /* LCHS guess with heads > 16 means that a BIOS LBA
            translation was active, so a standard physical disk
            geometry is OK */
-        lba_detected = 1;
-        goto default_geometry;
+        guess_chs_for_size(bs, pcyls, pheads, psecs);
+        if (translation == BIOS_ATA_TRANSLATION_AUTO) {
+            bdrv_set_translation_hint(bs,
+                                      *pcyls * *pheads <= 131072
+                                      ? BIOS_ATA_TRANSLATION_LARGE
+                                      : BIOS_ATA_TRANSLATION_LBA);
+        }
     } else {
         /* LCHS guess with heads <= 16: use as physical geometry */
         *pcyls = cylinders;
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 10/29] hd-geometry: Clean up confusing use of prior translation hint
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (8 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 09/29] hd-geometry: Clean up gratuitous goto in hd_geometry_guess() Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 11/29] hd-geometry: Cut out block layer translation middleman Markus Armbruster
                   ` (19 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

When hd_geometry_guess() picks a geometry, it also picks the
appropriate translation, but only when the prior translation hint is
BIOS_ATA_TRANSLATION_AUTO.  Looks wrong, because such a prior
translation would be passed to the BIOS whether it's suitable for the
geometry or not.

Fortunately, that can't happen.  There are just two ways for the
translation hint to get set to something other than
BIOS_ATA_TRANSLATION_AUTO: drive_init() on behalf of -drive trans=...,
and hd_geometry_guess().  Both set it only when they also set a valid
geometry hint, i.e. one with a non-zero number of cylinders.

Since hd_geometry_guess() returns right away when it finds a valid
geometry hint, translation can only be BIOS_ATA_TRANSLATION_AUTO in
the remainder of the function.

Assert this, and simplify accordingly.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/hd-geometry.c |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index fb849a3..241aed9 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -132,6 +132,8 @@ void hd_geometry_guess(BlockDriverState *bs,
         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);
@@ -140,12 +142,10 @@ void hd_geometry_guess(BlockDriverState *bs,
            translation was active, so a standard physical disk
            geometry is OK */
         guess_chs_for_size(bs, pcyls, pheads, psecs);
-        if (translation == BIOS_ATA_TRANSLATION_AUTO) {
-            bdrv_set_translation_hint(bs,
-                                      *pcyls * *pheads <= 131072
-                                      ? BIOS_ATA_TRANSLATION_LARGE
-                                      : BIOS_ATA_TRANSLATION_LBA);
-        }
+        bdrv_set_translation_hint(bs,
+                                  *pcyls * *pheads <= 131072
+                                  ? BIOS_ATA_TRANSLATION_LARGE
+                                  : BIOS_ATA_TRANSLATION_LBA);
     } else {
         /* LCHS guess with heads <= 16: use as physical geometry */
         *pcyls = cylinders;
@@ -153,10 +153,7 @@ void hd_geometry_guess(BlockDriverState *bs,
         *psecs = secs;
         /* disable any translation to be in sync with
            the logical geometry */
-        if (translation == BIOS_ATA_TRANSLATION_AUTO) {
-            bdrv_set_translation_hint(bs,
-                                      BIOS_ATA_TRANSLATION_NONE);
-        }
+        bdrv_set_translation_hint(bs, BIOS_ATA_TRANSLATION_NONE);
     }
     bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
     trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation);
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 11/29] hd-geometry: Cut out block layer translation middleman
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (9 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 10/29] hd-geometry: Clean up confusing use of prior translation hint Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 12/29] ide pc: Cut out the block layer geometry middleman Markus Armbruster
                   ` (18 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

hd_geometry_guess() picks geometry and translation.  Callers can get
the geometry directly, via parameters, but for translation they need
to go through the block layer.

Add a parameter for translation, so it can optionally be gotten just
like geometry.  In preparation of purging translation from the block
layer, which will happen later in this series.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/block-common.h |    3 ++-
 hw/hd-geometry.c  |   20 ++++++++++++++------
 hw/ide/core.c     |    2 +-
 hw/scsi-disk.c    |    4 ++--
 hw/virtio-blk.c   |    2 +-
 5 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/hw/block-common.h b/hw/block-common.h
index 3a4d4c6..bba817a 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -16,6 +16,7 @@
 /* Hard disk geometry */
 
 void hd_geometry_guess(BlockDriverState *bs,
-                       int *pcyls, int *pheads, int *psecs);
+                       int *pcyls, int *pheads, int *psecs,
+                       int *ptrans);
 
 #endif
diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 241aed9..4d746b7 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -117,7 +117,8 @@ static void guess_chs_for_size(BlockDriverState *bs,
 }
 
 void hd_geometry_guess(BlockDriverState *bs,
-                       int *pcyls, int *pheads, int *psecs)
+                       int *pcyls, int *pheads, int *psecs,
+                       int *ptrans)
 {
     int cylinders, heads, secs, translation;
 
@@ -129,6 +130,9 @@ void hd_geometry_guess(BlockDriverState *bs,
         *pcyls = cylinders;
         *pheads = heads;
         *psecs = secs;
+        if (ptrans) {
+            *ptrans = translation;
+        }
         return;
     }
 
@@ -142,10 +146,10 @@ void hd_geometry_guess(BlockDriverState *bs,
            translation was active, so a standard physical disk
            geometry is OK */
         guess_chs_for_size(bs, pcyls, pheads, psecs);
-        bdrv_set_translation_hint(bs,
-                                  *pcyls * *pheads <= 131072
-                                  ? BIOS_ATA_TRANSLATION_LARGE
-                                  : BIOS_ATA_TRANSLATION_LBA);
+        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;
@@ -153,7 +157,11 @@ void hd_geometry_guess(BlockDriverState *bs,
         *psecs = secs;
         /* disable any translation to be in sync with
            the logical geometry */
-        bdrv_set_translation_hint(bs, BIOS_ATA_TRANSLATION_NONE);
+        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/ide/core.c b/hw/ide/core.c
index 0d1bf10..28f04ad 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1934,7 +1934,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
     s->drive_kind = kind;
 
     bdrv_get_geometry(bs, &nb_sectors);
-    hd_geometry_guess(bs, &cylinders, &heads, &secs);
+    hd_geometry_guess(bs, &cylinders, &heads, &secs, NULL);
     if (cylinders < 1 || cylinders > 16383) {
         error_report("cyls must be between 1 and 16383");
         return -1;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 5339c2e..fc077f5 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -990,7 +990,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
             break;
         }
         /* if a geometry hint is available, use it */
-        hd_geometry_guess(bdrv, &cylinders, &heads, &secs);
+        hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL);
         p[2] = (cylinders >> 16) & 0xff;
         p[3] = (cylinders >> 8) & 0xff;
         p[4] = cylinders & 0xff;
@@ -1024,7 +1024,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         p[2] = 5000 >> 8;
         p[3] = 5000 & 0xff;
         /* if a geometry hint is available, use it */
-        hd_geometry_guess(bdrv, &cylinders, &heads, &secs);
+        hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL);
         p[4] = heads & 0xff;
         p[5] = secs & 0xff;
         p[6] = s->qdev.blocksize >> 8;
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index f16c5ce..d2709a7 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -623,7 +623,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     s->blk = blk;
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
-    hd_geometry_guess(s->bs, &cylinders, &heads, &secs);
+    hd_geometry_guess(s->bs, &cylinders, &heads, &secs, NULL);
 
     s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
 
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 12/29] ide pc: Cut out the block layer geometry middleman
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (10 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 11/29] hd-geometry: Cut out block layer translation middleman Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 13/29] blockdev: Save geometry in DriveInfo Markus Armbruster
                   ` (17 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

PC BIOS setup needs IDE geometry information.  Get it directly from
the device model rather than through the block layer.  In preparation
of purging geometry from the block layer, which will happen later in
this series.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide.h          |    4 +++-
 hw/ide/core.c     |    2 +-
 hw/ide/internal.h |    2 +-
 hw/ide/qdev.c     |   21 +++++++++++++++++----
 hw/pc.c           |   51 +++++++++++++++++++++++++++------------------------
 5 files changed, 49 insertions(+), 31 deletions(-)

diff --git a/hw/ide.h b/hw/ide.h
index 0b18c90..2db4079 100644
--- a/hw/ide.h
+++ b/hw/ide.h
@@ -29,7 +29,9 @@ void mmio_ide_init (target_phys_addr_t membase, target_phys_addr_t membase2,
                     qemu_irq irq, int shift,
                     DriveInfo *hd0, DriveInfo *hd1);
 
-void ide_get_bs(BlockDriverState *bs[], BusState *qbus);
+int ide_get_geometry(BusState *bus, int unit,
+                     int16_t *cyls, int8_t *heads, int8_t *secs);
+int ide_get_bios_chs_trans(BusState *bus, int unit);
 
 /* ide/core.c */
 void ide_drive_get(DriveInfo **hd, int max_bus);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 28f04ad..7f5ad07 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1934,7 +1934,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
     s->drive_kind = kind;
 
     bdrv_get_geometry(bs, &nb_sectors);
-    hd_geometry_guess(bs, &cylinders, &heads, &secs, NULL);
+    hd_geometry_guess(bs, &cylinders, &heads, &secs, &s->chs_trans);
     if (cylinders < 1 || cylinders > 16383) {
         error_report("cyls must be between 1 and 16383");
         return -1;
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 1a02f57..56c718e 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -344,7 +344,7 @@ struct IDEState {
     uint8_t unit;
     /* ide config */
     IDEDriveKind drive_kind;
-    int cylinders, heads, sectors;
+    int cylinders, heads, sectors, chs_trans;
     int64_t nb_sectors;
     int mult_sectors;
     int identify_set;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index c122395..87e0b75 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -111,11 +111,24 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
     return DO_UPCAST(IDEDevice, qdev, dev);
 }
 
-void ide_get_bs(BlockDriverState *bs[], BusState *qbus)
+int ide_get_geometry(BusState *bus, int unit,
+                     int16_t *cyls, int8_t *heads, int8_t *secs)
 {
-    IDEBus *bus = DO_UPCAST(IDEBus, qbus, qbus);
-    bs[0] = bus->master ? bus->master->conf.bs : NULL;
-    bs[1] = bus->slave  ? bus->slave->conf.bs  : NULL;
+    IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
+
+    if (!s->bs) {
+        return -1;
+    }
+
+    *cyls = s->cylinders;
+    *heads = s->heads;
+    *secs = s->sectors;
+    return 0;
+}
+
+int ide_get_bios_chs_trans(BusState *bus, int unit)
+{
+    return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans;
 }
 
 /* --------------------------------- */
diff --git a/hw/pc.c b/hw/pc.c
index 91cf77d..89a0c66 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -216,11 +216,9 @@ static int cmos_get_fd_drive_type(FDriveType fd0)
     return val;
 }
 
-static void cmos_init_hd(int type_ofs, int info_ofs, BlockDriverState *hd,
-                         ISADevice *s)
+static void cmos_init_hd(ISADevice *s, int type_ofs, int info_ofs,
+                         int16_t cylinders, int8_t heads, int8_t sectors)
 {
-    int cylinders, heads, sectors;
-    bdrv_get_geometry_hint(hd, &cylinders, &heads, &sectors);
     rtc_set_memory(s, type_ofs, 47);
     rtc_set_memory(s, info_ofs, cylinders);
     rtc_set_memory(s, info_ofs + 1, cylinders >> 8);
@@ -281,37 +279,42 @@ static int pc_boot_set(void *opaque, const char *boot_device)
 
 typedef struct pc_cmos_init_late_arg {
     ISADevice *rtc_state;
-    BusState *idebus0, *idebus1;
+    BusState *idebus[2];
 } pc_cmos_init_late_arg;
 
 static void pc_cmos_init_late(void *opaque)
 {
     pc_cmos_init_late_arg *arg = opaque;
     ISADevice *s = arg->rtc_state;
+    int16_t cylinders;
+    int8_t heads, sectors;
     int val;
-    BlockDriverState *hd_table[4];
     int i;
 
-    ide_get_bs(hd_table, arg->idebus0);
-    ide_get_bs(hd_table + 2, arg->idebus1);
-
-    rtc_set_memory(s, 0x12, (hd_table[0] ? 0xf0 : 0) | (hd_table[1] ? 0x0f : 0));
-    if (hd_table[0])
-        cmos_init_hd(0x19, 0x1b, hd_table[0], s);
-    if (hd_table[1])
-        cmos_init_hd(0x1a, 0x24, hd_table[1], s);
+    val = 0;
+    if (ide_get_geometry(arg->idebus[0], 0,
+                         &cylinders, &heads, &sectors) >= 0) {
+        cmos_init_hd(s, 0x19, 0x1b, cylinders, heads, sectors);
+        val |= 0xf0;
+    }
+    if (ide_get_geometry(arg->idebus[0], 1,
+                         &cylinders, &heads, &sectors) >= 0) {
+        cmos_init_hd(s, 0x1a, 0x24, cylinders, heads, sectors);
+        val |= 0x0f;
+    }
+    rtc_set_memory(s, 0x12, val);
 
     val = 0;
     for (i = 0; i < 4; i++) {
-        if (hd_table[i]) {
-            int cylinders, heads, sectors, translation;
-            /* NOTE: bdrv_get_geometry_hint() returns the physical
-                geometry.  It is always such that: 1 <= sects <= 63, 1
-                <= heads <= 16, 1 <= cylinders <= 16383. The BIOS
-                geometry can be different if a translation is done. */
-            translation = bdrv_get_translation_hint(hd_table[i]);
+        /* NOTE: ide_get_geometry() returns the physical
+           geometry.  It is always such that: 1 <= sects <= 63, 1
+           <= heads <= 16, 1 <= cylinders <= 16383. The BIOS
+           geometry can be different if a translation is done. */
+        if (ide_get_geometry(arg->idebus[i / 2], i % 2,
+                             &cylinders, &heads, &sectors) >= 0) {
+            int translation = ide_get_bios_chs_trans(arg->idebus[i / 2],
+                                                     i % 2);
             if (translation == BIOS_ATA_TRANSLATION_AUTO) {
-                bdrv_get_geometry_hint(hd_table[i], &cylinders, &heads, &sectors);
                 if (cylinders <= 1024 && heads <= 16 && sectors <= 63) {
                     /* No translation. */
                     translation = 0;
@@ -411,8 +414,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
 
     /* hard drives */
     arg.rtc_state = s;
-    arg.idebus0 = idebus0;
-    arg.idebus1 = idebus1;
+    arg.idebus[0] = idebus0;
+    arg.idebus[1] = idebus1;
     qemu_register_reset(pc_cmos_init_late, &arg);
 }
 
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 13/29] blockdev: Save geometry in DriveInfo
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (11 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 12/29] ide pc: Cut out the block layer geometry middleman Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 14/29] qdev: Introduce block geometry properties Markus Armbruster
                   ` (16 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

In preparation of purging it from the block layer, which will happen
later in this series.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 blockdev.c |    4 ++++
 blockdev.h |    1 +
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index a85a429..161985b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -530,6 +530,10 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
     dinfo->type = type;
     dinfo->bus = bus_id;
     dinfo->unit = unit_id;
+    dinfo->cyls = cyls;
+    dinfo->heads = heads;
+    dinfo->secs = secs;
+    dinfo->trans = translation;
     dinfo->opts = opts;
     dinfo->refcount = 1;
     if (serial) {
diff --git a/blockdev.h b/blockdev.h
index 26454c9..bc8c2dc 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -35,6 +35,7 @@ struct DriveInfo {
     int unit;
     int auto_del;               /* see blockdev_mark_auto_del() */
     int media_cd;
+    int cyls, heads, secs, trans;
     QemuOpts *opts;
     char serial[BLOCK_SERIAL_STRLEN + 1];
     QTAILQ_ENTRY(DriveInfo) next;
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 14/29] qdev: Introduce block geometry properties
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (12 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 13/29] blockdev: Save geometry in DriveInfo Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 15/29] hd-geometry: Switch to uint32_t to match BlockConf Markus Armbruster
                   ` (15 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block.h |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/block.h b/block.h
index 993894e..1cd8a01 100644
--- a/block.h
+++ b/block.h
@@ -426,6 +426,8 @@ typedef struct BlockConf {
     uint32_t opt_io_size;
     int32_t bootindex;
     uint32_t discard_granularity;
+    /* geometry, not all devices use this */
+    uint32_t cyls, heads, secs;
 } BlockConf;
 
 static inline unsigned int get_physical_block_exp(BlockConf *conf)
@@ -453,5 +455,9 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
     DEFINE_PROP_UINT32("discard_granularity", _state, \
                        _conf.discard_granularity, 0)
 
-#endif
+#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf)      \
+    DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0),  \
+    DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
+    DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
 
+#endif
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 15/29] hd-geometry: Switch to uint32_t to match BlockConf
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (13 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 14/29] qdev: Introduce block geometry properties Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 16/29] scsi-hd: qdev properties for disk geometry Markus Armbruster
                   ` (14 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

Best to use the same type, to avoid unwanted truncation or sign
extension.

BlockConf can't use plain int for cyls, heads and secs, because
integer properties require an exact width.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/block-common.h |    2 +-
 hw/hd-geometry.c  |    4 ++--
 hw/ide/core.c     |    2 +-
 hw/scsi-disk.c    |    2 +-
 hw/virtio-blk.c   |    2 +-
 trace-events      |    2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/block-common.h b/hw/block-common.h
index bba817a..2f65186 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -16,7 +16,7 @@
 /* Hard disk geometry */
 
 void hd_geometry_guess(BlockDriverState *bs,
-                       int *pcyls, int *pheads, int *psecs,
+                       uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
                        int *ptrans);
 
 #endif
diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 4d746b7..7626cbb 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -98,7 +98,7 @@ static int guess_disk_lchs(BlockDriverState *bs,
 }
 
 static void guess_chs_for_size(BlockDriverState *bs,
-                               int *pcyls, int *pheads, int *psecs)
+                uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs)
 {
     uint64_t nb_sectors;
     int cylinders;
@@ -117,7 +117,7 @@ static void guess_chs_for_size(BlockDriverState *bs,
 }
 
 void hd_geometry_guess(BlockDriverState *bs,
-                       int *pcyls, int *pheads, int *psecs,
+                       uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
                        int *ptrans)
 {
     int cylinders, heads, secs, translation;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 7f5ad07..f1966e3 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1927,7 +1927,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
                    const char *version, const char *serial, const char *model,
                    uint64_t wwn)
 {
-    int cylinders, heads, secs;
+    uint32_t cylinders, heads, secs;
     uint64_t nb_sectors;
 
     s->bs = bs;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index fc077f5..c881acf 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -968,7 +968,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
     };
 
     BlockDriverState *bdrv = s->qdev.conf.bs;
-    int cylinders, heads, secs;
+    uint32_t cylinders, heads, secs;
     uint8_t *p = *p_outbuf;
 
     if ((mode_sense_valid[page] & (1 << s->qdev.type)) == 0) {
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index d2709a7..4344e28 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -590,7 +590,7 @@ static const BlockDevOps virtio_block_ops = {
 VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
 {
     VirtIOBlock *s;
-    int cylinders, heads, secs;
+    uint32_t cylinders, heads, secs;
     static int virtio_blk_id;
     DriveInfo *dinfo;
 
diff --git a/trace-events b/trace-events
index acef082..5b5272b 100644
--- a/trace-events
+++ b/trace-events
@@ -143,7 +143,7 @@ ecc_diag_mem_readb(uint64_t addr, uint32_t ret) "Read diagnostic %"PRId64"= %02x
 
 # hw/hd-geometry.c
 hd_geometry_lchs_guess(void *bs, int cyls, int heads, int secs) "bs %p LCHS %d %d %d"
-hd_geometry_guess(void *bs, int cyls, int heads, int secs, int trans) "bs %p CHS %d %d %d trans %d"
+hd_geometry_guess(void *bs, uint32_t cyls, uint32_t heads, uint32_t secs, int trans) "bs %p CHS %u %u %u trans %d"
 
 # hw/jazz-led.c
 jazz_led_read(uint64_t addr, uint8_t val) "read addr=0x%"PRIx64": 0x%x"
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 16/29] scsi-hd: qdev properties for disk geometry
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (14 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 15/29] hd-geometry: Switch to uint32_t to match BlockConf Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 17/29] virtio-blk: " Markus Armbruster
                   ` (13 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

Geometry needs to be qdev properties, because it belongs to the
disk's guest part.

Maintain backward compatibility exactly like for serial: fall back to
DriveInfo's geometry, set with -drive cyls=...

Do this only for scsi-hd.  scsi-disk is legacy.  scsi-cd doesn't have
a geometry.  scsi-block should get geometry from the host disk.

Bonus: info qtree now shows the geometry.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/scsi-disk.c |   69 +++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index c881acf..0a182f9 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -966,9 +966,6 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         [MODE_PAGE_AUDIO_CTL]              = (1 << TYPE_ROM),
         [MODE_PAGE_CAPABILITIES]           = (1 << TYPE_ROM),
     };
-
-    BlockDriverState *bdrv = s->qdev.conf.bs;
-    uint32_t cylinders, heads, secs;
     uint8_t *p = *p_outbuf;
 
     if ((mode_sense_valid[page] & (1 << s->qdev.type)) == 0) {
@@ -990,19 +987,18 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
             break;
         }
         /* if a geometry hint is available, use it */
-        hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL);
-        p[2] = (cylinders >> 16) & 0xff;
-        p[3] = (cylinders >> 8) & 0xff;
-        p[4] = cylinders & 0xff;
-        p[5] = heads & 0xff;
+        p[2] = (s->qdev.conf.cyls >> 16) & 0xff;
+        p[3] = (s->qdev.conf.cyls >> 8) & 0xff;
+        p[4] = s->qdev.conf.cyls & 0xff;
+        p[5] = s->qdev.conf.heads & 0xff;
         /* Write precomp start cylinder, disabled */
-        p[6] = (cylinders >> 16) & 0xff;
-        p[7] = (cylinders >> 8) & 0xff;
-        p[8] = cylinders & 0xff;
+        p[6] = (s->qdev.conf.cyls >> 16) & 0xff;
+        p[7] = (s->qdev.conf.cyls >> 8) & 0xff;
+        p[8] = s->qdev.conf.cyls & 0xff;
         /* Reduced current start cylinder, disabled */
-        p[9] = (cylinders >> 16) & 0xff;
-        p[10] = (cylinders >> 8) & 0xff;
-        p[11] = cylinders & 0xff;
+        p[9] = (s->qdev.conf.cyls >> 16) & 0xff;
+        p[10] = (s->qdev.conf.cyls >> 8) & 0xff;
+        p[11] = s->qdev.conf.cyls & 0xff;
         /* Device step rate [ns], 200ns */
         p[12] = 0;
         p[13] = 200;
@@ -1024,18 +1020,17 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         p[2] = 5000 >> 8;
         p[3] = 5000 & 0xff;
         /* if a geometry hint is available, use it */
-        hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL);
-        p[4] = heads & 0xff;
-        p[5] = secs & 0xff;
+        p[4] = s->qdev.conf.heads & 0xff;
+        p[5] = s->qdev.conf.secs & 0xff;
         p[6] = s->qdev.blocksize >> 8;
-        p[8] = (cylinders >> 8) & 0xff;
-        p[9] = cylinders & 0xff;
+        p[8] = (s->qdev.conf.cyls >> 8) & 0xff;
+        p[9] = s->qdev.conf.cyls & 0xff;
         /* Write precomp start cylinder, disabled */
-        p[10] = (cylinders >> 8) & 0xff;
-        p[11] = cylinders & 0xff;
+        p[10] = (s->qdev.conf.cyls >> 8) & 0xff;
+        p[11] = s->qdev.conf.cyls & 0xff;
         /* Reduced current start cylinder, disabled */
-        p[12] = (cylinders >> 8) & 0xff;
-        p[13] = cylinders & 0xff;
+        p[12] = (s->qdev.conf.cyls >> 8) & 0xff;
+        p[13] = s->qdev.conf.cyls & 0xff;
         /* Device step rate [100us], 100us */
         p[14] = 0;
         p[15] = 1;
@@ -1755,6 +1750,33 @@ 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;
+        }
+    }
+
     if (!s->serial) {
         /* try to fall back to value set with legacy -drive serial=... */
         dinfo = drive_get_by_blockdev(s->qdev.conf.bs);
@@ -1975,6 +1997,7 @@ static Property scsi_hd_properties[] = {
     DEFINE_PROP_BIT("dpofua", SCSIDiskState, features,
                     SCSI_DISK_F_DPOFUA, false),
     DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0),
+    DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 17/29] virtio-blk: qdev properties for disk geometry
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (15 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 16/29] scsi-hd: qdev properties for disk geometry Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 18/29] ide: " Markus Armbruster
                   ` (12 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

Geometry needs to be qdev properties, because it belongs to the
disk's guest part.

Maintain backward compatibility exactly like for serial: fall back to
DriveInfo's geometry, set with -drive cyls=...

Bonus: info qtree now shows the geometry.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/s390-virtio-bus.c |    1 +
 hw/virtio-blk.c      |   41 ++++++++++++++++++++++++++++++++---------
 hw/virtio-pci.c      |    1 +
 3 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index 4d49b96..a245684 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -402,6 +402,7 @@ static TypeInfo s390_virtio_net = {
 
 static Property s390_virtio_blk_properties[] = {
     DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf),
+    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOS390Device, blk.conf),
     DEFINE_PROP_STRING("serial", VirtIOS390Device, blk.serial),
 #ifdef __linux__
     DEFINE_PROP_BIT("scsi", VirtIOS390Device, blk.scsi, 0, true),
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 4344e28..3885904 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -479,19 +479,17 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
     VirtIOBlock *s = to_virtio_blk(vdev);
     struct virtio_blk_config blkcfg;
     uint64_t capacity;
-    int cylinders, heads, secs;
     int blk_size = s->conf->logical_block_size;
 
     bdrv_get_geometry(s->bs, &capacity);
-    bdrv_get_geometry_hint(s->bs, &cylinders, &heads, &secs);
     memset(&blkcfg, 0, sizeof(blkcfg));
     stq_raw(&blkcfg.capacity, capacity);
     stl_raw(&blkcfg.seg_max, 128 - 2);
-    stw_raw(&blkcfg.cylinders, cylinders);
+    stw_raw(&blkcfg.cylinders, s->conf->cyls);
     stl_raw(&blkcfg.blk_size, blk_size);
     stw_raw(&blkcfg.min_io_size, s->conf->min_io_size / blk_size);
     stw_raw(&blkcfg.opt_io_size, s->conf->opt_io_size / blk_size);
-    blkcfg.heads = heads;
+    blkcfg.heads = s->conf->heads;
     /*
      * We must ensure that the block device capacity is a multiple of
      * the logical block size. If that is not the case, lets use
@@ -503,10 +501,10 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
      * divided by 512 - instead it is the amount of blk_size blocks
      * per track (cylinder).
      */
-    if (bdrv_getlength(s->bs) /  heads / secs % blk_size) {
-        blkcfg.sectors = secs & ~s->sector_mask;
+    if (bdrv_getlength(s->bs) /  s->conf->heads / s->conf->secs % blk_size) {
+        blkcfg.sectors = s->conf->secs & ~s->sector_mask;
     } else {
-        blkcfg.sectors = secs;
+        blkcfg.sectors = s->conf->secs;
     }
     blkcfg.size_max = 0;
     blkcfg.physical_block_exp = get_physical_block_exp(s->conf);
@@ -590,7 +588,6 @@ static const BlockDevOps virtio_block_ops = {
 VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
 {
     VirtIOBlock *s;
-    uint32_t cylinders, heads, secs;
     static int virtio_blk_id;
     DriveInfo *dinfo;
 
@@ -623,7 +620,33 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     s->blk = blk;
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
-    hd_geometry_guess(s->bs, &cylinders, &heads, &secs, NULL);
+
+    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);
 
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 9342eed..557d1d3 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -936,6 +936,7 @@ static int virtio_balloon_exit_pci(PCIDevice *pci_dev)
 static Property virtio_blk_properties[] = {
     DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
     DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
+    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOPCIProxy, blk.conf),
     DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial),
 #ifdef __linux__
     DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true),
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 18/29] ide: qdev properties for disk geometry
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (16 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 17/29] virtio-blk: " Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 19/29] qtest: Cover " Markus Armbruster
                   ` (11 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

Geometry needs to be qdev properties, because it belongs to the
disk's guest part.

Maintain backward compatibility exactly like for serial: fall back to
DriveInfo's geometry, set with -drive cyls=...

Do this only for ide-hd.  ide-drive is legacy.  ide-cd doesn't have a
geometry.

Bonus: info qtree now shows the geometry.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/core.c     |   19 ++++++++++++++-----
 hw/ide/internal.h |    4 +++-
 hw/ide/qdev.c     |   22 +++++++++++++++++++++-
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index f1966e3..bf1ce89 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1925,16 +1925,16 @@ static const BlockDevOps ide_cd_block_ops = {
 
 int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
                    const char *version, const char *serial, const char *model,
-                   uint64_t wwn)
+                   uint64_t wwn,
+                   uint32_t cylinders, uint32_t heads, uint32_t secs,
+                   int chs_trans)
 {
-    uint32_t cylinders, heads, secs;
     uint64_t nb_sectors;
 
     s->bs = bs;
     s->drive_kind = kind;
 
     bdrv_get_geometry(bs, &nb_sectors);
-    hd_geometry_guess(bs, &cylinders, &heads, &secs, &s->chs_trans);
     if (cylinders < 1 || cylinders > 16383) {
         error_report("cyls must be between 1 and 16383");
         return -1;
@@ -1950,6 +1950,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
     s->cylinders = cylinders;
     s->heads = heads;
     s->sectors = secs;
+    s->chs_trans = chs_trans;
     s->nb_sectors = nb_sectors;
     s->wwn = wwn;
     /* The SMART values should be preserved across power cycles
@@ -2076,17 +2077,25 @@ void ide_init2(IDEBus *bus, qemu_irq irq)
 void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
                                     DriveInfo *hd1, qemu_irq irq)
 {
-    int i;
+    int i, trans;
     DriveInfo *dinfo;
+    uint32_t cyls, heads, secs;
 
     for(i = 0; i < 2; i++) {
         dinfo = i == 0 ? hd0 : hd1;
         ide_init1(bus, i);
         if (dinfo) {
+            cyls  = dinfo->cyls;
+            heads = dinfo->heads;
+            secs  = dinfo->secs;
+            trans = dinfo->trans;
+            if (!cyls && !heads && !secs) {
+                hd_geometry_guess(dinfo->bdrv, &cyls, &heads, &secs, &trans);
+            }
             if (ide_init_drive(&bus->ifs[i], dinfo->bdrv,
                                dinfo->media_cd ? IDE_CD : IDE_HD, NULL,
                                *dinfo->serial ? dinfo->serial : NULL,
-                               NULL, 0) < 0) {
+                               NULL, 0, cyls, heads, secs, trans) < 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 56c718e..685e976 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -545,7 +545,9 @@ uint32_t ide_data_readl(void *opaque, uint32_t addr);
 
 int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
                    const char *version, const char *serial, const char *model,
-                   uint64_t wwn);
+                   uint64_t wwn,
+                   uint32_t cylinders, uint32_t heads, uint32_t secs,
+                   int chs_trans);
 void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
                                     DriveInfo *hd1, qemu_irq irq);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 87e0b75..3e297dc 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -21,6 +21,7 @@
 #include "qemu-error.h"
 #include <hw/ide/internal.h>
 #include "blockdev.h"
+#include "hw/block-common.h"
 #include "sysemu.h"
 
 /* --------------------------------- */
@@ -143,6 +144,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
     IDEState *s = bus->ifs + dev->unit;
     const char *serial;
     DriveInfo *dinfo;
+    int trans;
 
     if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) {
         error_report("discard_granularity must be 512 for ide");
@@ -158,8 +160,25 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
         }
     }
 
+    trans = BIOS_ATA_TRANSLATION_AUTO;
+    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;
+        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,
+                          &trans);
+    }
+
     if (ide_init_drive(s, dev->conf.bs, kind,
-                       dev->version, serial, dev->model, dev->wwn) < 0) {
+                       dev->version, serial, dev->model, dev->wwn,
+                       dev->conf.cyls, dev->conf.heads, dev->conf.secs,
+                       trans) < 0) {
         return -1;
     }
 
@@ -202,6 +221,7 @@ static int ide_drive_initfn(IDEDevice *dev)
 
 static Property ide_hd_properties[] = {
     DEFINE_IDE_DEV_PROPERTIES(),
+    DEFINE_BLOCK_CHS_PROPERTIES(IDEDrive, dev.conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 19/29] qtest: Cover qdev properties for disk geometry
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (17 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 18/29] ide: " Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 20/29] qdev: Collect private helpers in one place Markus Armbruster
                   ` (10 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/hd-geo-test.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index cc447a2..a47b945 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -321,13 +321,15 @@ static void test_ide_drive_user(const char *dev, bool trans)
     const CHST expected_chst = { secs / (4 * 32) , 4, 32, trans };
 
     argc = setup_common(argv, ARRAY_SIZE(argv));
-    opts = g_strdup_printf(",cyls=%d,heads=%d,secs=%d%s",
+    opts = g_strdup_printf("%s,cyls=%d,heads=%d,secs=%d%s",
+                           dev && !trans ? dev : "",
                            expected_chst.cyls, expected_chst.heads,
                            expected_chst.secs,
                            trans ? ",trans=lba" : "");
     cur_ide[0] = &expected_chst;
     argc = setup_ide(argc, argv, ARRAY_SIZE(argv),
-                     0, dev, backend_small, mbr_chs, opts);
+                     0, dev && !trans ? opts : NULL, backend_small, mbr_chs,
+                     dev && !trans ? "" : opts);
     g_free(opts);
     qtest_start(g_strjoinv(" ", argv));
     test_cmos();
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 20/29] qdev: Collect private helpers in one place
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (18 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 19/29] qtest: Cover " Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 21/29] qdev: New property type chs-translation Markus Armbruster
                   ` (9 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

Just code motion, with one long line wrapped to keep checkpatch.pl
happy.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/qdev-properties.c |  144 +++++++++++++++++++++++++-------------------------
 1 files changed, 72 insertions(+), 72 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 0b89462..002c7f9 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -10,6 +10,78 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
     return ptr;
 }
 
+static void get_pointer(Object *obj, Visitor *v, Property *prop,
+                        const char *(*print)(void *ptr),
+                        const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    void **ptr = qdev_get_prop_ptr(dev, prop);
+    char *p;
+
+    p = (char *) (*ptr ? print(*ptr) : "");
+    visit_type_str(v, &p, name, errp);
+}
+
+static void set_pointer(Object *obj, Visitor *v, Property *prop,
+                        int (*parse)(DeviceState *dev, const char *str,
+                                     void **ptr),
+                        const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Error *local_err = NULL;
+    void **ptr = qdev_get_prop_ptr(dev, prop);
+    char *str;
+    int ret;
+
+    if (dev->state != DEV_STATE_CREATED) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    visit_type_str(v, &str, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    if (!*str) {
+        g_free(str);
+        *ptr = NULL;
+        return;
+    }
+    ret = parse(dev, str, ptr);
+    error_set_from_qdev_prop_error(errp, ret, dev, prop, str);
+    g_free(str);
+}
+
+static void get_enum(Object *obj, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    int *ptr = qdev_get_prop_ptr(dev, prop);
+
+    visit_type_enum(v, ptr, prop->info->enum_table,
+                    prop->info->name, prop->name, errp);
+}
+
+static void set_enum(Object *obj, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    int *ptr = qdev_get_prop_ptr(dev, prop);
+
+    if (dev->state != DEV_STATE_CREATED) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    visit_type_enum(v, ptr, prop->info->enum_table,
+                    prop->info->name, prop->name, errp);
+}
+
+/* Bit */
+
 static uint32_t qdev_get_prop_mask(Property *prop)
 {
     assert(prop->info == &qdev_prop_bit);
@@ -26,8 +98,6 @@ static void bit_prop_set(DeviceState *dev, Property *props, bool val)
         *p &= ~mask;
 }
 
-/* Bit */
-
 static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
 {
     uint32_t *p = qdev_get_prop_ptr(dev, prop);
@@ -435,48 +505,6 @@ static const char *print_drive(void *ptr)
     return bdrv_get_device_name(ptr);
 }
 
-static void get_pointer(Object *obj, Visitor *v, Property *prop,
-                        const char *(*print)(void *ptr),
-                        const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    void **ptr = qdev_get_prop_ptr(dev, prop);
-    char *p;
-
-    p = (char *) (*ptr ? print(*ptr) : "");
-    visit_type_str(v, &p, name, errp);
-}
-
-static void set_pointer(Object *obj, Visitor *v, Property *prop,
-                        int (*parse)(DeviceState *dev, const char *str, void **ptr),
-                        const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    Error *local_err = NULL;
-    void **ptr = qdev_get_prop_ptr(dev, prop);
-    char *str;
-    int ret;
-
-    if (dev->state != DEV_STATE_CREATED) {
-        error_set(errp, QERR_PERMISSION_DENIED);
-        return;
-    }
-
-    visit_type_str(v, &str, name, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-    if (!*str) {
-        g_free(str);
-        *ptr = NULL;
-        return;
-    }
-    ret = parse(dev, str, ptr);
-    error_set_from_qdev_prop_error(errp, ret, dev, prop, str);
-    g_free(str);
-}
-
 static void get_drive(Object *obj, Visitor *v, void *opaque,
                       const char *name, Error **errp)
 {
@@ -735,7 +763,6 @@ PropertyInfo qdev_prop_macaddr = {
     .set   = set_mac,
 };
 
-
 /* --- lost tick policy --- */
 
 static const char *lost_tick_policy_table[LOST_TICK_MAX+1] = {
@@ -748,33 +775,6 @@ static const char *lost_tick_policy_table[LOST_TICK_MAX+1] = {
 
 QEMU_BUILD_BUG_ON(sizeof(LostTickPolicy) != sizeof(int));
 
-static void get_enum(Object *obj, Visitor *v, void *opaque,
-                     const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    int *ptr = qdev_get_prop_ptr(dev, prop);
-
-    visit_type_enum(v, ptr, prop->info->enum_table,
-                    prop->info->name, prop->name, errp);
-}
-
-static void set_enum(Object *obj, Visitor *v, void *opaque,
-                     const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    int *ptr = qdev_get_prop_ptr(dev, prop);
-
-    if (dev->state != DEV_STATE_CREATED) {
-        error_set(errp, QERR_PERMISSION_DENIED);
-        return;
-    }
-
-    visit_type_enum(v, ptr, prop->info->enum_table,
-                    prop->info->name, prop->name, errp);
-}
-
 PropertyInfo qdev_prop_losttickpolicy = {
     .name  = "LostTickPolicy",
     .enum_table  = lost_tick_policy_table,
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 21/29] qdev: New property type chs-translation
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (19 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 20/29] qdev: Collect private helpers in one place Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 22/29] ide: qdev property for BIOS CHS translation Markus Armbruster
                   ` (8 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/qdev-properties.c |   15 +++++++++++++++
 hw/qdev.h            |    3 +++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 002c7f9..0b18f8c 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -782,6 +782,21 @@ PropertyInfo qdev_prop_losttickpolicy = {
     .set   = set_enum,
 };
 
+/* --- BIOS CHS translation */
+
+static const char *bios_chs_trans_table[] = {
+    [BIOS_ATA_TRANSLATION_AUTO] = "auto",
+    [BIOS_ATA_TRANSLATION_NONE] = "none",
+    [BIOS_ATA_TRANSLATION_LBA]  = "lba",
+};
+
+PropertyInfo qdev_prop_bios_chs_trans = {
+    .name = "bios-chs-trans",
+    .enum_table = bios_chs_trans_table,
+    .get = get_enum,
+    .set = set_enum,
+};
+
 /* --- pci address --- */
 
 /*
diff --git a/hw/qdev.h b/hw/qdev.h
index f4683dc..9be35d4 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -232,6 +232,7 @@ extern PropertyInfo qdev_prop_chr;
 extern PropertyInfo qdev_prop_ptr;
 extern PropertyInfo qdev_prop_macaddr;
 extern PropertyInfo qdev_prop_losttickpolicy;
+extern PropertyInfo qdev_prop_bios_chs_trans;
 extern PropertyInfo qdev_prop_drive;
 extern PropertyInfo qdev_prop_netdev;
 extern PropertyInfo qdev_prop_vlan;
@@ -299,6 +300,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
 #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
                         LostTickPolicy)
+#define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int)
 #define DEFINE_PROP_BLOCKSIZE(_n, _s, _f, _d) \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
 #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 22/29] ide: qdev property for BIOS CHS translation
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (20 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 21/29] qdev: New property type chs-translation Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 23/29] qtest: Cover " Markus Armbruster
                   ` (7 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

This isn't quite orthodox.  CHS translation is firmware configuration,
communicated via the RTC's CMOS RAM, not a property of the disk.  But
it's best to treat it just like geometry anyway.

Maintain backward compatibility exactly like for geometry: fall back
to DriveInfo's translation, set with -drive trans=...

Bonus: info qtree now shows the translation.  Except when it shows
"auto": that's resolved by pc_cmos_init_late().  To be addressed
shortly.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/internal.h |    1 +
 hw/ide/qdev.c     |   10 +++++-----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 685e976..c3ecafc 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -474,6 +474,7 @@ struct IDEDevice {
     DeviceState qdev;
     uint32_t unit;
     BlockConf conf;
+    int chs_trans;
     char *version;
     char *serial;
     char *model;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 3e297dc..f191dd3 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -144,7 +144,6 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
     IDEState *s = bus->ifs + dev->unit;
     const char *serial;
     DriveInfo *dinfo;
-    int trans;
 
     if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) {
         error_report("discard_granularity must be 512 for ide");
@@ -160,25 +159,24 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
         }
     }
 
-    trans = BIOS_ATA_TRANSLATION_AUTO;
     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;
-        trans           = dinfo->trans;
+        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,
-                          &trans);
+                          &dev->chs_trans);
     }
 
     if (ide_init_drive(s, dev->conf.bs, kind,
                        dev->version, serial, dev->model, dev->wwn,
                        dev->conf.cyls, dev->conf.heads, dev->conf.secs,
-                       trans) < 0) {
+                       dev->chs_trans) < 0) {
         return -1;
     }
 
@@ -222,6 +220,8 @@ static int ide_drive_initfn(IDEDevice *dev)
 static Property ide_hd_properties[] = {
     DEFINE_IDE_DEV_PROPERTIES(),
     DEFINE_BLOCK_CHS_PROPERTIES(IDEDrive, dev.conf),
+    DEFINE_PROP_BIOS_CHS_TRANS("bios-chs-trans",
+                IDEDrive, dev.chs_trans, BIOS_ATA_TRANSLATION_AUTO),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 23/29] qtest: Cover qdev property for BIOS CHS translation
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (21 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 22/29] ide: qdev property for BIOS CHS translation Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 24/29] block: Geometry and translation hints are now useless, purge them Markus Armbruster
                   ` (6 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/hd-geo-test.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index a47b945..5d9d2e4 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -321,15 +321,16 @@ static void test_ide_drive_user(const char *dev, bool trans)
     const CHST expected_chst = { secs / (4 * 32) , 4, 32, trans };
 
     argc = setup_common(argv, ARRAY_SIZE(argv));
-    opts = g_strdup_printf("%s,cyls=%d,heads=%d,secs=%d%s",
-                           dev && !trans ? dev : "",
+    opts = g_strdup_printf("%s,%s%scyls=%d,heads=%d,secs=%d",
+                           dev ?: "",
+                           trans && dev ? "bios-chs-" : "",
+                           trans ? "trans=lba," : "",
                            expected_chst.cyls, expected_chst.heads,
-                           expected_chst.secs,
-                           trans ? ",trans=lba" : "");
+                           expected_chst.secs);
     cur_ide[0] = &expected_chst;
     argc = setup_ide(argc, argv, ARRAY_SIZE(argv),
-                     0, dev && !trans ? opts : NULL, backend_small, mbr_chs,
-                     dev && !trans ? "" : opts);
+                     0, dev ? opts : NULL, backend_small, mbr_chs,
+                     dev ? "" : opts);
     g_free(opts);
     qtest_start(g_strjoinv(" ", argv));
     test_cmos();
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 24/29] block: Geometry and translation hints are now useless, purge them
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (22 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 23/29] qtest: Cover " Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 25/29] ide pc: Put hard disk info into CMOS only for hard disks Markus Armbruster
                   ` (5 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

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 06323cf..ce7eb8f 100644
--- a/block.c
+++ b/block.c
@@ -996,12 +996,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
     bs_dest->block_timer        = bs_src->block_timer;
     bs_dest->io_limits_enabled  = bs_src->io_limits_enabled;
 
-    /* geometry */
-    bs_dest->cyls               = bs_src->cyls;
-    bs_dest->heads              = bs_src->heads;
-    bs_dest->secs               = bs_src->secs;
-    bs_dest->translation        = bs_src->translation;
-
     /* r/w error */
     bs_dest->on_read_error      = bs_src->on_read_error;
     bs_dest->on_write_error     = bs_src->on_write_error;
@@ -2132,27 +2126,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)
@@ -2161,11 +2134,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 1cd8a01..29c5eab 100644
--- a/block.h
+++ b/block.h
@@ -257,18 +257,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 161985b..06c997e 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 89a0c66..77b12b4 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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 25/29] ide pc: Put hard disk info into CMOS only for hard disks
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (23 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 24/29] block: Geometry and translation hints are now useless, purge them Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 26/29] qtest: Test we don't put hard disk info into CMOS for a CD-ROM Markus Armbruster
                   ` (4 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

In particular, don't set disk type and geometry when a CD-ROM on bus
ide.0 has media during CMOS initialization.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/qdev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index f191dd3..84097fd 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -117,7 +117,7 @@ int ide_get_geometry(BusState *bus, int unit,
 {
     IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
 
-    if (!s->bs) {
+    if (s->drive_kind != IDE_HD || !s->bs) {
         return -1;
     }
 
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 26/29] qtest: Test we don't put hard disk info into CMOS for a CD-ROM
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (24 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 25/29] ide pc: Put hard disk info into CMOS only for hard disks Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 27/29] hd-geometry: Compute BIOS CHS translation in one place Markus Armbruster
                   ` (3 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini


Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/hd-geo-test.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 5d9d2e4..9a31e85 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -369,6 +369,27 @@ static void test_ide_device_user_chst(void)
     test_ide_drive_user("ide-hd", true);
 }
 
+/*
+ * Test case: IDE devices (if=ide), but use index=0 for CD-ROM
+ */
+static void test_ide_drive_cd_0(void)
+{
+    char *argv[256];
+    int argc, ide_idx;
+    Backend i;
+
+    argc = setup_common(argv, ARRAY_SIZE(argv));
+    for (i = 0; i <= backend_empty; i++) {
+        ide_idx = backend_empty - i;
+        cur_ide[ide_idx] = &hd_chst[i][mbr_blank];
+        argc = setup_ide(argc, argv, ARRAY_SIZE(argv),
+                         ide_idx, NULL, i, mbr_blank, "");
+    }
+    qtest_start(g_strjoinv(" ", argv));
+    test_cmos();
+    qtest_quit(global_qtest);
+}
+
 int main(int argc, char **argv)
 {
     Backend i;
@@ -390,6 +411,7 @@ int main(int argc, char **argv)
     qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
     qtest_add_func("hd-geo/ide/drive/user/chs", test_ide_drive_user_chs);
     qtest_add_func("hd-geo/ide/drive/user/chst", test_ide_drive_user_chst);
+    qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
     qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank);
     qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
     qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs);
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 27/29] hd-geometry: Compute BIOS CHS translation in one place
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (25 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 26/29] qtest: Test we don't put hard disk info into CMOS for a CD-ROM Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 28/29] blockdev: Drop redundant CHS validation for if=ide Markus Armbruster
                   ` (2 subsequent siblings)
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

Currently, it is split between hd_geometry_guess() and
pc_cmos_init_late().  Confusing.  info qtree shows the result of the
former.  Also confusing.

Fold the part done in pc_cmos_init_late() into hd_geometry_guess().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/block-common.h |    1 +
 hw/hd-geometry.c  |    9 ++++++++-
 hw/ide/core.c     |    2 ++
 hw/ide/qdev.c     |    3 +++
 hw/pc.c           |   19 ++++---------------
 5 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/hw/block-common.h b/hw/block-common.h
index ec7810d..31e12ba 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -24,5 +24,6 @@
 void hd_geometry_guess(BlockDriverState *bs,
                        uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
                        int *ptrans);
+int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs);
 
 #endif
diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 74678a6..1cdb9fb 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -125,7 +125,7 @@ void hd_geometry_guess(BlockDriverState *bs,
     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;
+        translation = hd_bios_chs_auto_trans(*pcyls, *pheads, *psecs);
     } else if (heads > 16) {
         /* LCHS guess with heads > 16 means that a BIOS LBA
            translation was active, so a standard physical disk
@@ -148,3 +148,10 @@ void hd_geometry_guess(BlockDriverState *bs,
     }
     trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation);
 }
+
+int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs)
+{
+    return cyls <= 1024 && heads <= 16 && secs <= 63
+        ? BIOS_ATA_TRANSLATION_NONE
+        : BIOS_ATA_TRANSLATION_LBA;
+}
diff --git a/hw/ide/core.c b/hw/ide/core.c
index bf1ce89..1ca7cdf 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2091,6 +2091,8 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
             trans = dinfo->trans;
             if (!cyls && !heads && !secs) {
                 hd_geometry_guess(dinfo->bdrv, &cyls, &heads, &secs, &trans);
+            } else if (trans == BIOS_ATA_TRANSLATION_AUTO) {
+                trans = hd_bios_chs_auto_trans(cyls, heads, secs);
             }
             if (ide_init_drive(&bus->ifs[i], dinfo->bdrv,
                                dinfo->media_cd ? IDE_CD : IDE_HD, NULL,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 84097fd..de9db3b 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -171,6 +171,9 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
         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 (ide_init_drive(s, dev->conf.bs, kind,
diff --git a/hw/pc.c b/hw/pc.c
index 77b12b4..598267a 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -290,7 +290,7 @@ static void pc_cmos_init_late(void *opaque)
     int16_t cylinders;
     int8_t heads, sectors;
     int val;
-    int i;
+    int i, trans;
 
     val = 0;
     if (ide_get_geometry(arg->idebus[0], 0,
@@ -313,20 +313,9 @@ static void pc_cmos_init_late(void *opaque)
            geometry can be different if a translation is done. */
         if (ide_get_geometry(arg->idebus[i / 2], i % 2,
                              &cylinders, &heads, &sectors) >= 0) {
-            int translation = ide_get_bios_chs_trans(arg->idebus[i / 2],
-                                                     i % 2);
-            if (translation == BIOS_ATA_TRANSLATION_AUTO) {
-                if (cylinders <= 1024 && heads <= 16 && sectors <= 63) {
-                    /* No translation. */
-                    translation = 0;
-                } else {
-                    /* LBA translation. */
-                    translation = 1;
-                }
-            } else {
-                translation--;
-            }
-            val |= translation << (i * 2);
+            trans = ide_get_bios_chs_trans(arg->idebus[i / 2], i % 2) - 1;
+            assert((trans & ~3) == 0);
+            val |= trans << (i * 2);
         }
     }
     rtc_set_memory(s, 0x39, val);
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 28/29] blockdev: Drop redundant CHS validation for if=ide
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (26 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 27/29] hd-geometry: Compute BIOS CHS translation in one place Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 29/29] Relax IDE CHS limits from 16383, 16, 63 to 65535, 16, 255 Markus Armbruster
  2012-07-11 15:40 ` [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Kevin Wolf
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

Leave it to ide_init_drive().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 blockdev.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 06c997e..5f8677e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -330,15 +330,15 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
     max_devs = if_max_devs[type];
 
     if (cyls || heads || secs) {
-        if (cyls < 1 || (type == IF_IDE && cyls > 16383)) {
+        if (cyls < 1) {
             error_report("invalid physical cyls number");
 	    return NULL;
 	}
-        if (heads < 1 || (type == IF_IDE && heads > 16)) {
+        if (heads < 1) {
             error_report("invalid physical heads number");
 	    return NULL;
 	}
-        if (secs < 1 || (type == IF_IDE && secs > 63)) {
+        if (secs < 1) {
             error_report("invalid physical secs number");
 	    return NULL;
 	}
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [PATCH v3 29/29] Relax IDE CHS limits from 16383, 16, 63 to 65535, 16, 255
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (27 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 28/29] blockdev: Drop redundant CHS validation for if=ide Markus Armbruster
@ 2012-07-10  9:12 ` Markus Armbruster
  2012-07-11 15:40 ` [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Kevin Wolf
  29 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-10  9:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini

New limits straight from ATA4 6.2 Register delivered data transfer
command sector addressing.

I figure the old sector limit 63 was blindly copied from the BIOS
int 13 limit.  Doesn't apply to the hardware.  No idea where the old
cylinder limit comes from.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/core.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 1ca7cdf..58a454f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1935,16 +1935,16 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
     s->drive_kind = kind;
 
     bdrv_get_geometry(bs, &nb_sectors);
-    if (cylinders < 1 || cylinders > 16383) {
-        error_report("cyls must be between 1 and 16383");
+    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 > 63) {
-        error_report("secs must be between 1 and 63");
+    if (secs < 1 || secs > 255) {
+        error_report("secs must be between 1 and 255");
         return -1;
     }
     s->cylinders = cylinders;
-- 
1.7.6.5

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH v3 05/29] hd-geometry: Move disk geometry guessing back from block.c
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 05/29] hd-geometry: Move disk geometry guessing back from block.c Markus Armbruster
@ 2012-07-11 13:54   ` Kevin Wolf
  2012-07-11 14:35     ` Markus Armbruster
  0 siblings, 1 reply; 33+ messages in thread
From: Kevin Wolf @ 2012-07-11 13:54 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: blauwirbel, pbonzini, qemu-devel, stefanha

Am 10.07.2012 11:12, schrieb Markus Armbruster:
> Commit f3d54fc4 factored it out of hw/ide.c for reuse.  Sensible,
> except it was put into block.c.  Device-specific functionality should
> be kept in device code, not the block layer.  Move it to
> hw/hd-geometry.c, and make stylistic changes required to keep
> checkpatch.pl happy.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block.c           |  121 ---------------------------------------
>  block.h           |    1 -
>  blockdev.h        |    1 +
>  hw/Makefile.objs  |    2 +-
>  hw/block-common.h |   21 +++++++
>  hw/hd-geometry.c  |  162 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/ide/core.c     |    3 +-
>  hw/scsi-disk.c    |    5 +-
>  hw/virtio-blk.c   |    3 +-
>  9 files changed, 192 insertions(+), 127 deletions(-)
>  create mode 100644 hw/block-common.h
>  create mode 100644 hw/hd-geometry.c

> diff --git a/blockdev.h b/blockdev.h
> index 260e16b..26454c9 100644
> --- a/blockdev.h
> +++ b/blockdev.h
> @@ -62,4 +62,5 @@ void qmp_change_blockdev(const char *device, const char *filename,
>                           bool has_format, const char *format, Error **errp);
>  void do_commit(Monitor *mon, const QDict *qdict);
>  int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
> +
>  #endif

Any objections against dropping this hunk before applying?

Kevin

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH v3 05/29] hd-geometry: Move disk geometry guessing back from block.c
  2012-07-11 13:54   ` Kevin Wolf
@ 2012-07-11 14:35     ` Markus Armbruster
  0 siblings, 0 replies; 33+ messages in thread
From: Markus Armbruster @ 2012-07-11 14:35 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: blauwirbel, pbonzini, qemu-devel, stefanha

Kevin Wolf <kwolf@redhat.com> writes:

> Am 10.07.2012 11:12, schrieb Markus Armbruster:
>> Commit f3d54fc4 factored it out of hw/ide.c for reuse.  Sensible,
>> except it was put into block.c.  Device-specific functionality should
>> be kept in device code, not the block layer.  Move it to
>> hw/hd-geometry.c, and make stylistic changes required to keep
>> checkpatch.pl happy.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  block.c           |  121 ---------------------------------------
>>  block.h           |    1 -
>>  blockdev.h        |    1 +
>>  hw/Makefile.objs  |    2 +-
>>  hw/block-common.h |   21 +++++++
>>  hw/hd-geometry.c  |  162 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  hw/ide/core.c     |    3 +-
>>  hw/scsi-disk.c    |    5 +-
>>  hw/virtio-blk.c   |    3 +-
>>  9 files changed, 192 insertions(+), 127 deletions(-)
>>  create mode 100644 hw/block-common.h
>>  create mode 100644 hw/hd-geometry.c
>
>> diff --git a/blockdev.h b/blockdev.h
>> index 260e16b..26454c9 100644
>> --- a/blockdev.h
>> +++ b/blockdev.h
>> @@ -62,4 +62,5 @@ void qmp_change_blockdev(const char *device, const char *filename,
>>                           bool has_format, const char *format, Error **errp);
>>  void do_commit(Monitor *mon, const QDict *qdict);
>>  int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
>> +
>>  #endif
>
> Any objections against dropping this hunk before applying?
>
> Kevin

Nope.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup
  2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
                   ` (28 preceding siblings ...)
  2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 29/29] Relax IDE CHS limits from 16383, 16, 63 to 65535, 16, 255 Markus Armbruster
@ 2012-07-11 15:40 ` Kevin Wolf
  29 siblings, 0 replies; 33+ messages in thread
From: Kevin Wolf @ 2012-07-11 15:40 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: blauwirbel, pbonzini, qemu-devel, stefanha

Am 10.07.2012 11:12, schrieb Markus Armbruster:
> 29 patches may look discouraging, but most patches are small, and the
> ones that aren't just move code around.
> 
> Goals of this series:
> 
> 1. One more step towards a clean separation block device host and
>    guest part.
> 
> 2. Purge CHS geometry from the block layer
> 
> Part I    [PATCH    01/29]: Floppy geometry
> 
> Part II   [PATCH 02-03/29]: vvfat geometry bug fixes
> 
> Part III  [PATCH 04-10/29]: Clean up hard disk geometry guessing code
> 
> Part IV   [PATCH 11-12/29]: Clean up CMOS hard disk info setup
> 
> Part V    [PATCH 13-24/29]: qdev properties for disk geometry
> 
> Part VI   [PATCH 25-29/29]: A few more fixes and cleanups
> 
> A few more cleanups are in the works, in particular geometry checking
> code duplication pointed out by Kevin.
> 
> This patch series is also available at
> git://repo.or.cz/qemu/armbru.git
> tag geo-v3
> 
> v3: Rebase; drop the three patches that have been committed already
>     Fix uninitialized variable in PATCH 02/29 (Anthony)
> 
> v2: New hw/block-common.h (Blue & Kevin)
>     Coding style here & there (Blue)
>     Tracepoint parameter types (Stefan)
> 
> Markus Armbruster (29):
>   fdc: Move floppy geometry guessing back from block.c
>   vvfat: Fix partition table
>   vvfat: Do not clobber the user's geometry
>   qtest: Add hard disk geometry test
>   hd-geometry: Move disk geometry guessing back from block.c
>   hd-geometry: Add tracepoints
>   hd-geometry: Unnest conditional in hd_geometry_guess()
>   hd-geometry: Factor out guess_chs_for_size()
>   hd-geometry: Clean up gratuitous goto in hd_geometry_guess()
>   hd-geometry: Clean up confusing use of prior translation hint
>   hd-geometry: Cut out block layer translation middleman
>   ide pc: Cut out the block layer geometry middleman
>   blockdev: Save geometry in DriveInfo
>   qdev: Introduce block geometry properties
>   hd-geometry: Switch to uint32_t to match BlockConf
>   scsi-hd: qdev properties for disk geometry
>   virtio-blk: qdev properties for disk geometry
>   ide: qdev properties for disk geometry
>   qtest: Cover qdev properties for disk geometry
>   qdev: Collect private helpers in one place
>   qdev: New property type chs-translation
>   ide: qdev property for BIOS CHS translation
>   qtest: Cover qdev property for BIOS CHS translation
>   block: Geometry and translation hints are now useless, purge them
>   ide pc: Put hard disk info into CMOS only for hard disks
>   qtest: Test we don't put hard disk info into CMOS for a CD-ROM
>   hd-geometry: Compute BIOS CHS translation in one place
>   blockdev: Drop redundant CHS validation for if=ide
>   Relax IDE CHS limits from 16383,16,63 to 65535,16,255
> 
>  block.c              |  254 ------------------------------
>  block.h              |   39 +----
>  block/vvfat.c        |   57 ++++---
>  block_int.h          |    1 -
>  blockdev.c           |   24 +--
>  blockdev.h           |    2 +
>  hw/Makefile.objs     |    2 +-
>  hw/block-common.h    |   29 ++++
>  hw/fdc.c             |  122 +++++++++++++--
>  hw/fdc.h             |   10 +-
>  hw/hd-geometry.c     |  157 ++++++++++++++++++
>  hw/ide.h             |    4 +-
>  hw/ide/core.c        |   30 +++-
>  hw/ide/internal.h    |    7 +-
>  hw/ide/qdev.c        |   46 +++++-
>  hw/pc.c              |   78 ++++------
>  hw/qdev-properties.c |  160 ++++++++++---------
>  hw/qdev.h            |    3 +
>  hw/s390-virtio-bus.c |    1 +
>  hw/scsi-disk.c       |   70 ++++++---
>  hw/virtio-blk.c      |   42 ++++-
>  hw/virtio-pci.c      |    1 +
>  tests/Makefile       |    2 +
>  tests/hd-geo-test.c  |  428 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  trace-events         |    4 +
>  vl.c                 |    2 +-
>  26 files changed, 1067 insertions(+), 508 deletions(-)
>  create mode 100644 hw/block-common.h
>  create mode 100644 hw/hd-geometry.c
>  create mode 100644 tests/hd-geo-test.c
> 

Thanks, applied all to the block branch.

Kevin

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2012-07-11 15:40 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-10  9:12 [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 01/29] fdc: Move floppy geometry guessing back from block.c Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 02/29] vvfat: Fix partition table Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 03/29] vvfat: Do not clobber the user's geometry Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 04/29] qtest: Add hard disk geometry test Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 05/29] hd-geometry: Move disk geometry guessing back from block.c Markus Armbruster
2012-07-11 13:54   ` Kevin Wolf
2012-07-11 14:35     ` Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 06/29] hd-geometry: Add tracepoints Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 07/29] hd-geometry: Unnest conditional in hd_geometry_guess() Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 08/29] hd-geometry: Factor out guess_chs_for_size() Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 09/29] hd-geometry: Clean up gratuitous goto in hd_geometry_guess() Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 10/29] hd-geometry: Clean up confusing use of prior translation hint Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 11/29] hd-geometry: Cut out block layer translation middleman Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 12/29] ide pc: Cut out the block layer geometry middleman Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 13/29] blockdev: Save geometry in DriveInfo Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 14/29] qdev: Introduce block geometry properties Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 15/29] hd-geometry: Switch to uint32_t to match BlockConf Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 16/29] scsi-hd: qdev properties for disk geometry Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 17/29] virtio-blk: " Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 18/29] ide: " Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 19/29] qtest: Cover " Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 20/29] qdev: Collect private helpers in one place Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 21/29] qdev: New property type chs-translation Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 22/29] ide: qdev property for BIOS CHS translation Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 23/29] qtest: Cover " Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 24/29] block: Geometry and translation hints are now useless, purge them Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 25/29] ide pc: Put hard disk info into CMOS only for hard disks Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 26/29] qtest: Test we don't put hard disk info into CMOS for a CD-ROM Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 27/29] hd-geometry: Compute BIOS CHS translation in one place Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 28/29] blockdev: Drop redundant CHS validation for if=ide Markus Armbruster
2012-07-10  9:12 ` [Qemu-devel] [PATCH v3 29/29] Relax IDE CHS limits from 16383, 16, 63 to 65535, 16, 255 Markus Armbruster
2012-07-11 15:40 ` [Qemu-devel] [PATCH v3 00/29] Disk geometry cleanup Kevin Wolf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.