Re: [Minios-devel] [UNIKRAFT PATCH v3 04/14] plat/drivers: Register virtio-blk with libukblkdev

[Justin He <Justin.He@xxxxxxx>]

LGTM except for one minor issue below, but you can fix it in the future.

Reviewed-by: Jia He <justin.he@xxxxxxx>

--
Cheers,
Justin (Jia He)


> -----Original Message-----
> From: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>
> Sent: Friday, March 6, 2020 4:30 AM
> To: minios-devel@xxxxxxxxxxxxx
> Cc: Justin He <Justin.He@xxxxxxx>; Roxana Nicolescu
> <nicolescu.roxana1996@xxxxxxxxx>
> Subject: [UNIKRAFT PATCH v3 04/14] plat/drivers: Register virtio-blk with
> libukblkdev
>
> This patch registers virtio-blk with libukblkdev.
>
> Signed-off-by: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>
> ---
>  plat/drivers/virtio/virtio_blk.c | 38 ++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/plat/drivers/virtio/virtio_blk.c 
> b/plat/drivers/virtio/virtio_blk.c
> index 89fd1779..884f86d9 100644
> --- a/plat/drivers/virtio/virtio_blk.c
> +++ b/plat/drivers/virtio/virtio_blk.c
> @@ -19,20 +19,58 @@
>   */
>  /* Taken and adapted from virtio-net because of consistency reason. */
>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <uk/print.h>
> +#include <errno.h>
> +#include <fcntl.h>
>  #include <virtio/virtio_bus.h>
>  #include <virtio/virtio_ids.h>
> +#include <uk/blkdev.h>
> +#include <uk/blkdev_driver.h>
>
>  #define DRIVER_NAME"virtio-blk"
>
> +#define to_virtioblkdev(bdev) \
> +__containerof(bdev, struct virtio_blk_device, blkdev)
> +
> +
>  static struct uk_alloc *a;
> +static const char *drv_name = DRIVER_NAME;
> +
> +struct virtio_blk_device {
> +/* Pointer to Unikraft Block Device */
> +struct uk_blkdev blkdev;
How about using struct uk_blkdev* Instead of struct uk_blkdev here?
Although it is not a big issue now, struct * make it more graceful.

> +/* The blkdevice identifier */
> +__u16 uid;
> +};
>
>  static int virtio_blk_add_dev(struct virtio_dev *vdev)
>  {
> +struct virtio_blk_device *vbdev;
>  int rc = 0;
>
>  UK_ASSERT(vdev != NULL);
>
> +vbdev = uk_calloc(a, 1, sizeof(*vbdev));
> +if (!vbdev)
> +return -ENOMEM;
> +
> +rc = uk_blkdev_drv_register(&vbdev->blkdev, a, drv_name);
> +if (rc < 0) {
> +uk_pr_err("Failed to register virtio_blk device: %d\n", rc);
> +goto err_out;
> +}
> +
> +vbdev->uid = rc;
> +uk_pr_info("Virtio-blk device registered with libukblkdev\n");
> +
> +out:
>  return rc;
> +err_out:
> +uk_free(a, vbdev);
> +goto out;
>  }
>
>  static int virtio_blk_drv_init(struct uk_alloc *drv_allocator)
> --
> 2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel