| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-01-16 | |||
| 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 | |
| 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 | |