| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-07-27 | |||
| 09:57:12 | sean-k-mooney | CeeMac: if we were to expose the timer settign i think it woudl ahve to be at teh image level rghat then host by the way | |
| 09:57:42 | CeeMac | sean-k-mooney: yes, that would make sense | |
| 09:57:55 | sean-k-mooney | CeeMac: host level config that modify the xml are basicaly terribel form a live migration standpoint as we havne to schdule on it or pass the data or both | |
| 09:58:36 | sean-k-mooney | so this type of info really needt to live with the instnace for its lifetime hence flavor or image | |
| 09:59:08 | sean-k-mooney | with that said i woudl be tempeted to start using server metadata for this personally | |
| 09:59:29 | sean-k-mooney | since this is a very pet like tuning | |
| 09:59:40 | CeeMac | it makes sense that it should follow the instance | |
| 09:59:48 | CeeMac | windows is all about the pet sadly | |
| 10:01:26 | CeeMac | server metadata would be useful i think | |
| 10:02:55 | sean-k-mooney | we dont currently allow it to modify xml exctra but i have always wondered if we should allow it to set anything that is setabel via image metadata | |
| 10:03:46 | sean-k-mooney | that would require a spec and some semi invasive changes to parts of the libvirt driver | |
| 10:03:49 | CeeMac | in some scenarios it could be beneficial, especially if its discovered you need to retrofit a value that would normally only be doable through an image | |
| 10:03:54 | sean-k-mooney | so not sure its worth it | |
| 10:03:59 | CeeMac | its still a very pet mentality granted | |
| 10:04:20 | CeeMac | hmm, cost/benefit are skewed i guess | |
| 10:04:30 | sean-k-mooney | CeeMac: ya we are adding a nova-manage command that operators can use for a limit set of image properties | |
| 10:04:57 | CeeMac | oh, that sounds interesting | |
| 10:05:25 | sean-k-mooney | the intent is for operators taht need to change things for upgrades | |
| 10:05:39 | sean-k-mooney | e.g. move to q35 which means you have to remove use of ide | |
| 10:05:48 | sean-k-mooney | ectra | |
| 10:06:15 | sean-k-mooney | we approved https://github.com/openstack/nova-specs/blob/master/specs/newton/approved/virt-image-props-boot-override.rst in the past but then decieded to no porceed with it when we came to implemenation | |
| 10:07:21 | sean-k-mooney | im still somewhat open to that idea but i dont fully recal what the main objects were | |
| 10:07:32 | CeeMac | i imagine its tricky finding a decent balance point between stability and the ability to make dynamic changes | |
| 10:08:15 | sean-k-mooney | ya and stricking a blance between upstream and downstream | |
| 10:08:35 | sean-k-mooney | downstream our hands are forced a bit by change made by other teams | |
| 10:09:21 | sean-k-mooney | for example qxl graphic is going away downstream so we have to provide a way to move instances off it which is what propmeted the change | |
| 10:25:26 | sean-k-mooney | CeeMac: this is the new nova-magage command by the way https://github.com/openstack/nova-specs/blob/master/specs/xena/approved/nova-manage-commands-to-update-libvirt-device-models.rst | |
| 10:25:42 | sean-k-mooney | CeeMac: lyarwood is currently workign on it for xena | |
| 10:28:49 | CeeMac | looks good | |
| 10:46:47 | opendevreview | Merged openstack/python-novaclient master: Use Block Storage API v3 instead of API v2 https://review.opendev.org/c/openstack/python-novaclient/+/802417 | |
| 11:13:51 | opendevreview | Merged openstack/nova master: Bump os-resource-classes to 1.1.0 https://review.opendev.org/c/openstack/nova/+/800976 | |
| 11:50:53 | opendevreview | Merged openstack/os-vif master: add configurable per port bridges https://review.opendev.org/c/openstack/os-vif/+/798055 | |
| 12:34:33 | brinzhang | gibi,sean-k-moonkey,stephenfin,alex_xu: we submited the PoC code for allowing PMEM's data wihle migrate vm, but it is not completely implemented in accordance with the spec, because some irrationality in the spec was discovered during the implementation process, so we hope you can review the PoC code. | |
| 12:35:18 | brinzhang | ah, gibi has a holday(pto), gibi_pto | |
| 12:36:08 | brinzhang | the PoC code link: https://review.opendev.org/c/openstack/nova/+/802225 | |
| 12:36:44 | sean-k-mooney | brinzhang: do you have a patch to update the spec whith what diverged | |
| 12:37:28 | sean-k-mooney | brinzhang: what exactly was teh "irrationality in the spec" it may have been stated that way for a reason which the impleation misses | |
| 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 | |