From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45308) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xrqwl-0006bo-Ar for qemu-devel@nongnu.org; Fri, 21 Nov 2014 11:20:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xrqwf-0005je-Gm for qemu-devel@nongnu.org; Fri, 21 Nov 2014 11:20:35 -0500 Received: from omzsmtpe04.verizonbusiness.com ([199.249.25.207]:23951) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xrqwf-0005jP-4L for qemu-devel@nongnu.org; Fri, 21 Nov 2014 11:20:29 -0500 From: Don Slutz Message-ID: <546F662A.6030704@terremark.com> Date: Fri, 21 Nov 2014 11:19:54 -0500 MIME-Version: 1.0 References: <1416522776-22169-1-git-send-email-dslutz@verizon.com> <20141121020142.GC3137@thinpad.lan.raisama.net> <546F3E92.5080406@terremark.com> <20141121135333.GD3137@thinpad.lan.raisama.net> <546F52A1.8060303@terremark.com> <20141121151024.GF3137@thinpad.lan.raisama.net> In-Reply-To: <20141121151024.GF3137@thinpad.lan.raisama.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 v5 1/1] -machine vmport=auto: Fix handling of VMWare ioport emulation for xen List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Anthony Liguori , "Michael S. Tsirkin" , Michael Tokarev , qemu-devel@nongnu.org, Don Slutz , "Dr. David Alan Gilbert" , Stefan Hajnoczi , Paolo Bonzini On 11/21/14 10:10, Eduardo Habkost wrote: > On Fri, Nov 21, 2014 at 09:56:33AM -0500, Don Slutz wrote: >> On 11/21/14 08:53, Eduardo Habkost wrote: >>> On Fri, Nov 21, 2014 at 08:30:58AM -0500, Don Slutz wrote: >>>> On 11/20/14 21:01, Eduardo Habkost wrote: >>>>> On Thu, Nov 20, 2014 at 05:32:56PM -0500, Don Slutz wrote: >>>>> [...] >> [...] >>>>>> + pcms->vmport = ON_OFF_AUTO_AUTO; >>>>>> + object_property_add(obj, PC_MACHINE_VMPORT, "str", >>>>> I believe "OnOffAuto" is a valid type name, if it is defined in the QAPI >>>>> schema. >>>> I can only find: >>>> >>>> qapi-types.h:typedef enum OnOffAuto >>>> qapi-types.h:} OnOffAuto; >>> I don't understand what you mean, where did you expect to find it? You added it >>> to the schema, so it is now a valid type name. >>> >>>> Which I use to define pcms->vmport. The best I can translate this is >>>> that "str" is what you are looking to replace. >>>> >>>> So I plan no change here. >>> You are not defining a string property, but an OnOffAuto enum property, >>> so why use "str"? >>> >>> See, for example, the type of an existing enum property: >>> ide-hd.bios-chs-trans: >>> >>> $ qemu-system-x86_64 -device ide-hd,? >>> [...] >>> ide-hd.bios-chs-trans=BiosAtaTranslation (Logical CHS translation algorithm, auto/none/lba/large/rechs) >>> [...] >>> $ >>> >>> >> Ok, I now understand what you are getting at. From object.h: >> >> * @type: the type name of the property. This namespace is pretty loosely >> * defined. Sub namespaces are constructed by using a prefix and then >> * to angle brackets. For instance, the type 'virtio-net-pci' in the >> * 'link' namespace would be 'link'. >> >> It turns out that BiosAtaTranslation is a .name not a .type: >> > "-device ...,?" above is showing the property type. See > qdev_device_help(). > >> hyper-0-21-52:~/qemu>git grep BiosAtaTranslation >> hw/core/qdev-properties.c:QEMU_BUILD_BUG_ON(sizeof(BiosAtaTranslation) != >> sizeof(int)); >> hw/core/qdev-properties.c: .name = "BiosAtaTranslation", >> hw/core/qdev-properties.c: .enum_table = BiosAtaTranslation_lookup, >> qapi/block.json:# BiosAtaTranslation: >> qapi/block.json:{ 'enum': 'BiosAtaTranslation', > PropertyInfo.name is what is used as the 'type' parameter to > object_property_add() when qdev adds a static property to the object. > See qdev_property_add_static(). > >> >> so to me there are a few options: >> >> 1) "enum" >> 2) "OnOffAuto" >> 3) "enum" >> 4) "enum OnOffAuto" >> 5) "enum-OnOffAuto" >> >> I think #2 was what you were thinking of, but I am not sure. Please advise. > The documentation is unclear about the set of valid type names. But as > existing enum properties simply use the enum type name directly, I > suggest "OnOffAuto" to follow existing practice. > Fine. I went with this. v6 posted. -Don Slutz