| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-08 | |||
| 20:47:10 | dansmith | and didn't originally | |
| 20:47:18 | cfriesen | mriedem: yeah, I think that'd make sense to use VIR_DOMAIN_DESTROY_GRACEFUL if we have attached volumes. | |
| 20:47:44 | cburgess | cfriesen Why not call stop internall? That already has logic to attempt to gracefully stop and if that fails hard kill. Then do the cleanup in destroy. | |
| 20:47:54 | cfriesen | dansmith: on stop we currently use virDomainShutdown first, then virDomainDestroy | |
| 20:48:23 | dansmith | cfriesen: I know | |
| 20:48:31 | cfriesen | cburgess: there's no need for a graceful stop if there are no attached volumes | |
| 20:48:41 | gatuus | even virsh will not allow such a corruption | |
| 20:48:53 | cfriesen | cburgess: but yeah, we could make it do a shutdown the same way we do for "nova stop" | |
| 20:48:57 | cburgess | cfriesen I agree but again rather then having different logic for being graceful, why not simple say if volumes, call stop, then destroy. | |
| 20:49:19 | cburgess | cfriesen Right. Then we don't have to implement a 2nd 'be graceful' routine. | |
| 20:50:04 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: api-ref: Add tags to server(s) detail response https://review.openstack.org/431149 | |
| 20:50:26 | cfriesen | okay....does this need a spec, just a blueprint, or is it a bugfix? | |
| 20:50:48 | dansmith | it's not just a bugfix, IMHO | |
| 20:51:00 | dansmith | and I'm not sure we can call stop without some real refactoring | |
| 20:51:13 | cburgess | That might true | |
| 20:51:24 | cburgess | Some of the logic might need to come out into its own function that can be shared with destroy | |
| 20:51:50 | melwitt | does mounting a volume in the vm mean it has to be attached from nova's perspective? like mounting of a shared drive? | |
| 20:52:03 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: api-ref: Add tags to server(s) detail response https://review.openstack.org/431149 | |
| 20:52:31 | cfriesen | melwitt: you're thinking some non-cinder volume? | |
| 20:52:44 | melwitt | cfriesen: yeah | |
| 20:53:02 | cfriesen | melwitt: right, for something like that you'd want a clean shutdown even if there aren't any cinder volumes attached. | |
| 20:53:10 | melwitt | I was thinking, is there a case where checking nova for "if volumes" isn't enough and if we should just do the graceful destroy regardless | |
| 20:53:31 | dansmith | if a cinder volume is attached from nova's perspective, we have to assume it's mounted or dirty | |
| 20:54:09 | cfriesen | melwitt: at the same time I could see a useful purpose for a "just kill it dead instantly" command | |
| 20:54:36 | dansmith | right, I mean if we care about the data, we have to assume the above | |
| 20:54:46 | dansmith | if we're told not to, then we could kill it outright | |
| 20:55:07 | cfriesen | do we need a "nova delete --hard" option? (or a --soft and default to hard)? | |
| 20:55:37 | dansmith | well, delete is kinda always documented as hard | |
| 20:55:56 | dansmith | the most compatible thing would be a delete --soft, IMHO | |
| 20:56:01 | dansmith | or just stopping the instance yourself first | |
| 20:56:16 | dansmith | but we love pets around here, so.. | |
| 20:56:45 | melwitt | fwiw, from a user perspective I would have expected a graceful delete by default. if anything in there was mounted | |
| 20:57:02 | cfriesen | heh...honestly if we're not going to default to soft, then I don't see much gain since we can just stop it first | |
| 20:57:24 | mriedem | until you forget to do soft first, | |
| 20:57:29 | mriedem | or didn't know you needed to, | |
| 20:57:35 | mriedem | and now your attached volume is corrupted | |
| 20:57:39 | mriedem | and you curse openstack and use aws | |
| 20:57:49 | cfriesen | yeah, it's a bit gross that attached cinder volumes get corrupted | |
| 20:58:00 | mriedem | s/soft/stop/ | |
| 20:58:13 | dansmith | to be clear, I think it's only application-level state, | |
| 20:58:20 | melwitt | I guess that means it doesn't detach them first before destroying | |
| 20:58:37 | dansmith | libvirt will still honor the write semantics of the block device I think | |
| 20:58:42 | dansmith | so the filesystem should be okay | |
| 20:58:53 | cfriesen | anything cached in the guest may get lost | |
| 20:58:56 | smcginnis | Depends on what IOs are in flight I think. | |
| 20:59:02 | dansmith | cfriesen: just like a power outage | |
| 20:59:03 | cburgess | dansmith That depends on how you mount the FS inside the VM really. | |
| 20:59:04 | cfriesen | but that should be okay assuming proper locking | |
| 20:59:12 | dansmith | cburgess: you mean "safely" or not? :P | |
| 20:59:17 | smcginnis | Unless the OS knows to flush its IO cache, filesystem or anything could get messed up. | |
| 20:59:23 | cburgess | dansmith Yes. Its stupid.. but it does happen | |
| 20:59:45 | cburgess | I feel like a best effort to gracefully stop first is a good idea. But I agree its probably a blueprint. | |
| 20:59:48 | dansmith | smcginnis: the only way to safely do it at all is with libvirt honoring disk-like cache semantics and using a proper filesystem config so that works anyway | |
| 20:59:53 | dansmith | smcginnis: otherwise you're running raid with memory cache and no battery | |
| 21:00:22 | mriedem | melwitt: we destroy the guest before we detach the volumes in _shutdown_instance | |
| 21:00:26 | dansmith | cburgess: it's just a change in behavior from right now, whereas we could add it for people that know, is all | |
| 21:00:37 | melwitt | mriedem: a-ha, okay | |
| 21:00:52 | dansmith | cburgess: anyone wondering why the volume isn't becoming available for a minute will have to realize we changed | |
| 21:00:55 | cburgess | dansmith Well I agree its a change which is why it should be a blueprint for P+? | |
| 21:01:02 | dansmith | cburgess: but agree it probably should have been this way from the beginning | |
| 21:01:14 | dansmith | cburgess: it should be a blueprint regardless of the default, IMHO | |
| 21:01:14 | cburgess | dansmith Can't we just handle that with release notes? | |
| 21:01:22 | dansmith | cburgess: lol | |
| 21:01:28 | cburgess | I mean we can add a flag I guess. | |
| 21:01:35 | cburgess | I know sdague will kill me for suggesting a flag. | |
| 21:01:37 | mriedem | no config options | |
| 21:01:44 | cburgess | Apparently mriedem too | |
| 21:01:47 | cburgess | :P | |
| 21:01:59 | dansmith | I'm not -2 on changing the default, I'm just not as up in arms about the current default | |
| 21:02:05 | dansmith | but I treat my data nicely, so | |
| 21:02:35 | melwitt | graceful destroy by default seems reasonable to me. I don't think we need a config option | |
| 21:03:19 | sdague | I think we've been back and forth across that line a bunch in the past | |
| 21:03:21 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: check if we can quiesce before volume-backed snapshot https://review.openstack.org/380433 | |
| 21:03:21 | cburgess | I agree it *should* be the default but if there is complaint I feel a config option is reasonable. Then again config options are so out of fashion now. | |
| 21:03:57 | dansmith | cburgess: were definitely not doing a config option :) | |
| 21:04:07 | sdague | one of the challenges with graceful shutdown by default is that you end up with a lot of cloud guests that don't have acpi actually working | |
| 21:04:15 | dansmith | right | |
| 21:04:20 | sdague | so it turns into add a 2 minute timeout to all deletes | |
| 21:04:54 | dansmith | well, four :) | |
| 21:04:55 | sdague | dansmith: oh, sorry, I only caught from cburgess yelling at me | |
| 21:05:09 | cburgess | sdague I didn't yell at you.. said you would yell at me. | |
| 21:06:18 | dansmith | with bfv, soft delete, and graceful shutdown before delete, we've really watered down the meaning of "destroy" | |
| 21:06:46 | cburgess | dansmith New call? New option to the call? destroy vs hard destroy? destroy + graceful? | |
| 21:07:12 | sdague | dansmith: well, we've given it a distinctly british flavour. please sir, may you stop now. | |
| 21:07:25 | dansmith | cburgess: well, my preference (after just telling people to shutdown their instances first) would be a "delete with flag for shut-me-down-first" | |
| 21:07:27 | cburgess | LOL | |
| 21:07:47 | dansmith | sdague: we should start apologizing to the users | |
| 21:07:49 | dansmith | 499: Sooory | |
| 21:07:54 | sdague | dansmith: ++ | |
| 21:07:56 | melwitt | the api doesn't call it "destroy" though. it's called "delete" which doesn't seem to imply anything to me | |
| 21:07:58 | cburgess | dansmith Thats not unreasonable either. New micro-version that adds an option graceful flag? | |
| 21:08:14 | cburgess | optional graceful flag even | |
| 21:08:14 | dansmith | cburgess: right | |
| 21:08:19 | melwitt | but, if the timeout is really 2 minutes then I could see the --graceful option being an option | |
| 21:08:45 | cburgess | OK well sorry to leave in the middle of this but wife just called to say the hot water heater is making a very loud banging noise continually.. so... yeah... thats fun. | |
| 21:09:08 | dansmith | melwitt: to me "delete" does not mean "don't really delete", but.. we lost the actually-deleting-things battle a while ago :P | |
| 21:09:36 | sdague | the main reason I think a lot of people want this is windows shutting down, and you get the "I'm applying patches don't turn me off" screen | |
| 21:09:51 | sdague | melwitt: so the timeout has to be at least 2 minutes so you don't kill that | |
| 21:09:58 | dansmith | oh forgot about that, that's a good one | |
| 21:10:00 | melwitt | dansmith: heh, I guess. as a layperson, I didn't think of "delete" as meaning much other than "get rid of this instance" and not thinking about what happens underneath | |