| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-03-26 | |||
| 19:26:47 | sean-k-mooney | and make it nova-manage image_property show <vm_uuid> hw_vif_multiqueue_enabled | |
| 19:28:43 | sean-k-mooney | i guess the intent was for this to be namespaced under an image_property subparser https://review.opendev.org/c/openstack/nova/+/880557/2/nova/cmd/manage.py#3360 | |
| 19:28:45 | melwitt | sean-k-mooney: looking ... yeah, you're right, sorry 😑 | |
| 19:28:58 | melwitt | let me update my comment and vote | |
| 19:29:32 | sean-k-mooney | well my really question is can we create a repoducer test of some kind to validate the old/new behavior | |
| 19:29:57 | sean-k-mooney | i might be able to check this on my home cloud quickly | |
| 19:30:11 | melwitt | it would take some doing bc we don't currently test the CLI in that manner that exercises these decorators | |
| 19:31:04 | melwitt | I tried briefly a long time ago when I first reviewed this and didn't come up with a good way (and i don't remember what I tried now) | |
| 19:31:57 | sean-k-mooney | so i tought someone submited a docs fix related to this recently | |
| 19:32:39 | melwitt | sean-k-mooney: ok so this isn't changing the subcommand, just the name of the arg. so I don't think it would break anyone? | |
| 19:33:10 | sean-k-mooney | if its just the name of the arg in python then no | |
| 19:33:20 | sean-k-mooney | so really i need to try and test this locally to review it | |
| 19:33:27 | sean-k-mooney | which menas i need a devstack | |
| 19:33:52 | melwitt | I had meant to do dest=image_property and call the arg just "property" because I was trying to avoid nova-manage image_property image_property from an aesthetic perspective | |
| 19:34:45 | sean-k-mooney | ya | |
| 19:34:50 | sean-k-mooney | it should be a postional arg | |
| 19:35:02 | sean-k-mooney | to the show command after the vm uuid | |
| 19:35:16 | melwitt | sean-k-mooney: sorry, I didn't mean for this to be involved. I think you would only need to install nova-manage and not a whole devstack bc the bug happens before anything is done, it's a parsing problem essentially | |
| 19:35:50 | sean-k-mooney | well i have a local copy of nova i can check it out in a venv with tox and see i guess | |
| 19:36:37 | melwitt | it is a positional arg. it's just the name of the arg doesn't match the keyword arg that goes with it. and if it isn't going to match you have to use "dest=whatever" but I messed up and didn't do that | |
| 19:38:12 | sean-k-mooney | i tought etavar='<image_property>', was ment to be the name of the variable | |
| 19:38:22 | sean-k-mooney | but i have not looked at the argpars docs in a while | |
| 19:38:37 | melwitt | I might have spaced and thought so too but it isn't, that's just for what it shows in the usage or help | |
| 19:39:17 | melwitt | dest= is what names the variable and if you omit dest= it will use the name of the positional arg | |
| 19:39:31 | melwitt | and property != image_property | |
| 19:40:39 | melwitt | so it either needs to be @args(property, dest=image_property, ...) or @args(image_property, ...) I think | |
| 19:42:59 | sean-k-mooney | ok so i can confim without hte patch i get the error and with it it fixes it and i get an sql error | |
| 19:43:13 | sean-k-mooney | but that just becasue i dont have a nova.conf with any real data for it to use | |
| 19:43:26 | melwitt | right | |
| 19:48:22 | sean-k-mooney | ok its on its way | |
| 19:49:00 | melwitt | thanks, sorry for the confusion | |
| 19:49:46 | sean-k-mooney | its ok | |
| 19:50:03 | sean-k-mooney | so this is something we should proably add test coverage for in the functional tests at some point | |
| 19:50:20 | sean-k-mooney | i.e. invoke nova manage in such a way that we test hte paring logic | |
| 19:50:34 | sean-k-mooney | i think that is doable but its not reasonable to ask for that in this case | |
| 19:50:50 | sean-k-mooney | failing that we coudl test this in the tempest jobs at some point | |
| 19:50:56 | melwitt | yeah.. I couldn't come up with a way other than process exec or something like that, but I could just be missing something obvious | |
| 19:51:31 | sean-k-mooney | process exec woudl work in a tempest type jobs that is kind fo what the openstck client devstack job does | |
| 19:51:48 | sean-k-mooney | but i think we can import the nova manage parser and just give it a string | |
| 19:52:38 | melwitt | agree it's ok in tempest, but for func tests, I would have thought to avoid that. but I dunno | |
| 19:53:00 | sean-k-mooney | like this is the nova manage main function | |
| 19:53:24 | sean-k-mooney | we call config.parse_args(sys.argv) to pars the cli opt | |
| 19:53:39 | sean-k-mooney | we cab just pass an arry of stings to that | |
| 19:54:06 | sean-k-mooney | like we can mock sys.arg an then call main | |
| 19:54:11 | melwitt | ok, I guess I was just being dumb then 😛 what you're saying makes sense | |
| 19:54:12 | sean-k-mooney | form a functional test | |
| 19:54:39 | sean-k-mooney | the tricky part woudl be jsut makeing sure we have the dbfixture extra in place to make it work | |
| 19:55:22 | sean-k-mooney | but we have a lot of that code in the fucntional tests already so it should be doable | |
| 19:55:27 | sean-k-mooney | we would just need to find the time | |
| 19:56:33 | melwitt | yeah. I could give it a go since it was my bug that caused this whole mess | |
| 19:57:25 | sean-k-mooney | if you like :) i was going to finish soon so i was also going to see if i could hack somethign togehter quickly | |
| 20:02:38 | melwitt | sean-k-mooney: please go ahead :) I will only do it if you don't want to | |
| 21:11:39 | opendevreview | sean mooney proposed openstack/nova master: add scafolding for cmd functional tests https://review.opendev.org/c/openstack/nova/+/914408 | |
| 21:11:47 | sean-k-mooney | melwitt: ^ somethign like that | |
| 21:17:21 | melwitt | sean-k-mooney: that looks very nice, I'll try it out | |
| 21:26:53 | opendevreview | sean mooney proposed openstack/nova master: add repoducer for bug 2016346 https://review.opendev.org/c/openstack/nova/+/914410 | |
| 21:27:02 | sean-k-mooney | melwitt: so ^ is the repoducer for that bug | |
| 21:27:23 | sean-k-mooney | i should be enabling the db mocks here but im not calling the db | |
| 21:27:30 | sean-k-mooney | so its not need yet | |
| 21:27:41 | sean-k-mooney | the second it would call the db we need to include the db fixture | |
| 22:00:53 | melwitt | sean-k-mooney: nice, thanks | |
| #openstack-nova - 2024-03-27 | |||
| 05:46:47 | manuvakery1 | Hey .. I am getting ConflictNovaUsingAttachment while dettaching volume from VM. I do have the service_user enabled in nova.conf as per https://docs.openstack.org/cinder/latest/configuration/block-storage/service-token.html | |
| 07:30:38 | sahid | bauzas, sean-k-mooney o/ | |
| 07:30:51 | sahid | all good for https://review.opendev.org/c/openstack/nova/+/896512 ? | |
| 07:30:57 | sahid | any action that I have to take? | |
| 07:34:05 | bauzas | sahid : the action is for me, I need to modify the blueprint to be accepted and add the series in our etherpad :-) | |
| 07:50:19 | sahid | bauzas: perfect ! thanks | |
| 08:01:02 | sahid | guys, quick question, we have noticed that some VMs that are using volumes do not have the right number of iscsi path, normally they should all have 8 paths | |
| 08:01:18 | sahid | but we can notice that some have only 4 or 3 or 7 paths | |
| 08:33:31 | opendevreview | ribaudr proposed openstack/nova master: Test live migration between hosts with differnet cpu_shared_sets https://review.opendev.org/c/openstack/nova/+/913744 | |
| 08:33:31 | opendevreview | ribaudr proposed openstack/nova master: Fix: Live migrating to a host with cpu_shared_set configured will now update the VM's configuration accordingly. https://review.opendev.org/c/openstack/nova/+/903706 | |
| 09:59:42 | bauzas | sahid: I'm not a specialist but I think it depends on the machine type | |
| 10:00:05 | bauzas | nevermind, looked too quickly | |
| 16:08:41 | melwitt | dansmith: thanks for looking at the spec. I haven't gone through it yet in depth but wanted to mention it might be easier to read(at least the tables) to look at the docs preview https://793e4b83b6aee9c90173-a0541882d2023eca9a1cc07087449de0.ssl.cf2.rackcdn.com/907654/7/check/openstack-tox-docs/55aebd2/docs/specs/2024.2/approved/ephemeral-storage-encryption.html | |
| 16:09:09 | dansmith | melwitt: oh yeah, that was the *only* way I could make sense of it :D | |
| 16:09:30 | dansmith | I was remarking about choosing which line to complain about in the source :) | |
| 16:09:40 | melwitt | oh, gotcha | |
| 16:15:03 | melwitt | sean-k-mooney: just a heads up that I did go through and test the systemd way of enabling gpu virtual functions and it worked for me https://review.opendev.org/c/openstack/nova/+/910041 | |
| 16:25:56 | bauzas | melwitt: I can shortly quick-approve back the persistent mdevs bp | |
| 16:26:03 | bauzas | so I could look at your patch | |
| 16:26:27 | melwitt | bauzas: ok, that would be great, thanks | |
| 16:26:28 | bauzas | we agreed last cycle it was a specless bp, so I feel brave enough to approve it again without asking other folks | |
| 16:27:30 | melwitt | specless bp ftw | |
| 16:27:47 | bauzas | done | |
| 16:31:44 | melwitt | thanks | |
| 16:31:55 | bauzas | melwitt: can you confirm that besides persisting the sriov-manage VFs, all the mdevs were persisted after reboot without modifying some systemd files or udev rules ? | |
| 16:32:49 | bauzas | melwitt: I have concern with documenting some specific nvidia command in our upstream docs, so I'll just leave a -1 about that tomorrow, but I just wanted to make sure it works | |
| 16:35:12 | melwitt | bauzas: yes it worked for me. to be clear, I could boot a vm with vgpu, reboot the host, sriov-manage -e (via systemd service file), then 'server start' the vm and it retained the mdev in its xml and started up fine | |
| 16:35:20 | bauzas | \o/ | |
| 16:40:52 | bauzas | melwitt: -1d with my comment https://review.opendev.org/c/openstack/nova/+/910041 | |
| 16:41:55 | melwitt | bauzas: ok thanks | |
| 16:44:07 | bauzas | melwitt: if you read the nvidia doc link I gave in the review, you'll see this paragraph : | |
| 16:44:09 | bauzas | " Note: If you are using a GPU that supports SR-IOV, the mdev device file persists after a host reboot only if you perform Step 1 before rebooting any VM that is configured with a vGPU on the | |
| 16:44:11 | bauzas | GPU. You cannot use the mdevctl command to make the mdev device file for a MIG-backed vGPU persistent. The mdev device file for a MIG-backed vGPU is not retained after the host is rebooted because MIG | |
| 16:44:13 | bauzas | instances are no longer available. " | |
| 16:44:32 | bauzas | tl;dr: this works for time-sliced vGPUs but if you enable MIG types, you're screwed | |
| 16:44:50 | dansmith | wow | |
| 16:45:17 | bauzas | because all of this is fuckingly on the userspacez | |
| 16:45:21 | bauzas | thanks nvidia | |
| 16:48:07 | melwitt | bauzas: oh, I see. I don't know what MIG types means but I'll look it up :) | |