From mboxrd@z Thu Jan 1 00:00:00 1970 From: Victor Denisov Subject: Re: blueprint: consistency groups Date: Fri, 3 Jun 2016 17:33:41 -0700 Message-ID: References: <20160325071933.GA14634@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: Received: from mail-yw0-f174.google.com ([209.85.161.174]:33791 "EHLO mail-yw0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750726AbcFDAdp (ORCPT ); Fri, 3 Jun 2016 20:33:45 -0400 Received: by mail-yw0-f174.google.com with SMTP id h19so95486937ywc.0 for ; Fri, 03 Jun 2016 17:33:44 -0700 (PDT) In-Reply-To: Sender: ceph-devel-owner@vger.kernel.org List-ID: To: Jason Dillaman Cc: Mykola Golub , ceph-devel Just updated the pull request with all fixes. Please review one more time. On Wed, May 25, 2016 at 6:01 PM, Victor Denisov wrote: > Here is the link to my pull request with operations on consistency > groups only: create cg, remove cg, list cgs. > > https://github.com/ceph/ceph/pull/9333 > > On Wed, May 25, 2016 at 5:53 PM, Jason Dillaman wrote: >> Perfect. I prefer to have the PR's commits reflect the clean >> end-state of the change instead of having multiple "tweak" commits >> fixing code review comments. Therefore, a rebase to address the >> comment in its associated comment and force push is the desired >> process. >> >> On Wed, May 25, 2016 at 5:34 PM, Victor Denisov wrote: >>> I this case I case create a separate pull request for consistency >>> group's operations only(create, remove, list). >>> Another pull request will be: add image to cg, remove image, list images. >>> >>> One more question. How do you prefer comments to be addressed, as an >>> additional commit to the pull request or amend the corresponding >>> commit? >>> >>> V. >>> >>> On Tue, May 24, 2016 at 10:30 PM, Jason Dillaman wrote: >>>> The current PR is at approximately 2500 lines -- it would be nice to >>>> have a PR under 1000 changed lines in a perfect world. The trouble >>>> with larger PR is that after you address comments, I have to re-read >>>> it all again. The smaller the PR, the faster it can be reviewed and >>>> merged. If changes depend on another PR, you can always >>>> non-fast-forward merge the other PR branch into your dependent PR >>>> branch so that it's clear what needs to be reviewed. >>>> >>>> On Tue, May 24, 2016 at 6:42 PM, Victor Denisov wrote: >>>>> Yes, I'm working on splitting it into small logically separated commits. >>>>> My current PR is for CRUD operations only(create, remove, add image, >>>>> remove image, show info). >>>>> Do you want even a smaller PR or this one is small enough? >>>>> >>>>> Thanks, >>>>> V. >>>>> >>>>> On Mon, May 23, 2016 at 7:39 PM, Jason Dillaman wrote: >>>>>> Definitely want to ensure that it cleanly merges with master. I would >>>>>> also request, if at all possible, that you break it into individual >>>>>> PRs of concrete sub-tasks for implementing consistency groups for ease >>>>>> of review. Have individual commits for each step of implementing the >>>>>> task would also help (i.e. squash related commits, fix style issues or >>>>>> bugs in the commit that introduced them, etc). >>>>>> >>>>>> On Mon, May 23, 2016 at 6:43 PM, Victor Denisov wrote: >>>>>>> Never mind. I just realized that it will be easier to build it on top >>>>>>> of the latest master in any case. >>>>>>> >>>>>>> On Mon, May 23, 2016 at 8:32 AM, Victor Denisov wrote: >>>>>>>> Jason, >>>>>>>> >>>>>>>> Do you prefer pull requests to be rebased on top of the latest master >>>>>>>> or should I keep it where I started the development? >>>>>>>> >>>>>>>> Thanks, >>>>>>>> V. >>>>>>>> >>>>>>>> On Mon, May 16, 2016 at 8:48 AM, Jason Dillaman wrote: >>>>>>>>> On Thu, May 12, 2016 at 10:45 PM, Victor Denisov wrote: >>>>>>>>>> Jason, >>>>>>>>>> >>>>>>>>>> Do you have any opinion regarding deleting images that are in a >>>>>>>>>> consistency group? >>>>>>>>>> >>>>>>>>>> Should we delete them as well as the references in the consistency >>>>>>>>>> group they belong to or should we prohibit deleting images that are in >>>>>>>>>> a consistency group? >>>>>>>>>> >>>>>>>>>> V. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Right now, if an image has a snapshot we required you to remove all >>>>>>>>> snapshots before removing the image. Along those lines, if an image >>>>>>>>> is in a consistency group and the consistency group has snapshots, the >>>>>>>>> user wouldn't be able to remove the image since it has snapshots nor >>>>>>>>> should the user be able to remove the snapshots associated with the >>>>>>>>> consistency group. In this case, the user would be forced to >>>>>>>>> dissociate the image from the group before attempting to delete it. >>>>>>>>> Therefore, just to keep the actions consistent, you might as well >>>>>>>>> force the user to dissociate an image from the consistency group even >>>>>>>>> if the image doesn't have snapshots. >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Jason >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Jason >>>> >>>> >>>> >>>> -- >>>> Jason >> >> >> >> -- >> Jason