From patchwork Thu Jun 14 14:55:02 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 164951 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 342C3B7037 for ; Fri, 15 Jun 2012 00:55:42 +1000 (EST) Received: from localhost ([::1]:43456 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfBSW-0001tm-5I for incoming@patchwork.ozlabs.org; Thu, 14 Jun 2012 10:55:40 -0400 Received: from eggs.gnu.org ([208.118.235.92]:41449) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfBSC-0001TL-Tz for qemu-devel@nongnu.org; Thu, 14 Jun 2012 10:55:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SfBS8-00032n-An for qemu-devel@nongnu.org; Thu, 14 Jun 2012 10:55:20 -0400 Received: from mail-wg0-f53.google.com ([74.125.82.53]:50871) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfBS7-00032E-Us for qemu-devel@nongnu.org; Thu, 14 Jun 2012 10:55:16 -0400 Received: by wgbfm10 with SMTP id fm10so1667330wgb.10 for ; Thu, 14 Jun 2012 07:55:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=XeXZ9PUV4IBe3R++791iS0T2GSymtuZgbad2uWSQBoM=; b=Cm/4C9fOu+AyiHv4cShu/CTh6yDlZvjIgckxGwsUfQhJMheZYiO5vL0jxSeI5WevnV EkqsueVNWmXnqarOY3kcFsUaCMs8Y4xPbLWNZDIWCEX43UnQeeLlNNTy6eH1NosJNjRE CAyg16kjZIc42LNxldbUWClvGxLkvnYzBH8ZYo48o8vEy3hD0hALx+IvlHymV9TiS4jC 3lxDG8RS4twgyvkZErtUUdKpRhkQDU5Jqykp0vGuL4PcUT+lM0hl4j71FEZrCMLJ5Rvk S6NG9jS4tLdSVpUvs25f7SOP6lsb66ja6dHwB01Fj8hb17MvHsQn+5EWoVJ44Jdo1DQ7 pn5Q== Received: by 10.180.101.103 with SMTP id ff7mr5091840wib.6.1339685713824; Thu, 14 Jun 2012 07:55:13 -0700 (PDT) Received: from yakj.usersys.redhat.com (nat-pool-mxp-t.redhat.com. [209.132.186.18]) by mx.google.com with ESMTPS id gb9sm19093713wib.8.2012.06.14.07.55.12 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 14 Jun 2012 07:55:13 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Thu, 14 Jun 2012 16:55:02 +0200 Message-Id: <1339685702-10176-3-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.7.10.2 In-Reply-To: <1339685702-10176-1-git-send-email-pbonzini@redhat.com> References: <1339685702-10176-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 74.125.82.53 Cc: kwolf@redhat.com, jcody@redhat.com Subject: [Qemu-devel] [PATCH 2/2] block: introduce bdrv_swap, implement bdrv_append on top of it 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 The new function can be made a bit nicer than bdrv_append. It swaps the whole contents, and then swaps back (using the usual t=a;a=b;b=t idiom) the fields that need to stay on top. Thus, it does not need explicit bdrv_detach_dev, bdrv_iostatus_disable, etc. Signed-off-by: Paolo Bonzini --- block.c | 184 ++++++++++++++++++++++++++++++++++----------------------------- block.h | 1 + 2 files changed, 100 insertions(+), 85 deletions(-) diff --git a/block.c b/block.c index 702821d..0991ded 100644 --- a/block.c +++ b/block.c @@ -971,116 +971,130 @@ static void bdrv_rebind(BlockDriverState *bs) } } -/* - * Add new bs contents at the top of an image chain while the chain is - * live, while keeping required fields on the top layer. - * - * This will modify the BlockDriverState fields, and swap contents - * between bs_new and bs_top. Both bs_new and bs_top are modified. - * - * bs_new is required to be anonymous. - * - * This function does not create any image files. - */ -void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) +static void bdrv_move_feature_fields(BlockDriverState *bs_dest, + BlockDriverState *bs_src) { - BlockDriverState tmp; - - /* bs_new must be anonymous */ - assert(bs_new->device_name[0] == '\0'); - - tmp = *bs_new; - - /* there are some fields that need to stay on the top layer: */ - tmp.open_flags = bs_top->open_flags; + /* move some fields that need to stay attached to the device */ + bs_dest->open_flags = bs_src->open_flags; /* dev info */ - tmp.dev_ops = bs_top->dev_ops; - tmp.dev_opaque = bs_top->dev_opaque; - tmp.dev = bs_top->dev; - tmp.buffer_alignment = bs_top->buffer_alignment; - tmp.copy_on_read = bs_top->copy_on_read; + bs_dest->dev_ops = bs_src->dev_ops; + bs_dest->dev_opaque = bs_src->dev_opaque; + bs_dest->dev = bs_src->dev; + bs_dest->buffer_alignment = bs_src->buffer_alignment; + bs_dest->copy_on_read = bs_src->copy_on_read; - tmp.enable_write_cache = bs_top->enable_write_cache; + bs_dest->enable_write_cache = bs_src->enable_write_cache; /* i/o timing parameters */ - tmp.slice_time = bs_top->slice_time; - tmp.slice_start = bs_top->slice_start; - tmp.slice_end = bs_top->slice_end; - tmp.io_limits = bs_top->io_limits; - tmp.io_base = bs_top->io_base; - tmp.throttled_reqs = bs_top->throttled_reqs; - tmp.block_timer = bs_top->block_timer; - tmp.io_limits_enabled = bs_top->io_limits_enabled; + bs_dest->slice_time = bs_src->slice_time; + bs_dest->slice_start = bs_src->slice_start; + bs_dest->slice_end = bs_src->slice_end; + bs_dest->io_limits = bs_src->io_limits; + bs_dest->io_base = bs_src->io_base; + bs_dest->throttled_reqs = bs_src->throttled_reqs; + bs_dest->block_timer = bs_src->block_timer; + bs_dest->io_limits_enabled = bs_src->io_limits_enabled; /* geometry */ - tmp.cyls = bs_top->cyls; - tmp.heads = bs_top->heads; - tmp.secs = bs_top->secs; - tmp.translation = bs_top->translation; + bs_dest->cyls = bs_src->cyls; + bs_dest->heads = bs_src->heads; + bs_dest->secs = bs_src->secs; + bs_dest->translation = bs_src->translation; /* r/w error */ - tmp.on_read_error = bs_top->on_read_error; - tmp.on_write_error = bs_top->on_write_error; + bs_dest->on_read_error = bs_src->on_read_error; + bs_dest->on_write_error = bs_src->on_write_error; /* i/o status */ - tmp.iostatus_enabled = bs_top->iostatus_enabled; - tmp.iostatus = bs_top->iostatus; + bs_dest->iostatus_enabled = bs_src->iostatus_enabled; + bs_dest->iostatus = bs_src->iostatus; /* dirty bitmap */ - tmp.dirty_count = bs_top->dirty_count; - tmp.dirty_bitmap = bs_top->dirty_bitmap; - assert(bs_new->dirty_bitmap == NULL); + bs_dest->dirty_count = bs_src->dirty_count; + bs_dest->dirty_bitmap = bs_src->dirty_bitmap; /* job */ - tmp.in_use = bs_top->in_use; - tmp.job = bs_top->job; - assert(bs_new->job == NULL); + bs_dest->in_use = bs_src->in_use; + bs_dest->job = bs_src->job; /* keep the same entry in bdrv_states */ - pstrcpy(tmp.device_name, sizeof(tmp.device_name), bs_top->device_name); - tmp.list = bs_top->list; + pstrcpy(bs_dest->device_name, sizeof(bs_dest->device_name), + bs_src->device_name); + bs_dest->list = bs_src->list; +} - /* The contents of 'tmp' will become bs_top, as we are - * swapping bs_new and bs_top contents. */ - tmp.backing_hd = bs_new; - pstrcpy(tmp.backing_file, sizeof(tmp.backing_file), bs_top->filename); - pstrcpy(tmp.backing_format, sizeof(tmp.backing_format), - bs_top->drv ? bs_top->drv->format_name : ""); - - /* swap contents of the fixed new bs and the current top */ - *bs_new = *bs_top; - *bs_top = tmp; - - /* device_name[] was carried over from the old bs_top. bs_new - * shouldn't be in bdrv_states, so we need to make device_name[] - * reflect the anonymity of bs_new - */ - bs_new->device_name[0] = '\0'; +/* + * Swap bs contents for two image chains while they are live, + * while keeping required fields on the BlockDriverState that is + * actually attached to a device. + * + * 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 is required to be anonymous. + * + * This function does not create any image files. + */ +void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) +{ + BlockDriverState tmp; - /* clear the copied fields in the new backing file */ - bdrv_detach_dev(bs_new, bs_new->dev); + /* bs_new must be anonymous and shouldn't have anything fancy enabled */ + assert(bs_new->device_name[0] == '\0'); + assert(bs_new->dirty_bitmap == NULL); + assert(bs_new->job == NULL); + assert(bs_new->dev == NULL); + assert(bs_new->in_use == 0); + assert(bs_new->io_limits_enabled == false); + assert(bs_new->block_timer == NULL); - bs_new->job = NULL; - bs_new->in_use = 0; - bs_new->dirty_bitmap = NULL; - bs_new->dirty_count = 0; + tmp = *bs_new; + *bs_new = *bs_old; + *bs_old = tmp; - qemu_co_queue_init(&bs_new->throttled_reqs); - memset(&bs_new->io_base, 0, sizeof(bs_new->io_base)); - memset(&bs_new->io_limits, 0, sizeof(bs_new->io_limits)); - bdrv_iostatus_disable(bs_new); + /* there are some fields that should not be swapped, move them back */ + bdrv_move_feature_fields(&tmp, bs_old); + bdrv_move_feature_fields(bs_old, bs_new); + bdrv_move_feature_fields(bs_new, &tmp); - /* we don't use bdrv_io_limits_disable() for this, because we don't want - * to affect or delete the block_timer, as it has been moved to bs_top */ - bs_new->io_limits_enabled = false; - bs_new->block_timer = NULL; - bs_new->slice_time = 0; - bs_new->slice_start = 0; - bs_new->slice_end = 0; + /* bs_new shouldn't be in bdrv_states even after the swap! */ + assert(bs_new->device_name[0] == '\0'); + + /* Check a few fields that should remain attached to the device */ + assert(bs_new->dev == NULL); + assert(bs_new->job == NULL); + assert(bs_new->in_use == 0); + assert(bs_new->io_limits_enabled == false); + assert(bs_new->block_timer == NULL); bdrv_rebind(bs_new); - bdrv_rebind(bs_top); + bdrv_rebind(bs_old); +} + +/* + * Add new bs contents at the top of an image chain while the chain is + * live, while keeping required fields on the top layer. + * + * This will modify the BlockDriverState fields, and swap contents + * between bs_new and bs_top. Both bs_new and bs_top are modified. + * + * bs_new is required to be anonymous. + * + * This function does not create any image files. + */ +void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) +{ + bdrv_swap(bs_new, bs_top); + + /* The contents of 'tmp' will become bs_top, as we are + * swapping bs_new and bs_top contents. */ + bs_top->backing_hd = bs_new; + bs_top->open_flags &= ~BDRV_O_NO_BACKING; + pstrcpy(bs_top->backing_file, sizeof(bs_top->backing_file), + bs_new->filename); + pstrcpy(bs_top->backing_format, sizeof(bs_top->backing_format), + bs_new->drv ? bs_new->drv->format_name : ""); } void bdrv_delete(BlockDriverState *bs) diff --git a/block.h b/block.h index 42e30d6..3af93c6 100644 --- a/block.h +++ b/block.h @@ -122,6 +122,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, int bdrv_create_file(const char* filename, QEMUOptionParameter *options); BlockDriverState *bdrv_new(const char *device_name); void bdrv_make_anon(BlockDriverState *bs); +void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old); void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top); void bdrv_delete(BlockDriverState *bs); int bdrv_parse_cache_flags(const char *mode, int *flags);