diff mbox series

[v4,1/2] selftests: kvm: Add vm_get_fd() in kvm_util

Message ID 20200408220818.4306-2-wainersm@redhat.com (mailing list archive)
State New
Headers show
Series selftests: kvm: Introduce the mem_slot_test test | expand

Commit Message

Wainer dos Santos Moschetta April 8, 2020, 10:08 p.m. UTC
Introduces the vm_get_fd() function in kvm_util which returns
the VM file descriptor.

Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 tools/testing/selftests/kvm/include/kvm_util.h | 1 +
 tools/testing/selftests/kvm/lib/kvm_util.c     | 5 +++++
 2 files changed, 6 insertions(+)

Comments

Krish Sadhukhan April 9, 2020, 1:25 a.m. UTC | #1
On 4/8/20 3:08 PM, Wainer dos Santos Moschetta wrote:
> Introduces the vm_get_fd() function in kvm_util which returns
> the VM file descriptor.
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>   tools/testing/selftests/kvm/include/kvm_util.h | 1 +
>   tools/testing/selftests/kvm/lib/kvm_util.c     | 5 +++++
>   2 files changed, 6 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index a99b875f50d2..4e122819ee24 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -254,6 +254,7 @@ bool vm_is_unrestricted_guest(struct kvm_vm *vm);
>   unsigned int vm_get_page_size(struct kvm_vm *vm);
>   unsigned int vm_get_page_shift(struct kvm_vm *vm);
>   unsigned int vm_get_max_gfn(struct kvm_vm *vm);
> +int vm_get_fd(struct kvm_vm *vm);
>   
>   unsigned int vm_calc_num_guest_pages(enum vm_guest_mode mode, size_t size);
>   unsigned int vm_num_host_pages(enum vm_guest_mode mode, unsigned int num_guest_pages);
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 8a3523d4434f..3e36a1eb8771 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -1734,6 +1734,11 @@ unsigned int vm_get_max_gfn(struct kvm_vm *vm)
>   	return vm->max_gfn;
>   }
>   
> +int vm_get_fd(struct kvm_vm *vm)
> +{
> +        return vm->fd;
> +}
> +


I am just trying to understand why we need a separate function when the 
'vm' variable is all local within the same file. There are a number of 
places in kvm_util.c where it is used directly.

>   static unsigned int vm_calc_num_pages(unsigned int num_pages,
>   				      unsigned int page_shift,
>   				      unsigned int new_page_shift,
Wainer dos Santos Moschetta April 9, 2020, 2:45 a.m. UTC | #2
On 4/8/20 10:25 PM, Krish Sadhukhan wrote:
>
> On 4/8/20 3:08 PM, Wainer dos Santos Moschetta wrote:
>> Introduces the vm_get_fd() function in kvm_util which returns
>> the VM file descriptor.
>>
>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>> ---
>>   tools/testing/selftests/kvm/include/kvm_util.h | 1 +
>>   tools/testing/selftests/kvm/lib/kvm_util.c     | 5 +++++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h 
>> b/tools/testing/selftests/kvm/include/kvm_util.h
>> index a99b875f50d2..4e122819ee24 100644
>> --- a/tools/testing/selftests/kvm/include/kvm_util.h
>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
>> @@ -254,6 +254,7 @@ bool vm_is_unrestricted_guest(struct kvm_vm *vm);
>>   unsigned int vm_get_page_size(struct kvm_vm *vm);
>>   unsigned int vm_get_page_shift(struct kvm_vm *vm);
>>   unsigned int vm_get_max_gfn(struct kvm_vm *vm);
>> +int vm_get_fd(struct kvm_vm *vm);
>>     unsigned int vm_calc_num_guest_pages(enum vm_guest_mode mode, 
>> size_t size);
>>   unsigned int vm_num_host_pages(enum vm_guest_mode mode, unsigned 
>> int num_guest_pages);
>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c 
>> b/tools/testing/selftests/kvm/lib/kvm_util.c
>> index 8a3523d4434f..3e36a1eb8771 100644
>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
>> @@ -1734,6 +1734,11 @@ unsigned int vm_get_max_gfn(struct kvm_vm *vm)
>>       return vm->max_gfn;
>>   }
>>   +int vm_get_fd(struct kvm_vm *vm)
>> +{
>> +        return vm->fd;
>> +}
>> +
>
>
> I am just trying to understand why we need a separate function when 
> the 'vm' variable is all local within the same file. There are a 
> number of places in kvm_util.c where it is used directly.


The problem is to access of kvm_vm attributes outside of kvm_utils.c. 
For example, if I try to vm->fd in my test I get the compiler error:

mem_slot_test.c: In function ‘test_add_max_slots’:
mem_slot_test.c:62:16: error: dereferencing pointer to incomplete type 
‘struct kvm_vm’
   ret = ioctl(vm->fd, KVM_SET_USER_MEMORY_REGION,
                 ^~

>
>
>>   static unsigned int vm_calc_num_pages(unsigned int num_pages,
>>                         unsigned int page_shift,
>>                         unsigned int new_page_shift,
>
Krish Sadhukhan April 9, 2020, 5:57 p.m. UTC | #3
On 4/8/20 7:45 PM, Wainer dos Santos Moschetta wrote:
>
> On 4/8/20 10:25 PM, Krish Sadhukhan wrote:
>>
>> On 4/8/20 3:08 PM, Wainer dos Santos Moschetta wrote:
>>> Introduces the vm_get_fd() function in kvm_util which returns
>>> the VM file descriptor.
>>>
>>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>>> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>>> ---
>>>   tools/testing/selftests/kvm/include/kvm_util.h | 1 +
>>>   tools/testing/selftests/kvm/lib/kvm_util.c     | 5 +++++
>>>   2 files changed, 6 insertions(+)
>>>
>>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h 
>>> b/tools/testing/selftests/kvm/include/kvm_util.h
>>> index a99b875f50d2..4e122819ee24 100644
>>> --- a/tools/testing/selftests/kvm/include/kvm_util.h
>>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
>>> @@ -254,6 +254,7 @@ bool vm_is_unrestricted_guest(struct kvm_vm *vm);
>>>   unsigned int vm_get_page_size(struct kvm_vm *vm);
>>>   unsigned int vm_get_page_shift(struct kvm_vm *vm);
>>>   unsigned int vm_get_max_gfn(struct kvm_vm *vm);
>>> +int vm_get_fd(struct kvm_vm *vm);
>>>     unsigned int vm_calc_num_guest_pages(enum vm_guest_mode mode, 
>>> size_t size);
>>>   unsigned int vm_num_host_pages(enum vm_guest_mode mode, unsigned 
>>> int num_guest_pages);
>>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c 
>>> b/tools/testing/selftests/kvm/lib/kvm_util.c
>>> index 8a3523d4434f..3e36a1eb8771 100644
>>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
>>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
>>> @@ -1734,6 +1734,11 @@ unsigned int vm_get_max_gfn(struct kvm_vm *vm)
>>>       return vm->max_gfn;
>>>   }
>>>   +int vm_get_fd(struct kvm_vm *vm)
>>> +{
>>> +        return vm->fd;
>>> +}
>>> +
>>
>>
>> I am just trying to understand why we need a separate function when 
>> the 'vm' variable is all local within the same file. There are a 
>> number of places in kvm_util.c where it is used directly.
>
>
> The problem is to access of kvm_vm attributes outside of kvm_utils.c. 
> For example, if I try to vm->fd in my test I get the compiler error:
>
> mem_slot_test.c: In function ‘test_add_max_slots’:
> mem_slot_test.c:62:16: error: dereferencing pointer to incomplete type 
> ‘struct kvm_vm’
>   ret = ioctl(vm->fd, KVM_SET_USER_MEMORY_REGION,
>                 ^~


My bad !  I missed the fact that the structure is defined in 
kvm_util_internal.h and not in kvm_util.h.


Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>

>
>>
>>
>>>   static unsigned int vm_calc_num_pages(unsigned int num_pages,
>>>                         unsigned int page_shift,
>>>                         unsigned int new_page_shift,
>>
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index a99b875f50d2..4e122819ee24 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -254,6 +254,7 @@  bool vm_is_unrestricted_guest(struct kvm_vm *vm);
 unsigned int vm_get_page_size(struct kvm_vm *vm);
 unsigned int vm_get_page_shift(struct kvm_vm *vm);
 unsigned int vm_get_max_gfn(struct kvm_vm *vm);
+int vm_get_fd(struct kvm_vm *vm);
 
 unsigned int vm_calc_num_guest_pages(enum vm_guest_mode mode, size_t size);
 unsigned int vm_num_host_pages(enum vm_guest_mode mode, unsigned int num_guest_pages);
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 8a3523d4434f..3e36a1eb8771 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1734,6 +1734,11 @@  unsigned int vm_get_max_gfn(struct kvm_vm *vm)
 	return vm->max_gfn;
 }
 
+int vm_get_fd(struct kvm_vm *vm)
+{
+        return vm->fd;
+}
+
 static unsigned int vm_calc_num_pages(unsigned int num_pages,
 				      unsigned int page_shift,
 				      unsigned int new_page_shift,