[libvirt] [PATCH 1/3] qemu: PPCs G3Beige has a default IDE controller

[Laine Stump]

On 11/20/2015 03:20 PM, Guido Günther wrote:
> so handle it like I440FX
> ---
>   src/qemu/qemu_command.c | 17 ++++++++++++-----
>   src/qemu/qemu_domain.c  |  7 +++++++
>   src/qemu/qemu_domain.h  |  1 +
>   3 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index ef5ef93..d6b7f09 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1054,11 +1054,13 @@ qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef,
>            */
>           return virAsprintf(&controller->info.alias, "pci.%d", controller->idx);
>       } else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE) {
> -        /* for any machine based on I440FX, the first (and currently
> +        /* for any machine based on I440FX or G3Beige, the first (and currently
>            * only) IDE controller is an integrated controller hardcoded
>            * with id "ide"
>            */
> -        if (qemuDomainMachineIsI440FX(domainDef) && controller->idx == 0)
> +        if ((qemuDomainMachineIsI440FX(domainDef) ||
> +             qemuDomainMachineIsG3Beige(domainDef)) &&
> +            controller->idx == 0)
>               return VIR_STRDUP(controller->info.alias, "ide");
>       } else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) {
>           /* for any Q35 machine, the first SATA controller is the
> @@ -4915,10 +4917,13 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
>   
>       case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
>           /* Since we currently only support the integrated IDE controller
> -         * on 440fx, if we ever get to here, it's because some other
> +         * on 440fx and G3Beige, if we ever get to here, it's because some other
>            * machinetype had an IDE controller specified, or a 440fx had
>            * multiple ide controllers.
>            */
> +        if (qemuDomainMachineIsG3Beige(domainDef))
> +                break;
> +

This would cause us to ignore the fact that the config has a secondary 
IDE controller defined, and we don't support adding a 2nd IDE controller 
to the qemu commandline. I think it should instead report the following 
error message just like i440FX (unless the g3beige has an implicit 
secondary IDE and it is properly named).

Other than that, ACK (and thanks for taking the time to look for other 
machines that have builtin IDE controllers and writing the patches! I 
got a private mail about the breakage and asked the question about which 
machines have builtint IDE on qemu-devel, but hadn't had any more time 
to follow up).
>           if (qemuDomainMachineIsI440FX(domainDef))
>               virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                              _("Only a single IDE controller is unsupported "

Ugh. I should have said "supported" there rather than "unsupported". Can 
you fix that while you're touching things?

> @@ -9900,9 +9905,11 @@ qemuBuildCommandLine(virConnectPtr conn,
>                       cont->idx == 0 && qemuDomainMachineIsQ35(def))
>                           continue;
>   
> -                /* first IDE controller on i440fx machines is implicit */
> +                /* first IDE controller on i440fx and g3beige machines
> +                 * is implicit */
>                   if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE &&
> -                    cont->idx == 0 && qemuDomainMachineIsI440FX(def))
> +                    cont->idx == 0 && (qemuDomainMachineIsI440FX(def) ||
> +                                       qemuDomainMachineIsG3Beige(def)))
>                           continue;
>   
>                   if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 0861bfd..bf1f0f1 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3724,6 +3724,13 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def,
>   }
>   
>   
> +bool
> +qemuDomainMachineIsG3Beige(const virDomainDef *def)
> +{
> +    return STREQ(def->os.machine, "g3beige");
> +}
> +
> +
>   /**
>    * qemuDomainUpdateCurrentMemorySize:
>    *
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 8b6b1a3..1d2245b 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -479,6 +479,7 @@ bool qemuDomainMachineIsQ35(const virDomainDef *def);
>   bool qemuDomainMachineIsI440FX(const virDomainDef *def);
>   bool qemuDomainMachineNeedsFDC(const virDomainDef *def);
>   bool qemuDomainMachineIsS390CCW(const virDomainDef *def);
> +bool qemuDomainMachineIsG3Beige(const virDomainDef *def);
>   
>   int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
>                                         virDomainObjPtr vm);