Re: [libvirt] [PATCH] Implement capabilities for Solaris

[Jim Meyering <jim@xxxxxxxxxxxx>]

john.levon@xxxxxxx wrote:
...
Hi John,

This looks fine, but there are a couple of nits.

> +#ifdef __sun
> +
> +static int
> +get_cpu_flags(virConnectPtr conn, const char **hvm, int *pae, int *longmode)
> +{
> +    struct {
> +        uint32_t r_eax, r_ebx, r_ecx, r_edx;
> +    } regs;
> +
> +    char tmpbuf[20];
> +    int fd = -1;

No need to initialize.

> +    int ret = 0;
> +
> +    /* returns -1, errno 22 if in 32-bit mode */
> +    *longmode = (sysinfo(SI_ARCHITECTURE_64, tmpbuf, 20) != -1);

It'd be nice to use "sizeof tmpbuf", rather than
repeating the literal size, "20".

> +    if ((fd = open("/dev/cpu/self/cpuid", O_RDONLY)) == -1 ||
> +        pread(fd, &regs, sizeof(regs), 0) != sizeof(regs)) {
> +        virXenError(conn, VIR_ERR_SYSTEM_ERROR,
> +            "couldn't read CPU flags: %s", strerror(errno));
> +        goto out;
> +    }
> +
> +    *pae = 0;
> +    *hvm = "";
> +
> +    if (strncmp((const char *)&regs.r_ebx, "AuthcAMDenti", 12) == 0) {

You'll have to use STREQLEN in place of that strncmp,
in order for "make syntax-check" to pass.

> +        if (pread(fd, &regs, sizeof (regs), 0x80000001) == sizeof (regs)) {
> +            /* Read secure virtual machine bit (bit 2 of ECX feature ID) */
> +            if ((regs.r_ecx >> 2) & 1) {
> +                *hvm = "svm";
> +            }
> +            if ((regs.r_edx >> 6) & 1)
> +                *pae = 1;
> +        }
> +    } else if (strncmp((const char *)&regs.r_ebx, "GenuntelineI", 12) == 0) {

Here, too.

...
+    /* Really, this never fails - look at the man-page. */
+    uname (&utsname);

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