[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH v3 03/14] plat/virtio: Skeleton for virtio block driver



Ok, if it was just inspiration, but the code you submitted is all yours, then I 
think we can leave the header as is, but to remove the "/* Taken and adapted 
from virtio-net because of consistency reason. */" line, which creates 
confusion.

What do you think?

-- Felipe

On 11.03.20, 11:17, "Roxana Nicolescu" <nicolescu.roxana1996@xxxxxxxxx> wrote:

    Hi Felipe,
    
    
    I am not sure if that line should stay in there. Indeed, `virtio-net` 
    was a starting point to get familiar with the virtio API, but 
    virtio-block and virtio-net have different implementations.
    
    It is up to you if that line should stay in there, I sincerely don't know.
    
    The virtio-net authors are:
    
    - Dan Williams
    - Martin Lucina
    - Ricardo Koller
    - Razvan Cojocaru <razvan.cojocaru93@xxxxxxxxx>
    - Sharan Santhanam
    
    You found them in `plat/drivers/virtio_net.c`.
    
    
    Thanks,
    
    Roxana
    
    On 3/11/20 11:57 AM, Felipe Huici wrote:
    > Hi Roxana,
    >
    > You wrote: " Taken and adapted from virtio-net because of consistency 
reason". Were there any authors listed in the original file? If so, please let 
us know which, we can fix that on upstreaming.
    >
    > -- Felipe
    >
    > On 11.03.20, 02:38, "Justin He" <Justin.He@xxxxxxx> wrote:
    >
    >      LGTM except for the license statement. I thought Simon or Felipe 
need to
    >      review the license part. Others e.g. functional, LGTM
    >      
    >      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 03/14] plat/virtio: Skeleton for 
virtio block
    >      > driver
    >      >
    >      > This patch introduces the virtio block driver skeleton.
    >      >
    >      > Signed-off-by: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>
    >      > ---
    >      >  plat/drivers/virtio/virtio_blk.c | 58 
++++++++++++++++++++++++++++++++
    >      >  plat/kvm/Config.uk               | 12 ++++++-
    >      >  plat/kvm/Makefile.uk             | 12 +++++++
    >      >  3 files changed, 81 insertions(+), 1 deletion(-)
    >      >  create mode 100644 plat/drivers/virtio/virtio_blk.c
    >      >
    >      > diff --git a/plat/drivers/virtio/virtio_blk.c 
b/plat/drivers/virtio/virtio_blk.c
    >      > new file mode 100644
    >      > index 00000000..89fd1779
    >      > --- /dev/null
    >      > +++ b/plat/drivers/virtio/virtio_blk.c
    >      > @@ -0,0 +1,58 @@
    >      > +/*
    >      > + * Authors: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>
    >      > + *
    >      > + * Copyright (c) 2019, University Politehnica of Bucharest.
    >      > + *
    >      > + * Permission to use, copy, modify, and/or distribute this 
software
    >      > + * for any purpose with or without fee is hereby granted, provided
    >      > + * that the above copyright notice and this permission notice 
appear
    >      > + * in all copies.
    >      > + *
    >      > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
    >      > + * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
    >      > + * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
    >      > THE
    >      > + * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
    >      > + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
    >      > FROM LOSS
    >      > + * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
    >      > + * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
    >      > + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
    >      > + */
    >      > +/* Taken and adapted from virtio-net because of consistency 
reason. */
    >      > +
    >      > +#include <virtio/virtio_bus.h>
    >      > +#include <virtio/virtio_ids.h>
    >      > +
    >      > +#define DRIVER_NAME"virtio-blk"
    >      > +
    >      > +static struct uk_alloc *a;
    >      > +
    >      > +static int virtio_blk_add_dev(struct virtio_dev *vdev)
    >      > +{
    >      > +int rc = 0;
    >      > +
    >      > +UK_ASSERT(vdev != NULL);
    >      > +
    >      > +return rc;
    >      > +}
    >      > +
    >      > +static int virtio_blk_drv_init(struct uk_alloc *drv_allocator)
    >      > +{
    >      > +/* driver initialization */
    >      > +if (!drv_allocator)
    >      > +return -EINVAL;
    >      > +
    >      > +a = drv_allocator;
    >      > +return 0;
    >      > +}
    >      > +
    >      > +static const struct virtio_dev_id vblk_dev_id[] = {
    >      > +{VIRTIO_ID_BLOCK},
    >      > +{VIRTIO_ID_INVALID} /* List Terminator */
    >      > +};
    >      > +
    >      > +static struct virtio_driver vblk_drv = {
    >      > +.dev_ids = vblk_dev_id,
    >      > +.init    = virtio_blk_drv_init,
    >      > +.add_dev = virtio_blk_add_dev
    >      > +};
    >      > +VIRTIO_BUS_REGISTER_DRIVER(&vblk_drv);
    >      > diff --git a/plat/kvm/Config.uk b/plat/kvm/Config.uk
    >      > index 432cdd95..9aa8a632 100644
    >      > --- a/plat/kvm/Config.uk
    >      > +++ b/plat/kvm/Config.uk
    >      > @@ -65,7 +65,7 @@ config VIRTIO_BUS
    >      >  menu "Virtio"
    >      >  config VIRTIO_PCI
    >      >         bool "Virtio PCI device support"
    >      > -       default y if (VIRTIO_NET || VIRTIO_9P)
    >      > +       default y if (VIRTIO_NET || VIRTIO_9P || VIRTIO_BLK)
    >      >         default n
    >      >         depends on KVM_PCI
    >      >         select VIRTIO_BUS
    >      > @@ -83,6 +83,16 @@ config VIRTIO_NET
    >      >         help
    >      >                Virtual network driver.
    >      >
    >      > +config VIRTIO_BLK
    >      > +bool "Virtio Block Device"
    >      > +default y if LIBUKBLKDEV
    >      > +default n
    >      > +depends on LIBUKBLKDEV
    >      > +select VIRTIO_BUS
    >      > +select LIBUKGLIST
    >      > +help
    >      > +Virtual block driver.
    >      > +
    >      >  config VIRTIO_9P
    >      >         bool "Virtio 9P device"
    >      >         default y if LIBUK9P
    >      > diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk
    >      > index 63ed9950..7f07f22f 100644
    >      > --- a/plat/kvm/Makefile.uk
    >      > +++ b/plat/kvm/Makefile.uk
    >      > @@ -10,6 +10,7 @@ $(eval $(call addplatlib,kvm,libkvmplat))
    >      >  $(eval $(call addplatlib_s,kvm,libkvmpci,$(CONFIG_KVM_PCI)))
    >      >  $(eval $(call addplatlib_s,kvm,libkvmvirtio,$(CONFIG_VIRTIO_BUS)))
    >      >  $(eval $(call 
addplatlib_s,kvm,libkvmvirtionet,$(CONFIG_VIRTIO_NET)))
    >      > +$(eval $(call 
addplatlib_s,kvm,libkvmvirtioblk,$(CONFIG_VIRTIO_BLK)))
    >      >  $(eval $(call 
addplatlib_s,kvm,libkvmvirtio9p,$(CONFIG_VIRTIO_9P)))
    >      >  $(eval $(call addplatlib_s,kvm,libkvmofw,$(CONFIG_LIBOFW)))
    >      >  $(eval $(call addplatlib_s,kvm,libkvmgicv2,$(CONFIG_LIBGICV2)))
    >      > @@ -139,6 +140,17 @@ LIBKVMVIRTIONET_ASINCLUDES-y   += -
    >      > I$(UK_PLAT_DRIVERS_BASE)/include
    >      >  LIBKVMVIRTIONET_CINCLUDES-y    += 
-I$(UK_PLAT_DRIVERS_BASE)/include
    >      >  LIBKVMVIRTIONET_SRCS-y +=\
    >      >  $(UK_PLAT_DRIVERS_BASE)/virtio/virtio_net.c
    >      > +##
    >      > +## Virtio BLK library definition
    >      > +##
    >      > +LIBKVMVIRTIOBLK_ASINCLUDES-y   += -I$(LIBKVMPLAT_BASE)/include
    >      > +LIBKVMVIRTIOBLK_CINCLUDES-y    += -I$(LIBKVMPLAT_BASE)/include
    >      > +LIBKVMVIRTIOBLK_ASINCLUDES-y   += -I$(UK_PLAT_COMMON_BASE)/include
    >      > +LIBKVMVIRTIOBLK_CINCLUDES-y    += -I$(UK_PLAT_COMMON_BASE)/include
    >      > +LIBKVMVIRTIOBLK_ASINCLUDES-y   += 
-I$(UK_PLAT_DRIVERS_BASE)/include
    >      > +LIBKVMVIRTIOBLK_CINCLUDES-y    += 
-I$(UK_PLAT_DRIVERS_BASE)/include
    >      > +LIBKVMVIRTIOBLK_SRCS-y +=\
    >      > +$(UK_PLAT_DRIVERS_BASE)/virtio/virtio_blk.c
    >      >
    >      >  ##
    >      >  ## Virtio 9P library definition
    >      > --
    >      > 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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.