| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-04 | |||
| 15:42:44 | jmccarthy | jgwentworth: It's no where in sight, until I confirm the resize/migrate - and then it appears on the host (the cold migrate completes fine) | |
| 15:43:03 | jmccarthy | *source host | |
| 15:43:09 | jgwentworth | ok | |
| 15:44:08 | jmccarthy | I'm going to check - I just remembered I looked at the file earlier - have to recheck what is actually in it somethingsomething qcow2 it said in the file | |
| 15:44:44 | giblet | mriedem: I will go through you comments on Monday | |
| 15:44:55 | jgwentworth | yeah, I can never remember what all of those are. I have to re-look at it every time in the code | |
| 15:45:31 | mriedem | dansmith: (1) where is superdan and (2) i've got a request spec modeling comment in https://review.openstack.org/#/c/502306/26/specs/rocky/approved/bandwidth-resource-provider.rst@172 that could use your input | |
| 15:45:37 | mriedem | giblet: ack | |
| 15:45:49 | superdan | mriedem: tab queued | |
| 15:50:55 | jgwentworth | jmccarthy: what's your CONF.libvirt.images_type set to? | |
| 15:51:33 | jmccarthy | one sec | |
| 15:51:44 | mriedem | umm https://review.openstack.org/#/c/516395/ | |
| 15:52:13 | jgwentworth | hah, nice | |
| 15:52:29 | mriedem | also added a tempest test for that https://review.openstack.org/#/c/516396/ | |
| 15:53:18 | jgwentworth | so maybe ... supposed to remove the disk.info that imagebackend creates but don't remove the whole instance directory? | |
| 15:53:23 | jmccarthy | Hmm it's images_type = rbd in nova.conf | |
| 15:53:35 | jgwentworth | k, thanks | |
| 15:53:37 | mriedem | yeah so that's exactly what that patch was meant to fix, | |
| 15:53:46 | mriedem | and from the tempest patch commit message, it's about shared storage backends | |
| 15:53:53 | mriedem | so in our ci, that's ceph and nfs | |
| 15:54:09 | mriedem | the problem was you would resize the instance and then try to get the console log which was deleted | |
| 15:54:25 | mriedem | https://bugs.launchpad.net/nova/+bug/1728603 | |
| 15:54:25 | openstack | Launchpad bug 1728603 in OpenStack Compute (nova) pike "Resize a boot-from-volume instance with NFS destroys instance" [High,Fix committed] - Assigned to Matt Riedemann (mriedem) | |
| 15:54:33 | jgwentworth | right, that's what I'm trying to say is that it's true we shouldn't nuke the entire instance dir | |
| 15:54:46 | jgwentworth | but, we should delete the disk.info by itself | |
| 15:55:46 | jgwentworth | the original intent of https://review.openstack.org/#/c/437356/3/nova/virt/libvirt/driver.py was to delete the disk.info that imagebackend sometimes creates | |
| 15:56:17 | jgwentworth | but it went ahead and took out the entire instance directory not realizing it would lose the console.log | |
| 15:56:35 | jmccarthy | Yea I was reading those comments in driver.py - but the rm commands seem to only involve the dir (which I agree you can remove i.e. nfs) | |
| 15:56:49 | jmccarthy | *can't | |
| 15:56:59 | Cardoe | mriedem: ok so a bit more info. Modifying vm_states.py to allow rescued to be rebooted then it works. | |
| 15:57:25 | mriedem | Cardoe: sure, when using the xenapi driver right? | |
| 15:57:29 | Cardoe | yes | |
| 15:57:35 | jmccarthy | jgwentworth: But there must be logic to know when it's safe to remove that dir on the source host ? | |
| 15:57:38 | jgwentworth | jmccarthy: did you see the patch mriedem linked though? it's bypassing all of that if volume-backed https://review.openstack.org/#/c/516395/2/nova/virt/libvirt/driver.py because it was trying to fix the case where the console.log was being deleted | |
| 15:57:41 | mriedem | i have no idea if libvirt/hyperv/vmware support that | |
| 15:57:59 | jmccarthy | 1728603 ? Let me look | |
| 15:58:18 | Cardoe | mriedem: I've been told that I do not believe that they do. | |
| 15:58:18 | jgwentworth | no, https://review.openstack.org/#/c/516395/2/nova/virt/libvirt/driver.py | |
| 15:58:31 | Cardoe | s/I/rax OpenStack folks/ | |
| 15:59:36 | jmccarthy | jswentworth: I think the driver.py that I have now looks like that one ? | |
| 16:00:01 | jgwentworth | jmccarthy: so because you are volume-backed, it's not removing anything at all. I think it needs to be adjusted to only delete disk.info if volume-backed, or something like that | |
| 16:00:14 | mriedem | jmccarthy: because you're on queens and that change was made in queens | |
| 16:00:31 | mriedem | and backported to pike and ocata because it was fixing a regression which was also backported to pike and ocata :) | |
| 16:00:31 | jmccarthy | jgwentworth: Ok so it's not just me - yes some logic is missing ? | |
| 16:00:35 | mriedem | it's bugs all the way down | |
| 16:00:52 | jmccarthy | jgwentworth: Leaving the dir there causes a problem for live migration too | |
| 16:00:57 | jgwentworth | yeah, there is definitely a bug. we were just trying to find what it was | |
| 16:01:07 | mriedem | jmccarthy: the dir or just the disk.info file itself? | |
| 16:01:13 | jmccarthy | jgwentworth: the dir | |
| 16:01:18 | jmccarthy | mriedman: the dir | |
| 16:01:30 | jmccarthy | Sorry I'm eventually getting there lol thanks for the help all ! :) | |
| 16:01:35 | jmccarthy | One sec | |
| 16:01:35 | jgwentworth | so what about the console.log then? where is it supposed to go | |
| 16:01:49 | openstackgerrit | Eric Fried proposed openstack/nova master: placement: Object changes for granular https://review.openstack.org/564351 | |
| 16:01:50 | openstackgerrit | Eric Fried proposed openstack/nova master: placement: Granular GET /allocation_candidates https://review.openstack.org/517757 | |
| 16:02:03 | jgwentworth | oh, wait, supposed to remove the dir from the source | |
| 16:02:27 | mriedem | jgwentworth: but if you're on shared local storage, deleting the dir on the source will also remove it from the dest | |
| 16:02:29 | jmccarthy | mriedman: Actually I have it at the end of the bug - https://bugs.launchpad.net/nova/+bug/1769131 about 'already exists, it is expected not to exist.' | |
| 16:02:29 | openstack | Launchpad bug 1769131 in OpenStack Compute (nova) "After cold-migration of a volume-backed instance, disk.info file leftover on source host" [Undecided,Triaged] | |
| 16:02:48 | jgwentworth | mriedem: yeah, right. okay. | |
| 16:02:59 | mriedem | jmccarthy: yeah DestinationDiskExists only happens if compute says you're not on shared storage, | |
| 16:03:09 | mriedem | but since you're using image_type=rbd i'd think your computes would be using shared storage | |
| 16:03:12 | jgwentworth | yeah so it sounds like just need to delete disk.info, the presence of the dir should be okay right? | |
| 16:03:24 | jmccarthy | Should it ? | |
| 16:03:29 | jmccarthy | I only have 2 computes | |
| 16:03:39 | jmccarthy | if I try to live migrate back I get the complaint about the dir being there | |
| 16:03:44 | mriedem | jgwentworth: i don't think so https://github.com/openstack/nova/blob/4b0d0ea9f18139d58103a520a6a4e9119e19a4de/nova/virt/libvirt/driver.py#L7457 | |
| 16:04:03 | jgwentworth | oh :\ | |
| 16:04:06 | mriedem | jmccarthy: i guess you can use image type rbd for local storage and not having it be shared, but that seems weird | |
| 16:04:07 | jmccarthy | 1) cold migrate - it works, but disk.info is left on source | |
| 16:04:16 | mriedem | apparently people do it, but i'm not sure why you'd be using rbd and not sharing it | |
| 16:04:17 | jmccarthy | 2) try to live back fails due to dir :/ | |
| 16:04:19 | jgwentworth | then how is this supposed to work with shared storage | |
| 16:04:48 | jmccarthy | mriedman: Not sure I follow, I'm using cinder for storage, not local ? | |
| 16:05:14 | mriedem | the console.log is stored on local disk for the compute host | |
| 16:05:15 | mriedem | not in cinder | |
| 16:05:16 | jmccarthy | Well after the live fails then it does: Cleaning up deleted instances with incomplete migration | |
| 16:05:16 | jmccarthy | _cleanup_incomplete_migrations | |
| 16:05:23 | jmccarthy | and the next attempt to live migrate works | |
| 16:05:44 | jgwentworth | okay yeah I see now, you are using rbd but not shared storage ... only does DestinationDiskExists if not shared | |
| 16:06:10 | mriedem | so is it just me, or does https://review.openstack.org/#/c/437356/3/nova/virt/libvirt/driver.py@1136 seem like kind of a hack? | |
| 16:06:13 | jmccarthy | mreidman: I'm confused, I do have a console log file there, but I am getting volumes also .. | |
| 16:06:21 | Cardoe | mriedem: what would you like to see happen with the rescue stuff? We can try and submit a patch to allow it for xenapi? | |
| 16:06:24 | mriedem | "self.image_backend.image for some backends recreates instance directory and image disk.info - remove it here if exists" | |
| 16:06:43 | jgwentworth | mriedem: yeah, it does. when I read it I was like, "why is imagebackend sometimes recreating the dir" | |
| 16:06:46 | mriedem | Cardoe: idk yet, that's why i posted to the ML | |
| 16:06:57 | jmccarthy | s/getting/using | |
| 16:07:05 | mriedem | those details are in https://bugs.launchpad.net/nova/+bug/1666831 | |
| 16:07:05 | openstack | Launchpad bug 1666831 in OpenStack Compute (nova) ocata "Nova recreates instance directory after migration/resize" [Low,Fix committed] - Assigned to Lee Yarwood (lyarwood) | |
| 16:08:14 | mriedem | apparently this recreates the disk.info on the source host https://github.com/openstack/nova/blob/4b0d0ea9f18139d58103a520a6a4e9119e19a4de/nova/virt/libvirt/driver.py#L1152 | |
| 16:08:31 | mriedem | and then https://github.com/openstack/nova/blob/4b0d0ea9f18139d58103a520a6a4e9119e19a4de/nova/virt/libvirt/driver.py#L1170 was added to delete it | |
| 16:09:29 | Cardoe | mriedem: I've been told that the reason why we didn't upstream the change is that there was no easy way to allow it only for xenapi at the time. | |
| 16:09:33 | jmccarthy | For me this happens, and it's ok | |
| 16:09:34 | jmccarthy | CMD "rm -rf /var/lib/nova/instances/371e669b-0f15-49f2-9a84-bd1e89f34294_resize" returned: 0 | |
| 16:09:34 | jmccarthy | except then | |
| 16:09:49 | jmccarthy | Lock "/var/lib/nova/instances/371e669b-0f15-49f2-9a84-bd1e89f34294/disk.info" acquired | |
| 16:10:01 | jmccarthy | and Lock "/var/lib/nova/instances/371e669b-0f15-49f2-9a84-bd1e89f34294/disk.info" released by "nova.virt.libvirt.imagebackend.write_to_disk_info_file" | |
| 16:10:10 | jmccarthy | recreates it from what I can tell ? | |
| 16:10:22 | jgwentworth | yeah, I think so | |
| 16:11:12 | jgwentworth | Cardoe: is rebooting a rescued instance only safe or something that makes sense for xenapi? | |