nova.virt.libvirt.driver._get_guest_config method is nearly 400 LOC and should be broken up

Bug #1494374 reported by Matt Riedemann on 2015-09-10
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Low
Maciej Kucia

Bug Description

https://github.com/openstack/nova/blob/12.0.0a0/nova/virt/libvirt/driver.py#L4026 is huge, in liberty-3 it's nearly 400 LOC.

We should really look at breaking that up in mitaka since it's too big to unit test properly and make sure we have the right coverage.

Matt Riedemann (mriedem) on 2015-09-10
Changed in nova:
status: New → Confirmed
importance: Undecided → Low
milestone: none → next
Daniel Berrange (berrange) wrote :

Addressing config building stuff is one of the reasons for the creation of the host.py + guest.py classes. i'd like to see all the config code moved out of driver.py and into designer.py, and only rely on host+guest classes + instance object

Matt Riedemann (mriedem) wrote :

To be clear, I'd expect a series of changes that break _get_guest_config into logical chunks (other private methods), e.g. getting the SPICE console guest config. Then we could review that series in order.

Ukesh (ukeshkumar) on 2015-09-22
Changed in nova:
assignee: nobody → Ukesh (ukeshkumar)
Ukesh (ukeshkumar) on 2015-09-30
Changed in nova:
status: Confirmed → In Progress

Change abandoned by Michael Still (<email address hidden>) on branch: master
Review: https://review.openstack.org/229382
Reason: This code hasn't been updated in a long time, and is in merge conflict. I am going to abandon this review, but feel free to restore it if you're still working on this.

Cleanup
=======

There are no open reviews for this bug report since more than 2 weeks.
To signal that to other contributors which might provide patches for
this bug, I switch the status from "In Progress" to "Confirmed" and
remove the assignee.
Feel free to add yourself as assignee and to push a review for it.

Changed in nova:
assignee: Ukesh (ukeshkumar) → nobody
status: In Progress → Confirmed
Harish Kumar (hkumarmk) on 2016-09-15
Changed in nova:
assignee: nobody → Harish Kumar (hkumarmk)

Fix proposed to branch: master
Review: https://review.openstack.org/371249

Changed in nova:
status: Confirmed → In Progress

Change abandoned by Michael Still (<email address hidden>) on branch: master
Review: https://review.openstack.org/371249
Reason: This patch has been sitting unchanged for more than 12 weeks. I am therefore going to abandon it to keep the review queue sane. Please feel free to restore the change if you're still working on it.

Fix proposed to branch: master
Review: https://review.openstack.org/470783

Changed in nova:
assignee: Harish Kumar (hkumarmk) → Maciej Kucia (maciejkucia)

Fix proposed to branch: master
Review: https://review.openstack.org/470784

Fix proposed to branch: master
Review: https://review.openstack.org/470785

Fix proposed to branch: master
Review: https://review.openstack.org/470786

Fix proposed to branch: master
Review: https://review.openstack.org/470787

Maciej Kucia (maciejkucia) wrote :

I have proposed some changes, but even without those the function is not as large as it was in liberty.

Reviewed: https://review.openstack.org/470783
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=a6f8634f56a1fd1940d230298c2ddcd10d4581f0
Submitter: Jenkins
Branch: master

commit a6f8634f56a1fd1940d230298c2ddcd10d4581f0
Author: Maciej Kucia <email address hidden>
Date: Sun Jun 4 23:36:32 2017 +0200

    libvirt: Extract method _guest_add_video_device

    Partial-Bug: #1494374
    Change-Id: I55b8eb29b33f5d5f5f7dfd6d6e6a825b1635c96d
    Signed-off-by: Maciej Kucia <email address hidden>

Reviewed: https://review.openstack.org/470784
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=81ab39e21fa9b4020cfde814b8319171e6c541f5
Submitter: Jenkins
Branch: master

commit 81ab39e21fa9b4020cfde814b8319171e6c541f5
Author: Maciej Kucia <email address hidden>
Date: Sun Jun 4 23:47:13 2017 +0200

    libvirt: Extract method _guest_add_pci_devices

    Partial-Bug: #1494374
    Change-Id: Icb96a67b173363cc619439cb375a11ef4db2af93
    Signed-off-by: Maciej Kucia <email address hidden>

Reviewed: https://review.openstack.org/470785
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=d3aae9684c1e529fbe3b839ecdde565de8867bd8
Submitter: Jenkins
Branch: master

commit d3aae9684c1e529fbe3b839ecdde565de8867bd8
Author: Maciej Kucia <email address hidden>
Date: Sun Jun 4 23:55:45 2017 +0200

    libvirt: Extract method _guest_add_watchdog_action

    Partial-Bug: #1494374
    Change-Id: Iff818bcd2416e57e8724a07f158b577e1a60a8fd
    Signed-off-by: Maciej Kucia <email address hidden>

Reviewed: https://review.openstack.org/470786
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=931503a0d9f3aada186986b90ef73e22d80cf6cb
Submitter: Jenkins
Branch: master

commit 931503a0d9f3aada186986b90ef73e22d80cf6cb
Author: Maciej Kucia <email address hidden>
Date: Sun Jun 4 23:59:06 2017 +0200

    libvirt: Extract method _guest_add_memory_balloon

    Partial-Bug: #1494374
    Change-Id: Ibf3bd59fb4ce5233c26c64ef70a0ab05094ce10d
    Signed-off-by: Maciej Kucia <email address hidden>

Changed in nova:
status: In Progress → Fix Released

Reviewed: https://review.openstack.org/470787
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=0db199b853be42138f6ee06c4a3caf6fce287f5b
Submitter: Jenkins
Branch: master

commit 0db199b853be42138f6ee06c4a3caf6fce287f5b
Author: Maciej Kucia <email address hidden>
Date: Mon Jun 5 00:04:24 2017 +0200

    libvirt: Extract method _guest_add_spice_channel

    Closes-Bug: #1494374
    Change-Id: I0a8b577a060dc90170403f7ad9bdb89e09cb9fa3
    Signed-off-by: Maciej Kucia <email address hidden>

This issue was fixed in the openstack/nova 16.0.0.0b3 development milestone.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers