| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-10-18 | |||
| 13:58:43 | sean-k-mooney | speaking of which i think its fair to say htat after the ptg wheree we should discuss some of this we should likely capture the plan in a spec | |
| 13:59:54 | sean-k-mooney | but for sysfs reading in genal plwad use https://github.com/openstack/nova/blob/master/nova/filesystem.py#L28 if you do implement this | |
| 14:00:05 | bauzas | sean-k-mooney: yup, I'll | |
| 14:01:02 | sean-k-mooney | besided the fact this should have a spec, the main reason i want one is to defien the scoep and what will be doen in which order and wthat will eb defered to a future release | |
| 14:01:14 | sean-k-mooney | partly so we dont forget soemthing | |
| 14:02:36 | bauzas | don't disagree | |
| 14:11:06 | dansmith | sean-k-mooney: bauzas I didn't save a link to the bug filed yesterday about the upgrade check, but was there any discussion there about revert vs. change to warning? | |
| 14:11:39 | bauzas | dansmith: I very briefly looked at your convo this morning, can you explain it again please ? | |
| 14:11:52 | bauzas | iirc, it was because of non-slurp | |
| 14:11:56 | dansmith | no | |
| 14:12:23 | dansmith | bauzas: basically sean-k-mooney strong-armed me into adding a status check for comptue-object-ids in this last cycle, which raised an error if you hadn't completed the upgrade (which you wouldn't have yet anyway) | |
| 14:12:59 | dansmith | I begged for mercy but there was no mercy given so I added the check and then an op showed up yesterday wondering why I had ruined his day | |
| 14:13:06 | dansmith | (I may be exaggerating here) | |
| 14:13:24 | dansmith | anyway, the status check really should be *next* cycle saying "you haven't finished your homework from last time" | |
| 14:13:52 | dansmith | so we have two options: 1. just revert out the check entirely from 2023.2 or 2. convert it to a warning result | |
| 14:14:20 | sean-k-mooney | fyi https://paste.opendev.org/show/bGuYFgfWt70oGOHcAuH9/ | |
| 14:14:28 | sean-k-mooney | it looks like its working for me too | |
| 14:14:37 | sean-k-mooney | but i have not tried to boot any vms with those mdevs yet | |
| 14:15:37 | sean-k-mooney | i have allcoated 4 mdevs in this case | |
| 14:15:38 | bauzas | sean-k-mooney: I went down to create and even sshing to the cirros guest, the serial was present | |
| 14:15:46 | bauzas | dansmith: recollecting | |
| 14:16:21 | sean-k-mooney | bauzas: ack ill do that quickly too just to confirm but i htink this is usabel for startting to do ci | |
| 14:16:36 | dansmith | \o/ | |
| 14:16:50 | sean-k-mooney | we have some basic mdev/gpu testsin whitebox | |
| 14:17:04 | sean-k-mooney | so we coudl try extending the job to depend on your patch and enable those | |
| 14:17:20 | dansmith | sean-k-mooney: what happens if you try to migrate an instance with a non-migratable mdev? Maybe that's something we can test in normal multinode tempest in the short term.. that we don't break | |
| 14:17:36 | sean-k-mooney | unfortunetlly the tests as written wont work | |
| 14:17:44 | sean-k-mooney | since they depend on pci | |
| 14:18:17 | dansmith | oh we have to do this in whitebox because we need to know mdev details on the host I guess? | |
| 14:18:25 | sean-k-mooney | dansmith: good question i currently only have singel node devstack deployed so cant check but sylvain should be able to just try that | |
| 14:18:38 | sean-k-mooney | dansmith: no we can do this is tempest too | |
| 14:18:44 | dansmith | okay | |
| 14:18:49 | dansmith | seems like a good negative test to add | |
| 14:18:57 | sean-k-mooney | we just would not eb able ot look at the xml but we can just check for the device in the guest via ssh | |
| 14:19:00 | dansmith | to make sure we don't sh*t the bed if the migration fail for that reason | |
| 14:19:29 | sean-k-mooney | i would expect libvirt to fail the migration wehn we call migrate_to_uri3 or whatever the libvirt function is | |
| 14:19:43 | sean-k-mooney | same as if the cpu compatiable check in libvirt fails | |
| 14:19:49 | dansmith | yep, just seems like a good check if it's easy | |
| 14:19:50 | sean-k-mooney | or if you have hostdev devices | |
| 14:19:55 | bauzas | dansmith: good question, I know that we test some move ops | |
| 14:20:20 | sean-k-mooney | so you think the best approch woudl be to start creating new geneic mdev tests in tempest right | |
| 14:20:32 | bauzas | and yeah I have a 2-node env with one mtty, can try to migrate my test instance | |
| 14:20:42 | sean-k-mooney | cool | |
| 14:20:56 | dansmith | sean-k-mooney: depends on what they look like really, but.. yeah why not? | |
| 14:20:59 | sean-k-mooney | cold migrate should work live migrate will likely fail or we/libvirt have a bug | |
| 14:21:01 | bauzas | back to dansmith's point about status checks | |
| 14:21:06 | dansmith | this is clearly a gap | |
| 14:21:37 | sean-k-mooney | dansmith: no reason not to just wanted to confirm that is your prefernce | |
| 14:22:04 | bauzas | dansmith: so, about status checks, I'd say revert it | |
| 14:22:05 | sean-k-mooney | so we will need a new compute feature config option in tempest for it and perhasp a cofnig option to identify how to detect the device in the vm | |
| 14:22:16 | dansmith | sean-k-mooney: yeah | |
| 14:22:23 | bauzas | no real gain in having a warning imho | |
| 14:22:31 | dansmith | bauzas: okay that was sean-k-mooney's opinion too | |
| 14:22:46 | dansmith | bauzas: so revert in master, backport that, and then maybe add it back later when we start depending on it? | |
| 14:22:54 | bauzas | wfm | |
| 14:23:06 | dansmith | aight | |
| 14:23:23 | sean-k-mooney | ya so i think that would make sense | |
| 14:23:32 | sean-k-mooney | fell like pushing the revert patch | |
| 14:23:50 | sean-k-mooney | and we can review it quickly since we are talking about it | |
| 14:24:42 | dansmith | I'm doing it | |
| 14:24:45 | dansmith | I need to find that bug tho | |
| 14:25:50 | opendevreview | Dan Smith proposed openstack/nova master: Revert "Add upgrade check for compute-object-ids linkage" https://review.opendev.org/c/openstack/nova/+/898741 | |
| 14:28:57 | bauzas | hmmm, looking more at the mdev parent being 'computer' in the device XML, there may be some confusions when creating inventories if we were about asking two different virtual devices, like say mtty and mdpy | |
| 14:29:14 | bauzas | that's a potential bug, so I'm pdb'ing it on my devstack | |
| 14:29:49 | bauzas | another slight difference from libvirt that may need to change on their side | |
| 14:51:54 | sean-k-mooney | well we can have mulitpel inventories of mdevs for diffent types | |
| 14:52:31 | sean-k-mooney | so it will be a slightly didfdfent toplogy i.e. 1 rp for all virtual devs rahter then one per pareent devce | |
| 14:52:34 | sean-k-mooney | but i think that is fine | |
| 14:52:55 | bauzas | no, because we don't support heterogenous VGPUs (yet) | |
| 14:53:13 | bauzas | a single device can only support one type | |
| 14:53:27 | sean-k-mooney | i didnt think that was enforecd there i tought that was done as part of the config | |
| 14:53:29 | bauzas | that's how we account those | |
| 14:53:30 | sean-k-mooney | parsing | |
| 14:53:44 | bauzas | there is a config parser that checks that yes | |
| 14:53:54 | bauzas | but our internal dicts rely on that too | |
| 14:53:59 | sean-k-mooney | ok well that another reason to use the device_adress | |
| 14:54:03 | sean-k-mooney | for virtrual stuff | |
| 14:54:14 | bauzas | yeah, I filed a bug against libvirt fwiw | |
| 14:54:24 | bauzas | the vfio guys are aware of the bug | |
| 14:54:30 | bauzas | and we'll just adapt | |
| 14:55:20 | bauzas | it occurs to me that OpenStack is probably the only high-level project that's about to integrate mtty for testing ;:) | |
| 14:55:35 | bauzas | mtty was probably just a toy project | |
| 14:55:54 | bauzas | but if that helps others's testing, that's a net win | |
| 15:43:13 | bauzas | netcalol | |
| 16:13:26 | bauzas | sean-k-mooney: fwiw, read_sys only reads files :) https://github.com/openstack/nova/blob/master/nova/filesystem.py#L28 | |
| 16:13:42 | bauzas | sean-k-mooney: and /sys/devices/virtual/mtty is a dir :) | |
| 16:14:27 | bauzas | I can provide some helper method that would wrap os.path.isdir() but would you nack if I was just calling os.path.isdir() straight in the driver ? | |
| 16:14:39 | bauzas | or .exists() | |
| 16:21:36 | colby__ | noonedeadpunk: Thanks Ill check on that. Did not realize the tunnel was dropped. Was this a change on xena to yoga? Everything worked fine on xena. | |
| 16:23:05 | sean-k-mooney | bauzas: good point on it being a dir i guess you can just add a new helper ya in the same module | |
| 16:23:29 | sean-k-mooney | or just call os.path.isdir() but it would be ok ot put in the filesystem module | |
| 16:23:45 | sean-k-mooney | an exist check is enough | |
| 16:23:53 | bauzas | sean-k-mooney: well, if the helper method only wraps isdir, that sounds to me a bit unnecessary | |
| 16:24:10 | sean-k-mooney | so i would go with exist because it might actully be a simlink | |
| 16:24:13 | colby__ | I have another question. It appears that the amount of local disk available changed in yoga. It appears to be only considering the actual local disk now (without ceph). But instances that use a ceph backed volume to boot are still counting as local disk usage. Was this intentional? Its causing no resources available issues for me. Im working on changing the disk allocation ratio to get around it. | |
| 16:24:27 | bauzas | yeah, agreed on exists(), semantically better | |
| 16:24:41 | bauzas | but if I'm just returning the boolean, meh | |
| 16:25:00 | bauzas | I can pretend to return a FileNotFound exception if exists says no | |
| 16:25:12 | bauzas | at least that would justify the helper | |
| 16:25:46 | sean-k-mooney | colby__: boot form volume instance shoudl use flavor with disk_gb = 0 | |