Re: [PATCH RFC 1/2] qemu: use "-accel" option to specify accelerator instead of "-machine"

[Paolo Bonzini <pbonzini@xxxxxxxxxx>]

On 09/01/21 19:58, huangy81@xxxxxxxxxxxxxxx wrote:
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6f970a3..9a64473 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6711,40 +6711,6 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
>       virCommandAddArg(cmd, "-machine");
>       virBufferAdd(&buf, def->os.machine, -1);
>   
> -    switch ((virDomainVirtType)def->virtType) {
> -    case VIR_DOMAIN_VIRT_QEMU:
> -        virBufferAddLit(&buf, ",accel=tcg");
> -        break;
> -
> -    case VIR_DOMAIN_VIRT_KVM:
> -        virBufferAddLit(&buf, ",accel=kvm");
> -        break;
> -
> -    case VIR_DOMAIN_VIRT_KQEMU:
> -    case VIR_DOMAIN_VIRT_XEN:
> -    case VIR_DOMAIN_VIRT_LXC:
> -    case VIR_DOMAIN_VIRT_UML:
> -    case VIR_DOMAIN_VIRT_OPENVZ:
> -    case VIR_DOMAIN_VIRT_TEST:
> -    case VIR_DOMAIN_VIRT_VMWARE:
> -    case VIR_DOMAIN_VIRT_HYPERV:
> -    case VIR_DOMAIN_VIRT_VBOX:
> -    case VIR_DOMAIN_VIRT_PHYP:
> -    case VIR_DOMAIN_VIRT_PARALLELS:
> -    case VIR_DOMAIN_VIRT_BHYVE:
> -    case VIR_DOMAIN_VIRT_VZ:
> -    case VIR_DOMAIN_VIRT_NONE:
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       _("the QEMU binary does not support %s"),
> -                       virDomainVirtTypeToString(def->virtType));
> -        return -1;
> -
> -    case VIR_DOMAIN_VIRT_LAST:
> -    default:
> -        virReportEnumRangeError(virDomainVirtType, def->virtType);
> -        return -1;
> -    }
> -
>       /* To avoid the collision of creating USB controllers when calling
>        * machine->init in QEMU, it needs to set usb=off
>        */
> @@ -6945,6 +6911,52 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
>       return 0;
>   }
>   
> +static int
> +qemuBuildAccelCommandLine(virCommandPtr cmd,
> +                          const virDomainDef *def)
> +{
> +    /* the '-machine' options for accelerator are legacy,
> +     * using the '-accel' options by default */
> +    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> +    virCommandAddArg(cmd, "-accel");
> +
> +    switch ((virDomainVirtType)def->virtType) {
> +    case VIR_DOMAIN_VIRT_QEMU:
> +        virBufferAddLit(&buf, "tcg");
> +        break;
> +
> +    case VIR_DOMAIN_VIRT_KVM:
> +        virBufferAddLit(&buf, "kvm");
> +        break;
> +
> +    case VIR_DOMAIN_VIRT_KQEMU:
> +    case VIR_DOMAIN_VIRT_XEN:
> +    case VIR_DOMAIN_VIRT_LXC:
> +    case VIR_DOMAIN_VIRT_UML:
> +    case VIR_DOMAIN_VIRT_OPENVZ:
> +    case VIR_DOMAIN_VIRT_TEST:
> +    case VIR_DOMAIN_VIRT_VMWARE:
> +    case VIR_DOMAIN_VIRT_HYPERV:
> +    case VIR_DOMAIN_VIRT_VBOX:
> +    case VIR_DOMAIN_VIRT_PHYP:
> +    case VIR_DOMAIN_VIRT_PARALLELS:
> +    case VIR_DOMAIN_VIRT_BHYVE:
> +    case VIR_DOMAIN_VIRT_VZ:
> +    case VIR_DOMAIN_VIRT_NONE:
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("the QEMU binary does not support %s"),
> +                       virDomainVirtTypeToString(def->virtType));
> +        return -1;
> +
> +    case VIR_DOMAIN_VIRT_LAST:
> +    default:
> +        virReportEnumRangeError(virDomainVirtType, def->virtType);
> +        return -1;
> +    }
> +
> +    virCommandAddArgBuffer(cmd, &buf);
> +    return 0;
> +}
>   
>   static void
>   qemuBuildTSEGCommandLine(virCommandPtr cmd,
> @@ -9840,6 +9852,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
>       if (qemuBuildMachineCommandLine(cmd, cfg, def, qemuCaps, priv) < 0)
>           return NULL;
>   
> +    if (qemuBuildAccelCommandLine(cmd, def) < 0)
> +        return NULL;
> +
>       qemuBuildTSEGCommandLine(cmd, def);

This unfortunately cannot be done unconditionally.  You need to probe 
for the availability of -accel, using something like

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4d132defbd..23efaccbad 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -609,6 +609,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
               "ncr53c90",
               "dc390",
               "am53c974",
+              "accel",
     );


@@ -3220,6 +3221,7 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
     { "fsdev", "multidevs", QEMU_CAPS_FSDEV_MULTIDEVS },
     { "fw_cfg", "file", QEMU_CAPS_FW_CFG },
     { "fsdev", "fmode", QEMU_CAPS_FSDEV_CREATEMODE }, /* Could have 
also checked fsdev->dmode */
+    { "accel", "accel", QEMU_CAPS_ACCEL },
 };

 static int

Then you can choose between the two code paths using 
virQEMUCapsGet(qemuCaps, QEMU_CAPS_ACCEL).

Thanks,

Paolo