On Fri, Sep 06, 2013 at 02:13:23PM -0500, Serge Hallyn wrote:
> Quoting Stéphane Graber (stgra...@ubuntu.com):
> > > +static struct lxc_container *lxcsnap_open(struct lxc_snapshot *s)
> > > +{
> > > + return NULL;
> > > +}
> > 
> > I guess that's because we may want to do some more stuff in there later
> > right? ^
> 
> That's what I was originally thinking, but I'm thinking I'll just yank
> this out.
> 
> > > +static void lxcsnap_free(struct lxc_snapshot *s)
> > > +{
> > > + if (s->name)
> > > +         free(s->name);
> > > + if (s->comment_pathname)
> > > +         free(s->comment_pathname);
> > > + if (s->timestamp)
> > > +         free(s->timestamp);
> > > + if (s->lxcpath)
> > > +         free(s->lxcpath);
> > > +}
> > > +
> > > +static char *get_snapcomment(char* snappath, char *name)
> > > +{
> > > + // $snappath/$name/comment
> > > + int ret, len = strlen(snappath) + strlen(name) + 10;
> > > + char *s = malloc(len);
> > > +
> > > + if (s) {
> > > +         ret = snprintf(s, len, "%s/%s/comment", snappath, name);
> > > +         if (ret < 0 || ret >= len) {
> > > +                 free(s);
> > > +                 s = NULL;
> > > +         }
> > > + }
> > > + return s;
> > 
> > Wouldn't this be better called get_snapcomment_path? Based on the
> > function name I was first surprised not to see any fopen/fread before I
> > actually took a closer look an understand it's only returning th path.
> 
> Yeah, it would, will change that.
> 
> We could just put the whole comment in there, but I could see many-page
> dissertations on the changes made to a particular snapshot. I've also
> considered not having comments, because as I've said I don't want to
> cross that line into being a version control for snapshots (that's
> for docker).  But I think a comment is too useful to not have it.
> 
> Thanks for taking a look.

Yeah, I think the current implementation with only the path being in the
structure is good enough and avoids unnecessary memory usage for those
cases where someone feels like writing a dissertation as comment :)

> 
> -serge

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com

Attachment: signature.asc
Description: Digital signature

------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58041391&iu=/4140/ostg.clktrk
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel

Reply via email to