[PATCH v2 05/17] qemu: Clean up qemuDomainDefaultSCSIControllerModel()

Andrea Bolognani posted 17 patches 11 months ago
[PATCH v2 05/17] qemu: Clean up qemuDomainDefaultSCSIControllerModel()
Posted by Andrea Bolognani 11 months ago
Use a better order for sections, improve comments.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_domain.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ab2cc427a2..f4af8fb6ba 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4101,30 +4101,49 @@ static virDomainControllerModelSCSI
 qemuDomainDefaultSCSIControllerModelInternal(const virDomainDef *def,
                                              virQEMUCaps *qemuCaps)
 {
-    if (qemuDomainIsPSeries(def))
-        return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+    /* For machine types with built-in SCSI controllers, the choice
+     * of model is obvious */
+    if (qemuDomainHasBuiltinESP(def))
+        return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90;
+
+    /* Most new architectures should ideally use virtio */
     if (ARCH_IS_S390(def->os.arch))
         return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
+
+    /* pSeries has its own special default */
+    if (qemuDomainIsPSeries(def))
+        return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+
+    /* If there is no preference, base the choice on device
+     * availability. In this case, lsilogic is favored over
+     * virtio-scsi for backwards compatibility reasons */
     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI))
         return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI))
         return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
-    if (qemuDomainHasBuiltinESP(def))
-        return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90;
 
     return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT;
 }
 
 
 /**
- * @def: Domain definition
- * @cont: Domain controller def
- * @qemuCaps: qemu capabilities
+ * qemuDomainDefaultSCSIControllerModel:
+ * @model: return location
+ * @def: domain definition
+ * @qemuCaps: QEMU capabilities, or NULL
+ *
+ * Choose a reasonable model to use for a SCSI controller where a
+ * specific one hasn't been provided by the user.
+ *
+ * The choice is based on a number of factors, including the guest's
+ * architecture and machine type. @qemuCaps, if provided, might be
+ * taken into consideration too.
  *
- * If the controller model is already defined, return it immediately;
- * otherwise, based on the @qemuCaps return a default model value.
+ * Not being able to come up with a value is NOT considered a
+ * failure. It's up to the caller to decide how to handle that
+ * scenario.
  *
- * Returns model on success, -1 on failure with error set.
+ * Returns: 0 on success, <0 on failure.
  */
 int
 qemuDomainDefaultSCSIControllerModel(virDomainControllerModelSCSI *model,
-- 
2.43.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v2 05/17] qemu: Clean up qemuDomainDefaultSCSIControllerModel()
Posted by Peter Krempa 11 months ago
On Wed, Feb 14, 2024 at 18:11:12 +0100, Andrea Bolognani wrote:
> Use a better order for sections, improve comments.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  src/qemu/qemu_domain.c | 39 +++++++++++++++++++++++++++++----------
>  1 file changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index ab2cc427a2..f4af8fb6ba 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4101,30 +4101,49 @@ static virDomainControllerModelSCSI
>  qemuDomainDefaultSCSIControllerModelInternal(const virDomainDef *def,
>                                               virQEMUCaps *qemuCaps)
>  {
> -    if (qemuDomainIsPSeries(def))
> -        return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
> +    /* For machine types with built-in SCSI controllers, the choice
> +     * of model is obvious */
> +    if (qemuDomainHasBuiltinESP(def))
> +        return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90;
> +
> +    /* Most new architectures should ideally use virtio */
>      if (ARCH_IS_S390(def->os.arch))
>          return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
> +
> +    /* pSeries has its own special default */
> +    if (qemuDomainIsPSeries(def))
> +        return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
> +
> +    /* If there is no preference, base the choice on device
> +     * availability. In this case, lsilogic is favored over
> +     * virtio-scsi for backwards compatibility reasons */
>      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI))
>          return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
>      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI))
>          return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
> -    if (qemuDomainHasBuiltinESP(def))
> -        return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90;
>  
>      return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT;
>  }
>  
>  
>  /**
> - * @def: Domain definition
> - * @cont: Domain controller def
> - * @qemuCaps: qemu capabilities
> + * qemuDomainDefaultSCSIControllerModel:
> + * @model: return location
> + * @def: domain definition
> + * @qemuCaps: QEMU capabilities, or NULL
> + *
> + * Choose a reasonable model to use for a SCSI controller where a
> + * specific one hasn't been provided by the user.
> + *
> + * The choice is based on a number of factors, including the guest's
> + * architecture and machine type. @qemuCaps, if provided, might be
> + * taken into consideration too.
>   *
> - * If the controller model is already defined, return it immediately;
> - * otherwise, based on the @qemuCaps return a default model value.
> + * Not being able to come up with a value is NOT considered a
> + * failure. It's up to the caller to decide how to handle that
> + * scenario.
>   *
> - * Returns model on success, -1 on failure with error set.
> + * Returns: 0 on success, <0 on failure.

As noted before, the return value here has no useful value, and you
might as well as directly return the model.

>   */
>  int
>  qemuDomainDefaultSCSIControllerModel(virDomainControllerModelSCSI *model,
> -- 
> 2.43.0
> _______________________________________________
> Devel mailing list -- devel@lists.libvirt.org
> To unsubscribe send an email to devel-leave@lists.libvirt.org
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org