[PATCH v3 1/1] virt: acrn: Mark the uuid field as unused

[PATCH v3 1/1] virt: acrn: Mark the uuid field as unused

[Andy Shevchenko]

After the commits for userspace (see Link tags below) the uuid field is
not being used in the ACRN code. Update kernel to reflect these changes.
I.e. we do the following:
- adding a comment explaining that it's not used anymore
- replacing the specific type by a raw buffer
- updating the example code accordingly

The advertised field confused users and actually never been used. So
the wrong part here is that kernel puts something which userspace never
used and hence this may confuse a reader of this code.

Link: https://github.com/projectacrn/acrn-hypervisor/commit/da0d24326ed6
Link: https://github.com/projectacrn/acrn-hypervisor/commit/bb0327e70097
Fixes: 5b06931d7f8b ("sample/acrn: Introduce a sample of HSM ioctl interface usage")
Fixes: 9c5137aedd11 ("virt: acrn: Introduce VM management interfaces")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: converted to Link tags (Rafael), explained what was wrong (Rafael)
v2: added tag (Fei)
 include/uapi/linux/acrn.h | 5 ++---
 samples/acrn/vm-sample.c  | 3 ---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/acrn.h b/include/uapi/linux/acrn.h
index ccf47ed92500..04fa83647ae5 100644
--- a/include/uapi/linux/acrn.h
+++ b/include/uapi/linux/acrn.h
@@ -12,7 +12,6 @@
 #define _UAPI_ACRN_H
 
 #include <linux/types.h>
-#include <linux/uuid.h>
 
 #define ACRN_IO_REQUEST_MAX		16
 
@@ -186,7 +185,7 @@ struct acrn_ioreq_notify {
  * @reserved0:		Reserved and must be 0
  * @vcpu_num:		Number of vCPU in the VM. Return from hypervisor.
  * @reserved1:		Reserved and must be 0
- * @uuid:		UUID of the VM. Pass to hypervisor directly.
+ * @uuid:		Reserved (used to be UUID of the VM)
  * @vm_flag:		Flag of the VM creating. Pass to hypervisor directly.
  * @ioreq_buf:		Service VM GPA of I/O request buffer. Pass to
  *			hypervisor directly.
@@ -198,7 +197,7 @@ struct acrn_vm_creation {
 	__u16	reserved0;
 	__u16	vcpu_num;
 	__u16	reserved1;
-	guid_t	uuid;
+	__u8	uuid[16];
 	__u64	vm_flag;
 	__u64	ioreq_buf;
 	__u64	cpu_affinity;
diff --git a/samples/acrn/vm-sample.c b/samples/acrn/vm-sample.c
index b2dad47a77a0..7abd68b20153 100644
--- a/samples/acrn/vm-sample.c
+++ b/samples/acrn/vm-sample.c
@@ -29,8 +29,6 @@ static struct acrn_io_request *io_req_buf = (struct acrn_io_request *)io_request
 
 __u16 vcpu_num;
 __u16 vmid;
-/* POST_STANDARD_VM_UUID1, refer to https://github.com/projectacrn/acrn-hypervisor/blob/master/hypervisor/include/common/vm_uuids.h */
-guid_t vm_uuid = GUID_INIT(0x385479d2, 0xd625, 0xe811, 0x86, 0x4e, 0xcb, 0x7a, 0x18, 0xb3, 0x46, 0x43);
 
 int hsm_fd;
 int is_running = 1;
@@ -63,7 +61,6 @@ int main(int argc, char **argv)
 	}
 	hsm_fd = open("/dev/acrn_hsm", O_RDWR|O_CLOEXEC);
 
-	memcpy(&create_vm.uuid, &vm_uuid, 16);
 	create_vm.ioreq_buf = (__u64)io_req_buf;
 	ret = ioctl(hsm_fd, ACRN_IOCTL_CREATE_VM, &create_vm);
 	printf("Created VM! [%d]\n", ret);
-- 
2.35.1

Re: [PATCH v3 1/1] virt: acrn: Mark the uuid field as unused

[Andy Shevchenko]

On Wed, Aug 31, 2022 at 03:23:56PM +0300, Andy Shevchenko wrote:
> After the commits for userspace (see Link tags below) the uuid field is
> not being used in the ACRN code. Update kernel to reflect these changes.
> I.e. we do the following:
> - adding a comment explaining that it's not used anymore
> - replacing the specific type by a raw buffer
> - updating the example code accordingly
> 
> The advertised field confused users and actually never been used. So
> the wrong part here is that kernel puts something which userspace never
> used and hence this may confuse a reader of this code.

Rafael, I think I have addressed all your concerns.
Anything else should I do?

> Link: https://github.com/projectacrn/acrn-hypervisor/commit/da0d24326ed6
> Link: https://github.com/projectacrn/acrn-hypervisor/commit/bb0327e70097
> Fixes: 5b06931d7f8b ("sample/acrn: Introduce a sample of HSM ioctl interface usage")
> Fixes: 9c5137aedd11 ("virt: acrn: Introduce VM management interfaces")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v3: converted to Link tags (Rafael), explained what was wrong (Rafael)
> v2: added tag (Fei)
>  include/uapi/linux/acrn.h | 5 ++---
>  samples/acrn/vm-sample.c  | 3 ---
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/include/uapi/linux/acrn.h b/include/uapi/linux/acrn.h
> index ccf47ed92500..04fa83647ae5 100644
> --- a/include/uapi/linux/acrn.h
> +++ b/include/uapi/linux/acrn.h
> @@ -12,7 +12,6 @@
>  #define _UAPI_ACRN_H
>  
>  #include <linux/types.h>
> -#include <linux/uuid.h>
>  
>  #define ACRN_IO_REQUEST_MAX		16
>  
> @@ -186,7 +185,7 @@ struct acrn_ioreq_notify {
>   * @reserved0:		Reserved and must be 0
>   * @vcpu_num:		Number of vCPU in the VM. Return from hypervisor.
>   * @reserved1:		Reserved and must be 0
> - * @uuid:		UUID of the VM. Pass to hypervisor directly.
> + * @uuid:		Reserved (used to be UUID of the VM)
>   * @vm_flag:		Flag of the VM creating. Pass to hypervisor directly.
>   * @ioreq_buf:		Service VM GPA of I/O request buffer. Pass to
>   *			hypervisor directly.
> @@ -198,7 +197,7 @@ struct acrn_vm_creation {
>  	__u16	reserved0;
>  	__u16	vcpu_num;
>  	__u16	reserved1;
> -	guid_t	uuid;
> +	__u8	uuid[16];
>  	__u64	vm_flag;
>  	__u64	ioreq_buf;
>  	__u64	cpu_affinity;
> diff --git a/samples/acrn/vm-sample.c b/samples/acrn/vm-sample.c
> index b2dad47a77a0..7abd68b20153 100644
> --- a/samples/acrn/vm-sample.c
> +++ b/samples/acrn/vm-sample.c
> @@ -29,8 +29,6 @@ static struct acrn_io_request *io_req_buf = (struct acrn_io_request *)io_request
>  
>  __u16 vcpu_num;
>  __u16 vmid;
> -/* POST_STANDARD_VM_UUID1, refer to https://github.com/projectacrn/acrn-hypervisor/blob/master/hypervisor/include/common/vm_uuids.h */
> -guid_t vm_uuid = GUID_INIT(0x385479d2, 0xd625, 0xe811, 0x86, 0x4e, 0xcb, 0x7a, 0x18, 0xb3, 0x46, 0x43);
>  
>  int hsm_fd;
>  int is_running = 1;
> @@ -63,7 +61,6 @@ int main(int argc, char **argv)
>  	}
>  	hsm_fd = open("/dev/acrn_hsm", O_RDWR|O_CLOEXEC);
>  
> -	memcpy(&create_vm.uuid, &vm_uuid, 16);
>  	create_vm.ioreq_buf = (__u64)io_req_buf;
>  	ret = ioctl(hsm_fd, ACRN_IOCTL_CREATE_VM, &create_vm);
>  	printf("Created VM! [%d]\n", ret);
> -- 
> 2.35.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/1] virt: acrn: Mark the uuid field as unused

[Rafael J. Wysocki]

On Wed, Aug 31, 2022 at 2:23 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> After the commits for userspace (see Link tags below) the uuid field is
> not being used in the ACRN code. Update kernel to reflect these changes.
> I.e. we do the following:
> - adding a comment explaining that it's not used anymore
> - replacing the specific type by a raw buffer
> - updating the example code accordingly
>
> The advertised field confused users and actually never been used. So
> the wrong part here is that kernel puts something which userspace never
> used and hence this may confuse a reader of this code.
>
> Link: https://github.com/projectacrn/acrn-hypervisor/commit/da0d24326ed6
> Link: https://github.com/projectacrn/acrn-hypervisor/commit/bb0327e70097
> Fixes: 5b06931d7f8b ("sample/acrn: Introduce a sample of HSM ioctl interface usage")
> Fixes: 9c5137aedd11 ("virt: acrn: Introduce VM management interfaces")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Looks good to me:

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

Thanks!

> ---
> v3: converted to Link tags (Rafael), explained what was wrong (Rafael)
> v2: added tag (Fei)
>  include/uapi/linux/acrn.h | 5 ++---
>  samples/acrn/vm-sample.c  | 3 ---
>  2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/acrn.h b/include/uapi/linux/acrn.h
> index ccf47ed92500..04fa83647ae5 100644
> --- a/include/uapi/linux/acrn.h
> +++ b/include/uapi/linux/acrn.h
> @@ -12,7 +12,6 @@
>  #define _UAPI_ACRN_H
>
>  #include <linux/types.h>
> -#include <linux/uuid.h>
>
>  #define ACRN_IO_REQUEST_MAX            16
>
> @@ -186,7 +185,7 @@ struct acrn_ioreq_notify {
>   * @reserved0:         Reserved and must be 0
>   * @vcpu_num:          Number of vCPU in the VM. Return from hypervisor.
>   * @reserved1:         Reserved and must be 0
> - * @uuid:              UUID of the VM. Pass to hypervisor directly.
> + * @uuid:              Reserved (used to be UUID of the VM)
>   * @vm_flag:           Flag of the VM creating. Pass to hypervisor directly.
>   * @ioreq_buf:         Service VM GPA of I/O request buffer. Pass to
>   *                     hypervisor directly.
> @@ -198,7 +197,7 @@ struct acrn_vm_creation {
>         __u16   reserved0;
>         __u16   vcpu_num;
>         __u16   reserved1;
> -       guid_t  uuid;
> +       __u8    uuid[16];
>         __u64   vm_flag;
>         __u64   ioreq_buf;
>         __u64   cpu_affinity;
> diff --git a/samples/acrn/vm-sample.c b/samples/acrn/vm-sample.c
> index b2dad47a77a0..7abd68b20153 100644
> --- a/samples/acrn/vm-sample.c
> +++ b/samples/acrn/vm-sample.c
> @@ -29,8 +29,6 @@ static struct acrn_io_request *io_req_buf = (struct acrn_io_request *)io_request
>
>  __u16 vcpu_num;
>  __u16 vmid;
> -/* POST_STANDARD_VM_UUID1, refer to https://github.com/projectacrn/acrn-hypervisor/blob/master/hypervisor/include/common/vm_uuids.h */
> -guid_t vm_uuid = GUID_INIT(0x385479d2, 0xd625, 0xe811, 0x86, 0x4e, 0xcb, 0x7a, 0x18, 0xb3, 0x46, 0x43);
>
>  int hsm_fd;
>  int is_running = 1;
> @@ -63,7 +61,6 @@ int main(int argc, char **argv)
>         }
>         hsm_fd = open("/dev/acrn_hsm", O_RDWR|O_CLOEXEC);
>
> -       memcpy(&create_vm.uuid, &vm_uuid, 16);
>         create_vm.ioreq_buf = (__u64)io_req_buf;
>         ret = ioctl(hsm_fd, ACRN_IOCTL_CREATE_VM, &create_vm);
>         printf("Created VM! [%d]\n", ret);
> --
> 2.35.1
>