[libvirt] [PATCH] Clarify why some controllers don't generate -device string in QEMU driver

[Daniel Veillard]

On Thu, Jan 28, 2010 at 12:53:56PM +0000, Matthew Booth wrote:
> The QEMU driver contained code to generate a -device string for piix4-ide, but
> wasn't using it. This change removes this string generation. It also adds a
> comment explaining why IDE and FDC controllers don't generate -device strings.
> 
> The change also generates an error if a sata controller is specified for a QEMU
> domain, as this isn't supported.
> 
> * src/qemu/qemu_conf.c: Remove VIR_DOMAIN_CONTROLLER_TYPE_IDE handler in
>                         qemuBuildControllerDevStr(). Ignore IDE and FDC
>                         controllers. Error if SATA controller is discovered. Add
>                         comments.
> ---
>  src/qemu/qemu_conf.c |   26 ++++++++++++++++++--------
>  1 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index f4a6c08..3b7793f 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -2121,11 +2121,8 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def)
>          virBufferVSprintf(&buf, ",id=scsi%d", def->idx);
>          break;
>  
> +    /* We always get an IDE controller, whether we want it or not. */
>      case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
> -        virBufferAddLit(&buf, "piix4-ide");
> -        virBufferVSprintf(&buf, ",id=ide%d", def->idx);
> -        break;
> -
>      default:
>          goto error;
>      }
> @@ -3141,16 +3138,29 @@ int qemudBuildCommandLine(virConnectPtr conn,
>  
>      if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
>          for (i = 0 ; i < def->ncontrollers ; i++) {
> -            char *scsi;
> -            if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
> +            virDomainControllerDefPtr cont = def->controllers[i];
> +
> +            /* We don't add an explicit IDE or FD controller because the
> +             * provided PIIX4 device already includes one. It isn't possible to
> +             * remove the PIIX4. */
> +            if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE ||
> +                cont->type == VIR_DOMAIN_CONTROLLER_TYPE_FDC)
>                  continue;
>  
> +            /* QEMU doesn't implement a SATA driver */
> +            if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) {
> +                qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
> +                                 "%s", _("SATA is not supported with this QEMU binary"));
> +                goto error;
> +            }
> +
>              ADD_ARG_LIT("-device");
>  
> -            if (!(scsi = qemuBuildControllerDevStr(def->controllers[i])))
> +            char *devstr;
> +            if (!(devstr = qemuBuildControllerDevStr(def->controllers[i])))
>                  goto no_memory;
>  
> -            ADD_ARG(scsi);
> +            ADD_ARG(devstr);
>          }
>      }
>  

  Okay, pushed, thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/