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

Lubomir Rintel posted 11 patches 7 years ago
Only 10 patches received!
There is a newer version of this series
[libvirt] [PATCH v2 08/11] tests/util: add RISC-V capabilities
Posted by Lubomir Rintel 7 years ago
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 tests/testutilsqemu.c | 72 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index dc7e90b952..c6376262b9 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -28,6 +28,8 @@ typedef enum {
     TEST_UTILS_QEMU_BIN_ARM,
     TEST_UTILS_QEMU_BIN_PPC64,
     TEST_UTILS_QEMU_BIN_PPC,
+    TEST_UTILS_QEMU_BIN_RISCV32,
+    TEST_UTILS_QEMU_BIN_RISCV64,
     TEST_UTILS_QEMU_BIN_S390X
 } QEMUBinType;
 
@@ -38,6 +40,8 @@ static const char *QEMUBinList[] = {
     "/usr/bin/qemu-system-arm",
     "/usr/bin/qemu-system-ppc64",
     "/usr/bin/qemu-system-ppc",
+    "/usr/bin/qemu-system-riscv32",
+    "/usr/bin/qemu-system-riscv64",
     "/usr/bin/qemu-system-s390x"
 };
 
@@ -285,6 +289,68 @@ static int testQemuAddPPCGuest(virCapsPtr caps)
     return -1;
 }
 
+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);
+    if (!machines)
+        goto error;
+
+    guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_RISCV32,
+                                    QEMUBinList[TEST_UTILS_QEMU_BIN_RISCV32],
+                                    NULL, 1, machines);
+    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);
+    return -1;
+}
+
+static int testQemuAddRISCV64Guest(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);
+    if (!machines)
+        goto error;
+
+    guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_RISCV64,
+                                    QEMUBinList[TEST_UTILS_QEMU_BIN_RISCV64],
+                                    NULL, 1, machines);
+    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);
+    return -1;
+}
+
 static int testQemuAddS390Guest(virCapsPtr caps)
 {
     static const char *s390_machines[] = { "s390-virtio",
@@ -422,6 +488,12 @@ virCapsPtr testQemuCapsInit(void)
     if (testQemuAddPPCGuest(caps))
         goto cleanup;
 
+    if (testQemuAddRISCV32Guest(caps))
+        goto cleanup;
+
+    if (testQemuAddRISCV64Guest(caps))
+        goto cleanup;
+
     if (testQemuAddS390Guest(caps))
         goto cleanup;
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 08/11] tests/util: add RISC-V capabilities
Posted by Andrea Bolognani 7 years ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list