| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-14 | |||
| 14:21:09 | alex_xu | mriedem: yea, I will try to review tomorrow | |
| 14:21:37 | mriedem | alex_xu: thanks | |
| 14:21:45 | alex_xu | mriedem: np | |
| 14:21:47 | mriedem | mdbooth: are you going to be able to address review comments on this series soon? https://review.openstack.org/#/c/526347/ | |
| 14:22:22 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transform missing delete notifications https://review.openstack.org/410297 | |
| 14:22:48 | mdbooth | mriedem: I'm about half way through. | |
| 14:22:58 | mdbooth | Probably be a day or so. | |
| 14:23:13 | mdbooth | I was expecting to get bumped to the back of the runway if that's what you're asking. | |
| 14:23:19 | mriedem | mdbooth: ok, yeah | |
| 14:29:11 | mriedem | mdbooth: anyway, regardless of the runway, ping me when those 3 or so bottom changes are updated | |
| 14:29:11 | openstackgerrit | Shilpa Devharakar proposed openstack/nova master: Update docs, API samples for noVNC 1.0.0 https://review.openstack.org/550173 | |
| 14:29:32 | mdbooth | mriedem: Will do, thanks. | |
| 14:39:15 | kashyap | If anyone wants to take a look a simple clean-up: https://review.openstack.org/#/c/567258/ -- "libvirt: Remove usage of migrateToURI2() API" | |
| 15:02:31 | mriedem | lyarwood: when I +2ed https://review.openstack.org/#/c/567899/ i had a false positive on the live migration recreate patch sitting on top of it; i fixed that this morning and now the job is still failing even with your patch | |
| 15:02:49 | mriedem | this is when force_config_drive=true, images_type=raw, config_drive_format=vfat | |
| 15:02:51 | mriedem | vdrok: ^ | |
| 15:03:13 | vdrok | hrm | |
| 15:03:25 | mriedem | http://logs.openstack.org/60/567860/6/check/nova-live-migration/702f809/logs/screen-n-cpu.txt.gz?level=TRACE#_May_14_14_39_42_271510 | |
| 15:03:57 | vdrok | different error though, that's good :) | |
| 15:05:17 | mriedem | tonyb: kashyap: remember https://bugs.launchpad.net/nova/+bug/1246201 at all and the related libvirt bug? i couldn't actually find a specific libvirt bug/patch in the related libvirt ML threads, but i'm wondering which version of libvirt fixes that so we can deprecate the workaround of using config_drive_format=vfat | |
| 15:05:18 | openstack | Launchpad bug 1246201 in OpenStack Compute (nova) juno "Live migration fails when the instance has a config_drive_format=iso9660" [High,Fix released] - Assigned to Matt Riedemann (mriedem) | |
| 15:05:20 | openstackgerrit | Dan Smith proposed openstack/nova master: Make scheduler client allow multiple member_of query parameters https://review.openstack.org/568313 | |
| 15:05:30 | mriedem | "If your hypervisor driver is libvirt, and you want live migrate to work without shared storage, then use VFAT." | |
| 15:05:42 | kashyap | Hey there | |
| 15:06:18 | kashyap | mriedem: Where are you quoting that from? | |
| 15:06:21 | mriedem | that should probably read, "and you want live migrate to work with a config drive but without shared storage" | |
| 15:06:29 | mriedem | the "config_drive_format" option help text | |
| 15:07:09 | kashyap | mriedem: So, looking at the Launchpad bug, it redirects me to this upstream libvirt bug that I asked to file: | |
| 15:07:12 | kashyap | https://bugzilla.redhat.com/show_bug.cgi?id=1203032 | |
| 15:07:13 | openstack | bugzilla.redhat.com bug 1203032 in libvirt "Enhancing block/disk migration in libvirt" [Low,Closed: nextrelease] - Assigned to pboldin | |
| 15:07:18 | BobBall | hi melwitt - thought I'd join here rather than discussing on the ticket. It seems that tempest is using a flavour with root disk = 0 (flavor ref 42 which is new and called m1.nano) | |
| 15:07:23 | kashyap | mriedem: And the FixedInVersion field says: libvirt-1.2.17 | |
| 15:07:49 | BobBall | melwitt: Because we are requesting a 0-size root disk, it means that we don't try and resize, hence the success | |
| 15:08:55 | mriedem | kashyap: hmm, presumably we can do away with some workaround code then | |
| 15:09:21 | kashyap | mriedem: Yeah, definitely. Can add it to my list if you like | |
| 15:09:28 | kashyap | If you are already on it, won't step on your toes | |
| 15:09:31 | mriedem | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7495 | |
| 15:09:34 | melwitt | BobBall: yeah, just saw your reply. that's news to me that tempest uses a root_gb=0 flavor for resizes ... although, is it resizing from a 0 disk to a different 0 disk with different cpu/ram? | |
| 15:10:03 | BobBall | melwitt: Yes - that's right - the resize target appears to be 84 (I think) which is m1.micro: root_gb=0 but more RAM | |
| 15:10:20 | vdrok | mriedem: tho that error happens on spawn, not on migration now | |
| 15:11:00 | mriedem | vdrok: yeah | |
| 15:11:01 | vdrok | yeah we can do iso9660 now i think but we need to pass a new param to libvirt-python migrate method, migrate_disks, specifying cdrom device iiuc | |
| 15:11:19 | mriedem | vdrok: i think kashyap removed that | |
| 15:11:31 | melwitt | BobBall: I see. seems like that would be problematic in tempest in general and miss covering functionality in drivers if it's not doing any disk resize | |
| 15:11:34 | kashyap | mriedem: Just to note my availability, this week until next Monday, I will be "sparsely available" due to a prior committment. From Monday I should be more available. | |
| 15:11:43 | kashyap | (But I will be responsive on IRC, though) | |
| 15:12:02 | kashyap | vdrok: Since Rocky onwards | |
| 15:12:11 | melwitt | BobBall: thanks for tracking that down | |
| 15:12:13 | mriedem | vdrok: i might be thinking of this https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6910 | |
| 15:12:15 | BobBall | melwitt: This appears to be set up in devstack's tempest plugin... So yes, I believe this is a common failing | |
| 15:12:17 | kashyap | ... the libvirt migrate API needs the 'params' dict passed. | |
| 15:13:47 | vdrok | ah so most of the things are in place already | |
| 15:13:51 | kashyap | Yeah | |
| 15:14:01 | kashyap | And note: this in-flight patch will remove completely the old API | |
| 15:14:14 | melwitt | BobBall: ack. I'll look into whether we could do minimal disk resizing in tempest | |
| 15:14:40 | kashyap | vdrok: As it's not needed, and the new one variant satsified by Nova's current min libvirt version is backwards compatible w/ the older variant: | |
| 15:14:43 | kashyap | https://review.openstack.org/#/c/567258/ | |
| 15:14:44 | melwitt | BobBall: the other question I had was, is the new image handler thing the only way to fix resize for xenapi? I didn't get how that's connected | |
| 15:15:44 | BobBall | melwitt: The fix for resize is common to all image handlers for XenServer - it's not related to the new image handler and could justifiably be split out into a separate patch. We just kept it here because this is where we were doing the manual testing | |
| 15:16:55 | melwitt | BobBall: okay. I think it would be best to open a bug and fix the breakage separately rather than roll it into the blueprint work | |
| 15:17:45 | BobBall | melwitt: Bah :D I knew you were going to say that :D I'll do it now so it doesn't have to wait for tomorrow with PRC's timezone. | |
| 15:18:05 | melwitt | I'm too predictable | |
| 15:18:47 | kashyap | Unrelated: Does anyone here use "lp-tools" to file Launchpad bugs via command-line? | |
| 15:18:59 | kashyap | (Launch, manage, query, etc.) | |
| 15:19:22 | kashyap | Which reminds me ... some projects have moved to Storyboard. One day, probably Nova will too | |
| 15:19:38 | melwitt | but yeah, I'd like to get the driver fixed ASAP and not have to wait behind the feature work that's in-progress | |
| 15:21:43 | BobBall | melwitt: We were hoping that patch for the feature was literally about to get merged once the tests all pass :) | |
| 15:22:06 | mriedem | vdrok: so we used to use this utility method to call mkfs.vfat https://review.openstack.org/#/c/527510/3/nova/utils.py | |
| 15:22:41 | mriedem | i don't see anything checking to see if the path exists first or creates the path first | |
| 15:23:16 | mriedem | reading https://linux.die.net/man/8/mkfs.vfat i'm not sure if -C should be specified or not | |
| 15:24:15 | mriedem | http://manpages.ubuntu.com/manpages/xenial/man8/mkfs.vfat.8.html | |
| 15:25:38 | mriedem | unless it's complaining that /opt/stack/data/nova/instances/51858976-6a84-467a-895b-39a22b43b27f/ doesn't exist, but i'm pretty sure it does at this point | |
| 15:25:51 | mriedem | or mkfs.vfat doesn't exist | |
| 15:26:31 | mriedem | trying something | |
| 15:26:57 | vdrok | yeah I see 'unable to open <path>' when specifying non-existing path | |
| 15:28:05 | vdrok | I was testing this on pike when privsep stuff did not exist | |
| 15:28:07 | mriedem | i figured it was because we don't install dosfstools in devstack | |
| 15:33:20 | openstackgerrit | Matt Riedemann proposed openstack/nova master: DNM: test raw image live migration with vfat config drive https://review.openstack.org/567860 | |
| 15:35:07 | openstackgerrit | Bob Ball proposed openstack/nova master: XenAPI: define a new image handler to use vdi streaming https://review.openstack.org/486475 | |
| 15:35:08 | openstackgerrit | Bob Ball proposed openstack/nova master: XenAPI: Pass expected return codes to resize2fs https://review.openstack.org/568318 | |
| 15:37:03 | melwitt | BobBall: I just looked at the processutils code and it defaults check_exit_code=[0]. I wonder if we should just do that in nova/privsep/fs.py instead of making callers pass [0] if they need the default. seems like we could hit this again easily with people thinking they could pass None if they don't care to check the exit codes more specifically | |
| 15:39:19 | BobBall | melwitt: resize2fs takes a check_exit_code - we could indeed put the default there, or decide not to pass it on if None was provided? Or perhaps default to True? | |
| 15:39:45 | BobBall | (IMO check_exit_code=True makes more sense than check_exit_code=[0] | |
| 15:39:48 | melwitt | BobBall: hang on, sorry | |
| 15:41:35 | melwitt | I'm seeing there's a default check_exit_code=False in resize2fs in nova/virt/disk/api.py and I wonder why the code wasn't written to call that instead of calling the privsep module methods directly | |
| 15:43:11 | BobBall | melwitt: I believe because we don't want to run the e2fsck twice? | |
| 15:43:34 | melwitt | BobBall: yeah, and it can't accept the flags kwarg | |
| 15:43:48 | BobBall | Ah - yes. I hadn't spotted that bit :) | |
| 15:44:46 | BobBall | The second call could more easily use the general resize2fs although that would run an unwanted e2fsck again | |
| 15:44:59 | BobBall | and on large disks that can take 10s of minutes... | |
| 15:45:12 | melwitt | no, it's okay, I'm just now realizing that virt/api.py method is for ... someone else apparently | |
| 15:51:55 | BobBall | Joyfulness... | |
| 15:52:26 | melwitt | yeah :| | |
| 15:52:56 | BobBall | How about we just pass check_exit_code=True then? A good enough improvement over check_exit_code=[0]? | |
| 15:52:58 | cfriesen | so we just hit https://bugs.launchpad.net/nova/+bug/1659062 in production, and it left the instance in a state where "reset-state" and "rebuild" didn't work to recover it. | |
| 15:52:59 | openstack | Launchpad bug 1659062 in OpenStack Compute (nova) "Failed evacuations leave neutron ports on destination host" [Wishlist,Opinion] | |
| 15:53:47 | cfriesen | anyone have any ideas on the best way to handle this? fixups in init_instance() on the source? | |
| 15:54:12 | melwitt | BobBall: we could. I was just looking at the old pre-privsep code and it used to be that you could omit check_exit_code and get the default and I think that would make things easier for people, but that can be a separate improvement | |
| 15:55:12 | BobBall | melwitt: I'm happy to make the default for fs.resize2fs be check_exit_code=True and remove it from the XenAPI code if that's the preferred fix. | |
| 15:56:25 | BobBall | melwitt: As it's currently non-optional, all callers must be passing a value therefore having the default set to True won't hurt anyone | |