Re: [PATCH 6/7] tools/tests/tsx: move guest creation to common area

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.02.2026 11:14, Edwin Török wrote:
> --- /dev/null
> +++ b/tools/tests/common/guests.c

Everything in the file uses singular, si I'd suggest to use singular also for
the file name.

> @@ -0,0 +1,87 @@
> +#define _GNU_SOURCE
> +#include "guests.h"
> +#include "tests.h"
> +
> +#include <err.h>
> +#include <errno.h>
> +#include <string.h>
> +
> +xc_interface *xch;
> +xc_physinfo_t physinfo;
> +bool xen_has_pv = true, xen_has_hvm = true;

static for at least some of these?

> +void test_guest_init(void)
> +{
> +    int rc;
> +    xch = xc_interface_open(NULL, NULL, 0);
> +
> +    if ( !xch )
> +        err(1, "xc_interface_open");
> +
> +    rc = xc_physinfo(xch, &physinfo);
> +    if ( rc )
> +        err(1, "Failed to obtain physinfo");
> +
> +    xen_has_hvm = physinfo.capabilities & XEN_SYSCTL_PHYSCAP_hvm;
> +    xen_has_pv = physinfo.capabilities & XEN_SYSCTL_PHYSCAP_pv;
> +}
> +
> +void test_guest(struct xen_domctl_createdomain *c)

static?

> +{
> +    uint32_t domid = 0;
> +    int rc;
> +
> +    if (!xch)

Everything else uses Xen style, so this wants to, too.

> +        return fail("test_guest_init() not called");
> +
> +    rc = xc_domain_create(xch, &domid, c);
> +    if ( rc )
> +        return fail("  Domain create failure: %d - %s\n",
> +                    errno, strerror(errno));
> +
> +    printf("  Created d%u\n", domid);
> +
> +    test_guest_domid(domid);

This might better be passed as a function pointer, so harnesses can have
multiple functions dealing with different aspects of the testing.

> +    rc = xc_domain_destroy(xch, domid);
> +    if ( rc )
> +        fail("  Failed to destroy domain: %d - %s\n",
> +             errno, strerror(errno));
> +}
> +
> +void test_guests(void)
> +{
> +    if ( xen_has_pv )
> +    {
> +        struct xen_domctl_createdomain c = {
> +            .max_vcpus = 1,
> +            .max_grant_frames = 1,
> +            .grant_opts = XEN_DOMCTL_GRANT_version(1),
> +        };
> +
> +        printf("Testing PV guest\n");

To easily distinguish things in the output of a harness creating multiple
guests, some disambuating string may want passing in.

> +        test_guest(&c);
> +    }
> +
> +    if ( xen_has_hvm )
> +    {
> +        struct xen_domctl_createdomain c = {
> +            .flags = XEN_DOMCTL_CDF_hvm,
> +            .max_vcpus = 1,
> +            .max_grant_frames = 1,
> +            .grant_opts = XEN_DOMCTL_GRANT_version(1),
> +            .arch = {
> +                .emulation_flags = XEN_X86_EMU_LAPIC,
> +            },
> +        };

For starters this may work, but longer term we may need ways to specify certain
propertied of the to be created guest(s).

> --- /dev/null
> +++ b/tools/tests/common/guests.h
> @@ -0,0 +1,11 @@
> +#include <xenctrl.h>
> +#include <xen/domctl.h>
> +
> +extern void test_guest_domid(domid_t domid);

Nit: If this was to stay, no need for "extern", just like ...

> +extern xc_interface *xch;
> +extern xc_physinfo_t physinfo;
> +extern bool xen_has_pv, xen_has_hvm;
> +
> +void test_guest_init(void);
> +void test_guest(struct xen_domctl_createdomain *c);
> +void test_guests(void);

... you have none here.

> --- a/tools/tests/tsx/Makefile
> +++ b/tools/tests/tsx/Makefile
> @@ -11,4 +11,6 @@ CFLAGS += $(CFLAGS_libxenguest)
>  LDFLAGS += $(LDLIBS_libxenctrl)
>  LDFLAGS += $(LDLIBS_libxenguest)
>  
> +$(TARGET): ../common/guests.o

Does this work properly with parallel builds, once more than one test would
use it?

> --- a/tools/tests/tsx/test-tsx.c
> +++ b/tools/tests/tsx/test-tsx.c
> @@ -31,6 +31,7 @@
>  #include <xen-tools/common-macros.h>
>  
>  #include "tests.h"
> +#include "guests.h"

Isn't this ../common/guests.h ? Plus perhaps common stuff could go straight
in the parent directory anyway?

Jan