From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ABB12C31E47 for ; Wed, 12 Jun 2019 11:36:42 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 851E421019 for ; Wed, 12 Jun 2019 11:36:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 851E421019 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:59118 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hb1YX-0006j6-RQ for qemu-devel@archiver.kernel.org; Wed, 12 Jun 2019 07:36:41 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:39589) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hb1VE-00048f-AK for qemu-devel@nongnu.org; Wed, 12 Jun 2019 07:33:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hb1QJ-0003gW-RX for qemu-devel@nongnu.org; Wed, 12 Jun 2019 07:28:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49306) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hb1QI-0003eF-MM; Wed, 12 Jun 2019 07:28:10 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 078413082E71; Wed, 12 Jun 2019 11:28:10 +0000 (UTC) Received: from localhost.localdomain (ovpn-117-60.ams2.redhat.com [10.36.117.60]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C6905282CA; Wed, 12 Jun 2019 11:28:08 +0000 (UTC) Date: Wed, 12 Jun 2019 13:28:07 +0200 From: Kevin Wolf To: Markus Armbruster Message-ID: <20190612112807.GB9699@localhost.localdomain> References: <20190611134043.9524-1-kwolf@redhat.com> <20190611134043.9524-4-kwolf@redhat.com> <875zpb5j4b.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <875zpb5j4b.fsf@dusky.pond.sub.org> User-Agent: Mutt/1.11.3 (2019-02-01) X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.46]); Wed, 12 Jun 2019 11:28:10 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: Re: [Qemu-devel] [PATCH v2 03/11] monitor: Make MonitorQMP a child class of Monitor X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: berrange@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org, dgilbert@redhat.com Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Am 12.06.2019 um 09:59 hat Markus Armbruster geschrieben: > Kevin Wolf writes: > > > Currently, struct Monitor mixes state that is only relevant for HMP, > > state that is only relevant for QMP, and some actually shared state. > > In particular, a MonitorQMP field is present in the state of any > > monitor, even if it's not a QMP monitor and therefore doesn't use the > > state. > > > > As a first step towards a clean separation between QMP and HMP, let > > MonitorQMP extend Monitor and create a MonitorQMP object only when the > > monitor is actually a QMP monitor. > > > > Signed-off-by: Kevin Wolf > > Reviewed-by: Dr. David Alan Gilbert > > This is a bit harder to review than necessary, because it mixes the > largely mechanical "replace QMP member by child class" with the > necessary prerequisite "clean up to access QMP stuff only when the > monitor is actually a QMP monitor". I'm going to post a split. > > Effectively preexisting: we go from Monitor * to MonitorQMP * without > checking in several places. I'll throw in assertions. Since I don't think doing both in one patch makes review a lot harder (and in fact think your patch 2.5 is harder to review for completeness that the combined patch) and since both Dave and you already reviewed the patch in its current form and I don't want to invalidate that review, I'm going to keep it as a single patch and just squash in the additional assertions where container_of() is used. The resulting code is the same anyway. Kevin