From patchwork Mon Jul 3 12:25:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 9822633 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 E74FC602F0 for ; Mon, 3 Jul 2017 12:28:58 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D63192843C for ; Mon, 3 Jul 2017 12:28:58 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C89D428503; Mon, 3 Jul 2017 12:28:58 +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 673192843C for ; Mon, 3 Jul 2017 12:28:58 +0000 (UTC) Received: from localhost ([::1]:34743 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dS0TJ-0000GD-KE for patchwork-qemu-devel@patchwork.kernel.org; Mon, 03 Jul 2017 08:28:57 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35577) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dS0Px-0006er-9w for qemu-devel@nongnu.org; Mon, 03 Jul 2017 08:25:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dS0Pw-0008DQ-DR for qemu-devel@nongnu.org; Mon, 03 Jul 2017 08:25:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:61763) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dS0Pt-0008At-FE; Mon, 03 Jul 2017 08:25:25 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6A3CDC04B938; Mon, 3 Jul 2017 12:25:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6A3CDC04B938 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=mreitz@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 6A3CDC04B938 Received: from localhost (unknown [10.40.205.14]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E9A468C060; Mon, 3 Jul 2017 12:25:22 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Mon, 3 Jul 2017 14:25:03 +0200 Message-Id: <20170703122505.32017-4-mreitz@redhat.com> In-Reply-To: <20170703122505.32017-1-mreitz@redhat.com> References: <20170703122505.32017-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 03 Jul 2017 12:25:24 +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 v3 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 | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/block.c b/block.c index 913bb43..45eb248 100644 --- a/block.c +++ b/block.c @@ -2947,19 +2947,24 @@ 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); - /* - * 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. - */ - const char *old = qdict_get_try_str(reopen_state->bs->options, - entry->key); - - if (!old || strcmp(new, old)) { + QObject *new = entry->value; + QObject *old = qdict_get(reopen_state->bs->options, entry->key); + + /* 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 simply can not specify options which cannot be changed + * anyway, so they will stay unchanged. */ + if (!qobject_is_equal(new, old)) { error_setg(errp, "Cannot change the option '%s'", entry->key); ret = -EINVAL; goto error;