Re: [Qemu-devel] [PATCH 2/6] Redesign of pciinit.c (take 2)

[Kevin O'Connor]

On Tue, Mar 13, 2012 at 05:45:19PM +1300, Alexey Korolev wrote:
> Added pci_region_entry structure and list operations to pciinit.c
> List is filled with entries during pci_check_devices.
> List is used just for printing space allocation if we were using lists. 
> Next step will resource allocation using mapping functions.
[...]
> +struct pci_bus;
> +struct pci_region_entry {
> +    struct pci_device *dev;
> +    int bar;
> +    u32 base;
> +    u32 size;
> +    int is64bit;
> +    enum pci_region_type type;
> +    struct pci_bus *this_bus;
> +    struct pci_bus *parent_bus;
> +    struct pci_region_entry *next;
> +    struct pci_region_entry **pprev;
> +};

It's fine to introduce a new struct, but a patch that does this should
have something like the following in the same patch:

--- a/src/pci.h
+++ b/src/pci.h
@@ -51,11 +51,6 @@ struct pci_device {
     u8 prog_if, revision;
     u8 header_type;
     u8 secondary_bus;
-    struct {
-        u32 addr;
-        u32 size;
-        int is64;
-    } bars[PCI_NUM_REGIONS];
 
     // Local information on device.
     int have_driver;

And it should compile and work fine after applying just that one
patch.  That is, you're not introducing a new struct, you're moving
the contents from one struct to another.  The code is being changed -
it's not new code being added and old code being deleted - the patches
need to reflect that.

(If it's a pain to move the struct out of struct pci_device at the
start, then add your new fields into struct pci_device.bars and use a
patch to move the whole thing out of pci.h at the end.)

-Kevin