[libvirt] [PATCH 03/10] qemu: Make basic upfront checks before creating command

[Peter Krempa]

On Tue, Feb 16, 2016 at 19:44:13 -0500, John Ferlan wrote:
> Create qemuBuildPreCheck to make some basic upfront checks before
> trying to build the command.
> 
> Unfortunately the 'spice' count was used later on, so yuck, handle that.

The count is not needed. Just the fact that there are spice devices is
required. Additionally I don't understand why spice-transported serial
ports are skipped rather than pointed out if there is no spice graphics
to transport them. I'd fix that one first rather than having to deal
with the pointer and having a checker that needs to leak stuff out to
the caller.

> 
> This will move some logic from much later to much earlier - we shouldn't
> be adjusting any data so that shouldn't matter.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_command.c | 169 +++++++++++++++++++++++++++---------------------
>  1 file changed, 97 insertions(+), 72 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index ab27619..36fe110 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6593,6 +6593,99 @@ qemuBuildTPMCommandLine(virDomainDefPtr def,
>  }
>  
>  
> +/*
> + * qemuBuildPreCheck:

I'd prefer qemuBuildCommandLineValidate

> + *
> + * Perform some basic configuration checks before taking the plunge.

And something either more descriptive or at least finishing the sentence
after 'checks'.

> + */
> +static int
> +qemuBuildPreCheck(virQEMUDriverPtr driver,
> +                  const virDomainDef  *def,
> +                  int *spicecnt)

Checks should not leak any value.

> +{
> +    size_t i;
> +    int sdl = 0;
> +    int vnc = 0;
> +    int spice = 0;

[...]

> +
> +
> +    return 0;
> +
> + error:
> +    return -1;

Nothing to clean up, thus the label doesn't make sense.

> +}
> +
> +
>  qemuBuildCommandLineCallbacks buildCommandLineCallbacks = {
>      .qemuGetSCSIDeviceSgName = virSCSIDeviceGetSgName,
>  };

[...]

> @@ -6677,47 +6768,8 @@ qemuBuildCommandLine(virConnectPtr conn,

> -
> -    for (i = 0; i < def->ngraphics; ++i) {
> -        switch (def->graphics[i]->type) {
> -        case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
> -            ++sdl;
> -            break;
> -        case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
> -            ++vnc;
> -            break;
> -        case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
> -            ++spice;
> -            break;
> -        }
> -    }
> +    if (qemuBuildPrecCheck(driver, def, &spicecnt) < 0)

This won't compile.

> +        goto error;
>  
>      /*
>       * do not use boot=on for drives when not using KVM since this

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/20160217/68dc5cc5/attachment-0001.sig>