* [Qemu-devel] [PATCH 01/32] fdc: Drop broken code for user-defined floppy geometry
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 02/32] fdc: Move floppy geometry guessing back from block.c Markus Armbruster
` (31 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini
bdrv_get_floppy_geometry_hint() fails to store through its parameter
drive when bs has a geometry hint. Makes fd_revalidate() assign
random crap to drv->drive.
Has been broken that way for ages. Harmless, because:
* The only way to set a geometry hint is -drive if=none,cyls=...
Since commit c219331e, probably unintentional.
* The only use of drv->drive is as argument to another
bdrv_get_floppy_geometry_hint(). Which doesn't use it, since the
geometry hint is still there.
Drop the broken code, ignore -drive parameter cyls, heads and secs for
floppies even with if=none, just like before commit c219331e. Matches
-help, which explains cyls, heads, secs as "hard disk physical
geometry".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block.c | 62 ++++++++++++++++++++++++++++----------------------------------
hw/fdc.c | 3 ---
2 files changed, 28 insertions(+), 37 deletions(-)
diff --git a/block.c b/block.c
index 0acdcac..f5e7cb6 100644
--- a/block.c
+++ b/block.c
@@ -2308,46 +2308,40 @@ void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads,
uint64_t nb_sectors, size;
int i, first_match, match;
- bdrv_get_geometry_hint(bs, nb_heads, max_track, last_sect);
- if (*nb_heads != 0 && *max_track != 0 && *last_sect != 0) {
- /* User defined disk */
- *rate = FDRIVE_RATE_500K;
- } else {
- 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) {
+ 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 (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;
+ first_match = i;
}
- 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;
}
+ 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)
diff --git a/hw/fdc.c b/hw/fdc.c
index 5b3224b..ec02e62 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -189,9 +189,6 @@ static void fd_revalidate(FDrive *drv)
&last_sect, drv->drive, &drive, &rate);
if (!bdrv_is_inserted(drv->bs)) {
FLOPPY_DPRINTF("No disk in drive\n");
- } else if (nb_heads != 0 && max_track != 0 && last_sect != 0) {
- FLOPPY_DPRINTF("User defined disk (%d %d %d)\n",
- nb_heads - 1, max_track, last_sect);
} else {
FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
max_track, last_sect, ro ? "ro" : "rw");
--
1.7.6.5
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 02/32] fdc: Move floppy geometry guessing back from block.c
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 01/32] fdc: Drop broken code for user-defined floppy geometry Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 03/32] vvfat: Fix partition table Markus Armbruster
` (30 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 | 13 ++-----
5 files changed, 124 insertions(+), 140 deletions(-)
diff --git a/block.c b/block.c
index f5e7cb6..66789d5 100644
--- a/block.c
+++ b/block.c
@@ -2243,107 +2243,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 d135652..f4c77a1 100644
--- a/block.h
+++ b/block.h
@@ -266,24 +266,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 ec02e62..2391fbc 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))
@@ -185,8 +292,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 {
@@ -2043,18 +2150,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..e5e7647 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;
- FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
- FDriveRate rate;
- BlockDriverState *fd[MAX_FD];
+ int val, nb, i;
+ FDriveType fd_type[2];
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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 03/32] vvfat: Fix partition table
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 01/32] fdc: Drop broken code for user-defined floppy geometry Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 02/32] fdc: Move floppy geometry guessing back from block.c Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 04/32] vvfat: Do not clobber the user's geometry Markus Armbruster
` (29 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 04/32] vvfat: Do not clobber the user's geometry
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (2 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 03/32] vvfat: Fix partition table Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 05/32] qtest: Tidy up temporary files properly Markus Armbruster
` (28 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 05/32] qtest: Tidy up temporary files properly
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (3 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 04/32] vvfat: Do not clobber the user's geometry Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-07 7:39 ` Blue Swirl
2012-07-06 6:57 ` [Qemu-devel] [PATCH 06/32] qtest: Add hard disk geometry test Markus Armbruster
` (27 subsequent siblings)
32 siblings, 1 reply; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini
Each test litters /tmp with several files: a pid file and two
sockets. Tidy up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/libqtest.c | 29 ++++++++++++++++-------------
1 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 071b6be..02d0392 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -40,6 +40,7 @@ struct QTestState
bool irq_level[MAX_IRQ];
GString *rx;
gchar *pid_file;
+ char *socket_path, *qmp_socket_path;
};
#define g_assert_no_errno(ret) do { \
@@ -88,8 +89,6 @@ QTestState *qtest_init(const char *extra_args)
{
QTestState *s;
int sock, qmpsock, ret, i;
- gchar *socket_path;
- gchar *qmp_socket_path;
gchar *pid_file;
gchar *command;
const char *qemu_binary;
@@ -98,14 +97,14 @@ QTestState *qtest_init(const char *extra_args)
qemu_binary = getenv("QTEST_QEMU_BINARY");
g_assert(qemu_binary != NULL);
- socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
- qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
- pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid());
-
s = g_malloc(sizeof(*s));
- sock = init_socket(socket_path);
- qmpsock = init_socket(qmp_socket_path);
+ s->socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
+ s->qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
+ pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid());
+
+ sock = init_socket(s->socket_path);
+ qmpsock = init_socket(s->qmp_socket_path);
pid = fork();
if (pid == 0) {
@@ -115,8 +114,8 @@ QTestState *qtest_init(const char *extra_args)
"-qmp unix:%s,nowait "
"-pidfile %s "
"-machine accel=qtest "
- "%s", qemu_binary, socket_path,
- qmp_socket_path, pid_file,
+ "%s", qemu_binary, s->socket_path,
+ s->qmp_socket_path, pid_file,
extra_args ?: "");
ret = system(command);
@@ -133,9 +132,6 @@ QTestState *qtest_init(const char *extra_args)
s->irq_level[i] = false;
}
- g_free(socket_path);
- g_free(qmp_socket_path);
-
/* Read the QMP greeting and then do the handshake */
qtest_qmp(s, "");
qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }");
@@ -160,6 +156,13 @@ void qtest_quit(QTestState *s)
fclose(f);
}
+
+ unlink(s->pid_file);
+ unlink(s->socket_path);
+ unlink(s->qmp_socket_path);
+ g_free(s->pid_file);
+ g_free(s->socket_path);
+ g_free(s->qmp_socket_path);
}
static void socket_sendf(int fd, const char *fmt, va_list ap)
--
1.7.6.5
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 05/32] qtest: Tidy up temporary files properly
2012-07-06 6:57 ` [Qemu-devel] [PATCH 05/32] qtest: Tidy up temporary files properly Markus Armbruster
@ 2012-07-07 7:39 ` Blue Swirl
2012-07-09 7:56 ` Markus Armbruster
0 siblings, 1 reply; 37+ messages in thread
From: Blue Swirl @ 2012-07-07 7:39 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, pbonzini, qemu-devel, stefanha
On Fri, Jul 6, 2012 at 6:57 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Each test litters /tmp with several files: a pid file and two
> sockets. Tidy up.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
While a nice patch, I don't think it should belong to this series.
> ---
> tests/libqtest.c | 29 ++++++++++++++++-------------
> 1 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 071b6be..02d0392 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -40,6 +40,7 @@ struct QTestState
> bool irq_level[MAX_IRQ];
> GString *rx;
> gchar *pid_file;
> + char *socket_path, *qmp_socket_path;
> };
>
> #define g_assert_no_errno(ret) do { \
> @@ -88,8 +89,6 @@ QTestState *qtest_init(const char *extra_args)
> {
> QTestState *s;
> int sock, qmpsock, ret, i;
> - gchar *socket_path;
> - gchar *qmp_socket_path;
> gchar *pid_file;
> gchar *command;
> const char *qemu_binary;
> @@ -98,14 +97,14 @@ QTestState *qtest_init(const char *extra_args)
> qemu_binary = getenv("QTEST_QEMU_BINARY");
> g_assert(qemu_binary != NULL);
>
> - socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
> - qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
> - pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid());
> -
> s = g_malloc(sizeof(*s));
>
> - sock = init_socket(socket_path);
> - qmpsock = init_socket(qmp_socket_path);
> + s->socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
> + s->qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
> + pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid());
> +
> + sock = init_socket(s->socket_path);
> + qmpsock = init_socket(s->qmp_socket_path);
>
> pid = fork();
> if (pid == 0) {
> @@ -115,8 +114,8 @@ QTestState *qtest_init(const char *extra_args)
> "-qmp unix:%s,nowait "
> "-pidfile %s "
> "-machine accel=qtest "
> - "%s", qemu_binary, socket_path,
> - qmp_socket_path, pid_file,
> + "%s", qemu_binary, s->socket_path,
> + s->qmp_socket_path, pid_file,
> extra_args ?: "");
>
> ret = system(command);
> @@ -133,9 +132,6 @@ QTestState *qtest_init(const char *extra_args)
> s->irq_level[i] = false;
> }
>
> - g_free(socket_path);
> - g_free(qmp_socket_path);
> -
> /* Read the QMP greeting and then do the handshake */
> qtest_qmp(s, "");
> qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }");
> @@ -160,6 +156,13 @@ void qtest_quit(QTestState *s)
>
> fclose(f);
> }
> +
> + unlink(s->pid_file);
> + unlink(s->socket_path);
> + unlink(s->qmp_socket_path);
> + g_free(s->pid_file);
> + g_free(s->socket_path);
> + g_free(s->qmp_socket_path);
> }
>
> static void socket_sendf(int fd, const char *fmt, va_list ap)
> --
> 1.7.6.5
>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 05/32] qtest: Tidy up temporary files properly
2012-07-07 7:39 ` Blue Swirl
@ 2012-07-09 7:56 ` Markus Armbruster
2012-07-09 9:09 ` Kevin Wolf
0 siblings, 1 reply; 37+ messages in thread
From: Markus Armbruster @ 2012-07-09 7:56 UTC (permalink / raw)
To: Blue Swirl; +Cc: kwolf, pbonzini, qemu-devel, stefanha
Blue Swirl <blauwirbel@gmail.com> writes:
> On Fri, Jul 6, 2012 at 6:57 AM, Markus Armbruster <armbru@redhat.com> wrote:
>> Each test litters /tmp with several files: a pid file and two
>> sockets. Tidy up.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> While a nice patch, I don't think it should belong to this series.
Understand, but what do you want me to do? I could repost this series
without this patch, and post the patch separately, but that feels like
busywork.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH 05/32] qtest: Tidy up temporary files properly
2012-07-09 7:56 ` Markus Armbruster
@ 2012-07-09 9:09 ` Kevin Wolf
0 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2012-07-09 9:09 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Blue Swirl, pbonzini, qemu-devel, stefanha
Am 09.07.2012 09:56, schrieb Markus Armbruster:
> Blue Swirl <blauwirbel@gmail.com> writes:
>
>> On Fri, Jul 6, 2012 at 6:57 AM, Markus Armbruster <armbru@redhat.com> wrote:
>>> Each test litters /tmp with several files: a pid file and two
>>> sockets. Tidy up.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>
>> While a nice patch, I don't think it should belong to this series.
>
> Understand, but what do you want me to do? I could repost this series
> without this patch, and post the patch separately, but that feels like
> busywork.
I have already picked up some patches including this, so I will include
it in a possible pull request before the whole series is merged.
Resending separately wouldn't change much, except maybe that another
maintainer would pick it up.
Kevin
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH 06/32] qtest: Add hard disk geometry test
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (4 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 05/32] qtest: Tidy up temporary files properly Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 07/32] block: Factor bdrv_read_unthrottled() out of guess_disk_lchs() Markus Armbruster
` (26 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 d66ab19..5d362bd 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -20,6 +20,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)
@@ -70,6 +71,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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 07/32] block: Factor bdrv_read_unthrottled() out of guess_disk_lchs()
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (5 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 06/32] qtest: Add hard disk geometry test Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 08/32] hd-geometry: Move disk geometry guessing back from block.c Markus Armbruster
` (25 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini
To prepare move of guess_disk_lchs() into hw/, where it poking
BlockDriverState member io_limits_enabled directly would be unclean.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block.c | 24 +++++++++++++++++-------
block.h | 2 ++
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/block.c b/block.c
index 66789d5..9c538f1 100644
--- a/block.c
+++ b/block.c
@@ -1610,6 +1610,20 @@ int bdrv_read(BlockDriverState *bs, int64_t sector_num,
return bdrv_rw_co(bs, sector_num, buf, nb_sectors, false);
}
+/* Just like bdrv_read(), but with I/O throttling temporarily disabled */
+int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
+ uint8_t *buf, int nb_sectors)
+{
+ bool enabled;
+ int ret;
+
+ enabled = bs->io_limits_enabled;
+ bs->io_limits_enabled = false;
+ ret = bdrv_read(bs, 0, buf, 1);
+ bs->io_limits_enabled = enabled;
+ return ret;
+}
+
#define BITS_PER_LONG (sizeof(unsigned long) * 8)
static void set_dirty_bitmap(BlockDriverState *bs, int64_t sector_num,
@@ -2107,11 +2121,10 @@ static int guess_disk_lchs(BlockDriverState *bs,
int *pcylinders, int *pheads, int *psectors)
{
uint8_t buf[BDRV_SECTOR_SIZE];
- int ret, i, heads, sectors, cylinders;
+ int i, heads, sectors, cylinders;
struct partition *p;
uint32_t nr_sects;
uint64_t nb_sectors;
- bool enabled;
bdrv_get_geometry(bs, &nb_sectors);
@@ -2120,12 +2133,9 @@ static int guess_disk_lchs(BlockDriverState *bs,
* but also in async I/O mode. So the I/O throttling function has to
* be disabled temporarily here, not permanently.
*/
- enabled = bs->io_limits_enabled;
- bs->io_limits_enabled = false;
- ret = bdrv_read(bs, 0, buf, 1);
- bs->io_limits_enabled = enabled;
- if (ret < 0)
+ if (bdrv_read_unthrottled(bs, 0, buf, 1) < 0) {
return -1;
+ }
/* test msdos magic */
if (buf[510] != 0x55 || buf[511] != 0xaa)
return -1;
diff --git a/block.h b/block.h
index f4c77a1..1c769ad 100644
--- a/block.h
+++ b/block.h
@@ -141,6 +141,8 @@ bool bdrv_dev_is_tray_open(BlockDriverState *bs);
bool bdrv_dev_is_medium_locked(BlockDriverState *bs);
int bdrv_read(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
+int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
+ uint8_t *buf, int nb_sectors);
int bdrv_write(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
int bdrv_pread(BlockDriverState *bs, int64_t offset,
--
1.7.6.5
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 08/32] hd-geometry: Move disk geometry guessing back from block.c
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (6 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 07/32] block: Factor bdrv_read_unthrottled() out of guess_disk_lchs() Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 09/32] hd-geometry: Add tracepoints Markus Armbruster
` (24 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 9c538f1..d021fd0 100644
--- a/block.c
+++ b/block.c
@@ -2103,127 +2103,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 1c769ad..052d0ce 100644
--- a/block.h
+++ b/block.h
@@ -177,7 +177,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 3d77259..5bf1d76 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -137,7 +137,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 ae25194..dca3945 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
@@ -948,7 +949,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;
@@ -982,7 +983,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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 09/32] hd-geometry: Add tracepoints
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (7 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 08/32] hd-geometry: Move disk geometry guessing back from block.c Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 10/32] hd-geometry: Unnest conditional in hd_geometry_guess() Markus Armbruster
` (23 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 c935ba2..cb1df3e 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 10/32] hd-geometry: Unnest conditional in hd_geometry_guess()
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (8 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 09/32] hd-geometry: Add tracepoints Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 11/32] hd-geometry: Factor out guess_chs_for_size() Markus Armbruster
` (22 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 11/32] hd-geometry: Factor out guess_chs_for_size()
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (9 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 10/32] hd-geometry: Unnest conditional in hd_geometry_guess() Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 12/32] hd-geometry: Clean up gratuitous goto in hd_geometry_guess() Markus Armbruster
` (21 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 12/32] hd-geometry: Clean up gratuitous goto in hd_geometry_guess()
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (10 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 11/32] hd-geometry: Factor out guess_chs_for_size() Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 13/32] hd-geometry: Clean up confusing use of prior translation hint Markus Armbruster
` (20 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 13/32] hd-geometry: Clean up confusing use of prior translation hint
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (11 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 12/32] hd-geometry: Clean up gratuitous goto in hd_geometry_guess() Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 14/32] hd-geometry: Cut out block layer translation middleman Markus Armbruster
` (19 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 14/32] hd-geometry: Cut out block layer translation middleman
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (12 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 13/32] hd-geometry: Clean up confusing use of prior translation hint Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 15/32] ide pc: Cut out the block layer geometry middleman Markus Armbruster
` (18 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 dca3945..ecc9d05 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -949,7 +949,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;
@@ -983,7 +983,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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 15/32] ide pc: Cut out the block layer geometry middleman
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (13 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 14/32] hd-geometry: Cut out block layer translation middleman Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 16/32] blockdev: Save geometry in DriveInfo Markus Armbruster
` (17 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 e5e7647..5fc64f7 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, §ors);
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, §ors) >= 0) {
+ cmos_init_hd(s, 0x19, 0x1b, cylinders, heads, sectors);
+ val |= 0xf0;
+ }
+ if (ide_get_geometry(arg->idebus[0], 1,
+ &cylinders, &heads, §ors) >= 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, §ors) >= 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, §ors);
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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 16/32] blockdev: Save geometry in DriveInfo
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (14 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 15/32] ide pc: Cut out the block layer geometry middleman Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 17/32] qdev: Introduce block geometry properties Markus Armbruster
` (16 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 9e0a72a..4d3b707 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 17/32] qdev: Introduce block geometry properties
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (15 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 16/32] blockdev: Save geometry in DriveInfo Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 18/32] hd-geometry: Switch to uint32_t to match BlockConf Markus Armbruster
` (15 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 052d0ce..e0121d5 100644
--- a/block.h
+++ b/block.h
@@ -427,6 +427,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)
@@ -454,5 +456,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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 18/32] hd-geometry: Switch to uint32_t to match BlockConf
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (16 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 17/32] qdev: Introduce block geometry properties Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 19/32] scsi-hd: qdev properties for disk geometry Markus Armbruster
` (14 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 ecc9d05..bb758f2 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -927,7 +927,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 cb1df3e..b0440ae 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 19/32] scsi-hd: qdev properties for disk geometry
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (17 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 18/32] hd-geometry: Switch to uint32_t to match BlockConf Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 20/32] virtio-blk: " Markus Armbruster
` (13 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 bb758f2..b48a4fa 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -925,9 +925,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) {
@@ -949,19 +946,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;
@@ -983,18 +979,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;
@@ -1707,6 +1702,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);
@@ -1926,6 +1948,7 @@ static Property scsi_hd_properties[] = {
SCSI_DISK_F_REMOVABLE, false),
DEFINE_PROP_BIT("dpofua", SCSIDiskState, features,
SCSI_DISK_F_DPOFUA, false),
+ DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf),
DEFINE_PROP_END_OF_LIST(),
};
--
1.7.6.5
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 20/32] virtio-blk: qdev properties for disk geometry
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (18 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 19/32] scsi-hd: qdev properties for disk geometry Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:57 ` [Qemu-devel] [PATCH 21/32] ide: " Markus Armbruster
` (12 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 21/32] ide: qdev properties for disk geometry
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (19 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 20/32] virtio-blk: " Markus Armbruster
@ 2012-07-06 6:57 ` Markus Armbruster
2012-07-06 6:58 ` [Qemu-devel] [PATCH 22/32] qtest: Cover " Markus Armbruster
` (11 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:57 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 22/32] qtest: Cover qdev properties for disk geometry
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (20 preceding siblings ...)
2012-07-06 6:57 ` [Qemu-devel] [PATCH 21/32] ide: " Markus Armbruster
@ 2012-07-06 6:58 ` Markus Armbruster
2012-07-06 6:58 ` [Qemu-devel] [PATCH 23/32] qdev: Collect private helpers in one place Markus Armbruster
` (10 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:58 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 23/32] qdev: Collect private helpers in one place
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (21 preceding siblings ...)
2012-07-06 6:58 ` [Qemu-devel] [PATCH 22/32] qtest: Cover " Markus Armbruster
@ 2012-07-06 6:58 ` Markus Armbruster
2012-07-06 6:58 ` [Qemu-devel] [PATCH 24/32] qdev: New property type chs-translation Markus Armbruster
` (9 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:58 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 24/32] qdev: New property type chs-translation
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (22 preceding siblings ...)
2012-07-06 6:58 ` [Qemu-devel] [PATCH 23/32] qdev: Collect private helpers in one place Markus Armbruster
@ 2012-07-06 6:58 ` Markus Armbruster
2012-07-06 6:58 ` [Qemu-devel] [PATCH 25/32] ide: qdev property for BIOS CHS translation Markus Armbruster
` (8 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:58 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 25/32] ide: qdev property for BIOS CHS translation
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (23 preceding siblings ...)
2012-07-06 6:58 ` [Qemu-devel] [PATCH 24/32] qdev: New property type chs-translation Markus Armbruster
@ 2012-07-06 6:58 ` Markus Armbruster
2012-07-06 6:58 ` [Qemu-devel] [PATCH 26/32] qtest: Cover " Markus Armbruster
` (7 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:58 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 26/32] qtest: Cover qdev property for BIOS CHS translation
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (24 preceding siblings ...)
2012-07-06 6:58 ` [Qemu-devel] [PATCH 25/32] ide: qdev property for BIOS CHS translation Markus Armbruster
@ 2012-07-06 6:58 ` Markus Armbruster
2012-07-06 6:58 ` [Qemu-devel] [PATCH 27/32] block: Geometry and translation hints are now useless, purge them Markus Armbruster
` (6 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:58 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 27/32] block: Geometry and translation hints are now useless, purge them
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (25 preceding siblings ...)
2012-07-06 6:58 ` [Qemu-devel] [PATCH 26/32] qtest: Cover " Markus Armbruster
@ 2012-07-06 6:58 ` Markus Armbruster
2012-07-06 6:58 ` [Qemu-devel] [PATCH 28/32] ide pc: Put hard disk info into CMOS only for hard disks Markus Armbruster
` (5 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:58 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 d021fd0..1eb5897 100644
--- a/block.c
+++ b/block.c
@@ -1013,12 +1013,6 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
tmp.block_timer = bs_top->block_timer;
tmp.io_limits_enabled = bs_top->io_limits_enabled;
- /* geometry */
- tmp.cyls = bs_top->cyls;
- tmp.heads = bs_top->heads;
- tmp.secs = bs_top->secs;
- tmp.translation = bs_top->translation;
-
/* r/w error */
tmp.on_read_error = bs_top->on_read_error;
tmp.on_write_error = bs_top->on_write_error;
@@ -2103,27 +2097,6 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr)
*nb_sectors_ptr = length;
}
-void bdrv_set_geometry_hint(BlockDriverState *bs,
- int cyls, int heads, int secs)
-{
- bs->cyls = cyls;
- bs->heads = heads;
- bs->secs = secs;
-}
-
-void bdrv_set_translation_hint(BlockDriverState *bs, int translation)
-{
- bs->translation = translation;
-}
-
-void bdrv_get_geometry_hint(BlockDriverState *bs,
- int *pcyls, int *pheads, int *psecs)
-{
- *pcyls = bs->cyls;
- *pheads = bs->heads;
- *psecs = bs->secs;
-}
-
/* throttling disk io limits */
void bdrv_set_io_limits(BlockDriverState *bs,
BlockIOLimit *io_limits)
@@ -2132,11 +2105,6 @@ void bdrv_set_io_limits(BlockDriverState *bs,
bs->io_limits_enabled = bdrv_io_limits_enabled(bs);
}
-int bdrv_get_translation_hint(BlockDriverState *bs)
-{
- return bs->translation;
-}
-
void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
BlockErrorAction on_write_error)
{
diff --git a/block.h b/block.h
index e0121d5..f02316d 100644
--- a/block.h
+++ b/block.h
@@ -256,18 +256,6 @@ int bdrv_has_zero_init(BlockDriverState *bs);
int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
int *pnum);
-#define BIOS_ATA_TRANSLATION_AUTO 0
-#define BIOS_ATA_TRANSLATION_NONE 1
-#define BIOS_ATA_TRANSLATION_LBA 2
-#define BIOS_ATA_TRANSLATION_LARGE 3
-#define BIOS_ATA_TRANSLATION_RECHS 4
-
-void bdrv_set_geometry_hint(BlockDriverState *bs,
- int cyls, int heads, int secs);
-void bdrv_set_translation_hint(BlockDriverState *bs, int translation);
-void bdrv_get_geometry_hint(BlockDriverState *bs,
- int *pcyls, int *pheads, int *psecs);
-int bdrv_get_translation_hint(BlockDriverState *bs);
void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
BlockErrorAction on_write_error);
BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read);
diff --git a/block_int.h b/block_int.h
index 1fb5352..d72317f 100644
--- a/block_int.h
+++ b/block_int.h
@@ -320,7 +320,6 @@ struct BlockDriverState {
/* NOTE: the following infos are only hints for real hardware
drivers. They are not used by the block driver */
- int cyls, heads, secs, translation;
BlockErrorAction on_read_error, on_write_error;
bool iostatus_enabled;
BlockDeviceIoStatus iostatus;
diff --git a/blockdev.c b/blockdev.c
index 4d3b707..3274923 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -7,8 +7,8 @@
* later. See the COPYING file in the top-level directory.
*/
-#include "block.h"
#include "blockdev.h"
+#include "hw/block-common.h"
#include "monitor.h"
#include "qerror.h"
#include "qemu-option.h"
@@ -551,17 +551,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
case IF_SCSI:
case IF_XEN:
case IF_NONE:
- switch(media) {
- case MEDIA_DISK:
- if (cyls != 0) {
- bdrv_set_geometry_hint(dinfo->bdrv, cyls, heads, secs);
- bdrv_set_translation_hint(dinfo->bdrv, translation);
- }
- break;
- case MEDIA_CDROM:
- dinfo->media_cd = 1;
- break;
- }
+ dinfo->media_cd = media == MEDIA_CDROM;
break;
case IF_SD:
case IF_FLOPPY:
diff --git a/hw/block-common.h b/hw/block-common.h
index 2f65186..ec7810d 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -15,6 +15,12 @@
/* Hard disk geometry */
+#define BIOS_ATA_TRANSLATION_AUTO 0
+#define BIOS_ATA_TRANSLATION_NONE 1
+#define BIOS_ATA_TRANSLATION_LBA 2
+#define BIOS_ATA_TRANSLATION_LARGE 3
+#define BIOS_ATA_TRANSLATION_RECHS 4
+
void hd_geometry_guess(BlockDriverState *bs,
uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
int *ptrans);
diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 7626cbb..74678a6 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -122,25 +122,10 @@ void hd_geometry_guess(BlockDriverState *bs,
{
int cylinders, heads, secs, translation;
- bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs);
- translation = bdrv_get_translation_hint(bs);
-
- if (cylinders != 0) {
- /* already got a geometry hint: use it */
- *pcyls = cylinders;
- *pheads = heads;
- *psecs = secs;
- if (ptrans) {
- *ptrans = translation;
- }
- return;
- }
-
- assert(translation == BIOS_ATA_TRANSLATION_AUTO);
-
if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) {
/* no LCHS guess: use a standard physical disk geometry */
guess_chs_for_size(bs, pcyls, pheads, psecs);
+ translation = BIOS_ATA_TRANSLATION_AUTO;
} else if (heads > 16) {
/* LCHS guess with heads > 16 means that a BIOS LBA
translation was active, so a standard physical disk
@@ -149,7 +134,6 @@ void hd_geometry_guess(BlockDriverState *bs,
translation = *pcyls * *pheads <= 131072
? BIOS_ATA_TRANSLATION_LARGE
: BIOS_ATA_TRANSLATION_LBA;
- bdrv_set_translation_hint(bs, translation);
} else {
/* LCHS guess with heads <= 16: use as physical geometry */
*pcyls = cylinders;
@@ -158,11 +142,9 @@ void hd_geometry_guess(BlockDriverState *bs,
/* disable any translation to be in sync with
the logical geometry */
translation = BIOS_ATA_TRANSLATION_NONE;
- bdrv_set_translation_hint(bs, translation);
}
if (ptrans) {
*ptrans = translation;
}
- bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation);
}
diff --git a/hw/pc.c b/hw/pc.c
index 5fc64f7..968376e 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -44,6 +44,7 @@
#include "kvm.h"
#include "xen.h"
#include "blockdev.h"
+#include "hw/block-common.h"
#include "ui/qemu-spice.h"
#include "memory.h"
#include "exec-memory.h"
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 0b18f8c..01c378f 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -2,6 +2,7 @@
#include "qdev.h"
#include "qerror.h"
#include "blockdev.h"
+#include "hw/block-common.h"
void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
{
diff --git a/vl.c b/vl.c
index 1329c30..a7b707c 100644
--- a/vl.c
+++ b/vl.c
@@ -130,8 +130,8 @@ int main(int argc, char **argv)
#include "qemu-timer.h"
#include "qemu-char.h"
#include "cache-utils.h"
-#include "block.h"
#include "blockdev.h"
+#include "hw/block-common.h"
#include "block-migration.h"
#include "dma.h"
#include "audio/audio.h"
--
1.7.6.5
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 28/32] ide pc: Put hard disk info into CMOS only for hard disks
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (26 preceding siblings ...)
2012-07-06 6:58 ` [Qemu-devel] [PATCH 27/32] block: Geometry and translation hints are now useless, purge them Markus Armbruster
@ 2012-07-06 6:58 ` Markus Armbruster
2012-07-06 6:58 ` [Qemu-devel] [PATCH 29/32] qtest: Test we don't put hard disk info into CMOS for a CD-ROM Markus Armbruster
` (4 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:58 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 29/32] qtest: Test we don't put hard disk info into CMOS for a CD-ROM
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (27 preceding siblings ...)
2012-07-06 6:58 ` [Qemu-devel] [PATCH 28/32] ide pc: Put hard disk info into CMOS only for hard disks Markus Armbruster
@ 2012-07-06 6:58 ` Markus Armbruster
2012-07-06 6:58 ` [Qemu-devel] [PATCH 30/32] hd-geometry: Compute BIOS CHS translation in one place Markus Armbruster
` (3 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:58 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 30/32] hd-geometry: Compute BIOS CHS translation in one place
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (28 preceding siblings ...)
2012-07-06 6:58 ` [Qemu-devel] [PATCH 29/32] qtest: Test we don't put hard disk info into CMOS for a CD-ROM Markus Armbruster
@ 2012-07-06 6:58 ` Markus Armbruster
2012-07-06 6:58 ` [Qemu-devel] [PATCH 31/32] blockdev: Drop redundant CHS validation for if=ide Markus Armbruster
` (2 subsequent siblings)
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:58 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 968376e..681a9a7 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, §ors) >= 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 31/32] blockdev: Drop redundant CHS validation for if=ide
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (29 preceding siblings ...)
2012-07-06 6:58 ` [Qemu-devel] [PATCH 30/32] hd-geometry: Compute BIOS CHS translation in one place Markus Armbruster
@ 2012-07-06 6:58 ` Markus Armbruster
2012-07-06 6:58 ` [Qemu-devel] [PATCH 32/32] Relax IDE CHS limits from 16383, 16, 63 to 65535, 16, 255 Markus Armbruster
2012-07-06 7:28 ` [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:58 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 3274923..201c641 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 related [flat|nested] 37+ messages in thread* [Qemu-devel] [PATCH 32/32] Relax IDE CHS limits from 16383, 16, 63 to 65535, 16, 255
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (30 preceding siblings ...)
2012-07-06 6:58 ` [Qemu-devel] [PATCH 31/32] blockdev: Drop redundant CHS validation for if=ide Markus Armbruster
@ 2012-07-06 6:58 ` Markus Armbruster
2012-07-06 7:28 ` [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 6:58 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 related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PATCH 00/32] Disk geometry cleanup
2012-07-06 6:57 [Qemu-devel] [PATCH 00/32] Disk geometry cleanup Markus Armbruster
` (31 preceding siblings ...)
2012-07-06 6:58 ` [Qemu-devel] [PATCH 32/32] Relax IDE CHS limits from 16383, 16, 63 to 65535, 16, 255 Markus Armbruster
@ 2012-07-06 7:28 ` Markus Armbruster
32 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2012-07-06 7:28 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, blauwirbel, stefanha, pbonzini
I screwed up Subject: lacks "v2". Sorry!
^ permalink raw reply [flat|nested] 37+ messages in thread