Open Stack

On 05/21/2013 05:16 AM, Flavio Percoco wrote:
> Hi guys,
>
> Small question about [0].
>
> The patch fixes a bug wrt VMs live migration but it also adds a new
> configuration flag to allow admins to disable multi-attach. IMHO,
> adding a new configuration flag shouldn't be allowed in a backport but
> the patch in matter fixes a bug. What would the right behavior be in
> that case?
>
> 1) Modify the patch so that it doesn't adds a new configuration
> parameter.
>
> 2) Let it land as-is.
>
> 3) Don't let it land at all.
>
> I'm more for #1 and #3 (maybe more 1 since it fixes a bug). Notice
> that adding that parameter doesn't change current behavior.
>
>
> Cheers,
> FF
>
> [0] https://review.openstack.org/#/c/29488/1
>

Hey Flavio--

I'd suggest #1 or #3.  I would personally prefer not to manually
backport patches unless the severity of the bugs they are fixed are
critical.  There were some cinder patches proposed for 2013.1.1 that
similarly included a bunch of unrelated stuff with a pretty minimal
fix.  Ideally, the merge review into master would dictate that the
commit be broken up into smaller pieces as a best practice, and that
will make it easier to land to stable branches if needed.  I'm curious
to know how others feel about when its appropriate to create new commits
that backport subsets of master commits vs. straight cherry-picking as a
default.

As for the review you cited, it looks like the bug has neither a
grizzly-backport-potential tag or severity set on the bug.  I'd check
with a core dev to see if thats something that should be considered for
backport.  Current policy on the wiki [1] states changes that introduce
"Incompatible config file changes" are forbidden from the stable
branches.  IMHO, adding a new config flag that is both documented and
comes with a sane default that does not change existing behavior is okay
(what do others think?)  That said, its unclear whether that flag thats
been added in 294881 is required to fix the bug or unrelated.

Adam

[1] https://wiki.openstack.org/wiki/StableBranch#Appropriate_Fixes