| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-01-16 | |||
| 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 Cyborg device profile groups to spec obj. https://review.openstack.org/631243 | |
| 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: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:07:31 | openstackgerrit | Sundar Nadathur proposed openstack/nova master: WIP: Create and bind Cyborg ARQs. https://review.openstack.org/631244 | |
| 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 | mriedem | kashyap: you said you'd follow up, | |
| 15:40:12 | kashyap | mriedem: (And thanks for your time!) | |
| 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 | artom | But I think we rebuild the whole metadata when we attach any device | |
| 15:42:43 | mriedem | i saw attach volume and then the two places where we build a config drive | |
| 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 | |
| 15:48:47 | artom | mriedem, as long as that bit happen before the XML is generated, it shouldn't be a problem | |
| 15:49:31 | mriedem | once the bdm.device_name is originally set, i also don't think it will change. _prep_block_device in the compute manager calls _add_missing_dev_names but that noops if a bdm already has a device_name set | |
| 15:50:06 | artom | mriedem, well, if we want to really be sure, could we add some tagged devices to rally tests? | |
| 15:50:42 | artom | Or grenade? What's the thing that does upgrades in the gate? | |
| 15:51:36 | dansmith | heh | |
| 15:51:39 | dansmith | grenade | |
| 15:51:42 | dansmith | oopsie | |
| 15:51:59 | dansmith | guess I never dropped my cape | |
| 15:53:29 | mriedem | artom: grenade does attach a volume on the old side before the upgrade, but it doesn't tag it - and even if it did, to verify the guest can read the tag properly on the new side (via tempest) would require running a test that does that ssh into the guest, which wouldn't be a "smoke" test and those are the only kinds of tests that grenade jobs run | |
| 15:53:31 | mriedem | plus, | |
| 15:53:46 | mriedem | the chances of the device name changing when you only have one volume attached is pretty slim | |
| 15:53:55 | mriedem | since i think it'd just be vda or vdb in that case | |
| 15:54:30 | mriedem | anyway, if the root answer is "the device naming schema is unchanged, but fixed to allow more than 26 names" then i tihnk we're ok | |
| 15:54:36 | mriedem | i.e. consistent naming schema, but unbounded | |
| 15:54:56 | mriedem | i think that's what the commit message in https://review.openstack.org/#/c/573066/ is saying | |
| 15:55:01 | dansmith | mriedem: just ensure the first 26 are the same pattern after the change, right? | |
| 15:55:12 | dansmith | only the 27th should be different and then you're probably fine yeah? | |
| 15:55:30 | artom | mriedem, I thought the question was "does the new schema retroactively get applied to existing devices?" | |
| 15:56:10 | mriedem | i think the real question is, why am i worrying about this and wasting my time on it | |
| 15:56:40 | artom | And actually not even that - "does the new schema retroactively get applied to *only* the database or *only* the XML?" | |
| 15:57:16 | artom | mriedem, when experiencing bouts of existential nihilism I usually turn to alcohol | |
| 15:57:30 | mriedem | if anything it would be only the xml | |
| 15:57:37 | mriedem | pretty sure we don't change bdm device_name once set | |
| 15:57:48 | mriedem | b/c that hoses up some db constraints and stuff we passed to cinder during the attach | |
| 15:58:07 | artom | Err, then we actually might have a problem :( | |
| 15:58:45 | mriedem | like dan said, if the old pattern and new pattern are the same for the first 26 then we're fine | |
| 15:59:33 | artom | Yeah, screw those people with more than 26 disks, what were they thinking anyways | |
| 15:59:54 | sean-k-mooney | artom: you could not have more then 26 disks before | |
| 16:00:09 | artom | sean-k-mooney, ohhhh, hah! | |
| 16:00:12 | artom | Yeah, we're fine then | |
| 16:00:56 | artom | And maybe mriedem's right, fix that bug if/when it comes up | |
| 16:01:09 | artom | Seems like a pretty edgy case | |