| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-25 | |||
| 19:01:41 | sean-k-mooney | i would expect the image to go into an error state on start if its requirements chagne | |
| 19:01:51 | sean-k-mooney | but that is not very friendly | |
| 19:02:03 | sean-k-mooney | or start with the old requirement i guess | |
| 19:02:46 | mriedem | inititally the spec never said anything about supporting detach/attach of the root volume for powered off instances | |
| 19:02:50 | mriedem | only shelved offloaded | |
| 19:02:56 | mriedem | which is how hpe wrote it up long ago | |
| 19:02:57 | sean-k-mooney | yep in mitaka | |
| 19:03:25 | mriedem | i do remember that the RequestSpec.image.id is *not* set for volume-backed servers, so some of our scheduler filters don't work on volume-backed servers | |
| 19:03:52 | mriedem | like IsolatedHostsFilter | |
| 19:04:21 | mriedem | during server create, we do calculate the numa topology requirements from the flavor and image meta, and for volume-backed we get that image meta from the root volume, like you said | |
| 19:04:34 | mriedem | so technically on unshelve we could screw up and not honor the image meta in the new root volume | |
| 19:04:42 | mriedem | b/c we don't update the request spec | |
| 19:05:57 | sean-k-mooney | right. i think that is just an oversight however. e.g. readign the spec i would have assume we woudl cater for that edgecase in the implemnation and they jsut chose not to document it in the spec | |
| 19:06:29 | mriedem | i wouldn't be so sure | |
| 19:06:41 | sean-k-mooney | anyway it looks like newton was still only allowing this for shelved instance so its only allowed for powered off instance in the stein spec | |
| 19:07:16 | mriedem | i'm pretty sure i didn't think about the root image meta changing when s10 pushed for supporting attach/detach of the root volume on stopped instances | |
| 19:07:17 | sean-k-mooney | mriedem: im giving people the benifit of the doubt but perhaps it was not taught of | |
| 19:09:40 | sean-k-mooney | so with 2 weeks to feature freeze i dont really want to reopen the design of this given how long people have been waiting for it. | |
| 19:10:37 | sean-k-mooney | i could propose a revision to the spec to reduce it to only shelved insntace or to document the behavior that will happen for poered off instace | |
| 19:11:16 | sean-k-mooney | *powered off | |
| 19:11:47 | sean-k-mooney | im going to grab dinner but ill be back in an hour or so | |
| 19:12:08 | sean-k-mooney | actully i have to push a small revision to a patch then dinner | |
| 19:13:47 | mriedem | sean-k-mooney: i commented on https://review.openstack.org/#/c/614750/ as well to recap our irc discussion. i'll talk with Kevin_Zheng about it tonight since we have a meeting anyway | |
| 19:14:55 | mriedem | i don't think we can support the stopped case - that would essentially get us back to volume-backed rebuild with a new image that we don't really honor | |
| 19:16:11 | sean-k-mooney | mriedem: ya that was my feeling too but i could see use allowing it and refusing to boot the instacne if the requirement could not be supported | |
| 19:16:20 | sean-k-mooney | that felt overly complicated however | |
| 19:16:42 | sean-k-mooney | mriedem: can we shelve a powered off instance? | |
| 19:17:24 | mriedem | yes | |
| 19:17:31 | mriedem | which i think i said in the spec review when this came up, | |
| 19:17:46 | mriedem | just stop the instance, shelve it, swap the root volume, and unshelve, | |
| 19:17:47 | sean-k-mooney | ok so you jsut need to shelve it and then unshelve after changing volumes and you can more or less achive the same use case | |
| 19:17:52 | mriedem | but people thought that was too complicated | |
| 19:18:11 | sean-k-mooney | perhaps but ill take complicated over broken any day | |
| 19:22:29 | mriedem | sean-k-mooney: i guess i did think about this https://review.openstack.org/#/c/600628/7/specs/stein/approved/detach-boot-volume.rst@127 | |
| 19:25:27 | sean-k-mooney | mriedem ya looks like you did but it never made it into the spec in the end. | |
| 19:26:12 | sean-k-mooney | mriedem: atleast a concreate proposal for what to do in this case was not added to the spec. your comment are obviously there | |
| 19:28:30 | openstackgerrit | sean mooney proposed openstack/os-vif master: modify functional base.py to allow using vscode https://review.openstack.org/638058 | |
| 19:30:22 | mriedem | yeah i just don't want to get into a case where someone swaps the root volume on a stopped instance, then they rebuild it with what they thought was the original volume and rebuild blows up saying you can't rebuild a volume-backed instance with a new image, or they pass the image_id of the new root volume and the scheduler kicks it out saying it's not valid for the current host | |
| 19:30:40 | mriedem | s/original volume/original image/ | |
| 20:04:11 | openstackgerrit | Merged openstack/nova master: quota: remove QuotaEngine.register_resources() https://review.openstack.org/615613 | |
| 20:51:04 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: New objects to transmit NUMA config from dest to source https://review.openstack.org/634827 | |
| 20:51:04 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Introduce live_migration_claim() https://review.openstack.org/635669 | |
| 20:51:05 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: LM: add support for updating NUMA-related XML on the source https://review.openstack.org/635229 | |
| 20:51:05 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: LM: add support for sending NUMAMigrateData to the source https://review.openstack.org/634828 | |
| 20:51:06 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: [WIP needs more tests] Full NUMA live migration support https://review.openstack.org/634606 | |
| 20:51:06 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: RPC changes to prepare for NUMA live migration https://review.openstack.org/634605 | |
| 20:51:26 | artom | So that was "sync" push - let's run this in my test env to see what I've broken | |
| 20:57:22 | dansmith | dang | |
| 20:57:28 | dansmith | I was just working my way through them | |
| 20:57:41 | dansmith | now I have to start all over aain | |
| 20:57:45 | dansmith | guess I'll wait for next monay | |
| 20:57:52 | dansmith | also, wtf is up with my keys | |
| 20:58:56 | sean-k-mooney | dansmith: are you on vacation until monday? or just monday is the next time you will be able to review it | |
| 20:59:11 | sean-k-mooney | dansmith: just wondering if you will be around later in the week is all | |
| 20:59:54 | artom | dansmith, I'm sorry, are you asking for "monay"? I thought RH payed you well ;) | |
| 21:01:09 | dansmith | artom: monay monay | |
| 21:01:23 | artom | It's a Smith-man world! | |
| 21:01:32 | dansmith | sean-k-mooney: I'm giving artom a hard time implying that I only review large, late patch series on monday mornings | |
| 21:02:20 | sean-k-mooney | hehe in that case feel free to add my/adrianc's sriov patch set to that list :P | |
| 21:02:41 | sean-k-mooney | but on a serios note artom i deployed v21 on friday | |
| 21:02:46 | sean-k-mooney | *serious | |
| 21:03:04 | artom | v21? | |
| 21:03:06 | sean-k-mooney | am ill redeploy tomorow ish do you think it will change much | |
| 21:05:08 | sean-k-mooney | this was the top patch in the series on friday https://review.openstack.org/#/c/635229/21 | |
| 21:05:10 | artom | sean-k-mooney, dunno, lemme run my tests against what I have now, see what breaks | |
| 21:05:45 | sean-k-mooney | it looks like https://review.openstack.org/#/c/634606/30 is not the top patchset in the series | |
| 21:06:25 | artom | sean-k-mooney, that's changed. Claim creating and cleanup needs to happen in the same patch, along with a version bump because we need to check that source and/or dest support claims | |
| 21:07:31 | sean-k-mooney | right but https://review.openstack.org/#/c/634606/30 is now the final patch in the series right | |
| 21:07:38 | artom | So there's 2 service bumps to keep the patches reasonnably sized | |
| 21:07:42 | artom | 1 for the RPC stuff | |
| 21:07:54 | artom | And 1 for the final "we support NUMA LM" switchover | |
| 21:08:18 | artom | sean-k-mooney, correct | |
| 21:08:45 | artom | The overall mechanics haven't really changed, but they way they're presented/ordered and some details have | |
| 21:09:01 | sean-k-mooney | i can put off deploying the it for another few days and just to a code review tomorow and focus on other tasks if that helps. it will take me the better part of a day to run all the test i want too on it so i was going to wait till it sables a bit more | |
| 21:10:24 | artom | sean-k-mooney, your call, really. Ideally any tests that are missing from https://review.rdoproject.org/r/#/c/18832/ would get added there, so that we don't have to manually redo everything everytime there's a new patchset | |
| 21:10:32 | sean-k-mooney | artom: ok what i want to be able to do is send an email to the list and say i test all these senarios with this reviesion and heres what work and didnt | |
| 21:10:56 | sean-k-mooney | there are quite a lot of tests that arent covered | |
| 21:11:06 | artom | For instance, I realized I wasn't saving the new instance numa topology (fixed now), so I should add that to the tests | |
| 21:12:01 | artom | sean-k-mooney, oh I'm sure there are :) So... if you like it, just add them directly, or just write them out in English and I'll code them up | |
| 21:12:12 | sean-k-mooney | i might spend some time writing automated test but i also want to manually run them and verify it. | |
| 21:12:17 | mriedem | efried: jaypipes: i've gone through the next bw provider change in the series https://review.openstack.org/#/c/622421/ - i might be overly harsh on this one | |
| 21:13:04 | openstackgerrit | Merged openstack/nova master: conf: Deprecated 'defer_iptables_apply' https://review.openstack.org/445436 | |
| 21:20:05 | jaypipes | mriedem: I'm still trudging through them... sorry for the slowness. :( | |
| 21:21:00 | mriedem | that's fine | |
| 21:21:04 | mriedem | i'm about reviewed out for the day | |
| 21:22:42 | sean-k-mooney | jaypipes: hi am do you want to pushout the os-vif release to tomrow morning. i can send an update to the list with where things are or should i propose a relase with the current master. | |
| 21:23:45 | sean-k-mooney | jaypipes: i would like to get the new release into the gate by tomorrow at the latest but we should be ok to wait anouter few hours e.g. till US moring to tag | |
| 21:27:11 | jroll | dansmith: on https://review.openstack.org/#/c/635006 , this code path is in a periodic. so if we explode, the nova-compute won't crash, just won't expose any resources. that okay with you? | |
| 21:28:41 | dansmith | jroll: I mean, if it's a case that makes sense to not do anything then sure | |
| 21:29:34 | jroll | dansmith: reworded, do you think operators will see that problem if nova-compute keeps running? | |
| 21:29:35 | jaypipes | sean-k-mooney: yes, that would be good if we can delay one more day. | |
| 21:29:55 | dansmith | jroll: I mean, it's the same deal as if they mis-configured with nothing in that list right? | |
| 21:30:07 | dansmith | jroll: obviously it should be error-logged | |
| 21:30:14 | jroll | dansmith: right, I mean if it's unset | |
| 21:30:16 | sean-k-mooney | jaypipes: sure. i just want to have 1-2 days of gate time before the non-client lib freeze. | |
| 21:31:06 | jaypipes | ack | |
| 21:31:13 | sean-k-mooney | jaypipes: ill propose a patch and -w flow it with the current master and ill updated it tommorow once that feature lands. ill update teh list with all this soon | |
| 21:31:22 | jaypipes | ++ | |
| 21:33:44 | dansmith | jroll: if we have no other option than to log errors in each periodic, then, I think that's about all we can do | |
| 21:33:45 | dansmith | that said, | |