On Mon, Aug 23, 2021 at 12:52 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Aug 23, 2021 at 11:43 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Aug 23, 2021 at 9:53 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Mon, Aug 23, 2021 at 9:11 AM houzj.fnst@fujitsu.com
> > > <houzj.fnst@fujitsu.com> wrote:
> > >
> > > > 4)
> > > > -extern File SharedFileSetCreate(SharedFileSet *fileset, const char *name);
> > > > -extern File SharedFileSetOpen(SharedFileSet *fileset, const char *name,
> > > > - int mode);
> > > > -extern bool SharedFileSetDelete(SharedFileSet *fileset, const char *name,
> > > > - bool error_on_failure);
> > > > extern void SharedFileSetDeleteAll(SharedFileSet *fileset);
> > > > -extern void SharedFileSetUnregister(SharedFileSet *input_fileset);
> > > >
> > > > I noticed the patch delete part of public api, is it better to keep the old api and
> > > > let them invoke new api internally ? Having said that, I didn’t find any open source
> > > > extension use these old api, so it might be fine to delete them.
> > >
> > > Right, those were internally used by buffile.c but now we have changed
> > > buffile.c to directly use the fileset interfaces, so we better remove
> > > them.
> > >
> >
> > I also don't see any reason to keep those exposed from
> > sharedfileset.h. I see that even in the original commit dc6c4c9dc2,
> > these APIs seem to be introduced to be used by buffile. Andres/Thomas,
> > do let us know if you think otherwise?
> >
> > One more comment:
> > I think v1-0001-Sharedfileset-refactoring doesn't have a way for
> > cleaning up worker.c temporary files on error/exit. It is better to
> > have that to make it an independent patch.
>
> I think we should handle that in worker.c itself, by adding a
> before_dsm_detach function before_shmem_exit right?
>
Yeah, I thought of handling it in worker.c similar to what you've in 0002 patch.
--
With Regards,
Amit Kapila.
