[libvirt] [RFC PATCH 08/12] qemu: introduce qemuBuildCPUDeviceStr

[Peter Krempa]

On Wed, Jan 21, 2015 at 16:01:00 +0800, Zhu Guihua wrote:
> qemuBuildCPUDeviceStr being introduced is responsible for creating command
> line argument for '-device' for given cpu device.
> 
> Signed-off-by: Zhu Guihua <zhugh.fnst at cn.fujitsu.com>
> ---
>  src/qemu/qemu_command.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_command.h |  5 +++++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 2ee3e10..824ad29 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5956,6 +5956,50 @@ static char *qemuBuildTPMDevStr(const virDomainDef *def,
>  }
>  
>  
> +int
> +qemuBuildCPUDeviceStr(char **deviceStr,
> +                      virDomainCPUDefPtr dev,
> +                      virQEMUCapsPtr qemuCaps)
> +{
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU64_CPU)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("%s not supported in this QEMU binary"), dev->driver);

You blindly assume that the user passed the correct model here and not
any other invalid string. As I've already said, having this converted to
an enum makes things easier.

> +        goto error;
> +    }
> +
> +    virBufferAsprintf(&buf, "%s,id=%s,apic-id=%d",
> +                      dev->driver, dev->info.alias,
> +                      dev->apic_id);

Having a buffer for a single virBufferAsprintf case is a bit overkill.

> +
> +    if (virBufferCheckError(&buf) < 0)
> +        goto error;
> +
> +    *deviceStr = virBufferContentAndReset(&buf);
> +    return 0;
> +
> + error:
> +    virBufferFreeAndReset(&buf);
> +    return -1;
> +}
> +
> +static int
> +qemuBuildCPUDeviceCommandLine(virCommandPtr cmd,
> +                              virDomainCPUDefPtr dev,
> +                              virQEMUCapsPtr qemuCaps)
> +{
> +    char *devstr = NULL;
> +
> +    if (qemuBuildCPUDeviceStr(&devstr, dev, qemuCaps) < 0)
> +        return -1;
> +
> +    virCommandAddArgList(cmd, "-device", devstr, NULL);
> +    VIR_FREE(devstr);
> +    return 0;
> +}
> +
> +
>  static char *qemuBuildSmbiosBiosStr(virSysinfoDefPtr def)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -9862,6 +9906,12 @@ qemuBuildCommandLine(virConnectPtr conn,
>              goto error;
>      }
>  
> +    /* add cpu devices */
> +    for (i = 0; i < def->ncpus; i++) {
> +        if (qemuBuildCPUDeviceCommandLine(cmd, def->cpus[i], qemuCaps) < 0)
> +            goto error;
> +    }


As I've asked before. The question here is whether this is equivalent
with just increasing the vCPU count in the "-cpu" argument.

If not it will require a bit more work for setting cgroups, numa pinning
and other stuff.

If they are equivalent we should use that instead and have the existing
code do the magic.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150121/e90d7a19/attachment-0001.sig>