| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-14 | |||
| 15:39:19 | BobBall | melwitt: resize2fs takes a check_exit_code - we could indeed put the default there, or decide not to pass it on if None was provided? Or perhaps default to True? | |
| 15:39:45 | BobBall | (IMO check_exit_code=True makes more sense than check_exit_code=[0] | |
| 15:39:48 | melwitt | BobBall: hang on, sorry | |
| 15:41:35 | melwitt | I'm seeing there's a default check_exit_code=False in resize2fs in nova/virt/disk/api.py and I wonder why the code wasn't written to call that instead of calling the privsep module methods directly | |
| 15:43:11 | BobBall | melwitt: I believe because we don't want to run the e2fsck twice? | |
| 15:43:34 | melwitt | BobBall: yeah, and it can't accept the flags kwarg | |
| 15:43:48 | BobBall | Ah - yes. I hadn't spotted that bit :) | |
| 15:44:46 | BobBall | The second call could more easily use the general resize2fs although that would run an unwanted e2fsck again | |
| 15:44:59 | BobBall | and on large disks that can take 10s of minutes... | |
| 15:45:12 | melwitt | no, it's okay, I'm just now realizing that virt/api.py method is for ... someone else apparently | |
| 15:51:55 | BobBall | Joyfulness... | |
| 15:52:26 | melwitt | yeah :| | |
| 15:52:56 | BobBall | How about we just pass check_exit_code=True then? A good enough improvement over check_exit_code=[0]? | |
| 15:52:58 | cfriesen | so we just hit https://bugs.launchpad.net/nova/+bug/1659062 in production, and it left the instance in a state where "reset-state" and "rebuild" didn't work to recover it. | |
| 15:52:59 | openstack | Launchpad bug 1659062 in OpenStack Compute (nova) "Failed evacuations leave neutron ports on destination host" [Wishlist,Opinion] | |
| 15:53:47 | cfriesen | anyone have any ideas on the best way to handle this? fixups in init_instance() on the source? | |
| 15:54:12 | melwitt | BobBall: we could. I was just looking at the old pre-privsep code and it used to be that you could omit check_exit_code and get the default and I think that would make things easier for people, but that can be a separate improvement | |
| 15:55:12 | BobBall | melwitt: I'm happy to make the default for fs.resize2fs be check_exit_code=True and remove it from the XenAPI code if that's the preferred fix. | |
| 15:56:25 | BobBall | melwitt: As it's currently non-optional, all callers must be passing a value therefore having the default set to True won't hurt anyone | |
| 15:58:30 | melwitt | BobBall: yeah, that's true. others might disagree and think the current way of "everybody has to pass it" is better, so I'm thinking not to add any potential debate to your fix | |
| 15:59:29 | BobBall | melwitt: OK - so do you now think we should leave the fix as-is for now? (i.e. passing [0]?) | |
| 15:59:58 | melwitt | yeah, I think that's fine | |
| 16:02:27 | BobBall | +1 | |
| 16:04:47 | melwitt | mriedem: simple fix for xenapi driver regression https://review.openstack.org/568318 tldr: resize broke from the privsep changes but tempest didn't catch it because tempest tests resize with a 0 disk flavor => other 0 disk flavor and xenapi driver skips disk resize in that case | |
| 16:09:47 | mriedem | done | |
| 16:10:47 | melwitt | thanks | |
| 16:14:57 | openstackgerrit | Eric Fried proposed openstack/nova master: Granular requests to get_allocation_candidates https://review.openstack.org/515811 | |
| 16:27:14 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Implement granular policy rules for placement https://review.openstack.org/524425 | |
| 16:27:15 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add granular policy rules for /resource_classes* https://review.openstack.org/565578 | |
| 16:38:12 | mriedem | vdrok: it was the missing dosfstools | |
| 16:38:14 | mriedem | http://logs.openstack.org/60/567860/7/check/nova-live-migration/09d27ca/ passes now | |
| 16:42:14 | mriedem | lyarwood: ^ so you can ignore my earlier ping, +2 on your fix again | |
| 16:47:11 | openstackgerrit | Matt Riedemann proposed openstack/nova master: add lower-constraints job https://review.openstack.org/555961 | |
| 17:07:04 | openstackgerrit | Sylvain Bauza proposed openstack/nova-specs master: Proposes Multiple GPU types https://review.openstack.org/557065 | |
| 17:07:43 | bauzas | jaypipes: dansmith: you reviewed https://review.openstack.org/557065 here is a new revision, but I also provided an implementation change with https://review.openstack.org/#/c/564214/ | |
| 17:07:52 | bauzas | jaypipes: dansmith: do we really need a spec for that ? | |
| 17:08:16 | bauzas | for sure, it's a new conf opt, but no upgrade change | |
| 17:08:47 | dansmith | bauzas: you're asking if you need a spec that you have a spec for? | |
| 17:09:14 | dansmith | seems like a reasonable thing to have a spec for, imho | |
| 17:09:52 | bauzas | dansmith: I meant that given we already discussed alot about the possibilities, now we have the implementation change, looks like it's just an implementation point | |
| 17:09:59 | bauzas | but anyway | |
| 17:10:03 | bauzas | the spec is there for folks | |
| 17:10:15 | bauzas | tbh, the most important for me is https://review.openstack.org/#/c/564214/ | |
| 17:25:25 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Wait for network-vif-plugged before starting live migration https://review.openstack.org/558001 | |
| 17:25:26 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add check if neutron "binding-extended" extension is available https://review.openstack.org/523548 | |
| 17:25:27 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add "bind_ports_to_host" neutron API method https://review.openstack.org/523604 | |
| 17:25:28 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add "delete_port_binding" network API method https://review.openstack.org/552170 | |
| 17:25:29 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add "activate_port_binding" neutron API method https://review.openstack.org/555947 | |
| 17:25:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Delete port bindings in setup_networks_on_host if teardown=True https://review.openstack.org/556333 | |
| 17:25:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Implement migrate_instance_start method for neutron https://review.openstack.org/556334 | |
| 17:25:32 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add VIFMigrateData object for live migration https://review.openstack.org/515423 | |
| 17:25:33 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add VIFMigrateData.get_dest_vif https://review.openstack.org/566931 | |
| 17:25:34 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: factor out pre_live_migration plug_vifs call https://review.openstack.org/566932 | |
| 17:25:35 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: use dest host port bindings during pre_live_migration https://review.openstack.org/566933 | |
| 17:25:36 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: use dest host vif migrate details for live migration https://review.openstack.org/551370 | |
| 17:25:37 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Annotate flows and handle PortBindingDeletionFailed in ComputeManager https://review.openstack.org/551371 | |
| 17:25:38 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Port binding based on events during live migration https://review.openstack.org/434870 | |
| 17:25:39 | openstackgerrit | Matt Riedemann proposed openstack/nova master: conductor: use port binding extended API in during live migrate https://review.openstack.org/522537 | |
| 17:30:19 | mnaser | can i please get eyes on https://review.openstack.org/#/c/566425 ? | |
| 17:39:51 | openstackgerrit | Dan Smith proposed openstack/nova master: Honor availability_zone hint via placement https://review.openstack.org/546282 | |
| 17:42:49 | mnaser | dansmith, efried, mriedem: thank you, also i like how dansmith and mriedem came up to the same exact thoguht process | |
| 17:43:32 | dansmith | heh | |
| 17:43:43 | mriedem | we finish each other's | |
| 17:44:01 | dansmith | sentences. | |
| 17:44:25 | efried | you guys are gross | |
| 17:44:32 | dansmith | hah | |
| 17:44:56 | dansmith | we're both planning to get pre-summit haircuts on wednesday | |
| 17:45:18 | mriedem | mine is tomorrow | |
| 17:45:22 | dansmith | aww | |
| 17:54:04 | mnaser | i cant be the only one who constantly types 'reno add', get an error then 'reno new' | |
| 17:58:42 | mriedem | tox -e venv -- reno new <slug> | |
| 18:01:44 | mriedem | dansmith: https://review.openstack.org/#/c/567899/ fixes a regression that was backported through to ocata so we should probably get that fix in and backported, | |
| 18:01:55 | mriedem | i had a recreate with our live migration job and the patch on top shows this fixes it | |
| 18:04:16 | zcorneli | dansmith: For file backed memory, any new thoughts on how to handle the old->new migration issue? | |
| 18:07:11 | openstackgerrit | Merged openstack/nova master: Suppress UUID warning in map_instance unit tests https://review.openstack.org/568263 | |
| 18:07:58 | openstackgerrit | Mohammed Naser proposed openstack/nova master: Added ability to configure default architecture for ImagePropertiesFilter https://review.openstack.org/566425 | |
| 18:08:23 | dansmith | mriedem: ack | |
| 18:08:48 | dansmith | zcorneli: remind me, the new xml is calculated on the sending node, to be used by the destination node right? | |
| 18:09:12 | zcorneli | dansmith: That's what I've seen, and how the code seems to expect it. | |
| 18:09:31 | zcorneli | dansmith: Haven't seen any XML modification on the destination yet. | |
| 18:10:06 | dansmith | zcorneli: yeah, so, if that's the case, then an older node migrating to a newer one may break things by sending it a non-file-backed xml, when the node is configured to use file-backed | |
| 18:10:16 | dansmith | because it's ignorant | |
| 18:10:55 | dansmith | zcorneli: so, if there's a way for the new node to reject the migration gracefully if it detects that situation, that'd be ideal | |
| 18:12:04 | dansmith | zcorneli: so on the destination, some way to check that the source node is new enough to be honoring this.. maybe with the flag in the migratedata object, if that gets passed to the destination from the source at that point? | |
| 18:12:18 | zcorneli | dansmith: Artom proposed rejecting the migration (He's working on something similar with NUMA?), but you mentioned that's not necessarily nice on the scheduler. | |
| 18:12:41 | zcorneli | I can check with him to see how he's checking for it, and use something similar (No point to reinventing the wheel) | |
| 18:12:50 | dansmith | zcorneli: we need to avoid banning cross-version migrations altogether | |
| 18:13:40 | dansmith | zcorneli: it'd be nice if the scheduler could know about it, but I think it's probably too much for the first go-round | |
| 18:13:49 | zcorneli | dansmith: I think we can just ban it when it would break things. Just need to do some investigation on how the migration_data is passed around, etc. | |
| 18:14:40 | dansmith | zcorneli: yeah, so I think maybe it's okay to just reject the migration on the destination if that happens, and then make a big stink in the release note that you probably shouldn't turn that on for any new nodes until everything is upgraded | |
| 18:16:45 | zcorneli | dansmith: Works for me. Maybe a note in the config entry too. | |
| 18:17:05 | dansmith | zcorneli: yeah, also put this wrinkle in the spec | |
| 18:37:39 | mriedem | dansmith: the source node modifies the xml to send to the dest | |
| 18:37:48 | mriedem | based on the migrate_data sent back from the dest during pre_live_migration | |
| 18:37:49 | dansmith | yeah | |
| 18:38:04 | dansmith | and the migratedata is created for the first time on the dest right? | |
| 18:38:09 | mriedem | yes | |
| 18:38:18 | mriedem | in check_dest_can_live_migrate.... something like that | |
| 18:38:21 | dansmith | yeah, so that'll make it hard for a new dest to know it should fail | |
| 18:38:28 | mriedem | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6444 | |