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

[Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>]

On 3/11/20 3:45 AM, Justin He wrote:
> 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.
It is not a pointer, in order to be able to get the reference for 
virtio_blk_device.
See `to_virtioblkdev` macro.

> > +/* 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