From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44840) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fLUfo-0003lh-7H for qemu-devel@nongnu.org; Wed, 23 May 2018 10:23:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fLUfk-0002Ps-6l for qemu-devel@nongnu.org; Wed, 23 May 2018 10:23:28 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:44368) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fLUfj-0002Ox-SB for qemu-devel@nongnu.org; Wed, 23 May 2018 10:23:24 -0400 Received: from pps.filterd (m0098396.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w4NEJOqM130331 for ; Wed, 23 May 2018 10:23:21 -0400 Received: from e34.co.us.ibm.com (e34.co.us.ibm.com [32.97.110.152]) by mx0a-001b2d01.pphosted.com with ESMTP id 2j58r0mqrb-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 23 May 2018 10:23:21 -0400 Received: from localhost by e34.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 23 May 2018 08:23:20 -0600 References: <20180517192325.8335-1-danielhb@linux.ibm.com> <20180517192325.8335-2-danielhb@linux.ibm.com> <87wow1gxi8.fsf@dusky.pond.sub.org> <20180521181435.GN25013@localhost.localdomain> <20180521202616.GT25013@localhost.localdomain> <874liyivcs.fsf@dusky.pond.sub.org> <20180523122749.GC8988@localhost.localdomain> From: Daniel Henrique Barboza Date: Wed, 23 May 2018 11:11:53 -0300 MIME-Version: 1.0 In-Reply-To: <20180523122749.GC8988@localhost.localdomain> Content-Language: en-US Message-Id: <3628eeb1-0976-a001-f2ab-37aa9fa979ef@linux.ibm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , Markus Armbruster Cc: Stefano Stabellini , "Michael S. Tsirkin" , libvir-list@redhat.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, Anthony Perard , Igor Mammedov , dgilbert@redhat.com Hi, On 05/23/2018 09:27 AM, Eduardo Habkost wrote: > On Wed, May 23, 2018 at 11:17:55AM +0200, Markus Armbruster wrote: >> Eduardo Habkost writes: >>> On Mon, May 21, 2018 at 04:46:36PM -0300, Daniel Henrique Barboza wro= te: > [...] >>>> Since no objection was made back then, this logic was put into query= -target >>>> starting >>>> in v2. Still, I don't have any favorites though: query-target looks = ok, >>>> query-machine >>>> looks ok and a new API looks ok too. It's all about what makes (more= ) sense >>>> in the >>>> management level, I think. >>> I understand the original objection from Eric: having to add a >>> new command for every runtime flag we want to expose to the user >>> looks wrong to me. >> Agreed. >> >>> However, extending query-machines and query-target looks wrong >>> too, however. query-target looks wrong because this not a >>> property of the target. query-machines is wrong because this is >>> not a static property of the machine-type, but of the running >>> machine instance. >> Of the two, query-machines looks less wrong. >> >> Arguably, -no-acpi should not exist. It's an ad hoc flag that sneakil= y >> splits a few machine types into two variants, with and without ACPI. >> It's silently ignored for other machine types, even APCI-capable ones. >> >> If the machine type variants with and without ACPI were separate types= , >> wakeup-suspend-support would be a static property of the machine type. >> >> However, "separate types" probably doesn't scale: I'm afraid we'd end = up >> with an undesirable number of machine types. Avoiding that is exactly >> why we have machine types with configurable options. I suspect that's >> how ACPI should be configured (if at all). >> >> So, should we make -no-acpi sugar for a machine type parameter? And >> then deprecate -no-acpi for good measure? > I think we should. > > >>> Can we have a new query command that could be an obvious >>> container for simple machine capabilities that are not static? A >>> name like "query-machine" would be generic enough for that, I >>> guess. >> Having command names differ only in a single letter is awkward, but >> let's focus on things other than naming now, and use >> query-current-machine like a working title. >> >> query-machines is wrong because wakeup-suspend-support isn't static fo= r >> some machine types. >> >> query-current-machine is also kind of wrong because >> wakeup-suspend-support *is* static for most machine types. >> > The most appropriate solution depends a lot on how/when > management software needs to query this. > > If they only need to query it at runtime for a running VM, > there's no reason for us to go of our way and add complexity just > to make it look like static data in query-machines. > > On the other hand, if they really need to query it before > configuring/starting a VM, it won't be useful at all to make it > available only at runtime. > > Daniel, when/how exactly software would need to query the new > flag? The original idea of this series was to provide a way to inform managemen= t when not to execute a pm-suspend* command. This is a command from the guest agent, thus it's only available when the guest is already running.=20 As far as I know there is no way to suspend the VM=C2=A0 without using the guest= agent. Thus, unless management wants to store this state to avoid querying it=20 multiple times during the VM lifetime (I remember from Libvirt code that it=20 stores some sort of capabilities of the domain in an internal state, although I=20 can't recall if this info would be eligible for that), there is no need to query this=20 until the VM is booted. > > >> Worse, a machine type property that is static for all machine types no= w >> could conceivably become dynamic when we add a machine type >> configuration knob. >> > This isn't the first time a machine capability that seems static > actually depends on other configuration arguments. We will > probably need to address this eventually. > > >> Would a way to tie the property to the configuration knob help? >> Something like wakeup-suspend-support taking values true (supported), >> false (not supported), and "acpi" (supported if machine type >> configuration knob "acpi" is switched on). >> > I would prefer a more generic mechanism. Maybe make > 'query-machines' accept a 'machine-options' argument? >