| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-06 | |||
| 10:04:12 | kashyap | (Hmm, the libvirt dev isn't sure if QEMU disallows forbids or not.) | |
| 10:04:27 | mdbooth | You'd need a side-channel between all attached vms to make that safe. | |
| 10:04:42 | mdbooth | And you can't do it in the guest os, because this operation isn't visible to the guest os | |
| 10:05:29 | kashyap | mdbooth: Mind filing a bug for this? | |
| 10:05:30 | mdbooth | kashyap: Which channel are you on? | |
| 10:05:35 | kashyap | #virt, OFTC | |
| 10:05:53 | mdbooth | kashyap: I'm not convinced it's a bug, tbh. | |
| 10:06:04 | mdbooth | Well actually I do think it's a bug, but in Nova. | |
| 10:06:20 | mdbooth | I don't think we should be attempting to do this in the first place. | |
| 10:06:22 | kashyap | (That's what I meant, Nova LP bug) | |
| 10:07:12 | mdbooth | The problem with that is that to fix it we have to break an api | |
| 10:07:24 | mdbooth | i.e. make something fail which previously succeeded | |
| 10:11:00 | kashyap | Hmm | |
| 10:13:38 | kashyap | mdbooth: Can you step back a bit and remind me the actual Nova test? | |
| 10:14:04 | kashyap | It is doing "swap volume", i.e. volume migration? But in "mult-attach" context? | |
| 10:20:29 | kashyap | mdbooth: So, the _default_ behaviour for libvirt is that it won't migrate the shared disks if you don't ask for them to be shared. | |
| 10:20:51 | kashyap | If you _do_ aske the shared disks to be migrated, libvirt will honor that. | |
| 10:21:02 | kashyap | I think Nova shouldn't do that is what you are saying. | |
| 10:21:25 | lyarwood | kashyap: volume migration in this context is swap volume, so rebasing in libvirt terms. | |
| 10:21:46 | kashyap | Yep. blockRebase()/blockCopy() --> calls QEMU 'drive-mirror' | |
| 10:22:32 | lyarwood | kashyap: right, just checking you didn't have it confused with actual block LM. | |
| 10:23:06 | kashyap | Yeah, my head already auto-maps _swap_volume() to the above APIs. But thank you :-) | |
| 10:23:35 | kashyap | lyarwood: Actual block LM also goes down to that 'drive-mirror' call, you know that? | |
| 10:24:16 | lyarwood | kashyap: yeah without going through blockRebase right? | |
| 10:24:30 | lyarwood | kashyap: it's just a copy | |
| 10:26:31 | kashyap | Not sure on that, have to check | |
| 10:48:48 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add request_spec.RequestGroup versioned object https://review.openstack.org/568840 | |
| 10:48:49 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add requested_resources field to RequestSpec https://review.openstack.org/567267 | |
| 10:48:50 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add bandwidth related standard resource classes https://review.openstack.org/570847 | |
| 10:48:51 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transfer port.resource_request to the scheduler https://review.openstack.org/567268 | |
| 10:48:52 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Send resource allocations in the port binding https://review.openstack.org/569459 | |
| 10:50:27 | openstackgerrit | Merged openstack/nova master: Ensure resource class cache when listing usages https://review.openstack.org/572652 | |
| 11:32:46 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Remove network info stubbing in functional test https://review.openstack.org/570018 | |
| 11:32:47 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add request_spec.RequestGroup versioned object https://review.openstack.org/568840 | |
| 11:32:48 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add requested_resources field to RequestSpec https://review.openstack.org/567267 | |
| 11:32:49 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add bandwidth related standard resource classes https://review.openstack.org/570847 | |
| 11:32:50 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transfer port.resource_request to the scheduler https://review.openstack.org/567268 | |
| 11:32:51 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Send resource allocations in the port binding https://review.openstack.org/569459 | |
| 12:21:23 | frickler | mriedem_away: yep, lgtm | |
| 12:38:06 | openstackgerrit | wanghongtao proposed openstack/nova master: Fix the metadata re to match the unicode https://review.openstack.org/536236 | |
| 12:48:36 | openstackgerrit | wanghongtao proposed openstack/nova master: Fix the metadata re to match the unicode https://review.openstack.org/536236 | |
| 13:04:28 | mriedem | lyarwood: i did find via code inspection at least one cinder bug while looking at the access mode logic in the volume manager, comments in the spec https://review.openstack.org/#/c/552078/ | |
| 13:04:59 | mriedem | lyarwood: i don't really have a great way to test this stuff until i get the stack written through the compute api, so i figure any issues that are on the cinder side with modeling per-attachment access mode will have to be resolved as dependent bugs | |
| 13:05:01 | artom | mdbooth, so, re: mailing list post about swap volume - I believe we could just change the response code in the multi-attach case to a 400 or 409 or whatever, without a new microversion | |
| 13:05:03 | artom | https://docs.openstack.org/nova/latest/contributor/microversions.html | |
| 13:05:21 | mriedem | lyarwood: beyond that, what is holding up the spec? i'll rename attach_mode to access_mode to match what's in the connection_info | |
| 13:06:22 | mdbooth | artom: I wasn't sure, but that was my intention. I don't think we should preserve this behaviour at all, even behind a microversion. | |
| 13:06:56 | mdbooth | artom: But I'm not clear on the policy here, hence the ML post. | |
| 13:07:00 | artom | mdbooth, I agree, which is why I would think using a response code that's already possible for that request would be OK | |
| 13:13:33 | sahid | jangutter: yes i understand what you mean, i commented back on the spec | |
| 13:14:33 | sahid | mriedem, stephenfin, if you have a moment to review https://review.openstack.org/#/c/553072/ | |
| 13:15:07 | mriedem | that old thing | |
| 13:16:21 | sahid | yes, that is why i wanted ping you | |
| 13:19:16 | sahid | melwitt: is this | |
| 13:19:51 | sahid | https://review.openstack.org/#/c/567472/ cannot have to be considered part of: bug/1686116 | |
| 13:20:12 | sahid | s/cannot have/cannot be | |
| 13:23:01 | mriedem | sahid: there was a blueprint for that | |
| 13:23:03 | mriedem | not sure why it's a bug now | |
| 13:24:00 | mriedem | sahid: i'll fix my nits in https://review.openstack.org/#/c/553072/ | |
| 13:31:12 | openstackgerrit | Chen proposed openstack/python-novaclient master: Fix typos https://review.openstack.org/572770 | |
| 13:32:33 | mriedem | dansmith: not sure who else should take a look at this network_data.json metadata API spec, but you're probably closest https://review.openstack.org/#/c/312626/ | |
| 13:33:29 | dansmith | I thought we had already done that actually | |
| 13:35:43 | jangutter | sahid: Thanks. It doesn't particularly bother me to use naked VIFHostDevice aside from breaking the pattern. | |
| 13:39:31 | sahid | jangutter: which pattern? for VIFHostDevice that looks redondant information | |
| 13:39:55 | mriedem | dansmith: i think that was a slightly different thing that was a bug fix | |
| 13:39:58 | openstackgerrit | Matt Riedemann proposed openstack/nova master: add mtu to libvirt xml for ethernet and bridge types https://review.openstack.org/553072 | |
| 13:40:32 | mriedem | dansmith: this https://github.com/openstack/nova/commit/4b333b989dfc778a8b61db4a1b8552e988a10471#diff-083754c4012f16fa0f9dd9cd703ee0c1 | |
| 13:41:14 | dansmith | ah yep | |
| 13:41:17 | jangutter | sahid: yep, but in https://github.com/openstack/nova/blob/stable/queens/nova/network/os_vif_util.py#L287-L297 it's used, and in https://github.com/openstack/nova/blob/stable/queens/nova/network/os_vif_util.py#L329-L347 the apparent redundancy is explained (poorly, by me) | |
| 13:41:39 | openstackgerrit | Merged openstack/nova master: Add traits check in nested provider candidates https://review.openstack.org/567150 | |
| 13:41:42 | mriedem | dansmith: and looking back, i'm still not sure why we didn't have a new metadata api version for that since it was adding a new key to the response | |
| 13:41:47 | mriedem | we'd never let that fly in the compute rest api | |
| 13:42:02 | mriedem | but metadata api is the wild west pew pew | |
| 13:42:26 | dansmith | well, probably because it's a fairly small iteration | |
| 13:42:33 | dansmith | but yeah | |
| 13:42:33 | dansmith | and because of how metadata is | |
| 13:42:43 | mriedem | sahid: +2 on https://review.openstack.org/#/c/553072/ now | |
| 13:42:56 | dansmith | mriedem: so I haven't looked at this spec in the past, but has someone gone through and looked at what the delta looks like from the existing? | |
| 13:43:15 | sahid | mriedem: ok cool | |
| 13:43:22 | dansmith | or is it just that we only ever populate networks[0] now? | |
| 13:43:39 | mriedem | dansmith: it adds a new ip_addresses field to the response which is a list of fixed ips rather than the first one from the list | |
| 13:43:47 | mriedem | we only take the first fixed ip today | |
| 13:43:53 | mriedem | which is the ip_address field | |
| 13:43:58 | sahid | jangutter: hum... i don't understand the note | |
| 13:43:59 | mriedem | this exposes the list of fixed ips | |
| 13:44:14 | mriedem | and still maintains the single ip_address field - which arguably we don't need to do since it's versioned | |
| 13:44:41 | mriedem | i don't know how sensitive apps are to doing version discovery with metadata | |
| 13:44:53 | dansmith | mriedem: this certainly looks like we're asserting multiple interfaces and ips in the test no? https://github.com/openstack/nova/blob/4b333b989dfc778a8b61db4a1b8552e988a10471/nova/tests/unit/network/test_network_info.py | |
| 13:44:59 | dansmith | er hang on | |
| 13:45:04 | dansmith | https://github.com/openstack/nova/blob/4b333b989dfc778a8b61db4a1b8552e988a10471/nova/tests/unit/network/test_network_info.py#L1118 | |
| 13:45:30 | mriedem | https://github.com/openstack/nova/blob/4b333b989dfc778a8b61db4a1b8552e988a10471/nova/tests/unit/network/test_network_info.py#L1121 | |
| 13:45:30 | mriedem | dansmith: how so? | |
| 13:45:35 | mriedem | that's a single ipv4 fixed ip | |
| 13:45:43 | mriedem | but you can have multiple fixed ips per port in neutron | |
| 13:45:53 | mriedem | which is what this spec is trying to expose via network_data.json | |
| 13:46:04 | dansmith | oh, | |
| 13:46:10 | dansmith | I thought the spec was saying it was the subsequent networks and the ips there that it needed to add | |
| 13:46:12 | jangutter | sahid: Back in the day, when I wrote that, my rationale was something like: the VIF is used to generate the xml for libvirt, the port_profile is used to find the representor and plug it into the bridge. | |
| 13:46:16 | mriedem | there is also something in the spec about multiple subnet routes which i didn't fully follow | |
| 13:46:51 | mriedem | i believe the networks list is per-vif (port) | |