From patchwork Tue Oct 7 11:59:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Markus Armbruster X-Patchwork-Id: 397258 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id E4397140096 for ; Tue, 7 Oct 2014 23:10:08 +1100 (EST) Received: from localhost ([::1]:58063 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbTag-0005WR-Ox for incoming@patchwork.ozlabs.org; Tue, 07 Oct 2014 08:10:06 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34167) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbTQh-0006Fa-0i for qemu-devel@nongnu.org; Tue, 07 Oct 2014 07:59:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XbTQX-0002YG-Rs for qemu-devel@nongnu.org; Tue, 07 Oct 2014 07:59:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7145) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbTQX-0002YA-H0 for qemu-devel@nongnu.org; Tue, 07 Oct 2014 07:59:37 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s97BxZqq010502 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 7 Oct 2014 07:59:35 -0400 Received: from blackfin.pond.sub.org (ovpn-116-51.ams2.redhat.com [10.36.116.51]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s97BxV7O018873 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 7 Oct 2014 07:59:32 -0400 Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id E48A23043FC1; Tue, 7 Oct 2014 13:59:26 +0200 (CEST) From: Markus Armbruster To: qemu-devel@nongnu.org Date: Tue, 7 Oct 2014 13:59:11 +0200 Message-Id: <1412683166-4934-10-git-send-email-armbru@redhat.com> In-Reply-To: <1412683166-4934-1-git-send-email-armbru@redhat.com> References: <1412683166-4934-1-git-send-email-armbru@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, benoit.canet@nodalink.com, stefanha@redhat.com, mreitz@redhat.com Subject: [Qemu-devel] [PATCH v6 09/24] block: Eliminate BlockDriverState member device_name[] X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org device_name[] can become non-empty only in bdrv_new_root() and bdrv_move_feature_fields(). The latter is used only to undo damage done by bdrv_swap(). The former is called only by blk_new_with_bs(). Therefore, when a BlockDriverState's device_name[] is non-empty, then it's been created with a BlockBackend, and vice versa. Furthermore, blk_new_with_bs() keeps the two names equal. Therefore, device_name[] is redundant. Eliminate it. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz --- block-migration.c | 12 +++++++----- block.c | 49 ++++++++++++++++++++++++++--------------------- block/mirror.c | 3 ++- block/qapi.c | 6 +++--- block/qcow.c | 4 ++-- block/qcow2.c | 4 ++-- block/qed.c | 2 +- block/quorum.c | 4 ++-- block/vdi.c | 2 +- block/vhdx.c | 2 +- block/vmdk.c | 4 ++-- block/vpc.c | 2 +- block/vvfat.c | 2 +- blockjob.c | 3 ++- include/block/block.h | 2 +- include/block/block_int.h | 2 -- 16 files changed, 55 insertions(+), 48 deletions(-) diff --git a/block-migration.c b/block-migration.c index cb3e16c..da30e93 100644 --- a/block-migration.c +++ b/block-migration.c @@ -14,7 +14,9 @@ */ #include "qemu-common.h" -#include "block/block_int.h" +#include "block/block.h" +#include "qemu/error-report.h" +#include "qemu/main-loop.h" #include "hw/hw.h" #include "qemu/queue.h" #include "qemu/timer.h" @@ -130,9 +132,9 @@ static void blk_send(QEMUFile *f, BlkMigBlock * blk) | flags); /* device name */ - len = strlen(blk->bmds->bs->device_name); + len = strlen(bdrv_get_device_name(blk->bmds->bs)); qemu_put_byte(f, len); - qemu_put_buffer(f, (uint8_t *)blk->bmds->bs->device_name, len); + qemu_put_buffer(f, (uint8_t *)bdrv_get_device_name(blk->bmds->bs), len); /* if a block is zero we need to flush here since the network * bandwidth is now a lot higher than the storage device bandwidth. @@ -382,9 +384,9 @@ static void init_blk_migration(QEMUFile *f) if (bmds->shared_base) { DPRINTF("Start migration for %s with shared base image\n", - bs->device_name); + bdrv_get_device_name(bs)); } else { - DPRINTF("Start full migration for %s\n", bs->device_name); + DPRINTF("Start full migration for %s\n", bdrv_get_device_name(bs)); } QSIMPLEQ_INSERT_TAIL(&block_mig_state.bmds_list, bmds, entry); diff --git a/block.c b/block.c index 5fd173a..fd57cc0 100644 --- a/block.c +++ b/block.c @@ -28,6 +28,7 @@ #include "block/blockjob.h" #include "qemu/module.h" #include "qapi/qmp/qjson.h" +#include "sysemu/block-backend.h" #include "sysemu/sysemu.h" #include "qemu/notify.h" #include "block/coroutine.h" @@ -360,9 +361,7 @@ BlockDriverState *bdrv_new_root(const char *device_name, Error **errp) bs = bdrv_new(); - pstrcpy(bs->device_name, sizeof(bs->device_name), device_name); QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list); - return bs; } @@ -1168,7 +1167,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) } else if (backing_hd) { error_setg(&bs->backing_blocker, "device is used as backing hd of '%s'", - bs->device_name); + bdrv_get_device_name(bs)); } bs->backing_hd = backing_hd; @@ -1542,7 +1541,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, } else { error_setg(errp, "Block format '%s' used by device '%s' doesn't " "support the option '%s'", drv->format_name, - bs->device_name, entry->key); + bdrv_get_device_name(bs), entry->key); } ret = -EINVAL; @@ -1749,7 +1748,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) && reopen_state->flags & BDRV_O_RDWR) { error_set(errp, QERR_DEVICE_IS_READ_ONLY, - reopen_state->bs->device_name); + bdrv_get_device_name(reopen_state->bs)); goto error; } @@ -1776,7 +1775,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, /* It is currently mandatory to have a bdrv_reopen_prepare() * handler for each supported drv. */ error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, - drv->format_name, reopen_state->bs->device_name, + drv->format_name, bdrv_get_device_name(reopen_state->bs), "reopening of file"); ret = -1; goto error; @@ -1964,10 +1963,17 @@ void bdrv_drain_all(void) Also, NULL terminate the device_name to prevent double remove */ void bdrv_make_anon(BlockDriverState *bs) { - if (bs->device_name[0] != '\0') { + /* + * Take care to remove bs from bdrv_states only when it's actually + * in it. Note that bs->device_list.tqe_prev is initially null, + * and gets set to non-null by QTAILQ_INSERT_TAIL(). Establish + * the useful invariant "bs in bdrv_states iff bs->tqe_prev" by + * resetting it to null on remove. + */ + if (bs->device_list.tqe_prev) { QTAILQ_REMOVE(&bdrv_states, bs, device_list); + bs->device_list.tqe_prev = NULL; } - bs->device_name[0] = '\0'; if (bs->node_name[0] != '\0') { QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list); } @@ -2021,8 +2027,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, bs_dest->job = bs_src->job; /* keep the same entry in bdrv_states */ - pstrcpy(bs_dest->device_name, sizeof(bs_dest->device_name), - bs_src->device_name); bs_dest->device_list = bs_src->device_list; bs_dest->blk = bs_src->blk; @@ -2038,7 +2042,7 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, * This will modify the BlockDriverState fields, and swap contents * between bs_new and bs_old. Both bs_new and bs_old are modified. * - * bs_new must be nameless and not attached to a BlockBackend. + * bs_new must not be attached to a BlockBackend. * * This function does not create any image files. */ @@ -2057,8 +2061,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) QTAILQ_REMOVE(&graph_bdrv_states, bs_old, node_list); } - /* bs_new must be nameless and shouldn't have anything fancy enabled */ - assert(bs_new->device_name[0] == '\0'); + /* bs_new must be unattached and shouldn't have anything fancy enabled */ assert(!bs_new->blk); assert(QLIST_EMPTY(&bs_new->dirty_bitmaps)); assert(bs_new->job == NULL); @@ -2075,8 +2078,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) bdrv_move_feature_fields(bs_old, bs_new); bdrv_move_feature_fields(bs_new, &tmp); - /* bs_new must remain nameless and unattached */ - assert(bs_new->device_name[0] == '\0'); + /* bs_new must remain unattached */ assert(!bs_new->blk); /* Check a few fields that should remain attached to the device */ @@ -2104,7 +2106,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) * This will modify the BlockDriverState fields, and swap contents * between bs_new and bs_top. Both bs_new and bs_top are modified. * - * bs_new must be nameless and not attached to a BlockBackend. + * bs_new must not be attached to a BlockBackend. * * This function does not create any image files. */ @@ -3820,7 +3822,7 @@ BlockDriverState *bdrv_find(const char *name) BlockDriverState *bs; QTAILQ_FOREACH(bs, &bdrv_states, device_list) { - if (!strcmp(name, bs->device_name)) { + if (!strcmp(name, bdrv_get_device_name(bs))) { return bs; } } @@ -3906,9 +3908,9 @@ BlockDriverState *bdrv_next(BlockDriverState *bs) return QTAILQ_NEXT(bs, device_list); } -const char *bdrv_get_device_name(BlockDriverState *bs) +const char *bdrv_get_device_name(const BlockDriverState *bs) { - return bs->device_name; + return bs->blk ? blk_name(bs->blk) : ""; } int bdrv_get_flags(BlockDriverState *bs) @@ -5270,13 +5272,15 @@ int bdrv_media_changed(BlockDriverState *bs) void bdrv_eject(BlockDriverState *bs, bool eject_flag) { BlockDriver *drv = bs->drv; + const char *device_name; if (drv && drv->bdrv_eject) { drv->bdrv_eject(bs, eject_flag); } - if (bs->device_name[0] != '\0') { - qapi_event_send_device_tray_moved(bdrv_get_device_name(bs), + device_name = bdrv_get_device_name(bs); + if (device_name[0] != '\0') { + qapi_event_send_device_tray_moved(device_name, eject_flag, &error_abort); } } @@ -5486,7 +5490,8 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp) blocker = QLIST_FIRST(&bs->op_blockers[op]); if (errp) { error_setg(errp, "Device '%s' is busy: %s", - bs->device_name, error_get_pretty(blocker->reason)); + bdrv_get_device_name(bs), + error_get_pretty(blocker->reason)); } return true; } diff --git a/block/mirror.c b/block/mirror.c index 18b18e0..829be2f 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -567,7 +567,8 @@ static void mirror_complete(BlockJob *job, Error **errp) return; } if (!s->synced) { - error_set(errp, QERR_BLOCK_JOB_NOT_READY, job->bs->device_name); + error_set(errp, QERR_BLOCK_JOB_NOT_READY, + bdrv_get_device_name(job->bs)); return; } diff --git a/block/qapi.c b/block/qapi.c index 9733ebd..d071ee5 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -272,7 +272,7 @@ void bdrv_query_info(BlockDriverState *bs, BlockDriverState *bs0; ImageInfo **p_image_info; Error *local_err = NULL; - info->device = g_strdup(bs->device_name); + info->device = g_strdup(bdrv_get_device_name(bs)); info->type = g_strdup("unknown"); info->locked = bdrv_dev_is_medium_locked(bs); info->removable = bdrv_dev_has_removable_media(bs); @@ -327,9 +327,9 @@ static BlockStats *bdrv_query_stats(const BlockDriverState *bs) s = g_malloc0(sizeof(*s)); - if (bs->device_name[0]) { + if (bdrv_get_device_name(bs)[0]) { s->has_device = true; - s->device = g_strdup(bs->device_name); + s->device = g_strdup(bdrv_get_device_name(bs)); } s->stats = g_malloc0(sizeof(*s->stats)); diff --git a/block/qcow.c b/block/qcow.c index a87bd69..ece2269 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -124,7 +124,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, snprintf(version, sizeof(version), "QCOW version %" PRIu32, header.version); error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, - bs->device_name, "qcow", version); + bdrv_get_device_name(bs), "qcow", version); ret = -ENOTSUP; goto fail; } @@ -231,7 +231,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, /* Disable migration when qcow images are used */ error_set(&s->migration_blocker, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, - "qcow", bs->device_name, "live migration"); + "qcow", bdrv_get_device_name(bs), "live migration"); migrate_add_blocker(s->migration_blocker); qemu_co_mutex_init(&s->lock); diff --git a/block/qcow2.c b/block/qcow2.c index fb28493..156a219 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -206,8 +206,8 @@ static void GCC_FMT_ATTR(3, 4) report_unsupported(BlockDriverState *bs, vsnprintf(msg, sizeof(msg), fmt, ap); va_end(ap); - error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, bs->device_name, "qcow2", - msg); + error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, + bdrv_get_device_name(bs), "qcow2", msg); } static void report_unsupported_feature(BlockDriverState *bs, diff --git a/block/qed.c b/block/qed.c index e2316d7..7f03c26 100644 --- a/block/qed.c +++ b/block/qed.c @@ -408,7 +408,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags, snprintf(buf, sizeof(buf), "%" PRIx64, s->header.features & ~QED_FEATURE_MASK); error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, - bs->device_name, "QED", buf); + bdrv_get_device_name(bs), "QED", buf); return -ENOTSUP; } if (!qed_is_cluster_size_valid(s->header.cluster_size)) { diff --git a/block/quorum.c b/block/quorum.c index 7687466..6ce5904 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -226,8 +226,8 @@ static void quorum_report_bad(QuorumAIOCB *acb, char *node_name, int ret) static void quorum_report_failure(QuorumAIOCB *acb) { - const char *reference = acb->common.bs->device_name[0] ? - acb->common.bs->device_name : + const char *reference = bdrv_get_device_name(acb->common.bs)[0] ? + bdrv_get_device_name(acb->common.bs) : acb->common.bs->node_name; qapi_event_send_quorum_failure(reference, acb->sector_num, diff --git a/block/vdi.c b/block/vdi.c index cfa08b0..9604721 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -490,7 +490,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, /* Disable migration when vdi images are used */ error_set(&s->migration_blocker, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, - "vdi", bs->device_name, "live migration"); + "vdi", bdrv_get_device_name(bs), "live migration"); migrate_add_blocker(s->migration_blocker); return 0; diff --git a/block/vhdx.c b/block/vhdx.c index b046448..12bfe75 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1004,7 +1004,7 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags, /* Disable migration when VHDX images are used */ error_set(&s->migration_blocker, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, - "vhdx", bs->device_name, "live migration"); + "vhdx", bdrv_get_device_name(bs), "live migration"); migrate_add_blocker(s->migration_blocker); return 0; diff --git a/block/vmdk.c b/block/vmdk.c index 4ae6c75..673d3f5 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -657,7 +657,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, snprintf(buf, sizeof(buf), "VMDK version %" PRId32, le32_to_cpu(header.version)); error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, - bs->device_name, "vmdk", buf); + bdrv_get_device_name(bs), "vmdk", buf); return -ENOTSUP; } else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR)) { /* VMware KB 2064959 explains that version 3 added support for @@ -939,7 +939,7 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags, /* Disable migration when VMDK images are used */ error_set(&s->migration_blocker, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, - "vmdk", bs->device_name, "live migration"); + "vmdk", bdrv_get_device_name(bs), "live migration"); migrate_add_blocker(s->migration_blocker); g_free(buf); return 0; diff --git a/block/vpc.c b/block/vpc.c index e08144a..38c4f02 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -320,7 +320,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags, /* Disable migration when VHD images are used */ error_set(&s->migration_blocker, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, - "vpc", bs->device_name, "live migration"); + "vpc", bdrv_get_device_name(bs), "live migration"); migrate_add_blocker(s->migration_blocker); return 0; diff --git a/block/vvfat.c b/block/vvfat.c index 6c9fde0..cefe3a4 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1182,7 +1182,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags, if (s->qcow) { error_set(&s->migration_blocker, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, - "vvfat (rw)", bs->device_name, "live migration"); + "vvfat (rw)", bdrv_get_device_name(bs), "live migration"); migrate_add_blocker(s->migration_blocker); } diff --git a/blockjob.c b/blockjob.c index 0689fdd..3af0f6c 100644 --- a/blockjob.c +++ b/blockjob.c @@ -107,7 +107,8 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) void block_job_complete(BlockJob *job, Error **errp) { if (job->paused || job->cancelled || !job->driver->complete) { - error_set(errp, QERR_BLOCK_JOB_NOT_READY, job->bs->device_name); + error_set(errp, QERR_BLOCK_JOB_NOT_READY, + bdrv_get_device_name(job->bs)); return; } diff --git a/include/block/block.h b/include/block/block.h index 6c97dd4..3880e05 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -417,7 +417,7 @@ int bdrv_set_key(BlockDriverState *bs, const char *key); int bdrv_query_missing_keys(void); void bdrv_iterate_format(void (*it)(void *opaque, const char *name), void *opaque); -const char *bdrv_get_device_name(BlockDriverState *bs); +const char *bdrv_get_device_name(const BlockDriverState *bs); int bdrv_get_flags(BlockDriverState *bs); int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num, const uint8_t *buf, int nb_sectors); diff --git a/include/block/block_int.h b/include/block/block_int.h index 14e0b7c..da00aa5 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -392,8 +392,6 @@ struct BlockDriverState { char node_name[32]; /* element of the list of named nodes building the graph */ QTAILQ_ENTRY(BlockDriverState) node_list; - /* Device name is the name associated with the "drive" the guest sees */ - char device_name[32]; /* element of the list of "drives" the guest sees */ QTAILQ_ENTRY(BlockDriverState) device_list; QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps;