| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-11 | |||
| 13:29:18 | bauzas | (reviewing all the blueprints for the review day) | |
| 13:29:49 | bauzas | artom: at least, I put -1 because of the confusing docstring | |
| 13:32:25 | bauzas | artom: heh, jinx for the recheck :) | |
| 13:32:29 | artom | bauzas, aha! Yeah, that probably makes sense | |
| 13:33:01 | artom | bauzas, heh, a lot of volumes tests failed on that one, so I got worried, but it looks like it's an unrelated MySQL failure and not my code | |
| 13:33:08 | bauzas | zactmy | |
| 13:33:28 | bauzas | artom: about the first change, amirite if I think you just need to signal something ? | |
| 13:33:36 | bauzas | you don't really care about the value itsefl | |
| 13:34:01 | bauzas | if so, is the tag info something we can get from the vif info ? | |
| 13:34:44 | bauzas | artom: ^ | |
| 13:34:47 | artom | bauzas, I don't think it's in vif info... | |
| 13:34:52 | bauzas | okay | |
| 13:35:06 | bauzas | if so, we need it in the signature indeed | |
| 13:35:38 | artom | Vif info is that weird network model class, right? If so (and IIRC it is) it's not in there | |
| 13:36:34 | artom | bauzas, strictly speaking, we need it as a boolean flag, so... just check whether it's in kwargs? You think that'd be better? | |
| 13:37:05 | bauzas | artom: I'm thinking about the interface | |
| 13:37:15 | bauzas | I mean that method signature | |
| 13:37:27 | bauzas | and whether a single boolean is all that good | |
| 13:37:53 | bauzas | I tend to avoid adding extra arguments just for a single conditional | |
| 13:38:04 | bauzas | so I tend to regroup all the needed into objects | |
| 13:38:20 | artom | Objects or dicts? Because we know how the latter worked out ;) | |
| 13:38:22 | bauzas | when you can actually lookup the needed within the methods | |
| 13:38:32 | bauzas | nah, objects | |
| 13:38:33 | bauzas | I mean | |
| 13:38:43 | bauzas | just amending a virt signature for passing a boolean is a huge thing | |
| 13:39:02 | artom | Yeah, I agree with that | |
| 13:39:34 | bauzas | that's why I'm trying to understand whether we can either #1 get that elsewhere but not passed in as arg, or #2 pass a better argument that would help more than just a boolean | |
| 13:40:09 | artom | I guess we can add it as a field in the vif info? | |
| 13:40:24 | bauzas | well | |
| 13:40:39 | bauzas | if so, that means a new os-vif version, right ? | |
| 13:40:40 | artom | Also, other virt drivers might need it, I suppose | |
| 13:40:58 | artom | And we pass the tag itself from the compute manager, not just a boolean flag | |
| 13:42:32 | artom | bauzas, hold up, daughter's awake, back a bit later | |
| 13:43:13 | bauzas | np | |
| 13:53:12 | huanxie | Hi sdague, I have rebased my nova patch of using os-xenapi https://review.openstack.org/#/c/406059/, could you help review again? This patch already got +2/+w before, but there is a unit test conflict with upstream, so I rebased and fixed, now it's ready. Thanks a lot :) | |
| 14:04:47 | openstackgerrit | Sergey Nikitin proposed openstack/nova: Standardization of VM diagnostics info API. https://review.openstack.org/355540 | |
| 14:04:47 | openstackgerrit | Sergey Nikitin proposed openstack/nova: Change parent of diagnostics objects to NovaObject class https://review.openstack.org/394480 | |
| 14:04:48 | openstackgerrit | Sergey Nikitin proposed openstack/nova: Completed implementation of instance diagnostics for Xen https://review.openstack.org/399613 | |
| 14:08:50 | huanxie | Hi mriedem, I have rebased my patch https://review.openstack.org/#/c/406059/ which is to use os-xenapi lib, could you help have a review again? Thanks a lot! | |
| 14:13:01 | sdague | huanxie: no prob | |
| 14:13:46 | huanxie | sdague :) | |
| 14:14:31 | openstackgerrit | John Garbutt proposed openstack/nova: Add and use ProcessedNetworkRequest object https://review.openstack.org/418025 | |
| 14:14:32 | openstackgerrit | John Garbutt proposed openstack/nova: Make allocate_for_instance take consistent args https://review.openstack.org/418024 | |
| 14:19:11 | cdent | Kevin_Zheng: are you still working on https://review.openstack.org/#/c/410128/ ? | |
| 14:26:55 | openstackgerrit | Pawel Koniszewski proposed openstack/nova: Update instance group during live migration https://review.openstack.org/339588 | |
| 14:27:21 | openstackgerrit | Markus Zoeller (markus_z) proposed openstack/nova: live-migration test hook for serial console https://review.openstack.org/347471 | |
| 14:46:22 | openstackgerrit | Dan Smith proposed openstack/nova: WIP: Test cellsv1 https://review.openstack.org/418141 | |
| 14:46:59 | mriedem | i smell a feature review sprint coming on https://etherpad.openstack.org/p/nova-ocata-feature-freeze | |
| 14:47:00 | bauzas | dansmith: I stopped reviewing cellsv1 effort since a while | |
| 14:47:13 | bauzas | dansmith: is there some change I should first look at | |
| 14:47:14 | bauzas | ? | |
| 14:47:40 | dansmith | bauzas: that patch is trying to fix cellsv1 when cellsv2 is enabled | |
| 14:47:49 | dansmith | and it's still a ways off from working, so.. ignore it for now | |
| 14:47:50 | bauzas | mriedem: FWIW, looking at the BP whiteboards is somehow difficult | |
| 14:48:11 | bauzas | dansmith: okay, that was because today means I should help others | |
| 14:49:41 | dansmith | bauzas: yeah, you should do that instead of looking at my not-working patch :) | |
| 14:49:49 | bauzas | roger. | |
| 14:50:11 | bauzas | dansmith: today's cells meeting is at 2100UTC, right? | |
| 14:50:30 | bauzas | I *need* to attend that one after lot of times missing it | |
| 14:50:35 | dansmith | bauzas: are you asking me to be your calendar? today's meeting is in the later slot. | |
| 14:51:01 | bauzas | dansmith: no, just verifying my own calendar that I don't trust :) | |
| 14:52:12 | openstackgerrit | Matt Riedemann proposed openstack/nova: xenapi: support the hotplug of a neutron port https://review.openstack.org/365505 | |
| 14:52:48 | mriedem | bauzas: how would you like me to custom tailor the etherpad to your delicate sensibilities? :) | |
| 14:53:01 | mriedem | bauzas: https://review.openstack.org/#/c/365505/ needs another core | |
| 14:53:02 | bauzas | mriedem: nothing, just a comment :) | |
| 14:53:26 | bauzas | mriedem: we should think about providing a dashboard for the next cycle | |
| 14:53:35 | bauzas | but meh | |
| 14:54:12 | mriedem | huanxie: got it | |
| 14:55:08 | huanxie | mriedem thanks! | |
| 14:55:25 | mriedem | huanxie: note that i already updated the release note on the vif hot plugging patch | |
| 14:55:27 | mriedem | so that's ready to go | |
| 15:01:04 | huanxie | mriedem, I was just trying to ssh to my linux and change this, you have already done, many many thanks:) | |
| 15:01:53 | mriedem | huanxie: in case you haven't done it yet, you can use the edit button in the gerrit UI, or the 'fix' button next to a comment and make changes in the gerrit UI directly | |
| 15:02:42 | mriedem | that will make it diverge from your local copy so i only do it for trivial things that are ready to get approved | |
| 15:04:24 | huanxie | mridem, I see, I haven't modified directly in gerrit UI, will have a try in my other patch, it's useful for very small change | |
| 15:07:08 | cdent | anywhere in particular I should put my eyes for sprinting, or just let the etherpad be my guide? | |
| 15:08:39 | huanxie | mriedem could you also help review this patch https://review.openstack.org/#/c/406059/, this patch is the first one when using os-xenapi, thanks a lot | |
| 15:09:51 | mriedem | huanxie: that's already approved | |
| 15:10:04 | mriedem | cdent: the diagnostics series could probably use a fresh set of eyes | |
| 15:10:11 | mriedem | as i think it was recently rebased | |
| 15:10:29 | mriedem | cdent: https://review.openstack.org/#/q/topic:bp/restore-vm-diagnostics,n,z | |
| 15:11:04 | cdent | thanks mriedem | |
| 15:12:25 | huanxie | mriedem, I see, remember each patch needs to get two core reviewers +2 or one +2 is enough? | |
| 15:13:00 | mriedem | huanxie: the bottom change was already approved, you just rebased it so sdague fast-approved it again | |
| 15:13:01 | mriedem | which is ok | |
| 15:16:41 | huanxie | mriedem, oh, I see, thanks for explaining this | |
| 15:17:48 | mriedem | raj_singh: some easy things to cleanup in https://review.openstack.org/#/c/397399/ - mostly remove the reset_globals() thing in service_auth.py | |
| 15:17:52 | mriedem | raj_singh: we don't want to repeat that pattern | |
| 15:19:03 | mriedem | raj_singh: and on the devstack change https://review.openstack.org/#/c/409329/ that needs to depend on the nova change which defines the config | |
| 15:19:37 | raj_singh | mriedem: ok, will respin it now. Thx for the review | |
| 15:20:32 | artom | bauzas, heya, back now. So yeah, I agree the method signature is not pretty, but I'm not sure I have a better idea | |
| 15:21:22 | bauzas | artom: yup, I understand | |
| 15:21:27 | bauzas | artom: me too honestly | |
| 15:21:39 | bauzas | fortunately, it's not a RPC API | |
| 15:21:56 | bauzas | so probably we could just add it now and discuss later on how to have a better argument | |
| 15:22:16 | artom | bauzas, I'll definitely update the docstring though | |
| 15:23:25 | mriedem | gibi: did you have a bug for the versioned notifications lazy-load issue? | |
| 15:24:33 | gibi | mriedem: yes | |
| 15:24:47 | gibi | mriedem: here it is https://review.openstack.org/#/c/415857/ | |
| 15:25:07 | mriedem | thanks | |