| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-12-02 | |||
| 15:32:44 | sean-k-mooney | ya is there an advantage to not useing the same config option | |
| 15:33:01 | sean-k-mooney | is it jsut to make it clear it applies to just this one call | |
| 15:37:24 | mriedem | lyarwood: i left some comments in the patch to try and summarize irc discussion and duplicated the bug to the existing one for the same issue. i'd hold off on writing a functional test until more people weigh in on what the correct direction is here given backports and such. | |
| 15:38:02 | gibi | sean-k-mooney: could you quickly respin https://review.opendev.org/#/c/695118/ to fix stephenfin nits in the conf doc and the release notes? The change looks good to me too but I guess you want to backpor it so a follow up won't help | |
| 15:38:58 | sean-k-mooney | yep i can do that in 5 mins thanks for taking a look | |
| 15:39:35 | gibi | sean-k-mooney: thanks. Ping me and I will +2 | |
| 15:39:50 | mriedem | would be nice if any of the blizzard people would ack that change since they reported it | |
| 15:45:01 | mriedem | stephenfin: tbc i'm waiting on vmware ci results on https://review.opendev.org/#/c/696503/ before moving it forward | |
| 15:45:33 | mriedem | if that ci is busted (let's say we don't get results within the next 24 hours or something) then i'm cool with moving forward since if they can't maintain a working ci then we can't maintain their driver for them | |
| 15:45:43 | stephenfin | coolness | |
| 15:55:35 | sean-k-mooney | ... the release notes tox env takes forever to run | |
| 15:59:30 | openstackgerrit | sean mooney proposed openstack/nova master: add [libvirt]/max_queues config option https://review.opendev.org/695118 | |
| 15:59:45 | sean-k-mooney | gibi: stephenfin ^ nits addressed no other changes | |
| 15:59:53 | gibi | sean-k-mooney: looking... | |
| 16:01:31 | gibi | eandersson: the bug is coming from blizzard ^^ could you check the bugfix from your perspective? | |
| 16:02:26 | sean-k-mooney | i will be filing and fixing a related bug for vhost-user seperatly but that should cover the case in the current bug if not let me know. | |
| 16:08:32 | sean-k-mooney | actully thinking about that i wonder if i should also add extra code to handel updating this on live migration? | |
| 16:11:12 | sean-k-mooney | i dont think we have code for the queue lenghts on live migration so i kind of feel a seperate patch to handel both queue lenght and max queues would make sense since that will need object changes and wont be backportable where as this simple fix is | |
| 16:13:15 | sean-k-mooney | gibi: stephenfin do you think that would be good to do as a followup? looking at the migrate data object we dont pass any of that info today | |
| 16:14:07 | gibi | sean-k-mooney: do you say that during live migration the server will use the source host config regarding the number of queues? | |
| 16:15:16 | sean-k-mooney | yes and rx/tx queue lenghts | |
| 16:15:35 | sean-k-mooney | we have no support for updating this at all | |
| 16:16:16 | sean-k-mooney | gibi: cold migrate will be fine and a hard reboot would fix it after a live migrate | |
| 16:16:52 | sean-k-mooney | it would only be an issue if you went form a new host with a higher limit to an old host with a smaller one | |
| 16:17:27 | sean-k-mooney | the bug in this case was we limit the queues to 8 eventhogh the kernel support 256 so in generally it wont be an issue | |
| 16:17:50 | sean-k-mooney | but for correct ness we should be using the destination value | |
| 16:18:12 | gibi | sean-k-mooney: yeah, I think that the fix to use the dest value make sense | |
| 16:18:30 | sean-k-mooney | it should be a seperate patch however right | |
| 16:18:38 | sean-k-mooney | so we can backport this one | |
| 16:18:57 | sean-k-mooney | the fix for dest value would need an object change | |
| 16:20:00 | sean-k-mooney | assuming livirt allows it at all. it might just say no an reject the live migration | |
| 16:21:09 | sean-k-mooney | gibi: ill file a bug and we can figure out what to do with it later | |
| 16:21:59 | gibi | sean-k-mooney: ack | |
| 16:27:35 | efried | stephenfin: can I get a quick re+A on https://review.opendev.org/#/c/696354/ ? (Updated UT) | |
| 16:32:14 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Enable cross-cell resize in the nova-multi-cell job https://review.opendev.org/656656 | |
| 16:32:15 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Flesh out docs for cross-cell resize/cold migrate https://review.opendev.org/696212 | |
| 16:32:15 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Simplify FinishResizeAtDestTask event handling https://review.opendev.org/695337 | |
| 16:32:16 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add negative test to delete server during cross-cell resize claim https://review.opendev.org/688832 | |
| 16:32:16 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Implement reschedule logic for cross-cell resize/migrate https://review.opendev.org/696213 | |
| 16:32:59 | openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Clarify configuration steps for PF devices https://review.opendev.org/694522 | |
| 16:33:03 | stephenfin | Sure, done | |
| 16:33:26 | stephenfin | efried: Some real nice docs there ^ :) | |
| 16:35:28 | efried | thanks, and noted. | |
| 16:36:06 | dansmith | efried: did you see my comment about the event getting 422? | |
| 16:36:14 | efried | not yet | |
| 16:36:42 | dansmith | okay I don't want sundar to be held up on a decision there or anything | |
| 16:37:04 | efried | reading now | |
| 16:38:41 | efried | dansmith: oh, that's a good point. If cyborg gets this particular 422 it should mean the bind finished before we even got to the host, so the poll inside wait_for_instance_event will trigger and prevent us from waiting (for an event that won't come) | |
| 16:39:16 | dansmith | efried: right, if it gets a 404 then it should fail the bind, but if it gets 422, it shouldn't freak out | |
| 16:39:25 | efried | ++ | |
| 16:39:37 | dansmith | in fact, 422 confirms, in a weird way, that the instance is still there, just not on a host yet | |
| 16:40:16 | dansmith | so if you could chuck that ++ into the review so sundar isn't waiting that would be good | |
| 16:43:30 | efried | dansmith: done | |
| 16:43:52 | dansmith | efried: spanks | |
| 16:44:05 | efried | comfortable and slimming | |
| 16:44:07 | dansmith | efried: have you chatted with him to know that he's responning? | |
| 16:44:11 | dansmith | *respinning | |
| 16:44:18 | efried | haltingly | |
| 16:44:30 | dansmith | haltingly? | |
| 16:44:41 | efried | think voicemail tag | |
| 16:44:45 | efried | but with slack | |
| 16:46:07 | dansmith | I, uh, will trust you | |
| 16:49:53 | stephenfin | dansmith: Can I remove remotable methods on objects if there are no callers left in-tree | |
| 16:50:11 | stephenfin | Or are they versioned since they're RPC'y | |
| 16:50:26 | dansmith | stephenfin: not according to the letter of the law | |
| 16:50:27 | dansmith | and yes, they're RPC by virtue of being remotable | |
| 16:50:43 | dansmith | stephenfin: I would just replace their bodies with raise NotImplementedError() | |
| 16:51:16 | stephenfin | Yeah, this is for Network.(create|destroy|associate|disassociate) | |
| 16:51:19 | stephenfin | I'll do just that | |
| 16:51:24 | dansmith | aye | |
| 16:59:27 | efried | stephenfin: +2 on that docs patch, but a pile of nits if you're so inclined. | |
| 17:00:24 | efried | gibi or stephenfin: are you on board with https://review.opendev.org/#/c/695985/ (ability to cancel instance events)? | |
| 17:01:21 | melwitt | stephenfin: did you see my note here that other quotas related to nova-network will be able to be removed? https://review.opendev.org/#/c/686812/7/nova/api/openstack/compute/quota_classes.py@38 | |
| 17:01:23 | stephenfin | That's way too much context to load up at 5pm /o\ I can hit it in the morning though | |
| 17:01:39 | efried | ack, thx | |
| 17:02:00 | stephenfin | melwitt: Sure did. It's on the list of stuff still to clean up | |
| 17:02:23 | melwitt | stephenfin: k, coolness | |
| 17:10:45 | Sundar | Responded to the discussion on exit_wait_early for instance events in https://review.opendev.org/#/c/631244 | |
| 17:12:33 | Sundar | efried: stephenfin: ^ relates to https://review.opendev.org/#/c/695985/ | |
| 17:12:34 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/stein: libvirt: Add a rbd_connect_timeout configurable https://review.opendev.org/669167 | |
| 17:12:36 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/rocky: libvirt: Add a rbd_connect_timeout configurable https://review.opendev.org/669168 | |
| 17:12:39 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/queens: libvirt: Add a rbd_connect_timeout configurable https://review.opendev.org/669169 | |
| 17:12:46 | lyarwood | melwitt: ^ reopened and rebased | |
| 17:16:48 | melwitt | lyarwood: ack thanks | |
| 17:19:21 | mriedem | lyarwood: did anyone that reported that issue actually say the fix resolved it for them? | |
| 17:19:39 | lyarwood | mriedem: we've just sent a test build out to a stable/queens user | |
| 17:20:14 | lyarwood | mriedem: it's going to fix their issues but will at least confirm that their network is borked | |
| 17:31:08 | efried | Sundar: back atcha. To be clear, I'm not seeing a problem; are you? | |
| 17:33:36 | Sundar | efried: The notification from Cyborg will fail in many/most cases; ignoring that error could be considered a hack, rather than a solution. After all, folks looking at the n-api logs will see an error. Are we ok with that? | |
| 17:33:57 | sean-k-mooney | gibi: this is the bug for the issue with live migrating https://bugs.launchpad.net/nova/+bug/1854844. ill try to think about ways to adress this in a backportable way. i think i know of one way but this might just be something we have to fix on master only | |
| 17:33:58 | openstack | Launchpad bug 1854844 in OpenStack Compute (nova) "libvirt: tx/rx queue lenght and max queues are not updtated on live migration" [Low,Triaged] - Assigned to sean mooney (sean-k-mooney) | |
| 17:34:28 | efried | Sundar: Only if they're looking at debug logs :) | |
| 17:35:00 | efried | The cyborg logs should of course explain what's happening as well. | |
| 17:35:17 | efried | And yes, I'm fine with this. | |
| 17:37:49 | Sundar | efried: From Cyborg's POV, the notification after binding would be best-effort: ignore any failures and complete the binding. The Cyborg logs will indicate as much. | |
| 17:38:25 | sean-k-mooney | gibi: we might be able to store the queue paramater in the port binding profile which is an unversioned dict of strigns filed and was intend to store hypervior specific info for a port. | |
| 17:39:13 | efried | Sundar: Not *any* failures. | |
| 17:39:18 | Sundar | If we're all ok with that, I'm good. But there are a few nuances to consider. First, the function to get the resolved ARQs is now defined in https://review.opendev.org/631245, while it needs to be used in the previous patch https://review.opendev.org/631244. | |
| 17:39:24 | sean-k-mooney | gibi: the use of multiple port bindings for live migration and the rx/tx queue size options were both intoduced in rocky so it should be backportable to all affected version if i do it right | |
| 17:40:00 | efried | Sundar: As dansmith suggested (oh, maybe it was in IRC, not in the patch) you should still fail and clean up the binding if you get e.g. a 404. | |