Re: [Qemu-devel] Re: [PATCH 0/10] qdev patches.

[Paul Brook <paul@codesourcery.com>]

On Friday 19 June 2009, Gerd Hoffmann wrote:
>    Hi,
>
> > Updated patch queue pushed to qdev.v5. Not posting to avoid spamming the
> > list too much. Online viewable via gitweb here:
> > http://git.et.redhat.com/?p=qemu-kraxel.git;a=shortlog;h=refs/heads/qdev.
> >v5
>
> Todays update pushed to qdev.v6.
> http://git.et.redhat.com/?p=qemu-kraxel.git;a=shortlog;h=refs/heads/qdev.v6

A few comments on specific patches:

* qdev: update pci device registration

I dislike passing an {array,length} pair. Especially when it requires every 
user to manually get the right length.

* qdev/core: bus list

I don't seen any good reason for this. In fact I think it is a major step 
backwards. A bus is uniquely identified by its name and parent device.

* qdev/pci: misc fixes.

All uses of the second argument to savevm should go away, not introduce new 
ones. I'm unconvinced by the dev->name change. If we're using the same value 
then why does it exist at all?

* qdev/pci: hook up i440fx

i440fx_init should not exist. c.f. versatile_pci.c

* qdev: update pci device registration

This is exactly the sort of fake conversion that I don't like, because you 
still require use of the old hardcoded initialization functions.
Convenience wrappers like smc91c111_init are fine (and will naturally 
disappear when we have a machine config), but you shouldn't be poking directly 
at device state.
In practice there's no way for the user to have more than one set of IDE 
busses, so I don't see much point pretending we allow this. i.e. remove the 
hd_table argument altogether and use drive_get_index directly.

* qdev: convert all vga

Likewise, pci_vga_init needs to go away.

* qdev/scsi: add scsi bus support to qdev, convert drivers

This still feels wrong, probably because you're using the same thing for both 
a parallel scsi bus, and for devices (usb-msd) that incorporate scsi 
functionality directly. The current qemu scsi API is actually a set of point 
to point links with individual devices. All the bus emulation is local to the 
host controller.

* qdev/usb*

I have not looked at these patches.