From b6c147622d31272f9728da9ec16d146bf8c45a74 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Mon, 21 May 2012 13:06:54 +0200 Subject: [PATCH 01/10] qcow2: don't leak buffer for unexpected qcow_version in header Signed-off-by: Jim Meyering Signed-off-by: Kevin Wolf --- block/qcow2.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index 655799c6a0..c2e49cded3 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -919,7 +919,8 @@ int qcow2_update_header(BlockDriverState *bs) ret = sizeof(*header); break; default: - return -EINVAL; + ret = -EINVAL; + goto fail; } buf += ret; From 9fda6ab1d9f33c04f35438bce101427dd557fef6 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Mon, 21 May 2012 14:58:05 +0100 Subject: [PATCH 02/10] qemu-img: Explain how rebase operation can be used to perform a 'diff' operation. Signed-off-by: Richard W.M. Jones Signed-off-by: Kevin Wolf --- qemu-img.texi | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/qemu-img.texi b/qemu-img.texi index b2ca3a542c..6fc3c28e0d 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -159,6 +159,24 @@ It can be used without an accessible old backing file, i.e. you can use it to fix an image whose backing file has already been moved/renamed. @end table +You can use @code{rebase} to perform a ``diff'' operation on two +disk images. This can be useful when you have copied or cloned +a guest, and you want to get back to a thin image on top of a +template or base image. + +Say that @code{base.img} has been cloned as @code{modified.img} by +copying it, and that the @code{modified.img} guest has run so there +are now some changes compared to @code{base.img}. To construct a thin +image called @code{diff.qcow2} that contains just the differences, do: + +@example +qemu-img create -f qcow2 -b modified.img diff.qcow2 +qemu-img rebase -b base.img diff.qcow2 +@end example + +At this point, @code{modified.img} can be discarded, since +@code{base.img + diff.qcow2} contains the same information. + @item resize @var{filename} [+ | -]@var{size} Change the disk image as if it had been created with @var{size}. From 622b6057beb3d8ce8035aaedab2137108bd6bfe4 Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Thu, 17 May 2012 03:15:31 +0900 Subject: [PATCH 03/10] sheepdog: mark image as snapshot when tag is specified When a snapshot tag is specified in the filename, the opened image is a snapshot. Signed-off-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index e01d371680..776a1cc969 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1103,7 +1103,7 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) } } - if (snapid) { + if (snapid || tag[0] != '\0') { dprintf("%" PRIx32 " snapshot inode was open.\n", vid); s->is_snapshot = 1; } From cb595887cc4ddd7c732b711164756eb0b1b31077 Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Thu, 17 May 2012 03:15:33 +0900 Subject: [PATCH 04/10] sheepdog: return -errno on error On error, BlockDriver APIs should return -errno instead of -1. Signed-off-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 78 ++++++++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 776a1cc969..991133e08b 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -468,7 +468,7 @@ static int connect_to_sdog(const char *addr, const char *port) if (ret) { error_report("unable to get address info %s, %s", addr, strerror(errno)); - return -1; + return -errno; } for (res = res0; res; res = res->ai_next) { @@ -495,7 +495,7 @@ static int connect_to_sdog(const char *addr, const char *port) dprintf("connected to %s:%s\n", addr, port); goto success; } - fd = -1; + fd = -errno; error_report("failed connect to %s:%s", addr, port); success: freeaddrinfo(res0); @@ -510,12 +510,13 @@ static int send_req(int sockfd, SheepdogReq *hdr, void *data, ret = qemu_send_full(sockfd, hdr, sizeof(*hdr), 0); if (ret < sizeof(*hdr)) { error_report("failed to send a req, %s", strerror(errno)); - return ret; + 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; @@ -553,6 +554,7 @@ static int do_req(int sockfd, SheepdogReq *hdr, void *data, 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; } @@ -564,6 +566,7 @@ static int do_req(int sockfd, SheepdogReq *hdr, void *data, 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; } } @@ -587,6 +590,7 @@ static int do_co_req(int sockfd, SheepdogReq *hdr, void *data, ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr)); if (ret < sizeof(*hdr)) { error_report("failed to get a rsp, %s", strerror(errno)); + ret = -errno; goto out; } @@ -598,6 +602,7 @@ static int do_co_req(int sockfd, SheepdogReq *hdr, void *data, ret = qemu_co_recv(sockfd, data, *rlen); if (ret < *rlen) { error_report("failed to get the data, %s", strerror(errno)); + ret = -errno; goto out; } } @@ -787,7 +792,7 @@ static int get_sheep_fd(BDRVSheepdogState *s) fd = connect_to_sdog(s->addr, s->port); if (fd < 0) { error_report("%s", strerror(errno)); - return -1; + return fd; } socket_set_nonblock(fd); @@ -796,7 +801,7 @@ static int get_sheep_fd(BDRVSheepdogState *s) if (ret) { error_report("%s", strerror(errno)); closesocket(fd); - return -1; + return -errno; } qemu_aio_set_fd_handler(fd, co_read_response, NULL, aio_flush_request, s); @@ -883,7 +888,7 @@ static int find_vdi_name(BDRVSheepdogState *s, char *filename, uint32_t snapid, fd = connect_to_sdog(s->addr, s->port); if (fd < 0) { - return -1; + return fd; } memset(buf, 0, sizeof(buf)); @@ -904,14 +909,17 @@ static int find_vdi_name(BDRVSheepdogState *s, char *filename, uint32_t snapid, ret = do_req(fd, (SheepdogReq *)&hdr, buf, &wlen, &rlen); if (ret) { - ret = -1; goto out; } if (rsp->result != SD_RES_SUCCESS) { error_report("cannot get vdi info, %s, %s %d %s", sd_strerror(rsp->result), filename, snapid, tag); - ret = -1; + if (rsp->result == SD_RES_NO_VDI) { + ret = -ENOENT; + } else { + ret = -EIO; + } goto out; } *vid = rsp->vdi_id; @@ -980,7 +988,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, if (ret < 0) { qemu_co_mutex_unlock(&s->lock); error_report("failed to send a req, %s", strerror(errno)); - return -EIO; + return -errno; } if (wlen) { @@ -988,7 +996,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, if (ret < 0) { qemu_co_mutex_unlock(&s->lock); error_report("failed to send a data, %s", strerror(errno)); - return -EIO; + return -errno; } } @@ -1038,7 +1046,7 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, ret = do_req(fd, (SheepdogReq *)&hdr, buf, &wlen, &rlen); if (ret) { error_report("failed to send a request to the sheep"); - return -1; + return ret; } switch (rsp->result) { @@ -1046,7 +1054,7 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, return 0; default: error_report("%s", sd_strerror(rsp->result)); - return -1; + return -EIO; } } @@ -1082,10 +1090,12 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) memset(vdi, 0, sizeof(vdi)); memset(tag, 0, sizeof(tag)); if (parse_vdiname(s, filename, vdi, &snapid, tag) < 0) { + ret = -EINVAL; goto out; } s->fd = get_sheep_fd(s); if (s->fd < 0) { + ret = s->fd; goto out; } @@ -1099,6 +1109,7 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) s->flush_fd = connect_to_sdog(s->addr, s->port); if (s->flush_fd < 0) { error_report("failed to connect"); + ret = s->flush_fd; goto out; } } @@ -1111,6 +1122,7 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) fd = connect_to_sdog(s->addr, s->port); if (fd < 0) { error_report("failed to connect"); + ret = fd; goto out; } @@ -1139,7 +1151,7 @@ out: closesocket(s->fd); } g_free(buf); - return -1; + return ret; } static int do_sd_create(char *filename, int64_t vdi_size, @@ -1154,7 +1166,7 @@ static int do_sd_create(char *filename, int64_t vdi_size, fd = connect_to_sdog(addr, port); if (fd < 0) { - return -EIO; + return fd; } memset(buf, 0, sizeof(buf)); @@ -1177,7 +1189,7 @@ static int do_sd_create(char *filename, int64_t vdi_size, closesocket(fd); if (ret) { - return -EIO; + return ret; } if (rsp->result != SD_RES_SUCCESS) { @@ -1294,8 +1306,9 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) } ret = bdrv_file_open(&bs, backing_file, 0); - if (ret < 0) - return -EIO; + if (ret < 0) { + return ret; + } s = bs->opaque; @@ -1379,7 +1392,7 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset) fd = connect_to_sdog(s->addr, s->port); if (fd < 0) { - return -EIO; + return fd; } /* we don't need to update entire object */ @@ -1391,10 +1404,9 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset) if (ret < 0) { error_report("failed to update an inode."); - return -EIO; } - return 0; + return ret; } /* @@ -1464,6 +1476,7 @@ static int sd_create_branch(BDRVSheepdogState *s) fd = connect_to_sdog(s->addr, s->port); if (fd < 0) { error_report("failed to connect"); + ret = fd; goto out; } @@ -1606,8 +1619,9 @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num, if (bs->growable && sector_num + nb_sectors > bs->total_sectors) { /* TODO: shouldn't block here */ - if (sd_truncate(bs, (sector_num + nb_sectors) * SECTOR_SIZE) < 0) { - return -EIO; + ret = sd_truncate(bs, (sector_num + nb_sectors) * SECTOR_SIZE); + if (ret < 0) { + return ret; } bs->total_sectors = sector_num + nb_sectors; } @@ -1724,7 +1738,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) /* refresh inode. */ fd = connect_to_sdog(s->addr, s->port); if (fd < 0) { - ret = -EIO; + ret = fd; goto cleanup; } @@ -1732,7 +1746,6 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) s->inode.nr_copies, datalen, 0, 0, s->cache_enabled); if (ret < 0) { error_report("failed to write snapshot's inode."); - ret = -EIO; goto cleanup; } @@ -1741,7 +1754,6 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) if (ret < 0) { error_report("failed to create inode for snapshot. %s", strerror(errno)); - ret = -EIO; goto cleanup; } @@ -1752,7 +1764,6 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) if (ret < 0) { error_report("failed to read new inode info. %s", strerror(errno)); - ret = -EIO; goto cleanup; } @@ -1773,7 +1784,7 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) char *buf = NULL; uint32_t vid; uint32_t snapid = 0; - int ret = -ENOENT, fd; + int ret = 0, fd; old_s = g_malloc(sizeof(BDRVSheepdogState)); @@ -1791,13 +1802,13 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) ret = find_vdi_name(s, vdi, snapid, tag, &vid, 1); if (ret) { error_report("Failed to find_vdi_name"); - ret = -ENOENT; goto out; } fd = connect_to_sdog(s->addr, s->port); if (fd < 0) { error_report("failed to connect"); + ret = fd; goto out; } @@ -1808,7 +1819,6 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) closesocket(fd); if (ret) { - ret = -ENOENT; goto out; } @@ -1861,6 +1871,7 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) fd = connect_to_sdog(s->addr, s->port); if (fd < 0) { + ret = fd; goto out; } @@ -1888,6 +1899,7 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) fd = connect_to_sdog(s->addr, s->port); if (fd < 0) { error_report("failed to connect"); + ret = fd; goto out; } @@ -1925,6 +1937,10 @@ out: g_free(vdi_inuse); + if (ret < 0) { + return ret; + } + return found; } @@ -1940,8 +1956,7 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data, fd = connect_to_sdog(s->addr, s->port); if (fd < 0) { - ret = -EIO; - goto cleanup; + return fd; } while (size) { @@ -1965,7 +1980,6 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data, if (ret < 0) { error_report("failed to save vmstate %s", strerror(errno)); - ret = -EIO; goto cleanup; } From b6fc8245e96dea6b7198a46e883d107403ddb90c Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Thu, 17 May 2012 03:15:34 +0900 Subject: [PATCH 05/10] sheepdog: use heap instead of stack for BDRVSheepdogState bdrv_create() is called in coroutine context now, so we cannot use more stack than 1 MB in the function if we use ucontext coroutine. This patch allocates BDRVSheepdogState, whose size is 4 MB, on the heap in sd_create(). Signed-off-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 991133e08b..6d52277a89 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1249,24 +1249,26 @@ out: static int sd_create(const char *filename, QEMUOptionParameter *options) { - int ret; + int ret = 0; uint32_t vid = 0, base_vid = 0; int64_t vdi_size = 0; char *backing_file = NULL; - BDRVSheepdogState s; + BDRVSheepdogState *s; char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN]; uint32_t snapid; int prealloc = 0; const char *vdiname; + s = g_malloc0(sizeof(BDRVSheepdogState)); + strstart(filename, "sheepdog:", &vdiname); - memset(&s, 0, sizeof(s)); memset(vdi, 0, sizeof(vdi)); memset(tag, 0, sizeof(tag)); - if (parse_vdiname(&s, vdiname, vdi, &snapid, tag) < 0) { + if (parse_vdiname(s, vdiname, vdi, &snapid, tag) < 0) { error_report("invalid filename"); - return -EINVAL; + ret = -EINVAL; + goto out; } while (options && options->name) { @@ -1282,7 +1284,8 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) } else { error_report("Invalid preallocation mode: '%s'", options->value.s); - return -EINVAL; + ret = -EINVAL; + goto out; } } options++; @@ -1290,7 +1293,8 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) if (vdi_size > SD_MAX_VDI_SIZE) { error_report("too big image size"); - return -EINVAL; + ret = -EINVAL; + goto out; } if (backing_file) { @@ -1302,12 +1306,13 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) drv = bdrv_find_protocol(backing_file); if (!drv || strcmp(drv->protocol_name, "sheepdog") != 0) { error_report("backing_file must be a sheepdog image"); - return -EINVAL; + ret = -EINVAL; + goto out; } ret = bdrv_file_open(&bs, backing_file, 0); if (ret < 0) { - return ret; + goto out; } s = bs->opaque; @@ -1315,19 +1320,23 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) if (!is_snapshot(&s->inode)) { error_report("cannot clone from a non snapshot vdi"); bdrv_delete(bs); - return -EINVAL; + ret = -EINVAL; + goto out; } base_vid = s->inode.vdi_id; bdrv_delete(bs); } - ret = do_sd_create(vdi, vdi_size, base_vid, &vid, 0, s.addr, s.port); + ret = do_sd_create(vdi, vdi_size, base_vid, &vid, 0, s->addr, s->port); if (!prealloc || ret) { - return ret; + goto out; } - return sd_prealloc(filename); + ret = sd_prealloc(filename); +out: + g_free(s); + return ret; } static void sd_close(BlockDriverState *bs) From df021791897cb7a171710dadf66648b4379de627 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 24 May 2012 12:56:32 +0200 Subject: [PATCH 06/10] qcow2: Check qcow2_alloc_clusters_at() return value When using qcow2_alloc_clusters_at(), the cluster allocation code checked the wrong variable for an error code. Signed-off-by: Kevin Wolf --- block/qcow2-cluster.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 10c22fe12b..4b3345b11b 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -762,7 +762,6 @@ static int do_alloc_cluster_offset(BlockDriverState *bs, uint64_t guest_offset, uint64_t *host_offset, unsigned int *nb_clusters) { BDRVQcowState *s = bs->opaque; - int64_t cluster_offset; QCowL2Meta *old_alloc; trace_qcow2_do_alloc_clusters_offset(qemu_coroutine_self(), guest_offset, @@ -808,17 +807,21 @@ static int do_alloc_cluster_offset(BlockDriverState *bs, uint64_t guest_offset, /* Allocate new clusters */ trace_qcow2_cluster_alloc_phys(qemu_coroutine_self()); if (*host_offset == 0) { - cluster_offset = qcow2_alloc_clusters(bs, *nb_clusters * s->cluster_size); + int64_t cluster_offset = + qcow2_alloc_clusters(bs, *nb_clusters * s->cluster_size); + if (cluster_offset < 0) { + return cluster_offset; + } + *host_offset = cluster_offset; + return 0; } else { - cluster_offset = *host_offset; - *nb_clusters = qcow2_alloc_clusters_at(bs, cluster_offset, *nb_clusters); + int ret = qcow2_alloc_clusters_at(bs, *host_offset, *nb_clusters); + if (ret < 0) { + return ret; + } + *nb_clusters = ret; + return 0; } - - if (cluster_offset < 0) { - return cluster_offset; - } - *host_offset = cluster_offset; - return 0; } /* From b84762e2456e15c091c25aff052afb8766394acd Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 24 May 2012 16:26:51 +0100 Subject: [PATCH 07/10] qemu-iotests: mark 035 qcow2-only The 035 parallel aio write test relies on knowledge of qcow2 metadata layout to stress parallel L2 table accesses. This only works for qcow2 unless we add additional calculations for qed or other formats. Mark this test as qcow2-only. Note that the test is strictly speaking non-deterministic although the output produced is reliable with qcow2. This is because the aio_write command returns before the aio write request has completed. Completions can occur at any time afterwards and cause a message to be printed. Therefore the exact output of this test is not deterministic but we seem to get away with it for qcow2 (maybe due to coroutine and main loop scheduling). Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/qemu-iotests/035 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/035 b/tests/qemu-iotests/035 index 56616a1b7d..9d2d3472e7 100755 --- a/tests/qemu-iotests/035 +++ b/tests/qemu-iotests/035 @@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.rc . ./common.filter -_supported_fmt generic +_supported_fmt qcow2 _supported_proto generic _supported_os Linux From 9ecd394753919b7395e620dffb8a5e23c45b5e07 Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Thu, 24 May 2012 11:02:28 +0200 Subject: [PATCH 08/10] fdc: floppy drive should be visible after start without media If you start guest with floppy drive but without media inserted, guest still should see floppy drive pressent. Signed-off-by: Pavel Hrdina Signed-off-by: Kevin Wolf --- hw/pc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/pc.c b/hw/pc.c index 4d34a335ed..967c17a3fe 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -382,7 +382,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, if (floppy) { fdc_get_bs(fd, floppy); for (i = 0; i < 2; i++) { - if (fd[i] && bdrv_is_inserted(fd[i])) { + if (fd[i]) { bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track, &last_sect, FDRIVE_DRV_NONE, &fd_type[i], &rate); From cfb08fbafcd946341bdf14103293887763802697 Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Thu, 24 May 2012 11:02:29 +0200 Subject: [PATCH 09/10] fdc: fix media detection We have to set up 'media_changed' after guest start so floppy driver could detect that there is no media in drive. For this purpose we call 'fdctrl_change_cb' instead of 'fd_revalidate' in 'fdctrl_connect_drives'. 'fd_revalidate' is called inside 'fdctrl_change_cb'. We still have to set default drive geometry in 'fd_revalidate' even if there is no media in drive. When you try to open (windows) or mount (linux) floppy the driver tries to seek on track 1. Linux guest stuck in loop then kernel crashes and windows guest prints error message. Signed-off-by: Pavel Hrdina Signed-off-by: Kevin Wolf --- hw/fdc.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index cb4cd25c11..30d34e3f1d 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv) FDriveRate rate; FLOPPY_DPRINTF("revalidate\n"); - if (drv->bs != NULL && bdrv_is_inserted(drv->bs)) { + 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); - if (nb_heads != 0 && max_track != 0 && last_sect != 0) { - FLOPPY_DPRINTF("User defined disk (%d %d %d)", + if (!bdrv_is_inserted(drv->bs)) { + FLOPPY_DPRINTF("No disk in drive\n"); + } else if (nb_heads != 0 && max_track != 0 && last_sect != 0) { + FLOPPY_DPRINTF("User defined disk (%d %d %d)\n", nb_heads - 1, max_track, last_sect); } else { FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads, @@ -201,7 +203,7 @@ static void fd_revalidate(FDrive *drv) drv->drive = drive; drv->media_rate = rate; } else { - FLOPPY_DPRINTF("No disk in drive\n"); + FLOPPY_DPRINTF("No drive connected\n"); drv->last_sect = 0; drv->max_track = 0; drv->flags &= ~FDISK_DBL_SIDES; @@ -709,7 +711,7 @@ static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0) FDrive *cur_drv; /* A seek clears the disk change line (if a disk is inserted) */ cur_drv = get_cur_drv(fdctrl); - if (cur_drv->max_track) { + if (cur_drv->bs != NULL && bdrv_is_inserted(cur_drv->bs)) { cur_drv->media_changed = 0; } } @@ -1878,7 +1880,7 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl) } fd_init(drive); - fd_revalidate(drive); + fdctrl_change_cb(drive, 0); if (drive->bs) { bdrv_set_dev_ops(drive->bs, &fdctrl_block_ops, drive); } From 7cd331617a48785faaf2aafe36db5c06285bfed8 Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Thu, 24 May 2012 11:02:30 +0200 Subject: [PATCH 10/10] fdc-test: introduced qtest no_media_on_start and cmos qtest for floppy As default a guest has always one floppy drive so 0x10 byte in CMOS has to have 0x40 value. Higher 4 bits means that the first floppy drive is 1.44 Mb 3"5 drive and lower 4 bits means the second drive is not present. After the guest starts DSKCHG bit in DIR register should be set. If there is no media in drive, this bit should be set all the time. Because we start the guest without media in drive, we have to swap 'eject' and 'change' in 'test_media_change'. Signed-off-by: Pavel Hrdina Signed-off-by: Kevin Wolf --- tests/fdc-test.c | 79 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 24 deletions(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index 5b5dd7481f..22d24ac7dc 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -63,6 +63,12 @@ char test_image[] = "/tmp/qtest.XXXXXX"; #define assert_bit_set(data, mask) g_assert_cmphex((data) & (mask), ==, (mask)) #define assert_bit_clear(data, mask) g_assert_cmphex((data) & (mask), ==, 0) +static uint8_t base = 0x70; + +enum { + CMOS_FLOPPY = 0x10, +}; + static void floppy_send(uint8_t byte) { uint8_t msr; @@ -108,16 +114,59 @@ static void send_step_pulse(void) cyl = (cyl + 1) % 4; } +static uint8_t cmos_read(uint8_t reg) +{ + outb(base + 0, reg); + return inb(base + 1); +} + +static void test_cmos(void) +{ + uint8_t cmos; + + cmos = cmos_read(CMOS_FLOPPY); + g_assert(cmos == 0x40); +} + +static void test_no_media_on_start(void) +{ + uint8_t dir; + + /* Media changed bit must be set all time after start if there is + * no media in drive. */ + 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(); + send_step_pulse(); + dir = inb(FLOPPY_BASE + reg_dir); + assert_bit_set(dir, DSKCHG); + dir = inb(FLOPPY_BASE + reg_dir); + assert_bit_set(dir, DSKCHG); +} + static void test_media_change(void) { uint8_t dir; - /* Media changed bit must be up-to-date after step pulse. Do two SEEKs - * because we may already happen to be on the right cylinder initially. */ - send_step_pulse(); + /* Insert media in drive. DSKCHK should not be reset until a step pulse + * is sent. */ + qmp("{'execute':'change', 'arguments':{ 'device':'floppy0', " + "'target': '%s' }}", test_image); + qmp(""); /* ignore event (FIXME open -> open transition?!) */ + qmp(""); /* ignore event */ + + 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(); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_clear(dir, DSKCHG); + dir = inb(FLOPPY_BASE + reg_dir); + assert_bit_clear(dir, DSKCHG); /* Eject the floppy and check that DSKCHG is set. Reading it out doesn't * reset the bit. */ @@ -134,24 +183,6 @@ static void test_media_change(void) assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); - - /* And then insert it again. DSKCHK should not be reset until a step pulse - * is sent. */ - qmp("{'execute':'change', 'arguments':{ 'device':'floppy0', " - "'target': '%s' }}", test_image); - qmp(""); /* ignore event (FIXME open -> open transition?!) */ - qmp(""); /* ignore event */ - - 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(); - dir = inb(FLOPPY_BASE + reg_dir); - assert_bit_clear(dir, DSKCHG); - dir = inb(FLOPPY_BASE + reg_dir); - assert_bit_clear(dir, DSKCHG); } int main(int argc, char **argv) @@ -177,12 +208,12 @@ int main(int argc, char **argv) /* Run the tests */ g_test_init(&argc, &argv, NULL); - cmdline = g_strdup_printf("-vnc none " - "-drive file=%s,if=floppy,cache=writeback ", - test_image); + cmdline = g_strdup_printf("-vnc none "); qtest_start(cmdline); qtest_irq_intercept_in(global_qtest, "ioapic"); + qtest_add_func("/fdc/cmos", test_cmos); + qtest_add_func("/fdc/no_media_on_start", test_no_media_on_start); qtest_add_func("/fdc/media_change", test_media_change); ret = g_test_run();