Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-05-04
15:41:17 mriedem giblet: good news is i think most of it is just asking for clarification
15:42:10 jgwentworth jmccarthy: you see disk.info on the source or destination?
15:42:18 openstackgerrit Eric Fried proposed openstack/nova master: placement: Object changes for granular https://review.openstack.org/564351
15:42:19 openstackgerrit Eric Fried proposed openstack/nova master: placement: Granular GET /allocation_candidates https://review.openstack.org/517757
15:42:23 giblet mriedem: thanks for the review.
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

Earlier   Later