Re: Status of migrated issues?

s. > > > On Mon, 29 Jan 2018 at 10:24, Iustin Pop <ius...@google.com> wrote: > >> On 29 January 2018 at 11:08, 'Federico Pareschi' via ganeti-devel < >> ganeti-devel@googlegroups.com> wrote: >> >>> Sorry, this email had completely flown under my radar. >

Re: Status of migrated issues?

January 2018 at 11:08, 'Federico Pareschi' via ganeti-devel < > ganeti-devel@googlegroups.com> wrote: > >> Sorry, this email had completely flown under my radar. >> > > No worries. > > The issues have all been migrated to the github repository, however th

Re: Status of migrated issues?

Sorry, this email had completely flown under my radar. The issues have all been migrated to the github repository, however there is a difference between 'fixed' and 'released' in the old issue tracker on code.google.com. By importing these issues in github, these states turned into tags. I assume

Re: [PATCH stable-2.16 1/1] Ceph/RBD rbd showmapped -p is no longer supported

Sorry for the late reply. LGTM, thanks for taking care of this. Morg. On 13 March 2017 at 10:30, 'Viktor Bachraty' via ganeti-devel wrote: > From: Ansgar Jazdzewski > > In order to solve the missing "-p" option in "rbd showmapped" it

Re: [PATCH stable-2.16] Ceph/RBD rbd showmapped -p is no longer supported

Mostly LGTM, however... I am not very familiar with the system this part of code is interfacing (Ceph/rbd) so I'll trust the committer with the logic behind removing the pool parameter. The py-tests are failing: == ERROR:

Re: [PATCH stable-2.15] Fix tuple-unpacking from QueryInstances result

Patch has been pushed to stable-2.15 @ 9a62b430e30e8a76df628c36fe8c1787535642c0 On 31 January 2017 at 10:59, Federico Pareschi wrote: > Good catch, the patch looks good to me. I've stumbled upon problems > like this in the past with python, it can be really annoying. I'll be >

Re: [PATCH stable-2.15] Fix tuple-unpacking from QueryInstances result

Good catch, the patch looks good to me. I've stumbled upon problems like this in the past with python, it can be really annoying. I'll be pushing this to 2.15, thanks. On 30 January 2017 at 13:18, Yiannis Tsiouris wrote: > Prior to this commit, some code segments tried to unpack

Re: [PATCH stable-2.15] Fix idx in RemoveDisks warning

Patch has been pushed to stable-2.15 @ 48d11548319cfebf5f701c35d02a4d79c0f51bb1 On 31 January 2017 at 10:40, Federico Pareschi wrote: > Hey Yannis. Patch looks good, thanks for submitting this, I'll be > pushing it to 2.15. > > On 26 January 2017 at 21:03, Yiannis Tsiouris

Re: [PATCH stable-2.15] Fix idx in RemoveDisks warning

Hey Yannis. Patch looks good, thanks for submitting this, I'll be pushing it to 2.15. On 26 January 2017 at 21:03, Yiannis Tsiouris wrote: > Before this patch, the warning of RemoveDisks was incorrectly showing > the index of the disk (whose removal failed) in the list of the >

Re: [PATCH stable-2.15] Make man pages more consistent in parameter format

Well-caught typo, Brian, thanks. I fixed the typo and submitted this @ stable-2.15 970ea7dcab391d4756ed5d7f1c89fcd035ad64ba On Monday, January 16, 2017 at 1:29:48 PM UTC, Brian Foley wrote: > > On Thu, Jan 12, 2017 at 10:59:55AM +, 'Federico Morg Pareschi' via > ganeti-devel wrote: > >

Re: [PATCH stable-2.15] Make man pages more consistent in parameter format

So, I ended up taking another more thorough look at the helper commands and gnt-* man pages. There was a lot of inconsistent syntax and I tried to clean it up and reflect what the commands actually do with regards to UUID vs resource name. Now this patch has gotten a bit bigger than I originally

Re: [PATCH master] Implement predictive queue cluster parameter

This commit was successfully pushed into master @ 2cb216028a063024a3ff4246ec417b23ab9507a1 On 9 January 2017 at 11:43, Federico Pareschi wrote: > I agree completely, and the --user-shutdown flag is the reason why I > kept it consistent with it. I'll be pushing this onto master

Re: [PATCH master] Implement predictive queue cluster parameter

I agree completely, and the --user-shutdown flag is the reason why I kept it consistent with it. I'll be pushing this onto master then, thanks for the review. On 9 January 2017 at 11:40, 'Viktor Bachraty' via ganeti-devel wrote: > Thanks for the patch! LGTM, just

Re: [PATCH stable-2.15] Re-add 2 imports incorrectly removed during cleanup

Yikes, LGTM. Sorry for not spotting this at review time. Thanks for fixing it. On 9 January 2017 at 11:37, 'Brian Foley' via ganeti-devel wrote: > This was done in commit 7b1fd2e60e856ac to quell pylint (my bad!) > > These broke ganeti-rapi and move-instance. Make

Re: [PATCH master] Fix incorrect 'any' function call

Commit has been pushed at ba7013826ab681a5c2b5c1ae7d17dda9cc9d03f6 on master. Thanks. On 4 January 2017 at 15:16, Brian Foley wrote: > On Wed, Jan 04, 2017 at 03:11:41PM +, 'Federico Morg Pareschi' via > ganeti-devel wrote: >> As introduced by commit 77807d83 and

Re: [PATCH master] Fix incorrect 'any' function call

It happens, it escaped my review as well so it's not just your fault. I assume this is an LGTM? :) On 4 January 2017 at 14:38, Brian Foley wrote: > On Wed, Jan 04, 2017 at 02:11:15PM +, 'Federico Morg Pareschi' via > ganeti-devel wrote: >> As introduced by commit

Re: [PATCH stable-2.15] Fix typo in cli_opts.py IGNORE_HVVERSIONS_OPT flag

Commit was pushed at 24ca5625813f087963a3f2c768a5a5bd23bfee4c on 2.15 branch. Thanks :) On 4 January 2017 at 12:08, Brian Foley wrote: > On Tue, Jan 03, 2017 at 08:46:06AM +, 'Federico Morg Pareschi' via > ganeti-devel wrote: >> Signed-off-by: Federico Morg Pareschi

Re: [PATCH stable-2.15] Make man pages more consistent in parameter format

Interesting, you raise valid points for a patch that I thought was trivial but indeed is not. > Per our discussion yesterday, all the gnt-group commands actually do work with > either group specified by name or by UUID, so this is an improvement to the > docs. However, the command line help in

Re: [PATCH stable-2.15] Fix miscellaneous typos found when reading design docs

LGTM, I was planning to go through a lot of these myself when I had the time, so thanks a lot for taking care of it! On 22 December 2016 at 15:29, 'Brian Foley' via ganeti-devel wrote: > Some of these are also in code comments, but don't touch any string > literals

Re: [PATCH master] Quell remaining pylint 1.6.4 warnings after merge

LGTM On Monday, December 19, 2016 at 2:24:09 PM UTC, Brian Foley wrote: > > These should all be harmless modifications with no functional change. > > Signed-off-by: Brian Foley > --- > lib/backend.py| 5 ++--- > lib/cmdlib/instance_set_params.py | 7

Re: [MERGE] stable-2.17 to master

LGTM, thanks for doing this, Brian On Monday, December 19, 2016 at 11:33:46 AM UTC, Brian Foley wrote: > > commit e18785cdd56cf614fe43b603db3a2a51c8c9c2aa > Merge: e763a23 7f2183e > Author: Brian Foley > Date: Mon Dec 19 11:31:06 2016 + > > Merge branch

Re: [MERGE] stable-2.16 to stable 2.17

LGTM On Friday, December 16, 2016 at 5:36:15 PM UTC, Brian Foley wrote: > > commit ac63745b0ee268fa267b1f29cea0aaa79587c7e6 > Merge: f52c6e1 d287130 > Author: Brian Foley > Date: Fri Dec 16 17:23:46 2016 + > > Merge branch 'stable-2.16' into stable-2.17 >

Re: [MERGE] stable-2.15 to stable-2.16

LGTM On Friday, December 16, 2016 at 4:11:30 PM UTC, Brian Foley wrote: > > commit 711fbc08fd895b826d63c1ffc7cb75f35dc4331e > Merge: 703e23e da3f300 > Author: Brian Foley > Date: Fri Dec 16 16:01:48 2016 + > > Merge branch 'stable-2.15' into stable-2.16 >

Re: [PATCH master] Improve the test data generation algorithms

No problem, thanks again for the reviews :) This commit has been pushed in master @ e763a23a339670110df279c900a052b64323c783 On Friday, December 16, 2016 at 3:18:52 PM UTC, Brian Foley wrote: > > On Fri, Dec 16, 2016 at 03:00:41PM +, 'Federico Morg Pareschi' via > ganeti-devel wrote: > >

Re: [PATCH master] Implement starvation-prevention mechanism in queue

Thanks, this patch has been submitted in master @ abf7bf0410a59a1449207a66e664074d4901e85c On Friday, December 16, 2016 at 3:07:25 PM UTC, Brian Foley wrote: > > On Fri, Dec 16, 2016 at 01:51:41PM +, 'Federico Morg Pareschi' via > ganeti-devel wrote: > > This patch adds a starvation

Re: [PATCH master 2/2] Implement starvation-prevention mechanism in queue

Thanks for the review, Brian. > > I suspect we might want these two as cluster parameters like max_running_jobs. > > At first I thought of exposing these as luxid command line options, but > then you'd have the problem of copying updates to these options in > /etc/defaults/ganeti to all nodes, so

Re: [PATCH stable-2.15 00/17] Cleanup for pylint 1.6.4, part two

I have had the exact same comments as Iustin on the entire patchset, seeing as they've already been addressed I'll say LGTM. Good job with the cleanup! On 6 December 2016 at 17:29, 'Brian Foley' via ganeti-devel < ganeti-devel@googlegroups.com> wrote: > This second set of patches cleans up

Re: [PATCH stable-2.15 00/37] Cleanup for pylint 1.6.4

I'm replying here for all the 37 patches, after addressing all the comments, I'd say this is an LGTM for me :) Thanks Brian for the good work. Cheers, Morg. On 5 December 2016 at 12:37, 'Iustin Pop' via ganeti-devel < ganeti-devel@googlegroups.com> wrote: > On 5 December 2016 at 13:04, 'Brian

Re: [PATCH stable-2.15 08/37] Cleanup: Remove unused format key

Ah yes, you're correct. Looks good then, thanks. On 5 December 2016 at 15:18, Brian Foley wrote: > On Mon, Dec 05, 2016 at 12:01:08PM +, Federico Pareschi wrote: > >This is an LGTM for me, however it made me wonder... > >We are adamant in not storing the secret

Re: [PATCH stable-2.15 37/37] Quell pylint socket.timeout warning

LGTM but little nit. s/doensn't/doesn't/ in the commit message :) On 5 December 2016 at 10:37, 'Brian Foley' via ganeti-devel < ganeti-devel@googlegroups.com> wrote: > socket.timeout doensn't have string or errno attributes. > > Signed-off-by: Brian Foley > --- >

Re: [PATCH stable-2.15 13/37] Fix pylint >1.4 pycurl no-member warnings

> > to avoid arbitrary code injection. > Is this safe? Should we be looking more into this or is it something that does not affect us?

Re: [PATCH stable-2.15 08/37] Cleanup: Remove unused format key

This is an LGTM for me, however it made me wonder... We are adamant in not storing the secret parameters anywhere on the cluster, at least according to our design docs. Wouldn't this be going against such policy by having them printed out in an error (and, I assume, logged to disk)? This is

Re: [PATCH stable-2.15 02/37] Cleanup: Simplify boolean assignment

> > diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py > index 40792e2..9ad82d8 100644 > --- a/lib/client/gnt_cluster.py > +++ b/lib/client/gnt_cluster.py > @@ -294,11 +294,6 @@ def InitCluster(opts, args): > >default_ialloc_params = opts.default_iallocator_params > > - if

Re: [PATCH master 2/6] Support OS params removal in gnt-instance modify

One thing that might be worth to keep in mind is that having a * as parameter might become annoying when people try to use it in a cli context with bash auto-expanding it. Just saying :) It's overall a good idea though, but I'm not sure if implementing extra functionality with contextualized glob

Re: [PATCH stable-2.15] Fix for incorrect parsing of DRBD versions

Thanks Brian and Iustin for the review. I've successfully pushed this to 2.15 after addressing some of Brian's comments (as discussed offline). Cheers, Morg. On Friday, December 2, 2016 at 12:21:23 PM UTC, Brian Foley wrote: > > On Fri, Dec 02, 2016 at 11:20:53AM +, 'Federico Morg

Re: [PATCH master 0/6] Clear/remove public/private OS params in gnt-instance modify/reinstall

Aside from the couple of nitpicks that I've already commented in the rest of the patchset, this looks like a reasonable patch to me. I haven't had the time to test it yet, but other than that it looks good to me. On 1 December 2016 at 10:34, Yiannis Tsiouris wrote: > This

Re: [PATCH master 3/6] Add cmdlib tests for removing instance OS params

On 1 December 2016 at 10:34, Yiannis Tsiouris wrote: > From: Nikos Skalkotos > > Add LUInstanceSetParams tests for removing individual public & private > parameters, as well as, for clearing out public & private parameters. > > Signed-off-by: Nikos Skalkotos

Re: [PATCH master 1/6] Add clear OS params options in gnt-instance modify

Thanks for the patch, this is mostly okay aside from a few small comments inline. On 1 December 2016 at 10:34, Yiannis Tsiouris wrote: > This patch adds two new options in 'gnt-instance modify' which allow the > user to clear all current public/private OS parameters of an

Re: [PATCH stable-2.15] Fix for incorrect parsing of DRBD versions

On Thursday, December 1, 2016 at 11:53:43 AM UTC, Iustin Pop wrote: > > On Thu, Dec 01, 2016 at 11:38:35AM +, Ganeti Development List wrote: > > Following issue #1194, this patch allows Ganeti to correctly > > parse drbd versions that also include a dash in their k-fix > > version

Re: [PATCH master] Fix for instance reinstall not updating config

This patch has been pushed to 2.16, thanks Viktor for reviewing it. On Tuesday, November 29, 2016 at 10:07:52 AM UTC, Viktor Bachraty wrote: > > I wouldn't mind backporting up to 2.16, since we haven't released stable > (yet). > > On Tue, Nov 29, 2016 at 9:59 AM, 'Federico Pares

Re: [PATCH master] Fix for instance reinstall not updating config

I also think that originally this behavior was intentional, however it is very non-intuitive to me (and otheres) and it seems to introduce unexpected behavior so it's probably better if we get rid of it. I would put this in master because it breaks user-facing behaviors, should this be

Re: [PATCH stable-2.15] Give atomicWriteFile temp filenames a more distinct pattern

LGTM Just small nitpick: s/filesnames/filenames/ in the comment. On 28 September 2016 at 14:28, 'Brian Foley' via ganeti-devel < ganeti-devel@googlegroups.com> wrote: > This stops them being confused with 'real' filenames by both users and > code if the daemon crashes before the temporary file

Re: [PATCH master] First implementation of the predictive scheduler

Yes, now I see the reasoning behind it. You're right and after giving some thoughts I realized that my changes don't really matter and it's better the way it was before. On Monday, September 12, 2016 at 11:50:59 AM UTC-4, Klaus Aehlig wrote: > > > > This change is dangerous: genName is also

Re: [PATCH master] First implementation of the predictive scheduler

Hey Klaus, thanks for the review. Responses inline. On Monday, September 12, 2016 at 7:16:06 AM UTC-4, Klaus Aehlig wrote: > > > +extractOpCode :: JobWithStat -> Maybe OpCode > > +extractOpCode job = > > + let qop = listToMaybe . qjOps . jJob $ job > > + metaOps = maybe []

Re: [PATCH master] Add design document for a predictive queue system

Responses inline On Friday, August 26, 2016 at 4:01:45 PM UTC+1, Viktor Bachraty wrote: > > As discussed previously IRL, I'm happy with the design so LGTM, I just > have a few nitpicks on the document. > > > On Thursday, August 25, 2016 at 2:00:32 PM UTC+1, Federico Pareschi wrote: >> >> This

Re: [PATCH master] Add design document for a predictive queue system

Thanks, answers inline. On Thursday, August 25, 2016 at 2:20:20 PM UTC+1, Klaus Aehlig wrote: > > [...] > > +something similar to, for example, 1 tick every 30 seconds. Using the > current > > +formula, we can define the actual job weight in the queue:: > > + > > + weight = spv(job)*(1 -

Re: [PATCH master] Add design document for a predictive queue system

This is actually a good point that I had not considered. How much of Ganeti does actually depend on 100% reproducible job queues? The only thing lost in case of luxid being restarted (on the same or on another master node) would be the aging parameter which should only come into play in cases

Re: [PATCH master] Add design document for a predictive queue system

>Will this age parameter be part of the job data? No, the age parameter is a runtime variable that lives in the job scheduler. It is not saved to disk nor propagated anywhere. In case of the master node crashing and the scheduler resuming operations on a new master, this age parameter will be

Re: [PATCH stable-2.17] Fix hlint warnings

LGTM On Wednesday, August 17, 2016 at 11:13:26 AM UTC+1, Brian Foley wrote: > > No functional change. > > Note to self: run make hlint before submitting patches! > > Signed-off-by: Brian Foley > --- > src/Ganeti/MaintD/Server.hs | 2 +- > src/Ganeti/Utils.hs | 2

Re: [PATCH stable-2.16] Use fork instead of spawnv in the watcher

Thanks Brian, I agree with your comments. I also noticed an issue that I hadn't spotted before, I did not rename "pid" to "child" in the code following the fork and that would cause an exception. I thought pylint would've caught that but apparently that's not the case. I'm sending you a

Re: [PATCH stable-2.17] Prevent watcher from holding lock on verify group

Yes, this is definitely the case. We have ideas to implement something in the future to obviate this problem, although we're still considering exactly how to implement it. This is a short-term quick fix to solve some blocker issues that show up as a consequence of it. On 17 June 2016 at 17:57,

Re: [PATCH stable-2.17] Prevent watcher from holding lock on verify group

When a ganeti-watcher runs on the nodegroup, it submits the verify-group job. If there is another job in the queue that is taking some locks that stop the verify-group job (like an instance creation) then the whole ganeti-watcher is blocked and has to wait for that job to finish. We have a case

Re: [PATCH stable-2.16] Improve test coverage for block device creation

Argh yes, silly of me not to notice, sorry. Thanks for reviewing and fixing! Cheers, Morg. On Tuesday, May 10, 2016 at 4:17:07 PM UTC+1, Brian Foley wrote: > > On Tue, May 10, 2016 at 02:28:39PM +0100, 'Federico Morg Pareschi' via > ganeti-devel wrote: > > This commit introduces further test

Re: [PATCH stable-2.15] Fix CLI option typo in NEWS file

Hey Brian, patch seems pretty straightforward, thanks for taking your time to go through it. I just have a very minor nitpick (I mean, this is a typo-based patch anyway). On Friday, May 6, 2016 at 3:35:34 PM UTC+1, Brian Foley wrote: > > As reported in 1177, the option is