Re: [PATCH 15/33] qemu: Simplify qemuDomainFindOrCreateSCSIDiskController()

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

 



On Wed, Jan 24, 2024 at 20:37:35 +0100, Andrea Bolognani wrote:
> Not a massive difference, but it will make upcoming changes
> nicer.
> 
> Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
> ---
>  src/qemu/qemu_hotplug.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index afb720fc0b..c883fef0e9 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -876,10 +876,9 @@ qemuDomainFindOrCreateSCSIDiskController(virDomainObj *vm,
>      cont = g_new0(virDomainControllerDef, 1);
>      cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI;
>      cont->idx = controller;
> -    if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT)
> -        cont->model = qemuDomainGetSCSIControllerModel(vm->def, cont, priv->qemuCaps);
> -    else
> -        cont->model = model;
> +    cont->model = model;
> +
> +    cont->model = qemuDomainGetSCSIControllerModel(vm->def, cont, priv->qemuCaps);

This makes no sense at first look. Commit message doesn't explain it,
nothing in the code explains it. You have to look into
qemuDomainGetSCSIControllerModel to see why it can be done.

I've looked ahead to see how you've refactored it, at which point it
makes sense, but I'm not really persuaded that this can't be part of the
commit that is actually fixing the function to not depend on the value
in the 'model' struct.

IMO this patch makes the code strictly worse, even when it's short
lived.
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




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

  Powered by Linux