| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-06-16 | |||
| 07:45:43 | opendevreview | Yongli He proposed openstack/nova master: smartnic support - reject server move and suspend https://review.opendev.org/c/openstack/nova/+/779913 | |
| 07:45:45 | opendevreview | Yongli He proposed openstack/nova master: smartnic support - functional tests https://review.opendev.org/c/openstack/nova/+/780147 | |
| 07:47:06 | yonglihe | gibi, alex_xu had reviewed the smart nic patch set several rounds, and about to review another round, just after i add more functional/unit test. | |
| 07:58:38 | gibi | yonglihe: OK. I will try to look at the patches as promised on the meeting yesterday | |
| 07:59:08 | yonglihe | thanks. | |
| 08:20:50 | opendevreview | liujiong proposed openstack/nova master: Do not create attachment for old root volume https://review.opendev.org/c/openstack/nova/+/795950 | |
| 08:42:58 | gibi | lyarwood: FYI limestone is now disabled https://review.opendev.org/c/openstack/project-config/+/796590 due to mirror issues. We only saw https://review.opendev.org/c/openstack/project-config/+/796590 from limestone so that issue hopefuly removed from the active gate faliures | |
| 08:43:33 | gibi | I wanted to link to the bug https://bugs.launchpad.net/nova/+bug/1931864 | |
| 08:44:13 | opendevreview | Lee Yarwood proposed openstack/nova stable/wallaby: libvirt: Set driver_iommu when attaching virtio devices to SEV instance https://review.opendev.org/c/openstack/nova/+/796607 | |
| 08:54:41 | stephenfin | gibi: alembic is a bit different. You don't need placeholders since the migrations don't have to be applied linearly. Each migration encodes its predecessor inline, so you could backport the new migration and simply rework the other migrations either side | |
| 08:55:11 | stephenfin | I haven't spent a long time thinking about it though. We haven't backported a migration in 4 or 5 years now, iirc | |
| 08:57:21 | gibi | stephenfin: I got a pointer from slaweq how neutron did it supporting a downstream backport in https://review.opendev.org/c/openstack/neutron/+/601336/92/neutron/db/migration/alembic_migrations/versions/xena/expand/d863c3bdc0c5_add_active_allowed_address_pairs.py | |
| 08:57:40 | gibi | stephenfin: so they needed to prepare the patch on master to be conditional | |
| 08:57:57 | gibi | i.e. not to fail on if the change already applied | |
| 08:58:19 | gibi | I have to run now but we can talk about it later | |
| 08:58:36 | stephenfin | okay, no worries. We'll just have to document it, I suppose | |
| 09:12:55 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove dead code https://review.opendev.org/c/openstack/nova/+/786291 | |
| 09:12:56 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove 'nova.db.sqlalchemy.utils' https://review.opendev.org/c/openstack/nova/+/786292 | |
| 09:12:57 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove unused DB methods https://review.opendev.org/c/openstack/nova/+/786293 | |
| 09:12:57 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Use module-level imports for sqlalchemy https://review.opendev.org/c/openstack/nova/+/786295 | |
| 09:12:58 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Fold in indexes https://review.opendev.org/c/openstack/nova/+/786296 | |
| 09:12:59 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Fold in ForeignKey constraints https://review.opendev.org/c/openstack/nova/+/786297 | |
| 09:13:00 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove 'nova.db.base' module https://review.opendev.org/c/openstack/nova/+/786298 | |
| 09:13:01 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Copy docs from 'nova.db.*' to 'nova.db.sqlalchemy.*' https://review.opendev.org/c/openstack/nova/+/786299 | |
| 09:13:02 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Synchronize function signatures https://review.opendev.org/c/openstack/nova/+/786300 | |
| 09:13:02 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Clean up migration code https://review.opendev.org/c/openstack/nova/+/786301 | |
| 09:13:03 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Use module-level imports for sqlalchemy (for real) https://review.opendev.org/c/openstack/nova/+/796519 | |
| 09:19:04 | opendevreview | Lee Yarwood proposed openstack/nova stable/victoria: libvirt: Set driver_iommu when attaching virtio devices to SEV instance https://review.opendev.org/c/openstack/nova/+/796611 | |
| 09:44:39 | opendevreview | Lee Yarwood proposed openstack/nova stable/ussuri: libvirt: Set driver_iommu when attaching virtio devices to SEV instance https://review.opendev.org/c/openstack/nova/+/796618 | |
| 09:49:01 | kashyap | sean-k-mooney[m]: Hey, remind me again: do you think we can set a different display device based on guest's capabilities? E.g. if the guest has a virtio-gpu driver, use "virtio-vga" and so on | |
| 10:06:43 | stephenfin | lyarwood, gibi: bauzas is out for all of this week, right? Could I ask you to work through that DB series before it goes into merge conflict again. It's very trivial, if that helps (mostly moving things around in prep for alembic) | |
| 10:06:57 | bauzas | no, I'm here | |
| 10:07:02 | alex_xu | gibi: I reviewed few around on first three patches, since the major logic on the the thrid one. | |
| 10:07:20 | bauzas | I was just having a conflict for the upstream meeting | |
| 10:07:40 | stephenfin | bauzas: oh, sorry /o\ in that case, I might ask you to take a look also, if you can :) | |
| 10:07:59 | bauzas | stephenfin: yup, I can do it | |
| 10:08:05 | stephenfin | great, ty | |
| 10:10:08 | stephenfin | elodilles_afk: lyarwood: melwitt: Now that we have the cherry-pick check to prevent us backporting a change out-of-order, would it make sense to start approving a whole series of backports at once and relying on said check to prevent out-of-order merges? | |
| 10:29:04 | lyarwood | stephenfin: sorry was on a call, happy to help with the DB series. We can but until the parent branch changes have landed the SHAs can still change but I guess that's the case either way. I'd be happy justifying +W'ing something if the parent is already W'd tbh. | |
| 10:29:33 | stephenfin | doesn't the cherry-pick script take care of that? | |
| 10:29:37 | stephenfin | the SHAs | |
| 10:29:51 | lyarwood | Yeah checking them, it doesn't automatically update anything | |
| 10:30:12 | lyarwood | I was agreeing that we may as well ACK things all the way down and rely on the script | |
| 10:30:21 | stephenfin | ah, okay, yeah, that's my thinking | |
| 10:30:34 | bauzas | in general, we prefer to hold stable changes until master merges | |
| 10:30:42 | bauzas | as indeed SHA1s can change | |
| 10:30:45 | lyarwood | right stephenfin's point is after that on stable | |
| 10:30:53 | stephenfin | oh, I'm not arguing for approving before master is merged | |
| 10:30:55 | lyarwood | instead of waiting for each branch to merge | |
| 10:30:58 | stephenfin | we should definitely wait for that | |
| 10:31:10 | lyarwood | there is a potential race at that point stephenfin | |
| 10:31:19 | lyarwood | the older branches having less CI | |
| 10:31:20 | bauzas | stephenfin: sorry if i misunderstood, your concern is about a series ? | |
| 10:31:24 | lyarwood | will fail first | |
| 10:31:38 | lyarwood | because the newer branches haven't merged the required change | |
| 10:32:30 | stephenfin | bauzas: I'm suggesting if you have a patch backported from stable/wallaby to stable/train, we can approve all of them at once and rely on the pep8 job to ensure they go in in the correct order | |
| 10:32:34 | opendevreview | Sylvain Bauza proposed openstack/nova-specs master: Add generic mdevs to Nova https://review.opendev.org/c/openstack/nova-specs/+/792796 | |
| 10:32:54 | lyarwood | I'm fine with it, it's just going to take a few rechecks still | |
| 10:33:05 | bauzas | stephenfin: ah that | |
| 10:33:05 | stephenfin | ah yeah, there's going to be a recheck anyway | |
| 10:33:19 | lyarwood | what's life without 20 rechecks | |
| 10:33:20 | stephenfin | since the pep8 job will fail on everything older than stable/wallaby | |
| 10:33:23 | lyarwood | yup | |
| 10:33:30 | bauzas | well, in general, I'm holding approvals until the original branch merges | |
| 10:33:48 | stephenfin | but it's easy to recheck one by one | |
| 10:33:50 | bauzas | as they can be races | |
| 10:33:55 | bauzas | there* | |
| 10:34:15 | stephenfin | certainly easier than asking stable cores every few days to review the latest stable branch | |
| 10:34:56 | stephenfin | bauzas: again, no issues holding off on approving (or even reviewing) backports until the master change has landed | |
| 10:35:19 | stephenfin | but the pep8 job means that e.g. a stable/train change simply can't land before the stable/ussuri one now | |
| 10:35:23 | stephenfin | even if +Wd | |
| 10:35:29 | bauzas | oh, I see your point | |
| 10:35:38 | bauzas | well, then sure | |
| 10:36:44 | stephenfin | I just want to avoid having to continuously poll stable cores as each stable branch lands, particularly since I'm typically going back to Train (so that's four instances of polling, at a minimum :)) | |
| 10:36:59 | lyarwood | the only issue I have personally is that it breaks my review dashboards | |
| 10:37:13 | lyarwood | as they rely on reviews being +1'd | |
| 10:37:35 | lyarwood | but I guess someone is asking us to review a given topic here making it easier | |
| 10:37:41 | lyarwood | and/or change-id | |
| 10:38:20 | stephenfin | yeah, I'd like it if we could use something other than the verified label for this | |
| 10:39:51 | stephenfin | like a Parent-Merged label, without which zuul wouldn't merge the patch (so like Verified in that way). I don't know how hard that is though. I don't know if a zuul job can set a label other than verified | |
| 10:40:40 | stephenfin | however, as things stand, the dashboard is already broken so nothing has changed, right lyarwood? | |
| 10:40:52 | stephenfin | because of the pep8 fail | |
| 10:41:05 | lyarwood | yeah correct things are hidden | |
| 10:41:45 | lyarwood | tinyurl.com/f9y6vr6d for example | |
| 10:42:11 | lyarwood | tbh I could just change them to drop the +1 requirement | |
| 10:42:16 | lyarwood | but then it gets a little mad | |
| 10:42:49 | stephenfin | I can imagine | |
| 10:43:07 | lyarwood | I still use https://review.opendev.org/q/project:openstack/nova+branch:%5Estable/.*+status:open from time to time to check everything | |
| 10:44:13 | stephenfin | oh, zuul itself | |
| 10:44:19 | stephenfin | https://opendev.org/zuul/zuul/src/branch/master/doc/source/examples/pipelines/gerrit-reference-pipelines.yaml | |
| 10:44:30 | stephenfin | so we'd need a new pipeline | |
| 10:44:37 | lyarwood | it would be nice to have something else the script could set tbh | |
| 10:45:51 | stephenfin | the other option is to have a separate "CI" running that particular job, so you could filter on that instead | |
| 10:46:01 | stephenfin | i.e. -1 by $PEP8_CI | |
| 10:46:19 | stephenfin | again, no idea how difficult that is. I suspect that name is global to the deployment | |
| 10:48:26 | stephenfin | oh, we could just move it to a separate job in the gate queue? | |
| 10:48:34 | stephenfin | instead of the check queue | |
| 10:51:32 | lyarwood | yeah I'm cool with that | |
| 10:51:55 | lyarwood | well it would have to be non-voting, slightly defeating the purpose of the script | |
| 10:52:57 | stephenfin | would it? The check job would now report +1 but the gate job would -2 if the parent wasn't merged | |