From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47379) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dOIKo-0001WU-EV for qemu-devel@nongnu.org; Fri, 23 Jun 2017 02:44:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dOIKk-0007iO-5i for qemu-devel@nongnu.org; Fri, 23 Jun 2017 02:44:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46534) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dOIKj-0007hj-VR for qemu-devel@nongnu.org; Fri, 23 Jun 2017 02:44:46 -0400 From: Markus Armbruster References: <20170613140546.28227-1-vadim.galitsyn@profitbricks.com> <20170614092040.GB2687@work-vm> <20170614154043.GE2687@work-vm> <874lva21jb.fsf@dusky.pond.sub.org> Date: Fri, 23 Jun 2017 08:44:39 +0200 In-Reply-To: (Vadim Galitsyn's message of "Thu, 22 Jun 2017 17:02:03 +0200") Message-ID: <87zicz5hk8.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v2] hmp, qmp: introduce "info memory" and "query-memory" commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vadim Galitsyn Cc: imammedo@redhat.com, Mohammed Gamal , "Dr. David Alan Gilbert" , qemu-devel@nongnu.org Vadim Galitsyn writes: > Hi Markus, > > Thank you for the input. > >> However, your query-memory looks like it could fail. > > With the latest version of a patch ( > http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg03475.html) > "query-memory" can fail only in two cases: > > a) if in QOM there is an object of type of TYPE_PC_DIMM which has no > property of type PC_DIMM_SIZE_PROP, or > b) PC_DIMM_SIZE_PROP is not represented as an integer. > > As far as I understand both (a) and (b) can happen only in case if QOM > somehow corrupted which is not a normal case > (please correct me if I am wrong). These feel like programming errors. They are impossible to test without screwing up the program. Untestable error recovery is commonly wrong. I recommend to assert instead. Igor, what do you think? > Please also note, this is not the last version of the patch since new > functionality was suggested > to be included (NUMA information). > > If patch will be accepted, I think we would need a test case for it since > command output differs once memory > was hot-added/removed per given NUMA node. When final functionality will be > negotiated, I will come up > with test scenario and test case itself. Thanks!