[libvirt] [PATCH v2 08/11] tests/util: add RISC-V capabilities

[Andrea Bolognani]

On Thu, 2018-06-14 at 22:32 +0200, Lubomir Rintel wrote:
> +static int testQemuAddRISCV32Guest(virCapsPtr caps)
> +{
> +    static const char *machine[] = { "spike_v1.10",
> +                                     "spike_v1.9.1",
> +                                     "sifive_e",
> +                                     "virt",
> +                                     "sifive_u" };
> +    virCapsGuestMachinePtr *machines = NULL;
> +    virCapsGuestPtr guest;
> +
> +    machines = virCapabilitiesAllocMachines(machine, 1);

This is wrong: the second argument to the function is the number
of machines you're adding, so it should look like

  virCapabilitiesAllocMachines(machine,
                               ARRAY_CARDINALITY(machine))

instead. While you're at it, you can give the variable a better
name, like for example... 'names' :)

Actually, since you're going to need that value more than once,
you will probably want to introduce

  static const int nmachines = ARRAY_CARDINALITY(names);

and just use 'nmachines' in the call to AllocMachines() above...

> +    if (!machines)
> +        goto error;
> +
> +    guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_RISCV32,
> +                                    QEMUBinList[TEST_UTILS_QEMU_BIN_RISCV32],
> +                                    NULL, 1, machines);

... as well as here...

> +    if (!guest)
> +        goto error;
> +
> +    if (!virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_QEMU, NULL, NULL, 0, NULL))
> +        goto error;
> +
> +    return 0;
> +
> + error:
> +    /* No way to free a guest? */
> +    virCapabilitiesFreeMachines(machines, 1);

... and here.

(Drop the comment here as well.)

Looking at this also made me realize there are several existing
instances of it not being handled correctly; I'll take care of
fixing them.

[...]
> @@ -422,6 +488,12 @@ virCapsPtr testQemuCapsInit(void)
>      if (testQemuAddPPCGuest(caps))
>          goto cleanup;
>  
> +    if (testQemuAddRISCV32Guest(caps))
> +        goto cleanup;
> +
> +    if (testQemuAddRISCV64Guest(caps))
> +        goto cleanup;

Despite the surrounding code suggesting otherwise, both these calls
should look like

  if (testQemuAddRISCV32Guest(caps) < 0)
    goto cleanup;

Again, I'll take care of fixing the existing instances.


Once all of the above have been taken care of, the patch will look
good; however, I think it doesn't make a lot of sense to merge it
on its own, and we should rather squash it into 07/11 instead and
use a commit message like

  tests: Add RISC-V architectures

for the whole thing.

-- 
Andrea Bolognani / Red Hat / Virtualization