| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-15 | |||
| 13:21:16 | alex_xu | efried: I think I agree that check should be removed, that isn't totally for fast return | |
| 13:24:31 | efried | alex_xu Where `all_rp_ids` is set(sharing_providers) | non_sharing_rp_ids ? | |
| 13:24:48 | efried | alex_xu Oh, never mind, I see it defined below. | |
| 13:26:20 | alex_xu | efried: https://review.openstack.org/#/c/516778/9/nova/objects/resource_provider.py@2616 | |
| 13:26:57 | alex_xu | the last word "this is the list of resource provider IDs that are NOT sharing resource" doesn't sound true also | |
| 13:27:31 | efried | alex_xu How so? | |
| 13:27:48 | alex_xu | if only request shared resources, "non_sharing_rp_ids" includes the RPs with that shared resources | |
| 13:28:16 | efried | Is that why we're passing those test cases when we don't expect to be? :) | |
| 13:28:26 | alex_xu | efried: yes | |
| 13:29:02 | efried | alex_xu I don't think that's for lack of trying. Looking at the if/else above, that comment *should* be true. | |
| 13:29:09 | alex_xu | efried: how about append a word to say "non_sharing_rp_ids" also includes the shared rp when there only request shared resource? | |
| 13:29:27 | efried | alex_xu If that were the case, we would want to rename that variable. | |
| 13:29:40 | alex_xu | efried: yea :( | |
| 13:30:01 | alex_xu | non_sharing_rp_ids_but_some_time_not | |
| 13:31:26 | efried | alex_xu The bug must be in one of the helpers - one or both of the following must be true: 1) _get_providers_with_shared_capacity is returning non-sharing RPs; 2) _get_all_with_shared is returning sharing RPs. | |
| 13:32:08 | alex_xu | yes | |
| 13:32:19 | efried | alex_xu I'll go write some functional tests to hit just those helpers and see which is the case. Thanks for talking through it with me. | |
| 13:32:42 | alex_xu | efried: cool | |
| 13:35:02 | efried | alex_xu How about this series? https://review.openstack.org/#/c/377138/ | |
| 13:36:02 | alex_xu | efried: I will try to review that seriews tomorrow | |
| 13:36:12 | efried | alex_xu Okay, thanks. | |
| 13:36:16 | alex_xu | efried: np | |
| 13:36:26 | alex_xu | efried: the next is holiday in US? | |
| 13:36:32 | alex_xu | sorry, next week | |
| 13:43:34 | efried | alex_xu Yes. Next Thursday (and many folks get Friday as well). | |
| 13:52:24 | efried | alex_xu _get_providers_with_shared_capacity appears to be okay. | |
| 13:53:27 | alex_xu | efried: what means okay? | |
| 13:53:49 | efried | alex_xu It's working as expected. There was a hole in the test case, but when I fixed it, the test still passed. So I think it's doing what it's supposed to. | |
| 13:55:53 | efried | alex_xu I also just looked over the test case for _get_all_with_shared and I'm convinced it's okay too. | |
| 13:56:26 | efried | alex_xu How else could we be getting sharing RPs in that var? | |
| 13:59:41 | alex_xu | efried: I don't another case to getting sharing rps in that var | |
| 14:00:53 | efried | alex_xu So how would you like to proceed? We know we have bugs in the stack; are we going to try to fix them in the series, or start merging code and attack them after? | |
| 14:11:59 | alex_xu | efried: which bug you reference? | |
| 14:12:25 | efried | alex_xu There are several. All the test patches on the top of this series. | |
| 14:12:36 | efried | alex_xu The good news is that *somewhere* in the series, we've fixed some of them. | |
| 14:13:03 | efried | At this point I think we have two main ones: One where we're getting candidates that cross aggregates; and one where we're getting duplicate candidates returned. | |
| 14:14:13 | openstackgerrit | Eric Fried proposed openstack/nova master: begin refactor AllocCandidates._get_by_filters() https://review.openstack.org/516778 | |
| 14:14:14 | openstackgerrit | Eric Fried proposed openstack/nova master: build ProviderSummary objects in sep function https://review.openstack.org/516779 | |
| 14:14:15 | openstackgerrit | Eric Fried proposed openstack/nova master: build alloc request resources for shared resources https://review.openstack.org/516780 | |
| 14:14:15 | openstackgerrit | Eric Fried proposed openstack/nova master: create allocation request for single provider https://review.openstack.org/516781 | |
| 14:14:16 | openstackgerrit | Eric Fried proposed openstack/nova master: finish refactor AllocCandidates._get_by_filters() https://review.openstack.org/516782 | |
| 14:14:16 | openstackgerrit | Eric Fried proposed openstack/nova master: required traits for no sharing providers https://review.openstack.org/517027 | |
| 14:14:17 | openstackgerrit | Eric Fried proposed openstack/nova master: Add ProviderSummary.resource_class_names @property https://review.openstack.org/517646 | |
| 14:14:17 | openstackgerrit | Eric Fried proposed openstack/nova master: handle traits with sharing providers https://review.openstack.org/517119 | |
| 14:14:18 | openstackgerrit | Eric Fried proposed openstack/nova master: Test alloc_cands with non overlapping sharing RPs https://review.openstack.org/519380 | |
| 14:14:18 | openstackgerrit | Eric Fried proposed openstack/nova master: Test alloc_cands with one RP shared between two RPs https://review.openstack.org/519617 | |
| 14:14:18 | alex_xu | efried: probably we should guarantee the basic use-case works first | |
| 14:14:19 | openstackgerrit | Eric Fried proposed openstack/nova master: Test allocation candidates: multiple aggregates https://review.openstack.org/518633 | |
| 14:14:19 | openstackgerrit | Eric Fried proposed openstack/nova master: Test helper: validate provider summaries https://review.openstack.org/518982 | |
| 14:14:34 | efried | alex_xu Here it is with that extra short-circuit you suggested, and the enhanced test case, and test fixes for the middle of the series. ^ | |
| 14:14:45 | efried | alex_xu Depends what you mean by "basic" :) | |
| 14:14:59 | alex_xu | efried: something like, compute node have local and shared storage at sometime, actually doesn't support by the nova | |
| 14:15:25 | efried | alex_xu Right, at the moment we don't even claim support for shared RPs at all. | |
| 14:15:26 | alex_xu | s/sometime/sametime/ | |
| 14:15:33 | alex_xu | efried: yea | |
| 14:16:15 | efried | alex_xu Taking that out of the picture - essentially just talking about compute-node-as-single-RP - I believe the code is working as expected. | |
| 14:16:33 | efried | jaypipes Good morning. Hope you didn't have local changes to any of your patches :) | |
| 14:17:06 | jaypipes | efried: no, had to take the afternoon off yesterday to deal with some family stuff. apologies. | |
| 14:17:25 | efried | jaypipes No worries, just happy I didn't step on your work. Hope everything is okay. | |
| 14:28:51 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/nova master: Change 'InstancePCIRequest' spec field https://review.openstack.org/449257 | |
| 14:30:09 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Test alloc_cands with indirectly sharing RPs https://review.openstack.org/519601 | |
| 14:33:57 | efried | gibi Thanks ^ - I wasn't positive where that guy should sit in the pile (though I don't suppose it mattered much). | |
| 14:36:26 | gibi | efried: I think it is OK at the end | |
| 14:36:32 | efried | nod | |
| 14:43:03 | mriedem | tonyb: yeah i see ianw's -1, so i guess i'll take a look at changing devstack-gate today instead | |
| 14:49:08 | openstackgerrit | Merged openstack/python-novaclient master: Updated from global requirements https://review.openstack.org/519806 | |
| 14:57:30 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/newton: Correct log message when removing a security group https://review.openstack.org/519586 | |
| 15:11:17 | mriedem | tasker: oh hi | |
| 15:13:08 | tasker | 'allo. | |
| 15:14:59 | openstackgerrit | Jay Pipes proposed openstack/nova master: XenAPI: get vGPU stats from hypervisor https://review.openstack.org/512965 | |
| 15:15:29 | jaypipes | jianghuaw: ^^ just a rebase... | |
| 15:16:12 | openstackgerrit | Jay Pipes proposed openstack/nova master: XenAPI: provide vGPU inventory in compute node https://review.openstack.org/516217 | |
| 15:16:22 | openstackgerrit | Jay Pipes proposed openstack/nova master: XenAPI: create vGPU for instance https://review.openstack.org/516899 | |
| 15:16:58 | tasker | my application has the potential to frequently trigger snapshots of two instances at the same time. regarding the future of bug#1731986, I'm concerned that checking cinder quota / limits prior to asking it for volume snapshots will casue many problems for me. | |
| 15:17:51 | tasker | i don't know Nova as well as you, and I know less about Cinder. maybe we can schedule some time with a cinder dev to discuss things? I'm hoping that since I'm here helping to implement this, I can guide it a bit to suit my needs. | |
| 15:19:29 | tasker | I'm in no rush; the major part of unqiescing the instance is done and I can implement some of these safeguards in my code. | |
| 15:22:04 | mriedem | tasker: i'm working a patch for the up-front quota check now, since i think that's sane and easy as a start, and then we can build on the cleanup stuff after that - what you had in your original patch. as noted, my concerns are with volume backends like NFS which call back to nova to complete a snapshot - i don't know how robust cinder handles deleting a snapshot while it's being created | |
| 15:22:12 | mriedem | my guess is there are no guarantees about behavior | |
| 15:22:28 | mriedem | just like deleting an instance in nova - you can do it at any point, but it's hard to test | |
| 15:26:17 | tasker | gotchya. | |
| 15:27:42 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Document the real behavior of notify_on_state_change https://review.openstack.org/516264 | |
| 15:31:02 | gibi | mriedem: hi! I replied to your comments in https://review.openstack.org/#/c/516264 | |
| 15:33:00 | mriedem | ack | |
| 15:53:15 | efried | jaypipes jianghuaw_ I was thinking to get started on the update_inventory virt driver method. Any concerns with that? (Or anyone already started?) | |
| 15:53:37 | mriedem | sdague: gibi's got a +2 on the flavor description api change - https://review.openstack.org/#/c/515213/ - would be cool if you could peruse that, given your familiarity with the spec | |
| 15:55:56 | openstackgerrit | Eric Berglund proposed openstack/nova master: Use NoDBTestCase for powervm driver tests https://review.openstack.org/520118 | |
| 15:59:19 | tasker | mriedem: what's the command to merge your changes to the patchset locally? | |
| 16:00:00 | jaypipes | efried: it needs to go at the end of the n-r-p series, but if you want to take that on, I'd be cool with it. | |
| 16:00:05 | mriedem | tasker: like my commit message change? | |
| 16:00:11 | efried | jaypipes Rockin | |
| 16:00:34 | tasker | yeah. minor, but I want to get my code up-to-date with what exists in the review system. | |
| 16:01:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: unquiesce instance on volume snapshot failure https://review.openstack.org/519464 | |
| 16:01:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add the ability to get absolute limits from Cinder https://review.openstack.org/520122 | |
| 16:01:55 | mriedem | tasker: i would normally just delete my local branch and use git review -d 519464 to pull the latest patch set from gerrit | |
| 16:05:03 | sdague | mriedem: I can, after lunch. I have to be out the door in a few minutes, and want to be able to give it a good look over | |
| 16:06:12 | mriedem | wfm, thanks | |
| 16:07:21 | sdague | starred it so I won't forget | |
| 16:08:42 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/newton: Pass requested_destination in filter_properties https://review.openstack.org/516119 | |
| 16:09:10 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/newton: Validate new image via scheduler during rebuild https://review.openstack.org/519684 | |
| 16:10:14 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/newton: Set group_members when converting to legacy request spec https://review.openstack.org/517868 | |