The current code has an early exit for PV guests, but it returns 0 having done
nothing.

Fixes: 524a98c2ac5 ("public / x86: introduce __HYPERCALL_dm_op...")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Ian Jackson <iwj@xenproject.org>
CC: Paul Durrant <paul@xen.org>

For 4.15.  Found while testing XEN_DMOP_nr_vcpus.  Needs backporting to all
stable releases.
---
 xen/arch/x86/hvm/dm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index 5bc172a5d4..612749442e 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -365,6 +365,7 @@ int dm_op(const struct dmop_args *op_args)
     if ( rc )
         return rc;
 
+    rc = -EINVAL;
     if ( !is_hvm_domain(d) )
         goto out;
 
-- 
2.11.0

Andrew Cooper writes ("[PATCH for-4.15] x86/dmop: Properly fail for PV guests"):
> The current code has an early exit for PV guests, but it returns 0 having done
> nothing.

Reviewed-by: Ian Jackson <iwj@xenproject.org>

> diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
> index 5bc172a5d4..612749442e 100644
> --- a/xen/arch/x86/hvm/dm.c
> +++ b/xen/arch/x86/hvm/dm.c
> @@ -365,6 +365,7 @@ int dm_op(const struct dmop_args *op_args)
>      if ( rc )
>          return rc;
>  
> +    rc = -EINVAL;
>      if ( !is_hvm_domain(d) )
>          goto out;

Is this style, of setting rc outside the if, the standard hypervisor
style ?

Ian.

On 25/02/2021 17:22, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH for-4.15] x86/dmop: Properly fail for PV guests"):
>> The current code has an early exit for PV guests, but it returns 0 having done
>> nothing.
> Reviewed-by: Ian Jackson <iwj@xenproject.org>

Thanks.

>
>> diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
>> index 5bc172a5d4..612749442e 100644
>> --- a/xen/arch/x86/hvm/dm.c
>> +++ b/xen/arch/x86/hvm/dm.c
>> @@ -365,6 +365,7 @@ int dm_op(const struct dmop_args *op_args)
>>      if ( rc )
>>          return rc;
>>  
>> +    rc = -EINVAL;
>>      if ( !is_hvm_domain(d) )
>>          goto out;
> Is this style, of setting rc outside the if, the standard hypervisor
> style ?

If you think the cyclomatic complexity is bad in libxl...

This is the prevailing style in this function.  Its common, but not
universal style.

~Andrew

Andrew Cooper writes ("Re: [PATCH for-4.15] x86/dmop: Properly fail for PV guests"):
> On 25/02/2021 17:22, Ian Jackson wrote:
> > Andrew Cooper writes ("[PATCH for-4.15] x86/dmop: Properly fail for PV guests"):
> >> The current code has an early exit for PV guests, but it returns 0 having done
> >> nothing.
> > Reviewed-by: Ian Jackson <iwj@xenproject.org>
> 
> Thanks.

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

On 25.02.2021 18:09, Andrew Cooper wrote:
> The current code has an early exit for PV guests, but it returns 0 having done
> nothing.
> 
> Fixes: 524a98c2ac5 ("public / x86: introduce __HYPERCALL_dm_op...")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>