Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function

[Gonglei (Arei)]

Hi,

> From: Eduardo Habkost [mailto:address@hidden
> Sent: Thursday, September 04, 2014 2:13 AM
> Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> 
> On Wed, Sep 03, 2014 at 06:45:56AM +0000, Gonglei (Arei) wrote:
> [...]
> > > > 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.
> > >
> > > When hot-unplugging virtio-net-pci I'd expect we free both
> > > virtio-net-pci and virtio-net-device (and therefore call
> > > del_boot_device_path twice, once for each device).  Can you check that?
> > >
> > Yes, I can.
> >
> > The del_boot_device_path() is called only once, just for virtio-net-pci.
> > For its child, virtio-net-devcie's resource is cleaned by qbus->child 
> > unrealizing
> > process, will not call device_finalize().
> 
> Then we need to fix this to make sure there's a corresponding
> del_boot_device_path() call (with the same pointer) to every
> add_boot_device_path() call, instead of adding a hack to
> del_boot_device_path().
> 
Good idea. We can add functions named $device_finalize_bootindex(), such as:

static void virtio_net_set_bootindex(Object *obj, Visitor *v, void *opaque,
                                     const char *name, Error **errp)
{
    VirtIONet *n = VIRTIO_NET(obj);
    
    set_bootindex(&n->nic_conf, v, name, errp);
    add_boot_device_path(n->nic_conf.bootindex,
                         DEVICE(obj), "/address@hidden"); 
}

static void virtio_net_finalize_bootindex(Object *obj, const char *name,
                                          void *opaque)
{
    del_boot_device_path(DEVICE(obj));
}
...
object_property_add(obj, "bootindex", "int",
                        virtio_net_get_bootindex, 
                        virtio_net_set_bootindex,
                        virtio_net_finalize_bootindex, dev, NULL);

as the previous email, we lay add_boot_device_path() in $device_set_bootindex,
and lay del_boot_device_path() in $device_finalize_bootindex is a good idea IMO.

Do you like it? Thanks.

Best regards,
-Gonglei