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
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