| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-12-12 | |||
| 00:35:01 | efried | oh | |
| 00:35:08 | efried | allocations etc only go away at offload? | |
| 00:35:27 | dansmith | I would think.. the virt driver doesn't know a shelve has happened until the offload | |
| 00:35:30 | efried | so yeah, that's what I meant by "make space". But again yeah, that's something the operator cares about, not the user. | |
| 00:35:35 | dansmith | right | |
| 00:35:50 | efried | the virt driver just gets told to snapshot(). I guess they could figure it out from a task state? | |
| 00:35:56 | efried | but yeah, they don't. | |
| 00:36:13 | dansmith | well, it would be super problematic if they did, | |
| 00:36:37 | efried | Well, on that note... | |
| 00:36:37 | efried | what's the task state during the virt driver's destroy() during the offload? | |
| 00:36:43 | dansmith | because the point of the shelved state is that it is still ready to be powered back on, which means that like POWERED_OFF state, it still takes the same amount of resources as ACTIVE | |
| 00:36:51 | efried | (I'm going to have to find out anyway, but if you know off the top...) | |
| 00:37:18 | dansmith | I dunno, but why? the virt driver shouldn't be implying anything from task_state during an op like that | |
| 00:37:52 | dansmith | the only time virt should ever really look at that stuff is when it's recovering and trying to figure out if something should be deleted or ignored or something like that | |
| 00:37:58 | efried | I was going to save the vTPM backing file during the offload. But that has to be done by the virt driver, so the virt driver was going to have to have some way of knowing it needed to do that. | |
| 00:38:17 | efried | But the fact that the offload is ever (usually) done with no (or admin?) context blows that out of the water. | |
| 00:38:24 | efried | I'm going to have to do it during shelve. | |
| 00:38:30 | dansmith | task_state is really a lock for the api and downstream services to avoid taking two simultaneous actions on an instance | |
| 00:38:37 | dansmith | then you need to tell the virt driver to do it, | |
| 00:38:41 | efried | Which means similarly the virt driver needs to be able to know it should do that during snapshot | |
| 00:38:45 | dansmith | not have the virt driver imply that it needs to do it from the state | |
| 00:38:57 | dansmith | because, for example, shelve is used under the covers for cross-cell-migration | |
| 00:38:59 | efried | so add a kwarg to snapshot()? | |
| 00:39:10 | efried | or a new virt driver method, prep_shelve()? | |
| 00:39:14 | dansmith | yeah, but also, shouldn't that be the job of the compute manager anyway? | |
| 00:39:23 | dansmith | like "give me the tpm so I can save it for you" | |
| 00:39:24 | efried | no, can't be. | |
| 00:39:31 | efried | oh, that could work, yeah. | |
| 00:39:51 | efried | but obv we can't have the compute manager knowing libvirt internals | |
| 00:40:17 | efried | i.e. where that file lives. | |
| 00:40:21 | dansmith | no, of course not, but it can ask for the tpm in a portable form | |
| 00:40:36 | efried | like... an IMAGE!!! (not) | |
| 00:40:40 | dansmith | or even "give me things I should stash for you during a shelve-like snapshot" | |
| 00:41:45 | efried | oo, nothing today. | |
| 00:41:49 | dansmith | then you let compute manager decide whether it is going to save or "lose" those things, because compute manager knows what high-level operation is happening | |
| 00:41:52 | dansmith | virt does not really | |
| 00:42:45 | efried | So could that be a file-like that compute manager is responsible for streaming from? | |
| 00:42:46 | dansmith | and, you let compute manager decide *how* to save those things, instead of teaching every virt driver about all the ways things can be saved (swift, glance, etc) | |
| 00:43:36 | dansmith | efried: I would call it a BytesIO yeah | |
| 00:43:54 | efried | okay, that can work. | |
| 00:43:57 | dansmith | or IOBase or whatever the right bytes-based primitive is | |
| 00:44:34 | efried | That's what I meant: "the appropriate streamable thing that I'm going to have to go learn about cause I clearly don't know it yet" | |
| 00:44:51 | dansmith | BytesIO for your unit test | |
| 00:45:08 | dansmith | file(name, 'rb') for your libvirt from-file implementation | |
| 00:45:18 | dansmith | er, open(name, 'rb') | |
| 00:46:24 | efried | That's gonna be a tarball that I need to clean up afterward. How do I do that? | |
| 00:46:30 | dansmith | which is an io.BufferedReader | |
| 00:46:30 | efried | (Musing to self) | |
| 00:46:49 | dansmith | then just spawn tar and read from the tar pipe, don't create it on disk first | |
| 00:46:56 | efried | I imagine there must be a way to just create the ... yeah, pipe. | |
| 00:47:19 | dansmith | p = subprocess.Popen('tar c - file1 file2', stdout=subprocess.PIPE) | |
| 00:47:31 | dansmith | return p.stdout | |
| 00:48:13 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Spec: Ussuri: Encrypted Emulated Virtual TPM https://review.opendev.org/686804 | |
| 00:49:49 | efried | dansmith: thanks for the help, this is gonna make some (more) things easier, again. | |
| 00:49:52 | efried | or at least simpler | |
| 00:50:03 | efried | And with that, I'm outta here. | |
| 00:50:06 | efried | o/ | |
| 00:55:42 | melwitt | TheJulia: around? | |
| 01:05:26 | mriedem | efried: fwiw reading scrollback, the libvirt driver does check task_state to flatten rbd images while unshelving i think | |
| 01:06:18 | mriedem | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L9246 | |
| 01:06:56 | mriedem | efried: also btw, immediate shelve offload is the default behavior and there is a context | |
| 01:07:09 | mriedem | the periodic or shelveOffload api only kick in if you've configured the computes to not immediately offload | |
| 01:07:21 | mriedem | https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.shelved_poll_interval | |
| 01:07:57 | openstackgerrit | Eric Xie proposed openstack/nova master: Report trait 'COMPUTE_IMAGE_TYPE_PLOOP' https://review.opendev.org/698132 | |
| 01:08:05 | mriedem | dansmith: btw, "because, for example, shelve is used under the covers for cross-cell-migration" is not true - it was for the original poc, but not what i have now | |
| 01:08:12 | mriedem | parts are similar though | |
| 01:09:19 | dansmith | mriedem: well, yeah, sorry I'm collapsing shelve-like snapshot behavior into it | |
| 01:09:50 | dansmith | mriedem: and the task_state check down in the rbd stuff is just a total hack, I hope you'd agree | |
| 01:10:00 | dansmith | because of the layers of plumbing that would be required | |
| 01:10:05 | mriedem | it is | |
| 01:11:09 | mriedem | we're the ones that approved it too :) | |
| 01:11:25 | dansmith | mriedem: given that a snapshot for migration would be very shelve-like, but a task-state check for shelving to save the tpm would be the exact kind of guessing the virt driver shouldn't be doing because composite operations on top of the primitives can't rely on predictable behavior | |
| 01:12:20 | mriedem | yup, case in point https://review.opendev.org/#/c/691991/ | |
| 01:12:24 | dansmith | very shelve-like, meaning it would *need* to store the tpm, but wouldn't if the task_state wasn't shelving, so migrations that happened to cross cells would end up with an empty tpm | |
| 01:12:28 | dansmith | which is exactly my point | |
| 01:12:45 | dansmith | ha yeah | |
| 01:13:57 | dansmith | this marks my 12th hour behind the keyboard today | |
| 01:24:28 | mriedem | dansmith: for tomorrow - a compute rpc api method that didn't return anything but now does - version bump? i would think so at least for the behavior change. | |
| 01:33:25 | openstackgerrit | Matt Riedemann proposed openstack/nova master: DNM: debug cross-cell resize https://review.opendev.org/698304 | |
| 01:48:09 | openstackgerrit | sean mooney proposed openstack/nova master: Sync ImageMetaPropsPayload fields https://review.opendev.org/698613 | |
| 01:50:28 | sean-k-mooney | gibi: efried ^ wanted to get that done before i went to sleep there are only 3 fields that were missing | |
| 01:50:44 | sean-k-mooney | anyway night o/ | |
| 02:30:22 | pmatulis | when implementing a policy override does one need to place /etc/nova/policy.json on just a single compute node? | |
| 02:45:49 | melwitt | pmatulis: not the compute node, the controller (where nova-api process is running) | |
| 02:46:05 | melwitt | policies are an api thing | |
| 02:48:00 | pmatulis | melwitt, ah ha right | |
| 02:54:13 | pmatulis | melwitt, this works but do you see anything wrong with it? | |
| 02:54:17 | pmatulis | https://paste.ubuntu.com/p/jcq7znKczd/ | |
| 02:54:33 | pmatulis | i left the default as a comment | |
| 02:56:09 | melwitt | pmatulis: I think you can't have a comment in a json file (but you said it works?). the new entry looks fine to me | |
| 02:57:37 | pmatulis | melwitt, yeah works :) | |
| 02:58:04 | melwitt | kewl | |
| 03:15:42 | pmatulis | melwitt, so this is odd. i removed /etc/nova/policy.json (it did not exist prior) and now i get error (log: "...stacktrace...cannot find policy.json). i put it back and error goes away | |
| 03:16:09 | pmatulis | confirm i can just drop a file in w/o stopping the service? | |
| 03:16:57 | melwitt | pmatulis: yeah, it shouldn't need it (all defaults are in the code). I would have thought you could drop it without restarting the service but I haven't tried it before. did you have to restart for it to notice it when you added it or no? | |
| 03:17:13 | melwitt | when you added it the first time I mean | |
| 03:19:03 | pmatulis | melwitt, no, i did not | |
| 03:19:16 | pmatulis | i read that it wasn't necessary | |
| 03:20:56 | melwitt | yeah, I do know it tries to read the file each policy check (if it is there) and you are meant to be able to change it on the fly | |
| 03:21:34 | melwitt | I don't know why it would error when you remove it. if you pastebin the stacktrace we can at least see where in the code it fails | |