From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Dillaman Subject: Re: Snapshots of consistency groups Date: Fri, 16 Dec 2016 09:53:16 -0500 Message-ID: References: <20160819124813.GB17630@gmail.com> <20160820162713.GA5997@gmail.com> Reply-To: dillaman@redhat.com Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: Received: from mail-lf0-f41.google.com ([209.85.215.41]:33726 "EHLO mail-lf0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754718AbcLPOxd (ORCPT ); Fri, 16 Dec 2016 09:53:33 -0500 Received: by mail-lf0-f41.google.com with SMTP id c13so9738108lfg.0 for ; Fri, 16 Dec 2016 06:53:18 -0800 (PST) In-Reply-To: Sender: ceph-devel-owner@vger.kernel.org List-ID: To: Victor Denisov Cc: Mykola Golub , ceph-devel Can you give a little background on this specific inconsistent case you are referring to? On Thu, Dec 15, 2016 at 7:05 PM, Victor Denisov wrote: > Yes, but if image's snapshot is renamed then I'm not able to find this > snapshot having only group's snapshot in an inconsistent state for > example. > > V. > > On Thu, Dec 15, 2016 at 7:10 AM, Jason Dillaman wrote: >> I think I might be confused. When creating a group snapshot, we have >> the ConsistencyGroupSnapshot that allows you to store the necessary >> linkage between the image's snapshot and its associated group snapshot >> [1]. Why not just name the image's snapshots to the same name as the >> parent group snapshot name and search the snapshot's metadata to get >> the linkage? >> >> [1] https://github.com/ceph/ceph/blob/master/src/cls/rbd/cls_rbd_types.h#L255 >> >> On Tue, Dec 13, 2016 at 8:03 PM, Victor Denisov wrote: >>> Jason, >>> >>> My current implementation of consistency group snapshot feature names >>> image snapshots like: __ >>> I rely on this fact when I need to remove a consistency group. It's >>> necessary because if some of image snapshots were created, but the >>> whole group snapshot operation failed, >>> then the only way to find those dangling image snapshots is by this name. >>> >>> It means that we should forbid renaming snapshots from >>> ConsistencyGroupSnapshot namespace. >>> >>> Another option is to allocate image snapshot ids during the creation >>> of group snapshot, but this requires a major rewrite of the whole >>> process of snapshot creation for images. >>> >>> What is your opinion on this? >>> >>> Thanks, >>> V. >>> >>> >>> On Tue, Nov 1, 2016 at 8:01 AM, Jason Dillaman wrote: >>>> I chatted with Xing on IRC this morning re: Cinder generic groups. It >>>> sounds like RBD will need to support preserving the image's >>>> consistency group snapshots even if the image is removed from the >>>> group. In the OpenStack case, you won't have to worry about the image >>>> being deleted while it still has associated group snapshots. >>>> >>>> We will also want to support being able to clone child images from a >>>> group snapshot to ensure that we can thin provision new groups volumes >>>> when creating a new group from a group snapshot. This means that the >>>> group snapshots should be able to be protected/unprotected just like >>>> standard user snapshots. >>>> >>>> On Mon, Oct 31, 2016 at 9:07 AM, Jason Dillaman wrote: >>>>> Looking at the Cinder codebase, I don't see any such restriction that >>>>> would prevent you from removing a volume from a consistency group that >>>>> has associated snapshots. I would double-check on the OpenStack >>>>> development mailing list if this is correct and is the intent. Worst >>>>> case, the RBD driver could raise an exception if there are still >>>>> consistency group snapshots associated to the image. >>>>> >>>>> >>>>> On Fri, Oct 28, 2016 at 6:41 PM, Victor Denisov wrote: >>>>>> Another thing that bothers me. When we remove an image from a consistency group. >>>>>> Should we remove all snapshots of this image that were created as part >>>>>> of a consistency group snapshot? >>>>>> >>>>>> The easiest solution would be to remove all snapshots that are in >>>>>> GroupSnapshotNamespace and reference this consistency group. >>>>>> I looked into cinder docs for this feature: >>>>>> http://docs.openstack.org/admin-guide/blockstorage-consistency-groups.html >>>>>> >>>>>> But it's not clear to me which behavior cinder expects. >>>>>> >>>>>> Thanks, >>>>>> V. >>>>>> >>>>>> On Wed, Oct 26, 2016 at 6:16 AM, Jason Dillaman wrote: >>>>>>> In a perfect world, it would be nice to add a new optional to "rbd >>>>>>> snap ls" to show all snapshots (with a new column to indicate the >>>>>>> associated namespace). >>>>>>> >>>>>>> On Tue, Oct 25, 2016 at 11:07 PM, Victor Denisov wrote: >>>>>>>> Question. When we print out snapshots of an image, should the group >>>>>>>> snapshots be listed, or should they be marked as special snapshots? >>>>>>>> >>>>>>>> Thanks, >>>>>>>> V. >>>>>>>> >>>>>>>> On Mon, Oct 10, 2016 at 3:14 PM, Victor Denisov wrote: >>>>>>>>> Ok. I didn't have any intention to throw exceptions. >>>>>>>>> I was more concerned about whether it's ok to allocate and delete >>>>>>>>> objects or I should use smart pointers. >>>>>>>>> >>>>>>>>> On Mon, Oct 10, 2016 at 7:18 AM, Jason Dillaman wrote: >>>>>>>>>> The only place exceptions are routinely used is within the "::decode" >>>>>>>>>> functions. I would prefer to see the code not throwing new exceptions >>>>>>>>>> on purpose. >>>>>>>>>> >>>>>>>>>> On Fri, Oct 7, 2016 at 2:26 PM, Victor Denisov wrote: >>>>>>>>>>> Are any exceptions used in librbd code? Should the code be exception safe? >>>>>>>>>>> >>>>>>>>>>> Thanks, >>>>>>>>>>> V. >>>>>>>>>>> >>>>>>>>>>> On Fri, Sep 16, 2016 at 10:37 AM, Jason Dillaman wrote: >>>>>>>>>>>> On Thu, Sep 15, 2016 at 7:17 PM, Victor Denisov wrote: >>>>>>>>>>>>> if (struct_v >= 5) { >>>>>>>>>>>>> ::decode(snapshot_namespace, p); >>>>>>>>>>>>> } else { >>>>>>>>>>>>> snapshot_namespace = cls::rbd::UserSnapshotNamespace(); >>>>>>>>>>>>> } >>>>>>>>>>>>> >>>>>>>>>>>>> then code for ::encode function of cls_rbd_snap would change accordingly: >>>>>>>>>>>>> >>>>>>>>>>>>> instead of >>>>>>>>>>>>> >>>>>>>>>>>>> boost::apply_visitor(cls::rbd::EncodeSnapshotTypeVisitor(bl), >>>>>>>>>>>>> snapshot_namespace); >>>>>>>>>>>>> >>>>>>>>>>>>> I would do: >>>>>>>>>>>>> ::encode(snapshot_namespace, bl); >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> +1 -- looks good to me >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> Jason >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Jason >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Jason >>>>> >>>>> >>>>> >>>>> -- >>>>> Jason >>>> >>>> >>>> >>>> -- >>>> Jason >> >> >> >> -- >> Jason -- Jason