Re: [PATCH 6/7] target/i386: Implement mc->kvm_type() to get VM type

From: Paolo Bonzini <pbonzini@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, xiaoyao.li@intel.com, michael.roth@amd.com
Subject: Re: [PATCH 6/7] target/i386: Implement mc->kvm_type() to get VM type
Date: Tue, 19 Mar 2024 15:29:14 +0100	[thread overview]
Message-ID: <CABgObfaKe=EE0N6YHTqpaoK-LaACpzSWykWB+mo=_KDMCnFd1Q@mail.gmail.com> (raw)
In-Reply-To: <ZfmgybXHbCTbisum@redhat.com>

On Tue, Mar 19, 2024 at 3:27 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Mar 19, 2024 at 03:25:53PM +0100, Paolo Bonzini wrote:
> > On Tue, Mar 19, 2024 at 3:15 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > > +int kvm_get_vm_type(MachineState *ms, const char *vm_type)
> > >
> > > The 'vm_type' parameter is never used here. What value is it expected
> > > to have, and should be diagnosing an error if some unexpected value
> > > is provided.
> >
> > It's the value of the kvm-type machine property, if any; but no x86
> > machine defines one, so right now it's always NULL. I left it in
> > because then it's clearer than this is an implementation of
> > mc->kvm_type, but I can remove it or pass it down to
> > x86_confidential_guest_kvm_type().
>
> If we expect it to always be NULL, lets validate that is the
> case and error_report + exit, if not.

I think it's enough to have an assertion in x86_kvm_type():

    /*
     * No x86 machine has a kvm-type property.  If one is added that has
     * it, it should call kvm_get_vm_type() directly or not use it at all.
     */
    assert(vm_type == NULL);

Paolo