* [Qemu-devel] [PATCH 01/41] sheepdog: always use coroutine-based network functions
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
@ 2012-07-17 15:59 ` Kevin Wolf
2012-07-17 15:59 ` [Qemu-devel] [PATCH 02/41] sheepdog: do not blindly memset all read buffers Kevin Wolf
` (39 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 15:59 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
This reduces some code duplication.
Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/sheepdog.c | 113 ++++++++++++++++++++++-------------------------------
1 files changed, 47 insertions(+), 66 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 809df39..465dc97 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -498,26 +498,6 @@ success:
return fd;
}
-static int send_req(int sockfd, SheepdogReq *hdr, void *data,
- unsigned int *wlen)
-{
- int ret;
-
- ret = qemu_send_full(sockfd, hdr, sizeof(*hdr), 0);
- if (ret < sizeof(*hdr)) {
- error_report("failed to send a req, %s", strerror(errno));
- return -errno;
- }
-
- ret = qemu_send_full(sockfd, data, *wlen, 0);
- if (ret < *wlen) {
- error_report("failed to send a req, %s", strerror(errno));
- ret = -errno;
- }
-
- return ret;
-}
-
static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data,
unsigned int *wlen)
{
@@ -537,49 +517,6 @@ static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data,
return ret;
}
-static coroutine_fn int do_co_req(int sockfd, SheepdogReq *hdr, void *data,
- unsigned int *wlen, unsigned int *rlen);
-
-static int do_req(int sockfd, SheepdogReq *hdr, void *data,
- unsigned int *wlen, unsigned int *rlen)
-{
- int ret;
-
- if (qemu_in_coroutine()) {
- return do_co_req(sockfd, hdr, data, wlen, rlen);
- }
-
- socket_set_block(sockfd);
- ret = send_req(sockfd, hdr, data, wlen);
- if (ret < 0) {
- goto out;
- }
-
- ret = qemu_recv_full(sockfd, hdr, sizeof(*hdr), 0);
- if (ret < sizeof(*hdr)) {
- error_report("failed to get a rsp, %s", strerror(errno));
- ret = -errno;
- goto out;
- }
-
- if (*rlen > hdr->data_length) {
- *rlen = hdr->data_length;
- }
-
- if (*rlen) {
- ret = qemu_recv_full(sockfd, data, *rlen, 0);
- if (ret < *rlen) {
- error_report("failed to get the data, %s", strerror(errno));
- ret = -errno;
- goto out;
- }
- }
- ret = 0;
-out:
- socket_set_nonblock(sockfd);
- return ret;
-}
-
static void restart_co_req(void *opaque)
{
Coroutine *co = opaque;
@@ -587,11 +524,26 @@ static void restart_co_req(void *opaque)
qemu_coroutine_enter(co, NULL);
}
-static coroutine_fn int do_co_req(int sockfd, SheepdogReq *hdr, void *data,
- unsigned int *wlen, unsigned int *rlen)
+typedef struct SheepdogReqCo {
+ int sockfd;
+ SheepdogReq *hdr;
+ void *data;
+ unsigned int *wlen;
+ unsigned int *rlen;
+ int ret;
+ bool finished;
+} SheepdogReqCo;
+
+static coroutine_fn void do_co_req(void *opaque)
{
int ret;
Coroutine *co;
+ SheepdogReqCo *srco = opaque;
+ int sockfd = srco->sockfd;
+ SheepdogReq *hdr = srco->hdr;
+ void *data = srco->data;
+ unsigned int *wlen = srco->wlen;
+ unsigned int *rlen = srco->rlen;
co = qemu_coroutine_self();
qemu_aio_set_fd_handler(sockfd, NULL, restart_co_req, NULL, co);
@@ -627,7 +579,36 @@ static coroutine_fn int do_co_req(int sockfd, SheepdogReq *hdr, void *data,
out:
qemu_aio_set_fd_handler(sockfd, NULL, NULL, NULL, NULL);
socket_set_nonblock(sockfd);
- return ret;
+
+ srco->ret = ret;
+ srco->finished = true;
+}
+
+static int do_req(int sockfd, SheepdogReq *hdr, void *data,
+ unsigned int *wlen, unsigned int *rlen)
+{
+ Coroutine *co;
+ SheepdogReqCo srco = {
+ .sockfd = sockfd,
+ .hdr = hdr,
+ .data = data,
+ .wlen = wlen,
+ .rlen = rlen,
+ .ret = 0,
+ .finished = false,
+ };
+
+ if (qemu_in_coroutine()) {
+ do_co_req(&srco);
+ } else {
+ co = qemu_coroutine_create(do_co_req);
+ qemu_coroutine_enter(co, &srco);
+ while (!srco.finished) {
+ qemu_aio_wait();
+ }
+ }
+
+ return srco.ret;
}
static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 02/41] sheepdog: do not blindly memset all read buffers
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
2012-07-17 15:59 ` [Qemu-devel] [PATCH 01/41] sheepdog: always use coroutine-based network functions Kevin Wolf
@ 2012-07-17 15:59 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 03/41] fdc: Move floppy geometry guessing back from block.c Kevin Wolf
` (38 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 15:59 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Christoph Hellwig <hch@lst.de>
Only buffers that map to unallocated blocks need to be zeroed.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/sheepdog.c | 37 ++++++++++++++++++-------------------
1 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 465dc97..a04ad99 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1571,18 +1571,25 @@ static int coroutine_fn sd_co_rw_vector(void *p)
len = MIN(total - done, SD_DATA_OBJ_SIZE - offset);
- if (!inode->data_vdi_id[idx]) {
- if (acb->aiocb_type == AIOCB_READ_UDATA) {
+ switch (acb->aiocb_type) {
+ case AIOCB_READ_UDATA:
+ if (!inode->data_vdi_id[idx]) {
+ qemu_iovec_memset(acb->qiov, done, 0, len);
goto done;
}
-
- create = 1;
- } else if (acb->aiocb_type == AIOCB_WRITE_UDATA
- && !is_data_obj_writable(inode, idx)) {
- /* Copy-On-Write */
- create = 1;
- old_oid = oid;
- flags = SD_FLAG_CMD_COW;
+ break;
+ case AIOCB_WRITE_UDATA:
+ if (!inode->data_vdi_id[idx]) {
+ create = 1;
+ } else if (!is_data_obj_writable(inode, idx)) {
+ /* Copy-On-Write */
+ create = 1;
+ old_oid = oid;
+ flags = SD_FLAG_CMD_COW;
+ }
+ break;
+ default:
+ break;
}
if (create) {
@@ -1668,20 +1675,12 @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
SheepdogAIOCB *acb;
- int i, ret;
+ int ret;
acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors, NULL, NULL);
acb->aiocb_type = AIOCB_READ_UDATA;
acb->aio_done_func = sd_finish_aiocb;
- /*
- * TODO: we can do better; we don't need to initialize
- * blindly.
- */
- for (i = 0; i < qiov->niov; i++) {
- memset(qiov->iov[i].iov_base, 0, qiov->iov[i].iov_len);
- }
-
ret = sd_co_rw_vector(acb);
if (ret <= 0) {
qemu_aio_release(acb);
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 03/41] fdc: Move floppy geometry guessing back from block.c
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
2012-07-17 15:59 ` [Qemu-devel] [PATCH 01/41] sheepdog: always use coroutine-based network functions Kevin Wolf
2012-07-17 15:59 ` [Qemu-devel] [PATCH 02/41] sheepdog: do not blindly memset all read buffers Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 04/41] vvfat: Fix partition table Kevin Wolf
` (37 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 101 ---------------------------------------------------
block.h | 18 ---------
hw/fdc.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
hw/fdc.h | 10 +++++-
hw/pc.c | 11 +-----
5 files changed, 123 insertions(+), 139 deletions(-)
diff --git a/block.c b/block.c
index 0c923f2..ffda1c2 100644
--- a/block.c
+++ b/block.c
@@ -2282,107 +2282,6 @@ void bdrv_set_io_limits(BlockDriverState *bs,
bs->io_limits_enabled = bdrv_io_limits_enabled(bs);
}
-/* Recognize floppy formats */
-typedef struct FDFormat {
- FDriveType drive;
- uint8_t last_sect;
- uint8_t max_track;
- uint8_t max_head;
- FDriveRate rate;
-} FDFormat;
-
-static const FDFormat fd_formats[] = {
- /* First entry is default format */
- /* 1.44 MB 3"1/2 floppy disks */
- { FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, },
- { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
- { FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, },
- { FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, },
- { FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, },
- { FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, },
- { FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, },
- { FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, },
- /* 2.88 MB 3"1/2 floppy disks */
- { FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, },
- { FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, },
- { FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, },
- { FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, },
- { FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, },
- /* 720 kB 3"1/2 floppy disks */
- { FDRIVE_DRV_144, 9, 80, 1, FDRIVE_RATE_250K, },
- { FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, },
- { FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, },
- { FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, },
- { FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, },
- { FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, },
- /* 1.2 MB 5"1/4 floppy disks */
- { FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, },
- { FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, },
- { FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, },
- { FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, },
- { FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, },
- /* 720 kB 5"1/4 floppy disks */
- { FDRIVE_DRV_120, 9, 80, 1, FDRIVE_RATE_250K, },
- { FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, },
- /* 360 kB 5"1/4 floppy disks */
- { FDRIVE_DRV_120, 9, 40, 1, FDRIVE_RATE_300K, },
- { FDRIVE_DRV_120, 9, 40, 0, FDRIVE_RATE_300K, },
- { FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, },
- { FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, },
- /* 320 kB 5"1/4 floppy disks */
- { FDRIVE_DRV_120, 8, 40, 1, FDRIVE_RATE_250K, },
- { FDRIVE_DRV_120, 8, 40, 0, FDRIVE_RATE_250K, },
- /* 360 kB must match 5"1/4 better than 3"1/2... */
- { FDRIVE_DRV_144, 9, 80, 0, FDRIVE_RATE_250K, },
- /* end */
- { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
-};
-
-void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads,
- int *max_track, int *last_sect,
- FDriveType drive_in, FDriveType *drive,
- FDriveRate *rate)
-{
- const FDFormat *parse;
- uint64_t nb_sectors, size;
- int i, first_match, match;
-
- bdrv_get_geometry(bs, &nb_sectors);
- match = -1;
- first_match = -1;
- for (i = 0; ; i++) {
- parse = &fd_formats[i];
- if (parse->drive == FDRIVE_DRV_NONE) {
- break;
- }
- if (drive_in == parse->drive ||
- drive_in == FDRIVE_DRV_NONE) {
- size = (parse->max_head + 1) * parse->max_track *
- parse->last_sect;
- if (nb_sectors == size) {
- match = i;
- break;
- }
- if (first_match == -1) {
- first_match = i;
- }
- }
- }
- if (match == -1) {
- if (first_match == -1) {
- match = 1;
- } else {
- match = first_match;
- }
- parse = &fd_formats[match];
- }
- *nb_heads = parse->max_head + 1;
- *max_track = parse->max_track;
- *last_sect = parse->last_sect;
- *drive = parse->drive;
- *rate = parse->rate;
-}
-
int bdrv_get_translation_hint(BlockDriverState *bs)
{
return bs->translation;
diff --git a/block.h b/block.h
index e34d942..b24f664 100644
--- a/block.h
+++ b/block.h
@@ -269,24 +269,6 @@ void bdrv_set_geometry_hint(BlockDriverState *bs,
void bdrv_set_translation_hint(BlockDriverState *bs, int translation);
void bdrv_get_geometry_hint(BlockDriverState *bs,
int *pcyls, int *pheads, int *psecs);
-typedef enum FDriveType {
- FDRIVE_DRV_144 = 0x00, /* 1.44 MB 3"5 drive */
- FDRIVE_DRV_288 = 0x01, /* 2.88 MB 3"5 drive */
- FDRIVE_DRV_120 = 0x02, /* 1.2 MB 5"25 drive */
- FDRIVE_DRV_NONE = 0x03, /* No drive connected */
-} FDriveType;
-
-typedef enum FDriveRate {
- FDRIVE_RATE_500K = 0x00, /* 500 Kbps */
- FDRIVE_RATE_300K = 0x01, /* 300 Kbps */
- FDRIVE_RATE_250K = 0x02, /* 250 Kbps */
- FDRIVE_RATE_1M = 0x03, /* 1 Mbps */
-} FDriveRate;
-
-void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads,
- int *max_track, int *last_sect,
- FDriveType drive_in, FDriveType *drive,
- FDriveRate *rate);
int bdrv_get_translation_hint(BlockDriverState *bs);
void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
BlockErrorAction on_write_error);
diff --git a/hw/fdc.c b/hw/fdc.c
index edf0706..41191c7 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -52,6 +52,113 @@
/********************************************************/
/* Floppy drive emulation */
+typedef enum FDriveRate {
+ FDRIVE_RATE_500K = 0x00, /* 500 Kbps */
+ FDRIVE_RATE_300K = 0x01, /* 300 Kbps */
+ FDRIVE_RATE_250K = 0x02, /* 250 Kbps */
+ FDRIVE_RATE_1M = 0x03, /* 1 Mbps */
+} FDriveRate;
+
+typedef struct FDFormat {
+ FDriveType drive;
+ uint8_t last_sect;
+ uint8_t max_track;
+ uint8_t max_head;
+ FDriveRate rate;
+} FDFormat;
+
+static const FDFormat fd_formats[] = {
+ /* First entry is default format */
+ /* 1.44 MB 3"1/2 floppy disks */
+ { FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, },
+ { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
+ { FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, },
+ { FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, },
+ { FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, },
+ { FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, },
+ { FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, },
+ { FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, },
+ /* 2.88 MB 3"1/2 floppy disks */
+ { FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, },
+ { FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, },
+ { FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, },
+ { FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, },
+ { FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, },
+ /* 720 kB 3"1/2 floppy disks */
+ { FDRIVE_DRV_144, 9, 80, 1, FDRIVE_RATE_250K, },
+ { FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, },
+ { FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, },
+ { FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, },
+ { FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, },
+ { FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, },
+ /* 1.2 MB 5"1/4 floppy disks */
+ { FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, },
+ { FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, },
+ { FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, },
+ { FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, },
+ { FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, },
+ /* 720 kB 5"1/4 floppy disks */
+ { FDRIVE_DRV_120, 9, 80, 1, FDRIVE_RATE_250K, },
+ { FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, },
+ /* 360 kB 5"1/4 floppy disks */
+ { FDRIVE_DRV_120, 9, 40, 1, FDRIVE_RATE_300K, },
+ { FDRIVE_DRV_120, 9, 40, 0, FDRIVE_RATE_300K, },
+ { FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, },
+ { FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, },
+ /* 320 kB 5"1/4 floppy disks */
+ { FDRIVE_DRV_120, 8, 40, 1, FDRIVE_RATE_250K, },
+ { FDRIVE_DRV_120, 8, 40, 0, FDRIVE_RATE_250K, },
+ /* 360 kB must match 5"1/4 better than 3"1/2... */
+ { FDRIVE_DRV_144, 9, 80, 0, FDRIVE_RATE_250K, },
+ /* end */
+ { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
+};
+
+static void pick_geometry(BlockDriverState *bs, int *nb_heads,
+ int *max_track, int *last_sect,
+ FDriveType drive_in, FDriveType *drive,
+ FDriveRate *rate)
+{
+ const FDFormat *parse;
+ uint64_t nb_sectors, size;
+ int i, first_match, match;
+
+ bdrv_get_geometry(bs, &nb_sectors);
+ match = -1;
+ first_match = -1;
+ for (i = 0; ; i++) {
+ parse = &fd_formats[i];
+ if (parse->drive == FDRIVE_DRV_NONE) {
+ break;
+ }
+ if (drive_in == parse->drive ||
+ drive_in == FDRIVE_DRV_NONE) {
+ size = (parse->max_head + 1) * parse->max_track *
+ parse->last_sect;
+ if (nb_sectors == size) {
+ match = i;
+ break;
+ }
+ if (first_match == -1) {
+ first_match = i;
+ }
+ }
+ }
+ if (match == -1) {
+ if (first_match == -1) {
+ match = 1;
+ } else {
+ match = first_match;
+ }
+ parse = &fd_formats[match];
+ }
+ *nb_heads = parse->max_head + 1;
+ *max_track = parse->max_track;
+ *last_sect = parse->last_sect;
+ *drive = parse->drive;
+ *rate = parse->rate;
+}
+
#define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
#define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive))
@@ -187,8 +294,8 @@ static void fd_revalidate(FDrive *drv)
FLOPPY_DPRINTF("revalidate\n");
if (drv->bs != NULL) {
ro = bdrv_is_read_only(drv->bs);
- bdrv_get_floppy_geometry_hint(drv->bs, &nb_heads, &max_track,
- &last_sect, drv->drive, &drive, &rate);
+ pick_geometry(drv->bs, &nb_heads, &max_track,
+ &last_sect, drv->drive, &drive, &rate);
if (!bdrv_is_inserted(drv->bs)) {
FLOPPY_DPRINTF("No disk in drive\n");
} else {
@@ -2054,18 +2161,13 @@ static int sun4m_fdc_init1(SysBusDevice *dev)
return fdctrl_init_common(fdctrl);
}
-void fdc_get_bs(BlockDriverState *bs[], ISADevice *dev)
+FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
{
- FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
- FDCtrl *fdctrl = &isa->state;
- int i;
+ FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, fdc);
- for (i = 0; i < MAX_FD; i++) {
- bs[i] = fdctrl->drives[i].bs;
- }
+ return isa->state.drives[i].drive;
}
-
static const VMStateDescription vmstate_isa_fdc ={
.name = "fdc",
.version_id = 2,
diff --git a/hw/fdc.h b/hw/fdc.h
index 1b32b17..b5c9f31 100644
--- a/hw/fdc.h
+++ b/hw/fdc.h
@@ -6,11 +6,19 @@
/* fdc.c */
#define MAX_FD 2
+typedef enum FDriveType {
+ FDRIVE_DRV_144 = 0x00, /* 1.44 MB 3"5 drive */
+ FDRIVE_DRV_288 = 0x01, /* 2.88 MB 3"5 drive */
+ FDRIVE_DRV_120 = 0x02, /* 1.2 MB 5"25 drive */
+ FDRIVE_DRV_NONE = 0x03, /* No drive connected */
+} FDriveType;
+
ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
target_phys_addr_t mmio_base, DriveInfo **fds);
void sun4m_fdctrl_init(qemu_irq irq, target_phys_addr_t io_base,
DriveInfo **fds, qemu_irq *fdc_tc);
-void fdc_get_bs(BlockDriverState *bs[], ISADevice *dev);
+
+FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
#endif
diff --git a/hw/pc.c b/hw/pc.c
index c7e9ab3..91cf77d 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -335,10 +335,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
ISADevice *floppy, BusState *idebus0, BusState *idebus1,
ISADevice *s)
{
- int val, nb, nb_heads, max_track, last_sect, i;
+ int val, nb, i;
FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
- FDriveRate rate;
- BlockDriverState *fd[MAX_FD];
static pc_cmos_init_late_arg arg;
/* various important CMOS locations needed by PC/Bochs bios */
@@ -381,13 +379,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
/* floppy type */
if (floppy) {
- fdc_get_bs(fd, floppy);
for (i = 0; i < 2; i++) {
- if (fd[i]) {
- bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
- &last_sect, FDRIVE_DRV_NONE,
- &fd_type[i], &rate);
- }
+ fd_type[i] = isa_fdc_get_drive_type(floppy, i);
}
}
val = (cmos_get_fd_drive_type(fd_type[0]) << 4) |
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 04/41] vvfat: Fix partition table
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (2 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 03/41] fdc: Move floppy geometry guessing back from block.c Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 05/41] vvfat: Do not clobber the user's geometry Kevin Wolf
` (36 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 05/41] vvfat: Do not clobber the user's geometry
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (3 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 04/41] vvfat: Fix partition table Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 06/41] qtest: Add hard disk geometry test Kevin Wolf
` (35 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vvfat.c | 53 +++++++++++++++++++++++++++++------------------------
1 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index e2b83a2..7b1dcee 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,18 +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;
+ if (s->first_sectors_number == 0x40) {
+ init_mbr(s, cyls, heads, secs);
}
// assert(is_consistent(s));
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 06/41] qtest: Add hard disk geometry test
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (4 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 05/41] vvfat: Do not clobber the user's geometry Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 07/41] hd-geometry: Move disk geometry guessing back from block.c Kevin Wolf
` (34 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
So far covers only IDE and tests only CMOS contents.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/Makefile | 2 +
tests/hd-geo-test.c | 403 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 405 insertions(+), 0 deletions(-)
create mode 100644 tests/hd-geo-test.c
diff --git a/tests/Makefile b/tests/Makefile
index d687ecc..9675ba7 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -21,6 +21,7 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
# All QTests for now are POSIX-only, but the dependencies are
# really in libqtest, not in the testcases themselves.
check-qtest-i386-y = tests/fdc-test$(EXESUF)
+check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
check-qtest-i386-y += tests/rtc-test$(EXESUF)
check-qtest-x86_64-y = $(check-qtest-i386-y)
check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
@@ -72,6 +73,7 @@ tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(
tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y)
tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y)
+tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y)
# QTest rules
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
new file mode 100644
index 0000000..cc447a2
--- /dev/null
+++ b/tests/hd-geo-test.c
@@ -0,0 +1,403 @@
+/*
+ * Hard disk geometry test cases.
+ *
+ * Copyright (c) 2012 Red Hat Inc.
+ *
+ * Authors:
+ * Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+/*
+ * Covers only IDE and tests only CMOS contents. Better than nothing.
+ * Improvements welcome.
+ */
+
+#include <glib.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include "qemu-common.h"
+#include "libqtest.h"
+
+static const char test_image[] = "/tmp/qtest.XXXXXX";
+
+static char *create_test_img(int secs)
+{
+ char *template = strdup("/tmp/qtest.XXXXXX");
+ int fd, ret;
+
+ fd = mkstemp(template);
+ g_assert(fd >= 0);
+ ret = ftruncate(fd, (off_t)secs * 512);
+ g_assert(ret == 0);
+ close(fd);
+ return template;
+}
+
+typedef struct {
+ int cyls, heads, secs, trans;
+} CHST;
+
+typedef enum {
+ mbr_blank, mbr_lba, mbr_chs,
+ mbr_last
+} MBRcontents;
+
+typedef enum {
+ /* order is relevant */
+ backend_small, backend_large, backend_empty,
+ backend_last
+} Backend;
+
+static const int img_secs[backend_last] = {
+ [backend_small] = 61440,
+ [backend_large] = 8388608,
+ [backend_empty] = -1,
+};
+
+static const CHST hd_chst[backend_last][mbr_last] = {
+ [backend_small] = {
+ [mbr_blank] = { 60, 16, 63, 0 },
+ [mbr_lba] = { 60, 16, 63, 2 },
+ [mbr_chs] = { 60, 16, 63, 0 }
+ },
+ [backend_large] = {
+ [mbr_blank] = { 8322, 16, 63, 1 },
+ [mbr_lba] = { 8322, 16, 63, 1 },
+ [mbr_chs] = { 8322, 16, 63, 0 }
+ },
+};
+
+static const char *img_file_name[backend_last];
+
+static const CHST *cur_ide[4];
+
+static bool is_hd(const CHST *expected_chst)
+{
+ return expected_chst && expected_chst->cyls;
+}
+
+static void test_cmos_byte(int reg, int expected)
+{
+ enum { cmos_base = 0x70 };
+ int actual;
+
+ outb(cmos_base + 0, reg);
+ actual = inb(cmos_base + 1);
+ g_assert(actual == expected);
+}
+
+static void test_cmos_bytes(int reg0, int n, uint8_t expected[])
+{
+ int i;
+
+ for (i = 0; i < 9; i++) {
+ test_cmos_byte(reg0 + i, expected[i]);
+ }
+}
+
+static void test_cmos_disk_data(void)
+{
+ test_cmos_byte(0x12,
+ (is_hd(cur_ide[0]) ? 0xf0 : 0) |
+ (is_hd(cur_ide[1]) ? 0x0f : 0));
+}
+
+static void test_cmos_drive_cyl(int reg0, const CHST *expected_chst)
+{
+ if (is_hd(expected_chst)) {
+ int c = expected_chst->cyls;
+ int h = expected_chst->heads;
+ int s = expected_chst->secs;
+ uint8_t expected_bytes[9] = {
+ c & 0xff, c >> 8, h, 0xff, 0xff, 0xc0 | ((h > 8) << 3),
+ c & 0xff, c >> 8, s
+ };
+ test_cmos_bytes(reg0, 9, expected_bytes);
+ } else {
+ int i;
+
+ for (i = 0; i < 9; i++) {
+ test_cmos_byte(reg0 + i, 0);
+ }
+ }
+}
+
+static void test_cmos_drive1(void)
+{
+ test_cmos_byte(0x19, is_hd(cur_ide[0]) ? 47 : 0);
+ test_cmos_drive_cyl(0x1b, cur_ide[0]);
+}
+
+static void test_cmos_drive2(void)
+{
+ test_cmos_byte(0x1a, is_hd(cur_ide[1]) ? 47 : 0);
+ test_cmos_drive_cyl(0x24, cur_ide[1]);
+}
+
+static void test_cmos_disktransflag(void)
+{
+ int val, i;
+
+ val = 0;
+ for (i = 0; i < ARRAY_SIZE(cur_ide); i++) {
+ if (is_hd(cur_ide[i])) {
+ val |= cur_ide[i]->trans << (2 * i);
+ }
+ }
+ test_cmos_byte(0x39, val);
+}
+
+static void test_cmos(void)
+{
+ test_cmos_disk_data();
+ test_cmos_drive1();
+ test_cmos_drive2();
+ test_cmos_disktransflag();
+}
+
+static int append_arg(int argc, char *argv[], int argv_sz, char *arg)
+{
+ g_assert(argc + 1 < argv_sz);
+ argv[argc++] = arg;
+ argv[argc] = NULL;
+ return argc;
+}
+
+static int setup_common(char *argv[], int argv_sz)
+{
+ memset(cur_ide, 0, sizeof(cur_ide));
+ return append_arg(0, argv, argv_sz,
+ g_strdup("-nodefaults -display none"));
+}
+
+static void setup_mbr(int img_idx, MBRcontents mbr)
+{
+ static const uint8_t part_lba[16] = {
+ /* chs 0,1,1 (lba 63) to chs 0,127,63 (8001 sectors) */
+ 0x80, 1, 1, 0, 6, 127, 63, 0, 63, 0, 0, 0, 0x41, 0x1F, 0, 0,
+ };
+ static const uint8_t part_chs[16] = {
+ /* chs 0,1,1 (lba 63) to chs 7,15,63 (8001 sectors) */
+ 0x80, 1, 1, 0, 6, 15, 63, 7, 63, 0, 0, 0, 0x41, 0x1F, 0, 0,
+ };
+ uint8_t buf[512];
+ int fd, ret;
+
+ memset(buf, 0, sizeof(buf));
+
+ if (mbr != mbr_blank) {
+ buf[0x1fe] = 0x55;
+ buf[0x1ff] = 0xAA;
+ memcpy(buf + 0x1BE, mbr == mbr_lba ? part_lba : part_chs, 16);
+ }
+
+ fd = open(img_file_name[img_idx], O_WRONLY);
+ g_assert(fd >= 0);
+ ret = write(fd, buf, sizeof(buf));
+ g_assert(ret == sizeof(buf));
+ close(fd);
+}
+
+static int setup_ide(int argc, char *argv[], int argv_sz,
+ int ide_idx, const char *dev, int img_idx,
+ MBRcontents mbr, const char *opts)
+{
+ char *s1, *s2, *s3;
+
+ s1 = g_strdup_printf("-drive id=drive%d,if=%s",
+ ide_idx, dev ? "none" : "ide");
+ s2 = dev ? g_strdup("") : g_strdup_printf(",index=%d", ide_idx);
+
+ if (img_secs[img_idx] >= 0) {
+ setup_mbr(img_idx, mbr);
+ s3 = g_strdup_printf(",file=%s", img_file_name[img_idx]);
+ } else {
+ s3 = g_strdup(",media=cdrom");
+ }
+ argc = append_arg(argc, argv, argv_sz,
+ g_strdup_printf("%s%s%s%s", s1, s2, s3, opts));
+ g_free(s1);
+ g_free(s2);
+ g_free(s3);
+
+ if (dev) {
+ argc = append_arg(argc, argv, argv_sz,
+ g_strdup_printf("-device %s,drive=drive%d,"
+ "bus=ide.%d,unit=%d",
+ dev, ide_idx,
+ ide_idx / 2, ide_idx % 2));
+ }
+ return argc;
+}
+
+/*
+ * Test case: no IDE devices
+ */
+static void test_ide_none(void)
+{
+ char *argv[256];
+
+ setup_common(argv, ARRAY_SIZE(argv));
+ qtest_start(g_strjoinv(" ", argv));
+ test_cmos();
+ qtest_quit(global_qtest);
+}
+
+static void test_ide_mbr(bool use_device, MBRcontents mbr)
+{
+ char *argv[256];
+ int argc;
+ Backend i;
+ const char *dev;
+
+ argc = setup_common(argv, ARRAY_SIZE(argv));
+ for (i = 0; i < backend_last; i++) {
+ cur_ide[i] = &hd_chst[i][mbr];
+ dev = use_device ? (is_hd(cur_ide[i]) ? "ide-hd" : "ide-cd") : NULL;
+ argc = setup_ide(argc, argv, ARRAY_SIZE(argv), i, dev, i, mbr, "");
+ }
+ qtest_start(g_strjoinv(" ", argv));
+ test_cmos();
+ qtest_quit(global_qtest);
+}
+
+/*
+ * Test case: IDE devices (if=ide) with blank MBRs
+ */
+static void test_ide_drive_mbr_blank(void)
+{
+ test_ide_mbr(false, mbr_blank);
+}
+
+/*
+ * Test case: IDE devices (if=ide) with MBRs indicating LBA is in use
+ */
+static void test_ide_drive_mbr_lba(void)
+{
+ test_ide_mbr(false, mbr_lba);
+}
+
+/*
+ * Test case: IDE devices (if=ide) with MBRs indicating CHS is in use
+ */
+static void test_ide_drive_mbr_chs(void)
+{
+ test_ide_mbr(false, mbr_chs);
+}
+
+/*
+ * Test case: IDE devices (if=none) with blank MBRs
+ */
+static void test_ide_device_mbr_blank(void)
+{
+ test_ide_mbr(true, mbr_blank);
+}
+
+/*
+ * Test case: IDE devices (if=none) with MBRs indicating LBA is in use
+ */
+static void test_ide_device_mbr_lba(void)
+{
+ test_ide_mbr(true, mbr_lba);
+}
+
+/*
+ * Test case: IDE devices (if=none) with MBRs indicating CHS is in use
+ */
+static void test_ide_device_mbr_chs(void)
+{
+ test_ide_mbr(true, mbr_chs);
+}
+
+static void test_ide_drive_user(const char *dev, bool trans)
+{
+ char *argv[256], *opts;
+ int argc;
+ int secs = img_secs[backend_small];
+ const CHST expected_chst = { secs / (4 * 32) , 4, 32, trans };
+
+ argc = setup_common(argv, ARRAY_SIZE(argv));
+ opts = g_strdup_printf(",cyls=%d,heads=%d,secs=%d%s",
+ expected_chst.cyls, expected_chst.heads,
+ expected_chst.secs,
+ trans ? ",trans=lba" : "");
+ cur_ide[0] = &expected_chst;
+ argc = setup_ide(argc, argv, ARRAY_SIZE(argv),
+ 0, dev, backend_small, mbr_chs, opts);
+ g_free(opts);
+ qtest_start(g_strjoinv(" ", argv));
+ test_cmos();
+ qtest_quit(global_qtest);
+}
+
+/*
+ * Test case: IDE device (if=ide) with explicit CHS
+ */
+static void test_ide_drive_user_chs(void)
+{
+ test_ide_drive_user(NULL, false);
+}
+
+/*
+ * Test case: IDE device (if=ide) with explicit CHS and translation
+ */
+static void test_ide_drive_user_chst(void)
+{
+ test_ide_drive_user(NULL, true);
+}
+
+/*
+ * Test case: IDE device (if=none) with explicit CHS
+ */
+static void test_ide_device_user_chs(void)
+{
+ test_ide_drive_user("ide-hd", false);
+}
+
+/*
+ * Test case: IDE device (if=none) with explicit CHS and translation
+ */
+static void test_ide_device_user_chst(void)
+{
+ test_ide_drive_user("ide-hd", true);
+}
+
+int main(int argc, char **argv)
+{
+ Backend i;
+ int ret;
+
+ g_test_init(&argc, &argv, NULL);
+
+ for (i = 0; i < backend_last; i++) {
+ if (img_secs[i] >= 0) {
+ img_file_name[i] = create_test_img(img_secs[i]);
+ } else {
+ img_file_name[i] = NULL;
+ }
+ }
+
+ qtest_add_func("hd-geo/ide/none", test_ide_none);
+ qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
+ qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
+ qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
+ qtest_add_func("hd-geo/ide/drive/user/chs", test_ide_drive_user_chs);
+ qtest_add_func("hd-geo/ide/drive/user/chst", test_ide_drive_user_chst);
+ qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank);
+ qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
+ qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs);
+ qtest_add_func("hd-geo/ide/device/user/chs", test_ide_device_user_chs);
+ qtest_add_func("hd-geo/ide/device/user/chst", test_ide_device_user_chst);
+
+ ret = g_test_run();
+
+ for (i = 0; i < backend_last; i++) {
+ unlink(img_file_name[i]);
+ }
+
+ return ret;
+}
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 07/41] hd-geometry: Move disk geometry guessing back from block.c
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (5 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 06/41] qtest: Add hard disk geometry test Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 08/41] hd-geometry: Add tracepoints Kevin Wolf
` (33 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 121 ---------------------------------------
block.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 +-
8 files changed, 191 insertions(+), 127 deletions(-)
create mode 100644 hw/block-common.h
create mode 100644 hw/hd-geometry.c
diff --git a/block.c b/block.c
index ffda1c2..06323cf 100644
--- a/block.c
+++ b/block.c
@@ -2132,127 +2132,6 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr)
*nb_sectors_ptr = length;
}
-struct partition {
- uint8_t boot_ind; /* 0x80 - active */
- uint8_t head; /* starting head */
- uint8_t sector; /* starting sector */
- uint8_t cyl; /* starting cylinder */
- uint8_t sys_ind; /* What partition type */
- uint8_t end_head; /* end head */
- uint8_t end_sector; /* end sector */
- uint8_t end_cyl; /* end cylinder */
- uint32_t start_sect; /* starting sector counting from 0 */
- uint32_t nr_sects; /* nr of sectors in partition */
-} QEMU_PACKED;
-
-/* try to guess the disk logical geometry from the MSDOS partition table. Return 0 if OK, -1 if could not guess */
-static int guess_disk_lchs(BlockDriverState *bs,
- int *pcylinders, int *pheads, int *psectors)
-{
- uint8_t buf[BDRV_SECTOR_SIZE];
- int i, heads, sectors, cylinders;
- struct partition *p;
- uint32_t nr_sects;
- uint64_t nb_sectors;
-
- bdrv_get_geometry(bs, &nb_sectors);
-
- /**
- * The function will be invoked during startup not only in sync I/O mode,
- * but also in async I/O mode. So the I/O throttling function has to
- * be disabled temporarily here, not permanently.
- */
- if (bdrv_read_unthrottled(bs, 0, buf, 1) < 0) {
- return -1;
- }
- /* test msdos magic */
- if (buf[510] != 0x55 || buf[511] != 0xaa)
- return -1;
- for(i = 0; i < 4; i++) {
- p = ((struct partition *)(buf + 0x1be)) + i;
- nr_sects = le32_to_cpu(p->nr_sects);
- if (nr_sects && p->end_head) {
- /* We make the assumption that the partition terminates on
- a cylinder boundary */
- heads = p->end_head + 1;
- sectors = p->end_sector & 63;
- if (sectors == 0)
- continue;
- cylinders = nb_sectors / (heads * sectors);
- if (cylinders < 1 || cylinders > 16383)
- continue;
- *pheads = heads;
- *psectors = sectors;
- *pcylinders = cylinders;
-#if 0
- printf("guessed geometry: LCHS=%d %d %d\n",
- cylinders, heads, sectors);
-#endif
- return 0;
- }
- }
- return -1;
-}
-
-void bdrv_guess_geometry(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs)
-{
- int translation, lba_detected = 0;
- int cylinders, heads, secs;
- uint64_t nb_sectors;
-
- /* if a geometry hint is available, use it */
- bdrv_get_geometry(bs, &nb_sectors);
- bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs);
- translation = bdrv_get_translation_hint(bs);
- if (cylinders != 0) {
- *pcyls = cylinders;
- *pheads = heads;
- *psecs = secs;
- } else {
- if (guess_disk_lchs(bs, &cylinders, &heads, &secs) == 0) {
- if (heads > 16) {
- /* if heads > 16, it means that a BIOS LBA
- translation was active, so the default
- hardware geometry is OK */
- lba_detected = 1;
- goto default_geometry;
- } else {
- *pcyls = cylinders;
- *pheads = heads;
- *psecs = secs;
- /* disable any translation to be in sync with
- the logical geometry */
- if (translation == BIOS_ATA_TRANSLATION_AUTO) {
- bdrv_set_translation_hint(bs,
- BIOS_ATA_TRANSLATION_NONE);
- }
- }
- } else {
- default_geometry:
- /* if no geometry, use a standard physical disk geometry */
- cylinders = nb_sectors / (16 * 63);
-
- if (cylinders > 16383)
- cylinders = 16383;
- else if (cylinders < 2)
- cylinders = 2;
- *pcyls = cylinders;
- *pheads = 16;
- *psecs = 63;
- if ((lba_detected == 1) && (translation == BIOS_ATA_TRANSLATION_AUTO)) {
- if ((*pcyls * *pheads) <= 131072) {
- bdrv_set_translation_hint(bs,
- BIOS_ATA_TRANSLATION_LARGE);
- } else {
- bdrv_set_translation_hint(bs,
- BIOS_ATA_TRANSLATION_LBA);
- }
- }
- }
- bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
- }
-}
-
void bdrv_set_geometry_hint(BlockDriverState *bs,
int cyls, int heads, int secs)
{
diff --git a/block.h b/block.h
index b24f664..993894e 100644
--- a/block.h
+++ b/block.h
@@ -178,7 +178,6 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset);
int64_t bdrv_getlength(BlockDriverState *bs);
int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
-void bdrv_guess_geometry(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs);
int bdrv_commit(BlockDriverState *bs);
int bdrv_commit_all(void);
int bdrv_change_backing_file(BlockDriverState *bs,
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 9a350de..c3bdedc 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -138,7 +138,7 @@ common-obj-$(CONFIG_MAX111X) += max111x.o
common-obj-$(CONFIG_DS1338) += ds1338.o
common-obj-y += i2c.o smbus.o smbus_eeprom.o
common-obj-y += eeprom93xx.o
-common-obj-y += scsi-disk.o cdrom.o
+common-obj-y += scsi-disk.o cdrom.o hd-geometry.o
common-obj-y += scsi-generic.o scsi-bus.o
common-obj-y += hid.o
common-obj-$(CONFIG_SSI) += ssi.o
diff --git a/hw/block-common.h b/hw/block-common.h
new file mode 100644
index 0000000..3a4d4c6
--- /dev/null
+++ b/hw/block-common.h
@@ -0,0 +1,21 @@
+/*
+ * Common code for block device models
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#ifndef HW_BLOCK_COMMON_H
+#define HW_BLOCK_COMMON_H
+
+#include "qemu-common.h"
+
+/* Hard disk geometry */
+
+void hd_geometry_guess(BlockDriverState *bs,
+ int *pcyls, int *pheads, int *psecs);
+
+#endif
diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
new file mode 100644
index 0000000..c45eafd
--- /dev/null
+++ b/hw/hd-geometry.c
@@ -0,0 +1,162 @@
+/*
+ * Hard disk geometry utilities
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * This file incorporates work covered by the following copyright and
+ * permission notice:
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "block.h"
+#include "hw/block-common.h"
+
+struct partition {
+ uint8_t boot_ind; /* 0x80 - active */
+ uint8_t head; /* starting head */
+ uint8_t sector; /* starting sector */
+ uint8_t cyl; /* starting cylinder */
+ uint8_t sys_ind; /* What partition type */
+ uint8_t end_head; /* end head */
+ uint8_t end_sector; /* end sector */
+ uint8_t end_cyl; /* end cylinder */
+ uint32_t start_sect; /* starting sector counting from 0 */
+ uint32_t nr_sects; /* nr of sectors in partition */
+} QEMU_PACKED;
+
+/* try to guess the disk logical geometry from the MSDOS partition table.
+ Return 0 if OK, -1 if could not guess */
+static int guess_disk_lchs(BlockDriverState *bs,
+ int *pcylinders, int *pheads, int *psectors)
+{
+ uint8_t buf[BDRV_SECTOR_SIZE];
+ int i, heads, sectors, cylinders;
+ struct partition *p;
+ uint32_t nr_sects;
+ uint64_t nb_sectors;
+
+ bdrv_get_geometry(bs, &nb_sectors);
+
+ /**
+ * The function will be invoked during startup not only in sync I/O mode,
+ * but also in async I/O mode. So the I/O throttling function has to
+ * be disabled temporarily here, not permanently.
+ */
+ if (bdrv_read_unthrottled(bs, 0, buf, 1) < 0) {
+ return -1;
+ }
+ /* test msdos magic */
+ if (buf[510] != 0x55 || buf[511] != 0xaa) {
+ return -1;
+ }
+ for (i = 0; i < 4; i++) {
+ p = ((struct partition *)(buf + 0x1be)) + i;
+ nr_sects = le32_to_cpu(p->nr_sects);
+ if (nr_sects && p->end_head) {
+ /* We make the assumption that the partition terminates on
+ a cylinder boundary */
+ heads = p->end_head + 1;
+ sectors = p->end_sector & 63;
+ if (sectors == 0) {
+ continue;
+ }
+ cylinders = nb_sectors / (heads * sectors);
+ if (cylinders < 1 || cylinders > 16383) {
+ continue;
+ }
+ *pheads = heads;
+ *psectors = sectors;
+ *pcylinders = cylinders;
+#if 0
+ printf("guessed geometry: LCHS=%d %d %d\n",
+ cylinders, heads, sectors);
+#endif
+ return 0;
+ }
+ }
+ return -1;
+}
+
+void hd_geometry_guess(BlockDriverState *bs,
+ int *pcyls, int *pheads, int *psecs)
+{
+ int translation, lba_detected = 0;
+ int cylinders, heads, secs;
+ uint64_t nb_sectors;
+
+ /* if a geometry hint is available, use it */
+ bdrv_get_geometry(bs, &nb_sectors);
+ bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs);
+ translation = bdrv_get_translation_hint(bs);
+ if (cylinders != 0) {
+ *pcyls = cylinders;
+ *pheads = heads;
+ *psecs = secs;
+ } else {
+ if (guess_disk_lchs(bs, &cylinders, &heads, &secs) == 0) {
+ if (heads > 16) {
+ /* if heads > 16, it means that a BIOS LBA
+ translation was active, so the default
+ hardware geometry is OK */
+ lba_detected = 1;
+ goto default_geometry;
+ } else {
+ *pcyls = cylinders;
+ *pheads = heads;
+ *psecs = secs;
+ /* disable any translation to be in sync with
+ the logical geometry */
+ if (translation == BIOS_ATA_TRANSLATION_AUTO) {
+ bdrv_set_translation_hint(bs,
+ BIOS_ATA_TRANSLATION_NONE);
+ }
+ }
+ } else {
+ default_geometry:
+ /* if no geometry, use a standard physical disk geometry */
+ cylinders = nb_sectors / (16 * 63);
+
+ if (cylinders > 16383) {
+ cylinders = 16383;
+ } else if (cylinders < 2) {
+ cylinders = 2;
+ }
+ *pcyls = cylinders;
+ *pheads = 16;
+ *psecs = 63;
+ if ((lba_detected == 1)
+ && (translation == BIOS_ATA_TRANSLATION_AUTO)) {
+ if ((*pcyls * *pheads) <= 131072) {
+ bdrv_set_translation_hint(bs,
+ BIOS_ATA_TRANSLATION_LARGE);
+ } else {
+ bdrv_set_translation_hint(bs,
+ BIOS_ATA_TRANSLATION_LBA);
+ }
+ }
+ }
+ bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
+ }
+}
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 71d4d77..0d1bf10 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -30,6 +30,7 @@
#include "qemu-timer.h"
#include "sysemu.h"
#include "dma.h"
+#include "hw/block-common.h"
#include "blockdev.h"
#include <hw/ide/internal.h>
@@ -1933,7 +1934,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
s->drive_kind = kind;
bdrv_get_geometry(bs, &nb_sectors);
- bdrv_guess_geometry(bs, &cylinders, &heads, &secs);
+ hd_geometry_guess(bs, &cylinders, &heads, &secs);
if (cylinders < 1 || cylinders > 16383) {
error_report("cyls must be between 1 and 16383");
return -1;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 34336b1..5339c2e 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -34,6 +34,7 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
#include "scsi-defs.h"
#include "sysemu.h"
#include "blockdev.h"
+#include "hw/block-common.h"
#include "dma.h"
#ifdef __linux
@@ -989,7 +990,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
break;
}
/* if a geometry hint is available, use it */
- bdrv_guess_geometry(bdrv, &cylinders, &heads, &secs);
+ hd_geometry_guess(bdrv, &cylinders, &heads, &secs);
p[2] = (cylinders >> 16) & 0xff;
p[3] = (cylinders >> 8) & 0xff;
p[4] = cylinders & 0xff;
@@ -1023,7 +1024,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
p[2] = 5000 >> 8;
p[3] = 5000 & 0xff;
/* if a geometry hint is available, use it */
- bdrv_guess_geometry(bdrv, &cylinders, &heads, &secs);
+ hd_geometry_guess(bdrv, &cylinders, &heads, &secs);
p[4] = heads & 0xff;
p[5] = secs & 0xff;
p[6] = s->qdev.blocksize >> 8;
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index fe07746..f16c5ce 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -14,6 +14,7 @@
#include "qemu-common.h"
#include "qemu-error.h"
#include "trace.h"
+#include "hw/block-common.h"
#include "blockdev.h"
#include "virtio-blk.h"
#include "scsi-defs.h"
@@ -622,7 +623,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
s->blk = blk;
s->rq = NULL;
s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
- bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
+ hd_geometry_guess(s->bs, &cylinders, &heads, &secs);
s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 08/41] hd-geometry: Add tracepoints
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (6 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 07/41] hd-geometry: Move disk geometry guessing back from block.c Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 09/41] hd-geometry: Unnest conditional in hd_geometry_guess() Kevin Wolf
` (32 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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 fc32bc6..5f27f1a 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] 42+ messages in thread* [Qemu-devel] [PATCH 09/41] hd-geometry: Unnest conditional in hd_geometry_guess()
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (7 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 08/41] hd-geometry: Add tracepoints Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 10/41] hd-geometry: Factor out guess_chs_for_size() Kevin Wolf
` (31 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 10/41] hd-geometry: Factor out guess_chs_for_size()
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (8 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 09/41] hd-geometry: Unnest conditional in hd_geometry_guess() Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 11/41] hd-geometry: Clean up gratuitous goto in hd_geometry_guess() Kevin Wolf
` (30 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 11/41] hd-geometry: Clean up gratuitous goto in hd_geometry_guess()
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (9 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 10/41] hd-geometry: Factor out guess_chs_for_size() Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 12/41] hd-geometry: Clean up confusing use of prior translation hint Kevin Wolf
` (29 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 12/41] hd-geometry: Clean up confusing use of prior translation hint
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (10 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 11/41] hd-geometry: Clean up gratuitous goto in hd_geometry_guess() Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 13/41] hd-geometry: Cut out block layer translation middleman Kevin Wolf
` (28 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 13/41] hd-geometry: Cut out block layer translation middleman
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (11 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 12/41] hd-geometry: Clean up confusing use of prior translation hint Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 14/41] ide pc: Cut out the block layer geometry middleman Kevin Wolf
` (27 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/block-common.h | 3 ++-
hw/hd-geometry.c | 20 ++++++++++++++------
hw/ide/core.c | 2 +-
hw/scsi-disk.c | 4 ++--
hw/virtio-blk.c | 2 +-
5 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/hw/block-common.h b/hw/block-common.h
index 3a4d4c6..bba817a 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -16,6 +16,7 @@
/* Hard disk geometry */
void hd_geometry_guess(BlockDriverState *bs,
- int *pcyls, int *pheads, int *psecs);
+ int *pcyls, int *pheads, int *psecs,
+ int *ptrans);
#endif
diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 241aed9..4d746b7 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -117,7 +117,8 @@ static void guess_chs_for_size(BlockDriverState *bs,
}
void hd_geometry_guess(BlockDriverState *bs,
- int *pcyls, int *pheads, int *psecs)
+ int *pcyls, int *pheads, int *psecs,
+ int *ptrans)
{
int cylinders, heads, secs, translation;
@@ -129,6 +130,9 @@ void hd_geometry_guess(BlockDriverState *bs,
*pcyls = cylinders;
*pheads = heads;
*psecs = secs;
+ if (ptrans) {
+ *ptrans = translation;
+ }
return;
}
@@ -142,10 +146,10 @@ void hd_geometry_guess(BlockDriverState *bs,
translation was active, so a standard physical disk
geometry is OK */
guess_chs_for_size(bs, pcyls, pheads, psecs);
- bdrv_set_translation_hint(bs,
- *pcyls * *pheads <= 131072
- ? BIOS_ATA_TRANSLATION_LARGE
- : BIOS_ATA_TRANSLATION_LBA);
+ translation = *pcyls * *pheads <= 131072
+ ? BIOS_ATA_TRANSLATION_LARGE
+ : BIOS_ATA_TRANSLATION_LBA;
+ bdrv_set_translation_hint(bs, translation);
} else {
/* LCHS guess with heads <= 16: use as physical geometry */
*pcyls = cylinders;
@@ -153,7 +157,11 @@ void hd_geometry_guess(BlockDriverState *bs,
*psecs = secs;
/* disable any translation to be in sync with
the logical geometry */
- bdrv_set_translation_hint(bs, BIOS_ATA_TRANSLATION_NONE);
+ translation = BIOS_ATA_TRANSLATION_NONE;
+ bdrv_set_translation_hint(bs, translation);
+ }
+ if (ptrans) {
+ *ptrans = translation;
}
bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 0d1bf10..28f04ad 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1934,7 +1934,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
s->drive_kind = kind;
bdrv_get_geometry(bs, &nb_sectors);
- hd_geometry_guess(bs, &cylinders, &heads, &secs);
+ hd_geometry_guess(bs, &cylinders, &heads, &secs, NULL);
if (cylinders < 1 || cylinders > 16383) {
error_report("cyls must be between 1 and 16383");
return -1;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 5339c2e..fc077f5 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -990,7 +990,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
break;
}
/* if a geometry hint is available, use it */
- hd_geometry_guess(bdrv, &cylinders, &heads, &secs);
+ hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL);
p[2] = (cylinders >> 16) & 0xff;
p[3] = (cylinders >> 8) & 0xff;
p[4] = cylinders & 0xff;
@@ -1024,7 +1024,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
p[2] = 5000 >> 8;
p[3] = 5000 & 0xff;
/* if a geometry hint is available, use it */
- hd_geometry_guess(bdrv, &cylinders, &heads, &secs);
+ hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL);
p[4] = heads & 0xff;
p[5] = secs & 0xff;
p[6] = s->qdev.blocksize >> 8;
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index f16c5ce..d2709a7 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -623,7 +623,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
s->blk = blk;
s->rq = NULL;
s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
- hd_geometry_guess(s->bs, &cylinders, &heads, &secs);
+ hd_geometry_guess(s->bs, &cylinders, &heads, &secs, NULL);
s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 14/41] ide pc: Cut out the block layer geometry middleman
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (12 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 13/41] hd-geometry: Cut out block layer translation middleman Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 15/41] blockdev: Save geometry in DriveInfo Kevin Wolf
` (26 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/ide.h | 4 +++-
hw/ide/core.c | 2 +-
hw/ide/internal.h | 2 +-
hw/ide/qdev.c | 21 +++++++++++++++++----
hw/pc.c | 51 +++++++++++++++++++++++++++------------------------
5 files changed, 49 insertions(+), 31 deletions(-)
diff --git a/hw/ide.h b/hw/ide.h
index 0b18c90..2db4079 100644
--- a/hw/ide.h
+++ b/hw/ide.h
@@ -29,7 +29,9 @@ void mmio_ide_init (target_phys_addr_t membase, target_phys_addr_t membase2,
qemu_irq irq, int shift,
DriveInfo *hd0, DriveInfo *hd1);
-void ide_get_bs(BlockDriverState *bs[], BusState *qbus);
+int ide_get_geometry(BusState *bus, int unit,
+ int16_t *cyls, int8_t *heads, int8_t *secs);
+int ide_get_bios_chs_trans(BusState *bus, int unit);
/* ide/core.c */
void ide_drive_get(DriveInfo **hd, int max_bus);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 28f04ad..7f5ad07 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1934,7 +1934,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
s->drive_kind = kind;
bdrv_get_geometry(bs, &nb_sectors);
- hd_geometry_guess(bs, &cylinders, &heads, &secs, NULL);
+ hd_geometry_guess(bs, &cylinders, &heads, &secs, &s->chs_trans);
if (cylinders < 1 || cylinders > 16383) {
error_report("cyls must be between 1 and 16383");
return -1;
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 1a02f57..56c718e 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -344,7 +344,7 @@ struct IDEState {
uint8_t unit;
/* ide config */
IDEDriveKind drive_kind;
- int cylinders, heads, sectors;
+ int cylinders, heads, sectors, chs_trans;
int64_t nb_sectors;
int mult_sectors;
int identify_set;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index c122395..87e0b75 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -111,11 +111,24 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
return DO_UPCAST(IDEDevice, qdev, dev);
}
-void ide_get_bs(BlockDriverState *bs[], BusState *qbus)
+int ide_get_geometry(BusState *bus, int unit,
+ int16_t *cyls, int8_t *heads, int8_t *secs)
{
- IDEBus *bus = DO_UPCAST(IDEBus, qbus, qbus);
- bs[0] = bus->master ? bus->master->conf.bs : NULL;
- bs[1] = bus->slave ? bus->slave->conf.bs : NULL;
+ IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
+
+ if (!s->bs) {
+ return -1;
+ }
+
+ *cyls = s->cylinders;
+ *heads = s->heads;
+ *secs = s->sectors;
+ return 0;
+}
+
+int ide_get_bios_chs_trans(BusState *bus, int unit)
+{
+ return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans;
}
/* --------------------------------- */
diff --git a/hw/pc.c b/hw/pc.c
index 91cf77d..89a0c66 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -216,11 +216,9 @@ static int cmos_get_fd_drive_type(FDriveType fd0)
return val;
}
-static void cmos_init_hd(int type_ofs, int info_ofs, BlockDriverState *hd,
- ISADevice *s)
+static void cmos_init_hd(ISADevice *s, int type_ofs, int info_ofs,
+ int16_t cylinders, int8_t heads, int8_t sectors)
{
- int cylinders, heads, sectors;
- bdrv_get_geometry_hint(hd, &cylinders, &heads, §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] 42+ messages in thread* [Qemu-devel] [PATCH 15/41] blockdev: Save geometry in DriveInfo
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (13 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 14/41] ide pc: Cut out the block layer geometry middleman Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 16/41] qdev: Introduce block geometry properties Kevin Wolf
` (25 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
In preparation of purging it from the block layer, which will happen
later in this series.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 4 ++++
blockdev.h | 1 +
2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index a85a429..161985b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -530,6 +530,10 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
dinfo->type = type;
dinfo->bus = bus_id;
dinfo->unit = unit_id;
+ dinfo->cyls = cyls;
+ dinfo->heads = heads;
+ dinfo->secs = secs;
+ dinfo->trans = translation;
dinfo->opts = opts;
dinfo->refcount = 1;
if (serial) {
diff --git a/blockdev.h b/blockdev.h
index 260e16b..9c29948 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] 42+ messages in thread* [Qemu-devel] [PATCH 16/41] qdev: Introduce block geometry properties
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (14 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 15/41] blockdev: Save geometry in DriveInfo Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 17/41] hd-geometry: Switch to uint32_t to match BlockConf Kevin Wolf
` (24 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.h | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/block.h b/block.h
index 993894e..1cd8a01 100644
--- a/block.h
+++ b/block.h
@@ -426,6 +426,8 @@ typedef struct BlockConf {
uint32_t opt_io_size;
int32_t bootindex;
uint32_t discard_granularity;
+ /* geometry, not all devices use this */
+ uint32_t cyls, heads, secs;
} BlockConf;
static inline unsigned int get_physical_block_exp(BlockConf *conf)
@@ -453,5 +455,9 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
DEFINE_PROP_UINT32("discard_granularity", _state, \
_conf.discard_granularity, 0)
-#endif
+#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
+ DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
+ DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
+ DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
+#endif
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 17/41] hd-geometry: Switch to uint32_t to match BlockConf
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (15 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 16/41] qdev: Introduce block geometry properties Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 18/41] scsi-hd: qdev properties for disk geometry Kevin Wolf
` (23 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/block-common.h | 2 +-
hw/hd-geometry.c | 4 ++--
hw/ide/core.c | 2 +-
hw/scsi-disk.c | 2 +-
hw/virtio-blk.c | 2 +-
trace-events | 2 +-
6 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/block-common.h b/hw/block-common.h
index bba817a..2f65186 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -16,7 +16,7 @@
/* Hard disk geometry */
void hd_geometry_guess(BlockDriverState *bs,
- int *pcyls, int *pheads, int *psecs,
+ uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
int *ptrans);
#endif
diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 4d746b7..7626cbb 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -98,7 +98,7 @@ static int guess_disk_lchs(BlockDriverState *bs,
}
static void guess_chs_for_size(BlockDriverState *bs,
- int *pcyls, int *pheads, int *psecs)
+ uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs)
{
uint64_t nb_sectors;
int cylinders;
@@ -117,7 +117,7 @@ static void guess_chs_for_size(BlockDriverState *bs,
}
void hd_geometry_guess(BlockDriverState *bs,
- int *pcyls, int *pheads, int *psecs,
+ uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
int *ptrans)
{
int cylinders, heads, secs, translation;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 7f5ad07..f1966e3 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1927,7 +1927,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
const char *version, const char *serial, const char *model,
uint64_t wwn)
{
- int cylinders, heads, secs;
+ uint32_t cylinders, heads, secs;
uint64_t nb_sectors;
s->bs = bs;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index fc077f5..c881acf 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -968,7 +968,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
};
BlockDriverState *bdrv = s->qdev.conf.bs;
- int cylinders, heads, secs;
+ uint32_t cylinders, heads, secs;
uint8_t *p = *p_outbuf;
if ((mode_sense_valid[page] & (1 << s->qdev.type)) == 0) {
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index d2709a7..4344e28 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -590,7 +590,7 @@ static const BlockDevOps virtio_block_ops = {
VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
{
VirtIOBlock *s;
- int cylinders, heads, secs;
+ uint32_t cylinders, heads, secs;
static int virtio_blk_id;
DriveInfo *dinfo;
diff --git a/trace-events b/trace-events
index 5f27f1a..8b1fb24 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] 42+ messages in thread* [Qemu-devel] [PATCH 18/41] scsi-hd: qdev properties for disk geometry
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (16 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 17/41] hd-geometry: Switch to uint32_t to match BlockConf Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 19/41] virtio-blk: " Kevin Wolf
` (22 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/scsi-disk.c | 69 +++++++++++++++++++++++++++++++++++++------------------
1 files changed, 46 insertions(+), 23 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index c881acf..0a182f9 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -966,9 +966,6 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
[MODE_PAGE_AUDIO_CTL] = (1 << TYPE_ROM),
[MODE_PAGE_CAPABILITIES] = (1 << TYPE_ROM),
};
-
- BlockDriverState *bdrv = s->qdev.conf.bs;
- uint32_t cylinders, heads, secs;
uint8_t *p = *p_outbuf;
if ((mode_sense_valid[page] & (1 << s->qdev.type)) == 0) {
@@ -990,19 +987,18 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
break;
}
/* if a geometry hint is available, use it */
- hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL);
- p[2] = (cylinders >> 16) & 0xff;
- p[3] = (cylinders >> 8) & 0xff;
- p[4] = cylinders & 0xff;
- p[5] = heads & 0xff;
+ p[2] = (s->qdev.conf.cyls >> 16) & 0xff;
+ p[3] = (s->qdev.conf.cyls >> 8) & 0xff;
+ p[4] = s->qdev.conf.cyls & 0xff;
+ p[5] = s->qdev.conf.heads & 0xff;
/* Write precomp start cylinder, disabled */
- p[6] = (cylinders >> 16) & 0xff;
- p[7] = (cylinders >> 8) & 0xff;
- p[8] = cylinders & 0xff;
+ p[6] = (s->qdev.conf.cyls >> 16) & 0xff;
+ p[7] = (s->qdev.conf.cyls >> 8) & 0xff;
+ p[8] = s->qdev.conf.cyls & 0xff;
/* Reduced current start cylinder, disabled */
- p[9] = (cylinders >> 16) & 0xff;
- p[10] = (cylinders >> 8) & 0xff;
- p[11] = cylinders & 0xff;
+ p[9] = (s->qdev.conf.cyls >> 16) & 0xff;
+ p[10] = (s->qdev.conf.cyls >> 8) & 0xff;
+ p[11] = s->qdev.conf.cyls & 0xff;
/* Device step rate [ns], 200ns */
p[12] = 0;
p[13] = 200;
@@ -1024,18 +1020,17 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
p[2] = 5000 >> 8;
p[3] = 5000 & 0xff;
/* if a geometry hint is available, use it */
- hd_geometry_guess(bdrv, &cylinders, &heads, &secs, NULL);
- p[4] = heads & 0xff;
- p[5] = secs & 0xff;
+ p[4] = s->qdev.conf.heads & 0xff;
+ p[5] = s->qdev.conf.secs & 0xff;
p[6] = s->qdev.blocksize >> 8;
- p[8] = (cylinders >> 8) & 0xff;
- p[9] = cylinders & 0xff;
+ p[8] = (s->qdev.conf.cyls >> 8) & 0xff;
+ p[9] = s->qdev.conf.cyls & 0xff;
/* Write precomp start cylinder, disabled */
- p[10] = (cylinders >> 8) & 0xff;
- p[11] = cylinders & 0xff;
+ p[10] = (s->qdev.conf.cyls >> 8) & 0xff;
+ p[11] = s->qdev.conf.cyls & 0xff;
/* Reduced current start cylinder, disabled */
- p[12] = (cylinders >> 8) & 0xff;
- p[13] = cylinders & 0xff;
+ p[12] = (s->qdev.conf.cyls >> 8) & 0xff;
+ p[13] = s->qdev.conf.cyls & 0xff;
/* Device step rate [100us], 100us */
p[14] = 0;
p[15] = 1;
@@ -1755,6 +1750,33 @@ static int scsi_initfn(SCSIDevice *dev)
return -1;
}
+ if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
+ /* try to fall back to value set with legacy -drive cyls=... */
+ dinfo = drive_get_by_blockdev(s->qdev.conf.bs);
+ dev->conf.cyls = dinfo->cyls;
+ dev->conf.heads = dinfo->heads;
+ dev->conf.secs = dinfo->secs;
+ }
+ if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
+ hd_geometry_guess(s->qdev.conf.bs,
+ &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs,
+ NULL);
+ }
+ if (dev->conf.cyls || dev->conf.heads || dev->conf.secs) {
+ if (dev->conf.cyls < 1 || dev->conf.cyls > 65535) {
+ error_report("cyls must be between 1 and 65535");
+ return -1;
+ }
+ if (dev->conf.heads < 1 || dev->conf.heads > 255) {
+ error_report("heads must be between 1 and 255");
+ return -1;
+ }
+ if (dev->conf.secs < 1 || dev->conf.secs > 255) {
+ error_report("secs must be between 1 and 255");
+ return -1;
+ }
+ }
+
if (!s->serial) {
/* try to fall back to value set with legacy -drive serial=... */
dinfo = drive_get_by_blockdev(s->qdev.conf.bs);
@@ -1975,6 +1997,7 @@ static Property scsi_hd_properties[] = {
DEFINE_PROP_BIT("dpofua", SCSIDiskState, features,
SCSI_DISK_F_DPOFUA, false),
DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0),
+ DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf),
DEFINE_PROP_END_OF_LIST(),
};
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 19/41] virtio-blk: qdev properties for disk geometry
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (17 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 18/41] scsi-hd: qdev properties for disk geometry Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 20/41] ide: " Kevin Wolf
` (21 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 20/41] ide: qdev properties for disk geometry
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (18 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 19/41] virtio-blk: " Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 21/41] qtest: Cover " Kevin Wolf
` (20 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 21/41] qtest: Cover qdev properties for disk geometry
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (19 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 20/41] ide: " Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 22/41] qdev: Collect private helpers in one place Kevin Wolf
` (19 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 22/41] qdev: Collect private helpers in one place
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (20 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 21/41] qtest: Cover " Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 23/41] qdev: New property type chs-translation Kevin Wolf
` (18 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Just code motion, with one long line wrapped to keep checkpatch.pl
happy.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 23/41] qdev: New property type chs-translation
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (21 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 22/41] qdev: Collect private helpers in one place Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 24/41] ide: qdev property for BIOS CHS translation Kevin Wolf
` (17 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 24/41] ide: qdev property for BIOS CHS translation
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (22 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 23/41] qdev: New property type chs-translation Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 25/41] qtest: Cover " Kevin Wolf
` (16 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 25/41] qtest: Cover qdev property for BIOS CHS translation
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (23 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 24/41] ide: qdev property for BIOS CHS translation Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 26/41] block: Geometry and translation hints are now useless, purge them Kevin Wolf
` (15 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 26/41] block: Geometry and translation hints are now useless, purge them
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (24 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 25/41] qtest: Cover " Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 27/41] ide pc: Put hard disk info into CMOS only for hard disks Kevin Wolf
` (14 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
There are two producers of these hints: drive_init() on behalf of
-drive, and hd_geometry_guess().
The only consumer of the hint is hd_geometry_guess().
The callers of hd_geometry_guess() call it only when drive_init()
didn't set the hints. Therefore, drive_init()'s hints are never used.
Thus, hd_geometry_guess() only ever sees hints it produced itself in a
prior call. Only the first call computes something, subsequent calls
just repeat the first call's results. However, hd_geometry_guess() is
never called more than once: the device models don't, and the block
device is destroyed on unplug. Thus, dropping the repeat feature
doesn't break anything now.
If a block device wasn't destroyed on unplug and could be reused with
a new device, then repeating old results would be wrong. Thus,
dropping the repeat feature prevents future breakage.
This renders the hints unused. Purge them from the block layer.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 32 --------------------------------
block.h | 12 ------------
block_int.h | 1 -
blockdev.c | 14 ++------------
hw/block-common.h | 6 ++++++
hw/hd-geometry.c | 20 +-------------------
hw/pc.c | 1 +
hw/qdev-properties.c | 1 +
vl.c | 2 +-
9 files changed, 12 insertions(+), 77 deletions(-)
diff --git a/block.c b/block.c
index 06323cf..ce7eb8f 100644
--- a/block.c
+++ b/block.c
@@ -996,12 +996,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
bs_dest->block_timer = bs_src->block_timer;
bs_dest->io_limits_enabled = bs_src->io_limits_enabled;
- /* geometry */
- bs_dest->cyls = bs_src->cyls;
- bs_dest->heads = bs_src->heads;
- bs_dest->secs = bs_src->secs;
- bs_dest->translation = bs_src->translation;
-
/* r/w error */
bs_dest->on_read_error = bs_src->on_read_error;
bs_dest->on_write_error = bs_src->on_write_error;
@@ -2132,27 +2126,6 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr)
*nb_sectors_ptr = length;
}
-void bdrv_set_geometry_hint(BlockDriverState *bs,
- int cyls, int heads, int secs)
-{
- bs->cyls = cyls;
- bs->heads = heads;
- bs->secs = secs;
-}
-
-void bdrv_set_translation_hint(BlockDriverState *bs, int translation)
-{
- bs->translation = translation;
-}
-
-void bdrv_get_geometry_hint(BlockDriverState *bs,
- int *pcyls, int *pheads, int *psecs)
-{
- *pcyls = bs->cyls;
- *pheads = bs->heads;
- *psecs = bs->secs;
-}
-
/* throttling disk io limits */
void bdrv_set_io_limits(BlockDriverState *bs,
BlockIOLimit *io_limits)
@@ -2161,11 +2134,6 @@ void bdrv_set_io_limits(BlockDriverState *bs,
bs->io_limits_enabled = bdrv_io_limits_enabled(bs);
}
-int bdrv_get_translation_hint(BlockDriverState *bs)
-{
- return bs->translation;
-}
-
void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
BlockErrorAction on_write_error)
{
diff --git a/block.h b/block.h
index 1cd8a01..29c5eab 100644
--- a/block.h
+++ b/block.h
@@ -257,18 +257,6 @@ int bdrv_has_zero_init(BlockDriverState *bs);
int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
int *pnum);
-#define BIOS_ATA_TRANSLATION_AUTO 0
-#define BIOS_ATA_TRANSLATION_NONE 1
-#define BIOS_ATA_TRANSLATION_LBA 2
-#define BIOS_ATA_TRANSLATION_LARGE 3
-#define BIOS_ATA_TRANSLATION_RECHS 4
-
-void bdrv_set_geometry_hint(BlockDriverState *bs,
- int cyls, int heads, int secs);
-void bdrv_set_translation_hint(BlockDriverState *bs, int translation);
-void bdrv_get_geometry_hint(BlockDriverState *bs,
- int *pcyls, int *pheads, int *psecs);
-int bdrv_get_translation_hint(BlockDriverState *bs);
void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
BlockErrorAction on_write_error);
BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read);
diff --git a/block_int.h b/block_int.h
index 1fb5352..d72317f 100644
--- a/block_int.h
+++ b/block_int.h
@@ -320,7 +320,6 @@ struct BlockDriverState {
/* NOTE: the following infos are only hints for real hardware
drivers. They are not used by the block driver */
- int cyls, heads, secs, translation;
BlockErrorAction on_read_error, on_write_error;
bool iostatus_enabled;
BlockDeviceIoStatus iostatus;
diff --git a/blockdev.c b/blockdev.c
index 161985b..06c997e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -7,8 +7,8 @@
* later. See the COPYING file in the top-level directory.
*/
-#include "block.h"
#include "blockdev.h"
+#include "hw/block-common.h"
#include "monitor.h"
#include "qerror.h"
#include "qemu-option.h"
@@ -551,17 +551,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
case IF_SCSI:
case IF_XEN:
case IF_NONE:
- switch(media) {
- case MEDIA_DISK:
- if (cyls != 0) {
- bdrv_set_geometry_hint(dinfo->bdrv, cyls, heads, secs);
- bdrv_set_translation_hint(dinfo->bdrv, translation);
- }
- break;
- case MEDIA_CDROM:
- dinfo->media_cd = 1;
- break;
- }
+ dinfo->media_cd = media == MEDIA_CDROM;
break;
case IF_SD:
case IF_FLOPPY:
diff --git a/hw/block-common.h b/hw/block-common.h
index 2f65186..ec7810d 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -15,6 +15,12 @@
/* Hard disk geometry */
+#define BIOS_ATA_TRANSLATION_AUTO 0
+#define BIOS_ATA_TRANSLATION_NONE 1
+#define BIOS_ATA_TRANSLATION_LBA 2
+#define BIOS_ATA_TRANSLATION_LARGE 3
+#define BIOS_ATA_TRANSLATION_RECHS 4
+
void hd_geometry_guess(BlockDriverState *bs,
uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
int *ptrans);
diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 7626cbb..74678a6 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -122,25 +122,10 @@ void hd_geometry_guess(BlockDriverState *bs,
{
int cylinders, heads, secs, translation;
- bdrv_get_geometry_hint(bs, &cylinders, &heads, &secs);
- translation = bdrv_get_translation_hint(bs);
-
- if (cylinders != 0) {
- /* already got a geometry hint: use it */
- *pcyls = cylinders;
- *pheads = heads;
- *psecs = secs;
- if (ptrans) {
- *ptrans = translation;
- }
- return;
- }
-
- assert(translation == BIOS_ATA_TRANSLATION_AUTO);
-
if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) {
/* no LCHS guess: use a standard physical disk geometry */
guess_chs_for_size(bs, pcyls, pheads, psecs);
+ translation = BIOS_ATA_TRANSLATION_AUTO;
} else if (heads > 16) {
/* LCHS guess with heads > 16 means that a BIOS LBA
translation was active, so a standard physical disk
@@ -149,7 +134,6 @@ void hd_geometry_guess(BlockDriverState *bs,
translation = *pcyls * *pheads <= 131072
? BIOS_ATA_TRANSLATION_LARGE
: BIOS_ATA_TRANSLATION_LBA;
- bdrv_set_translation_hint(bs, translation);
} else {
/* LCHS guess with heads <= 16: use as physical geometry */
*pcyls = cylinders;
@@ -158,11 +142,9 @@ void hd_geometry_guess(BlockDriverState *bs,
/* disable any translation to be in sync with
the logical geometry */
translation = BIOS_ATA_TRANSLATION_NONE;
- bdrv_set_translation_hint(bs, translation);
}
if (ptrans) {
*ptrans = translation;
}
- bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation);
}
diff --git a/hw/pc.c b/hw/pc.c
index 89a0c66..77b12b4 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -44,6 +44,7 @@
#include "kvm.h"
#include "xen.h"
#include "blockdev.h"
+#include "hw/block-common.h"
#include "ui/qemu-spice.h"
#include "memory.h"
#include "exec-memory.h"
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 0b18f8c..01c378f 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -2,6 +2,7 @@
#include "qdev.h"
#include "qerror.h"
#include "blockdev.h"
+#include "hw/block-common.h"
void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
{
diff --git a/vl.c b/vl.c
index 46248b9..8904db1 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] 42+ messages in thread* [Qemu-devel] [PATCH 27/41] ide pc: Put hard disk info into CMOS only for hard disks
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (25 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 26/41] block: Geometry and translation hints are now useless, purge them Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 28/41] qtest: Test we don't put hard disk info into CMOS for a CD-ROM Kevin Wolf
` (13 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 28/41] qtest: Test we don't put hard disk info into CMOS for a CD-ROM
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (26 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 27/41] ide pc: Put hard disk info into CMOS only for hard disks Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 29/41] hd-geometry: Compute BIOS CHS translation in one place Kevin Wolf
` (12 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 29/41] hd-geometry: Compute BIOS CHS translation in one place
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (27 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 28/41] qtest: Test we don't put hard disk info into CMOS for a CD-ROM Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 30/41] blockdev: Drop redundant CHS validation for if=ide Kevin Wolf
` (11 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/block-common.h | 1 +
hw/hd-geometry.c | 9 ++++++++-
hw/ide/core.c | 2 ++
hw/ide/qdev.c | 3 +++
hw/pc.c | 19 ++++---------------
5 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/hw/block-common.h b/hw/block-common.h
index ec7810d..31e12ba 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -24,5 +24,6 @@
void hd_geometry_guess(BlockDriverState *bs,
uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
int *ptrans);
+int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs);
#endif
diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c
index 74678a6..1cdb9fb 100644
--- a/hw/hd-geometry.c
+++ b/hw/hd-geometry.c
@@ -125,7 +125,7 @@ void hd_geometry_guess(BlockDriverState *bs,
if (guess_disk_lchs(bs, &cylinders, &heads, &secs) < 0) {
/* no LCHS guess: use a standard physical disk geometry */
guess_chs_for_size(bs, pcyls, pheads, psecs);
- translation = BIOS_ATA_TRANSLATION_AUTO;
+ translation = hd_bios_chs_auto_trans(*pcyls, *pheads, *psecs);
} else if (heads > 16) {
/* LCHS guess with heads > 16 means that a BIOS LBA
translation was active, so a standard physical disk
@@ -148,3 +148,10 @@ void hd_geometry_guess(BlockDriverState *bs,
}
trace_hd_geometry_guess(bs, *pcyls, *pheads, *psecs, translation);
}
+
+int hd_bios_chs_auto_trans(uint32_t cyls, uint32_t heads, uint32_t secs)
+{
+ return cyls <= 1024 && heads <= 16 && secs <= 63
+ ? BIOS_ATA_TRANSLATION_NONE
+ : BIOS_ATA_TRANSLATION_LBA;
+}
diff --git a/hw/ide/core.c b/hw/ide/core.c
index bf1ce89..1ca7cdf 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2091,6 +2091,8 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
trans = dinfo->trans;
if (!cyls && !heads && !secs) {
hd_geometry_guess(dinfo->bdrv, &cyls, &heads, &secs, &trans);
+ } else if (trans == BIOS_ATA_TRANSLATION_AUTO) {
+ trans = hd_bios_chs_auto_trans(cyls, heads, secs);
}
if (ide_init_drive(&bus->ifs[i], dinfo->bdrv,
dinfo->media_cd ? IDE_CD : IDE_HD, NULL,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 84097fd..de9db3b 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -171,6 +171,9 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
hd_geometry_guess(dev->conf.bs,
&dev->conf.cyls, &dev->conf.heads, &dev->conf.secs,
&dev->chs_trans);
+ } else if (dev->chs_trans == BIOS_ATA_TRANSLATION_AUTO) {
+ dev->chs_trans = hd_bios_chs_auto_trans(dev->conf.cyls,
+ dev->conf.heads, dev->conf.secs);
}
if (ide_init_drive(s, dev->conf.bs, kind,
diff --git a/hw/pc.c b/hw/pc.c
index 77b12b4..598267a 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -290,7 +290,7 @@ static void pc_cmos_init_late(void *opaque)
int16_t cylinders;
int8_t heads, sectors;
int val;
- int i;
+ int i, trans;
val = 0;
if (ide_get_geometry(arg->idebus[0], 0,
@@ -313,20 +313,9 @@ static void pc_cmos_init_late(void *opaque)
geometry can be different if a translation is done. */
if (ide_get_geometry(arg->idebus[i / 2], i % 2,
&cylinders, &heads, §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] 42+ messages in thread* [Qemu-devel] [PATCH 30/41] blockdev: Drop redundant CHS validation for if=ide
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (28 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 29/41] hd-geometry: Compute BIOS CHS translation in one place Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 31/41] Relax IDE CHS limits from 16383, 16, 63 to 65535, 16, 255 Kevin Wolf
` (10 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Leave it to ide_init_drive().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 06c997e..5f8677e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -330,15 +330,15 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
max_devs = if_max_devs[type];
if (cyls || heads || secs) {
- if (cyls < 1 || (type == IF_IDE && cyls > 16383)) {
+ if (cyls < 1) {
error_report("invalid physical cyls number");
return NULL;
}
- if (heads < 1 || (type == IF_IDE && heads > 16)) {
+ if (heads < 1) {
error_report("invalid physical heads number");
return NULL;
}
- if (secs < 1 || (type == IF_IDE && secs > 63)) {
+ if (secs < 1) {
error_report("invalid physical secs number");
return NULL;
}
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 31/41] Relax IDE CHS limits from 16383, 16, 63 to 65535, 16, 255
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (29 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 30/41] blockdev: Drop redundant CHS validation for if=ide Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 32/41] hw/block-common: Move BlockConf & friends from block.h Kevin Wolf
` (9 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
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>
Signed-off-by: Kevin Wolf <kwolf@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] 42+ messages in thread* [Qemu-devel] [PATCH 32/41] hw/block-common: Move BlockConf & friends from block.h
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (30 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 31/41] Relax IDE CHS limits from 16383, 16, 63 to 65535, 16, 255 Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 33/41] hw/block-common: Factor out fall back to legacy -drive serial= Kevin Wolf
` (8 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
This stuff doesn't belong to block layer, and was put there only
because a better home didn't exist then. Now it does.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.h | 45 ---------------------------------------------
hw/block-common.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
hw/ide/internal.h | 1 +
hw/scsi.h | 1 +
hw/usb.h | 1 -
hw/virtio-blk.h | 2 +-
hw/virtio.h | 1 -
7 files changed, 48 insertions(+), 48 deletions(-)
diff --git a/block.h b/block.h
index 29c5eab..c89590d 100644
--- a/block.h
+++ b/block.h
@@ -403,49 +403,4 @@ typedef enum {
#define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt)
void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event);
-
-/* Convenience for block device models */
-
-typedef struct BlockConf {
- BlockDriverState *bs;
- uint16_t physical_block_size;
- uint16_t logical_block_size;
- uint16_t min_io_size;
- 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)
-{
- unsigned int exp = 0, size;
-
- for (size = conf->physical_block_size;
- size > conf->logical_block_size;
- size >>= 1) {
- exp++;
- }
-
- return exp;
-}
-
-#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \
- DEFINE_PROP_DRIVE("drive", _state, _conf.bs), \
- DEFINE_PROP_BLOCKSIZE("logical_block_size", _state, \
- _conf.logical_block_size, 512), \
- DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \
- _conf.physical_block_size, 512), \
- DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \
- DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \
- DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1), \
- DEFINE_PROP_UINT32("discard_granularity", _state, \
- _conf.discard_granularity, 0)
-
-#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
diff --git a/hw/block-common.h b/hw/block-common.h
index 31e12ba..f0d509b 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -13,6 +13,51 @@
#include "qemu-common.h"
+/* Configuration */
+
+typedef struct BlockConf {
+ BlockDriverState *bs;
+ uint16_t physical_block_size;
+ uint16_t logical_block_size;
+ uint16_t min_io_size;
+ 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)
+{
+ unsigned int exp = 0, size;
+
+ for (size = conf->physical_block_size;
+ size > conf->logical_block_size;
+ size >>= 1) {
+ exp++;
+ }
+
+ return exp;
+}
+
+#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \
+ DEFINE_PROP_DRIVE("drive", _state, _conf.bs), \
+ DEFINE_PROP_BLOCKSIZE("logical_block_size", _state, \
+ _conf.logical_block_size, 512), \
+ DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \
+ _conf.physical_block_size, 512), \
+ DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \
+ DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \
+ DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1), \
+ DEFINE_PROP_UINT32("discard_granularity", _state, \
+ _conf.discard_granularity, 0)
+
+#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)
+
+
/* Hard disk geometry */
#define BIOS_ATA_TRANSLATION_AUTO 0
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index c3ecafc..7170bd9 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -11,6 +11,7 @@
#include "iorange.h"
#include "dma.h"
#include "sysemu.h"
+#include "hw/block-common.h"
#include "hw/scsi-defs.h"
/* debug IDE devices */
diff --git a/hw/scsi.h b/hw/scsi.h
index 76f06d4..d90e970 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -3,6 +3,7 @@
#include "qdev.h"
#include "block.h"
+#include "hw/block-common.h"
#include "sysemu.h"
#define MAX_SCSI_DEVS 255
diff --git a/hw/usb.h b/hw/usb.h
index 7ed8fb8..432ccae 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -25,7 +25,6 @@
* THE SOFTWARE.
*/
-#include "block.h"
#include "qdev.h"
#include "qemu-queue.h"
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index d785001..79ebccc 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -15,7 +15,7 @@
#define _QEMU_VIRTIO_BLK_H
#include "virtio.h"
-#include "block.h"
+#include "hw/block-common.h"
/* from Linux's linux/virtio_blk.h */
diff --git a/hw/virtio.h b/hw/virtio.h
index 85aabe5..42a7762 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -18,7 +18,6 @@
#include "net.h"
#include "qdev.h"
#include "sysemu.h"
-#include "block.h"
#include "event_notifier.h"
#ifdef CONFIG_LINUX
#include "9p.h"
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 33/41] hw/block-common: Factor out fall back to legacy -drive serial=...
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (31 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 32/41] hw/block-common: Move BlockConf & friends from block.h Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 34/41] blockdev: Don't limit DriveInfo serial to 20 characters Kevin Wolf
` (7 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/Makefile.objs | 2 +-
hw/block-common.c | 24 ++++++++++++++++++++++++
hw/block-common.h | 3 +++
hw/ide/qdev.c | 12 ++----------
hw/scsi-disk.c | 8 +-------
hw/usb/dev-storage.c | 10 ++--------
hw/virtio-blk.c | 8 +-------
7 files changed, 34 insertions(+), 33 deletions(-)
create mode 100644 hw/block-common.c
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index c3bdedc..8327e55 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -138,7 +138,7 @@ common-obj-$(CONFIG_MAX111X) += max111x.o
common-obj-$(CONFIG_DS1338) += ds1338.o
common-obj-y += i2c.o smbus.o smbus_eeprom.o
common-obj-y += eeprom93xx.o
-common-obj-y += scsi-disk.o cdrom.o hd-geometry.o
+common-obj-y += scsi-disk.o cdrom.o hd-geometry.o block-common.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.c b/hw/block-common.c
new file mode 100644
index 0000000..036334b
--- /dev/null
+++ b/hw/block-common.c
@@ -0,0 +1,24 @@
+/*
+ * Common code for block device models
+ *
+ * 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.
+ */
+
+#include "blockdev.h"
+#include "hw/block-common.h"
+
+void blkconf_serial(BlockConf *conf, char **serial)
+{
+ DriveInfo *dinfo;
+
+ if (!*serial) {
+ /* try to fall back to value set with legacy -drive serial=... */
+ dinfo = drive_get_by_blockdev(conf->bs);
+ if (*dinfo->serial) {
+ *serial = g_strdup(dinfo->serial);
+ }
+ }
+}
diff --git a/hw/block-common.h b/hw/block-common.h
index f0d509b..52bddda 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -57,6 +57,9 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
+/* Configuration helpers */
+
+void blkconf_serial(BlockConf *conf, char **serial);
/* Hard disk geometry */
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index de9db3b..7fe803c 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -142,7 +142,6 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
{
IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
IDEState *s = bus->ifs + dev->unit;
- const char *serial;
DriveInfo *dinfo;
if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) {
@@ -150,14 +149,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
return -1;
}
- serial = dev->serial;
- if (!serial) {
- /* try to fall back to value set with legacy -drive serial=... */
- dinfo = drive_get_by_blockdev(dev->conf.bs);
- if (*dinfo->serial) {
- serial = dinfo->serial;
- }
- }
+ blkconf_serial(&dev->conf, &dev->serial);
if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
/* try to fall back to value set with legacy -drive cyls=... */
@@ -177,7 +169,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
}
if (ide_init_drive(s, dev->conf.bs, kind,
- dev->version, serial, dev->model, dev->wwn,
+ dev->version, dev->serial, dev->model, dev->wwn,
dev->conf.cyls, dev->conf.heads, dev->conf.secs,
dev->chs_trans) < 0) {
return -1;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 0a182f9..39a07d7 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1777,13 +1777,7 @@ static int scsi_initfn(SCSIDevice *dev)
}
}
- if (!s->serial) {
- /* try to fall back to value set with legacy -drive serial=... */
- dinfo = drive_get_by_blockdev(s->qdev.conf.bs);
- if (*dinfo->serial) {
- s->serial = g_strdup(dinfo->serial);
- }
- }
+ blkconf_serial(&s->qdev.conf, &s->serial);
if (!s->version) {
s->version = g_strdup(qemu_get_version());
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 251e7de..7fa8b83 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -532,13 +532,14 @@ static int usb_msd_initfn(USBDevice *dev)
{
MSDState *s = DO_UPCAST(MSDState, dev, dev);
BlockDriverState *bs = s->conf.bs;
- DriveInfo *dinfo;
if (!bs) {
error_report("drive property not set");
return -1;
}
+ blkconf_serial(&s->conf, &s->serial);
+
/*
* Hack alert: this pretends to be a block device, but it's really
* a SCSI bus that can serve only a single device, which it
@@ -551,13 +552,6 @@ static int usb_msd_initfn(USBDevice *dev)
bdrv_detach_dev(bs, &s->dev.qdev);
s->conf.bs = NULL;
- if (!s->serial) {
- /* try to fall back to value set with legacy -drive serial=... */
- dinfo = drive_get_by_blockdev(bs);
- if (*dinfo->serial) {
- s->serial = strdup(dinfo->serial);
- }
- }
if (s->serial) {
usb_desc_set_string(dev, STR_SERIALNUMBER, s->serial);
} else {
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 3885904..ba087bc 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -600,13 +600,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
return NULL;
}
- if (!blk->serial) {
- /* try to fall back to value set with legacy -drive serial=... */
- dinfo = drive_get_by_blockdev(blk->conf.bs);
- if (*dinfo->serial) {
- blk->serial = strdup(dinfo->serial);
- }
- }
+ blkconf_serial(&blk->conf, &blk->serial);
s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
sizeof(struct virtio_blk_config),
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 34/41] blockdev: Don't limit DriveInfo serial to 20 characters
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (32 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 33/41] hw/block-common: Factor out fall back to legacy -drive serial= Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 35/41] hw/block-common: Factor out fall back to legacy -drive cyls= Kevin Wolf
` (6 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
All current users (IDE, SCSI and virtio-blk) happen to share this 20
characters limit. Still, it should be left to device models. They
already enforce their limits. They have to, as the DriveInfo limit
only affects legacy -drive serial=..., not the qdev properties.
usb-storage, which doesn't limit serial number length, also uses
DriveInfo for -usbdevice. But that doesn't provide access to
DriveInfo serial.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 4 +---
blockdev.h | 4 +---
hw/block-common.c | 2 +-
hw/ide/core.c | 6 +++---
4 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 5f8677e..3d75015 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -536,9 +536,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
dinfo->trans = translation;
dinfo->opts = opts;
dinfo->refcount = 1;
- if (serial) {
- pstrcpy(dinfo->serial, sizeof(dinfo->serial), serial);
- }
+ dinfo->serial = serial;
QTAILQ_INSERT_TAIL(&drives, dinfo, next);
bdrv_set_on_error(dinfo->bdrv, on_read_error, on_write_error);
diff --git a/blockdev.h b/blockdev.h
index 9c29948..5f27b64 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -17,8 +17,6 @@
void blockdev_mark_auto_del(BlockDriverState *bs);
void blockdev_auto_del(BlockDriverState *bs);
-#define BLOCK_SERIAL_STRLEN 20
-
typedef enum {
IF_DEFAULT = -1, /* for use with drive_add() only */
IF_NONE,
@@ -37,7 +35,7 @@ struct DriveInfo {
int media_cd;
int cyls, heads, secs, trans;
QemuOpts *opts;
- char serial[BLOCK_SERIAL_STRLEN + 1];
+ const char *serial;
QTAILQ_ENTRY(DriveInfo) next;
int refcount;
};
diff --git a/hw/block-common.c b/hw/block-common.c
index 036334b..0a0542a 100644
--- a/hw/block-common.c
+++ b/hw/block-common.c
@@ -17,7 +17,7 @@ void blkconf_serial(BlockConf *conf, char **serial)
if (!*serial) {
/* try to fall back to value set with legacy -drive serial=... */
dinfo = drive_get_by_blockdev(conf->bs);
- if (*dinfo->serial) {
+ if (dinfo->serial) {
*serial = g_strdup(dinfo->serial);
}
}
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 58a454f..5378fc3 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2095,9 +2095,9 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
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,
- *dinfo->serial ? dinfo->serial : NULL,
- NULL, 0, cyls, heads, secs, trans) < 0) {
+ dinfo->media_cd ? IDE_CD : IDE_HD,
+ NULL, dinfo->serial, NULL, 0,
+ cyls, heads, secs, trans) < 0) {
error_report("Can't set up IDE drive %s", dinfo->id);
exit(1);
}
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 35/41] hw/block-common: Factor out fall back to legacy -drive cyls=...
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (33 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 34/41] blockdev: Don't limit DriveInfo serial to 20 characters Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 36/41] qemu-io: Fix memory leaks Kevin Wolf
` (5 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/block-common.c | 40 ++++++++++++++++++++++++++++++++++++++++
hw/block-common.h | 2 ++
hw/ide/core.c | 24 ++++++++++++------------
hw/ide/qdev.c | 19 ++-----------------
hw/scsi-disk.c | 31 +++----------------------------
hw/virtio-blk.c | 31 +++----------------------------
6 files changed, 62 insertions(+), 85 deletions(-)
diff --git a/hw/block-common.c b/hw/block-common.c
index 0a0542a..f0196d7 100644
--- a/hw/block-common.c
+++ b/hw/block-common.c
@@ -9,6 +9,7 @@
#include "blockdev.h"
#include "hw/block-common.h"
+#include "qemu-error.h"
void blkconf_serial(BlockConf *conf, char **serial)
{
@@ -22,3 +23,42 @@ void blkconf_serial(BlockConf *conf, char **serial)
}
}
}
+
+int blkconf_geometry(BlockConf *conf, int *ptrans,
+ unsigned cyls_max, unsigned heads_max, unsigned secs_max)
+{
+ DriveInfo *dinfo;
+
+ if (!conf->cyls && !conf->heads && !conf->secs) {
+ /* try to fall back to value set with legacy -drive cyls=... */
+ dinfo = drive_get_by_blockdev(conf->bs);
+ conf->cyls = dinfo->cyls;
+ conf->heads = dinfo->heads;
+ conf->secs = dinfo->secs;
+ if (ptrans) {
+ *ptrans = dinfo->trans;
+ }
+ }
+ if (!conf->cyls && !conf->heads && !conf->secs) {
+ hd_geometry_guess(conf->bs,
+ &conf->cyls, &conf->heads, &conf->secs,
+ ptrans);
+ } else if (ptrans && *ptrans == BIOS_ATA_TRANSLATION_AUTO) {
+ *ptrans = hd_bios_chs_auto_trans(conf->cyls, conf->heads, conf->secs);
+ }
+ if (conf->cyls || conf->heads || conf->secs) {
+ if (conf->cyls < 1 || conf->cyls > cyls_max) {
+ error_report("cyls must be between 1 and %u", cyls_max);
+ return -1;
+ }
+ if (conf->heads < 1 || conf->heads > heads_max) {
+ error_report("heads must be between 1 and %u", heads_max);
+ return -1;
+ }
+ if (conf->secs < 1 || conf->secs > secs_max) {
+ error_report("secs must be between 1 and %u", secs_max);
+ return -1;
+ }
+ }
+ return 0;
+}
diff --git a/hw/block-common.h b/hw/block-common.h
index 52bddda..bb808f7 100644
--- a/hw/block-common.h
+++ b/hw/block-common.h
@@ -60,6 +60,8 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
/* Configuration helpers */
void blkconf_serial(BlockConf *conf, char **serial);
+int blkconf_geometry(BlockConf *conf, int *trans,
+ unsigned cyls_max, unsigned heads_max, unsigned secs_max);
/* Hard disk geometry */
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 5378fc3..d65ef3d 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1935,18 +1935,6 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
s->drive_kind = kind;
bdrv_get_geometry(bs, &nb_sectors);
- if (cylinders < 1 || cylinders > 65535) {
- error_report("cyls must be between 1 and 65535");
- return -1;
- }
- if (heads < 1 || heads > 16) {
- error_report("heads must be between 1 and 16");
- return -1;
- }
- if (secs < 1 || secs > 255) {
- error_report("secs must be between 1 and 255");
- return -1;
- }
s->cylinders = cylinders;
s->heads = heads;
s->sectors = secs;
@@ -2094,6 +2082,18 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
} else if (trans == BIOS_ATA_TRANSLATION_AUTO) {
trans = hd_bios_chs_auto_trans(cyls, heads, secs);
}
+ if (cyls < 1 || cyls > 65535) {
+ error_report("cyls must be between 1 and 65535");
+ exit(1);
+ }
+ if (heads < 1 || heads > 16) {
+ error_report("heads must be between 1 and 16");
+ exit(1);
+ }
+ if (secs < 1 || secs > 255) {
+ error_report("secs must be between 1 and 255");
+ exit(1);
+ }
if (ide_init_drive(&bus->ifs[i], dinfo->bdrv,
dinfo->media_cd ? IDE_CD : IDE_HD,
NULL, dinfo->serial, NULL, 0,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 7fe803c..22e58df 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -142,7 +142,6 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
{
IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
IDEState *s = bus->ifs + dev->unit;
- DriveInfo *dinfo;
if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) {
error_report("discard_granularity must be 512 for ide");
@@ -150,22 +149,8 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
}
blkconf_serial(&dev->conf, &dev->serial);
-
- if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
- /* try to fall back to value set with legacy -drive cyls=... */
- dinfo = drive_get_by_blockdev(dev->conf.bs);
- dev->conf.cyls = dinfo->cyls;
- dev->conf.heads = dinfo->heads;
- dev->conf.secs = dinfo->secs;
- dev->chs_trans = dinfo->trans;
- }
- if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
- hd_geometry_guess(dev->conf.bs,
- &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs,
- &dev->chs_trans);
- } else if (dev->chs_trans == BIOS_ATA_TRANSLATION_AUTO) {
- dev->chs_trans = hd_bios_chs_auto_trans(dev->conf.cyls,
- dev->conf.heads, dev->conf.secs);
+ if (blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255) < 0) {
+ return -1;
}
if (ide_init_drive(s, dev->conf.bs, kind,
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 39a07d7..525816c 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1737,7 +1737,6 @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev)
static int scsi_initfn(SCSIDevice *dev)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
- DriveInfo *dinfo;
if (!s->qdev.conf.bs) {
error_report("drive property not set");
@@ -1750,34 +1749,10 @@ static int scsi_initfn(SCSIDevice *dev)
return -1;
}
- if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
- /* try to fall back to value set with legacy -drive cyls=... */
- dinfo = drive_get_by_blockdev(s->qdev.conf.bs);
- dev->conf.cyls = dinfo->cyls;
- dev->conf.heads = dinfo->heads;
- dev->conf.secs = dinfo->secs;
- }
- if (!dev->conf.cyls && !dev->conf.heads && !dev->conf.secs) {
- hd_geometry_guess(s->qdev.conf.bs,
- &dev->conf.cyls, &dev->conf.heads, &dev->conf.secs,
- NULL);
- }
- if (dev->conf.cyls || dev->conf.heads || dev->conf.secs) {
- if (dev->conf.cyls < 1 || dev->conf.cyls > 65535) {
- error_report("cyls must be between 1 and 65535");
- return -1;
- }
- if (dev->conf.heads < 1 || dev->conf.heads > 255) {
- error_report("heads must be between 1 and 255");
- return -1;
- }
- if (dev->conf.secs < 1 || dev->conf.secs > 255) {
- error_report("secs must be between 1 and 255");
- return -1;
- }
- }
-
blkconf_serial(&s->qdev.conf, &s->serial);
+ if (blkconf_geometry(&dev->conf, NULL, 65535, 255, 255) < 0) {
+ return -1;
+ }
if (!s->version) {
s->version = g_strdup(qemu_get_version());
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index ba087bc..f21757e 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -589,7 +589,6 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
{
VirtIOBlock *s;
static int virtio_blk_id;
- DriveInfo *dinfo;
if (!blk->conf.bs) {
error_report("drive property not set");
@@ -601,6 +600,9 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
}
blkconf_serial(&blk->conf, &blk->serial);
+ if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {
+ return NULL;
+ }
s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
sizeof(struct virtio_blk_config),
@@ -615,33 +617,6 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
s->rq = NULL;
s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
- if (!blk->conf.cyls && !blk->conf.heads && !blk->conf.secs) {
- /* try to fall back to value set with legacy -drive cyls=... */
- dinfo = drive_get_by_blockdev(blk->conf.bs);
- blk->conf.cyls = dinfo->cyls;
- blk->conf.heads = dinfo->heads;
- blk->conf.secs = dinfo->secs;
- }
- if (!blk->conf.cyls && !blk->conf.heads && !blk->conf.secs) {
- hd_geometry_guess(s->bs,
- &blk->conf.cyls, &blk->conf.heads, &blk->conf.secs,
- NULL);
- }
- if (blk->conf.cyls || blk->conf.heads || blk->conf.secs) {
- if (blk->conf.cyls < 1 || blk->conf.cyls > 65535) {
- error_report("cyls must be between 1 and 65535");
- return NULL;
- }
- if (blk->conf.heads < 1 || blk->conf.heads > 255) {
- error_report("heads must be between 1 and 255");
- return NULL;
- }
- if (blk->conf.secs < 1 || blk->conf.secs > 255) {
- error_report("secs must be between 1 and 255");
- return NULL;
- }
- }
-
s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 36/41] qemu-io: Fix memory leaks
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (34 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 35/41] hw/block-common: Factor out fall back to legacy -drive cyls= Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 37/41] coroutine-ucontext: Help valgrind understand coroutines Kevin Wolf
` (4 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
Almost all callers of create_iovec() forgot to destroy the qiov when the
request has completed.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
qemu-io.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index 5882067..8f3b94b 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -670,6 +670,7 @@ static int readv_f(int argc, char **argv)
print_report("read", &t2, offset, qiov.size, total, cnt, Cflag);
out:
+ qemu_iovec_destroy(&qiov);
qemu_io_free(buf);
return 0;
}
@@ -928,6 +929,7 @@ static int writev_f(int argc, char **argv)
t2 = tsub(t2, t1);
print_report("wrote", &t2, offset, qiov.size, total, cnt, Cflag);
out:
+ qemu_iovec_destroy(&qiov);
qemu_io_free(buf);
return 0;
}
@@ -1126,6 +1128,7 @@ static void aio_write_done(void *opaque, int ret)
ctx->qiov.size, 1, ctx->Cflag);
out:
qemu_io_free(ctx->buf);
+ qemu_iovec_destroy(&ctx->qiov);
g_free(ctx);
}
@@ -1166,6 +1169,7 @@ static void aio_read_done(void *opaque, int ret)
ctx->qiov.size, 1, ctx->Cflag);
out:
qemu_io_free(ctx->buf);
+ qemu_iovec_destroy(&ctx->qiov);
g_free(ctx);
}
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 37/41] coroutine-ucontext: Help valgrind understand coroutines
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (35 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 36/41] qemu-io: Fix memory leaks Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 38/41] qemu-iotests: Valgrind support Kevin Wolf
` (3 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
valgrind tends to get confused and report false positives when you
switch stacks and don't tell it about it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
configure | 20 ++++++++++++++++++++
coroutine-ucontext.c | 28 ++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/configure b/configure
index 0a3896e..cef0a71 100755
--- a/configure
+++ b/configure
@@ -2871,6 +2871,22 @@ if compile_prog "" "" ; then
fi
########################################
+# check if we have valgrind/valgrind.h
+
+valgrind_h=no
+cat > $TMPC << EOF
+#include <valgrind/valgrind.h>
+#pragma GCC diagnostic ignored "-Wunused-but-set-variable"
+int main(void) {
+ VALGRIND_STACK_DEREGISTER(0);
+ return 0;
+}
+EOF
+if compile_prog "" "" ; then
+ valgrind_h=yes
+fi
+
+########################################
# check if environ is declared
has_environ=no
@@ -3379,6 +3395,10 @@ if test "$linux_magic_h" = "yes" ; then
echo "CONFIG_LINUX_MAGIC_H=y" >> $config_host_mak
fi
+if test "$valgrind_h" = "yes" ; then
+ echo "CONFIG_VALGRIND_H=y" >> $config_host_mak
+fi
+
if test "$has_environ" = "yes" ; then
echo "CONFIG_HAS_ENVIRON=y" >> $config_host_mak
fi
diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
index 5f43083..e3c450b 100644
--- a/coroutine-ucontext.c
+++ b/coroutine-ucontext.c
@@ -30,6 +30,10 @@
#include "qemu-common.h"
#include "qemu-coroutine-int.h"
+#ifdef CONFIG_VALGRIND_H
+#include <valgrind/valgrind.h>
+#endif
+
enum {
/* Maximum free pool size prevents holding too many freed coroutines */
POOL_MAX_SIZE = 64,
@@ -43,6 +47,11 @@ typedef struct {
Coroutine base;
void *stack;
jmp_buf env;
+
+#ifdef CONFIG_VALGRIND_H
+ unsigned int valgrind_stack_id;
+#endif
+
} CoroutineUContext;
/**
@@ -159,6 +168,11 @@ static Coroutine *coroutine_new(void)
uc.uc_stack.ss_size = stack_size;
uc.uc_stack.ss_flags = 0;
+#ifdef CONFIG_VALGRIND_H
+ co->valgrind_stack_id =
+ VALGRIND_STACK_REGISTER(co->stack, co->stack + stack_size);
+#endif
+
arg.p = co;
makecontext(&uc, (void (*)(void))coroutine_trampoline,
@@ -185,6 +199,16 @@ Coroutine *qemu_coroutine_new(void)
return co;
}
+#ifdef CONFIG_VALGRIND_H
+/* Work around an unused variable in the valgrind.h macro... */
+#pragma GCC diagnostic ignored "-Wunused-but-set-variable"
+static inline void valgrind_stack_deregister(CoroutineUContext *co)
+{
+ VALGRIND_STACK_DEREGISTER(co->valgrind_stack_id);
+}
+#pragma GCC diagnostic error "-Wunused-but-set-variable"
+#endif
+
void qemu_coroutine_delete(Coroutine *co_)
{
CoroutineUContext *co = DO_UPCAST(CoroutineUContext, base, co_);
@@ -196,6 +220,10 @@ void qemu_coroutine_delete(Coroutine *co_)
return;
}
+#ifdef CONFIG_VALGRIND_H
+ valgrind_stack_deregister(co);
+#endif
+
g_free(co->stack);
g_free(co);
}
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 38/41] qemu-iotests: Valgrind support
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (36 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 37/41] coroutine-ucontext: Help valgrind understand coroutines Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 39/41] fdc: fix relative seek Kevin Wolf
` (2 subsequent siblings)
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
check -valgrind wraps all qemu-io calls with valgrind. This makes it a
bit easier to debug problems that occur somewhere deep in a test case.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
tests/qemu-iotests/common | 11 +++++++++++
tests/qemu-iotests/common.rc | 10 ++++++++++
2 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index eeb70cb..1f6fdf5 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -41,6 +41,7 @@ sortme=false
expunge=true
have_test_arg=false
randomize=false
+valgrind=false
rm -f $tmp.list $tmp.tmp $tmp.sed
export IMGFMT=raw
@@ -212,6 +213,11 @@ testlist options
xpand=false
;;
+ -valgrind)
+ valgrind=true
+ xpand=false
+ ;;
+
-g) # -g group ... pick from group file
group=true
xpand=false
@@ -345,3 +351,8 @@ fi
[ "$QEMU" = "" ] && _fatal "qemu not found"
[ "$QEMU_IMG" = "" ] && _fatal "qemu-img not found"
[ "$QEMU_IO" = "" ] && _fatal "qemu-img not found"
+
+if $valgrind; then
+ export REAL_QEMU_IO="$QEMU_IO_PROG"
+ export QEMU_IO_PROG=valgrind_qemu_io
+fi
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index e535874..5e3a524 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -53,6 +53,16 @@ else
TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
fi
+function valgrind_qemu_io()
+{
+ valgrind --log-file=/tmp/$$.valgrind --error-exitcode=99 $REAL_QEMU_IO "$@"
+ if [ $? != 0 ]; then
+ cat /tmp/$$.valgrind
+ fi
+ rm -f /tmp/$$.valgrind
+}
+
+
_optstr_add()
{
if [ -n "$1" ]; then
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 39/41] fdc: fix relative seek
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (37 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 38/41] qemu-iotests: Valgrind support Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 40/41] fdc-test: introduce test_relative_seek Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 41/41] fdc-test: Clean up a bit Kevin Wolf
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/fdc.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index 41191c7..08830c1 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1802,7 +1802,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct
}
}
-static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)
+static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction)
{
FDrive *cur_drv;
@@ -1812,14 +1812,15 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)
fd_seek(cur_drv, cur_drv->head, cur_drv->max_track - 1,
cur_drv->sect, 1);
} else {
- fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
+ fd_seek(cur_drv, cur_drv->head,
+ cur_drv->track + fdctrl->fifo[2], cur_drv->sect, 1);
}
fdctrl_reset_fifo(fdctrl);
/* Raise Interrupt */
fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
}
-static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction)
+static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)
{
FDrive *cur_drv;
@@ -1828,7 +1829,8 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction)
if (fdctrl->fifo[2] > cur_drv->track) {
fd_seek(cur_drv, cur_drv->head, 0, cur_drv->sect, 1);
} else {
- fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
+ fd_seek(cur_drv, cur_drv->head,
+ cur_drv->track - fdctrl->fifo[2], cur_drv->sect, 1);
}
fdctrl_reset_fifo(fdctrl);
/* Raise Interrupt */
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 40/41] fdc-test: introduce test_relative_seek
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (38 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 39/41] fdc: fix relative seek Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
2012-07-17 16:00 ` [Qemu-devel] [PATCH 41/41] fdc-test: Clean up a bit Kevin Wolf
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/fdc-test.c | 46 +++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 585fb0e..10d11a4 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -47,9 +47,11 @@ enum {
};
enum {
- CMD_SENSE_INT = 0x08,
- CMD_SEEK = 0x0f,
- CMD_READ = 0xe6,
+ CMD_SENSE_INT = 0x08,
+ CMD_SEEK = 0x0f,
+ CMD_READ = 0xe6,
+ CMD_RELATIVE_SEEK_OUT = 0x8f,
+ CMD_RELATIVE_SEEK_IN = 0xcf,
};
enum {
@@ -91,13 +93,17 @@ static uint8_t floppy_recv(void)
return inb(FLOPPY_BASE + reg_fifo);
}
-static void ack_irq(void)
+static uint8_t ack_irq(void)
{
+ uint8_t ret;
+
g_assert(get_irq(FLOPPY_IRQ));
floppy_send(CMD_SENSE_INT);
floppy_recv();
- floppy_recv();
+ ret = floppy_recv();
g_assert(!get_irq(FLOPPY_IRQ));
+
+ return ret;
}
static uint8_t send_read_command(void)
@@ -281,6 +287,35 @@ static void test_sense_interrupt(void)
floppy_recv();
}
+static void test_relative_seek(void)
+{
+ uint8_t drive = 0;
+ uint8_t head = 0;
+ uint8_t cyl = 1;
+ uint8_t ret;
+
+ /* Send seek to track 0 */
+ send_step_pulse(0);
+
+ /* Send relative seek to increase track by 1 */
+ floppy_send(CMD_RELATIVE_SEEK_IN);
+ floppy_send(head << 2 | drive);
+ g_assert(!get_irq(FLOPPY_IRQ));
+ floppy_send(cyl);
+
+ ret = ack_irq();
+ g_assert(ret == 1);
+
+ /* Send relative seek to decrease track by 1 */
+ floppy_send(CMD_RELATIVE_SEEK_OUT);
+ floppy_send(head << 2 | drive);
+ g_assert(!get_irq(FLOPPY_IRQ));
+ floppy_send(cyl);
+
+ ret = ack_irq();
+ g_assert(ret == 0);
+}
+
/* success if no crash or abort */
static void fuzz_registers(void)
{
@@ -329,6 +364,7 @@ int main(int argc, char **argv)
qtest_add_func("/fdc/read_without_media", test_read_without_media);
qtest_add_func("/fdc/media_change", test_media_change);
qtest_add_func("/fdc/sense_interrupt", test_sense_interrupt);
+ qtest_add_func("/fdc/relative_seek", test_relative_seek);
qtest_add_func("/fdc/fuzz-registers", fuzz_registers);
ret = g_test_run();
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread* [Qemu-devel] [PATCH 41/41] fdc-test: Clean up a bit
2012-07-17 15:59 [Qemu-devel] [PULL 00/41] Block patches Kevin Wolf
` (39 preceding siblings ...)
2012-07-17 16:00 ` [Qemu-devel] [PATCH 40/41] fdc-test: introduce test_relative_seek Kevin Wolf
@ 2012-07-17 16:00 ` Kevin Wolf
40 siblings, 0 replies; 42+ messages in thread
From: Kevin Wolf @ 2012-07-17 16:00 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
Readability of the test code has suffered as the test case evolved. This
should improve it a bit again.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/fdc-test.c | 36 ++++++++++++++++++++----------------
1 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 10d11a4..fa74411 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -93,17 +93,21 @@ static uint8_t floppy_recv(void)
return inb(FLOPPY_BASE + reg_fifo);
}
-static uint8_t ack_irq(void)
+/* pcn: Present Cylinder Number */
+static void ack_irq(uint8_t *pcn)
{
uint8_t ret;
g_assert(get_irq(FLOPPY_IRQ));
floppy_send(CMD_SENSE_INT);
floppy_recv();
+
ret = floppy_recv();
- g_assert(!get_irq(FLOPPY_IRQ));
+ if (pcn != NULL) {
+ *pcn = ret;
+ }
- return ret;
+ g_assert(!get_irq(FLOPPY_IRQ));
}
static uint8_t send_read_command(void)
@@ -162,7 +166,7 @@ static uint8_t send_read_command(void)
return ret;
}
-static void send_step_pulse(int cyl)
+static void send_seek(int cyl)
{
int drive = 0;
int head = 0;
@@ -171,7 +175,7 @@ static void send_step_pulse(int cyl)
floppy_send(head << 2 | drive);
g_assert(!get_irq(FLOPPY_IRQ));
floppy_send(cyl);
- ack_irq();
+ ack_irq(NULL);
}
static uint8_t cmos_read(uint8_t reg)
@@ -198,7 +202,7 @@ static void test_no_media_on_start(void)
assert_bit_set(dir, DSKCHG);
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_set(dir, DSKCHG);
- send_step_pulse(1);
+ send_seek(1);
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_set(dir, DSKCHG);
dir = inb(FLOPPY_BASE + reg_dir);
@@ -229,14 +233,14 @@ static void test_media_change(void)
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_set(dir, DSKCHG);
- send_step_pulse(0);
+ send_seek(0);
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_set(dir, DSKCHG);
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_set(dir, DSKCHG);
/* Step to next track should clear DSKCHG bit. */
- send_step_pulse(1);
+ send_seek(1);
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_clear(dir, DSKCHG);
dir = inb(FLOPPY_BASE + reg_dir);
@@ -252,13 +256,13 @@ static void test_media_change(void)
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_set(dir, DSKCHG);
- send_step_pulse(0);
+ send_seek(0);
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_set(dir, DSKCHG);
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_set(dir, DSKCHG);
- send_step_pulse(1);
+ send_seek(1);
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_set(dir, DSKCHG);
dir = inb(FLOPPY_BASE + reg_dir);
@@ -292,10 +296,10 @@ static void test_relative_seek(void)
uint8_t drive = 0;
uint8_t head = 0;
uint8_t cyl = 1;
- uint8_t ret;
+ uint8_t pcn;
/* Send seek to track 0 */
- send_step_pulse(0);
+ send_seek(0);
/* Send relative seek to increase track by 1 */
floppy_send(CMD_RELATIVE_SEEK_IN);
@@ -303,8 +307,8 @@ static void test_relative_seek(void)
g_assert(!get_irq(FLOPPY_IRQ));
floppy_send(cyl);
- ret = ack_irq();
- g_assert(ret == 1);
+ ack_irq(&pcn);
+ g_assert(pcn == 1);
/* Send relative seek to decrease track by 1 */
floppy_send(CMD_RELATIVE_SEEK_OUT);
@@ -312,8 +316,8 @@ static void test_relative_seek(void)
g_assert(!get_irq(FLOPPY_IRQ));
floppy_send(cyl);
- ret = ack_irq();
- g_assert(ret == 0);
+ ack_irq(&pcn);
+ g_assert(pcn == 0);
}
/* success if no crash or abort */
--
1.7.6.5
^ permalink raw reply related [flat|nested] 42+ messages in thread