From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58340) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SPLPk-0006zq-0c for qemu-devel@nongnu.org; Tue, 01 May 2012 18:19:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SPLPh-0001SG-NI for qemu-devel@nongnu.org; Tue, 01 May 2012 18:19:19 -0400 Received: from e4.ny.us.ibm.com ([32.97.182.144]:34909) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SPLPh-0001LT-JQ for qemu-devel@nongnu.org; Tue, 01 May 2012 18:19:17 -0400 Received: from /spool/local by e4.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 1 May 2012 18:19:12 -0400 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 513EB6E8049 for ; Tue, 1 May 2012 18:18:40 -0400 (EDT) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q41MIe3N106294 for ; Tue, 1 May 2012 18:18:40 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q41MIdWx021920 for ; Tue, 1 May 2012 19:18:40 -0300 Message-ID: <4FA0613E.9090500@us.ibm.com> Date: Tue, 01 May 2012 17:18:38 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1335896294-9530-1-git-send-email-aliguori@us.ibm.com> <1335896294-9530-5-git-send-email-aliguori@us.ibm.com> <4FA04974.30503@redhat.com> <4FA04B92.3080200@us.ibm.com> <4FA059EA.1060900@redhat.com> In-Reply-To: <4FA059EA.1060900@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Peter Maydell , Wanpeng Li , Andreas Faerber , qemu-devel@nongnu.org On 05/01/2012 04:47 PM, Paolo Bonzini wrote: > Il 01/05/2012 22:46, Anthony Liguori ha scritto: >>>> >>>> >>>> So I think we can safely break it :-) >>> >>> Does this work with compat properties set on a bus? >> >> No :-( >> >> This is pretty easy to fix though. The attached should do the trick, >> I'll update and send out. >> >> Even if it does, >>> perhaps it's better to avoid the cleverness and wait for my series that >>> moves bus properties to the appropriate abstract superclass. >> >> This series does that FWIW (patch 6/14). > > Yeah, it should---modulo bisectability of course. > > It's a fairly different approach WRT my series (using > qdev_add_properties instead of klass->props). In theory I like it, but > I fail to see right now whether it breaks "-device foo,?" somehow. I > think it does: > > $ qemu-system-x86_64 -device rtl8139,? > rtl8139.mac=macaddr > rtl8139.vlan=vlan > rtl8139.netdev=netdev > rtl8139.bootindex=int32 > rtl8139.addr=pci-devfn<<< here start bus props > rtl8139.romfile=string > rtl8139.rombar=uint32 > rtl8139.multifunction=on/off > rtl8139.command_serr_enable=on/off > > I think it's too late for this series to go into 1.1. No, this all works as expected (well, almost): $ qemu-system-x86_64 -device rtl8139,? rtl8139.vlan=vlan rtl8139.addr=pci-devfn rtl8139.romfile=string rtl8139.multifunction=on/off rtl8139.command_serr_enable=on/off But there is stuff missing... The problem is that qdev_device_help() only prints out legacy properties and munges them to look the way they used to. But when you refactored some of the legacy types to be non-legacy, it meant that the legacy- version disappeared and those options disappeared from -device ,? So we need to fix this either way. I think the simple solution is to store property info in a list instead of directly printing it. Then we can walk the list and remove non-legacy properties for given legacy properties and munge the names in place. But this is true even with master (but much, much worse): anthony@titi:~/build/qemu$ x86_64-softmmu/qemu-system-x86_64 -device rtl8139,? This is because we only look at DeviceClass::props which only contains what happens to only contain static properties for rtl8139. We totally ignore static properties in current master. So my series makes the situation better and I think it's easier to fix the full problem. I also don't view the current bug as a -rc0 blocker (although it's obviously a release blocker). I can send a proper patch later in the week but I'd still like to commit the bus changes before -rc0. We have a month before 1.1. I think that's plenty of time to address any fall out here.. Regards, Anthony Liguori > > Paolo >