From 08388273a3c4e895bf7e10f356f66c2363cfe90b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Mon, 6 Feb 2012 22:29:02 +0100 Subject: [PATCH 01/27] fdc: take side count into account MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Floppies can be simple or double-sided. However, current code was only taking the common case into account (ie 2 sides). This repairs single-sided floppies, which where totally broken before this patch : for track > 0, wrong sector number was calculated, and data was read/written at wrong place on underlying device. Fortunately, only some 360 kB floppies are single-sided, so this bug was probably not seen much. Signed-off-by: Hervé Poussineau Signed-off-by: Kevin Wolf --- hw/fdc.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index 38fad587cb..64e635a849 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -95,16 +95,19 @@ static void fd_init(FDrive *drv) drv->max_track = 0; } +#define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1) + static int fd_sector_calc(uint8_t head, uint8_t track, uint8_t sect, - uint8_t last_sect) + uint8_t last_sect, uint8_t num_sides) { - return (((track * 2) + head) * last_sect) + sect - 1; + return (((track * num_sides) + head) * last_sect) + sect - 1; } /* Returns current position, in sectors, for given drive */ static int fd_sector(FDrive *drv) { - return fd_sector_calc(drv->head, drv->track, drv->sect, drv->last_sect); + return fd_sector_calc(drv->head, drv->track, drv->sect, drv->last_sect, + NUM_SIDES(drv)); } /* Seek to a new position: @@ -135,7 +138,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, drv->max_track, drv->last_sect); return 3; } - sector = fd_sector_calc(head, track, sect, drv->last_sect); + sector = fd_sector_calc(head, track, sect, drv->last_sect, NUM_SIDES(drv)); ret = 0; if (sector != fd_sector(drv)) { #if 0 @@ -1019,7 +1022,8 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction) ks = fdctrl->fifo[4]; FLOPPY_DPRINTF("Start transfer at %d %d %02x %02x (%d)\n", GET_CUR_DRV(fdctrl), kh, kt, ks, - fd_sector_calc(kh, kt, ks, cur_drv->last_sect)); + fd_sector_calc(kh, kt, ks, cur_drv->last_sect, + NUM_SIDES(cur_drv))); switch (fd_seek(cur_drv, kh, kt, ks, fdctrl->config & FD_CONFIG_EIS)) { case 2: /* sect too big */ @@ -1289,7 +1293,8 @@ static void fdctrl_format_sector(FDCtrl *fdctrl) ks = fdctrl->fifo[8]; FLOPPY_DPRINTF("format sector at %d %d %02x %02x (%d)\n", GET_CUR_DRV(fdctrl), kh, kt, ks, - fd_sector_calc(kh, kt, ks, cur_drv->last_sect)); + fd_sector_calc(kh, kt, ks, cur_drv->last_sect, + NUM_SIDES(cur_drv))); switch (fd_seek(cur_drv, kh, kt, ks, fdctrl->config & FD_CONFIG_EIS)) { case 2: /* sect too big */ From 1457a75843740b0433edfc88666c793858f4423a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Mon, 6 Feb 2012 22:29:03 +0100 Subject: [PATCH 02/27] fdc: set busy bit when starting a command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This bit must be active while a command is currently executed. Signed-off-by: Hervé Poussineau Signed-off-by: Kevin Wolf --- hw/fdc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/fdc.c b/hw/fdc.c index 64e635a849..05edc3aa3c 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -1446,7 +1446,6 @@ static void fdctrl_handle_readid(FDCtrl *fdctrl, int direction) { FDrive *cur_drv = get_cur_drv(fdctrl); - /* XXX: should set main status register to busy */ cur_drv->head = (fdctrl->fifo[1] >> 2) & 1; qemu_mod_timer(fdctrl->result_timer, qemu_get_clock_ns(vm_clock) + (get_ticks_per_sec() / 50)); @@ -1734,6 +1733,7 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value) pos = command_to_handler[value & 0xff]; FLOPPY_DPRINTF("%s command\n", handlers[pos].name); fdctrl->data_len = handlers[pos].parameters + 1; + fdctrl->msr |= FD_MSR_CMDBUSY; } FLOPPY_DPRINTF("%s: %02x\n", __func__, value); From a005186c176e0e630dd146cd3b973384d5e62f34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Mon, 6 Feb 2012 22:29:04 +0100 Subject: [PATCH 03/27] fdc: most control commands do not generate interrupts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In fact, only three control commands generate an interrupt: read_id, recalibrate and seek Signed-off-by: Hervé Poussineau Signed-off-by: Kevin Wolf --- hw/fdc.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index 05edc3aa3c..9d1f5b37a3 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -1348,7 +1348,7 @@ static void fdctrl_handle_lock(FDCtrl *fdctrl, int direction) { fdctrl->lock = (fdctrl->fifo[0] & 0x80) ? 1 : 0; fdctrl->fifo[0] = fdctrl->lock << 4; - fdctrl_set_fifo(fdctrl, 1, fdctrl->lock); + fdctrl_set_fifo(fdctrl, 1, 0); } static void fdctrl_handle_dumpreg(FDCtrl *fdctrl, int direction) @@ -1380,7 +1380,7 @@ static void fdctrl_handle_version(FDCtrl *fdctrl, int direction) { /* Controller's version */ fdctrl->fifo[0] = fdctrl->version; - fdctrl_set_fifo(fdctrl, 1, 1); + fdctrl_set_fifo(fdctrl, 1, 0); } static void fdctrl_handle_partid(FDCtrl *fdctrl, int direction) @@ -1439,7 +1439,7 @@ static void fdctrl_handle_save(FDCtrl *fdctrl, int direction) fdctrl->fifo[12] = fdctrl->pwrd; fdctrl->fifo[13] = 0; fdctrl->fifo[14] = 0; - fdctrl_set_fifo(fdctrl, 15, 1); + fdctrl_set_fifo(fdctrl, 15, 0); } static void fdctrl_handle_readid(FDCtrl *fdctrl, int direction) @@ -1580,7 +1580,7 @@ static void fdctrl_handle_powerdown_mode(FDCtrl *fdctrl, int direction) { fdctrl->pwrd = fdctrl->fifo[1]; fdctrl->fifo[0] = fdctrl->fifo[1]; - fdctrl_set_fifo(fdctrl, 1, 1); + fdctrl_set_fifo(fdctrl, 1, 0); } static void fdctrl_handle_option(FDCtrl *fdctrl, int direction) @@ -1599,7 +1599,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct fdctrl->fifo[0] = fdctrl->fifo[1]; fdctrl->fifo[2] = 0; fdctrl->fifo[3] = 0; - fdctrl_set_fifo(fdctrl, 4, 1); + fdctrl_set_fifo(fdctrl, 4, 0); } else { fdctrl_reset_fifo(fdctrl); } @@ -1607,7 +1607,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct /* ERROR */ fdctrl->fifo[0] = 0x80 | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl); - fdctrl_set_fifo(fdctrl, 1, 1); + fdctrl_set_fifo(fdctrl, 1, 0); } } From 8510854ee7dc04f70b53aea4d2518a0585fe5d09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Mon, 6 Feb 2012 22:29:05 +0100 Subject: [PATCH 04/27] fdc: handle read-only floppies (abort early on write commands) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A real floppy doesn't attempt to write to read-only media either. Signed-off-by: Hervé Poussineau Signed-off-by: Kevin Wolf --- hw/fdc.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/hw/fdc.c b/hw/fdc.c index 9d1f5b37a3..3d4bb116cb 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -300,6 +300,7 @@ enum { }; enum { + FD_SR1_NW = 0x02, /* Not writable */ FD_SR1_EC = 0x80, /* End of cylinder */ }; @@ -1179,6 +1180,16 @@ static int fdctrl_transfer_handler (void *opaque, int nchan, break; case FD_DIR_WRITE: /* WRITE commands */ + if (cur_drv->ro) { + /* Handle readonly medium early, no need to do DMA, touch the + * LED or attempt any writes. A real floppy doesn't attempt + * to write to readonly media either. */ + fdctrl_stop_transfer(fdctrl, + FD_SR0_ABNTERM | FD_SR0_SEEK, FD_SR1_NW, + 0x00); + goto transfer_error; + } + DMA_read_memory (nchan, fdctrl->fifo + rel_pos, fdctrl->data_pos, len); if (bdrv_write(cur_drv->bs, fd_sector(cur_drv), From a758f8f415985abc85be1da6d22f0590a74ca23d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Mon, 6 Feb 2012 22:29:06 +0100 Subject: [PATCH 05/27] fdc: add CCR (Configuration Control Register) write register MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DIR and CCR registers share the same address ; DIR is read-only while CCR is write-only CCR register is used to change media transfer rate, which will be checked in following changes. Signed-off-by: Hervé Poussineau Signed-off-by: Kevin Wolf --- hw/fdc.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/hw/fdc.c b/hw/fdc.c index 3d4bb116cb..02ced2225f 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -224,6 +224,7 @@ static void fdctrl_write_rate(FDCtrl *fdctrl, uint32_t value); static uint32_t fdctrl_read_data(FDCtrl *fdctrl); static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value); static uint32_t fdctrl_read_dir(FDCtrl *fdctrl); +static void fdctrl_write_ccr(FDCtrl *fdctrl, uint32_t value); enum { FD_DIR_WRITE = 0, @@ -248,6 +249,7 @@ enum { FD_REG_DSR = 0x04, FD_REG_FIFO = 0x05, FD_REG_DIR = 0x07, + FD_REG_CCR = 0x07, }; enum { @@ -491,6 +493,9 @@ static void fdctrl_write (void *opaque, uint32_t reg, uint32_t value) case FD_REG_FIFO: fdctrl_write_data(fdctrl, value); break; + case FD_REG_CCR: + fdctrl_write_ccr(fdctrl, value); + break; default: break; } @@ -881,6 +886,23 @@ static void fdctrl_write_rate(FDCtrl *fdctrl, uint32_t value) fdctrl->dsr = value; } +/* Configuration control register: 0x07 (write) */ +static void fdctrl_write_ccr(FDCtrl *fdctrl, uint32_t value) +{ + /* Reset mode */ + if (!(fdctrl->dor & FD_DOR_nRESET)) { + FLOPPY_DPRINTF("Floppy controller in RESET state !\n"); + return; + } + FLOPPY_DPRINTF("configuration control register set to 0x%02x\n", value); + + /* Only the rate selection bits used in AT mode, and we + * store those in the DSR. + */ + fdctrl->dsr = (fdctrl->dsr & ~FD_DSR_DRATEMASK) | + (value & FD_DSR_DRATEMASK); +} + static int fdctrl_media_changed(FDrive *drv) { int ret; From f8d3d128576d2b492c4267083f67671b19edb63a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Mon, 6 Feb 2012 22:29:07 +0100 Subject: [PATCH 06/27] block: add a transfer rate for floppy types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Floppies must be read at a specific transfer rate, depending of its own format. Update floppy description table to include required transfer rate. Signed-off-by: Hervé Poussineau Signed-off-by: Kevin Wolf --- block.c | 74 +++++++++++++++++++++++++++++--------------------------- block.h | 10 +++++++- hw/fdc.c | 3 ++- hw/pc.c | 3 ++- 4 files changed, 52 insertions(+), 38 deletions(-) diff --git a/block.c b/block.c index e27d528ac7..1150437b70 100644 --- a/block.c +++ b/block.c @@ -2013,58 +2013,60 @@ typedef struct FDFormat { 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_DRV_144, 20, 80, 1, }, - { FDRIVE_DRV_144, 21, 80, 1, }, - { FDRIVE_DRV_144, 21, 82, 1, }, - { FDRIVE_DRV_144, 21, 83, 1, }, - { FDRIVE_DRV_144, 22, 80, 1, }, - { FDRIVE_DRV_144, 23, 80, 1, }, - { FDRIVE_DRV_144, 24, 80, 1, }, + { 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_DRV_288, 39, 80, 1, }, - { FDRIVE_DRV_288, 40, 80, 1, }, - { FDRIVE_DRV_288, 44, 80, 1, }, - { FDRIVE_DRV_288, 48, 80, 1, }, + { 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_DRV_144, 10, 80, 1, }, - { FDRIVE_DRV_144, 10, 82, 1, }, - { FDRIVE_DRV_144, 10, 83, 1, }, - { FDRIVE_DRV_144, 13, 80, 1, }, - { FDRIVE_DRV_144, 14, 80, 1, }, + { 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_DRV_120, 18, 80, 1, }, - { FDRIVE_DRV_120, 18, 82, 1, }, - { FDRIVE_DRV_120, 18, 83, 1, }, - { FDRIVE_DRV_120, 20, 80, 1, }, + { 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_DRV_120, 11, 80, 1, }, + { 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_DRV_120, 9, 40, 0, }, - { FDRIVE_DRV_120, 10, 41, 1, }, - { FDRIVE_DRV_120, 10, 42, 1, }, + { 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_DRV_120, 8, 40, 0, }, + { 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_DRV_144, 9, 80, 0, FDRIVE_RATE_250K, }, /* end */ - { FDRIVE_DRV_NONE, -1, -1, 0, }, + { 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) + FDriveType drive_in, FDriveType *drive, + FDriveRate *rate) { const FDFormat *parse; uint64_t nb_sectors, size; @@ -2073,6 +2075,7 @@ void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads, bdrv_get_geometry_hint(bs, nb_heads, max_track, last_sect); if (*nb_heads != 0 && *max_track != 0 && *last_sect != 0) { /* User defined disk */ + *rate = FDRIVE_RATE_500K; } else { bdrv_get_geometry(bs, &nb_sectors); match = -1; @@ -2107,6 +2110,7 @@ void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads, *max_track = parse->max_track; *last_sect = parse->last_sect; *drive = parse->drive; + *rate = parse->rate; } } diff --git a/block.h b/block.h index 49bca5aec0..bbc38300e1 100644 --- a/block.h +++ b/block.h @@ -252,9 +252,17 @@ typedef enum FDriveType { 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); + 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 02ced2225f..08012f9fef 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -172,12 +172,13 @@ static void fd_revalidate(FDrive *drv) { int nb_heads, max_track, last_sect, ro; FDriveType drive; + FDriveRate rate; FLOPPY_DPRINTF("revalidate\n"); if (drv->bs != NULL && bdrv_is_inserted(drv->bs)) { ro = bdrv_is_read_only(drv->bs); bdrv_get_floppy_geometry_hint(drv->bs, &nb_heads, &max_track, - &last_sect, drv->drive, &drive); + &last_sect, drv->drive, &drive, &rate); if (nb_heads != 0 && max_track != 0 && last_sect != 0) { FLOPPY_DPRINTF("User defined disk (%d %d %d)", nb_heads - 1, max_track, last_sect); diff --git a/hw/pc.c b/hw/pc.c index 59a7f3928f..12c02f2044 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -335,6 +335,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, { int val, nb, nb_heads, max_track, last_sect, i; FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE }; + FDriveRate rate; BlockDriverState *fd[MAX_FD]; static pc_cmos_init_late_arg arg; @@ -383,7 +384,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, if (fd[i] && bdrv_is_inserted(fd[i])) { bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track, &last_sect, FDRIVE_DRV_NONE, - &fd_type[i]); + &fd_type[i], &rate); } } } From 09c6d5850fbd2b8abeba927fffff036585a755e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Mon, 6 Feb 2012 22:29:09 +0100 Subject: [PATCH 07/27] fdc: add a 'check media rate' property. Not used yet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Set it to true for current Qemu versions, and false for previous ones Signed-off-by: Hervé Poussineau Signed-off-by: Kevin Wolf --- hw/fdc.c | 3 +++ hw/pc_piix.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/hw/fdc.c b/hw/fdc.c index 08012f9fef..1c7e7759f6 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -420,6 +420,7 @@ struct FDCtrl { int sun4m; FDrive drives[MAX_FD]; int reset_sensei; + uint32_t check_media_rate; /* Timers state */ uint8_t timer0; uint8_t timer1; @@ -2003,6 +2004,8 @@ static Property isa_fdc_properties[] = { DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].bs), DEFINE_PROP_INT32("bootindexA", FDCtrlISABus, bootindexA, -1), DEFINE_PROP_INT32("bootindexB", FDCtrlISABus, bootindexB, -1), + DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus, state.check_media_rate, + 0, true), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 5e11d15026..6c5c40f5df 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -384,6 +384,10 @@ static QEMUMachine pc_machine_v1_0 = { .driver = "pc-sysfw", .property = "rom_only", .value = stringify(1), + }, { + .driver = "isa-fdc", + .property = "check_media_rate", + .value = "off", }, { /* end of list */ } }, @@ -399,6 +403,10 @@ static QEMUMachine pc_machine_v0_15 = { .driver = "pc-sysfw", .property = "rom_only", .value = stringify(1), + }, { + .driver = "isa-fdc", + .property = "check_media_rate", + .value = "off", }, { /* end of list */ } }, @@ -434,6 +442,10 @@ static QEMUMachine pc_machine_v0_14 = { .driver = "virtio-balloon-pci", .property = "event_idx", .value = "off", + },{ + .driver = "isa-fdc", + .property = "check_media_rate", + .value = "off", }, { .driver = "pc-sysfw", @@ -486,6 +498,10 @@ static QEMUMachine pc_machine_v0_13 = { .driver = "AC97", .property = "use_broken_id", .value = stringify(1), + },{ + .driver = "isa-fdc", + .property = "check_media_rate", + .value = "off", }, { .driver = "pc-sysfw", @@ -542,6 +558,10 @@ static QEMUMachine pc_machine_v0_12 = { .driver = "AC97", .property = "use_broken_id", .value = stringify(1), + },{ + .driver = "isa-fdc", + .property = "check_media_rate", + .value = "off", }, { .driver = "pc-sysfw", @@ -606,6 +626,10 @@ static QEMUMachine pc_machine_v0_11 = { .driver = "AC97", .property = "use_broken_id", .value = stringify(1), + },{ + .driver = "isa-fdc", + .property = "check_media_rate", + .value = "off", }, { .driver = "pc-sysfw", @@ -682,6 +706,10 @@ static QEMUMachine pc_machine_v0_10 = { .driver = "AC97", .property = "use_broken_id", .value = stringify(1), + },{ + .driver = "isa-fdc", + .property = "check_media_rate", + .value = "off", }, { .driver = "pc-sysfw", From 844f65d661fa6d7e5460cc76ccabfe1398849fe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Mon, 6 Feb 2012 22:29:10 +0100 Subject: [PATCH 08/27] fdc: check if media rate is correct before doing any transfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The programmed rate has to be the same as the required rate for the floppy format ; if that's not the case, the transfer should abort. This check can be disabled by using the 'check_media_rate' property. Save media rate value only if media rate check is enabled. Signed-off-by: Hervé Poussineau Signed-off-by: Kevin Wolf --- hw/fdc.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index 1c7e7759f6..cc03326608 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -62,12 +62,15 @@ #define FD_SECTOR_SC 2 /* Sector size code */ #define FD_RESET_SENSEI_COUNT 4 /* Number of sense interrupts on RESET */ +typedef struct FDCtrl FDCtrl; + /* Floppy disk drive emulation */ typedef enum FDiskFlags { FDISK_DBL_SIDES = 0x01, } FDiskFlags; typedef struct FDrive { + FDCtrl *fdctrl; BlockDriverState *bs; /* Drive status */ FDriveType drive; @@ -83,6 +86,7 @@ typedef struct FDrive { uint16_t bps; /* Bytes per sector */ uint8_t ro; /* Is read-only */ uint8_t media_changed; /* Is media changed */ + uint8_t media_rate; /* Data rate of medium */ } FDrive; static void fd_init(FDrive *drv) @@ -195,6 +199,7 @@ static void fd_revalidate(FDrive *drv) drv->last_sect = last_sect; drv->ro = ro; drv->drive = drive; + drv->media_rate = rate; } else { FLOPPY_DPRINTF("No disk in drive\n"); drv->last_sect = 0; @@ -206,8 +211,6 @@ static void fd_revalidate(FDrive *drv) /********************************************************/ /* Intel 82078 floppy disk controller emulation */ -typedef struct FDCtrl FDCtrl; - static void fdctrl_reset(FDCtrl *fdctrl, int do_irq); static void fdctrl_reset_fifo(FDCtrl *fdctrl); static int fdctrl_transfer_handler (void *opaque, int nchan, @@ -303,6 +306,7 @@ enum { }; enum { + FD_SR1_MA = 0x01, /* Missing address mark */ FD_SR1_NW = 0x02, /* Not writable */ FD_SR1_EC = 0x80, /* End of cylinder */ }; @@ -549,6 +553,24 @@ static const VMStateDescription vmstate_fdrive_media_changed = { } }; +static bool fdrive_media_rate_needed(void *opaque) +{ + FDrive *drive = opaque; + + return drive->fdctrl->check_media_rate; +} + +static const VMStateDescription vmstate_fdrive_media_rate = { + .name = "fdrive/media_rate", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT8(media_rate, FDrive), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_fdrive = { .name = "fdrive", .version_id = 1, @@ -564,6 +586,9 @@ static const VMStateDescription vmstate_fdrive = { { .vmsd = &vmstate_fdrive_media_changed, .needed = &fdrive_media_changed_needed, + } , { + .vmsd = &vmstate_fdrive_media_rate, + .needed = &fdrive_media_rate_needed, } , { /* empty */ } @@ -1078,6 +1103,19 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction) break; } + /* Check the data rate. If the programmed data rate does not match + * the currently inserted medium, the operation has to fail. */ + if (fdctrl->check_media_rate && + (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) { + FLOPPY_DPRINTF("data rate mismatch (fdc=%d, media=%d)\n", + fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate); + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00); + fdctrl->fifo[3] = kt; + fdctrl->fifo[4] = kh; + fdctrl->fifo[5] = ks; + return; + } + /* Set the FIFO state */ fdctrl->data_dir = direction; fdctrl->data_pos = 0; @@ -1800,7 +1838,15 @@ static void fdctrl_result_timer(void *opaque) if (cur_drv->last_sect != 0) { cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) + 1; } - fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00); + /* READ_ID can't automatically succeed! */ + if (fdctrl->check_media_rate && + (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) { + FLOPPY_DPRINTF("read id rate mismatch (fdc=%d, media=%d)\n", + fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate); + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00); + } else { + fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00); + } } static void fdctrl_change_cb(void *opaque, bool load) @@ -1822,6 +1868,7 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl) for (i = 0; i < MAX_FD; i++) { drive = &fdctrl->drives[i]; + drive->fdctrl = fdctrl; if (drive->bs) { if (bdrv_get_on_error(drive->bs, 0) != BLOCK_ERR_STOP_ENOSPC) { From b072a3c85da1391d86aaea60451738131034c7c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Mon, 6 Feb 2012 22:29:11 +0100 Subject: [PATCH 09/27] fdc: fix seek command, which shouldn't check tracks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seek command just sends step pulses to the drive and doesn't care if there is a medium inserted of if it is banging the head against the drive. Signed-off-by: Hervé Poussineau Signed-off-by: Kevin Wolf --- hw/fdc.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index cc03326608..7879b70bc6 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -1622,13 +1622,16 @@ static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction) SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK); cur_drv = get_cur_drv(fdctrl); fdctrl_reset_fifo(fdctrl); + /* The seek command just sends step pulses to the drive and doesn't care if + * there is a medium inserted of if it's banging the head against the drive. + */ if (fdctrl->fifo[2] > cur_drv->max_track) { - fdctrl_raise_irq(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK); + cur_drv->track = cur_drv->max_track; } else { cur_drv->track = fdctrl->fifo[2]; - /* Raise Interrupt */ - fdctrl_raise_irq(fdctrl, FD_SR0_SEEK); } + /* Raise Interrupt */ + fdctrl_raise_irq(fdctrl, FD_SR0_SEEK); } static void fdctrl_handle_perpendicular_mode(FDCtrl *fdctrl, int direction) From a2df5fa324cd09a31225701e5b2f9fb067c237cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= Date: Mon, 6 Feb 2012 22:29:12 +0100 Subject: [PATCH 10/27] fdc: DIR (Digital Input Register) should return status of current drive... MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Hervé Poussineau Signed-off-by: Kevin Wolf --- hw/fdc.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index 7879b70bc6..a0236b7295 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -216,6 +216,7 @@ static void fdctrl_reset_fifo(FDCtrl *fdctrl); static int fdctrl_transfer_handler (void *opaque, int nchan, int dma_pos, int dma_len); static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0); +static FDrive *get_cur_drv(FDCtrl *fdctrl); static uint32_t fdctrl_read_statusA(FDCtrl *fdctrl); static uint32_t fdctrl_read_statusB(FDCtrl *fdctrl); @@ -956,14 +957,9 @@ static uint32_t fdctrl_read_dir(FDCtrl *fdctrl) { uint32_t retval = 0; - if (fdctrl_media_changed(drv0(fdctrl)) - || fdctrl_media_changed(drv1(fdctrl)) -#if MAX_FD == 4 - || fdctrl_media_changed(drv2(fdctrl)) - || fdctrl_media_changed(drv3(fdctrl)) -#endif - ) + if (fdctrl_media_changed(get_cur_drv(fdctrl))) { retval |= FD_DIR_DSKCHG; + } if (retval != 0) { FLOPPY_DPRINTF("Floppy digital input register: 0x%02x\n", retval); } From d53cdb307a6f6d7a5136898902659f2ded8f6582 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 15 Feb 2012 11:46:11 +0100 Subject: [PATCH 11/27] ide: fail I/O to empty disk Requesting a read or a write operation on an empty disk can lead to QEMU dumping core. Also fix a few braces here and there. Signed-off-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/ide/core.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index ce570a7ce5..4d568acc9c 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -1068,6 +1068,9 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) ide_set_signature(s); /* odd, but ATA4 8.27.5.2 requires it */ goto abort_cmd; } + if (!s->bs) { + goto abort_cmd; + } ide_cmd_lba48_transform(s, lba48); s->req_nb_sectors = 1; ide_sector_read(s); @@ -1078,6 +1081,9 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) case WIN_WRITE_ONCE: case CFA_WRITE_SECT_WO_ERASE: case WIN_WRITE_VERIFY: + if (!s->bs) { + goto abort_cmd; + } ide_cmd_lba48_transform(s, lba48); s->error = 0; s->status = SEEK_STAT | READY_STAT; @@ -1088,8 +1094,12 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) case WIN_MULTREAD_EXT: lba48 = 1; case WIN_MULTREAD: - if (!s->mult_sectors) + if (!s->bs) { goto abort_cmd; + } + if (!s->mult_sectors) { + goto abort_cmd; + } ide_cmd_lba48_transform(s, lba48); s->req_nb_sectors = s->mult_sectors; ide_sector_read(s); @@ -1098,8 +1108,12 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) lba48 = 1; case WIN_MULTWRITE: case CFA_WRITE_MULTI_WO_ERASE: - if (!s->mult_sectors) + if (!s->bs) { goto abort_cmd; + } + if (!s->mult_sectors) { + goto abort_cmd; + } ide_cmd_lba48_transform(s, lba48); s->error = 0; s->status = SEEK_STAT | READY_STAT; @@ -1114,8 +1128,9 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) lba48 = 1; case WIN_READDMA: case WIN_READDMA_ONCE: - if (!s->bs) + if (!s->bs) { goto abort_cmd; + } ide_cmd_lba48_transform(s, lba48); ide_sector_start_dma(s, IDE_DMA_READ); break; @@ -1123,8 +1138,9 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) lba48 = 1; case WIN_WRITEDMA: case WIN_WRITEDMA_ONCE: - if (!s->bs) + if (!s->bs) { goto abort_cmd; + } ide_cmd_lba48_transform(s, lba48); ide_sector_start_dma(s, IDE_DMA_WRITE); s->media_changed = 1; From 423477e5568c75e5b6e468c561e75417d8ae9585 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 17 Feb 2012 18:45:33 +0100 Subject: [PATCH 12/27] qcow2: Fix build with DEBUG_EXT enabled Signed-off-by: Kevin Wolf --- block/qcow2.c | 1 - 1 file changed, 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index 3692b4523b..dea12c1b24 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -89,7 +89,6 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, while (offset < end_offset) { #ifdef DEBUG_EXT - BDRVQcowState *s = bs->opaque; /* Sanity check */ if (offset > s->cluster_size) printf("qcow2_read_extension: suspicious offset %lu\n", offset); From 56116a14699b9d898a8d6581a58ec81e4abae6e6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 20 Feb 2012 17:58:34 +0100 Subject: [PATCH 13/27] block: remove unused fields in BlockDriverState sync_aiocb is unused since commit ce1a14d (Dynamically allocate AIO Completion Blocks., 2006-08-07). private is unused since commit 56a1493 (drive cleanup fixes., 2009-09-25). Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block_int.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/block_int.h b/block_int.h index 04f4b836ca..25aff8013f 100644 --- a/block_int.h +++ b/block_int.h @@ -259,10 +259,6 @@ struct BlockDriverState { /* number of in-flight copy-on-read requests */ unsigned int copy_on_read_in_flight; - /* async read/write emulation */ - - void *sync_aiocb; - /* the time for latest disk I/O */ int64_t slice_time; int64_t slice_start; @@ -299,7 +295,6 @@ struct BlockDriverState { int64_t dirty_count; int in_use; /* users other than guest access, eg. block migration */ QTAILQ_ENTRY(BlockDriverState) list; - void *private; QLIST_HEAD(, BdrvTrackedRequest) tracked_requests; From b6a127a156d5c26ea714493731db35a9491919da Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 21 Feb 2012 16:43:52 +0100 Subject: [PATCH 14/27] block: drop aio_multiwrite in BlockDriver These were never used. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block.c | 23 +++-------------------- block_int.h | 6 ------ 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/block.c b/block.c index 1150437b70..f23eccccd6 100644 --- a/block.c +++ b/block.c @@ -2791,7 +2791,6 @@ typedef struct MultiwriteCB { BlockDriverCompletionFunc *cb; void *opaque; QEMUIOVector *free_qiov; - void *free_buf; } callbacks[]; } MultiwriteCB; @@ -2805,7 +2804,6 @@ static void multiwrite_user_cb(MultiwriteCB *mcb) qemu_iovec_destroy(mcb->callbacks[i].free_qiov); } g_free(mcb->callbacks[i].free_qiov); - qemu_vfree(mcb->callbacks[i].free_buf); } } @@ -2862,20 +2860,11 @@ static int multiwrite_merge(BlockDriverState *bs, BlockRequest *reqs, int merge = 0; int64_t oldreq_last = reqs[outidx].sector + reqs[outidx].nb_sectors; - // This handles the cases that are valid for all block drivers, namely - // exactly sequential writes and overlapping writes. + // Handle exactly sequential writes and overlapping writes. if (reqs[i].sector <= oldreq_last) { merge = 1; } - // The block driver may decide that it makes sense to combine requests - // even if there is a gap of some sectors between them. In this case, - // the gap is filled with zeros (therefore only applicable for yet - // unused space in format like qcow2). - if (!merge && bs->drv->bdrv_merge_requests) { - merge = bs->drv->bdrv_merge_requests(bs, &reqs[outidx], &reqs[i]); - } - if (reqs[outidx].qiov->niov + reqs[i].qiov->niov + 1 > IOV_MAX) { merge = 0; } @@ -2891,14 +2880,8 @@ static int multiwrite_merge(BlockDriverState *bs, BlockRequest *reqs, size = (reqs[i].sector - reqs[outidx].sector) << 9; qemu_iovec_concat(qiov, reqs[outidx].qiov, size); - // We might need to add some zeros between the two requests - if (reqs[i].sector > oldreq_last) { - size_t zero_bytes = (reqs[i].sector - oldreq_last) << 9; - uint8_t *buf = qemu_blockalign(bs, zero_bytes); - memset(buf, 0, zero_bytes); - qemu_iovec_add(qiov, buf, zero_bytes); - mcb->callbacks[i].free_buf = buf; - } + // We should need to add any zeros between the two requests + assert (reqs[i].sector <= oldreq_last); // Add the second request qemu_iovec_concat(qiov, reqs[i].qiov, reqs[i].qiov->size); diff --git a/block_int.h b/block_int.h index 25aff8013f..bd86af00a2 100644 --- a/block_int.h +++ b/block_int.h @@ -162,12 +162,6 @@ struct BlockDriver { */ int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs); - int (*bdrv_aio_multiwrite)(BlockDriverState *bs, BlockRequest *reqs, - int num_reqs); - int (*bdrv_merge_requests)(BlockDriverState *bs, BlockRequest* a, - BlockRequest *b); - - const char *protocol_name; int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset); int64_t (*bdrv_getlength)(BlockDriverState *bs); From fd29b4bbef9f75bba64ad7c4db38babc397a4814 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 22 Feb 2012 12:31:47 +0100 Subject: [PATCH 15/27] qcow2: Fix offset in qcow2_read_extensions The spec says that the length of extensions is padded to 8 bytes, not the offset. Currently this is the same because the header size is a multiple of 8, so this is only about compatibility with future changes to the header size. While touching it, move the calculation to a common place instead of duplicating it for each header extension type. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- block/qcow2.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index dea12c1b24..f68f0e1074 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -126,7 +126,6 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, #ifdef DEBUG_EXT printf("Qcow2: Got format extension %s\n", bs->backing_format); #endif - offset = ((offset + ext.len + 7) & ~7); break; default: @@ -143,11 +142,11 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, if (ret < 0) { return ret; } - - offset = ((offset + ext.len + 7) & ~7); } break; } + + offset += ((ext.len + 7) & ~7); } return 0; From 64ca6aee4f06a3af869e5e09f0afeb6721966875 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 22 Feb 2012 12:37:13 +0100 Subject: [PATCH 16/27] qcow2: Reject too large header extensions Image files that make qemu-img info read several gigabytes into the unknown header extensions list are bad. Just fail opening the image if an extension claims to be larger than the header extension area. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- block/qcow2.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index f68f0e1074..eb5ea485d9 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -108,6 +108,11 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset, #ifdef DEBUG_EXT printf("ext.magic = 0x%x\n", ext.magic); #endif + if (ext.len > end_offset - offset) { + error_report("Header extension too large"); + return -EINVAL; + } + switch (ext.magic) { case QCOW2_EXT_MAGIC_END: return 0; From 8802d1fdd4b73e02ce13fb3a233c64c1913634ab Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Tue, 28 Feb 2012 15:54:06 -0500 Subject: [PATCH 17/27] qapi: Introduce blockdev-group-snapshot-sync command This is a QAPI/QMP only command to take a snapshot of a group of devices. This is similar to the blockdev-snapshot-sync command, except blockdev-group-snapshot-sync accepts a list devices, filenames, and formats. It is attempted to keep the snapshot of the group atomic; if the creation or open of any of the new snapshots fails, then all of the new snapshots are abandoned, and the name of the snapshot image that failed is returned. The failure case should not interrupt any operations. Rather than use bdrv_close() along with a subsequent bdrv_open() to perform the pivot, the original image is never closed and the new image is placed 'in front' of the original image via manipulation of the BlockDriverState fields. Thus, once the new snapshot image has been successfully created, there are no more failure points before pivoting to the new snapshot. This allows the group of disks to remain consistent with each other, even across snapshot failures. Signed-off-by: Jeff Cody Acked-by: Luiz Capitulino Signed-off-by: Kevin Wolf --- block.c | 81 +++++++++++++++++++++++++++++ block.h | 1 + block_int.h | 6 +++ blockdev.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++ qapi-schema.json | 38 ++++++++++++++ 5 files changed, 257 insertions(+) diff --git a/block.c b/block.c index f23eccccd6..52ffe1494a 100644 --- a/block.c +++ b/block.c @@ -882,6 +882,87 @@ void bdrv_make_anon(BlockDriverState *bs) bs->device_name[0] = '\0'; } +/* + * Add new bs contents at the top of an image chain while the chain is + * live, while keeping required fields on the top layer. + * + * This will modify the BlockDriverState fields, and swap contents + * between bs_new and bs_top. Both bs_new and bs_top are modified. + * + * This function does not create any image files. + */ +void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) +{ + BlockDriverState tmp; + + /* the new bs must not be in bdrv_states */ + bdrv_make_anon(bs_new); + + tmp = *bs_new; + + /* there are some fields that need to stay on the top layer: */ + + /* dev info */ + tmp.dev_ops = bs_top->dev_ops; + tmp.dev_opaque = bs_top->dev_opaque; + tmp.dev = bs_top->dev; + tmp.buffer_alignment = bs_top->buffer_alignment; + tmp.copy_on_read = bs_top->copy_on_read; + + /* i/o timing parameters */ + tmp.slice_time = bs_top->slice_time; + tmp.slice_start = bs_top->slice_start; + tmp.slice_end = bs_top->slice_end; + tmp.io_limits = bs_top->io_limits; + tmp.io_base = bs_top->io_base; + tmp.throttled_reqs = bs_top->throttled_reqs; + tmp.block_timer = bs_top->block_timer; + tmp.io_limits_enabled = bs_top->io_limits_enabled; + + /* geometry */ + tmp.cyls = bs_top->cyls; + tmp.heads = bs_top->heads; + tmp.secs = bs_top->secs; + tmp.translation = bs_top->translation; + + /* r/w error */ + tmp.on_read_error = bs_top->on_read_error; + tmp.on_write_error = bs_top->on_write_error; + + /* i/o status */ + tmp.iostatus_enabled = bs_top->iostatus_enabled; + tmp.iostatus = bs_top->iostatus; + + /* keep the same entry in bdrv_states */ + pstrcpy(tmp.device_name, sizeof(tmp.device_name), bs_top->device_name); + tmp.list = bs_top->list; + + /* The contents of 'tmp' will become bs_top, as we are + * swapping bs_new and bs_top contents. */ + tmp.backing_hd = bs_new; + pstrcpy(tmp.backing_file, sizeof(tmp.backing_file), bs_top->filename); + + /* swap contents of the fixed new bs and the current top */ + *bs_new = *bs_top; + *bs_top = tmp; + + /* clear the copied fields in the new backing file */ + bdrv_detach_dev(bs_new, bs_new->dev); + + qemu_co_queue_init(&bs_new->throttled_reqs); + memset(&bs_new->io_base, 0, sizeof(bs_new->io_base)); + memset(&bs_new->io_limits, 0, sizeof(bs_new->io_limits)); + bdrv_iostatus_disable(bs_new); + + /* we don't use bdrv_io_limits_disable() for this, because we don't want + * to affect or delete the block_timer, as it has been moved to bs_top */ + bs_new->io_limits_enabled = false; + bs_new->block_timer = NULL; + bs_new->slice_time = 0; + bs_new->slice_start = 0; + bs_new->slice_end = 0; +} + void bdrv_delete(BlockDriverState *bs) { assert(!bs->dev); diff --git a/block.h b/block.h index bbc38300e1..48d0bf3592 100644 --- a/block.h +++ b/block.h @@ -114,6 +114,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, int bdrv_create_file(const char* filename, QEMUOptionParameter *options); BlockDriverState *bdrv_new(const char *device_name); void bdrv_make_anon(BlockDriverState *bs); +void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top); void bdrv_delete(BlockDriverState *bs); int bdrv_parse_cache_flags(const char *mode, int *flags); int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags); diff --git a/block_int.h b/block_int.h index bd86af00a2..b460c369ca 100644 --- a/block_int.h +++ b/block_int.h @@ -221,6 +221,12 @@ struct BlockDriver { QLIST_ENTRY(BlockDriver) list; }; +/* + * Note: the function bdrv_append() copies and swaps contents of + * BlockDriverStates, so if you add new fields to this struct, please + * inspect bdrv_append() to determine if the new fields need to be + * copied as well. + */ struct BlockDriverState { int64_t total_sectors; /* if we are reading a disk image, give its size in sectors */ diff --git a/blockdev.c b/blockdev.c index 2c132a308b..d78aa51af5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -714,6 +714,137 @@ void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file, } } + +/* New and old BlockDriverState structs for group snapshots */ +typedef struct BlkGroupSnapshotStates { + BlockDriverState *old_bs; + BlockDriverState *new_bs; + QSIMPLEQ_ENTRY(BlkGroupSnapshotStates) entry; +} BlkGroupSnapshotStates; + +/* + * 'Atomic' group snapshots. The snapshots are taken as a set, and if any fail + * then we do not pivot any of the devices in the group, and abandon the + * snapshots + */ +void qmp_blockdev_group_snapshot_sync(SnapshotDevList *dev_list, + Error **errp) +{ + int ret = 0; + SnapshotDevList *dev_entry = dev_list; + SnapshotDev *dev_info = NULL; + BlkGroupSnapshotStates *states; + BlockDriver *proto_drv; + BlockDriver *drv; + int flags; + const char *format; + const char *snapshot_file; + + QSIMPLEQ_HEAD(snap_bdrv_states, BlkGroupSnapshotStates) snap_bdrv_states; + QSIMPLEQ_INIT(&snap_bdrv_states); + + /* drain all i/o before any snapshots */ + bdrv_drain_all(); + + /* We don't do anything in this loop that commits us to the snapshot */ + while (NULL != dev_entry) { + dev_info = dev_entry->value; + dev_entry = dev_entry->next; + + states = g_malloc0(sizeof(BlkGroupSnapshotStates)); + QSIMPLEQ_INSERT_TAIL(&snap_bdrv_states, states, entry); + + states->old_bs = bdrv_find(dev_info->device); + + if (!states->old_bs) { + error_set(errp, QERR_DEVICE_NOT_FOUND, dev_info->device); + goto delete_and_fail; + } + + if (bdrv_in_use(states->old_bs)) { + error_set(errp, QERR_DEVICE_IN_USE, dev_info->device); + goto delete_and_fail; + } + + if (!bdrv_is_read_only(states->old_bs) && + bdrv_is_inserted(states->old_bs)) { + + if (bdrv_flush(states->old_bs)) { + error_set(errp, QERR_IO_ERROR); + goto delete_and_fail; + } + } + + snapshot_file = dev_info->snapshot_file; + + flags = states->old_bs->open_flags; + + if (!dev_info->has_format) { + format = "qcow2"; + } else { + format = dev_info->format; + } + + drv = bdrv_find_format(format); + if (!drv) { + error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); + goto delete_and_fail; + } + + proto_drv = bdrv_find_protocol(snapshot_file); + if (!proto_drv) { + error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); + goto delete_and_fail; + } + + /* create new image w/backing file */ + ret = bdrv_img_create(snapshot_file, format, + states->old_bs->filename, + drv->format_name, NULL, -1, flags); + if (ret) { + error_set(errp, QERR_OPEN_FILE_FAILED, snapshot_file); + goto delete_and_fail; + } + + /* We will manually add the backing_hd field to the bs later */ + states->new_bs = bdrv_new(""); + ret = bdrv_open(states->new_bs, snapshot_file, + flags | BDRV_O_NO_BACKING, drv); + if (ret != 0) { + error_set(errp, QERR_OPEN_FILE_FAILED, snapshot_file); + goto delete_and_fail; + } + } + + + /* Now we are going to do the actual pivot. Everything up to this point + * is reversible, but we are committed at this point */ + QSIMPLEQ_FOREACH(states, &snap_bdrv_states, entry) { + /* This removes our old bs from the bdrv_states, and adds the new bs */ + bdrv_append(states->new_bs, states->old_bs); + } + + /* success */ + goto exit; + +delete_and_fail: + /* + * failure, and it is all-or-none; abandon each new bs, and keep using + * the original bs for all images + */ + QSIMPLEQ_FOREACH(states, &snap_bdrv_states, entry) { + if (states->new_bs) { + bdrv_delete(states->new_bs); + } + } +exit: + QSIMPLEQ_FOREACH(states, &snap_bdrv_states, entry) { + g_free(states); + } + return; +} + + static void eject_device(BlockDriverState *bs, int force, Error **errp) { if (bdrv_in_use(bs)) { diff --git a/qapi-schema.json b/qapi-schema.json index d0b6792e3c..5f293c4403 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1117,6 +1117,44 @@ ## { 'command': 'block_resize', 'data': { 'device': 'str', 'size': 'int' }} +## +# @SnapshotDev +# +# @device: the name of the device to generate the snapshot from. +# +# @snapshot-file: the target of the new image. A new file will be created. +# +# @format: #optional the format of the snapshot image, default is 'qcow2'. +## +{ 'type': 'SnapshotDev', + 'data': {'device': 'str', 'snapshot-file': 'str', '*format': 'str' } } + +## +# @blockdev-group-snapshot-sync +# +# Generates a synchronous snapshot of a group of one or more block devices, +# as atomically as possible. If the snapshot of any device in the group +# fails, then the entire group snapshot will be abandoned and the +# appropriate error returned. +# +# List of: +# @SnapshotDev: information needed for the device snapshot +# +# Returns: nothing on success +# If @device is not a valid block device, DeviceNotFound +# If @device is busy, DeviceInUse will be returned +# If @snapshot-file can't be created, OpenFileFailed +# If @snapshot-file can't be opened, OpenFileFailed +# If @format is invalid, InvalidBlockFormat +# +# Note: The group snapshot attempt returns failure on the first snapshot +# device failure. Therefore, there will be only one device or snapshot file +# returned in an error condition, and subsequent devices will not have been +# attempted. +## +{ 'command': 'blockdev-group-snapshot-sync', + 'data': { 'devlist': [ 'SnapshotDev' ] } } + ## # @blockdev-snapshot-sync # From c186402c444a9598a71da6aef1360f4171e57ad3 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Tue, 28 Feb 2012 15:54:07 -0500 Subject: [PATCH 18/27] QMP: Add qmp command for blockdev-group-snapshot-sync This adds the QMP command for blockdev-group-snapshot-sync. It takes an array in as the input, for the argument devlist. The array consists of the following elements: + device: device to snapshot. e.g. "ide-hd0", "virtio0" + snapshot-file: path & file for the snapshot image. e.g. "/tmp/file.img" + format: snapshot format. e.g., "qcow2". Optional There is no HMP equivalent for the command. Signed-off-by: Jeff Cody Acked-by: Luiz Capitulino Signed-off-by: Kevin Wolf --- qmp-commands.hx | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/qmp-commands.hx b/qmp-commands.hx index 705f704021..0c9bfac20d 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -686,6 +686,45 @@ EQMP .args_type = "device:B", .mhandler.cmd_new = qmp_marshal_input_block_job_cancel, }, + { + .name = "blockdev-group-snapshot-sync", + .args_type = "devlist:O", + .params = "device:B,snapshot-file:s,format:s?", + .mhandler.cmd_new = qmp_marshal_input_blockdev_group_snapshot_sync, + }, + +SQMP +blockdev-group-snapshot-sync +---------------------- + +Synchronous snapshot of one or more block devices. A list array input +is accepted, that contains the device and snapshot file information for +each device in group. The default format, if not specified, is qcow2. + +If there is any failure creating or opening a new snapshot, all snapshots +for the group are abandoned, and the original disks pre-snapshot attempt +are used. + + +Arguments: + +devlist array: + - "device": device name to snapshot (json-string) + - "snapshot-file": name of new image file (json-string) + - "format": format of new image (json-string, optional) + +Example: + +-> { "execute": "blockdev-group-snapshot-sync", "arguments": + { "devlist": [{ "device": "ide-hd0", + "snapshot-file": "/some/place/my-image", + "format": "qcow2" }, + { "device": "ide-hd1", + "snapshot-file": "/some/place/my-image2", + "format": "qcow2" }] } } +<- { "return": {} } + +EQMP { .name = "blockdev-snapshot-sync", From 049255b60c2bd95df09e3da9de0d9658b2ec4a98 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 29 Feb 2012 13:25:20 +0000 Subject: [PATCH 19/27] qemu-iotests: export TEST_DIR for non-bash tests Since qemu-iotests may need to create large image files it is possible to specify the test directory. The TEST_DIR variable needs to be exported so non-bash tests can make use of it. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/qemu-iotests/common.config | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index d07f435696..a220684b0f 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -121,6 +121,8 @@ if [ ! -d "$TEST_DIR" ]; then exit 1 fi +export TEST_DIR + _readlink() { if [ $# -ne 1 ]; then From f345cfd01935c6d7ab19f841a2eee2901ca3221e Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 29 Feb 2012 13:25:21 +0000 Subject: [PATCH 20/27] qemu-iotests: add iotests Python module Block layer tests that involve QMP commands rather than qemu-img or qemu-io are not well-suited for shell scripting. This patch adds a Python module which allows tests to be written in Python instead. The basic API is: VM - class for launching and interacting with a VM QMPTestCase - abstract base class for tests that use QMP qemu_img() - wrapper function for invoking qemu-img qemu_io() - wrapper function for invoking qemu-io imgfmt - the image format under test (e.g. qcow2, qed) test_dir - scratch directory path for temporary files main() - entry point for running tests Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/qemu-iotests/iotests.py | 164 ++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 tests/qemu-iotests/iotests.py diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py new file mode 100644 index 0000000000..ec1a86a632 --- /dev/null +++ b/tests/qemu-iotests/iotests.py @@ -0,0 +1,164 @@ +# Common utilities and Python wrappers for qemu-iotests +# +# Copyright (C) 2012 IBM Corp. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import os +import re +import subprocess +import unittest +import sys; sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'QMP')) +import qmp + +__all__ = ['imgfmt', 'imgproto', 'test_dir' 'qemu_img', 'qemu_io', + 'VM', 'QMPTestCase', 'notrun', 'main'] + +# This will not work if arguments or path contain spaces but is necessary if we +# want to support the override options that ./check supports. +qemu_img_args = os.environ.get('QEMU_IMG', 'qemu-img').split(' ') +qemu_io_args = os.environ.get('QEMU_IO', 'qemu-io').split(' ') +qemu_args = os.environ.get('QEMU', 'qemu').split(' ') + +imgfmt = os.environ.get('IMGFMT', 'raw') +imgproto = os.environ.get('IMGPROTO', 'file') +test_dir = os.environ.get('TEST_DIR', '/var/tmp') + +def qemu_img(*args): + '''Run qemu-img and return the exit code''' + devnull = open('/dev/null', 'r+') + return subprocess.call(qemu_img_args + list(args), stdin=devnull, stdout=devnull) + +def qemu_io(*args): + '''Run qemu-io and return the stdout data''' + args = qemu_io_args + list(args) + return subprocess.Popen(args, stdout=subprocess.PIPE).communicate()[0] + +class VM(object): + '''A QEMU VM''' + + def __init__(self): + self._monitor_path = os.path.join(test_dir, 'qemu-mon.%d' % os.getpid()) + self._qemu_log_path = os.path.join(test_dir, 'qemu-log.%d' % os.getpid()) + self._args = qemu_args + ['-chardev', + 'socket,id=mon,path=' + self._monitor_path, + '-mon', 'chardev=mon,mode=control', '-nographic'] + self._num_drives = 0 + + def add_drive(self, path, opts=''): + '''Add a virtio-blk drive to the VM''' + options = ['if=virtio', + 'format=%s' % imgfmt, + 'cache=none', + 'file=%s' % path, + 'id=drive%d' % self._num_drives] + if opts: + options.append(opts) + + self._args.append('-drive') + self._args.append(','.join(options)) + self._num_drives += 1 + return self + + def launch(self): + '''Launch the VM and establish a QMP connection''' + devnull = open('/dev/null', 'rb') + qemulog = open(self._qemu_log_path, 'wb') + try: + self._qmp = qmp.QEMUMonitorProtocol(self._monitor_path, server=True) + self._popen = subprocess.Popen(self._args, stdin=devnull, stdout=qemulog, + stderr=subprocess.STDOUT) + self._qmp.accept() + except: + os.remove(self._monitor_path) + raise + + def shutdown(self): + '''Terminate the VM and clean up''' + self._qmp.cmd('quit') + self._popen.wait() + os.remove(self._monitor_path) + os.remove(self._qemu_log_path) + + def qmp(self, cmd, **args): + '''Invoke a QMP command and return the result dict''' + return self._qmp.cmd(cmd, args=args) + + def get_qmp_events(self, wait=False): + '''Poll for queued QMP events and return a list of dicts''' + events = self._qmp.get_events(wait=wait) + self._qmp.clear_events() + return events + +index_re = re.compile(r'([^\[]+)\[([^\]]+)\]') + +class QMPTestCase(unittest.TestCase): + '''Abstract base class for QMP test cases''' + + def dictpath(self, d, path): + '''Traverse a path in a nested dict''' + for component in path.split('/'): + m = index_re.match(component) + if m: + component, idx = m.groups() + idx = int(idx) + + if not isinstance(d, dict) or component not in d: + self.fail('failed path traversal for "%s" in "%s"' % (path, str(d))) + d = d[component] + + if m: + if not isinstance(d, list): + self.fail('path component "%s" in "%s" is not a list in "%s"' % (component, path, str(d))) + try: + d = d[idx] + except IndexError: + self.fail('invalid index "%s" in path "%s" in "%s"' % (idx, path, str(d))) + return d + + def assert_qmp(self, d, path, value): + '''Assert that the value for a specific path in a QMP dict matches''' + result = self.dictpath(d, path) + self.assertEqual(result, value, 'values not equal "%s" and "%s"' % (str(result), str(value))) + +def notrun(reason): + '''Skip this test suite''' + # Each test in qemu-iotests has a number ("seq") + seq = os.path.basename(sys.argv[0]) + + open('%s.notrun' % seq, 'wb').write(reason + '\n') + print '%s not run: %s' % (seq, reason) + sys.exit(0) + +def main(supported_fmts=[]): + '''Run tests''' + + if supported_fmts and (imgfmt not in supported_fmts): + notrun('not suitable for this image format: %s' % imgfmt) + + # We need to filter out the time taken from the output so that qemu-iotest + # can reliably diff the results against master output. + import StringIO + output = StringIO.StringIO() + + class MyTestRunner(unittest.TextTestRunner): + def __init__(self, stream=output, descriptions=True, verbosity=1): + unittest.TextTestRunner.__init__(self, stream, descriptions, verbosity) + + # unittest.main() will use sys.exit() so expect a SystemExit exception + try: + unittest.main(testRunner=MyTestRunner) + finally: + sys.stderr.write(re.sub(r'Ran (\d+) test[s] in [\d.]+s', r'Ran \1 tests', output.getvalue())) From 37ce63eb23ad092e88891cf458bd3491af65d0a3 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 29 Feb 2012 13:25:22 +0000 Subject: [PATCH 21/27] test: add image streaming tests This patch adds a test suite for the image streaming feature. It exercises the 'block_stream', 'block_job_cancel', 'block_job_set_speed', and 'query-block-jobs' QMP commands. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/qemu-iotests/030 | 151 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/030.out | 5 ++ tests/qemu-iotests/group | 1 + 3 files changed, 157 insertions(+) create mode 100755 tests/qemu-iotests/030 create mode 100644 tests/qemu-iotests/030.out diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 new file mode 100755 index 0000000000..1faf984200 --- /dev/null +++ b/tests/qemu-iotests/030 @@ -0,0 +1,151 @@ +#!/usr/bin/env python +# +# Tests for image streaming. +# +# Copyright (C) 2012 IBM Corp. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import os +import iotests +from iotests import qemu_img, qemu_io + +backing_img = os.path.join(iotests.test_dir, 'backing.img') +test_img = os.path.join(iotests.test_dir, 'test.img') + +class ImageStreamingTestCase(iotests.QMPTestCase): + '''Abstract base class for image streaming test cases''' + + def assert_no_active_streams(self): + result = self.vm.qmp('query-block-jobs') + self.assert_qmp(result, 'return', []) + +class TestSingleDrive(ImageStreamingTestCase): + image_len = 1 * 1024 * 1024 # MB + + def setUp(self): + qemu_img('create', backing_img, str(TestSingleDrive.image_len)) + qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img) + self.vm = iotests.VM().add_drive(test_img) + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + os.remove(test_img) + os.remove(backing_img) + + def test_stream(self): + self.assert_no_active_streams() + + result = self.vm.qmp('block_stream', device='drive0') + self.assert_qmp(result, 'return', {}) + + completed = False + while not completed: + for event in self.vm.get_qmp_events(wait=True): + if event['event'] == 'BLOCK_JOB_COMPLETED': + self.assert_qmp(event, 'data/type', 'stream') + self.assert_qmp(event, 'data/device', 'drive0') + self.assert_qmp(event, 'data/offset', self.image_len) + self.assert_qmp(event, 'data/len', self.image_len) + completed = True + + self.assert_no_active_streams() + + self.assertFalse('sectors not allocated' in qemu_io('-c', 'map', test_img), + 'image file not fully populated after streaming') + + def test_device_not_found(self): + result = self.vm.qmp('block_stream', device='nonexistent') + self.assert_qmp(result, 'error/class', 'DeviceNotFound') + +class TestStreamStop(ImageStreamingTestCase): + image_len = 8 * 1024 * 1024 * 1024 # GB + + def setUp(self): + qemu_img('create', backing_img, str(TestStreamStop.image_len)) + qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img) + self.vm = iotests.VM().add_drive(test_img) + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + os.remove(test_img) + os.remove(backing_img) + + def test_stream_stop(self): + import time + + self.assert_no_active_streams() + + result = self.vm.qmp('block_stream', device='drive0') + self.assert_qmp(result, 'return', {}) + + time.sleep(1) + events = self.vm.get_qmp_events(wait=False) + self.assertEqual(events, [], 'unexpected QMP event: %s' % events) + + self.vm.qmp('block_job_cancel', device='drive0') + self.assert_qmp(result, 'return', {}) + + cancelled = False + while not cancelled: + for event in self.vm.get_qmp_events(wait=True): + if event['event'] == 'BLOCK_JOB_CANCELLED': + self.assert_qmp(event, 'data/type', 'stream') + self.assert_qmp(event, 'data/device', 'drive0') + cancelled = True + + self.assert_no_active_streams() + +# This is a short performance test which is not run by default. +# Invoke "IMGFMT=qed ./030 TestSetSpeed.perf_test_set_speed" +class TestSetSpeed(ImageStreamingTestCase): + image_len = 80 * 1024 * 1024 # MB + + def setUp(self): + qemu_img('create', backing_img, str(TestSetSpeed.image_len)) + qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img) + self.vm = iotests.VM().add_drive(test_img) + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + os.remove(test_img) + os.remove(backing_img) + + def perf_test_set_speed(self): + self.assert_no_active_streams() + + result = self.vm.qmp('block_stream', device='drive0') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('block_job_set_speed', device='drive0', value=8 * 1024 * 1024) + self.assert_qmp(result, 'return', {}) + + completed = False + while not completed: + for event in self.vm.get_qmp_events(wait=True): + if event['event'] == 'BLOCK_JOB_COMPLETED': + self.assert_qmp(event, 'data/type', 'stream') + self.assert_qmp(event, 'data/device', 'drive0') + self.assert_qmp(event, 'data/offset', self.image_len) + self.assert_qmp(event, 'data/len', self.image_len) + completed = True + + self.assert_no_active_streams() + +if __name__ == '__main__': + iotests.main(supported_fmts=['qcow2', 'qed']) diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out new file mode 100644 index 0000000000..8d7e996700 --- /dev/null +++ b/tests/qemu-iotests/030.out @@ -0,0 +1,5 @@ +... +---------------------------------------------------------------------- +Ran 3 tests + +OK diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 0a5c866014..fcf869d36e 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -36,3 +36,4 @@ 027 rw auto 028 rw backing auto 029 rw auto +030 rw auto From a06d5cc20b574e24630d9e59bf2a707728bc3101 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 29 Apr 2011 15:30:25 +0200 Subject: [PATCH 22/27] qemu-iotests: Filter out DOS line endings This one makes it possible to run qemu-iotests on a Windows build using Wine and get somewhat meaningful results. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- tests/qemu-iotests/common.filter | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index da77ede25f..fa26b62dd3 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -140,10 +140,16 @@ _filter_imgfmt() sed -e "s#$IMGFMT#IMGFMT#g" } +# Removes \r from messages +_filter_win32() +{ + sed -e 's/\r//g' +} + # sanitize qemu-io output _filter_qemu_io() { - sed -e "s/[0-9]* ops\; [0-9/:. sec]* ([0-9/.inf]* [EPTGMKiBbytes]*\/sec and [0-9/.inf]* ops\/sec)/X ops\; XX:XX:XX.X (XXX YYY\/sec and XXX ops\/sec)/" + _filter_win32 | sed -e "s/[0-9]* ops\; [0-9/:. sec]* ([0-9/.inf]* [EPTGMKiBbytes]*\/sec and [0-9/.inf]* ops\/sec)/X ops\; XX:XX:XX.X (XXX YYY\/sec and XXX ops\/sec)/" } # make sure this script returns success From 92ab69b61fe04d811378ae4eda9799a5ad6ddb14 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 29 Apr 2011 15:32:55 +0200 Subject: [PATCH 23/27] qemu-iotests: 026: Reduce output changes for cache=none qcow2 qemu-iotests supports the -nocache option which makes the tests run with cache=none. For blkdebug tests with qcow2 this means that we may see test results that differ from cache=writethrough. This patch makes the diff a bit smaller and therefore easier to review. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- tests/qemu-iotests/026 | 6 ++++++ tests/qemu-iotests/check | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026 index 33e7da6abe..1602ccd2a5 100755 --- a/tests/qemu-iotests/026 +++ b/tests/qemu-iotests/026 @@ -87,6 +87,12 @@ _make_test_img 1G echo echo "Event: $event; errno: $errno; imm: $imm; once: $once; write $vmstate" + +# We want to catch a simple L2 update, not the allocation of the first L2 table +if [ "$event" == "l2_update" ]; then + $QEMU_IO -c "write $vmstate 0 512" $TEST_IMG > /dev/null 2>&1 +fi + $QEMU_IO -c "write $vmstate 0 128k " $BLKDBG_TEST_IMG | _filter_qemu_io # l2_load is not called on allocation, so issue a second write diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 8499a04d3e..aae1378998 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -243,7 +243,7 @@ do echo " - no qualified output" err=true else - if diff $seq.out $tmp.out >/dev/null 2>&1 + if diff -w $seq.out $tmp.out >/dev/null 2>&1 then echo "" if $err @@ -255,7 +255,7 @@ do else echo " - output mismatch (see $seq.out.bad)" mv $tmp.out $seq.out.bad - $diff $seq.out $seq.out.bad + $diff -w $seq.out $seq.out.bad err=true fi fi From 6ce2d77abeb7dece24612e884a74d149cdf52376 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 15 Feb 2012 15:03:25 +0100 Subject: [PATCH 24/27] qemu-iotests: Test rebase with short backing file This tests that qemu-img rebase doesn't assume that the backing file has the same size as the image, but considers that it can be smaller. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- tests/qemu-iotests/028 | 5 +++++ tests/qemu-iotests/028.out | 1 + 2 files changed, 6 insertions(+) diff --git a/tests/qemu-iotests/028 b/tests/qemu-iotests/028 index 07c5bb6975..b091ba9f07 100755 --- a/tests/qemu-iotests/028 +++ b/tests/qemu-iotests/028 @@ -96,6 +96,11 @@ io_zero readv $(( offset + 32 * 1024 )) 512 1024 32 _check_test_img +# Rebase it on top of its base image +$QEMU_IMG rebase -b $TEST_IMG.base $TEST_IMG + +_check_test_img + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/028.out b/tests/qemu-iotests/028.out index f4290298c8..fe007887e3 100644 --- a/tests/qemu-iotests/028.out +++ b/tests/qemu-iotests/028.out @@ -465,4 +465,5 @@ qemu-io> read 512/512 bytes at offset 3221257728 qemu-io> read 512/512 bytes at offset 3221258752 512.000000 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) qemu-io> No errors were found on the image. +No errors were found on the image. *** done From 4889978e94bd16c5374f473d57f5e757fbe72485 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 29 Feb 2012 14:41:32 +0000 Subject: [PATCH 25/27] qemu-tool: revert cpu_get_clock() abort(3) Despite the fact that the qemu-tool environment has no guest running and vm_clock therefore does not make sense, there is code that gets the vm_clock time even in qemu-tool. Therefore, revert the abort(3) call and just return 0 like we used to. This unbreaks qemu-img/qemu-io with QED and Kevin has also expressed interest in this for qcow2. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- qemu-tool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-tool.c b/qemu-tool.c index 183a583fec..edb84f5f5d 100644 --- a/qemu-tool.c +++ b/qemu-tool.c @@ -61,7 +61,7 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) int64_t cpu_get_clock(void) { - abort(); + return 0; } int64_t cpu_get_icount(void) From a57d114389bdf8ce493456f85f2ec2229f7d0c00 Mon Sep 17 00:00:00 2001 From: Zhi Yong Wu Date: Sun, 19 Feb 2012 22:24:59 +0800 Subject: [PATCH 26/27] qemu-io: fix segment fault when the image format is qed [root@f15 qemu]# qemu-io -c info /home/zwu/work/misc/rh6.img format name: qed cluster size: 64 KiB vm state offset: 0.000000 bytes Segmentation fault (core dumped) This reason is same as the former patch Signed-off-by: Zhi Yong Wu Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- qemu-io.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qemu-io.c b/qemu-io.c index 0249be4e71..31895305f1 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -1856,6 +1856,8 @@ int main(int argc, char **argv) bdrv_init(); + qemu_init_main_loop(); + /* initialize commands */ quit_init(); help_init(); From 67d384e80471e91aa490d4573cac5a46f92f685f Mon Sep 17 00:00:00 2001 From: Zhi Yong Wu Date: Sun, 19 Feb 2012 22:24:35 +0800 Subject: [PATCH 27/27] qemu-img: fix segment fault when the image format is qed [root@f15 qemu]# qemu-img info /home/zwu/work/misc/rh6.img image: /home/zwu/work/misc/rh6.img file format: qed virtual size: 4.0G (4294967296 bytes) disk size: 1.2G cluster_size: 65536 Segmentation fault (core dumped) Today when i were fixing another issue, i found this issue; After simple investigation, i found that the required clock vm_clock is not created for qemu tool. Signed-off-by: Zhi Yong Wu Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- qemu-img.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qemu-img.c b/qemu-img.c index c4bcf41e15..8df35648e9 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1655,6 +1655,8 @@ int main(int argc, char **argv) cmdname = argv[1]; argc--; argv++; + qemu_init_main_loop(); + /* find the command */ for(cmd = img_cmds; cmd->name != NULL; cmd++) { if (!strcmp(cmdname, cmd->name)) {