linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Iurii Kamenev <damtev@yandex-team.ru>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Cc: "mst@redhat.com" <mst@redhat.com>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"virtualization@lists.linux-foundation.org" 
	<virtualization@lists.linux-foundation.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH 1/1] virtio: disable partitions scanning for no partitions block
Date: Tue, 25 May 2021 15:00:37 +0300	[thread overview]
Message-ID: <d7665bc7-1806-ba88-9d7d-0c1352df43d3@yandex-team.ru> (raw)
In-Reply-To: <21b08ea7-71c6-1b4b-3833-1c51d0e1d310@redhat.com>

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@redhat.com>:
>>
>>     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@yandex-team.ru
>>         <mailto:damtev@yandex-team.ru>>
>>           ---
>>            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)
>>
>

  reply	other threads:[~2021-05-25 12:04 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-20 13:39 [PATCH 0/1] virtio: disable partitions scanning for no partitions block Yury Kamenev
2021-05-20 13:39 ` [PATCH 1/1] " Yury Kamenev
2021-05-24 14:29   ` Stefan Hajnoczi
2021-05-24 14:56     ` Christoph Hellwig
2021-05-24 16:25       ` Ulf Hansson
     [not found]     ` <90021621883891@mail.yandex-team.ru>
2021-05-24 19:41       ` Paolo Bonzini
2021-05-25 12:00         ` Iurii Kamenev [this message]
2021-07-15  9:47 [PATCH 0/1] " Yury Kamenev
2021-07-15  9:47 ` [PATCH 1/1] " Yury Kamenev
2021-07-15 11:22   ` Paolo Bonzini
2021-07-16  1:09   ` kernel test robot
2021-07-16  2:57   ` Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d7665bc7-1806-ba88-9d7d-0c1352df43d3@yandex-team.ru \
    --to=damtev@yandex-team.ru \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=jasowang@redhat.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).