| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-07-27 | |||
| 12:40:46 | brinzhang | sean-k-mooney: in spec we want to ask ensure which pmem we can copy in the target host when run migtate_disk_and_power_off, but this time we cannt know the target pmem's path in the context | |
| 12:41:04 | sean-k-mooney | yes we can | |
| 12:41:12 | brinzhang | https://review.opendev.org/c/openstack/nova-specs/+/785563/14/specs/xena/approved/allow-migrate-pmem-data.rst#65 | |
| 12:41:41 | sean-k-mooney | its includeded in the instance claim if i recall correctly | |
| 12:41:44 | brinzhang | we test and write the logs, and we cannt get the target PMEM's path from the context | |
| 12:43:31 | sean-k-mooney | ill have to review but i was pretty sure i pointed to the code where you could get it at one point | |
| 12:43:43 | brinzhang | not exectly, it's cannot claim it now, now we just get the pmem's path when we were started if I wasn's missed some key info | |
| 12:43:53 | sean-k-mooney | no | |
| 12:44:03 | sean-k-mooney | it must claim it before you do the data copy | |
| 12:44:25 | sean-k-mooney | you cannot pass back paths to unclaimed devices | |
| 12:44:33 | sean-k-mooney | that is a potential securrity bug | |
| 12:46:19 | brinzhang | yes, hope we were wrong, you can review the PoC code and give some points, thanks | |
| 12:46:27 | sean-k-mooney | prep_resize should have claimed the remote pmemdevices and store them in the migration context | |
| 12:46:53 | sean-k-mooney | brinzhang: i can but since it deviates form the spec im -1 untill you can show why the spec wont work | |
| 12:47:18 | sean-k-mooney | why did you not save the pmem device in the migration context in prep_resize | |
| 12:48:29 | sean-k-mooney | we create a resize claim here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L5153-L5156 | |
| 12:52:38 | brinzhang | yeah, it looks like he jumped the prep_resize, so he cannot get the target pmem's info | |
| 12:53:13 | sean-k-mooney | right now the claims dont claim the pci devices | |
| 12:53:19 | sean-k-mooney | * pmem devcies | |
| 12:53:36 | sean-k-mooney | but the intent of the spec was to extend it to do that | |
| 12:53:53 | sean-k-mooney | which can be sotre either in the claim direclty or we can update the migration or migration context | |
| 12:54:06 | sean-k-mooney | we hvae access to both | |
| 12:56:13 | brinzhang | as I know, the VM only knows the path information of the PMEM after it is started, otherwise the VM does not know what the PMEM patch used inside the virtual machine is (e.g. /dev/pmem0) | |
| 12:56:48 | brinzhang | may this is my confusing | |
| 12:56:49 | sean-k-mooney | i dont belive that is correct | |
| 12:57:13 | sean-k-mooney | we must know the path before we start the vm since we need to specify the path in the vm xml | |
| 12:57:22 | brinzhang | right | |
| 12:58:05 | sean-k-mooney | and we cannot copy any data untile we have claimed the devcie because other wise we coudl race with a differnte vm and that could be a security bug or at least a data loss bug | |
| 12:59:55 | brinzhang | yes, if it's right we will lost the datas in pmem | |
| 13:00:24 | brinzhang | I will check the prep_resize tomorrow | |
| 13:01:51 | brinzhang | may it need to claim the pci devices in resource tracker | |
| 13:02:33 | sean-k-mooney | im currenlty trying to figure out why it does not appear to be doign that although i may have missed it | |
| 13:02:40 | sean-k-mooney | i tought it was doing that already | |
| 13:03:09 | sean-k-mooney | but yes when we aquire the instnace clam or move claim it should for cold migration at least | |
| 13:03:27 | sean-k-mooney | for live migration we explitly claim the neutron pci devices in the pci tracker | |
| 13:03:39 | sean-k-mooney | we do not use move claims for that | |
| 13:04:06 | sean-k-mooney | for pmems though you will need to cliam them in the resouce table in the db | |
| 13:05:28 | brinzhang | yes, we shuold claims it firstly before cold migrate the instance | |
| 13:07:54 | sean-k-mooney | yep because we want to abort the migration if we cant claim | |
| 13:08:34 | sean-k-mooney | this is why we insited the spec be written the way it currently is | |
| 13:12:52 | sean-k-mooney | brinzhang: the poc wont work by the way | |
| 13:13:10 | sean-k-mooney | brinzhang: ill push comments in a second but its using processutiles incorrectly | |
| 13:13:45 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/802225/1/nova/virt/libvirt/driver.py#10757 | |
| 13:13:50 | brinzhang | it's ok, I agree your point, and I think firstly we should claim the pmem devices in prep_resize interface, and then check it when we want to execute migrage_server then to copy date form source pmem to the target pmem devices | |
| 13:14:31 | sean-k-mooney | well if the claim fails then the migration shoudl go to error and the vm will stay in active | |
| 13:15:08 | opendevreview | Pavlo Shchelokovskyy proposed openstack/nova stable/queens: libvirt: Skip encryption metadata lookups if secret already exists on host https://review.opendev.org/c/openstack/nova/+/765774 | |
| 13:16:31 | brinzhang | sean-k-mooney:ok, hope you can review if you find the error in that patch, we will update and try to update in next patch | |
| 13:19:41 | sean-k-mooney | the issue is the 3 commands in _migrate_vpmem_data will all execute on the same host. to fix it you need to pipe the output of the first commadn into the input of the second and the second and third commands should be combined | |
| 13:21:07 | brinzhang | agree, I saw your comment, it's useful to improve efficiency | |
| 13:28:02 | sean-k-mooney | well it wont work the way it is now | |
| 13:28:58 | sean-k-mooney | it might work on the same host but both daxio command woudl execute on the local host | |
| 13:32:13 | brinzhang | ack, it should be update, in local we have tested the CLI, it works fine | |
| 13:33:09 | sean-k-mooney | if you use those comand on the cli it would wokr but only because the second comamdn will put you in a new shell on the dest | |
| 13:33:34 | sean-k-mooney | to emulate this on the cli you need to rune each exec in a new terminal | |
| 13:33:58 | brinzhang | ack | |
| 15:12:22 | bauzas | reminder : nova meeting in 47 mins-ish here in #openstack-nova | |
| 15:17:03 | opendevreview | Merged openstack/nova stable/ussuri: Reject open redirection in the console proxy https://review.opendev.org/c/openstack/nova/+/791806 | |
| 15:29:55 | opendevreview | Stephen Finucane proposed openstack/nova master: Cleanup 'drop_move_claim' and '_drop_move_claim' https://review.opendev.org/c/openstack/nova/+/747747 | |
| 15:29:55 | opendevreview | Stephen Finucane proposed openstack/nova master: manager: Address TODO https://review.opendev.org/c/openstack/nova/+/747748 | |
| 15:29:56 | opendevreview | Stephen Finucane proposed openstack/nova master: manager: Move context manager up one level https://review.opendev.org/c/openstack/nova/+/745674 | |
| 15:29:56 | opendevreview | Stephen Finucane proposed openstack/nova master: compute: Add type hints for resize functions https://review.opendev.org/c/openstack/nova/+/745341 | |
| 15:29:57 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: compute: Add more type hints for resize functions https://review.opendev.org/c/openstack/nova/+/745675 | |
| 15:48:51 | opendevreview | Stephen Finucane proposed openstack/nova master: objects: Remove 'NovaObjectDictCompat' from 'Migration' https://review.opendev.org/c/openstack/nova/+/723572 | |
| 15:48:52 | opendevreview | Stephen Finucane proposed openstack/nova master: objects: Remove 'NovaObjectDictCompat' from 'InstancePCIRequest' https://review.opendev.org/c/openstack/nova/+/723573 | |
| 15:50:15 | opendevreview | Stephen Finucane proposed openstack/nova master: Remove use of pkg_resources https://review.opendev.org/c/openstack/nova/+/740661 | |
| 15:50:25 | bauzas | last reminder : nova meeting in 10 mins here in this chan. Sharpen your pencils. | |
| 15:56:14 | opendevreview | Stephen Finucane proposed openstack/nova master: api: Improve extra spec validator help texts https://review.opendev.org/c/openstack/nova/+/782412 | |
| 15:57:51 | stephenfin | lyarwood: Can you look at https://review.opendev.org/c/openstack/nova/+/773640/ and https://review.opendev.org/c/openstack/nova/+/797513/ seeing as you're +2 on patches later in the series | |
| 15:58:42 | stephenfin | also addressed your comments on https://review.opendev.org/c/openstack/nova/+/782412 | |
| 16:00:05 | bauzas | #startmeeting nova | |
| 16:00:05 | opendevmeet | Meeting started Tue Jul 27 16:00:05 2021 UTC and is due to finish in 60 minutes. The chair is bauzas. Information about MeetBot at http://wiki.debian.org/MeetBot. | |
| 16:00:05 | opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | |
| 16:00:05 | opendevmeet | The meeting name has been set to 'nova' | |
| 16:00:31 | stephenfin | o/ | |
| 16:00:39 | bauzas | howdy folks I'll be your chair for this meeting given our Supreme Leader is on vacations | |
| 16:00:50 | bauzas | \o | |
| 16:00:53 | sean-k-mooney | o/ | |
| 16:01:21 | bauzas | awesome, one more people from the last meeting I chaired \o/ | |
| 16:01:46 | bauzas | agenda is up at https://wiki.openstack.org/wiki/Meetings/Nova | |
| 16:01:58 | elodilles | o/ | |
| 16:02:27 | bauzas | feel free to add items you wanna discuss in the last section above ^ | |
| 16:02:31 | bauzas | moving on now | |
| 16:02:33 | bauzas | #topic Bugs (stuck/critical) | |
| 16:02:41 | bauzas | No Critical bugs | |
| 16:02:48 | bauzas | #link 11 new untriaged bugs (+1 since the last meeting): #link https://bugs.launchpad.net/nova/+bugs?search=Search&field.status=New | |
| 16:02:58 | bauzas | I'll try to look at some of them tomorrow | |
| 16:03:14 | bauzas | any other bugs people wanna raise ? | |
| 16:03:46 | stephenfin | nope, we had a gate issue due to Sphinx 4.x but sean-k-mooney fixed that for us | |
| 16:04:11 | bauzas | we could have had a cinderclient bug, but the v3 change is now merged, right? | |
| 16:04:22 | bauzas | stephenfin: excellent, thanks sean-k-mooney | |
| 16:04:22 | sean-k-mooney | ya i think that is merged now | |
| 16:04:28 | stephenfin | Yes, the nova one landed last week and the novaclient one went in earlier today | |
| 16:04:41 | bauzas | oki doki | |
| 16:04:58 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/802334 | |
| 16:04:59 | bauzas | were we limiting the cinderclient version ? | |
| 16:05:02 | sean-k-mooney | that was the nova one | |
| 16:05:15 | sean-k-mooney | bauzas: no we jsut had a refernce to v2 | |
| 16:05:22 | bauzas | ok | |
| 16:05:28 | sean-k-mooney | replced it with v3 | |
| 16:05:28 | bauzas | anyway, moving | |