| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-01-16 | |||
| 13:32:14 | ShilpaSD | gibi: help me to understand, in Unit test case framework what all changes i have to carry to resolve this issue | |
| 13:32:23 | ShilpaSD | looking into fake_instance.py | |
| 13:34:02 | sean-k-mooney | kashyap: there are probaly things we can do to hanel some and maybe all the edgecase but we need to be sure the upgrade impact is fully understood. anyway i need to go rebase a path and write tests :) | |
| 13:34:30 | kashyap | [Just checking w/ DanPB elsewhere (since he's not here) about a couple of things.] | |
| 13:34:36 | kashyap | sean-k-mooney: Oh, absolutely. | |
| 13:34:53 | kashyap | I'm with you on that "upgrade impact" needs to be carefully considered. I am all for careful consideration. | |
| 13:36:53 | openstackgerrit | Merged openstack/nova master: Make functional-py37 job work like others https://review.openstack.org/630745 | |
| 13:41:58 | ShilpaSD | gibi: hi | |
| 13:47:15 | gibi | ShilpaSD: I'm not sure about the answer | |
| 13:47:47 | ShilpaSD | gibi: ok, will check further | |
| 13:47:56 | gibi | ShilpaSD: what code in your patch makes sure that the BlockDeviceMapping is loaded from the db? | |
| 13:48:26 | ShilpaSD | gibi: ok, will check that | |
| 13:50:33 | gibi | ShilpaSD: also I think jichenjc has a valid point about loading bdm directly instread of through the instance https://review.openstack.org/#/c/612626/2/nova/api/openstack/compute/simple_tenant_usage.py@170 | |
| 13:52:15 | ShilpaSD | gibi: IMO it will increase DB call and will imact performance | |
| 13:53:36 | gibi | ShilpaSD: I dont see how it is different from db load perspective if we load the bdm through the instance or load it directly by instance.uuid | |
| 13:55:05 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Fix ComputeNode ovo compatibility code https://review.openstack.org/631199 | |
| 13:55:20 | gibi | stephenfin: fixed your comment in ^^ | |
| 14:49:14 | ShilpaSD | mriedem: hi, please give me your views on https://review.openstack.org/#/c/612626/2/nova/api/openstack/compute/simple_tenant_usage.py@170 | |
| 14:50:22 | mriedem | ShilpaSD: what you added wasn't really what i was thinking | |
| 14:50:24 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Fix overcommit for NUMA-based instances https://review.openstack.org/629281 | |
| 14:50:24 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Handle libvirt reporting incorrect 4k page quantities https://review.openstack.org/631038 | |
| 14:50:25 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Make overcommit check for pinned instance pagesize aware https://review.openstack.org/631053 | |
| 14:50:25 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Split '_get_host_numa_topology' function https://review.openstack.org/631237 | |
| 14:52:02 | ShilpaSD | mriedem: ohh, i think misunderstood, can you please expalin in simpler way | |
| 14:52:22 | stephenfin | mriedem: I _think_ most of your comments are addressed for the second of those ^ | |
| 14:52:55 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Fix ovo compatibility code unit tests https://review.openstack.org/631239 | |
| 14:54:57 | mriedem | ShilpaSD: i mean something like https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/models.py#L100 where that creates a 'services' attribute on the Instance model and uses that join query | |
| 14:56:06 | mriedem | we already have a relationship like that between instances and bdms https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/models.py#L599 | |
| 14:56:25 | mriedem | but this would be a more targeted query based on bdm boot_index and destination_type | |
| 14:57:21 | mriedem | and i'm wondering if we can do that when pulling the instance from the db rather than make multiple queries to get all of the instances and then for each instance, get all of the bdms for that instance | |
| 14:59:11 | ShilpaSD | mriedem: thank you for this explaination, i will again go through the said above links and revisit the fixes. | |
| 15:00:39 | mriedem | ShilpaSD: if that works, i'm not sure if it's a field we'd want to always populate on the Instance object or make it optional, like the other fields in _INSTANCE_OPTIONAL_JOINED_FIELDS | |
| 15:01:18 | mriedem | if it's optional, then it could just be a very targeted thing that gets populated in the db api if that field is requested in columns_to_join | |
| 15:01:30 | mriedem | *expected_attrs | |
| 15:01:51 | mriedem | dansmith might have an opinion on that | |
| 15:02:45 | dansmith | yep, like bdms, makes sense | |
| 15:03:55 | efried | mriedem: Do I have my time wrong, or are we supposed to be doing the delete-placement-from-nova hangout right now? | |
| 15:04:20 | gibi | efried: isn't it 17:00 UTC? | |
| 15:04:38 | efried | d'oh, that's my bad. Thanks gibi | |
| 15:04:58 | efried | gibi: and the bw one is also 1700 UTC, on friday? | |
| 15:05:03 | gibi | efried: yes | |
| 15:05:27 | mriedem | ShilpaSD: i left some comments in the patch | |
| 15:06:51 | ShilpaSD | mriedem: will check, thank you for explaination and comments | |
| 15:07:30 | openstackgerrit | Sundar Nadathur proposed openstack/nova master: WIP: Add utility function to get Cyborg client. https://review.openstack.org/631242 | |
| 15:07:30 | openstackgerrit | Sundar Nadathur proposed openstack/nova master: WIP: Add Cyborg device profile groups to spec obj. https://review.openstack.org/631243 | |
| 15:07:31 | openstackgerrit | Sundar Nadathur proposed openstack/nova master: WIP: Create and bind Cyborg ARQs. https://review.openstack.org/631244 | |
| 15:07:31 | openstackgerrit | Sundar Nadathur proposed openstack/nova master: WIP: Get resolved Cyborg ARQs and add PCI BDFs to VM's domain XML. https://review.openstack.org/631245 | |
| 15:14:36 | mriedem | melwitt: will your max volume attach series change the device name of existing attached bdms if the user reboots their server? i believe device tags are linked to bdms via device name, so was wondering if we could possibly have an upgrade impact with the device tag <> bdm.device_name getting out of whack | |
| 15:15:02 | mriedem | artom: ^ you might know off the top of your head on the device tag stuff | |
| 15:15:46 | artom | mriedem, it's been a while, lemme check the code | |
| 15:18:12 | artom | mriedem, yeah, it's by name | |
| 15:24:58 | mriedem | artom: do you remember why we used bdm.device_name as the correlation key for that stuff? maybe bdm uuid didn't exist when you wrote that | |
| 15:25:46 | mriedem | or the volume id for that matter | |
| 15:26:02 | artom | mriedem, there was attempt, I think | |
| 15:26:07 | artom | Lemme trawl through the patches | |
| 15:26:32 | dpawlik | mriedem: hi, if you have free time pls check https://review.openstack.org/#/c/628128/ https://review.openstack.org/#/c/628956/ | |
| 15:26:36 | dpawlik | thanks | |
| 15:27:26 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Removing pip-missing-reqs from default tox jobs https://review.openstack.org/628956 | |
| 15:30:55 | jaypipes | sean-k-mooney: did you see Numan's comments on https://review.openstack.org/#/c/602384/ causing intermittent failures in tempest runs? | |
| 15:32:04 | sean-k-mooney | reading it now. | |
| 15:33:05 | sean-k-mooney | ok so the nova patch i abandoned to chagne the libvirt type to ethernet woudl adress there issue | |
| 15:33:21 | sean-k-mooney | i can un abandon it and see if that helps. | |
| 15:33:57 | sean-k-mooney | idelaly i dont want libvirt involed in configuring ovs | |
| 15:35:03 | sean-k-mooney | jaypipes: i also wonder if ovn support multiple port bidning. | |
| 15:37:39 | jaypipes | sean-k-mooney: since Numan is another RHatter, can you get in touch with him about this? | |
| 15:38:14 | mriedem | dpawlik: done | |
| 15:38:18 | sean-k-mooney | yep ill bring it up internally | |
| 15:39:18 | openstackgerrit | sean mooney proposed openstack/nova master: libvirt: delegate ovs plug to os-vif https://review.openstack.org/602432 | |
| 15:39:22 | mriedem | kashyap: so on https://review.openstack.org/#/c/629627/ i guess you forgot my comment about referring to the new doc from the kvm live migration doc and/or the 'security' doc? | |
| 15:39:31 | kashyap | mriedem: Hi | |
| 15:39:52 | kashyap | mriedem: I did comment in the abandoned follow-up change I guess | |
| 15:39:54 | kashyap | Let me see | |
| 15:40:07 | jaypipes | sean-k-mooney: danke | |
| 15:40:12 | kashyap | mriedem: (And thanks for your time!) | |
| 15:40:12 | mriedem | kashyap: you said you'd follow up, | |
| 15:40:14 | mriedem | and then abandoned the follow up | |
| 15:40:25 | mriedem | so now i guess you need to re-follow-up | |
| 15:40:52 | artom | mriedem, ah, so when we detach a tagged BDM, we remove its metadata using its serial/volume id | |
| 15:40:53 | kashyap | mriedem: Heh, so the "strategy" was: | |
| 15:41:07 | artom | mriedem, but when we build the metadata, we go by device name | |
| 15:41:27 | kashyap | mriedem: (1) Squash the first follow-up into the main doc as we needed a re-spin; (2) Do the re-follow-up with all the hyperlinking goodness. | |
| 15:41:32 | mriedem | artom: and if i'm looking at the libvirt driver code correctly, we only build the metadata on spawn, attach and resize | |
| 15:41:53 | mriedem | so my fear about screwing up the linkage during reboot with different device names is probably unfounded | |
| 15:42:05 | artom | mriedem, detach as well | |
| 15:42:09 | artom | Wait | |
| 15:42:24 | artom | No, you're right, only attach | |
| 15:42:43 | mriedem | i saw attach volume and then the two places where we build a config drive | |
| 15:42:43 | artom | But I think we rebuild the whole metadata when we attach any device | |
| 15:42:45 | mriedem | spawn and resize | |
| 15:43:11 | mriedem | yeah looks like it | |
| 15:43:17 | mriedem | re rebuild the whole metadata | |
| 15:45:09 | artom | mriedem, we're talking about https://review.openstack.org/#/c/616777/8/nova/compute/utils.py, right? | |
| 15:45:39 | artom | I'm still parsing it, but... does it only apply the new naming scheme to newly attached/booted-with bdms? | |
| 15:45:56 | mriedem | artom: yeah | |
| 15:46:09 | mriedem | yeah in that is the patch i'm talking about | |
| 15:46:24 | mriedem | i don't know about the latter, that's what i'm wondering | |
| 15:46:32 | artom | So then I don't think that's a problem? Because while the scheme changes, the device name will still be consistent between the libvirt XML and the bdm object in the db | |
| 15:46:37 | artom | Which is what we care about | |
| 15:47:07 | mriedem | for libvirt this is really the code that matters, not compute.utils https://review.openstack.org/#/c/616777/8/nova/virt/libvirt/blockinfo.py | |
| 15:47:14 | mriedem | since libvirt is special and has it's own device naming thing | |