Re: [PATCH 1/1] virtio: disable partitions scanning for no partitions block

[Iurii Kamenev]

Thanks for your remark. I guess it is possible, I will try to rewrite it 
that way.

24.05.2021 22:41, Paolo Bonzini пишет:
> On 24/05/21 21:34, Юрий Каменев wrote:
> > Hi
> >
> >     Is your goal to avoid accidentally detecting partitions because it's
> >     confusing when that happens?
> >
> > The main goal is reducing the kernel start time. It might be use 
> > useful in tiny systems that use, for example, squashfs images with 
> > certainly no partitions. Disabling partitions scanning for these 
> > images can save a few tens of milliseconds which can be a significant 
> > acceleration for starting such systems.
>
> Perhaps that could be configured in the image, for example in the 
> kernel command line?
>
> Paolo
>
> > 24.05.2021, 17:29, "Stefan Hajnoczi" <stefanha@xxxxxxxxxx>:
> >
> >     On Thu, May 20, 2021 at 04:39:08PM +0300, Yury Kamenev wrote:
> >
> >     Hi,
> >     Is there a VIRTIO spec change for the new VIRTIO_BLK_F_NO_PS feature
> >     bit? Please send one:
> > https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio#feedback
> > <https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio#feedback>
> >
> >     GENHD_FL_NO_PART_SCAN is not used much in other drivers. This 
> > makes me
> >     wonder if the same use case is addressed through other means with 
> > SCSI,
> >     NVMe, etc devices. Maybe Christoph or Jens can weigh in on whether
> >     adding a bit to disable partition scanning for a virtio-blk fits 
> > into
> >     the big picture?
> >
> >     Is your goal to avoid accidentally detecting partitions because it's
> >     confusing when that happens?
> >
> >     VIRTIO is currently undergoing auditing and changes to support 
> > untrusted
> >     devices. From that perspective adding a device feature bit to 
> > disable
> >     partition scanning does not help protect the guest from an untrusted
> >     disk. The guest cannot trust the device, instead the guest itself 
> > would
> >     need to be configured to avoid partition scanning of untrusted 
> > devices.
> >
> >     Stefan
> >
> >           Signed-off-by: Yury Kamenev <damtev@xxxxxxxxxxxxxx
> >         <mailto:damtev@xxxxxxxxxxxxxx>>
> >           ---
> >            drivers/block/virtio_blk.c | 6 ++++++
> >            include/uapi/linux/virtio_blk.h | 1 +
> >            2 files changed, 7 insertions(+)
> >
> >           diff --git a/drivers/block/virtio_blk.c
> >         b/drivers/block/virtio_blk.c
> >           index b9fa3ef5b57c..17edcfee2208 100644
> >           --- a/drivers/block/virtio_blk.c
> >           +++ b/drivers/block/virtio_blk.c
> >           @@ -799,6 +799,10 @@ static int virtblk_probe(struct
> >         virtio_device *vdev)
> >                    vblk->disk->flags |= GENHD_FL_EXT_DEVT;
> >                    vblk->index = index;
> >
> >           + /*Disable partitions scanning for no-partitions block*/
> >
> >
> >     Formatting cleanup and rephrasing:
> >
> >        /* Disable partition scanning for devices with no partitions */
> >
> >           + if (virtio_has_feature(vdev, VIRTIO_BLK_F_NO_PS))
> >
> >
> >     I suggest user a more obvious name:
> >
> >        VIRTIO_BLK_F_NO_PART_SCAN
> >
> >           + vblk->disk->flags |= GENHD_FL_NO_PART_SCAN;
> >           +
> >                    /* configure queue flush support */
> >                    virtblk_update_cache_mode(vdev);
> >
> >           @@ -977,6 +981,7 @@ static unsigned int features_legacy[] = {
> >                    VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
> >                    VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY,
> >         VIRTIO_BLK_F_CONFIG_WCE,
> >                    VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD,
> >         VIRTIO_BLK_F_WRITE_ZEROES,
> >           + VIRTIO_BLK_F_NO_PS,
> >            }
> >            ;
> >            static unsigned int features[] = {
> >           @@ -984,6 +989,7 @@ static unsigned int features[] = {
> >                    VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
> >                    VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY,
> >         VIRTIO_BLK_F_CONFIG_WCE,
> >                    VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD,
> >         VIRTIO_BLK_F_WRITE_ZEROES,
> >           + VIRTIO_BLK_F_NO_PS,
> >            };
> >
> >            static struct virtio_driver virtio_blk = {
> >           diff --git a/include/uapi/linux/virtio_blk.h
> >         b/include/uapi/linux/virtio_blk.h
> >           index d888f013d9ff..f197d07afb05 100644
> >           --- a/include/uapi/linux/virtio_blk.h
> >           +++ b/include/uapi/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_NO_PS 16 /* No partitions */
> >
> >            /* Legacy feature bits */
> >            #ifndef VIRTIO_BLK_NO_LEGACY
> >           --
> >           2.24.3 (Apple Git-128)