| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-13 | |||
| 15:57:17 | mriedem | which would be a bug since....juno? | |
| 15:59:38 | mriedem | johnthetubaguy_: i added you to https://review.openstack.org/#/c/552078/ in case you can give any feedback before i rev it | |
| 16:00:34 | johnthetubaguy_ | mriedem: will try take a peak at that, been out at a conference-ey thing all week, so miles behind the curve right now | |
| 16:01:28 | mriedem | ack thanks | |
| 16:01:56 | cfriesen | mriedem: I think the issue is more that we don't recalculate the numa topology on a rebuild....we actually used to do a claim on a rebuild | |
| 16:02:04 | mriedem | johnthetubaguy_: were these guys there? https://i.ytimg.com/vi/sVw0i5Rd_Is/maxresdefault.jpg | |
| 16:02:22 | mriedem | cfriesen: when did we ever do a claim on a rebuild? | |
| 16:02:29 | mriedem | and don't confuse rebuild with evacuate | |
| 16:02:49 | cfriesen | mriedem: yeah, you're right it was for the evac case. | |
| 16:02:49 | johnthetubaguy_ | mriedem: I hadn't noticed the resemblance till just now, but yes they were | |
| 16:03:32 | johnthetubaguy_ | mriedem: although it probably makes me scooter | |
| 16:04:02 | mriedem | ha | |
| 16:04:11 | jgwentworth | mriedem: https://review.openstack.org/#/c/561261 | |
| 16:04:28 | openstackgerrit | Dan Smith proposed openstack/nova master: Implement certificate_utils https://review.openstack.org/479949 | |
| 16:04:29 | openstackgerrit | Dan Smith proposed openstack/nova master: Add trusted_image_certificates to REST API https://review.openstack.org/486204 | |
| 16:04:30 | openstackgerrit | Dan Smith proposed openstack/nova master: Plumb trusted_certs through libvirt driver image paths https://review.openstack.org/561262 | |
| 16:04:39 | cfriesen | mriedem: you going to raise a bug report for the image validation on rebuild? | |
| 16:04:48 | mriedem | cfriesen: no, can you please? | |
| 16:04:51 | cfriesen | sure | |
| 16:04:55 | mriedem | thanks | |
| 16:06:00 | jgwentworth | superdan, mriedem, leakypipes, alex_xu, johnthetubaguy_, finucannot, bauzas, fried_rice: been meaning to ask for review on the rocky priorities doc page https://review.openstack.org/#/c/557796 | |
| 16:06:51 | mriedem | ack | |
| 16:07:05 | fried_rice | jgwentworth: There was a typo it looked like Mel wanted to fix. Otherwise I'm +1 (discussion inline leading to possible external actions, not in the patch) | |
| 16:07:23 | fried_rice | jgwentworth: ...a typo and a link | |
| 16:07:50 | jgwentworth | oh, right. I forgot | |
| 16:08:11 | jgwentworth | I don't think there's a way to filter only Approved blueprints in launchpad, which sucks | |
| 16:08:26 | jgwentworth | for linking | |
| 16:09:29 | fried_rice | jgwentworth: There is some mystical querystring syntax that only mriedem knows how to use. | |
| 16:10:10 | jgwentworth | I thought I've asked him about that before. maybe he's holding out on me | |
| 16:10:56 | mriedem | not in the UI that i know of, | |
| 16:10:59 | mriedem | via the REST API yes | |
| 16:11:09 | superdan | I think LP has a bunch of inaccessible query strings you can use | |
| 16:11:13 | mriedem | right | |
| 16:11:32 | mriedem | https://github.com/openstack/nova-specs/blob/master/tools/move_implemented_specs.py | |
| 16:12:40 | sahid | superdan: i saw your comments and questioning on the tx/rx queue size patch. I tried to give more details I hope that is going to help | |
| 16:15:31 | superdan | sahid: okay the thing I'm worried about is I thought that the guest had to support vhost (or is it vhostuser?) in a specific way, and I haven't gone to chase down all the places where we choose between those different types of virtio backends | |
| 16:15:31 | jgwentworth | mriedem: cool script | |
| 16:18:22 | cfriesen | mriedem: here's the bug: | |
| 16:18:23 | cfriesen | LOG.info("Device %s not found in instance.", | |
| 16:18:25 | cfriesen | disk_dev, instance=instance | |
| 16:18:30 | cfriesen | grr......https://bugs.launchpad.net/nova/+bug/1763766 | |
| 16:18:30 | openstack | Launchpad bug 1763766 in OpenStack Compute (nova) "nova needs to disallow topology changes on image rebuild" [Undecided,New] | |
| 16:20:14 | superdan | sahid: okay re-reading this with your context I have a little better idea on what to look at | |
| 16:20:20 | sahid | superdan: the driver are on the guest OS. that is your question? | |
| 16:20:23 | superdan | sahid: I'll try to do that while you wor on your unit test | |
| 16:20:32 | superdan | sahid: yeah, the guest | |
| 16:21:02 | superdan | sahid: I thought that if we specify one of those (I think vhostuser) the guest _has_ to have something new enough to support it? | |
| 16:21:15 | superdan | although maybe that doesn't matter since you're only overriding back to vhost/ | |
| 16:22:51 | superdan | sahid: I guess it would be less confusing if you did your refactor of get_base_config() first | |
| 16:23:09 | mriedem | jgwentworth: one thing i've always meant to do with that is move the validation that the spec filename matches the blueprint in LP into the doc/test validation code that runs on each patch | |
| 16:23:17 | sahid | well for vhostuser the virtio driver needs to be recent enough and for vhost the kernel recent enough | |
| 16:23:18 | superdan | sahid: can/do we validate some of these types in upstream CI? | |
| 16:23:19 | mriedem | since people frequently make that mistake and then we have to fix it up later | |
| 16:24:01 | jgwentworth | mriedem: you mean in the tox run of nova-specs | |
| 16:24:06 | mriedem | yeah | |
| 16:24:08 | sahid | superdan: yes probably i was thinking about to refactor that method first... | |
| 16:24:08 | superdan | sahid: yeah, I'm just trying to make sure we're not ever selecting something different after this patch, or at least, selecting something different than what would work today | |
| 16:24:20 | sahid | oh i see | |
| 16:24:22 | jgwentworth | makes sense | |
| 16:24:26 | sahid | normally no :) | |
| 16:24:51 | superdan | sahid: if you did that first, then if we have a problem with this later, it would point to the refactor patch instead of your queues patch which would be better I think | |
| 16:25:13 | superdan | sahid: "if we had a problem" meaning.. we selected vhostuser when we should/would have selected vhost or something | |
| 16:26:19 | jgwentworth | mriedem: I think the only wrinkle there could be the LP timeouts that I've run into a fair bit blocking people's specs from passing the tox check | |
| 16:26:47 | sahid | superdan: the thing is the refactor could be big i wanted to limit the scope | |
| 16:27:23 | sahid | i will have to touch lot of thing like the multiqueue support | |
| 16:27:41 | superdan | sahid: so we call all of this from a block that says "if os_vif, do _get_config_os_vif(), otherwise call these per-type methods" | |
| 16:28:00 | superdan | sahid: do we call the os_vif path in this case or not? | |
| 16:28:29 | sahid | superdan: yes for bridge | |
| 16:29:01 | sahid | and actually for vhostuser | |
| 16:29:59 | superdan | okay, so, vif_type could be bridge, where we end up with driver as qemu or vhost, and then vif_type could be vhostuser where we always have driver as vhostuser, is that right? | |
| 16:30:13 | sahid | yes that is the point | |
| 16:30:27 | superdan | any other types that would have driver=vhost | |
| 16:30:27 | superdan | / | |
| 16:30:28 | superdan | ? | |
| 16:30:47 | sahid | yes except if the virt_type is qemu | |
| 16:31:27 | mriedem | jgwentworth: yeah good point | |
| 16:31:38 | superdan | sahid: I'm not sure I know what you mean.. bridge can be vhost or qemu, are there things other than bridge that can use vhost as the driver? | |
| 16:32:45 | superdan | oh, virt-type you meant parallels being the alternative I guess | |
| 16:32:52 | sahid | i mean we always want driver=vhost except when virt-type!=kvm | |
| 16:33:55 | sahid | i need to check my thinking is other driver may use vhost | |
| 16:34:02 | sahid | other vif | |
| 16:34:28 | openstackgerrit | ayoung proposed openstack/nova master: Set scope_types for admin password policies https://review.openstack.org/553613 | |
| 16:34:55 | superdan | sahid: did you say that vhostuser would go through os_vif now too? | |
| 16:36:15 | sahid | i'm not sure i understand what you mean by go throught os_vif | |
| 16:37:05 | sahid | all ovs and linux bridge vifs pass go through os_vif | |
| 16:37:14 | sahid | here we are configuring guest xml | |
| 16:37:45 | sahid | os vif is configuring the network on host | |
| 16:38:02 | superdan | sahid: this: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L554-L559 | |
| 16:38:29 | superdan | because if we take that path, we never call get_config_vhostuser() | |
| 16:39:00 | sahid | get_base_config is called anytime, and yes we have a specific class in os-vif for vhostuser | |
| 16:39:08 | sahid | everytime | |
| 16:39:30 | superdan | sahid: right, so will this ever get run? https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L420 | |
| 16:39:44 | sahid | but no real relation is just about to pass some metadata which will be used by ovs | |
| 16:40:32 | sahid | hum that is why i said i need to double check i think vrouter is using that method | |
| 16:40:59 | sahid | but perhaps they now have a plugin in os_vif | |
| 16:41:04 | superdan | okay I'm not sure how that would get called | |
| 16:42:47 | sahid | superdan: i just check, you think we sould remove it? | |
| 16:43:28 | superdan | sahid: well, your patch modifies it, and it's one of the things I was trying to verify, so if it's dead code then we should remove it so I don't have to ask questions about whether or not your changes are correct :) | |
| 16:45:13 | sahid | i'm just asking myself why we don't have removed it when we have introduced it in os_vif | |
| 16:45:31 | sahid | i will try to find the git commit log | |