| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-12-15 | |||
| 16:52:27 | bauzas | 3/ iterate over every ignored rule | |
| 16:52:33 | gibi | bauzas: if you are after to split the hacking version change and the introdcution of new rules into separate patches then fine, but the current patch is not too big too | |
| 16:52:50 | gmann | bauzas: but this change with not-ignoring does not change much code | |
| 16:52:51 | stephenfin | if there were a significant amount of changes then I'd agree, but it doesn't seem worth the effort here | |
| 16:53:06 | bauzas | gibi: the problem is that we are silently changing placement without really explaining which hacking errors we wanted to fixc | |
| 16:53:29 | gibi | bauzas: sure, we can add a list of new rules applied to the commit message | |
| 16:53:40 | bauzas | and I don't think it's hard to just split this change in twice if you really want to unignore E128 and all the other rules | |
| 16:54:41 | gibi | OK, I'm fine in both ways: a) add a bit more text the commit message about the new rules that required the code change OR b) split the current patch into two | |
| 16:54:49 | gmann | giving it unblock the gate I preferred to move the code to latest hacking which is easy than doing one by one. | |
| 16:55:02 | gibi | gmann: agree, new hacknig is better | |
| 16:55:27 | gmann | I can add checks fix in commit msg. | |
| 16:55:28 | stephenfin | I don't think E128 is a big issue. W503/W504 seem to be the bulk of it. We could drop one of those, seeing as they conflict | |
| 16:55:49 | gmann | yeah W504 is ignored now | |
| 16:56:12 | stephenfin | sweet, then I don't care :) | |
| 16:56:39 | gmann | https://review.opendev.org/c/openstack/placement/+/766994/9/tox.ini | |
| 16:57:15 | bauzas | phew ok | |
| 16:57:25 | bauzas | I just feel we're overcomplicating | |
| 16:57:52 | bauzas | bumping hacking and providing the right ignore list will allow the gate to pass without needing to touch code | |
| 16:58:13 | bauzas | unless I'm missing something obvious | |
| 16:58:46 | bauzas | my take is, gmann had to touch code instead of amending the ignore list in order to fix the gate, I just feel it was unnecessary | |
| 16:58:51 | stephenfin | bauzas: not quite; a couple of the checks have been improved in newer versions of flake8 etc. To ignore those you'd have to disable the check and actually regress | |
| 16:59:13 | bauzas | stephenfin: why then nova isn't a problem ? | |
| 16:59:22 | stephenfin | because we fixed them there | |
| 16:59:27 | stephenfin | example: https://review.opendev.org/c/openstack/placement/+/766994/9/placement/conf/database.py | |
| 16:59:49 | stephenfin | that extra newline was (mistakenly) not required in an old version of flake8 | |
| 17:00:27 | bauzas | for the extra newline, OK | |
| 17:00:39 | bauzas | for the continuous indentation, that's still there | |
| 17:00:55 | bauzas | so, ok, I mixed things | |
| 17:00:56 | gmann | nova is on 3.1.0. i could move placement too to that but 4.0.0 is already out to moving to it is easy. | |
| 17:01:26 | bauzas | gmann has to fix single lines within modules, but can just continue to leave uncontinuous indentation if he wishes | |
| 17:01:49 | gmann | anyways code is up and green so merging it is faster than splitting the patch now | |
| 17:02:53 | bauzas | well, I'm OK to remove my -1 for the sake of merging | |
| 17:03:01 | bauzas | but honestly, this is confisuing | |
| 17:04:12 | gmann | bauzas: how about if i explain those check fixes in cmt msg ? | |
| 17:04:43 | bauzas | I'm just saying we're again touching code for the beauty of our eyes and we're overcomplicating things, exactly like we did a couple of times in nova | |
| 17:04:50 | bauzas | but meh, I bail out | |
| 17:05:01 | bauzas | so, stephenfin can look at the change, I removed my -1 | |
| 17:05:18 | bauzas | the +2 from gibi is there, so... | |
| 17:05:24 | gibi | yeah | |
| 17:05:34 | gibi | honestly I don't feel this as such a big issue | |
| 17:05:41 | gibi | but that jsutme | |
| 17:05:46 | gibi | just me | |
| 17:05:55 | gmann | bauzas: you seems do not like hacking checks much :) | |
| 17:06:19 | bauzas | it's more or less a matter of touching production code for the sake of ourselves being pedantic :) | |
| 17:06:28 | bauzas | but okay | |
| 17:07:15 | bauzas | gmann: well, I'm just pragmatic about things that work | |
| 17:07:26 | bauzas | but fair enough | |
| 17:08:18 | gmann | I see your point. | |
| 17:20:32 | stephenfin | ta, done | |
| 17:33:27 | stephenfin | gmann: Could you look at https://review.opendev.org/c/openstack/nova/+/765798/ at some point? | |
| 17:33:30 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Merge 'test_hypervisor_status' into 'test_hypervisors' https://review.opendev.org/c/openstack/nova/+/764039 | |
| 17:33:31 | openstackgerrit | Stephen Finucane proposed openstack/nova master: api: Drop statistics-style fields from os-hypervisors https://review.opendev.org/c/openstack/nova/+/764040 | |
| 17:33:31 | openstackgerrit | Stephen Finucane proposed openstack/nova master: WIP: Change default policy for '/os-hypervisors' https://review.opendev.org/c/openstack/nova/+/765798 | |
| 17:33:33 | stephenfin | I'm not sure if I'm tackling that correctly | |
| 17:33:43 | stephenfin | Also, resolved your comment on https://review.opendev.org/c/openstack/nova/+/764039 | |
| 17:33:52 | gmann | stephenfin: sure, checking | |
| 17:36:29 | stephenfin | lyarwood: you'd probably be interested in this at some point https://bugs.launchpad.net/nova/+bug/1906266 | |
| 17:36:29 | openstack | Launchpad bug 1906266 in OpenStack Compute (nova) "After upgrade: "libvirt.libvirtError: Requested operation is not valid: format of backing image %s of image %s was not specified"" [Undecided,New] | |
| 17:37:04 | lyarwood | stephenfin: didn't we fix that a while ago? | |
| 17:37:31 | stephenfin | yup, they're saying the fix is incomplete | |
| 17:37:52 | gmann | stephenfin: yeah that is right way, as system scope is disable by default and we still have old legacy rule in deprecated rule so we are good, - https://review.opendev.org/c/openstack/nova/+/765798/2/nova/policies/hypervisors.py#b37 | |
| 17:38:08 | lyarwood | oh for existing images you actually need to add the backing format in, there's nothing n-cpu can do iirc | |
| 17:38:33 | gmann | stephenfin: no microversion needed for policy default, but please add releasenotes for that. I am checking other patches in that series | |
| 17:38:45 | stephenfin | gmann++ Great, thanks :) | |
| 17:38:54 | lyarwood | tbh I thought this was relaxed | |
| 17:39:21 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: WIP: Add a routed networks scheduler pre-filter https://review.opendev.org/c/openstack/nova/+/749068 | |
| 17:39:35 | stephenfin | maybe it was; they don't give any details about the versions they're using | |
| 17:43:46 | stephenfin | lyarwood: https://github.com/libvirt/libvirt/commit/ae9e6c2a2b75d958995c661f7bb64ed4353a6404 | |
| 17:44:23 | stephenfin | the few patches I grepped from https://www.redhat.com/archives/libvir-list/2020-February/msg00616.html appear to be in v6.1.0 | |
| 17:46:36 | lyarwood | ah! | |
| 17:46:46 | lyarwood | the error is still there but we don't get there now | |
| 17:47:02 | stephenfin | that's my reading of it too, yeah | |
| 17:50:25 | ganso | Hi nova folks! Yesterday I logged bug https://bugs.launchpad.net/nova/+bug/1908133 and it has been closed as a duplicated. I'd like to discuss what possible workarounds there are (there are no mentions of workarounds in the duplicate entry). In my "steps to reproduce" I took 2 different approaches, the second one being relying on placement to control the shared allocations, but it is quite 100%, as I am not sure I am configuring it correctly. The | |
| 17:50:25 | openstack | Launchpad bug 1522307 in OpenStack Compute (nova) "duplicate for #1908133 Disk usage not work for shared storage" [Medium,Confirmed] | |
| 17:50:25 | ganso | instructions in the placement docs are not very detailed to achieve what I need. Is anyone familiar with this use case or can think of any possible workarounds? | |
| 18:06:46 | openstackgerrit | Stephen Finucane proposed openstack/nova master: WIP: Change default policy for '/os-hypervisors' https://review.opendev.org/c/openstack/nova/+/765798 | |
| 19:13:42 | jawad_axd | Hi folks! Error 43, windows 10 with gpu passthrough. Added img_hide_hypervisor_id=True to image property. Is there anything I need to do to load nvidia drivers in windows instance? | |
| 19:14:10 | jawad_axd | *anything else | |
| 19:48:50 | openstackgerrit | Ghanshyam proposed openstack/nova master: Reuse code from oslo lib for JSON policy migration https://review.opendev.org/c/openstack/nova/+/763487 | |
| 19:49:44 | openstackgerrit | Ghanshyam proposed openstack/nova master: Reuse code from oslo lib for JSON policy migration https://review.opendev.org/c/openstack/nova/+/763487 | |
| 20:02:16 | openstackgerrit | Elod Illes proposed openstack/nova stable/ussuri: [stable-only] fix lower-constraints and disable qos resize https://review.opendev.org/c/openstack/nova/+/766738 | |
| 20:44:11 | openstackgerrit | Merged openstack/nova master: libvirt: make cross cell resize spawn from snapshot image https://review.opendev.org/c/openstack/nova/+/765561 | |
| #openstack-nova - 2020-12-16 | |||
| 00:13:33 | openstackgerrit | Ghanshyam proposed openstack/placement master: Remove deprecated [placement]/policy_file config option https://review.opendev.org/c/openstack/placement/+/764409 | |
| 00:13:57 | openstackgerrit | Ghanshyam proposed openstack/placement master: [goal] Deprecate the JSON formatted policy file https://review.opendev.org/c/openstack/placement/+/764394 | |
| 00:15:14 | gmann | gibi: bauzas stephenfin please check these two placement patches also. for json->yaml migration community goal. https://review.opendev.org/c/openstack/placement/+/764409 https://review.opendev.org/c/openstack/placement/+/764394 | |
| 00:16:04 | gmann | 764409 is to remove the deprecated policy_file config option which is needed for this migration | |
| 00:56:56 | brinzhang | gibi, bauzas, sean-k-mooney: Since Microversion 2.77, we support specifying an avaiability_zone to unshleve an instance, it allowed {'unshelve': null} and {'unshelve': {'availability_zone': |
|
| 00:57:55 | brinzhang | gibi, bauzas, sean-k-mooney, gmann: this is the record https://review.opendev.org/c/openstack/nova/+/663851/13/nova/api/openstack/compute/schemas/servers.py#659 | |
| 00:59:50 | gmann | brinzhang: yeah as this action API need specific request body so disallow empty make sense. for other action API there is no specific request body so allowing null or anything is what we have during legacy way | |
| 00:59:55 | brinzhang | and we note that in https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/shelve.py#L28-L31 | |
| 01:02:15 | brinzhang | gmann: yes, I think gibi and bauzas's concern in the docs description, because of the availability_zone is optional, but we allowed request {'unshelve': {'availability_zone': <string>}}, if we dont chose the availability_zone, it will be {'unshelve': {} | |
| 01:03:11 | gmann | ok and we only allow {'unshelve': null} in that case | |
| 01:04:45 | gmann | brinzhang: may be we can update it in api-ref too https://docs.openstack.org/api-ref/compute/?expanded=unshelve-restore-shelved-server-unshelve-action-detail | |
| 01:04:54 | brinzhang | yes, I think this is reasonable too | |
| 01:05:00 | gmann | saying 2.77 onwards | |
| 01:05:44 | brinzhang | gmann: ok, let submit a patch to add a Note for the availability_zone | |
| 01:06:07 | brinzhang | s/let/ let me | |
| 01:06:13 | gmann | +1 | |
| 01:06:46 | brinzhang | Will do, and pls review it later | |
| 01:08:04 | gmann | thanks | |