All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Damir Shaikhutdinov <Damir.Shaikhutdinov@opensynergy.com>
Cc: "virtio-comment@lists.oasis-open.org"
	<virtio-comment@lists.oasis-open.org>
Subject: Re: [virtio-comment] Block Device VIRTIO_BLK_F_RO clarification needed.
Date: Mon, 20 Jan 2020 11:21:36 -0500	[thread overview]
Message-ID: <20200120111648-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <21D64D2457144C4AA23D071E2F6A51EC01592F5BFC@MXS01.open-synergy.com>

On Mon, Jan 20, 2020 at 11:45:05AM +0000, Damir Shaikhutdinov wrote:
> Hello!
> 
> While reading section 5.2 of the spec, I found that VIRTIO_BLK_F_RO feature could use
> some additional clarification regarding handling requests when it was offered.
> 
> This RO feature is described as "Device is read only" (Section 5.2.3). There are some explicit requirements for this feature, but they only describe a "write request".
> 
> 5.2.5 p3 says "If the VIRTIO_BLK_F_RO feature is set by the device, any write requests will fail."
> 
> 5.2.6 says "The type of the request is either a read (VIRTIO_BLK_T_IN), a write (VIRTIO_BLK_T_OUT) ...", so "write request" mentioned in 5.2.5 p3 is VIRTIO_BLK_T_OUT.
> 
> 5.2.6.2 says "A device MUST set the status byte to VIRTIO_BLK_S_IOERR for a write request if the VIRTIO_BLK_F_RO
> feature if offered, and MUST NOT write any data."
> 
> What is missing here is a requirement for features and other types of requests in case of VIRTIO_BLK_F_RO:
> 
> * config.writeback field (guarded by VIRTIO_BLK_F_CONFIG_WCE feature)
> * VIRTIO_BLK_T_ FLUSH (guarded by VIRTIO_BLK_F_FLUSH feature)
> * VIRTIO_BLK_T_DISCARD (guarded by VIRTIO_BLK_F_DISCARD feature)
> * VIRTIO_BLK_T_WRITE_ZEROES (guarded by VIRTIO_BLK_F_WRITE_ZEROES feature)
> 
> Configuration field, requests and features described above make no sense in presence of VIRTIO_BLK_F_RO.
> 
> This can be clarified in several ways, for example:
> 
> 1. Device should not offer CONFIG_WCE, FLUSH, DISCARD and WRITE_ZEROES features if it offers RO feature (partially contradicts 5.2.5.2 "Device SHOULD always offer VIRTIO_BLK_F_FLUSH")
> 2. Device must set the status byte to IOERR for FLUSH, DISCARD and WRITE_ZEROES requests if it offers RO feature.
> 
> 
> What do you think about that?

I guess flush can even be allowed, it's harmless if nothing is written,
right? I agree DISCARD and WRITE_ZEROES must fail. And set VIRTIO_BLK_S_IOERR
I guess?

It's too late to disallow configurations such as a combination of VIRTIO_BLK_F_CONFIG_WCE
and VIRTIO_BLK_F_RO, devices out there might be setting this combination.

> With best regards,
> Damir Shaikhutdinov
> 
> Please mind our privacy notice<https://www.opensynergy.com/datenschutzerklaerung/privacy-notice-for-business-partners-pursuant-to-article-13-of-the-general-data-protection-regulation-gdpr/> pursuant to Art. 13 GDPR. // Unsere Hinweise zum Datenschutz gem. Art. 13 DSGVO finden Sie hier.<https://www.opensynergy.com/de/datenschutzerklaerung/datenschutzhinweise-fuer-geschaeftspartner-gem-art-13-dsgvo/>
> 
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
> 
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
> 
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


  reply	other threads:[~2020-01-20 16:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-20 11:45 [virtio-comment] Block Device VIRTIO_BLK_F_RO clarification needed Damir Shaikhutdinov
2020-01-20 16:21 ` Michael S. Tsirkin [this message]
2020-01-21 16:08   ` Stefan Hajnoczi
2020-01-22  5:53     ` Michael S. Tsirkin
2020-01-22 10:12       ` Stefan Hajnoczi
2020-01-23 11:08         ` Damir Shaikhutdinov

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=20200120111648-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=Damir.Shaikhutdinov@opensynergy.com \
    --cc=virtio-comment@lists.oasis-open.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.