Re: [PATCH v4 02/25] [ACKED but has additions] qemu: replace a lot of "def->controllers[i]" with equivalent "cont"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/18/2016 07:22 AM, Andrea Bolognani wrote:
On Fri, 2016-10-14 at 15:53 -0400, Laine Stump wrote:
There's no functional change here. This pointer was just used so many
times that the extra long lines became annoying.
---
Change: added more instances of the same change. src/qemu/qemu_domain_address.c | 208 ++++++++++++++++++++++-------------------
   1 file changed, 111 insertions(+), 97 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index dc67d51..e6abadf 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -220,18 +220,22 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
       }
for (i = 0; i < def->ncontrollers; i++) {
-        model = def->controllers[i]->model;
-        if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
+        virDomainControllerDefPtr cont = def->controllers[i];
+
+        model = cont->model;
+        if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
               if (qemuDomainSetSCSIControllerModel(def, qemuCaps, &model) < 0)
Definitely not something that should be touched by this patch,
but shouldn't we pass &cont->model here?

I mean, if the value stored in model will be different than
the one that was already in cont->model, it means that the
default controller model was not set properly earlier...

On the other hand, the default model should really have been
set in PostParse() or something like that.

The function qemuDomainSetSCSIControllerModel() seems to be a confused mess. If model is set (non-0) then it checks the qemuCaps to makes sure the model that's set is allowed. Otherwise, it sets a default model. *All* calls to this function are on a temporary variable rather than the item in the controller object, so any non-zero model set will never be stored in the config. Why it is that way, I have no idea.

I choose to not touch it, for fear that something offbeat would break. (Maybe it's done this way because some old version of libvirt 6 or 7 years ago didn't support setting a scsi controller model or something? Who knows...)

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]