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 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.

> @@ -743,36 +749,38 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
>      virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPE_PCIE_DEVICE;
>  
>      for (i = 0; i < def->ncontrollers; i++) {
> -        switch (def->controllers[i]->type) {
> +        virDomainControllerDefPtr cont = def->controllers[i];
> +
> +        switch (cont->type) {
>          case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
>              /* Verify that the first SATA controller is at 00:1F.2 the
>               * q35 machine type *always* has a SATA controller at this
>               * address.
>               */
> -            if (def->controllers[i]->idx == 0) {
> -                if (virDeviceInfoPCIAddressPresent(&def->controllers[i]->info)) {
> -                    if (def->controllers[i]->info.addr.pci.domain != 0 ||
> -                        def->controllers[i]->info.addr.pci.bus != 0 ||
> -                        def->controllers[i]->info.addr.pci.slot != 0x1F ||
> -                        def->controllers[i]->info.addr.pci.function != 2) {
> +            if (cont->idx == 0) {
> +                if (virDeviceInfoPCIAddressPresent(&cont->info)) {
> +                    if (cont->info.addr.pci.domain != 0 ||
> +                        cont->info.addr.pci.bus != 0 ||
> +                        cont->info.addr.pci.slot != 0x1F ||
> +                        cont->info.addr.pci.function != 2) {
>                          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                                         _("Primary SATA controller must have PCI address 0:0:1f.2"));
>                          goto cleanup;
>                      }
>                  } else {
> -                    def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> -                    def->controllers[i]->info.addr.pci.domain = 0;
> -                    def->controllers[i]->info.addr.pci.bus = 0;
> -                    def->controllers[i]->info.addr.pci.slot = 0x1F;
> -                    def->controllers[i]->info.addr.pci.function = 2;
> +                    cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> +                    cont->info.addr.pci.domain = 0;
> +                    cont->info.addr.pci.bus = 0;
> +                    cont->info.addr.pci.slot = 0x1F;
> +                    cont->info.addr.pci.function = 2;
>                  }
>              }
>              break;
>  
>          case VIR_DOMAIN_CONTROLLER_TYPE_USB:
> -            if ((def->controllers[i]->model
> +            if ((cont->model
>                   == VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1) &&

You can now join these two lines...

> -                (def->controllers[i]->info.type
> +                (cont->info.type
>                   == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)) {

... and these two.

ACK with that nit fixed.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
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]