From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50254) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fOOuw-0004yc-2N for qemu-devel@nongnu.org; Thu, 31 May 2018 10:51:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fOOus-0000Oy-Vz for qemu-devel@nongnu.org; Thu, 31 May 2018 10:51:06 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:36982 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fOOus-0000OD-QI for qemu-devel@nongnu.org; Thu, 31 May 2018 10:51:02 -0400 References: <20180531051641.8473-1-peterx@redhat.com> <20180531051641.8473-5-peterx@redhat.com> From: Eric Blake Message-ID: <5706d352-39d8-9a06-5d25-bba4da747cd9@redhat.com> Date: Thu, 31 May 2018 09:51:00 -0500 MIME-Version: 1.0 In-Reply-To: <20180531051641.8473-5-peterx@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC v2 4/4] Revert "tests: Add parameter to qtest_init_without_qmp_handshake" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu , qemu-devel@nongnu.org Cc: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , "Daniel P . Berrange" , Christian Borntraeger , Fam Zheng , Kevin Wolf , Max Reitz , Eric Auger , John Snow , Markus Armbruster , Peter Maydell , "Dr . David Alan Gilbert" On 05/31/2018 12:16 AM, Peter Xu wrote: > This reverts commit ddee57e0176f6ab53b13c6c97605b62737a8fd7a. > > Meanwhile, revert one line from fa198ad9bdef to make sure > qtest_init_without_qmp_handshake() will only pass in one parameter. > > Signed-off-by: Peter Xu > --- > +++ b/tests/libqtest.h > @@ -56,14 +56,11 @@ QTestState *qtest_init(const char *extra_args); > > /** > * qtest_init_without_qmp_handshake: > - * @use_oob: true to have the server advertise OOB support > - * @extra_args: other arguments to pass to QEMU. CAUTION: these > - * arguments are subject to word splitting and shell evaluation. > + * @extra_args: other arguments to pass to QEMU. The CAUTION message was a useful improvement, I don't think we should revert that part of the patch. > @@ -200,13 +199,12 @@ QTestState *qtest_init_without_qmp_handshake(bool use_oob, > command = g_strdup_printf("exec %s " > "-qtest unix:%s,nowait " > "-qtest-log %s " > - "-chardev socket,path=%s,nowait,id=char0 " > - "-mon chardev=char0,mode=control%s " > + "-qmp unix:%s,nowait " > "-machine accel=qtest " > "-display none " > "%s", qemu_binary, socket_path, > getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null", > - qmp_socket_path, use_oob ? ",x-oob=on" : "", > + qmp_socket_path, I agree with your cover letter that you don't want to check in patch 3/4 as-is, as it temporarily breaks this test. But I disagree with your idea of squashing 3 and 4 into one patch, as having two separate reversion patches that both call out the primary commit that they are reverting is nicer than one patch that reverts two things at once. My suggestion: in patch 3, tweak this line to be: qmp_socket_path, "", so that the test will pass, then in patch 4, remove the useless "" parameter (a minor conflict resolution rather than a clean revert, but we already have that because of fa198ad9b, and I think it results in a nicer git commit history). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org