Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-12-02
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.
17:40:10 efried meaning the instance was deleted by the time you went to emit the notification.
17:40:55 sean-k-mooney gibi: well assuming the call order works out ill follow up with this later
17:41:25 efried So, to be clear, the 422 is a special case, with a prominent NOTE and a nice debug log stating that the binding completed before the instance landed on the host, so it's acceptable that the notification is dropped.
17:42:06 efried Sundar: refactoring method X from patch Z into patch Y because that's where it's now going to be used, yeah, that's SOP.
17:42:19 Sundar efried: There could be a bunch of different failure modes: cannot locate n-api service, located but it timed out?, etc. We need to distinguish that category from 'called n-api and it returned an error'. In the latter case, is there granularity of error codes for why it failed?
17:42:50 efried Sundar: Yes, the 422 is a special case; any other error is an error.
17:43:41 Sundar Need to ceck if 422 is used for any other error case.
17:43:44 Sundar *check
17:47:10 Sundar Re. the patch series, I think we need to squash https://review.opendev.org/631245 into the previous patch https://review.opendev.org/631244. Because once the get_resolved_arqs() moves to previous patch, the only things left behind are the virt driver changes. It would arguably be easier to understand those changes together with the changes to get

Earlier   Later