[lxc-devel] [PATCH] [RFC] snapshots: move snapshot directory (v3)

[S.Çağlar Onur]

On Sat, May 24, 2014 at 11:37 PM, Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> Quoting S.Çağlar Onur (caglar at 10ur.org):
>> Hey Serge,
>>
>> On Sat, May 24, 2014 at 8:15 PM, Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
>> > Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
>> >> Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
>> >> > Originally we kept snapshots under /var/lib/lxcsnaps.  If a
>> >> > separate btrfs is mounted at /var/lib/lxc, then we can't
>> >> > make btrfs snapshots under /var/lib/lxcsnaps.
>> >> >
>> >> > This patch moves the default directory to /var/lib/lxc/c/snaps.
>> >> > If /var/lib/lxcsnaps already exists, then use that.
>> >> >
>> >> > If we are deleting a container which has snapshots, we currently
>> >> > will delete the container itself and its rootfs, but not its
>> >> > snapshots.  This could be confusing for the user, and there is
>> >> > no option to c->destroy() to ask for different behavior.  So
>> >> > currently a user would have to delete all snapshots first, then
>> >> > delete the container.  Ideas for better handling this would be
>> >> > welcome, but we don't want to change the current api, so while
>> >> > adding a new c->destroy_full() would be ok, adding a flags
>> >> > argument to c->destroy(c, flags) is not.
>> >>
>> >> I'm thinking that
>> >>
>> >>       c->snapshot_destroy(c, NULL);
>> >>
>> >> should tell lxc to remove all snapshots.  So then at least we can
>> >>
>> >>       c->snapshot_destroy(c, NULL);
>> >>       c->destroy(c);
>> >>       lxc_container_put(c);
>> >>
>> >> as a way of making sure we delete the whole thing.
>> >
>> > I'm working on a full patch to go about it this way.
>>
>> Sorry for being late in the game but I'm wondering would be make more
>> sense to introduce a new method called lxcapi_snapshot_destroy_all to
>> do lxcapi_snapshot_list followed by a loop to call
>> lxcapi_snapshot_destroy instead of overloading the snapname parameter?
>
> Hm, we could do that.  Note that since snapshots are always called
> snap%d, 'ALL' can't realistically be in use unless the user has
> manually created a container into the snapshot path.
>
> I don't have strong feelings either way.

I have some mixed feelings against the ALL method in v4 :) Personally,
if we have to pick either a magic parameter (referring passing the
NULL as snapname) or passing a magic string (referring passing the
"ALL" string as snapname) my preference would be the magic parameter.
Providing the intent via a pre-defined string sounds trouble some to
me.

I guess you chose the later so that one can pass ALL to lxc-snapshot
utility but this behavior could be implemented via new parameter
called --all/-a (eg; lxc-snapshot -name xyz -d --all)

What do you think?

> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel

Cheers,
-- 
S.Çağlar Onur <caglar at 10ur.org>