From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=35959 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PmSPW-0001Mj-Sp for qemu-devel@nongnu.org; Mon, 07 Feb 2011 09:49:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PmSPU-0005wq-V5 for qemu-devel@nongnu.org; Mon, 07 Feb 2011 09:49:50 -0500 Received: from mail-ey0-f173.google.com ([209.85.215.173]:44332) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PmSPU-0005wL-N4 for qemu-devel@nongnu.org; Mon, 07 Feb 2011 09:49:48 -0500 Received: by eyg7 with SMTP id 7so2333402eyg.4 for ; Mon, 07 Feb 2011 06:49:47 -0800 (PST) Message-ID: <4D500680.3020303@codemonkey.ws> Date: Mon, 07 Feb 2011 08:49:36 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 03/16] qdev-properties: add PROP_TYPE_ENUM References: <1296773152-23279-1-git-send-email-alevy@redhat.com> <1296773152-23279-4-git-send-email-alevy@redhat.com> <20110207104348.GA5754@playa.tlv.redhat.com> <4D4FF08D.2040505@codemonkey.ws> <20110207141437.GH5754@playa.tlv.redhat.com> In-Reply-To: <20110207141437.GH5754@playa.tlv.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org, Alon Levy , "Daniel P. Berrange" On 02/07/2011 08:14 AM, Alon Levy wrote: > Fair enough, so a patch that added enumeration through QMP would be acceptable? > I'm not even sure that makes sense, would you mind outlining how you see this > implemented? > Before we write any code, we need to figure out what an enum is, how we want to transport it via QMP, and how we will support introspection on it. In terms of the protocol format, I'm inclined to suggest that we transmit enums as integers as it maps better to C. That way, we can write a first class enumeration type and build our interfaces on top of that. I'm working on a QMP rewrite using a schema located at http://repo.or.cz/w/qemu/aliguori.git/shortlog/refs/heads/glib The schema has the notion of defining types along side with defining interfaces. Adding a syntax to define an enum would be pretty straight forward. Probably something like: { 'DriftCompensationPolicy': [{'gradual': 'int'}, {'fast': 'int'}, {'none': 'int'}] } This would in turn generate: typedef enum DriftCompensationPolicy { DRIFT_COMPENSATION_POLICY_GRADUAL = 0, DRIFT_COMPENSATION_POLICY_FAST, DRIFT_COMPENSATION_POLICY_NONE } DriftCompensationPolicy; From a QMP side, the value would be marshalled as an integer but the schema would contain the type 'DriftCompensationPolicy' as the type. For -device, this would mean that enums would be treated as integer properties, and we'd need some boiler plate code to register the enum with symbolic names. From a qdev perspective, I think it would be easier to generate a unique property type for every specific enum type. That means the qdev side ends up looking like: PropertyInfo qdev_prop_drift_compensation_policy = { .name = "DriftCompensationPolicy", .type = PROP_TYPE_INT32, .size = sizeof(DriftCompensationPolicy), .parse = parse_drift_compensation_policy, .print = print_drift_compensation_policy, }; struct MyDevice { DriftCompensationPolicy drift_policy; }; DEFINE_PROP_DRIFT_COMPENSATION_POLICY("drift_policy", MyDevice, drift_policy, DRIFT_COMPENSATION_POLICY_NONE); We could autogenerate all of this code from the QMP schema too. It's possible to do a one-off forwards compatible enum type for qdev but I'd strongly prefer to get the QMP infrastructure in place first. But the advantage of this approach is pretty significant IMHO. We get to work in native C enumeration types both in QEMU and libqmp. That's a big win for type safety. BTW, if we treat QMP introspection as just returning the QMP schema that we use for code generation (it's valid JSON afterall), then we get enumeration introspection for free. I think this efficiently gives us what danpb's earlier series was going for with his introspection patch set. Regards, Anthony Liguori >> Regards, >> >> Anthony Liguori >> >> >> >