From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59200) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SAu7J-0004U4-Sb for qemu-devel@nongnu.org; Thu, 22 Mar 2012 22:20:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SAu7D-0000E1-J0 for qemu-devel@nongnu.org; Thu, 22 Mar 2012 22:20:37 -0400 Received: from e23smtp01.au.ibm.com ([202.81.31.143]:52874) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SAu7D-0000Dn-0Y for qemu-devel@nongnu.org; Thu, 22 Mar 2012 22:20:31 -0400 Received: from /spool/local by e23smtp01.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 23 Mar 2012 02:13:18 +1000 Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q2N2KF1C1359970 for ; Fri, 23 Mar 2012 13:20:18 +1100 Received: from d23av01.au.ibm.com (loopback [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q2N2KEE4026936 for ; Fri, 23 Mar 2012 13:20:14 +1100 Date: Fri, 23 Mar 2012 12:52:13 +1100 From: David Gibson Message-ID: <20120323015213.GD9087@truffala.fritz.box> References: <1332407367-26551-1-git-send-email-david@gibson.dropbear.id.au> <20120322115332.GA13043@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120322115332.GA13043@redhat.com> Subject: Re: [Qemu-devel] [PATCH] Remove PCI class code from virtio balloon device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: aliguori@us.ibm.com, Rusty Russell , qemu-devel@nongnu.org On Thu, Mar 22, 2012 at 01:53:33PM +0200, Michael S. Tsirkin wrote: > On Thu, Mar 22, 2012 at 08:09:27PM +1100, David Gibson wrote: > > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > > index a0fb7c1..1fd5768 100644 > > --- a/hw/virtio-pci.c > > +++ b/hw/virtio-pci.c > > @@ -790,6 +790,10 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev) > > VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); > > VirtIODevice *vdev; > > > > + if (proxy->class_code != PCI_CLASS_OTHERS && > > + proxy->class_code != PCI_CLASS_MEMORY_RAM) /* qemu < 1.1 */ > > + proxy->class_code = PCI_CLASS_OTHERS; > > + > > {} around if. Fixed. I was copying the bad example from the other virtio pci devices. > > vdev = virtio_balloon_init(&pci_dev->qdev); > > if (!vdev) { > > return -1; > > @@ -905,6 +909,7 @@ static TypeInfo virtio_serial_info = { > > }; > > > > static Property virtio_balloon_properties[] = { > > + DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, PCI_CLASS_OTHERS), > > DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > > DEFINE_PROP_END_OF_LIST(), > > }; > > Sorry to bug you - why not set > + DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), > > and then > > + if (!proxy->class_code) { > + proxy->class_code = PCI_CLASS_OTHERS; > + } > > This is what other devices do. There is no fragment of code quite like the one you quote, only the check for valid class values, which will accomplish the same thing. It seemed clearer to have the default class value in the property definition be, well, the default class value, rather than setting it to 0 and having it overwritten. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson