| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-06-19 | |||
| 15:54:59 | artom | OK, I guess it's just me then, and I had it wrong? | |
| 15:55:07 | stephenfin | bad artom | |
| 15:55:24 | artom | *sigh* yeah | |
| 15:55:53 | dansmith | select()'s FDSET is an inclusion mask yeah? | |
| 15:56:40 | stephenfin | oh, err, I think so? | |
| 15:57:14 | dansmith | it is, the question mark was rhetorical :) | |
| 15:57:24 | stephenfin | oh | |
| 15:57:33 | artom | dansmith, I'm trying to understand what you mean by reading `man 2 select` | |
| 15:57:55 | stephenfin | we'll come fish you out of that rabbit hole later | |
| 15:57:58 | artom | There's FD_SET(), but that's a macro | |
| 15:58:26 | dansmith | artom: the readfds, for example is a pointer to a word, with bits set nonzero if the FD represented by that bit should be checked for readability | |
| 15:58:31 | artom | (Also, `man 2 select` is the perfect title for a geek version of The Bachelorette) | |
| 16:02:18 | artom | dansmith, there's nothing about masks in the readfds para... | |
| 16:02:27 | artom | It's a set of file descriptors | |
| 16:02:49 | dansmith | artom: no, it's not, you're just not meant to know how it works under the covers | |
| 16:03:27 | dansmith | FD_SET is just readfds|=(1 << fd) | |
| 17:05:57 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Guard against missing image cache directory https://review.opendev.org/736964 | |
| 17:07:44 | stephenfin | dansmith: ^ | |
| 17:09:25 | dansmith | stephenfin: I think you got that logic wrong | |
| 17:09:32 | stephenfin | crap | |
| 17:09:48 | dansmith | if the directory does not exist, then True, else if it does, True if st_dev matches | |
| 17:11:12 | stephenfin | At first glance it reads correctly to me. Bear with me | |
| 17:11:35 | dansmith | it's okay it's confusing, so make sure I'm right here | |
| 17:12:15 | dansmith | if the directory does exist, then we need to check to see if the st_dev matches.. if the directory does not exist, then we return True because when we create it, it will be on the same device | |
| 17:12:24 | stephenfin | right, yeah, makes sense | |
| 17:12:34 | stephenfin | yay, Fridays | |
| 17:12:39 | stephenfin | I'll respin again | |
| 17:18:53 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Guard against missing image cache directory https://review.opendev.org/736964 | |
| 17:23:14 | dansmith | stephenfin: got it | |
| 17:24:07 | stephenfin | I've done enough on that so I'll defer to melwitt or someone to approve | |
| 17:24:28 | melwitt | yeah I'll review it | |
| 17:25:57 | dansmith | ack, I think that's a good idea | |
| 17:47:09 | melwitt | dansmith: added some comments, am I missing something? because to me it seems this is doing the opposite of what is wanted in the commit message https://review.opendev.org/736964 | |
| 17:48:29 | melwitt | I see that you've said the first time an image is cached, the dir will be created, but update_provider_tree is a periodic task right? what will ensure the cache dir is created by the time we ask for get_disk_usage()? | |
| 17:49:18 | dansmith | nothing will ensure that it's created | |
| 17:49:32 | dansmith | the bug is that if it's not created and this runs, it doesn't handle the case where it's not created | |
| 17:49:56 | dansmith | however, we don't just want to do nothing if it's not created, we want to take the path that assumes that when it does get created, it will necessarily be on the same st_dev, and thus not report any cache | |
| 17:50:35 | melwitt | yeah, I got that part but from what I was looking, if the cache dir doesn't exist it will fall through and start checking usage of the cache dir and if it doesn't exist won't that be bad? | |
| 17:51:09 | dansmith | where will it fall through? | |
| 17:51:14 | melwitt | cause we'll end up here if the dir doesn't exist https://github.com/openstack/nova/blob/f5f7c2540150c7ee7640c834d5caec31b3f5a7ab/nova/virt/libvirt/imagecache.py#L366 | |
| 17:52:11 | dansmith | er wait, maybe I'm inverting that in my head | |
| 17:52:47 | melwitt | it will return 0 disk usage if self.cache_dir_is_on_same_dev_as_instances_dir is False, right? | |
| 17:52:49 | dansmith | okay right right, I forgot how this ended up | |
| 17:52:58 | dansmith | we initially talked about reporting inventory for the cache disk itself, | |
| 17:53:03 | melwitt | else it will do the math on the cache dir | |
| 17:53:28 | dansmith | and this is the opposite where we reserve only if the cache is the _same_ disk, which I think is why we endded up checking the reverse condition instead of what we're looking for | |
| 17:54:05 | dansmith | so yeah, we need to bail if the directory doesn't exist because we're checking the cache_dir which might not exist yet | |
| 17:54:25 | dansmith | I still think the property shouldnt' explode if the directory doesn't exist, | |
| 17:54:44 | dansmith | but you're right, it'll still hit the same problem but a little later on | |
| 17:55:19 | dansmith | the logic is right, it just doesn't prevent us from hitting the same problem in the sum() | |
| 17:55:45 | melwitt | oh, ok, so the logic on exists() should not be inverted to remove the 'not' | |
| 17:56:44 | dansmith | no, because if it does not exist, the not will make that true, which will short-circuit and return true without evaluating the st_dev part | |
| 17:57:03 | melwitt | oh, right | |
| 17:58:29 | dansmith | I'll pull this down and make that change, just a sec | |
| 17:59:20 | melwitt | ok, cool | |
| 17:59:26 | dansmith | eff, just realized I didn't commit something I did earlier today unrelated | |
| 17:59:40 | openstackgerrit | Dan Smith proposed openstack/nova master: DNM: Try to make a glance multistore job https://review.opendev.org/734184 | |
| 17:59:53 | dansmith | so another few days in the queue for me | |
| 18:00:33 | melwitt | heh :( | |
| 18:10:56 | openstackgerrit | Dan Smith proposed openstack/nova master: Guard against missing image cache directory https://review.opendev.org/736964 | |
| 18:10:58 | dansmith | melwitt: ^ | |
| 18:12:45 | dansmith | added comments to hopefully make it clearer why each thing is happening | |
| 18:14:14 | melwitt | looking ... ah yep, that is helpful | |
| 18:19:48 | dansmith | thanks. I was clearly focused on getting that property to return the right thing all the time and not realizing we'd be introducing a new vector to the error :) | |
| 20:43:10 | mnaser | just wanna put eyes on https://review.opendev.org/#/c/736370/ if/when possible :) | |
| 21:04:42 | sean-k-mooney | mnaser: form the title that makes sense | |
| 21:05:05 | sean-k-mooney | ill take a look on monday | |
| 21:05:21 | sean-k-mooney | actully its pretty short | |
| 21:07:53 | mnaser | sean-k-mooney: thank you! | |
| 21:18:08 | sean-k-mooney | melwitt: just finished reviewing https://review.opendev.org/#/c/736370/2 want to take a look? | |
| 21:18:39 | sean-k-mooney | mnaser: the only thing im not sure is do we want to have a release note or not. | |
| 21:19:43 | sean-k-mooney | mnaser: this would have failed anyway later so the only behavior change is that by failing in _validate_flavor_image_nostatus we now fail before creating a server record | |
| 21:20:08 | sean-k-mooney | so instead of ending up with a server in error state we just get an excption form spawn | |
| 21:21:37 | sean-k-mooney | which is better but as an operator would you want that called out in the release notes give the excption you are raising inherits form FlavorDiskTooSmall which can already casue the same behvoiur if the volume is less the min_gb | |
| 21:21:42 | sean-k-mooney | mnaser: ^ | |
| 21:31:22 | arjunbaindur | hey, have a question about configuring PCI passthrough. We have a device that has 2 functions. On the hypervisor, the PCI address has it on same bus and same slot. When it shows up inside the VM, the PCI address has it under different slots | |
| 21:31:31 | arjunbaindur | for example, suppose device has two functions at 83:00.0 and 83:00.1 | |
| 21:31:39 | arjunbaindur | Under the VM, the devices show up under different slots, for example 0:5.0 and 0:6.0 | |
| 21:32:34 | sean-k-mooney | im guessing its a gpu with a second function for the audio encoder on the gpu? | |
| 21:32:38 | arjunbaindur | any idea how to get them to show up as two functions under the same slot | |
| 21:32:39 | sean-k-mooney | or a similar device | |
| 21:32:56 | sean-k-mooney | unfortunetly we do not have a way to do that currently | |
| 21:33:12 | sean-k-mooney | we did dicuss it breifly about 3-6 months ago | |
| 21:33:46 | sean-k-mooney | efffectivly we would need to modify the pci alisia to support multifuntion devices | |
| 21:34:46 | sean-k-mooney | such as allowing one pci aliase to refrence another and adding a multifuntion=true tag | |
| 21:35:17 | sean-k-mooney | so the parent alias would refence the child aliase and but or would have the multifunction=ture tag | |
| 21:36:13 | sean-k-mooney | arjunbaindur: i think on windows this can break the audio encoder on nvidga gpus since the driver is expecting it to be a second fuction on the same bus/slot | |
| 21:36:18 | sean-k-mooney | but on linux it works | |
| 21:36:34 | sean-k-mooney | im sure it would be an issue for other device that had the same toplogy | |
| 21:39:01 | arjunbaindur | to be specific, this is a Xilinx card | |
| 21:39:12 | arjunbaindur | https://www.xilinx.com/support/documentation/boards_and_kits/accelerator-cards/1_5/ug1301-getting-started-guide-alveo-accelerator-cards.pdf | |
| 21:39:57 | arjunbaindur | since the two devices show up under diff PCI slots, we arent able to install drivers | |
| 21:40:35 | sean-k-mooney | ah ok well its the same issue unfortunetly | |
| 21:40:48 | sean-k-mooney | are you trying to reproguam the aclerat using 1 fucntion | |
| 21:40:56 | sean-k-mooney | and consume it using the second | |
| 21:42:54 | sean-k-mooney | 32-64GB of ddr4 + and fpga and some qsfp ports. that looks like a pretty interesting card | |
| 21:43:14 | sean-k-mooney | arjunbaindur: are you trying to use this via cyborg or just novas pci passhtough | |
| 21:44:08 | arjunbaindur | just Nova PCI passthrough: followed this doc basically: https://docs.openstack.org/nova/rocky/admin/pci-passthrough.html | |
| 21:44:41 | arjunbaindur | Added the two PCI addresses for each function in the whitelist | |
| 21:44:42 | sean-k-mooney | ya so as i said we are missing the ablity to group multiple fucntion together form teh same device that are schulded togehter | |