Re: [PATCH] [UBI] 1/4 UBI volume notifications - UBI changes

[dmitry pervushin <dpervushin@gmail.com>]

On Tue, 2008-12-09 at 12:36 +0200, Artem Bityutskiy wrote:

Thanks for your review, my the only comment is inlined below.

[skipped]
> <snip>
> > +/**
> >   * ubi_get_device - get UBI device.
> >   * @ubi_num: UBI device number
> >   *
> > @@ -842,6 +870,8 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
> >  			goto out_detach;
> >  	}
> >  
> > +	/* when processing uif_init, we already might want to open the volume */
> > +	ubi_devices[ubi_num] = ubi;
> >  	err = uif_init(ubi);
> >  	if (err)
> >  		goto out_nofree;
> 
> I do not understand this change. The point is to prevent anyone from
> opening the volume before it is completely initialized. What you do -
> you allow the volume to be opened while it is in the middle of
> initialization, which is wrong. E.g., what if the initialization fails
> at some point?
> 
> And this change does not seem to be relevant to this patch.
This change is absolutely needed :)
Well, the sequence of steps is as follows:
1. uif_init calls ubi_add_volume
2. ubi_add_volume notifies everyone about volume adding
3. (successful exit is not interested to us)
4. in case of errors reported by uif_init ubi_attach_mtd_dev calls
ubi_kill_volumes
5. ubi_kill_volumes calls ubi_free_volume, which notifies everyone about
volume deleting.

In current version, "notifies about volume adding" corresponds to
ubi_create_gluebi and "...deleting" means ubi_destory_gluebi"

Does this sound right? OK, then I am replacing ubi_create_gluebi with
notification, notification function tries to open new (just appeared)
volume... and fails, because ubi_open_volume tries to ubi_get_device.
It, in turn checks the "ubi_devices[ubi_num]" which is not filled yet.

So, I do insist on the change.