[Starlingx-discuss] V1 Review Request: Story 29990: libvirt and qemu patch reduction
Hi Dean and other interested parties, I've finished reducing the patches on libvirt and qemu. I was able to get rid of virtually all of the RHEL patches, replacing them with just a minor "support for running on CentOS" patch or two. This will make our lives a lot easier moving to newer versions. qemu went from 97 patches down to 14, and libvirt from 23 to 13. The STX patches themselves required very little rework, this was mostly a testing exercise in the container realm with things changing frequently, making it quite challenging. This passed our regular sanity test run, and we subsequently did a full regression test run. All of the interesting failures in the regression run were explainable via existing bug reports. I feel reasonably confident that this isn't going to break anything, but, hey, famous last words and all that. Once you're satisfied with the review, I'll issue pull requests. Once you've pulled and created new branches, I'll follow up with the two commits, one referring to the new branches in the manifest, and the other with minor changes to the qemu spec file in the stx-integ repo. Linked so they both go in together. One issue concerns me a bit, and that is the tis patch number. It starts counting from the last upstream commit, and with me removing patches, it is now lower than it used to be. If this is a real concern I could just add a fixed 100 to the gitrevcount in both qemu and libvirt build_data files, guaranteeing package versions will not collide with ones in the past. Your thoughts? https://github.com/jsomervi/stx-qemu/commits/v3.0.0-patch-reduction-1 https://github.com/jsomervi/stx-libvirt-1/commits/v4.7.0-patch-reduction-1 Thanks, -Jim
On 2019-04-18 11:21 a.m., Jim Somerville wrote:
Hi Dean and other interested parties,
I've finished reducing the patches on libvirt and qemu. I was able to get rid of virtually all of the RHEL patches, replacing them with just a minor "support for running on CentOS" patch or two. This will make our lives a lot easier moving to newer versions. qemu went from 97 patches down to 14, and libvirt from 23 to 13. The STX patches themselves required very little rework, this was mostly a testing exercise in the container realm with things changing frequently, making it quite challenging.
This passed our regular sanity test run, and we subsequently did a full regression test run. All of the interesting failures in the regression run were explainable via existing bug reports. I feel reasonably confident that this isn't going to break anything, but, hey, famous last words and all that.
Once you're satisfied with the review, I'll issue pull requests. Once you've pulled and created new branches, I'll follow up with the two commits, one referring to the new branches in the manifest, and the other with minor changes to the qemu spec file in the stx-integ repo. Linked so they both go in together.
One issue concerns me a bit, and that is the tis patch number. It starts counting from the last upstream commit, and with me removing patches, it is now lower than it used to be. If this is a real concern I could just add a fixed 100 to the gitrevcount in both qemu and libvirt build_data files, guaranteeing package versions will not collide with ones in the past. Your thoughts?
https://github.com/jsomervi/stx-qemu/commits/v3.0.0-patch-reduction-1 https://github.com/jsomervi/stx-libvirt-1/commits/v4.7.0-patch-reduction-1
Link to the story: https://storyboard.openstack.org/#!/story/2005212 -Jim
Thanks,
-Jim
Hi Jim, This looks like great work and a strong effort to reduce patches, thanks! On 4/18/19 8:21 AM, Jim Somerville wrote:
Hi Dean and other interested parties,
I've finished reducing the patches on libvirt and qemu. I was able to get rid of virtually all of the RHEL patches, replacing them with just a minor "support for running on CentOS" patch or two. This will make our lives a lot easier moving to newer versions. qemu went from 97 patches down to 14, and libvirt from 23 to 13. The STX patches themselves required very little rework, this was mostly a testing exercise in the container realm with things changing frequently, making it quite challenging.
I have not yet reviewed your repos, but want to know if you have given thoughts to upstreaming any of the remaining patches to qemu or libvirt as appropriate?
This passed our regular sanity test run, and we subsequently did a full regression test run. All of the interesting failures in the regression run were explainable via existing bug reports. I feel reasonably confident that this isn't going to break anything, but, hey, famous last words and all that.
Once you're satisfied with the review, I'll issue pull requests. Once you've pulled and created new branches, I'll follow up with the two commits, one referring to the new branches in the manifest, and the other with minor changes to the qemu spec file in the stx-integ repo. Linked so they both go in together.
Is there a reason to not issue the pull requests directly to the stx-staging repos now if your ready?
One issue concerns me a bit, and that is the tis patch number. It starts counting from the last upstream commit, and with me removing patches, it is now lower than it used to be. If this is a real concern I could just add a fixed 100 to the gitrevcount in both qemu and libvirt build_data files, guaranteeing package versions will not collide with ones in the past. Your thoughts?
At the last F2F in Chandler the discussion about TIS_PATCH_VER determined that it was a sequential version number, and not a count of patches. If this was a rebase with a version change, then you would start at 1 again, but since this is a rebase without, you should bump TIS_PATCH_VER by 1.
https://github.com/jsomervi/stx-qemu/commits/v3.0.0-patch-reduction-1 https://github.com/jsomervi/stx-libvirt-1/commits/v4.7.0-patch-reduction-1
Thanks Sau!
Thanks,
-Jim
On 2019-04-18 12:48 p.m., Saul Wold wrote:
Hi Jim,
This looks like great work and a strong effort to reduce patches, thanks!
Thanks Saul, appreciated.
On 4/18/19 8:21 AM, Jim Somerville wrote:
Hi Dean and other interested parties,
I've finished reducing the patches on libvirt and qemu. I was able to get rid of virtually all of the RHEL patches, replacing them with just a minor "support for running on CentOS" patch or two. This will make our lives a lot easier moving to newer versions. qemu went from 97 patches down to 14, and libvirt from 23 to 13. The STX patches themselves required very little rework, this was mostly a testing exercise in the container realm with things changing frequently, making it quite challenging.
I have not yet reviewed your repos, but want to know if you have given thoughts to upstreaming any of the remaining patches to qemu or libvirt as appropriate?
I haven't given it much thought. Not being the actual author of most of them, I don't feel all that qualified to embark on the sales job of getting them in upstream.
This passed our regular sanity test run, and we subsequently did a full regression test run. All of the interesting failures in the regression run were explainable via existing bug reports. I feel reasonably confident that this isn't going to break anything, but, hey, famous last words and all that.
Once you're satisfied with the review, I'll issue pull requests. Once you've pulled and created new branches, I'll follow up with the two commits, one referring to the new branches in the manifest, and the other with minor changes to the qemu spec file in the stx-integ repo. Linked so they both go in together.
Is there a reason to not issue the pull requests directly to the stx-staging repos now if your ready?
No reason other than I just wanted folks to have a chance to look/review before I pestered the stx-staging repo controllers with pull requests.
One issue concerns me a bit, and that is the tis patch number. It starts counting from the last upstream commit, and with me removing patches, it is now lower than it used to be. If this is a real concern I could just add a fixed 100 to the gitrevcount in both qemu and libvirt build_data files, guaranteeing package versions will not collide with ones in the past. Your thoughts?
At the last F2F in Chandler the discussion about TIS_PATCH_VER determined that it was a sequential version number, and not a count of patches. If this was a rebase with a version change, then you would start at 1 again, but since this is a rebase without, you should bump TIS_PATCH_VER by 1.
The way it is currently done in libvirt/qemu is via the GITREVCOUNT mechanism. This change I'm making is essentially just rewriting a repo branch, and doesn't include an underlying version change to the code such as 3.0.0 to 3.0.1. I could abandon GITREVCOUNT and just set TIS_PATCH_VER to a version manually, 98 for qemu and 24 for libvirt. -Jim
https://github.com/jsomervi/stx-qemu/commits/v3.0.0-patch-reduction-1 https://github.com/jsomervi/stx-libvirt-1/commits/v4.7.0-patch-reduction-1
Thanks Sau!
Thanks,
-Jim
On Thu, Apr 18, 2019 at 10:22 AM Jim Somerville <jim.somerville@windriver.com> wrote:
I've finished reducing the patches on libvirt and qemu. I was able to get rid of virtually all of the RHEL patches, replacing them with just a minor "support for running on CentOS" patch or two. This will make our lives a lot easier moving to newer versions. qemu went from 97 patches down to 14, and libvirt from 23 to 13. The STX patches themselves required very little rework, this was mostly a testing exercise in the container realm with things changing frequently, making it quite challenging.
Awesome!
Once you're satisfied with the review, I'll issue pull requests. Once you've pulled and created new branches, I'll follow up with the two commits, one referring to the new branches in the manifest, and the other with minor changes to the qemu spec file in the stx-integ repo. Linked so they both go in together.
It looks like these are on the same upstream base version, correct? We'll have to add a suffix but that isn't a problem. I'll use '-N' for that so it doesn't look like part of the upstream version (we used '.N' for the Nova stable branch in stx-nova, /me kicks self). I have created stx-qemu/stx/v3.0.0-1 and stx-libvirt/stx/v4.7.0-1. Fire away with the PRs.
One issue concerns me a bit, and that is the tis patch number. It starts counting from the last upstream commit, and with me removing patches, it is now lower than it used to be. If this is a real concern I could just add a fixed 100 to the gitrevcount in both qemu and libvirt build_data files, guaranteeing package versions will not collide with ones in the past. Your thoughts?
Is this that number that is supposed to be based on the patch count? I think we should get rid of that idea and just increment it every time it need to be incremented. Overloading things like that just makes everything more brittle. Also... I still want to encourage folks to do dev work in the primary places (Gerrit and starlngx-staging on GitHub), this is a very important part of The Four Opens[0] that is fundamental to being part of the OpenStack Foundation. In this case it isn't so much development as cleanup but it still counts as working in the open. Updating a WIP PR is just as doable as a WIP Gerrit review as things progress. And that lets people find the work without having to know beforehand where it is, even as in this case it was on GitHub anyway. [I am trying to not pick on Jim specifically here but I did recently say something in a meeting about this particular work and I thought this was a good place to expand on why I feel so strongly on this topic. These principles are fundamental to StarlingX being accepted as an OpenStack Foundation project and we _will_ be judged on things like this. We already are (informally) in fact...] dt [0] The Four Opens: https://governance.openstack.org/tc/reference/opens.html -- Dean Troyer dtroyer@gmail.com
On 2019-04-18 4:04 p.m., Dean Troyer wrote:
On Thu, Apr 18, 2019 at 10:22 AM Jim Somerville <jim.somerville@windriver.com> wrote:
I've finished reducing the patches on libvirt and qemu. I was able to get rid of virtually all of the RHEL patches, replacing them with just a minor "support for running on CentOS" patch or two. This will make our lives a lot easier moving to newer versions. qemu went from 97 patches down to 14, and libvirt from 23 to 13. The STX patches themselves required very little rework, this was mostly a testing exercise in the container realm with things changing frequently, making it quite challenging.
Awesome!
Thanks.
Once you're satisfied with the review, I'll issue pull requests. Once you've pulled and created new branches, I'll follow up with the two commits, one referring to the new branches in the manifest, and the other with minor changes to the qemu spec file in the stx-integ repo. Linked so they both go in together.
It looks like these are on the same upstream base version, correct?
Yes, same upstream base.
We'll have to add a suffix but that isn't a problem. I'll use '-N' for that so it doesn't look like part of the upstream version (we used '.N' for the Nova stable branch in stx-nova, /me kicks self). I have created stx-qemu/stx/v3.0.0-1 and stx-libvirt/stx/v4.7.0-1. Fire away with the PRs.
Will do.
One issue concerns me a bit, and that is the tis patch number. It starts counting from the last upstream commit, and with me removing patches, it is now lower than it used to be. If this is a real concern I could just add a fixed 100 to the gitrevcount in both qemu and libvirt build_data files, guaranteeing package versions will not collide with ones in the past. Your thoughts?
Is this that number that is supposed to be based on the patch count?
Yes.
I think we should get rid of that idea and just increment it every time it need to be incremented. Overloading things like that just makes everything more brittle.
Agreed. I will start the number for both qemu and libvirt at 100 so there is no chance of a collision with an earlier released version of either package. -Jim
Also...
I still want to encourage folks to do dev work in the primary places (Gerrit and starlngx-staging on GitHub), this is a very important part of The Four Opens[0] that is fundamental to being part of the OpenStack Foundation. In this case it isn't so much development as cleanup but it still counts as working in the open. Updating a WIP PR is just as doable as a WIP Gerrit review as things progress. And that lets people find the work without having to know beforehand where it is, even as in this case it was on GitHub anyway.
[I am trying to not pick on Jim specifically here but I did recently say something in a meeting about this particular work and I thought this was a good place to expand on why I feel so strongly on this topic. These principles are fundamental to StarlingX being accepted as an OpenStack Foundation project and we _will_ be judged on things like this. We already are (informally) in fact...]
dt
[0] The Four Opens: https://governance.openstack.org/tc/reference/opens.html
On 2019-04-18 4:04 p.m., Dean Troyer wrote:
On Thu, Apr 18, 2019 at 10:22 AM Jim Somerville <jim.somerville@windriver.com> wrote:
I've finished reducing the patches on libvirt and qemu. I was able to get rid of virtually all of the RHEL patches, replacing them with just a minor "support for running on CentOS" patch or two. This will make our lives a lot easier moving to newer versions. qemu went from 97 patches down to 14, and libvirt from 23 to 13. The STX patches themselves required very little rework, this was mostly a testing exercise in the container realm with things changing frequently, making it quite challenging.
Awesome!
Once you're satisfied with the review, I'll issue pull requests. Once you've pulled and created new branches, I'll follow up with the two commits, one referring to the new branches in the manifest, and the other with minor changes to the qemu spec file in the stx-integ repo. Linked so they both go in together.
It looks like these are on the same upstream base version, correct? We'll have to add a suffix but that isn't a problem. I'll use '-N' for that so it doesn't look like part of the upstream version (we used '.N' for the Nova stable branch in stx-nova, /me kicks self). I have created stx-qemu/stx/v3.0.0-1 and stx-libvirt/stx/v4.7.0-1. Fire away with the PRs.
Hi Dean, Saul finished approving the new branch contents, so they're ready to merge into your newly created -1 branch versions, assuming you're good with them as well. -Jim
One issue concerns me a bit, and that is the tis patch number. It starts counting from the last upstream commit, and with me removing patches, it is now lower than it used to be. If this is a real concern I could just add a fixed 100 to the gitrevcount in both qemu and libvirt build_data files, guaranteeing package versions will not collide with ones in the past. Your thoughts?
Is this that number that is supposed to be based on the patch count? I think we should get rid of that idea and just increment it every time it need to be incremented. Overloading things like that just makes everything more brittle.
Also...
I still want to encourage folks to do dev work in the primary places (Gerrit and starlngx-staging on GitHub), this is a very important part of The Four Opens[0] that is fundamental to being part of the OpenStack Foundation. In this case it isn't so much development as cleanup but it still counts as working in the open. Updating a WIP PR is just as doable as a WIP Gerrit review as things progress. And that lets people find the work without having to know beforehand where it is, even as in this case it was on GitHub anyway.
[I am trying to not pick on Jim specifically here but I did recently say something in a meeting about this particular work and I thought this was a good place to expand on why I feel so strongly on this topic. These principles are fundamental to StarlingX being accepted as an OpenStack Foundation project and we _will_ be judged on things like this. We already are (informally) in fact...]
dt
[0] The Four Opens: https://governance.openstack.org/tc/reference/opens.html
On Fri, Apr 26, 2019 at 2:52 PM Jim Somerville <jim.somerville@windriver.com> wrote:
Saul finished approving the new branch contents, so they're ready to merge into your newly created -1 branch versions, assuming you're good with them as well.
Done. Thanks Jim dt -- Dean Troyer dtroyer@gmail.com
The associated starlingx updates have now merged. -----Original Message----- From: Dean Troyer [mailto:dtroyer@gmail.com] Sent: Friday, April 26, 2019 4:42 PM To: Somerville, Jim Cc: Xie, Cindy; Rowsell, Brent; Khalil, Ghada; Saul Wold; Hu, Yong; Liu, ZhipengS; starlingx Subject: Re: [Starlingx-discuss] V1 Review Request: Story 29990: libvirt and qemu patch reduction On Fri, Apr 26, 2019 at 2:52 PM Jim Somerville <jim.somerville@windriver.com> wrote:
Saul finished approving the new branch contents, so they're ready to merge into your newly created -1 branch versions, assuming you're good with them as well.
Done. Thanks Jim dt -- Dean Troyer dtroyer@gmail.com _______________________________________________ Starlingx-discuss mailing list Starlingx-discuss@lists.starlingx.io http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
participants (4)
-
Dean Troyer
-
Jim Somerville
-
Penney, Don
-
Saul Wold