RE: [PATCH 2/2] virtio-blk: support BLKSECDISCARD

["Qi, Yadong" <yadong.qi@intel.com>]

> The Linux block layer shares the DISCARD queue limits with SECDISCARD.
> That's different from BLKZEROOUT (QEMU's WRITE_ZEROES). This is a Linux
> implementation detail but I guess virtio-blk can share the DISCARD limits with
> SECDISCARD too...
> 
> > @@ -622,6 +628,7 @@ static int virtio_blk_handle_request(VirtIOBlockReq
> *req, MultiReqBuffer *mrb)
> >      unsigned out_num = req->elem.out_num;
> >      VirtIOBlock *s = req->dev;
> >      VirtIODevice *vdev = VIRTIO_DEVICE(s);
> > +    bool is_secdiscard = false;
> >
> >      if (req->elem.out_num < 1 || req->elem.in_num < 1) {
> >          virtio_error(vdev, "virtio-blk missing headers"); @@ -722,6
> > +729,9 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req,
> MultiReqBuffer *mrb)
> >       * VIRTIO_BLK_T_OUT flag set. We masked this flag in the switch statement,
> >       * so we must mask it for these requests, then we will check if it is set.
> >       */
> > +    case VIRTIO_BLK_T_SECDISCARD & ~VIRTIO_BLK_T_OUT:
> > +        is_secdiscard = true;
> > +        __attribute__((fallthrough));
> 
> The DISCARD case doesn't use __attribute__((fallthrough)) so this is inconsistent.
> QEMU doesn't use __attribute__((fallthrough)) so I suggest dropping this.
Sure, will try to drop the fallthrough case.

> 
> > diff --git a/include/standard-headers/linux/virtio_blk.h
> > b/include/standard-headers/linux/virtio_blk.h
> > index 2dcc90826a..c55a07840c 100644
> > --- a/include/standard-headers/linux/virtio_blk.h
> > +++ b/include/standard-headers/linux/virtio_blk.h
> > @@ -40,6 +40,7 @@
> >  #define VIRTIO_BLK_F_MQ		12	/* support more than one vq
> */
> >  #define VIRTIO_BLK_F_DISCARD	13	/* DISCARD is supported */
> >  #define VIRTIO_BLK_F_WRITE_ZEROES	14	/* WRITE ZEROES is
> supported */
> > +#define VIRTIO_BLK_F_SECDISCARD	15	/* WRITE ZEROES is supported
> */
> 
> The comment is copy-pasted from WRITE_ZEROES.
Will fix it.