From patchwork Wed Jul 5 19:04:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 9826941 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D94D760237 for ; Wed, 5 Jul 2017 19:05:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C88FC28582 for ; Wed, 5 Jul 2017 19:05:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BB3F82857F; Wed, 5 Jul 2017 19:05:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 573842857F for ; Wed, 5 Jul 2017 19:05:49 +0000 (UTC) Received: from localhost ([::1]:47699 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dSpcS-0004Ci-Hf for patchwork-qemu-devel@patchwork.kernel.org; Wed, 05 Jul 2017 15:05:48 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40075) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dSpb5-00043k-6N for qemu-devel@nongnu.org; Wed, 05 Jul 2017 15:04:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dSpb4-0005wN-76 for qemu-devel@nongnu.org; Wed, 05 Jul 2017 15:04:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60726) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dSpaz-0005uM-Gd; Wed, 05 Jul 2017 15:04:17 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7D9EFC056791; Wed, 5 Jul 2017 19:04:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7D9EFC056791 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=mreitz@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 7D9EFC056791 Received: from localhost (ovpn-204-111.brq.redhat.com [10.40.204.111]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F01915D723; Wed, 5 Jul 2017 19:04:15 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 5 Jul 2017 21:04:02 +0200 Message-Id: <20170705190404.22449-4-mreitz@redhat.com> In-Reply-To: <20170705190404.22449-1-mreitz@redhat.com> References: <20170705190404.22449-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 05 Jul 2017 19:04:16 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v4 3/5] block: qobject_is_equal() in bdrv_reopen_prepare() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Markus Armbruster , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Currently, bdrv_reopen_prepare() assumes that all BDS options are strings. However, this is not the case if the BDS has been created through the json: pseudo-protocol or blockdev-add. Note that the user-invokable reopen command is an HMP command, so you can only specify strings there. Therefore, specifying a non-string option with the "same" value as it was when originally created will now return an error because the values are supposedly similar (and there is no way for the user to circumvent this but to just not specify the option again -- however, this is still strictly better than just crashing). Signed-off-by: Max Reitz Reviewed-by: Eric Blake --- block.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/block.c b/block.c index 913bb43..f8c8e92 100644 --- a/block.c +++ b/block.c @@ -2947,19 +2947,26 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, const QDictEntry *entry = qdict_first(reopen_state->options); do { - QString *new_obj = qobject_to_qstring(entry->value); - const char *new = qstring_get_str(new_obj); + QObject *new = entry->value; + QObject *old = qdict_get(reopen_state->bs->options, entry->key); + /* - * Caution: while qdict_get_try_str() is fine, getting - * non-string types would require more care. When - * bs->options come from -blockdev or blockdev_add, its - * members are typed according to the QAPI schema, but - * when they come from -drive, they're all QString. + * TODO: When using -drive to specify blockdev options, all values + * will be strings; however, when using -blockdev, blockdev-add or + * filenames using the json:{} pseudo-protocol, they will be + * correctly typed. + * In contrast, reopening options are (currently) always strings + * (because you can only specify them through qemu-io; all other + * callers do not specify any options). + * Therefore, when using anything other than -drive to create a BDS, + * this cannot detect non-string options as unchanged, because + * qobject_is_equal() always returns false for objects of different + * type. In the future, this should be remedied by correctly typing + * all options. For now, this is not too big of an issue because + * the user can simply omit options which cannot be changed anyway, + * so they will stay unchanged. */ - const char *old = qdict_get_try_str(reopen_state->bs->options, - entry->key); - - if (!old || strcmp(new, old)) { + if (!qobject_is_equal(new, old)) { error_setg(errp, "Cannot change the option '%s'", entry->key); ret = -EINVAL; goto error;