Re: [Qemu-devel] [PATCH] Remove PCI class code from virtio balloon device

[David Gibson <dwg@au1.ibm.com>]

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