Re: [Qemu-devel] [PATCH 02/14] hvf: add code base from Google's QEMU repository

[Daniel P. Berrange]

On Sun, Aug 27, 2017 at 08:56:42PM -0500, Sergio Andres Gomez Del Real wrote:
> This file begins tracking the files that will be the code base for HVF
> support in QEMU.
> 
> Signed-off-by: Sergio Andres Gomez Del Real <address@hidden>
> ---

> diff --git a/target/i386/hvf-all.c b/target/i386/hvf-all.c
> new file mode 100644
> index 0000000000..06cd8429eb
> --- /dev/null
> +++ b/target/i386/hvf-all.c
> @@ -0,0 +1,982 @@

There's no copyright header on this file.

A few others a missing copyright headers too, but they're all simple
.h files, so not critical. This, though, is a major .c file so I think
it really should have a copyright header present. IIUC, this is not
your code, but rather copied from the google repo, so ought to get the
original author to update this.

> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +
> +#include "sysemu/hvf.h"
> +#include "hvf-i386.h"
> +#include "hvf-utils/vmcs.h"
> +#include "hvf-utils/vmx.h"
> +#include "hvf-utils/x86.h"
> +#include "hvf-utils/x86_descr.h"
> +#include "hvf-utils/x86_mmu.h"
> +#include "hvf-utils/x86_decode.h"
> +#include "hvf-utils/x86_emu.h"
> +#include "hvf-utils/x86_cpuid.h"
> +#include "hvf-utils/x86hvf.h"
> +
> +#include <Hypervisor/hv.h>
> +#include <Hypervisor/hv_vmx.h>
> +
> +#include "exec/address-spaces.h"
> +#include "exec/exec-all.h"
> +#include "exec/ioport.h"
> +#include "hw/i386/apic_internal.h"
> +#include "hw/boards.h"
> +#include "qemu/main-loop.h"
> +#include "strings.h"
> +#include "trace.h"
> +#include "sysemu/accel.h"
> +#include "sysemu/sysemu.h"
> +#include "target/i386/cpu.h"
> +
> +pthread_rwlock_t mem_lock = PTHREAD_RWLOCK_INITIALIZER;
> +HVFState *hvf_state;
> +static int hvf_disabled = 1;
> +
> +static void assert_hvf_ok(hv_return_t ret)
> +{
> +    if (ret == HV_SUCCESS)
> +        return;
> +
> +    switch (ret) {
> +        case HV_ERROR:
> +            fprintf(stderr, "Error: HV_ERROR\n");
> +            break;
> +        case HV_BUSY:
> +            fprintf(stderr, "Error: HV_BUSY\n");
> +            break;
> +        case HV_BAD_ARGUMENT:
> +            fprintf(stderr, "Error: HV_BAD_ARGUMENT\n");
> +            break;
> +        case HV_NO_RESOURCES:
> +            fprintf(stderr, "Error: HV_NO_RESOURCES\n");
> +            break;
> +        case HV_NO_DEVICE:
> +            fprintf(stderr, "Error: HV_NO_DEVICE\n");
> +            break;
> +        case HV_UNSUPPORTED:
> +            fprintf(stderr, "Error: HV_UNSUPPORTED\n");
> +            break;
> +        default:
> +            fprintf(stderr, "Unknown Error\n");
> +    }
> +
> +    abort();
> +}

There's loads of fprintfs() throughout this file - it really needs to
be fixed to use error_report() 


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|