| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-01 | |||
| 12:32:27 | sean-k-mooney | lyarwood: mdbooth im not sure if we have any gate jobs that test livemigration with multiatached volumes | |
| 12:33:05 | mdbooth | Probably not | |
| 12:33:28 | lyarwood | yeah I don't think there's any multihost multiattach testing at the moment | |
| 12:34:18 | sean-k-mooney | is it just lvm that supports it currently or perhaps the nfs dirver? ceph doesnt right now right | |
| 12:35:57 | lyarwood | sean-k-mooney: yeah LVM/iSCSI does, rbd doesn't etc. | |
| 12:38:37 | sean-k-mooney | right i think we have an existing lvm based multinode job we could add a test too. i might take a crack at that. its been a while since i looked at tempest and kind of want to brush up on it | |
| 12:46:19 | mdbooth | lyarwood: There's also resize/cold migrate to consider | |
| 12:57:15 | openstackgerrit | sean mooney proposed openstack/nova master: PCI: do not force remove allocated devices https://review.openstack.org/626381 | |
| 13:03:15 | mdbooth | lyarwood: Comment in review. That assumption makes me uncomfortable. However, I perhaps haven't had enough coffee to see where it's incorrect yet. | |
| 13:03:31 | mdbooth | Throwing it to you to see if you can come up with anything yourself :) | |
| 13:08:21 | mdbooth | lyarwood: No, this is way way more complicated than this | |
| 13:08:45 | mdbooth | Because we're also potentially racing with other live migrations, resizes or cold migrations | |
| 13:09:02 | mdbooth | We're assuming that only *this* attachment might be in a special state | |
| 13:09:21 | mdbooth | But what if, for eg, we're in the middle of host-evacuate-live, and *everything* is currently live migrating | |
| 13:09:35 | mdbooth | This whole approach is just wrong | |
| 13:10:47 | mdbooth | My head is also too fuzzy to consider the case of cleaning up evacuated instances in init_host | |
| 13:21:15 | mdbooth | lyarwood: Sorry, I don't think we can spit-shine that turd. | |
| 13:25:36 | mdbooth | lyarwood: The design of the nfs volume driver handles both shared targets and multi-attach. I'm pretty sure we could mostly copy it. | |
| 13:25:54 | mdbooth | We also backport that, btw. | |
| 13:26:00 | mdbooth | backported | |
| 13:34:09 | lyarwood | mdbooth: back, reading | |
| 13:34:52 | mdbooth | lyarwood: TL;DR Your edge case has edge cases | |
| 13:35:18 | mdbooth | It just makes things edgier | |
| 13:37:42 | lyarwood | and now this is stuck in my head https://www.youtube.com/watch?v=H8dZwXnMrRU | |
| 13:38:43 | mdbooth | lyarwood: LOL | |
| 13:38:59 | lyarwood | mdbooth: right there's a bigger issue with this anyway that came to me during lunch, instance.host isn't updated until later in the post_live_migration flow. | |
| 13:39:28 | lyarwood | https://docs.openstack.org/nova/latest/reference/live-migration.html - post_live_migration_at_destination actually updates it | |
| 13:39:53 | mdbooth | lyarwood: And that will differ for cold migration, of course. | |
| 13:41:28 | mdbooth | Bill Bailey is awesome | |
| 13:43:12 | mdbooth | lyarwood: So you're suggesting we just overlay the edge cases with a massive effects library which makes them seem awesome? | |
| 13:43:45 | mdbooth | A truly generic solution. | |
| 13:45:04 | lyarwood | os-twiddly-dee | |
| 13:49:36 | mdbooth | lyarwood: Incidentally, I had to watch that twice just to convince myself he was playing Jingle Bells from the start. | |
| 13:50:52 | openstackgerrit | Eric Fried proposed openstack/os-resource-classes master: Add normalize_name utility https://review.openstack.org/634258 | |
| 14:22:34 | lyarwood | mdbooth: okay, found the actual issue causing _should_disconnect_target to return True, during post_live_migration we are fetching the old connection_info from c-api again - https://github.com/openstack/nova/blob/5a4863aa152f58f6de426b3908a75c2cc1b2efca/nova/virt/libvirt/driver.py#L7970-L7976 and that doesn't contain the multiattach key, that's only added by n-cpu during the attach flows | |
| 14:22:36 | lyarwood | https://github.com/openstack/nova/blob/5a4863aa152f58f6de426b3908a75c2cc1b2efca/nova/virt/block_device.py#L538-L549 | |
| 14:23:44 | lyarwood | I'll just grab the volume earlier in _should_disconnect_target and use it to tell if it's multiattach enabled or not. | |
| 14:23:56 | mdbooth | lyarwood: Ah, that one. Didn't I fix that already? | |
| 14:24:37 | mdbooth | lyarwood: Sec... lemme see if I can find those patches | |
| 14:24:40 | lyarwood | mdbooth: not that I can see in this flow | |
| 14:25:20 | mdbooth | lyarwood: Is it related to this patch: https://review.openstack.org/#/c/551302/ ? | |
| 14:27:15 | lyarwood | mdbooth: yeah | |
| 14:28:14 | lyarwood | mdbooth: https://review.openstack.org/#/c/551349/ was still WIP I think | |
| 14:28:36 | mdbooth | Right, but I think the two are somewhat independent | |
| 14:28:51 | mdbooth | Would the first one, appropriately rebased, fix your issue? | |
| 14:29:43 | lyarwood | mdbooth: yes AFAICT | |
| 14:29:54 | mdbooth | lyarwood: Would you like to take it? | |
| 14:31:29 | lyarwood | mdbooth: sure, I'll rebase and push this along. | |
| 14:32:05 | mdbooth | lyarwood: Thanks! | |
| 14:33:54 | mriedem | https://review.openstack.org/#/c/632720/ | |
| 14:33:54 | mriedem | bauzas: can you hit this as stable core? v | |
| 14:34:07 | bauzas | mriedem: of course yeah | |
| 14:42:00 | openstackgerrit | Eric Fried proposed openstack/os-resource-classes master: Add normalize_name utility https://review.openstack.org/634258 | |
| 14:43:08 | rha | Hello. Anybody here, who's willing to do a review on: https://review.openstack.org/#/c/420026/ ? It should be rather straightforward I hope, | |
| 14:50:14 | dansmith | mriedem: I hadn't saved my comments on that serial number patch, so I just did and see that a new rev was postd | |
| 14:50:44 | dansmith | mriedem: I -1d the new one because I don't think we really should or need to deprecate that thing, but I'm not going to die on that hill | |
| 14:52:05 | sean-k-mooney | dansmith: Zhenyu Zheng send an email this morning saying he would push a new version following the discusssion on the ml thread | |
| 14:52:24 | dansmith | sean-k-mooney: you mean the new rev I mention above/ | |
| 14:52:48 | sean-k-mooney | dansmith: i havent looked but i assume so | |
| 14:55:46 | sean-k-mooney | this is the mail i was refering too http://lists.openstack.org/pipermail/openstack-discuss/2019-February/002285.html what would be your preference? keep the config option undeprecated | |
| 14:56:03 | dansmith | sean-k-mooney: that's what I just said, yes :) | |
| 14:56:40 | sean-k-mooney | well what i should of asked is that long term or just for stien | |
| 14:57:07 | dansmith | I don't think there's any reason to deprecate it at all | |
| 14:57:22 | dansmith | there's a potential use for it, it's not blocking other things, nor is it a large maintenance burden, etc | |
| 14:58:42 | sean-k-mooney | it is exposing host info to the guest in a non obficated way. | |
| 14:59:26 | dansmith | it is, which is a good reason to default it to unique, but it's not useful data.. meaning the only thing you could really use it for is to determine if two instances are on the same host | |
| 14:59:31 | dansmith | which you can already do | |
| 14:59:36 | sean-k-mooney | i dont really care i guess one way or another if there is a config but i do think it would be nice to have uniqe id be the default | |
| 14:59:43 | dansmith | agree | |
| 15:07:49 | mriedem | the point of the flavor extra spec and image property were for users that really needed the unique serial to be able to get that regardless of the underlying config, which most operators likely don't even know this exists. in a public cloud, the flavor/image are probably controlled by the operator anyway, so having said that, if you don't have the spec/image property, you could still control this with aggregates if needed, i. | |
| 15:07:50 | mriedem | efault unique everywhere, restrict via aggregate to hosts configured with host serial | |
| 15:07:52 | mriedem | if needed | |
| 15:09:02 | mriedem | and i think we've come full circle now because i think when this originally came up in a nova meeting as a specless blueprint, the idea was just a new config option choice | |
| 15:10:00 | mriedem | http://eavesdrop.openstack.org/meetings/nova/2018/nova.2018-10-18-14.00.log.html#l-199 | |
| 15:30:06 | mnaser | dansmith: fyi the difference between the way hostId exposes things is that it hashes with a tenant_id so a tenant cant know its if on a box with another tenant | |
| 15:30:25 | dansmith | mnaser: yeah I know | |
| 15:30:33 | mnaser | the one exposed via the serial is not hashed, so technically i could keep retrying and spawn a vm on the same host knowingly | |
| 15:30:37 | mnaser | ok cool :) | |
| 15:30:49 | dansmith | yeah, but you also can't use that value anywhere in the api | |
| 15:31:00 | dansmith | I totes think it should be defaulted to unique | |
| 15:31:25 | sean-k-mooney | dansmith: im not sure about that but i dont want to specalate publicly | |
| 15:31:28 | mnaser | ++ i guess you just want to retain the ability to keep that old behaviour if a deployer wants to be | |
| 15:31:40 | dansmith | yes | |
| 15:31:46 | dansmith | because there's really no reason to just remove it | |
| 15:34:26 | mriedem | sean-k-mooney: couple small comments in https://review.openstack.org/#/c/626381/ | |
| 15:35:45 | sean-k-mooney | mriedem: just on a call but ill respin shortly | |
| 15:43:44 | mriedem | stephenfin: alex_xu said he can take over https://blueprints.launchpad.net/nova/+spec/remove-force-flag-from-live-migrate-and-evacuate when he's back in a week so i'm just going to assign it to him | |
| 15:45:49 | sean-k-mooney | mriedem: i think he is currently flying to fosdem but ill let him know | |
| 15:47:17 | openstackgerrit | Elod Illes proposed openstack/nova stable/queens: Fix incompatible version handling in BuildRequest https://review.openstack.org/634439 | |
| 15:48:39 | mriedem | zzzeek: thanks for https://bugs.launchpad.net/oslo.db/+bug/1814199 i can confirm with your patch to oslo.db that the SAWarning is gone from nova unit test runs on py35 | |
| 15:48:40 | openstack | Launchpad bug 1814199 in oslo.db "soft_delete is wrong" [Undecided,New] | |
| 15:48:48 | mriedem | at least the one i recreated the with | |
| 15:49:05 | mriedem | isn't that oslo.db bug just a duplicate of https://bugs.launchpad.net/oslo.db/+bug/1814182 though? do we need both bugs? | |
| 15:49:06 | openstack | Launchpad bug 1814182 in oslo.db "SAWarning: Evaluating non-mapped column expression 'id' onto ORM instances; this is a deprecated use case. Please make use of the actual mapped columns in ORM-evaluated UPDATE / DELETE expressions." [Medium,Confirmed] | |
| 15:50:07 | mriedem | Oh I guess maybe you meant to use bug 1814182 as a workaround to pass in synchronize_session=False to soft_delete()? | |
| 15:50:08 | openstack | bug 1814182 in oslo.db "SAWarning: Evaluating non-mapped column expression 'id' onto ORM instances; this is a deprecated use case. Please make use of the actual mapped columns in ORM-evaluated UPDATE / DELETE expressions." [Medium,Confirmed] https://launchpad.net/bugs/1814182 | |
| 16:02:43 | openstackgerrit | sean mooney proposed openstack/nova master: PCI: do not force remove allocated devices https://review.openstack.org/626381 | |
| 16:15:24 | openstackgerrit | sean mooney proposed openstack/nova master: Libvirt: do not set mac when unplugging macvtap vf https://review.openstack.org/624842 | |
| 16:15:25 | openstackgerrit | sean mooney proposed openstack/nova master: Allow per-port modification of vnic_type and profile https://review.openstack.org/607365 | |
| 16:15:25 | openstackgerrit | sean mooney proposed openstack/nova master: Add free for claimed, allocated devices https://review.openstack.org/616120 | |