Re: [Qemu-devel] [PATCH v4 1/1] s390 pci infrastructure modelling

[Michael S. Tsirkin]

On Thu, Jul 09, 2015 at 05:30:08PM +0800, zyimin wrote:
> 
> 
> On 7/9/2015 3:48 PM, Michael S. Tsirkin wrote:
> >On Wed, Jul 08, 2015 at 01:44:55PM +0800, Yi Min Zhao wrote:
> >>@@ -588,9 +606,172 @@ static const TypeInfo s390_pcihost_info = {
> >>      }
> >>  };
> >>+static void s390_pci_device_hot_plug(HotplugHandler *hotplug_dev,
> >>+                                     DeviceState *dev, Error **errp)
> >>+{
> >>+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
> >>+    S390PCIBusDevice *tmp;
> >>+    S390PCIFacility *f = S390_PCI_FACILITY(
> >>+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> >>+
> >>+    QTAILQ_FOREACH(tmp, &f->zpci_list, next) {
> >>+        /* for now, we use fid to sort the list, need to use uid instead
> >>+         * when uid is ready.
> >What does ready mean in this context?
> uid is a new feature on s390 arch for pci card management.
> But support of uid on OS level has not been ready.

Does this matter here? I thought it's an arbitrary order -
just making it stable is enough?


> >>+         */
> >>+        if (tmp->fid > zpci->fid) {
> >>+            break;
> >>+        }
> >>+    }
> >>+
> >>+    if (tmp) {
> >>+        QTAILQ_INSERT_BEFORE(tmp, zpci, next);
> >>+    } else {
> >>+        QTAILQ_INSERT_TAIL(&f->zpci_list, zpci, next);
> >>+    }
> >>+    f->token_valid = false;
> >>+}
> >This still means hotplug will change the index.
> >How about just using an explicit property to set the index?
> >Harder to use but keeps code simple.
> >
> These code were written by Hong Bo. I think he wants to use zpci->fid to
> sort pci cards.
> "index" what you talked is the order of hotplug? right? If it is, I think
> this code actually
> can change the index.

I don't really understand what you are saying. id is how guest looks
up the device, right? it shouldn't change really.

-- 
MST