Hi, > From: Eduardo Habkost [mailto:ehabk...@redhat.com] > Sent: Wednesday, September 03, 2014 2:00 AM > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function > Importance: High > > On Mon, Sep 01, 2014 at 06:47:13AM +0000, Gonglei (Arei) wrote: > > > From: Gerd Hoffmann [mailto:kra...@redhat.com] > > > Sent: Monday, September 01, 2014 2:43 PM > > > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path > function > > > Importance: High > > > > > > Hi, > > > > > > > +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst) > > > > +{ > > > > + bool ret = false; > > > > + char *devpath_src = qdev_get_fw_dev_path(src); > > > > + char *devpath_dst = qdev_get_fw_dev_path(dst); > > > > + > > > > + if (!strcmp(devpath_src, devpath_dst)) { > > > > + ret = true; > > > > + } > > > > + > > > > + g_free(devpath_src); > > > > + g_free(devpath_dst); > > > > + return ret; > > > > +} > > > > + > > > > +void del_boot_device_path(DeviceState *dev) > > > > +{ > > > > + FWBootEntry *i; > > > > + > > > > + assert(dev != NULL); > > > > + > > > > + /* remove all entries of the assigned device */ > > > > + QTAILQ_FOREACH(i, &fw_boot_order, link) { > > > > + if (i->dev == NULL) { > > > > + continue; > > > > + } > > > > + if ((i->dev == dev || is_same_fw_dev_path(i->dev, dev))) { > > > > > > Why this is needed? Is there any case where i-->dev != dev but > > > is_same_fw_dev_path() returns true? > > > > > Yes, it is needed. At present, the virito-net-pci device > > compliance with this situation. > > > > Please see the qom path about virtio-net-pci and virtio-net device: > > > > id: null, /machine/peripheral/nic1/virtio-backend > > id: nic1, /machine/peripheral/nic1 > > And why exactly is the caller passing a different pointer to > del_boot_device_path()? Why not simply change the caller to pass the > same pointer to add_boot_device_path() and del_boot_device_path()? > 1. When we want to create a virtio-net device, we must use '-device virtio-net-pci'. This device is a pci device, and virtio-net-device is its child: object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL); 2. Both virtio-net-pci and virtio-net-device own bootindex property because they include "DEFINE_NIC_PROPERTIES(VirtIONetPCI, vdev.nic_conf)". 3. Only virtio-net-device will call add_boot_device_path() in virtio_net_device_realize(), which use its own pointer, but not virtio-net-pci's pointer: add_boot_device_path(n->nic_conf.bootindex, dev, "/ethernet-phy@0"); 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's pointer to del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add a function named is_same_fw_dev_path() to handle this situation.
Best regards, -Gonglei
