From 5839e53bbc0fec56021d758aab7610df421ed8c8 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 19 Aug 2014 10:31:08 +0200 Subject: block: Use g_new() & friends where that makes obvious sense g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer, for two reasons. One, it catches multiplication overflowing size_t. Two, it returns T * rather than void *, which lets the compiler catch more type errors. Patch created with Coccinelle, with two manual changes on top: * Add const to bdrv_iterate_format() to keep the types straight * Convert the allocation in bdrv_drop_intermediate(), which Coccinelle inexplicably misses Coccinelle semantic patch: @@ type T; @@ -g_malloc(sizeof(T)) +g_new(T, 1) @@ type T; @@ -g_try_malloc(sizeof(T)) +g_try_new(T, 1) @@ type T; @@ -g_malloc0(sizeof(T)) +g_new0(T, 1) @@ type T; @@ -g_try_malloc0(sizeof(T)) +g_try_new0(T, 1) @@ type T; expression n; @@ -g_malloc(sizeof(T) * (n)) +g_new(T, n) @@ type T; expression n; @@ -g_try_malloc(sizeof(T) * (n)) +g_try_new(T, n) @@ type T; expression n; @@ -g_malloc0(sizeof(T) * (n)) +g_new0(T, n) @@ type T; expression n; @@ -g_try_malloc0(sizeof(T) * (n)) +g_try_new0(T, n) @@ type T; expression p, n; @@ -g_realloc(p, sizeof(T) * (n)) +g_renew(T, p, n) @@ type T; expression p, n; @@ -g_try_realloc(p, sizeof(T) * (n)) +g_try_renew(T, p, n) Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Jeff Cody Signed-off-by: Kevin Wolf --- block-migration.c | 6 +++--- block.c | 14 +++++++------- block/archipelago.c | 6 +++--- block/gluster.c | 8 ++++---- block/iscsi.c | 2 +- block/nfs.c | 2 +- block/qcow.c | 2 +- block/qcow2-cluster.c | 2 +- block/qcow2-refcount.c | 8 ++++---- block/qcow2-snapshot.c | 8 ++++---- block/raw-posix.c | 2 +- block/rbd.c | 4 ++-- block/sheepdog.c | 4 ++-- block/vdi.c | 2 +- block/vhdx.c | 4 ++-- block/vmdk.c | 7 +++---- block/vvfat.c | 2 +- blockdev-nbd.c | 2 +- blockdev.c | 2 +- hw/ide/ahci.c | 2 +- qemu-io-cmds.c | 2 +- qemu-io.c | 2 +- 22 files changed, 46 insertions(+), 47 deletions(-) diff --git a/block-migration.c b/block-migration.c index ba3ed36f77..3ad31a2c70 100644 --- a/block-migration.c +++ b/block-migration.c @@ -283,7 +283,7 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds) nr_sectors = total_sectors - cur_sector; } - blk = g_malloc(sizeof(BlkMigBlock)); + blk = g_new(BlkMigBlock, 1); blk->buf = g_malloc(BLOCK_SIZE); blk->bmds = bmds; blk->sector = cur_sector; @@ -354,7 +354,7 @@ static void init_blk_migration_it(void *opaque, BlockDriverState *bs) return; } - bmds = g_malloc0(sizeof(BlkMigDevState)); + bmds = g_new0(BlkMigDevState, 1); bmds->bs = bs; bmds->bulk_completed = 0; bmds->total_sectors = sectors; @@ -465,7 +465,7 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds, } else { nr_sectors = BDRV_SECTORS_PER_DIRTY_CHUNK; } - blk = g_malloc(sizeof(BlkMigBlock)); + blk = g_new(BlkMigBlock, 1); blk->buf = g_malloc(BLOCK_SIZE); blk->bmds = bmds; blk->sector = sector; diff --git a/block.c b/block.c index 6fa0201074..712f5db758 100644 --- a/block.c +++ b/block.c @@ -351,7 +351,7 @@ BlockDriverState *bdrv_new(const char *device_name, Error **errp) return NULL; } - bs = g_malloc0(sizeof(BlockDriverState)); + bs = g_new0(BlockDriverState, 1); QLIST_INIT(&bs->dirty_bitmaps); pstrcpy(bs->device_name, sizeof(bs->device_name), device_name); if (device_name[0] != '\0') { @@ -2628,7 +2628,7 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, * into our deletion queue, until we hit the 'base' */ while (intermediate) { - intermediate_state = g_malloc0(sizeof(BlkIntermediateStates)); + intermediate_state = g_new0(BlkIntermediateStates, 1); intermediate_state->bs = intermediate; QSIMPLEQ_INSERT_TAIL(&states_to_delete, intermediate_state, entry); @@ -3755,7 +3755,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name), } if (!found) { - formats = g_realloc(formats, (count + 1) * sizeof(char *)); + formats = g_renew(const char *, formats, count + 1); formats[count++] = drv->format_name; it(opaque, drv->format_name); } @@ -5330,7 +5330,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity, errno = -bitmap_size; return NULL; } - bitmap = g_malloc0(sizeof(BdrvDirtyBitmap)); + bitmap = g_new0(BdrvDirtyBitmap, 1); bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1); QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list); return bitmap; @@ -5356,8 +5356,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) BlockDirtyInfoList **plist = &list; QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) { - BlockDirtyInfo *info = g_malloc0(sizeof(BlockDirtyInfo)); - BlockDirtyInfoList *entry = g_malloc0(sizeof(BlockDirtyInfoList)); + BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1); + BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1); info->count = bdrv_get_dirty_count(bs, bm); info->granularity = ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap)); @@ -5451,7 +5451,7 @@ void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason) BdrvOpBlocker *blocker; assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX); - blocker = g_malloc0(sizeof(BdrvOpBlocker)); + blocker = g_new0(BdrvOpBlocker, 1); blocker->reason = reason; QLIST_INSERT_HEAD(&bs->op_blockers[op], blocker, list); } diff --git a/block/archipelago.c b/block/archipelago.c index 6629d03a1d..34f72dc5a5 100644 --- a/block/archipelago.c +++ b/block/archipelago.c @@ -750,7 +750,7 @@ static int archipelago_submit_request(BDRVArchipelagoState *s, char *target; void *data = NULL; struct xseg_request *req; - AIORequestData *reqdata = g_malloc(sizeof(AIORequestData)); + AIORequestData *reqdata = g_new(AIORequestData, 1); targetlen = strlen(s->volname); req = xseg_get_request(s->xseg, s->srcport, s->vportno, X_ALLOC); @@ -827,7 +827,7 @@ static int archipelago_aio_segmented_rw(BDRVArchipelagoState *s, int i, ret, segments_nr, last_segment_size; ArchipelagoSegmentedRequest *segreq; - segreq = g_malloc(sizeof(ArchipelagoSegmentedRequest)); + segreq = g_new(ArchipelagoSegmentedRequest, 1); if (op == ARCHIP_OP_FLUSH) { segments_nr = 1; @@ -960,7 +960,7 @@ static int64_t archipelago_volume_info(BDRVArchipelagoState *s) int ret, targetlen; struct xseg_request *req; struct xseg_reply_info *xinfo; - AIORequestData *reqdata = g_malloc(sizeof(AIORequestData)); + AIORequestData *reqdata = g_new(AIORequestData, 1); const char *volname = s->volname; targetlen = strlen(volname); diff --git a/block/gluster.c b/block/gluster.c index 9274dead7d..1912cf9d07 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -291,7 +291,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options, BDRVGlusterState *s = bs->opaque; int open_flags = 0; int ret = 0; - GlusterConf *gconf = g_malloc0(sizeof(GlusterConf)); + GlusterConf *gconf = g_new0(GlusterConf, 1); QemuOpts *opts; Error *local_err = NULL; const char *filename; @@ -351,12 +351,12 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state, assert(state != NULL); assert(state->bs != NULL); - state->opaque = g_malloc0(sizeof(BDRVGlusterReopenState)); + state->opaque = g_new0(BDRVGlusterReopenState, 1); reop_s = state->opaque; qemu_gluster_parse_flags(state->flags, &open_flags); - gconf = g_malloc0(sizeof(GlusterConf)); + gconf = g_new0(GlusterConf, 1); reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, errp); if (reop_s->glfs == NULL) { @@ -486,7 +486,7 @@ static int qemu_gluster_create(const char *filename, int prealloc = 0; int64_t total_size = 0; char *tmp = NULL; - GlusterConf *gconf = g_malloc0(sizeof(GlusterConf)); + GlusterConf *gconf = g_new0(GlusterConf, 1); glfs = qemu_gluster_init(gconf, filename, errp); if (!glfs) { diff --git a/block/iscsi.c b/block/iscsi.c index 2c9cfc18eb..cdd19c2bda 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1532,7 +1532,7 @@ static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp) /* Read out options */ total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; - bs->opaque = g_malloc0(sizeof(struct IscsiLun)); + bs->opaque = g_new0(struct IscsiLun, 1); iscsilun = bs->opaque; bs_options = qdict_new(); diff --git a/block/nfs.c b/block/nfs.c index fe46c33709..93d87f3256 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -409,7 +409,7 @@ static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp) { int ret = 0; int64_t total_size = 0; - NFSClient *client = g_malloc0(sizeof(NFSClient)); + NFSClient *client = g_new0(NFSClient, 1); client->aio_context = qemu_get_aio_context(); diff --git a/block/qcow.c b/block/qcow.c index 67332f03c1..67c237fe7d 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -182,7 +182,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, } s->l1_table_offset = header.l1_table_offset; - s->l1_table = g_try_malloc(s->l1_size * sizeof(uint64_t)); + s->l1_table = g_try_new(uint64_t, s->l1_size); if (s->l1_table == NULL) { error_setg(errp, "Could not allocate memory for L1 table"); ret = -ENOMEM; diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 5b36018b3e..735f687b06 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -711,7 +711,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m) trace_qcow2_cluster_link_l2(qemu_coroutine_self(), m->nb_clusters); assert(m->nb_clusters > 0); - old_cluster = g_try_malloc(m->nb_clusters * sizeof(uint64_t)); + old_cluster = g_try_new(uint64_t, m->nb_clusters); if (old_cluster == NULL) { ret = -ENOMEM; goto err; diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 3b7747048e..43665b86e7 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -350,7 +350,7 @@ static int alloc_refcount_block(BlockDriverState *bs, uint64_t meta_offset = (blocks_used * refcount_block_clusters) * s->cluster_size; uint64_t table_offset = meta_offset + blocks_clusters * s->cluster_size; - uint64_t *new_table = g_try_malloc0(table_size * sizeof(uint64_t)); + uint64_t *new_table = g_try_new0(uint64_t, table_size); uint16_t *new_blocks = g_try_malloc0(blocks_clusters * s->cluster_size); assert(table_size > 0 && blocks_clusters > 0); @@ -1524,7 +1524,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, return -EFBIG; } - refcount_table = g_try_malloc0(nb_clusters * sizeof(uint16_t)); + refcount_table = g_try_new0(uint16_t, nb_clusters); if (nb_clusters && refcount_table == NULL) { res->check_errors++; return -ENOMEM; @@ -1605,8 +1605,8 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, /* increase refcount_table size if necessary */ int old_nb_clusters = nb_clusters; nb_clusters = (new_offset >> s->cluster_bits) + 1; - refcount_table = g_realloc(refcount_table, - nb_clusters * sizeof(uint16_t)); + refcount_table = g_renew(uint16_t, refcount_table, + nb_clusters); memset(&refcount_table[old_nb_clusters], 0, (nb_clusters - old_nb_clusters) * sizeof(uint16_t)); } diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index f67b47282f..f52d7fdd22 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -58,7 +58,7 @@ int qcow2_read_snapshots(BlockDriverState *bs) } offset = s->snapshots_offset; - s->snapshots = g_malloc0(s->nb_snapshots * sizeof(QCowSnapshot)); + s->snapshots = g_new0(QCowSnapshot, s->nb_snapshots); for(i = 0; i < s->nb_snapshots; i++) { /* Read statically sized part of the snapshot header */ @@ -381,7 +381,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) sn->l1_table_offset = l1_table_offset; sn->l1_size = s->l1_size; - l1_table = g_try_malloc(s->l1_size * sizeof(uint64_t)); + l1_table = g_try_new(uint64_t, s->l1_size); if (s->l1_size && l1_table == NULL) { ret = -ENOMEM; goto fail; @@ -417,7 +417,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) } /* Append the new snapshot to the snapshot list */ - new_snapshot_list = g_malloc((s->nb_snapshots + 1) * sizeof(QCowSnapshot)); + new_snapshot_list = g_new(QCowSnapshot, s->nb_snapshots + 1); if (s->snapshots) { memcpy(new_snapshot_list, s->snapshots, s->nb_snapshots * sizeof(QCowSnapshot)); @@ -661,7 +661,7 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) return s->nb_snapshots; } - sn_tab = g_malloc0(s->nb_snapshots * sizeof(QEMUSnapshotInfo)); + sn_tab = g_new0(QEMUSnapshotInfo, s->nb_snapshots); for(i = 0; i < s->nb_snapshots; i++) { sn_info = sn_tab + i; sn = s->snapshots + i; diff --git a/block/raw-posix.c b/block/raw-posix.c index 1194eb00ad..5c745b9a18 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -517,7 +517,7 @@ static int raw_reopen_prepare(BDRVReopenState *state, s = state->bs->opaque; - state->opaque = g_malloc0(sizeof(BDRVRawReopenState)); + state->opaque = g_new0(BDRVRawReopenState, 1); raw_s = state->opaque; #ifdef CONFIG_LINUX_AIO diff --git a/block/rbd.c b/block/rbd.c index 4459102adf..3aaf8559dd 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -652,7 +652,7 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs, off = sector_num * BDRV_SECTOR_SIZE; size = nb_sectors * BDRV_SECTOR_SIZE; - rcb = g_malloc(sizeof(RADOSCB)); + rcb = g_new(RADOSCB, 1); rcb->done = 0; rcb->acb = acb; rcb->buf = buf; @@ -873,7 +873,7 @@ static int qemu_rbd_snap_list(BlockDriverState *bs, goto done; } - sn_tab = g_malloc0(snap_count * sizeof(QEMUSnapshotInfo)); + sn_tab = g_new0(QEMUSnapshotInfo, snap_count); for (i = 0; i < snap_count; i++) { const char *snap_name = snaps[i].name; diff --git a/block/sheepdog.c b/block/sheepdog.c index 8d9350c26d..ba1ef43502 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1682,7 +1682,7 @@ static int sd_create(const char *filename, QemuOpts *opts, uint32_t snapid; bool prealloc = false; - s = g_malloc0(sizeof(BDRVSheepdogState)); + s = g_new0(BDRVSheepdogState, 1); memset(tag, 0, sizeof(tag)); if (strstr(filename, "://")) { @@ -2273,7 +2273,7 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) uint32_t snapid = 0; int ret = 0; - old_s = g_malloc(sizeof(BDRVSheepdogState)); + old_s = g_new(BDRVSheepdogState, 1); memcpy(old_s, s, sizeof(BDRVSheepdogState)); diff --git a/block/vdi.c b/block/vdi.c index adc6aa9a5f..4b10aacc3b 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -292,7 +292,7 @@ static int vdi_check(BlockDriverState *bs, BdrvCheckResult *res, return -ENOTSUP; } - bmap = g_try_malloc(s->header.blocks_in_image * sizeof(uint32_t)); + bmap = g_try_new(uint32_t, s->header.blocks_in_image); if (s->header.blocks_in_image && bmap == NULL) { res->check_errors++; return -ENOMEM; diff --git a/block/vhdx.c b/block/vhdx.c index f666940db7..87c99fc260 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1381,7 +1381,7 @@ static int vhdx_create_new_headers(BlockDriverState *bs, uint64_t image_size, int ret = 0; VHDXHeader *hdr = NULL; - hdr = g_malloc0(sizeof(VHDXHeader)); + hdr = g_new0(VHDXHeader, 1); hdr->signature = VHDX_HEADER_SIGNATURE; hdr->sequence_number = g_random_int(); @@ -1654,7 +1654,7 @@ static int vhdx_create_new_region_table(BlockDriverState *bs, /* Populate enough of the BDRVVHDXState to be able to use the * pre-existing BAT calculation, translation, and update functions */ - s = g_malloc0(sizeof(BDRVVHDXState)); + s = g_new0(BDRVVHDXState, 1); s->chunk_ratio = (VHDX_MAX_SECTORS_PER_BLOCK) * (uint64_t) sector_size / (uint64_t) block_size; diff --git a/block/vmdk.c b/block/vmdk.c index 01412a8939..f000d2a613 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -233,7 +233,7 @@ static void vmdk_free_last_extent(BlockDriverState *bs) return; } s->num_extents--; - s->extents = g_realloc(s->extents, s->num_extents * sizeof(VmdkExtent)); + s->extents = g_renew(VmdkExtent, s->extents, s->num_extents); } static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent) @@ -418,8 +418,7 @@ static int vmdk_add_extent(BlockDriverState *bs, return length; } - s->extents = g_realloc(s->extents, - (s->num_extents + 1) * sizeof(VmdkExtent)); + s->extents = g_renew(VmdkExtent, s->extents, s->num_extents + 1); extent = &s->extents[s->num_extents]; s->num_extents++; @@ -497,7 +496,7 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent, } extent->l2_cache = - g_malloc(extent->l2_size * L2_CACHE_SIZE * sizeof(uint32_t)); + g_new(uint32_t, extent->l2_size * L2_CACHE_SIZE); return 0; fail_l1b: g_free(extent->l1_backup_table); diff --git a/block/vvfat.c b/block/vvfat.c index 70176b1619..f877e851d4 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2950,7 +2950,7 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp) bdrv_set_backing_hd(s->bs, bdrv_new("", &error_abort)); s->bs->backing_hd->drv = &vvfat_write_target; - s->bs->backing_hd->opaque = g_malloc(sizeof(void*)); + s->bs->backing_hd->opaque = g_new(void *, 1); *(void**)s->bs->backing_hd->opaque = s; return 0; diff --git a/blockdev-nbd.c b/blockdev-nbd.c index b3a24740b2..06f901ef6f 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -108,7 +108,7 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable, nbd_export_set_name(exp, device); - n = g_malloc0(sizeof(NBDCloseNotifier)); + n = g_new0(NBDCloseNotifier, 1); n->n.notify = nbd_close_notifier; n->exp = exp; bdrv_add_close_notifier(bs, &n->n); diff --git a/blockdev.c b/blockdev.c index 48bd9a37bc..8acd264407 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1094,7 +1094,7 @@ SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device, return NULL; } - info = g_malloc0(sizeof(SnapshotInfo)); + info = g_new0(SnapshotInfo, 1); info->id = g_strdup(sn.id_str); info->name = g_strdup(sn.name); info->date_nsec = sn.date_nsec; diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 932b0d508c..0ee713b0ff 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1203,7 +1203,7 @@ void ahci_init(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports) s->as = as; s->ports = ports; - s->dev = g_malloc0(sizeof(AHCIDevice) * ports); + s->dev = g_new0(AHCIDevice, ports); ahci_reg_init(s); /* XXX BAR size should be 1k, but that breaks, so bump it to 4k for now */ memory_region_init_io(&s->mem, OBJECT(qdev), &ahci_mem_ops, s, diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index c503fc66aa..3a1e11e1ea 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -114,7 +114,7 @@ static char **breakline(char *input, int *count) { int c = 0; char *p; - char **rval = g_malloc0(sizeof(char *)); + char **rval = g_new0(char *, 1); char **tmp; while (rval && (p = qemu_strsep(&input, " ")) != NULL) { diff --git a/qemu-io.c b/qemu-io.c index b55a550f27..33c96c4c1c 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -356,7 +356,7 @@ static void command_loop(void) static void add_user_command(char *optarg) { - cmdline = g_realloc(cmdline, ++ncmdline * sizeof(char *)); + cmdline = g_renew(char *, cmdline, ++ncmdline); cmdline[ncmdline-1] = optarg; } -- cgit v1.2.3 From 02c4f26b1517d9e403ec10d6f6ca3c0276d19e43 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 19 Aug 2014 10:31:09 +0200 Subject: block: Use g_new() & friends to avoid multiplying sizes g_new(T, n) is safer than g_malloc(sizeof(*v) * n) for two reasons. One, it catches multiplication overflowing size_t. Two, it returns T * rather than void *, which lets the compiler catch more type errors. Perhaps a conversion to g_malloc_n() would be neater in places, but that's merely four years old, and we can't use such newfangled stuff. This commit only touches allocations with size arguments of the form sizeof(T), plus two that use 4 instead of sizeof(uint32_t). We can make the others safe by converting to g_malloc_n() when it becomes available to us in a couple of years. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Jeff Cody Signed-off-by: Kevin Wolf --- block/bochs.c | 2 +- block/parallels.c | 2 +- block/qcow2-cache.c | 2 +- block/qed-check.c | 3 +-- block/rbd.c | 2 +- block/sheepdog.c | 2 +- hw/block/nvme.c | 8 ++++---- qemu-io-cmds.c | 10 +++++----- 8 files changed, 15 insertions(+), 16 deletions(-) diff --git a/block/bochs.c b/block/bochs.c index 6674b27438..199ac2b9af 100644 --- a/block/bochs.c +++ b/block/bochs.c @@ -131,7 +131,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags, return -EFBIG; } - s->catalog_bitmap = g_try_malloc(s->catalog_size * 4); + s->catalog_bitmap = g_try_new(uint32_t, s->catalog_size); if (s->catalog_size && s->catalog_bitmap == NULL) { error_setg(errp, "Could not allocate memory for catalog"); return -ENOMEM; diff --git a/block/parallels.c b/block/parallels.c index 1774ab8e8e..2a814f3db4 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -121,7 +121,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, ret = -EFBIG; goto fail; } - s->catalog_bitmap = g_try_malloc(s->catalog_size * 4); + s->catalog_bitmap = g_try_new(uint32_t, s->catalog_size); if (s->catalog_size && s->catalog_bitmap == NULL) { ret = -ENOMEM; goto fail; diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index 5353b44828..fe0615a995 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -50,7 +50,7 @@ Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables) c = g_malloc0(sizeof(*c)); c->size = num_tables; - c->entries = g_malloc0(sizeof(*c->entries) * num_tables); + c->entries = g_new0(Qcow2CachedTable, num_tables); for (i = 0; i < c->size; i++) { c->entries[i].table = qemu_try_blockalign(bs->file, s->cluster_size); diff --git a/block/qed-check.c b/block/qed-check.c index 40a882cc93..36ecd290d6 100644 --- a/block/qed-check.c +++ b/block/qed-check.c @@ -227,8 +227,7 @@ int qed_check(BDRVQEDState *s, BdrvCheckResult *result, bool fix) }; int ret; - check.used_clusters = g_try_malloc0(((check.nclusters + 31) / 32) * - sizeof(check.used_clusters[0])); + check.used_clusters = g_try_new0(uint32_t, (check.nclusters + 31) / 32); if (check.nclusters && check.used_clusters == NULL) { return -ENOMEM; } diff --git a/block/rbd.c b/block/rbd.c index 3aaf8559dd..ea969e7beb 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -862,7 +862,7 @@ static int qemu_rbd_snap_list(BlockDriverState *bs, int max_snaps = RBD_MAX_SNAPS; do { - snaps = g_malloc(sizeof(*snaps) * max_snaps); + snaps = g_new(rbd_snap_info_t, max_snaps); snap_count = rbd_snap_list(s->image, snaps, &max_snaps); if (snap_count <= 0) { g_free(snaps); diff --git a/block/sheepdog.c b/block/sheepdog.c index ba1ef43502..12cbd9dcb4 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -2357,7 +2357,7 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) goto out; } - sn_tab = g_malloc0(nr * sizeof(*sn_tab)); + sn_tab = g_new0(QEMUSnapshotInfo, nr); /* calculate a vdi id with hash function */ hval = fnv_64a_buf(s->name, strlen(s->name), FNV1A_64_INIT); diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 6d9a0651d8..04459e583c 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -319,7 +319,7 @@ static void nvme_init_sq(NvmeSQueue *sq, NvmeCtrl *n, uint64_t dma_addr, sq->size = size; sq->cqid = cqid; sq->head = sq->tail = 0; - sq->io_req = g_malloc(sq->size * sizeof(*sq->io_req)); + sq->io_req = g_new(NvmeRequest, sq->size); QTAILQ_INIT(&sq->req_list); QTAILQ_INIT(&sq->out_req_list); @@ -773,9 +773,9 @@ static int nvme_init(PCIDevice *pci_dev) n->reg_size = 1 << qemu_fls(0x1004 + 2 * (n->num_queues + 1) * 4); n->ns_size = bs_size / (uint64_t)n->num_namespaces; - n->namespaces = g_malloc0(sizeof(*n->namespaces)*n->num_namespaces); - n->sq = g_malloc0(sizeof(*n->sq)*n->num_queues); - n->cq = g_malloc0(sizeof(*n->cq)*n->num_queues); + n->namespaces = g_new0(NvmeNamespace, n->num_namespaces); + n->sq = g_new0(NvmeSQueue *, n->num_queues); + n->cq = g_new0(NvmeCQueue *, n->num_queues); memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n, "nvme", n->reg_size); diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 3a1e11e1ea..afd8867591 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -29,7 +29,7 @@ static int compare_cmdname(const void *a, const void *b) void qemuio_add_command(const cmdinfo_t *ci) { - cmdtab = g_realloc(cmdtab, ++ncmds * sizeof(*cmdtab)); + cmdtab = g_renew(cmdinfo_t, cmdtab, ++ncmds); cmdtab[ncmds - 1] = *ci; qsort(cmdtab, ncmds, sizeof(*cmdtab), compare_cmdname); } @@ -122,7 +122,7 @@ static char **breakline(char *input, int *count) continue; } c++; - tmp = g_realloc(rval, sizeof(*rval) * (c + 1)); + tmp = g_renew(char *, rval, (c + 1)); if (!tmp) { g_free(rval); rval = NULL; @@ -1264,9 +1264,9 @@ static int multiwrite_f(BlockDriverState *bs, int argc, char **argv) } } - reqs = g_malloc0(nr_reqs * sizeof(*reqs)); - buf = g_malloc0(nr_reqs * sizeof(*buf)); - qiovs = g_malloc(nr_reqs * sizeof(*qiovs)); + reqs = g_new0(BlockRequest, nr_reqs); + buf = g_new0(char *, nr_reqs); + qiovs = g_new(QEMUIOVector, nr_reqs); for (i = 0; i < nr_reqs && optind < argc; i++) { int j; -- cgit v1.2.3 From 08193dd52ba911077c9a29f62157bc4f3b9564eb Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 19 Aug 2014 10:31:10 +0200 Subject: qemu-io-cmds: g_renew() can't fail, bury dead error handling Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Jeff Cody Signed-off-by: Kevin Wolf --- qemu-io-cmds.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index afd8867591..b224ede5fa 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -115,22 +115,13 @@ static char **breakline(char *input, int *count) int c = 0; char *p; char **rval = g_new0(char *, 1); - char **tmp; while (rval && (p = qemu_strsep(&input, " ")) != NULL) { if (!*p) { continue; } c++; - tmp = g_renew(char *, rval, (c + 1)); - if (!tmp) { - g_free(rval); - rval = NULL; - c = 0; - break; - } else { - rval = tmp; - } + rval = g_renew(char *, rval, (c + 1)); rval[c - 1] = p; rval[c] = NULL; } -- cgit v1.2.3 From d4df3dbc021d1bfd1be6e3fa94e0e20086fbf183 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 19 Aug 2014 10:31:11 +0200 Subject: block: Drop some superfluous casts from void * They clutter the code. Unfortunately, I can't figure out how to make Coccinelle drop all of them, so I have to settle for common special cases: @@ type T; T *pt; void *pv; @@ - pt = (T *)pv; + pt = pv; @@ type T; @@ - (T *) (\(g_malloc\|g_malloc0\|g_realloc\|g_new\|g_new0\|g_renew\| g_try_malloc\|g_try_malloc0\|g_try_realloc\| g_try_new\|g_try_new0\|g_try_renew\)(...)) Topped off with minor manual style cleanups. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Jeff Cody Signed-off-by: Kevin Wolf --- block/vhdx-log.c | 2 +- block/vvfat.c | 8 ++++---- hw/ide/microdrive.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/block/vhdx-log.c b/block/vhdx-log.c index eb5c7a097b..6547bec404 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -923,7 +923,7 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s, buffer = qemu_blockalign(bs, total_length); memcpy(buffer, &new_hdr, sizeof(new_hdr)); - new_desc = (VHDXLogDescriptor *) (buffer + sizeof(new_hdr)); + new_desc = buffer + sizeof(new_hdr); data_sector = buffer + (desc_sectors * VHDX_LOG_SECTOR_SIZE); data_tmp = data; diff --git a/block/vvfat.c b/block/vvfat.c index f877e851d4..e56f766338 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -732,7 +732,7 @@ static int read_directory(BDRVVVFATState* s, int mapping_index) if(first_cluster == 0 && (is_dotdot || is_dot)) continue; - buffer=(char*)g_malloc(length); + buffer = g_malloc(length); snprintf(buffer,length,"%s/%s",dirname,entry->d_name); if(stat(buffer,&st)<0) { @@ -767,7 +767,7 @@ static int read_directory(BDRVVVFATState* s, int mapping_index) /* create mapping for this file */ if(!is_dot && !is_dotdot && (S_ISDIR(st.st_mode) || st.st_size)) { - s->current_mapping=(mapping_t*)array_get_next(&(s->mapping)); + s->current_mapping = array_get_next(&(s->mapping)); s->current_mapping->begin=0; s->current_mapping->end=st.st_size; /* @@ -811,12 +811,12 @@ static int read_directory(BDRVVVFATState* s, int mapping_index) } /* reget the mapping, since s->mapping was possibly realloc()ed */ - mapping = (mapping_t*)array_get(&(s->mapping), mapping_index); + mapping = array_get(&(s->mapping), mapping_index); first_cluster += (s->directory.next - mapping->info.dir.first_dir_index) * 0x20 / s->cluster_size; mapping->end = first_cluster; - direntry = (direntry_t*)array_get(&(s->directory), mapping->dir_index); + direntry = array_get(&(s->directory), mapping->dir_index); set_begin_of_direntry(direntry, mapping->begin); return 0; diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c index 2d70ddb757..15671b8c4d 100644 --- a/hw/ide/microdrive.c +++ b/hw/ide/microdrive.c @@ -567,7 +567,7 @@ PCMCIACardState *dscm1xxxx_init(DriveInfo *dinfo) } md->bus.ifs[0].drive_kind = IDE_CFATA; md->bus.ifs[0].mdata_size = METADATA_SIZE; - md->bus.ifs[0].mdata_storage = (uint8_t *) g_malloc0(METADATA_SIZE); + md->bus.ifs[0].mdata_storage = g_malloc0(METADATA_SIZE); return PCMCIA_CARD(md); } -- cgit v1.2.3 From 9d256ca616a298495e46c64da249b1620540930a Mon Sep 17 00:00:00 2001 From: Maria Kustova Date: Tue, 19 Aug 2014 00:02:34 +0400 Subject: runner: Add an argument for test duration After the specified duration the runner stops executing new tests, but it doesn't interrupt running ones. Reviewed-by: Fam Zheng Signed-off-by: Maria Kustova Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/image-fuzzer/runner.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py index 58079d331e..1e38b95fd7 100755 --- a/tests/image-fuzzer/runner.py +++ b/tests/image-fuzzer/runner.py @@ -25,6 +25,7 @@ import subprocess import random import shutil from itertools import count +import time import getopt import StringIO import resource @@ -269,6 +270,7 @@ if __name__ == '__main__': Optional arguments: -h, --help display this help and exit + -d, --duration=NUMBER finish tests after NUMBER of seconds -c, --command=JSON run tests for all commands specified in the JSON array -s, --seed=STRING seed for a test image generation, @@ -325,10 +327,15 @@ if __name__ == '__main__': finally: test.finish() + def should_continue(duration, start_time): + """Return True if a new test can be started and False otherwise.""" + current_time = int(time.time()) + return (duration is None) or (current_time - start_time < duration) + try: - opts, args = getopt.gnu_getopt(sys.argv[1:], 'c:hs:kv', + opts, args = getopt.gnu_getopt(sys.argv[1:], 'c:hs:kvd:', ['command=', 'help', 'seed=', 'config=', - 'keep_passed', 'verbose']) + 'keep_passed', 'verbose', 'duration=']) except getopt.error, e: print >>sys.stderr, \ "Error: %s\n\nTry 'runner.py --help' for more information" % e @@ -339,6 +346,8 @@ if __name__ == '__main__': log_all = False seed = None config = None + duration = None + for opt, arg in opts: if opt in ('-h', '--help'): usage() @@ -357,6 +366,8 @@ if __name__ == '__main__': log_all = True elif opt in ('-s', '--seed'): seed = arg + elif opt in ('-d', '--duration'): + duration = int(arg) elif opt == '--config': try: config = json.loads(arg) @@ -394,9 +405,11 @@ if __name__ == '__main__': resource.setrlimit(resource.RLIMIT_CORE, (-1, -1)) # If a seed is specified, only one test will be executed. # Otherwise runner will terminate after a keyboard interruption - for test_id in count(1): + start_time = int(time.time()) + test_id = count(1) + while should_continue(duration, start_time): try: - run_test(str(test_id), seed, work_dir, run_log, cleanup, + run_test(str(test_id.next()), seed, work_dir, run_log, cleanup, log_all, command, config) except (KeyboardInterrupt, SystemExit): sys.exit(1) -- cgit v1.2.3 From 18a7d0c56e1377f3d5fa1dc4d78a15dbab01cd83 Mon Sep 17 00:00:00 2001 From: Maria Kustova Date: Tue, 19 Aug 2014 00:02:35 +0400 Subject: runner: Kill a program under test by time-out If a program under test get frozen, the test should finish and report about its failure. In such cases the runner waits for 10 minutes until the program ends its execution. After this time-out the program will be terminated and the test will be marked as failed. For current limitation of test image size to 10 MB as a maximum an execution of each command takes about several seconds in general, so 10 minutes is enough to discriminate freeze, but not drastically increase an overall test duration. Signed-off-by: Maria Kustova Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/image-fuzzer/runner.py | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py index 1e38b95fd7..c903c8a342 100755 --- a/tests/image-fuzzer/runner.py +++ b/tests/image-fuzzer/runner.py @@ -65,14 +65,35 @@ def run_app(fd, q_args): """Start an application with specified arguments and return its exit code or kill signal depending on the result of execution. """ + + class Alarm(Exception): + """Exception for signal.alarm events.""" + pass + + def handler(*arg): + """Notify that an alarm event occurred.""" + raise Alarm + + signal.signal(signal.SIGALRM, handler) + signal.alarm(600) + term_signal = signal.SIGKILL devnull = open('/dev/null', 'r+') process = subprocess.Popen(q_args, stdin=devnull, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - out, err = process.communicate() - fd.write(out) - fd.write(err) - return process.returncode + try: + out, err = process.communicate() + signal.alarm(0) + fd.write(out) + fd.write(err) + fd.flush() + return process.returncode + + except Alarm: + os.kill(process.pid, term_signal) + fd.write('The command was terminated by timeout.\n') + fd.flush() + return -term_signal class TestException(Exception): -- cgit v1.2.3 From 440ba08aea3d841996efaf6a6b88426b0d59abf4 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 18 Aug 2014 22:07:31 +0200 Subject: qcow2: Constant cache size in bytes Specifying the metadata cache sizes in clusters results in less clusters (and much less bytes) covered for small cluster sizes and vice versa. Using a constant byte size reduces this difference, and makes it possible to manually specify the cache size in an easily comprehensible unit. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2.c | 15 +++++++++++++-- block/qcow2.h | 10 ++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 435e0e11d0..0f1be2e384 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -470,6 +470,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, uint64_t l1_vm_state_index; const char *opt_overlap_check; int overlap_check_template = 0; + uint64_t l2_cache_size, refcount_cache_size; ret = bdrv_pread(bs->file, 0, &header, sizeof(header)); if (ret < 0) { @@ -707,8 +708,18 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, } /* alloc L2 table/refcount block cache */ - s->l2_table_cache = qcow2_cache_create(bs, L2_CACHE_SIZE); - s->refcount_block_cache = qcow2_cache_create(bs, REFCOUNT_CACHE_SIZE); + l2_cache_size = DEFAULT_L2_CACHE_BYTE_SIZE / s->cluster_size; + if (l2_cache_size < MIN_L2_CACHE_SIZE) { + l2_cache_size = MIN_L2_CACHE_SIZE; + } + + refcount_cache_size = l2_cache_size / DEFAULT_L2_REFCOUNT_SIZE_RATIO; + if (refcount_cache_size < MIN_REFCOUNT_CACHE_SIZE) { + refcount_cache_size = MIN_REFCOUNT_CACHE_SIZE; + } + + s->l2_table_cache = qcow2_cache_create(bs, l2_cache_size); + s->refcount_block_cache = qcow2_cache_create(bs, refcount_cache_size); if (s->l2_table_cache == NULL || s->refcount_block_cache == NULL) { error_setg(errp, "Could not allocate metadata caches"); ret = -ENOMEM; diff --git a/block/qcow2.h b/block/qcow2.h index b49424b85e..671783defe 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -64,10 +64,16 @@ #define MIN_CLUSTER_BITS 9 #define MAX_CLUSTER_BITS 21 -#define L2_CACHE_SIZE 16 +#define MIN_L2_CACHE_SIZE 1 /* cluster */ /* Must be at least 4 to cover all cases of refcount table growth */ -#define REFCOUNT_CACHE_SIZE 4 +#define MIN_REFCOUNT_CACHE_SIZE 4 /* clusters */ + +#define DEFAULT_L2_CACHE_BYTE_SIZE 1048576 /* bytes */ + +/* The refblock cache needs only a fourth of the L2 cache size to cover as many + * clusters */ +#define DEFAULT_L2_REFCOUNT_SIZE_RATIO 4 #define DEFAULT_CLUSTER_SIZE 65536 -- cgit v1.2.3 From 02004bd4ba1ace29812e977d3cd0bc20fd6bf677 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 18 Aug 2014 22:07:32 +0200 Subject: qcow2: Use g_try_new0() for cache array With a variable cache size, the number given to qcow2_cache_create() may be huge. Therefore, use g_try_new0(). While at it, use g_new0() instead of g_malloc0() for allocating the Qcow2Cache object. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-cache.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index fe0615a995..904f6b1f44 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -48,9 +48,12 @@ Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables) Qcow2Cache *c; int i; - c = g_malloc0(sizeof(*c)); + c = g_new0(Qcow2Cache, 1); c->size = num_tables; - c->entries = g_new0(Qcow2CachedTable, num_tables); + c->entries = g_try_new0(Qcow2CachedTable, num_tables); + if (!c->entries) { + goto fail; + } for (i = 0; i < c->size; i++) { c->entries[i].table = qemu_try_blockalign(bs->file, s->cluster_size); @@ -62,8 +65,10 @@ Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables) return c; fail: - for (i = 0; i < c->size; i++) { - qemu_vfree(c->entries[i].table); + if (c->entries) { + for (i = 0; i < c->size; i++) { + qemu_vfree(c->entries[i].table); + } } g_free(c->entries); g_free(c); -- cgit v1.2.3 From 6c1c8d5d3ebd2e3bcdc89255363c19a88e14ddec Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 18 Aug 2014 22:07:33 +0200 Subject: qcow2: Add runtime options for cache sizes Add options for specifying the size of the metadata caches. This can either be done directly for each cache (if only one is given, the other will be derived according to a default ratio) or combined for both. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++------ block/qcow2.h | 3 ++ 2 files changed, 103 insertions(+), 11 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 0f1be2e384..f9e045ff2b 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -442,6 +442,22 @@ static QemuOptsList qcow2_runtime_opts = { .type = QEMU_OPT_BOOL, .help = "Check for unintended writes into an inactive L2 table", }, + { + .name = QCOW2_OPT_CACHE_SIZE, + .type = QEMU_OPT_SIZE, + .help = "Maximum combined metadata (L2 tables and refcount blocks) " + "cache size", + }, + { + .name = QCOW2_OPT_L2_CACHE_SIZE, + .type = QEMU_OPT_SIZE, + .help = "Maximum L2 table cache size", + }, + { + .name = QCOW2_OPT_REFCOUNT_CACHE_SIZE, + .type = QEMU_OPT_SIZE, + .help = "Maximum refcount block cache size", + }, { /* end of list */ } }, }; @@ -457,6 +473,61 @@ static const char *overlap_bool_option_names[QCOW2_OL_MAX_BITNR] = { [QCOW2_OL_INACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L2, }; +static void read_cache_sizes(QemuOpts *opts, uint64_t *l2_cache_size, + uint64_t *refcount_cache_size, Error **errp) +{ + uint64_t combined_cache_size; + bool l2_cache_size_set, refcount_cache_size_set, combined_cache_size_set; + + combined_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_CACHE_SIZE); + l2_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_L2_CACHE_SIZE); + refcount_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_REFCOUNT_CACHE_SIZE); + + combined_cache_size = qemu_opt_get_size(opts, QCOW2_OPT_CACHE_SIZE, 0); + *l2_cache_size = qemu_opt_get_size(opts, QCOW2_OPT_L2_CACHE_SIZE, 0); + *refcount_cache_size = qemu_opt_get_size(opts, + QCOW2_OPT_REFCOUNT_CACHE_SIZE, 0); + + if (combined_cache_size_set) { + if (l2_cache_size_set && refcount_cache_size_set) { + error_setg(errp, QCOW2_OPT_CACHE_SIZE ", " QCOW2_OPT_L2_CACHE_SIZE + " and " QCOW2_OPT_REFCOUNT_CACHE_SIZE " may not be set " + "the same time"); + return; + } else if (*l2_cache_size > combined_cache_size) { + error_setg(errp, QCOW2_OPT_L2_CACHE_SIZE " may not exceed " + QCOW2_OPT_CACHE_SIZE); + return; + } else if (*refcount_cache_size > combined_cache_size) { + error_setg(errp, QCOW2_OPT_REFCOUNT_CACHE_SIZE " may not exceed " + QCOW2_OPT_CACHE_SIZE); + return; + } + + if (l2_cache_size_set) { + *refcount_cache_size = combined_cache_size - *l2_cache_size; + } else if (refcount_cache_size_set) { + *l2_cache_size = combined_cache_size - *refcount_cache_size; + } else { + *refcount_cache_size = combined_cache_size + / (DEFAULT_L2_REFCOUNT_SIZE_RATIO + 1); + *l2_cache_size = combined_cache_size - *refcount_cache_size; + } + } else { + if (!l2_cache_size_set && !refcount_cache_size_set) { + *l2_cache_size = DEFAULT_L2_CACHE_BYTE_SIZE; + *refcount_cache_size = *l2_cache_size + / DEFAULT_L2_REFCOUNT_SIZE_RATIO; + } else if (!l2_cache_size_set) { + *l2_cache_size = *refcount_cache_size + * DEFAULT_L2_REFCOUNT_SIZE_RATIO; + } else if (!refcount_cache_size_set) { + *refcount_cache_size = *l2_cache_size + / DEFAULT_L2_REFCOUNT_SIZE_RATIO; + } + } +} + static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { @@ -707,17 +778,43 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, } } - /* alloc L2 table/refcount block cache */ - l2_cache_size = DEFAULT_L2_CACHE_BYTE_SIZE / s->cluster_size; + /* get L2 table/refcount block cache size from command line options */ + opts = qemu_opts_create(&qcow2_runtime_opts, NULL, 0, &error_abort); + qemu_opts_absorb_qdict(opts, options, &local_err); + if (local_err) { + error_propagate(errp, local_err); + ret = -EINVAL; + goto fail; + } + + read_cache_sizes(opts, &l2_cache_size, &refcount_cache_size, &local_err); + if (local_err) { + error_propagate(errp, local_err); + ret = -EINVAL; + goto fail; + } + + l2_cache_size /= s->cluster_size; if (l2_cache_size < MIN_L2_CACHE_SIZE) { l2_cache_size = MIN_L2_CACHE_SIZE; } + if (l2_cache_size > INT_MAX) { + error_setg(errp, "L2 cache size too big"); + ret = -EINVAL; + goto fail; + } - refcount_cache_size = l2_cache_size / DEFAULT_L2_REFCOUNT_SIZE_RATIO; + refcount_cache_size /= s->cluster_size; if (refcount_cache_size < MIN_REFCOUNT_CACHE_SIZE) { refcount_cache_size = MIN_REFCOUNT_CACHE_SIZE; } + if (refcount_cache_size > INT_MAX) { + error_setg(errp, "Refcount cache size too big"); + ret = -EINVAL; + goto fail; + } + /* alloc L2 table/refcount block cache */ s->l2_table_cache = qcow2_cache_create(bs, l2_cache_size); s->refcount_block_cache = qcow2_cache_create(bs, refcount_cache_size); if (s->l2_table_cache == NULL || s->refcount_block_cache == NULL) { @@ -809,14 +906,6 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, } /* Enable lazy_refcounts according to image and command line options */ - opts = qemu_opts_create(&qcow2_runtime_opts, NULL, 0, &error_abort); - qemu_opts_absorb_qdict(opts, options, &local_err); - if (local_err) { - error_propagate(errp, local_err); - ret = -EINVAL; - goto fail; - } - s->use_lazy_refcounts = qemu_opt_get_bool(opts, QCOW2_OPT_LAZY_REFCOUNTS, (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS)); diff --git a/block/qcow2.h b/block/qcow2.h index 671783defe..6aeb7ea90f 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -91,6 +91,9 @@ #define QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE "overlap-check.snapshot-table" #define QCOW2_OPT_OVERLAP_INACTIVE_L1 "overlap-check.inactive-l1" #define QCOW2_OPT_OVERLAP_INACTIVE_L2 "overlap-check.inactive-l2" +#define QCOW2_OPT_CACHE_SIZE "cache-size" +#define QCOW2_OPT_L2_CACHE_SIZE "l2-cache-size" +#define QCOW2_OPT_REFCOUNT_CACHE_SIZE "refcount-cache-size" typedef struct QCowHeader { uint32_t magic; -- cgit v1.2.3 From a1cb48a3bffdb62c429297ac7057d1ce76fc4839 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 18 Aug 2014 22:07:34 +0200 Subject: iotests: Add test for qcow2's cache options Add a test which tests various combinations of qcow2's cache options (some of which are valid, some of which are not). Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/103 | 99 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/103.out | 29 ++++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 129 insertions(+) create mode 100755 tests/qemu-iotests/103 create mode 100644 tests/qemu-iotests/103.out diff --git a/tests/qemu-iotests/103 b/tests/qemu-iotests/103 new file mode 100755 index 0000000000..0f1dc9fa7d --- /dev/null +++ b/tests/qemu-iotests/103 @@ -0,0 +1,99 @@ +#!/bin/bash +# +# Test case for qcow2 metadata cache size specification +# +# Copyright (C) 2014 Red Hat, Inc. +# +# 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 . +# + +# creator +owner=mreitz@redhat.com + +seq=$(basename $0) +echo "QA output created by $seq" + +here=$PWD +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt qcow2 +_supported_proto file +_supported_os Linux + +IMG_SIZE=64K + +_make_test_img $IMG_SIZE +$QEMU_IO -c 'write -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io + +echo +echo '=== Testing invalid option combinations ===' +echo + +# all sizes set at the same time +$QEMU_IO -c "open -o cache-size=1.25M,l2-cache-size=1M,refcount-cache-size=0.25M $TEST_IMG" \ + 2>&1 | _filter_testdir | _filter_imgfmt +# l2-cache-size may not exceed cache-size +$QEMU_IO -c "open -o cache-size=1M,l2-cache-size=2M $TEST_IMG" 2>&1 \ + | _filter_testdir | _filter_imgfmt +# refcount-cache-size may not exceed cache-size +$QEMU_IO -c "open -o cache-size=1M,refcount-cache-size=2M $TEST_IMG" 2>&1 \ + | _filter_testdir | _filter_imgfmt +# 0 should be a valid size (e.g. for enforcing the minimum), so this should not +# work +$QEMU_IO -c "open -o cache-size=0,l2-cache-size=0,refcount-cache-size=0 $TEST_IMG" \ + 2>&1 | _filter_testdir | _filter_imgfmt + +echo +echo '=== Testing valid option combinations ===' +echo + +# There should be a reasonable and working minimum +$QEMU_IO -c "open -o cache-size=0 $TEST_IMG" -c 'read -P 42 0 64k' \ + | _filter_qemu_io +$QEMU_IO -c "open -o l2-cache-size=0 $TEST_IMG" -c 'read -P 42 0 64k' \ + | _filter_qemu_io +$QEMU_IO -c "open -o refcount-cache-size=0 $TEST_IMG" -c 'read -P 42 0 64k' \ + | _filter_qemu_io + +# Derive cache sizes from combined size (with a reasonable ratio, but we cannot +# test that) +$QEMU_IO -c "open -o cache-size=2M $TEST_IMG" -c 'read -P 42 0 64k' \ + | _filter_qemu_io +# Fix one cache, derive the other +$QEMU_IO -c "open -o cache-size=2M,l2-cache-size=1M $TEST_IMG" \ + -c 'read -P 42 0 64k' \ + | _filter_qemu_io +$QEMU_IO -c "open -o cache-size=2M,refcount-cache-size=1M $TEST_IMG" \ + -c 'read -P 42 0 64k' \ + | _filter_qemu_io +# Directly set both caches +$QEMU_IO -c "open -o l2-cache-size=1M,refcount-cache-size=0.25M $TEST_IMG" \ + -c 'read -P 42 0 64k' \ + | _filter_qemu_io + +# success, all done +echo '*** done' +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/103.out b/tests/qemu-iotests/103.out new file mode 100644 index 0000000000..ddf6b5a075 --- /dev/null +++ b/tests/qemu-iotests/103.out @@ -0,0 +1,29 @@ +QA output created by 103 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536 +wrote 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +=== Testing invalid option combinations === + +qemu-io: can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set the same time +qemu-io: can't open device TEST_DIR/t.IMGFMT: l2-cache-size may not exceed cache-size +qemu-io: can't open device TEST_DIR/t.IMGFMT: refcount-cache-size may not exceed cache-size +qemu-io: can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set the same time + +=== Testing valid option combinations === + +read 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 6e67f61262..1c8d453f4e 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -100,3 +100,4 @@ 091 rw auto quick 092 rw auto quick 095 rw auto quick +103 rw auto quick -- cgit v1.2.3 From 61ff8cfbec64f08f6955fe04649665f9f7eb0cc6 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Wed, 13 Aug 2014 18:08:47 +0800 Subject: test-coroutine: test cost introduced by coroutine This test runs dummy function with coroutine by using two enter and one yield since which is a common usage. So we can see the cost introduced by corouting for running one function, for example: Run operation 20000000 iterations 4.841071 s, 4131K operations/s 242ns per coroutine Signed-off-by: Ming Lei Signed-off-by: Kevin Wolf --- tests/test-coroutine.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/test-coroutine.c b/tests/test-coroutine.c index 6e634f4a78..e22fae170a 100644 --- a/tests/test-coroutine.c +++ b/tests/test-coroutine.c @@ -311,6 +311,35 @@ static void perf_baseline(void) maxcycles, duration); } +static __attribute__((noinline)) void perf_cost_func(void *opaque) +{ + qemu_coroutine_yield(); +} + +static void perf_cost(void) +{ + const unsigned long maxcycles = 40000000; + unsigned long i = 0; + double duration; + unsigned long ops; + Coroutine *co; + + g_test_timer_start(); + while (i++ < maxcycles) { + co = qemu_coroutine_create(perf_cost_func); + qemu_coroutine_enter(co, &i); + qemu_coroutine_enter(co, NULL); + } + duration = g_test_timer_elapsed(); + ops = (long)(maxcycles / (duration * 1000)); + + g_test_message("Run operation %lu iterations %f s, %luK operations/s, " + "%luns per coroutine", + maxcycles, + duration, ops, + (unsigned long)(1000000000 * duration) / maxcycles); +} + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); @@ -325,6 +354,7 @@ int main(int argc, char **argv) g_test_add_func("/perf/nesting", perf_nesting); g_test_add_func("/perf/yield", perf_yield); g_test_add_func("/perf/function-call", perf_baseline); + g_test_add_func("/perf/cost", perf_cost); } return g_test_run(); } -- cgit v1.2.3 From 6ffb4cb6fd2046e2efb658fb0136e22e4cd0c670 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 19 Aug 2014 19:28:58 +0200 Subject: qemu-iotests: Fix 028 reference output for qed We need to filter out driver-specific options in the "Formatting..." string printed by qemu when creating the backup image. Reported-by: Peter Wu Signed-off-by: Kevin Wolf Tested-by: Peter Wu --- tests/qemu-iotests/028 | 1 + tests/qemu-iotests/028.out | 3 ++- tests/qemu-iotests/common.filter | 22 ++++++++++++++++++++++ tests/qemu-iotests/common.rc | 19 +------------------ 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/tests/qemu-iotests/028 b/tests/qemu-iotests/028 index 9e701e1c2b..a1f4423d4a 100755 --- a/tests/qemu-iotests/028 +++ b/tests/qemu-iotests/028 @@ -113,6 +113,7 @@ QEMU_COMM_TIMEOUT=1 # Silence output since it contains the disk image path and QEMU's readline # character echoing makes it very hard to filter the output _send_qemu_cmd $h "drive_backup disk ${TEST_IMG}.copy" "(qemu)" >/dev/null +_send_qemu_cmd $h "" "Formatting" | _filter_img_create qemu_cmd_repeat=20 _send_qemu_cmd $h "info block-jobs" "No active jobs" _send_qemu_cmd $h 'quit' "" diff --git a/tests/qemu-iotests/028.out b/tests/qemu-iotests/028.out index 0e1a5ae65d..e8d02459be 100644 --- a/tests/qemu-iotests/028.out +++ b/tests/qemu-iotests/028.out @@ -468,7 +468,8 @@ No errors were found on the image. block-backup -Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=4294968832 backing_file='TEST_DIR/t.qcow2.base' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off +Formatting 'TEST_DIR/t.IMGFMT.copy', fmt=IMGFMT size=4294968832 backing_file='TEST_DIR/t.IMGFMT.base' backing_fmt='IMGFMT' +(qemu) (qemu) iininfinfoinfo info binfo blinfo bloinfo blocinfo blockinfo block-info block-jinfo block-joinfo block-jobinfo block-jobs Type backup, device disk: Completed 0 of 4294968832 bytes, speed limit 0 bytes/s iininfinfoinfo info binfo blinfo bloinfo blocinfo blockinfo block-info block-jinfo block-joinfo block-jobinfo block-jobs diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index a04df7f6dc..51192c8511 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -170,5 +170,27 @@ _filter_qmp() -e 's#^{"QMP":.*}$#QMP_VERSION#' } +# replace driver-specific options in the "Formatting..." line +_filter_img_create() +{ + sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \ + -e "s#$TEST_DIR#TEST_DIR#g" \ + -e "s#$IMGFMT#IMGFMT#g" \ + -e "s# encryption=off##g" \ + -e "s# cluster_size=[0-9]\\+##g" \ + -e "s# table_size=[0-9]\\+##g" \ + -e "s# compat='[^']*'##g" \ + -e "s# compat6=\\(on\\|off\\)##g" \ + -e "s# static=\\(on\\|off\\)##g" \ + -e "s# zeroed_grain=\\(on\\|off\\)##g" \ + -e "s# subformat='[^']*'##g" \ + -e "s# adapter_type='[^']*'##g" \ + -e "s# lazy_refcounts=\\(on\\|off\\)##g" \ + -e "s# block_size=[0-9]\\+##g" \ + -e "s# block_state_zero=\\(on\\|off\\)##g" \ + -e "s# log_size=[0-9]\\+##g" \ + -e "s/archipelago:a/TEST_DIR\//g" +} + # make sure this script returns success /bin/true diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 3fd691e6cd..9c49deb3dd 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -149,24 +149,7 @@ _make_test_img() else $QEMU_IMG create -f $IMGFMT $extra_img_options "$img_name" $image_size 2>&1 fi - ) | \ - sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \ - -e "s#$TEST_DIR#TEST_DIR#g" \ - -e "s#$IMGFMT#IMGFMT#g" \ - -e "s# encryption=off##g" \ - -e "s# cluster_size=[0-9]\\+##g" \ - -e "s# table_size=[0-9]\\+##g" \ - -e "s# compat='[^']*'##g" \ - -e "s# compat6=\\(on\\|off\\)##g" \ - -e "s# static=\\(on\\|off\\)##g" \ - -e "s# zeroed_grain=\\(on\\|off\\)##g" \ - -e "s# subformat='[^']*'##g" \ - -e "s# adapter_type='[^']*'##g" \ - -e "s# lazy_refcounts=\\(on\\|off\\)##g" \ - -e "s# block_size=[0-9]\\+##g" \ - -e "s# block_state_zero=\\(on\\|off\\)##g" \ - -e "s# log_size=[0-9]\\+##g" \ - -e "s/archipelago:a/TEST_DIR\//g" + ) | _filter_img_create # Start an NBD server on the image file, which is what we'll be talking to if [ $IMGPROTO = "nbd" ]; then -- cgit v1.2.3 From 927e0e769f4008f458de8a94a809e85c1fd016eb Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 18 Aug 2014 14:52:28 +0100 Subject: block: acquire AioContext in qmp_block_resize() Make block_resize safe for dataplane where another thread may be running the BlockDriverState's AioContext. Signed-off-by: Stefan Hajnoczi Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/blockdev.c b/blockdev.c index 8acd264407..6a204c662d 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1799,6 +1799,7 @@ void qmp_block_resize(bool has_device, const char *device, { Error *local_err = NULL; BlockDriverState *bs; + AioContext *aio_context; int ret; bs = bdrv_lookup_bs(has_device ? device : NULL, @@ -1809,19 +1810,22 @@ void qmp_block_resize(bool has_device, const char *device, return; } + aio_context = bdrv_get_aio_context(bs); + aio_context_acquire(aio_context); + if (!bdrv_is_first_non_filter(bs)) { error_set(errp, QERR_FEATURE_DISABLED, "resize"); - return; + goto out; } if (size < 0) { error_set(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size"); - return; + goto out; } if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) { error_set(errp, QERR_DEVICE_IN_USE, device); - return; + goto out; } /* complete all in-flight operations before resizing the device */ @@ -1847,6 +1851,9 @@ void qmp_block_resize(bool has_device, const char *device, error_setg_errno(errp, -ret, "Could not resize"); break; } + +out: + aio_context_release(aio_context); } static void block_job_cb(void *opaque, int ret) -- cgit v1.2.3 From 466560b9fcada2656b276eb30e25da15a6e706df Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 18 Aug 2014 14:52:29 +0100 Subject: virtio-blk: allow block_resize with dataplane Now that block_resize acquires the AioContext we can safely allow resizing the disk. Reported-by: Andrey Korolyov Signed-off-by: Stefan Hajnoczi Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 24a6b71395..c07adc6e4f 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -193,6 +193,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, error_setg(&s->blocker, "block device is in use by data plane"); bdrv_op_block_all(blk->conf.bs, s->blocker); + bdrv_op_unblock(blk->conf.bs, BLOCK_OP_TYPE_RESIZE, s->blocker); *dataplane = s; } -- cgit v1.2.3 From 1bdb176ac5add5dc9d54a230da7511b66851f1e7 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Mon, 18 Aug 2014 15:42:50 +0800 Subject: virtio-blk: fix reference a pointer which might be freed In function virtio_blk_handle_request, it may freed memory pointed by req, So do not access member of req after calling this function. Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Reviewed-by: Stefan Hajnoczi Signed-off-by: zhanghailiang Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 302c39e2be..d9167ce9a3 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -469,8 +469,9 @@ static void virtio_blk_dma_restart_bh(void *opaque) s->rq = NULL; while (req) { + VirtIOBlockReq *next = req->next; virtio_blk_handle_request(req, &mrb); - req = req->next; + req = next; } virtio_submit_multiwrite(s->bs, &mrb); -- cgit v1.2.3 From 91af7014125895cc74141be6b60f3a3e882ed743 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 18 Jul 2014 20:24:56 +0200 Subject: block: Add bdrv_refresh_filename() Some block devices may not have a filename in their BDS; and for some, there may not even be a normal filename at all. To work around this, add a function which tries to construct a valid filename for the BDS.filename field. If a filename exists or a block driver is able to reconstruct a valid filename (which is placed in BDS.exact_filename), this can directly be used. If no filename can be constructed, we can still construct an options QDict which is then converted to a JSON object and prefixed with the "json:" pseudo protocol prefix. The QDict is placed in BDS.full_open_options. For most block drivers, this process can be done automatically; those that need special handling may define a .bdrv_refresh_filename() method to fill BDS.exact_filename and BDS.full_open_options themselves. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++ include/block/block.h | 1 + include/block/block_int.h | 6 +++ 3 files changed, 142 insertions(+) diff --git a/block.c b/block.c index 712f5db758..e9380f6c58 100644 --- a/block.c +++ b/block.c @@ -964,6 +964,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, } else { bs->filename[0] = '\0'; } + pstrcpy(bs->exact_filename, sizeof(bs->exact_filename), bs->filename); bs->drv = drv; bs->opaque = g_malloc0(drv->instance_size); @@ -1505,6 +1506,8 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, } } + bdrv_refresh_filename(bs); + /* For snapshot=on, create a temporary qcow2 overlay. bs points to the * temporary snapshot afterwards. */ if (snapshot_flags) { @@ -1845,6 +1848,8 @@ void bdrv_close(BlockDriverState *bs) bs->zero_beyond_eof = false; QDECREF(bs->options); bs->options = NULL; + QDECREF(bs->full_open_options); + bs->full_open_options = NULL; if (bs->file != NULL) { bdrv_unref(bs->file); @@ -5894,3 +5899,133 @@ void bdrv_flush_io_queue(BlockDriverState *bs) bdrv_flush_io_queue(bs->file); } } + +static bool append_open_options(QDict *d, BlockDriverState *bs) +{ + const QDictEntry *entry; + bool found_any = false; + + for (entry = qdict_first(bs->options); entry; + entry = qdict_next(bs->options, entry)) + { + /* Only take options for this level and exclude all non-driver-specific + * options */ + if (!strchr(qdict_entry_key(entry), '.') && + strcmp(qdict_entry_key(entry), "node-name")) + { + qobject_incref(qdict_entry_value(entry)); + qdict_put_obj(d, qdict_entry_key(entry), qdict_entry_value(entry)); + found_any = true; + } + } + + return found_any; +} + +/* Updates the following BDS fields: + * - exact_filename: A filename which may be used for opening a block device + * which (mostly) equals the given BDS (even without any + * other options; so reading and writing must return the same + * results, but caching etc. may be different) + * - full_open_options: Options which, when given when opening a block device + * (without a filename), result in a BDS (mostly) + * equalling the given one + * - filename: If exact_filename is set, it is copied here. Otherwise, + * full_open_options is converted to a JSON object, prefixed with + * "json:" (for use through the JSON pseudo protocol) and put here. + */ +void bdrv_refresh_filename(BlockDriverState *bs) +{ + BlockDriver *drv = bs->drv; + QDict *opts; + + if (!drv) { + return; + } + + /* This BDS's file name will most probably depend on its file's name, so + * refresh that first */ + if (bs->file) { + bdrv_refresh_filename(bs->file); + } + + if (drv->bdrv_refresh_filename) { + /* Obsolete information is of no use here, so drop the old file name + * information before refreshing it */ + bs->exact_filename[0] = '\0'; + if (bs->full_open_options) { + QDECREF(bs->full_open_options); + bs->full_open_options = NULL; + } + + drv->bdrv_refresh_filename(bs); + } else if (bs->file) { + /* Try to reconstruct valid information from the underlying file */ + bool has_open_options; + + bs->exact_filename[0] = '\0'; + if (bs->full_open_options) { + QDECREF(bs->full_open_options); + bs->full_open_options = NULL; + } + + opts = qdict_new(); + has_open_options = append_open_options(opts, bs); + + /* If no specific options have been given for this BDS, the filename of + * the underlying file should suffice for this one as well */ + if (bs->file->exact_filename[0] && !has_open_options) { + strcpy(bs->exact_filename, bs->file->exact_filename); + } + /* Reconstructing the full options QDict is simple for most format block + * drivers, as long as the full options are known for the underlying + * file BDS. The full options QDict of that file BDS should somehow + * contain a representation of the filename, therefore the following + * suffices without querying the (exact_)filename of this BDS. */ + if (bs->file->full_open_options) { + qdict_put_obj(opts, "driver", + QOBJECT(qstring_from_str(drv->format_name))); + QINCREF(bs->file->full_open_options); + qdict_put_obj(opts, "file", QOBJECT(bs->file->full_open_options)); + + bs->full_open_options = opts; + } else { + QDECREF(opts); + } + } else if (!bs->full_open_options && qdict_size(bs->options)) { + /* There is no underlying file BDS (at least referenced by BDS.file), + * so the full options QDict should be equal to the options given + * specifically for this block device when it was opened (plus the + * driver specification). + * Because those options don't change, there is no need to update + * full_open_options when it's already set. */ + + opts = qdict_new(); + append_open_options(opts, bs); + qdict_put_obj(opts, "driver", + QOBJECT(qstring_from_str(drv->format_name))); + + if (bs->exact_filename[0]) { + /* This may not work for all block protocol drivers (some may + * require this filename to be parsed), but we have to find some + * default solution here, so just include it. If some block driver + * does not support pure options without any filename at all or + * needs some special format of the options QDict, it needs to + * implement the driver-specific bdrv_refresh_filename() function. + */ + qdict_put_obj(opts, "filename", + QOBJECT(qstring_from_str(bs->exact_filename))); + } + + bs->full_open_options = opts; + } + + if (bs->exact_filename[0]) { + pstrcpy(bs->filename, sizeof(bs->filename), bs->exact_filename); + } else if (bs->full_open_options) { + QString *json = qobject_to_json(QOBJECT(bs->full_open_options)); + snprintf(bs->filename, sizeof(bs->filename), "json:%s", + qstring_get_str(json)); + QDECREF(json); + } +} diff --git a/include/block/block.h b/include/block/block.h index e94b701667..8f4ad16d8f 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -274,6 +274,7 @@ int coroutine_fn bdrv_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, const char *backing_file); int bdrv_get_backing_file_depth(BlockDriverState *bs); +void bdrv_refresh_filename(BlockDriverState *bs); int bdrv_truncate(BlockDriverState *bs, int64_t offset); int64_t bdrv_nb_sectors(BlockDriverState *bs); int64_t bdrv_getlength(BlockDriverState *bs); diff --git a/include/block/block_int.h b/include/block/block_int.h index 7b541a0691..233489547e 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -123,6 +123,9 @@ struct BlockDriver { int (*bdrv_create)(const char *filename, QemuOpts *opts, Error **errp); int (*bdrv_set_key)(BlockDriverState *bs, const char *key); int (*bdrv_make_empty)(BlockDriverState *bs); + + void (*bdrv_refresh_filename)(BlockDriverState *bs); + /* aio */ BlockDriverAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, @@ -323,6 +326,9 @@ struct BlockDriverState { this file image */ char backing_format[16]; /* if non-zero and backing_file exists */ + QDict *full_open_options; + char exact_filename[1024]; + BlockDriverState *backing_hd; BlockDriverState *file; -- cgit v1.2.3 From 2c31b04c94864fda7b28321c98b904043afe3db2 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 18 Jul 2014 20:24:57 +0200 Subject: blkdebug: Implement bdrv_refresh_filename() Because blkdebug cannot simply create a configuration file, simply refuse to reconstruct a plain filename and only generate an options QDict from the rules instead. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/blkdebug.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/block/blkdebug.c b/block/blkdebug.c index 1586ed9664..95b72445a7 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -26,6 +26,10 @@ #include "qemu/config-file.h" #include "block/block_int.h" #include "qemu/module.h" +#include "qapi/qmp/qbool.h" +#include "qapi/qmp/qdict.h" +#include "qapi/qmp/qint.h" +#include "qapi/qmp/qstring.h" typedef struct BDRVBlkdebugState { int state; @@ -706,6 +710,98 @@ static int64_t blkdebug_getlength(BlockDriverState *bs) return bdrv_getlength(bs->file); } +static void blkdebug_refresh_filename(BlockDriverState *bs) +{ + BDRVBlkdebugState *s = bs->opaque; + struct BlkdebugRule *rule; + QDict *opts; + QList *inject_error_list = NULL, *set_state_list = NULL; + QList *suspend_list = NULL; + int event; + + if (!bs->file->full_open_options) { + /* The config file cannot be recreated, so creating a plain filename + * is impossible */ + return; + } + + opts = qdict_new(); + qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("blkdebug"))); + + QINCREF(bs->file->full_open_options); + qdict_put_obj(opts, "image", QOBJECT(bs->file->full_open_options)); + + for (event = 0; event < BLKDBG_EVENT_MAX; event++) { + QLIST_FOREACH(rule, &s->rules[event], next) { + if (rule->action == ACTION_INJECT_ERROR) { + QDict *inject_error = qdict_new(); + + qdict_put_obj(inject_error, "event", QOBJECT(qstring_from_str( + BlkdebugEvent_lookup[rule->event]))); + qdict_put_obj(inject_error, "state", + QOBJECT(qint_from_int(rule->state))); + qdict_put_obj(inject_error, "errno", QOBJECT(qint_from_int( + rule->options.inject.error))); + qdict_put_obj(inject_error, "sector", QOBJECT(qint_from_int( + rule->options.inject.sector))); + qdict_put_obj(inject_error, "once", QOBJECT(qbool_from_int( + rule->options.inject.once))); + qdict_put_obj(inject_error, "immediately", + QOBJECT(qbool_from_int( + rule->options.inject.immediately))); + + if (!inject_error_list) { + inject_error_list = qlist_new(); + } + + qlist_append_obj(inject_error_list, QOBJECT(inject_error)); + } else if (rule->action == ACTION_SET_STATE) { + QDict *set_state = qdict_new(); + + qdict_put_obj(set_state, "event", QOBJECT(qstring_from_str( + BlkdebugEvent_lookup[rule->event]))); + qdict_put_obj(set_state, "state", + QOBJECT(qint_from_int(rule->state))); + qdict_put_obj(set_state, "new_state", QOBJECT(qint_from_int( + rule->options.set_state.new_state))); + + if (!set_state_list) { + set_state_list = qlist_new(); + } + + qlist_append_obj(set_state_list, QOBJECT(set_state)); + } else if (rule->action == ACTION_SUSPEND) { + QDict *suspend = qdict_new(); + + qdict_put_obj(suspend, "event", QOBJECT(qstring_from_str( + BlkdebugEvent_lookup[rule->event]))); + qdict_put_obj(suspend, "state", + QOBJECT(qint_from_int(rule->state))); + qdict_put_obj(suspend, "tag", QOBJECT(qstring_from_str( + rule->options.suspend.tag))); + + if (!suspend_list) { + suspend_list = qlist_new(); + } + + qlist_append_obj(suspend_list, QOBJECT(suspend)); + } + } + } + + if (inject_error_list) { + qdict_put_obj(opts, "inject-error", QOBJECT(inject_error_list)); + } + if (set_state_list) { + qdict_put_obj(opts, "set-state", QOBJECT(set_state_list)); + } + if (suspend_list) { + qdict_put_obj(opts, "suspend", QOBJECT(suspend_list)); + } + + bs->full_open_options = opts; +} + static BlockDriver bdrv_blkdebug = { .format_name = "blkdebug", .protocol_name = "blkdebug", @@ -715,6 +811,7 @@ static BlockDriver bdrv_blkdebug = { .bdrv_file_open = blkdebug_open, .bdrv_close = blkdebug_close, .bdrv_getlength = blkdebug_getlength, + .bdrv_refresh_filename = blkdebug_refresh_filename, .bdrv_aio_readv = blkdebug_aio_readv, .bdrv_aio_writev = blkdebug_aio_writev, -- cgit v1.2.3 From 74b36b2eb840c0f63d4ed2cfbfc8bb3c0d23cdd2 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 18 Jul 2014 20:24:58 +0200 Subject: blkverify: Implement bdrv_refresh_filename() Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/blkverify.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/block/blkverify.c b/block/blkverify.c index 621b78593b..7c78ca41a5 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -10,6 +10,8 @@ #include #include "qemu/sockets.h" /* for EINPROGRESS on Windows */ #include "block/block_int.h" +#include "qapi/qmp/qdict.h" +#include "qapi/qmp/qstring.h" typedef struct { BlockDriverState *test_file; @@ -320,6 +322,32 @@ static void blkverify_attach_aio_context(BlockDriverState *bs, bdrv_attach_aio_context(s->test_file, new_context); } +static void blkverify_refresh_filename(BlockDriverState *bs) +{ + BDRVBlkverifyState *s = bs->opaque; + + /* bs->file has already been refreshed */ + bdrv_refresh_filename(s->test_file); + + if (bs->file->full_open_options && s->test_file->full_open_options) { + QDict *opts = qdict_new(); + qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("blkverify"))); + + QINCREF(bs->file->full_open_options); + qdict_put_obj(opts, "raw", QOBJECT(bs->file->full_open_options)); + QINCREF(s->test_file->full_open_options); + qdict_put_obj(opts, "test", QOBJECT(s->test_file->full_open_options)); + + bs->full_open_options = opts; + } + + if (bs->file->exact_filename[0] && s->test_file->exact_filename[0]) { + snprintf(bs->exact_filename, sizeof(bs->exact_filename), + "blkverify:%s:%s", + bs->file->exact_filename, s->test_file->exact_filename); + } +} + static BlockDriver bdrv_blkverify = { .format_name = "blkverify", .protocol_name = "blkverify", @@ -329,6 +357,7 @@ static BlockDriver bdrv_blkverify = { .bdrv_file_open = blkverify_open, .bdrv_close = blkverify_close, .bdrv_getlength = blkverify_getlength, + .bdrv_refresh_filename = blkverify_refresh_filename, .bdrv_aio_readv = blkverify_aio_readv, .bdrv_aio_writev = blkverify_aio_writev, -- cgit v1.2.3 From 2019d68b3b4bf16228779ff50c4422c07b504824 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 18 Jul 2014 20:24:59 +0200 Subject: nbd: Implement bdrv_refresh_filename() Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/nbd.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/block/nbd.c b/block/nbd.c index 4eda0958d7..89775e1e2b 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -31,8 +31,10 @@ #include "block/block_int.h" #include "qemu/module.h" #include "qemu/sockets.h" +#include "qapi/qmp/qdict.h" #include "qapi/qmp/qjson.h" #include "qapi/qmp/qint.h" +#include "qapi/qmp/qstring.h" #include #include @@ -338,6 +340,37 @@ static void nbd_attach_aio_context(BlockDriverState *bs, nbd_client_session_attach_aio_context(&s->client, new_context); } +static void nbd_refresh_filename(BlockDriverState *bs) +{ + BDRVNBDState *s = bs->opaque; + QDict *opts = qdict_new(); + const char *path = qemu_opt_get(s->socket_opts, "path"); + const char *host = qemu_opt_get(s->socket_opts, "host"); + const char *port = qemu_opt_get(s->socket_opts, "port"); + const char *export = qemu_opt_get(s->socket_opts, "export"); + + qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd"))); + + if (path) { + snprintf(bs->exact_filename, sizeof(bs->exact_filename), + "nbd+unix:%s", path); + qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(path))); + } else if (export) { + snprintf(bs->exact_filename, sizeof(bs->exact_filename), + "nbd:%s:%s/%s", host, port, export); + qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host))); + qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port))); + qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(export))); + } else { + snprintf(bs->exact_filename, sizeof(bs->exact_filename), + "nbd:%s:%s", host, port); + qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host))); + qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port))); + } + + bs->full_open_options = opts; +} + static BlockDriver bdrv_nbd = { .format_name = "nbd", .protocol_name = "nbd", @@ -352,6 +385,7 @@ static BlockDriver bdrv_nbd = { .bdrv_getlength = nbd_getlength, .bdrv_detach_aio_context = nbd_detach_aio_context, .bdrv_attach_aio_context = nbd_attach_aio_context, + .bdrv_refresh_filename = nbd_refresh_filename, }; static BlockDriver bdrv_nbd_tcp = { @@ -368,6 +402,7 @@ static BlockDriver bdrv_nbd_tcp = { .bdrv_getlength = nbd_getlength, .bdrv_detach_aio_context = nbd_detach_aio_context, .bdrv_attach_aio_context = nbd_attach_aio_context, + .bdrv_refresh_filename = nbd_refresh_filename, }; static BlockDriver bdrv_nbd_unix = { @@ -384,6 +419,7 @@ static BlockDriver bdrv_nbd_unix = { .bdrv_getlength = nbd_getlength, .bdrv_detach_aio_context = nbd_detach_aio_context, .bdrv_attach_aio_context = nbd_attach_aio_context, + .bdrv_refresh_filename = nbd_refresh_filename, }; static void bdrv_nbd_init(void) -- cgit v1.2.3 From fafcfe228df4353d35ae523e9b78158fb764e79a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 18 Jul 2014 20:25:00 +0200 Subject: quorum: Implement bdrv_refresh_filename() Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/quorum.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/block/quorum.c b/block/quorum.c index d5ee9c0059..0de07bb036 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -16,7 +16,12 @@ #include #include #include "block/block_int.h" +#include "qapi/qmp/qbool.h" +#include "qapi/qmp/qdict.h" +#include "qapi/qmp/qint.h" #include "qapi/qmp/qjson.h" +#include "qapi/qmp/qlist.h" +#include "qapi/qmp/qstring.h" #include "qapi-event.h" #define HASH_LENGTH 32 @@ -945,6 +950,39 @@ static void quorum_attach_aio_context(BlockDriverState *bs, } } +static void quorum_refresh_filename(BlockDriverState *bs) +{ + BDRVQuorumState *s = bs->opaque; + QDict *opts; + QList *children; + int i; + + for (i = 0; i < s->num_children; i++) { + bdrv_refresh_filename(s->bs[i]); + if (!s->bs[i]->full_open_options) { + return; + } + } + + children = qlist_new(); + for (i = 0; i < s->num_children; i++) { + QINCREF(s->bs[i]->full_open_options); + qlist_append_obj(children, QOBJECT(s->bs[i]->full_open_options)); + } + + opts = qdict_new(); + qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("quorum"))); + qdict_put_obj(opts, QUORUM_OPT_VOTE_THRESHOLD, + QOBJECT(qint_from_int(s->threshold))); + qdict_put_obj(opts, QUORUM_OPT_BLKVERIFY, + QOBJECT(qbool_from_int(s->is_blkverify))); + qdict_put_obj(opts, QUORUM_OPT_REWRITE, + QOBJECT(qbool_from_int(s->rewrite_corrupted))); + qdict_put_obj(opts, "children", QOBJECT(children)); + + bs->full_open_options = opts; +} + static BlockDriver bdrv_quorum = { .format_name = "quorum", .protocol_name = "quorum", @@ -953,6 +991,7 @@ static BlockDriver bdrv_quorum = { .bdrv_file_open = quorum_open, .bdrv_close = quorum_close, + .bdrv_refresh_filename = quorum_refresh_filename, .bdrv_co_flush_to_disk = quorum_co_flush, -- cgit v1.2.3 From 911864c6e5f58a86ac987aa4d2e5743851374c05 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 18 Jul 2014 20:25:01 +0200 Subject: iotests: Add test for image filename construction Testing a real in-use protocol such as NBD is hard; testing blkdebug and blkverify in its stead is easier and tests basically the same functionality. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/099 | 116 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/099.out | 20 ++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 137 insertions(+) create mode 100755 tests/qemu-iotests/099 create mode 100644 tests/qemu-iotests/099.out diff --git a/tests/qemu-iotests/099 b/tests/qemu-iotests/099 new file mode 100755 index 0000000000..a26d3d243e --- /dev/null +++ b/tests/qemu-iotests/099 @@ -0,0 +1,116 @@ +#!/bin/bash +# +# Test valid filenames for blkdebug and blkverify representatively for +# other protocols (such as NBD) when queried +# +# Copyright (C) 2014 Red Hat, Inc. +# +# 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 . +# + +# creator +owner=mreitz@redhat.com + +seq="$(basename $0)" +echo "QA output created by $seq" + +here="$PWD" +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +# Basically all formats, but "raw" has issues with _filter_imgfmt regarding the +# raw comparison image for blkverify; also, all images have to support creation +_supported_fmt cow qcow qcow2 qed vdi vhdx vmdk vpc +_supported_proto file +_supported_os Linux + + +function do_run_qemu() +{ + $QEMU -nographic -qmp stdio -serial none "$@" +} + +function run_qemu() +{ + # Get the "file": "foo" entry ($foo may only contain escaped double quotes, + # which is how we can extract it) + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_imgfmt | _filter_qmp \ + | grep "drv0" \ + | sed -e 's/^.*"file": "\(\(\\"\|[^"]\)*\)".*$/\1/' -e 's/\\"/"/g' +} + +function test_qemu() +{ + run_qemu -drive "if=none,id=drv0,$1" < Date: Wed, 20 Aug 2014 19:02:38 +0400 Subject: block/vvfat.c: remove debugging code to reinit stderr if NULL Just log to stderr unconditionally, like other similar code does. Signed-off-by: Michael Tokarev Reviewed-by: Peter Maydell Signed-off-by: Kevin Wolf --- block/vvfat.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index e56f766338..731e591ec1 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -52,10 +52,6 @@ #define DLOG(a) a -#undef stderr -#define stderr STDERR -FILE* stderr = NULL; - static void checkpoint(void); #ifdef __MINGW32__ @@ -1082,11 +1078,6 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags, vvv = s; #endif -DLOG(if (stderr == NULL) { - stderr = fopen("vvfat.log", "a"); - setbuf(stderr, NULL); -}) - opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, options, &local_err); if (local_err) { -- cgit v1.2.3 From d832fb4d66ead62da4af7e44cce34cd939e865e1 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Fri, 22 Aug 2014 10:08:49 +0200 Subject: block/iscsi: fix memory corruption on iscsi resize bs->total_sectors is not yet updated at this point. resulting in memory corruption if the volume has grown and data is written to the newly availble areas. CC: qemu-stable@nongnu.org Signed-off-by: Peter Lieven Signed-off-by: Kevin Wolf --- block/iscsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/iscsi.c b/block/iscsi.c index cdd19c2bda..3e19202488 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1512,7 +1512,8 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset) if (iscsilun->allocationmap != NULL) { g_free(iscsilun->allocationmap); iscsilun->allocationmap = - bitmap_new(DIV_ROUND_UP(bs->total_sectors, + bitmap_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks, + iscsilun), iscsilun->cluster_sectors)); } -- cgit v1.2.3 From 61ed73cff427206b3a959b18a4877952f566279b Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 21 Aug 2014 13:44:07 +0100 Subject: raw-posix: fix O_DIRECT short reads The following O_DIRECT read from a <512 byte file fails: $ truncate -s 320 test.img $ qemu-io -n -c 'read -P 0 0 512' test.img qemu-io: can't open device test.img: Could not read image for determining its format: Invalid argument Note that qemu-io completes successfully without the -n (O_DIRECT) option. This patch fixes qemu-iotests ./check -nocache -vmdk 059. Cc: qemu-stable@nongnu.org Suggested-by: Kevin Wolf Reported-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/raw-posix.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/block/raw-posix.c b/block/raw-posix.c index 5c745b9a18..d737f3a0c5 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -747,6 +747,15 @@ static ssize_t handle_aiocb_rw_linear(RawPosixAIOData *aiocb, char *buf) } if (len == -1 && errno == EINTR) { continue; + } else if (len == -1 && errno == EINVAL && + (aiocb->bs->open_flags & BDRV_O_NOCACHE) && + !(aiocb->aio_type & QEMU_AIO_WRITE) && + offset > 0) { + /* O_DIRECT pread() may fail with EINVAL when offset is unaligned + * after a short read. Assume that O_DIRECT short reads only occur + * at EOF. Therefore this is a short read, not an I/O error. + */ + break; } else if (len == -1) { offset = -errno; break; -- cgit v1.2.3 From 8d9eb33ca0bbb8bca0f1775623ed3cf5f39760cd Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 21 Aug 2014 13:44:08 +0100 Subject: qemu-iotests: add test case 101 for short file I/O Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/qemu-iotests/101 | 58 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/101.out | 10 ++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 69 insertions(+) create mode 100755 tests/qemu-iotests/101 create mode 100644 tests/qemu-iotests/101.out diff --git a/tests/qemu-iotests/101 b/tests/qemu-iotests/101 new file mode 100755 index 0000000000..70fbf25f68 --- /dev/null +++ b/tests/qemu-iotests/101 @@ -0,0 +1,58 @@ +#!/bin/bash +# +# Test short file I/O +# +# Copyright (C) 2014 Red Hat, Inc. +# +# 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 . +# + +# creator +owner=stefanha@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt raw +_supported_proto file +_supported_os Linux + + +echo +echo "== creating short image file ==" +dd if=/dev/zero of="$TEST_IMG" bs=1 count=320 + +echo +echo "== reading bytes beyond EOF gives zeroes ==" +$QEMU_IO -c "read -P 0 0 512" "$TEST_IMG" | _filter_qemu_io + + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/101.out b/tests/qemu-iotests/101.out new file mode 100644 index 0000000000..9a996e8fc8 --- /dev/null +++ b/tests/qemu-iotests/101.out @@ -0,0 +1,10 @@ +QA output created by 101 + +== creating short image file == +320+0 records in +320+0 records out + +== reading bytes beyond EOF gives zeroes == +read 512/512 bytes at offset 0 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index f157864c66..2803d68d62 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -101,4 +101,5 @@ 092 rw auto quick 095 rw auto quick 099 rw auto quick +101 rw auto quick 103 rw auto quick -- cgit v1.2.3 From cbf95a0b117461473f05ab3cce4d01ba2b29e60a Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 22 Aug 2014 12:45:50 +0800 Subject: blkdebug: Delete BH in bdrv_aio_cancel Otherwise error_callback_bh will access the already released acb. Cc: qemu-stable@nongnu.org Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/blkdebug.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/block/blkdebug.c b/block/blkdebug.c index 95b72445a7..69b330eced 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -453,6 +453,10 @@ static void error_callback_bh(void *opaque) static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb) { BlkdebugAIOCB *acb = container_of(blockacb, BlkdebugAIOCB, common); + if (acb->bh) { + qemu_bh_delete(acb->bh); + acb->bh = NULL; + } qemu_aio_release(acb); } -- cgit v1.2.3 From 0a156f7c750c4e4e1c5dfb2135debcf399e4e2a7 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 21 Aug 2014 14:36:19 +0200 Subject: vmdk: Use bdrv_nb_sectors() where sectors, not bytes are wanted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of bdrv_getlength(). Commit 57322b7 did this all over block, but one more bdrv_getlength() has crept in since. Signed-off-by: Markus Armbruster Reviewed-by: Fam Zheng Reviewed-by: Benoît Canet Signed-off-by: Kevin Wolf --- block/vmdk.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index f000d2a613..07cb62ceb7 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -397,7 +397,7 @@ static int vmdk_add_extent(BlockDriverState *bs, { VmdkExtent *extent; BDRVVmdkState *s = bs->opaque; - int64_t length; + int64_t nb_sectors; if (cluster_sectors > 0x200000) { /* 0x200000 * 512Bytes = 1GB for one cluster is unrealistic */ @@ -413,9 +413,9 @@ static int vmdk_add_extent(BlockDriverState *bs, return -EFBIG; } - length = bdrv_getlength(file); - if (length < 0) { - return length; + nb_sectors = bdrv_nb_sectors(file); + if (nb_sectors < 0) { + return nb_sectors; } s->extents = g_renew(VmdkExtent, s->extents, s->num_extents + 1); @@ -432,8 +432,7 @@ static int vmdk_add_extent(BlockDriverState *bs, extent->l1_entry_sectors = l2_size * cluster_sectors; extent->l2_size = l2_size; extent->cluster_sectors = flat ? sectors : cluster_sectors; - extent->next_cluster_sector = - ROUND_UP(DIV_ROUND_UP(length, BDRV_SECTOR_SIZE), cluster_sectors); + extent->next_cluster_sector = ROUND_UP(nb_sectors, cluster_sectors); if (s->num_extents > 1) { extent->end_sector = (*(extent - 1)).end_sector + extent->sectors; -- cgit v1.2.3 From 40055951a7afbfc037c6c7351d72a5c5d83ed99b Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 22 Jul 2014 22:58:42 +0200 Subject: qemu-img: Allow source cache mode specification Many qemu-img subcommands only read the source file(s) once. For these use cases, a full write-back cache is unnecessary and mainly clutters host cache memory. Though this is generally no concern as cache memory is freely available and can be scaled by the host OS, it may become a concern with thin provisioning. For these cases, it makes sense to allow users to freely specify the source cache mode (e.g. use no cache at all). This commit adds a new switch (-T) for the qemu-img subcommands check, compare, convert and rebase to specify the cache to be used for source images (the backing file in case of rebase). Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- qemu-img-cmds.hx | 16 ++++++------ qemu-img.c | 78 ++++++++++++++++++++++++++++++++++++++++++++------------ qemu-img.texi | 14 +++++++--- 3 files changed, 80 insertions(+), 28 deletions(-) diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index d02960921a..48df0b83c2 100644 --- a/qemu-img-cmds.hx +++ b/qemu-img-cmds.hx @@ -10,9 +10,9 @@ STEXI ETEXI DEF("check", img_check, - "check [-q] [-f fmt] [--output=ofmt] [-r [leaks | all]] filename") + "check [-q] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache] filename") STEXI -@item check [-q] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] @var{filename} +@item check [-q] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T @var{src_cache}] @var{filename} ETEXI DEF("create", img_create, @@ -28,15 +28,15 @@ STEXI ETEXI DEF("compare", img_compare, - "compare [-f fmt] [-F fmt] [-p] [-q] [-s] filename1 filename2") + "compare [-f fmt] [-F fmt] [-T src_cache] [-p] [-q] [-s] filename1 filename2") STEXI -@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-q] [-s] @var{filename1} @var{filename2} +@item compare [-f @var{fmt}] [-F @var{fmt}] [-T @var{src_cache}] [-p] [-q] [-s] @var{filename1} @var{filename2} ETEXI DEF("convert", img_convert, - "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename") + "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename") STEXI -@item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} +@item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} ETEXI DEF("info", img_info, @@ -58,9 +58,9 @@ STEXI ETEXI DEF("rebase", img_rebase, - "rebase [-q] [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] filename") + "rebase [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] -b backing_file [-F backing_fmt] filename") STEXI -@item rebase [-q] [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename} +@item rebase [-q] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename} ETEXI DEF("resize", img_resize, diff --git a/qemu-img.c b/qemu-img.c index 18caa4b450..df854059be 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -109,6 +109,7 @@ static void QEMU_NORETURN help(void) " 'cache' is the cache mode used to write the output disk image, the valid\n" " options are: 'none', 'writeback' (default, except for convert), 'writethrough',\n" " 'directsync' and 'unsafe' (default for convert)\n" + " 'src_cache' in contrast is the cache mode used to read input disk images\n" " 'size' is the disk image size in bytes. Optional suffixes\n" " 'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M),\n" " 'T' (terabyte, 1024G), 'P' (petabyte, 1024T) and 'E' (exabyte, 1024P) are\n" @@ -591,7 +592,7 @@ static int img_check(int argc, char **argv) { int c, ret; OutputFormat output_format = OFORMAT_HUMAN; - const char *filename, *fmt, *output; + const char *filename, *fmt, *output, *cache; BlockDriverState *bs; int fix = 0; int flags = BDRV_O_FLAGS | BDRV_O_CHECK; @@ -600,6 +601,7 @@ static int img_check(int argc, char **argv) fmt = NULL; output = NULL; + cache = BDRV_DEFAULT_CACHE; for(;;) { int option_index = 0; static const struct option long_options[] = { @@ -609,7 +611,7 @@ static int img_check(int argc, char **argv) {"output", required_argument, 0, OPTION_OUTPUT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, "f:hr:q", + c = getopt_long(argc, argv, "hf:r:T:q", long_options, &option_index); if (c == -1) { break; @@ -637,6 +639,9 @@ static int img_check(int argc, char **argv) case OPTION_OUTPUT: output = optarg; break; + case 'T': + cache = optarg; + break; case 'q': quiet = true; break; @@ -656,6 +661,12 @@ static int img_check(int argc, char **argv) return 1; } + ret = bdrv_parse_cache_flags(cache, &flags); + if (ret < 0) { + error_report("Invalid source cache option: %s", cache); + return 1; + } + bs = bdrv_new_open("image", filename, fmt, flags, true, quiet); if (!bs) { return 1; @@ -948,7 +959,7 @@ static int check_empty_sectors(BlockDriverState *bs, int64_t sect_num, */ static int img_compare(int argc, char **argv) { - const char *fmt1 = NULL, *fmt2 = NULL, *filename1, *filename2; + const char *fmt1 = NULL, *fmt2 = NULL, *cache, *filename1, *filename2; BlockDriverState *bs1, *bs2; int64_t total_sectors1, total_sectors2; uint8_t *buf1 = NULL, *buf2 = NULL; @@ -956,14 +967,16 @@ static int img_compare(int argc, char **argv) int allocated1, allocated2; int ret = 0; /* return value - 0 Ident, 1 Different, >1 Error */ bool progress = false, quiet = false, strict = false; + int flags; int64_t total_sectors; int64_t sector_num = 0; int64_t nb_sectors; int c, pnum; uint64_t progress_base; + cache = BDRV_DEFAULT_CACHE; for (;;) { - c = getopt(argc, argv, "hpf:F:sq"); + c = getopt(argc, argv, "hf:F:T:pqs"); if (c == -1) { break; } @@ -978,6 +991,9 @@ static int img_compare(int argc, char **argv) case 'F': fmt2 = optarg; break; + case 'T': + cache = optarg; + break; case 'p': progress = true; break; @@ -1002,17 +1018,25 @@ static int img_compare(int argc, char **argv) filename1 = argv[optind++]; filename2 = argv[optind++]; + flags = BDRV_O_FLAGS; + ret = bdrv_parse_cache_flags(cache, &flags); + if (ret < 0) { + error_report("Invalid source cache option: %s", cache); + ret = 2; + goto out3; + } + /* Initialize before goto out */ qemu_progress_init(progress, 2.0); - bs1 = bdrv_new_open("image 1", filename1, fmt1, BDRV_O_FLAGS, true, quiet); + bs1 = bdrv_new_open("image 1", filename1, fmt1, flags, true, quiet); if (!bs1) { error_report("Can't open file %s", filename1); ret = 2; goto out3; } - bs2 = bdrv_new_open("image 2", filename2, fmt2, BDRV_O_FLAGS, true, quiet); + bs2 = bdrv_new_open("image 2", filename2, fmt2, flags, true, quiet); if (!bs2) { error_report("Can't open file %s", filename2); ret = 2; @@ -1191,8 +1215,8 @@ static int img_convert(int argc, char **argv) { int c, n, n1, bs_n, bs_i, compress, cluster_sectors, skip_create; int64_t ret = 0; - int progress = 0, flags; - const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename; + int progress = 0, flags, src_flags; + const char *fmt, *out_fmt, *cache, *src_cache, *out_baseimg, *out_filename; BlockDriver *drv, *proto_drv; BlockDriverState **bs = NULL, *out_bs = NULL; int64_t total_sectors, nb_sectors, sector_num, bs_offset; @@ -1214,11 +1238,12 @@ static int img_convert(int argc, char **argv) fmt = NULL; out_fmt = "raw"; cache = "unsafe"; + src_cache = BDRV_DEFAULT_CACHE; out_baseimg = NULL; compress = 0; skip_create = 0; for(;;) { - c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qnl:"); + c = getopt(argc, argv, "hf:O:B:ce6o:s:l:S:pt:T:qn"); if (c == -1) { break; } @@ -1299,6 +1324,9 @@ static int img_convert(int argc, char **argv) case 't': cache = optarg; break; + case 'T': + src_cache = optarg; + break; case 'q': quiet = true; break; @@ -1335,6 +1363,13 @@ static int img_convert(int argc, char **argv) goto out; } + src_flags = BDRV_O_FLAGS; + ret = bdrv_parse_cache_flags(src_cache, &src_flags); + if (ret < 0) { + error_report("Invalid source cache option: %s", src_cache); + goto out; + } + qemu_progress_print(0, 100); bs = g_new0(BlockDriverState *, bs_n); @@ -1344,7 +1379,7 @@ static int img_convert(int argc, char **argv) for (bs_i = 0; bs_i < bs_n; bs_i++) { char *id = bs_n > 1 ? g_strdup_printf("source %d", bs_i) : g_strdup("source"); - bs[bs_i] = bdrv_new_open(id, argv[optind + bs_i], fmt, BDRV_O_FLAGS, + bs[bs_i] = bdrv_new_open(id, argv[optind + bs_i], fmt, src_flags, true, quiet); g_free(id); if (!bs[bs_i]) { @@ -2291,8 +2326,8 @@ static int img_rebase(int argc, char **argv) BlockDriverState *bs, *bs_old_backing = NULL, *bs_new_backing = NULL; BlockDriver *old_backing_drv, *new_backing_drv; char *filename; - const char *fmt, *cache, *out_basefmt, *out_baseimg; - int c, flags, ret; + const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg; + int c, flags, src_flags, ret; int unsafe = 0; int progress = 0; bool quiet = false; @@ -2301,10 +2336,11 @@ static int img_rebase(int argc, char **argv) /* Parse commandline parameters */ fmt = NULL; cache = BDRV_DEFAULT_CACHE; + src_cache = BDRV_DEFAULT_CACHE; out_baseimg = NULL; out_basefmt = NULL; for(;;) { - c = getopt(argc, argv, "uhf:F:b:pt:q"); + c = getopt(argc, argv, "hf:F:b:upt:T:q"); if (c == -1) { break; } @@ -2331,6 +2367,9 @@ static int img_rebase(int argc, char **argv) case 't': cache = optarg; break; + case 'T': + src_cache = optarg; + break; case 'q': quiet = true; break; @@ -2359,6 +2398,13 @@ static int img_rebase(int argc, char **argv) return -1; } + src_flags = BDRV_O_FLAGS; + ret = bdrv_parse_cache_flags(src_cache, &src_flags); + if (ret < 0) { + error_report("Invalid source cache option: %s", src_cache); + return -1; + } + /* * Open the images. * @@ -2402,7 +2448,7 @@ static int img_rebase(int argc, char **argv) bs_old_backing = bdrv_new("old_backing", &error_abort); bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name)); - ret = bdrv_open(&bs_old_backing, backing_name, NULL, NULL, BDRV_O_FLAGS, + ret = bdrv_open(&bs_old_backing, backing_name, NULL, NULL, src_flags, old_backing_drv, &local_err); if (ret) { error_report("Could not open old backing file '%s': %s", @@ -2412,8 +2458,8 @@ static int img_rebase(int argc, char **argv) } if (out_baseimg[0]) { bs_new_backing = bdrv_new("new_backing", &error_abort); - ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL, - BDRV_O_FLAGS, new_backing_drv, &local_err); + ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL, src_flags, + new_backing_drv, &local_err); if (ret) { error_report("Could not open new backing file '%s': %s", out_baseimg, error_get_pretty(local_err)); diff --git a/qemu-img.texi b/qemu-img.texi index 514be90f7d..688b28d87b 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -72,6 +72,9 @@ down to the nearest 512 bytes. You may use the common size suffixes like specifies the cache mode that should be used with the (destination) file. See the documentation of the emulator's @code{-drive cache=...} option for allowed values. +@item -T @var{src_cache} +in contrast specifies the cache mode that should be used with the source +file(s). @end table Parameters to snapshot subcommand: @@ -113,7 +116,7 @@ Skip the creation of the target volume Command description: @table @option -@item check [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] @var{filename} +@item check [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T @var{src_cache}] @var{filename} Perform a consistency check on the disk image @var{filename}. The command can output in the format @var{ofmt} which is either @code{human} or @code{json}. @@ -172,7 +175,7 @@ the backing file, the backing file will not be truncated. If you want the backing file to match the size of the smaller snapshot, you can safely truncate it yourself once the commit operation successfully completes. -@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} @var{filename2} +@item compare [-f @var{fmt}] [-F @var{fmt}] [-T @var{src_cache}] [-p] [-s] [-q] @var{filename1} @var{filename2} Check if two images have the same content. You can compare images with different format or settings. @@ -213,7 +216,7 @@ Error on reading data @end table -@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} +@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} Convert the disk image @var{filename} or a snapshot @var{snapshot_param}(@var{snapshot_id_or_name} is deprecated) to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c} @@ -325,7 +328,7 @@ source code. List, apply, create or delete snapshots in image @var{filename}. -@item rebase [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename} +@item rebase [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename} Changes the backing file of an image. Only the formats @code{qcow2} and @code{qed} support changing the backing file. @@ -336,6 +339,9 @@ The backing file is changed to @var{backing_file} and (if the image format of string), then the image is rebased onto no backing file (i.e. it will exist independently of any backing file). +@var{cache} specifies the cache mode to be used for @var{filename}, whereas +@var{src_cache} specifies the cache mode for reading the new backing file. + There are two different modes in which @code{rebase} can operate: @table @option @item Safe mode -- cgit v1.2.3 From bd39e6ed0b88a1473c652c97e731a156cccf16e2 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 22 Jul 2014 22:58:43 +0200 Subject: qemu-img: Allow cache mode specification for amend qemu-img amend may extensively modify the target image, depending on the options to be amended (e.g. conversion to qcow2 compat level 0.10 from 1.1 for an image with many unallocated zero clusters). Therefore it makes sense to allow the user to specify the cache mode to be used. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- qemu-img-cmds.hx | 4 ++-- qemu-img.c | 19 +++++++++++++++---- qemu-img.texi | 2 +- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index 48df0b83c2..55aec6bded 100644 --- a/qemu-img-cmds.hx +++ b/qemu-img-cmds.hx @@ -70,8 +70,8 @@ STEXI ETEXI DEF("amend", img_amend, - "amend [-q] [-f fmt] -o options filename") + "amend [-q] [-f fmt] [-t cache] -o options filename") STEXI -@item amend [-q] [-f @var{fmt}] -o @var{options} @var{filename} +@item amend [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename} @end table ETEXI diff --git a/qemu-img.c b/qemu-img.c index df854059be..c8434206b8 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2778,12 +2778,14 @@ static int img_amend(int argc, char **argv) char *options = NULL; QemuOptsList *create_opts = NULL; QemuOpts *opts = NULL; - const char *fmt = NULL, *filename; + const char *fmt = NULL, *filename, *cache; + int flags; bool quiet = false; BlockDriverState *bs = NULL; + cache = BDRV_DEFAULT_CACHE; for (;;) { - c = getopt(argc, argv, "hqf:o:"); + c = getopt(argc, argv, "ho:f:t:q"); if (c == -1) { break; } @@ -2810,6 +2812,9 @@ static int img_amend(int argc, char **argv) case 'f': fmt = optarg; break; + case 't': + cache = optarg; + break; case 'q': quiet = true; break; @@ -2832,8 +2837,14 @@ static int img_amend(int argc, char **argv) error_exit("Expecting one image file name"); } - bs = bdrv_new_open("image", filename, fmt, - BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet); + flags = BDRV_O_FLAGS | BDRV_O_RDWR; + ret = bdrv_parse_cache_flags(cache, &flags); + if (ret < 0) { + error_report("Invalid cache option: %s", cache); + goto out; + } + + bs = bdrv_new_open("image", filename, fmt, flags, true, quiet); if (!bs) { error_report("Could not open image '%s'", filename); ret = -1; diff --git a/qemu-img.texi b/qemu-img.texi index 688b28d87b..cb689483b6 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -397,7 +397,7 @@ After using this command to grow a disk image, you must use file system and partitioning tools inside the VM to actually begin using the new space on the device. -@item amend [-f @var{fmt}] -o @var{options} @var{filename} +@item amend [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename} Amends the image format specific @var{options} for the image file @var{filename}. Not all file formats support this operation. -- cgit v1.2.3