[vdsm][oVirt 4.0] trimming down vm.py, stage 1
Hi all, as many already know, we have a problem in the vdsm/virt/* area with the size of vm.py. As in today's master (SHA1: ed7618102e4e3aedf859adb1084eecded8e5158d), vm.py weights like: 1082 11:28:09 fromani@musashi ~/Projects/upstream/vdsm $ ls -lh vdsm/virt/vm.py -rw-rw-r--. 1 fromani fromani 209K Nov 24 11:15 vdsm/virt/vm.py 1083 11:30:22 fromani@musashi ~/Projects/upstream/vdsm $ wc vdsm/virt/vm.py 5202 17125 213026 vdsm/virt/vm.py I'd like to renew my commitment to bring this monster down to a sane size. I don't think we can make it in one go. We should tackle this monster task in stages. We have roughly five weeks left until the end of the year. Sounds like a good timespan for the first stage, and I'm aiming for go down under the 4000 line mark. So we have roughly 1200 lines to eliminate somehow. This is the initial plan I'm thinking off, in expected increasing difficulty level, from easies to hardest: 1. LiveMergeCleanupThread: Chance to break things: minimal - roughly 120 lines (10% of the goal) - can be moved verbatim (cut/paste) elsewhere, like vdsm/virt/livemerge.py, fixing only import paths. 2. the findDrive* family of functions: Chance to break things: minimal/very low - roughly 50 lines (~4% of the goal) - can be moved with minimal changes elsewhere, like vdsm/virt/storage/????.py, 3. the getConf* faimily of functions: Chance to break things: medium-to-high in the 3.x branch, low on 4.x because we drop backward compat. - roughly 110 lines (~9% of the goal) - just drop them for 4.0! - alternatively, need to figure out a proper place, maybe a new module? 4. the Devices/VM setup bits All scatthered through vm.py (e.g. _buildDomainXML, DeviceMapping table) Chance to break things: medium-to-low, depends on individual patches - all summed up, roughly 200 lines (~18% of the goal) - need serious cleanup - no definite plan - mostly is cut/paste or transform Vm methods into functions 5. the getUnderlying* familiy of functions: Chance to break things: low (maybe medium on some cases, to lean on the safe side) - roughly 550 lines (~46% of the goal) - easy to test (just run vms!) - can be moved in the devices/ subpackage with some changes. - require further work in the area to properly integrate this code in the device objects - more work is planned in this area, including move from minidom to ElementTree So far, I've gathered ~1030 lines, so I'm short of 170 lines to reach the goal, but I'm confident I can find something more. The plan now is to start from the easy parts *and* in background with #4, which requires way more work than everything else. Thoughts, objections and suggestions all welcome, especially from storage people, because first items in my list affects them. Thanks and bests, -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani
----- Original Message -----
From: "Francesco Romani" <fromani@redhat.com> To: devel@ovirt.org Sent: Tuesday, November 24, 2015 11:51:10 AM Subject: [ovirt-devel] [vdsm][oVirt 4.0] trimming down vm.py, stage 1
[...]
We have roughly five weeks left until the end of the year. Sounds like a good timespan for the first stage, and I'm aiming for go down under the 4000 line mark.
So we have roughly 1200 lines to eliminate somehow.
This is the initial plan I'm thinking off, in expected increasing difficulty level, from easies to hardest:
And here's the first status update
1. LiveMergeCleanupThread: Chance to break things: minimal - roughly 120 lines (10% of the goal) - can be moved verbatim (cut/paste) elsewhere, like vdsm/virt/livemerge.py, fixing only import paths.
Patch ready and not published. I was thinking about moving more code in the newly created livemerge.py, but not sure about the partitioning. I'll just push the patch so we can discuss there.
2. the findDrive* family of functions: Chance to break things: minimal/very low - roughly 50 lines (~4% of the goal) - can be moved with minimal changes elsewhere, like vdsm/virt/storage/????.py,
Done in https://gerrit.ovirt.org/#/c/49209/
3. the getConf* faimily of functions: Chance to break things: medium-to-high in the 3.x branch, low on 4.x because we drop backward compat. - roughly 110 lines (~9% of the goal) - just drop them for 4.0! - alternatively, need to figure out a proper place, maybe a new module?
Just dropped in https://gerrit.ovirt.org/#/c/49173/ The patch is quite large but simple, in the end all the code is guarded by a couple of bug if()s. The only concern is we break compatibility with older Engines, so maybe merge this later in the development cycles. Except for maybe some glitches, I don't expect further work here (of course except verification!)
4. the Devices/VM setup bits All scatthered through vm.py (e.g. _buildDomainXML, DeviceMapping table) Chance to break things: medium-to-low, depends on individual patches - all summed up, roughly 200 lines (~18% of the goal) - need serious cleanup - no definite plan - mostly is cut/paste or transform Vm methods into functions
Not tackled yet
5. the getUnderlying* familiy of functions: Chance to break things: low (maybe medium on some cases, to lean on the safe side) - roughly 550 lines (~46% of the goal) - easy to test (just run vms!) - can be moved in the devices/ subpackage with some changes. - require further work in the area to properly integrate this code in the device objects - more work is planned in this area, including move from minidom to ElementTree
Not tackled yet Bests, -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani
----- Original Message -----
From: "Francesco Romani" <fromani@redhat.com> To: devel@ovirt.org Sent: Monday, November 30, 2015 10:49:58 AM Subject: Re: [ovirt-devel] [vdsm][oVirt 4.0] trimming down vm.py, stage 1
----- Original Message -----
From: "Francesco Romani" <fromani@redhat.com> To: devel@ovirt.org Sent: Tuesday, November 24, 2015 11:51:10 AM Subject: [ovirt-devel] [vdsm][oVirt 4.0] trimming down vm.py, stage 1
[...]
We have roughly five weeks left until the end of the year. Sounds like a good timespan for the first stage, and I'm aiming for go down under the 4000 line mark.
So we have roughly 1200 lines to eliminate somehow.
This is the initial plan I'm thinking off, in expected increasing difficulty level, from easies to hardest:
And here's the first status update
Here's the second status update
1. LiveMergeCleanupThread: Chance to break things: minimal - roughly 120 lines (10% of the goal) - can be moved verbatim (cut/paste) elsewhere, like vdsm/virt/livemerge.py, fixing only import paths.
Patch ready and not published. I was thinking about moving more code in the newly created livemerge.py, but not sure about the partitioning. I'll just push the patch so we can discuss there.
Done here: https://gerrit.ovirt.org/#/q/status:open+project:vdsm+branch:master+topic:vm... I think this code could and should be split, not sure about the boundary. Please take this as draft and material for discussion. This is self contained and low-hanging fruit but not urgent, so hurry at all to review (I know storage devs are busy those days).
2. the findDrive* family of functions: Chance to break things: minimal/very low - roughly 50 lines (~4% of the goal) - can be moved with minimal changes elsewhere, like vdsm/virt/storage/????.py,
Again not completeley sure about the split, however this ended up being free functions.
3. the getConf* faimily of functions: Chance to break things: medium-to-high in the 3.x branch, low on 4.x because we drop backward compat. - roughly 110 lines (~9% of the goal) - just drop them for 4.0! - alternatively, need to figure out a proper place, maybe a new module?
Just dropped in https://gerrit.ovirt.org/#/c/49173/
The patch is quite large but simple, in the end all the code is guarded by a couple of bug if()s.
The only concern is we break compatibility with older Engines, so maybe merge this later in the development cycles. Except for maybe some glitches, I don't expect further work here (of course except verification!)
Dan added good points about further backward compatibility concerns. Will address his comments and update before the end of the year. Migration from 3.6 VDSMs should work fine, but will be tested, of course.
4. the Devices/VM setup bits All scatthered through vm.py (e.g. _buildDomainXML, DeviceMapping table) Chance to break things: medium-to-low, depends on individual patches - all summed up, roughly 200 lines (~18% of the goal) - need serious cleanup - no definite plan - mostly is cut/paste or transform Vm methods into functions
Not tackled yet
Still no progress.
5. the getUnderlying* familiy of functions: Chance to break things: low (maybe medium on some cases, to lean on the safe side) - roughly 550 lines (~46% of the goal) - easy to test (just run vms!) - can be moved in the devices/ subpackage with some changes. - require further work in the area to properly integrate this code in the device objects - more work is planned in this area, including move from minidom to ElementTree
This is gonna be large, but could be taken in steps, the first are almost mechanic. Unfortunately, the largest bits (Drive, Interface) are the less easy to simplify. I begun with some cleanup which may want anyway. This alone factorizes five devices classes, and in the end shaves ~100 lines from vm.py: https://gerrit.ovirt.org/#/q/status:open+project:vdsm+branch:master+topic:vm... To avoid to clog gerrit and because reviewers resources are scarce as usual, will work now to have posted series merged or abandonded (should we find out this is wrong approach) before to post the remaining patches for stage 1. Bests, -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani
participants (1)
-
Francesco Romani