Skip to content

doc/rbd: update front matter#49035

Closed
zdover23 wants to merge 1 commit intoceph:mainfrom
zdover23:wip-doc-2022-11-24-rbd-front-matter
Closed

doc/rbd: update front matter#49035
zdover23 wants to merge 1 commit intoceph:mainfrom
zdover23:wip-doc-2022-11-24-rbd-front-matter

Conversation

@zdover23
Copy link
Contributor

@zdover23 zdover23 commented Nov 23, 2022

Swap the order of the first two paragraphs; make minor improvements.

Signed-off-by: Zac Dover zac.dover@gmail.com

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@zdover23 zdover23 requested a review from a team as a code owner November 23, 2022 16:29
@zdover23 zdover23 requested a review from a team November 23, 2022 16:29
@zdover23
Copy link
Contributor Author

Copy link
Contributor

@idryomov idryomov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing order made more sense to me: first the concept of block-based storage was introduced (including the definition of a block), then Ceph block block device offering was presented. But I'm not a docs guy ;)

multiple OSDs. Ceph block devices leverage :abbr:`RADOS (Reliable Autonomic
Distributed Object Store)` capabilities including snapshotting and replication,
and they provide strong consistency. Ceph block storage clients communicate
with Ceph clusters by means of kernel modules or the ``librbd`` library.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
with Ceph clusters by means of kernel modules or the ``librbd`` library.
with Ceph clusters by means of a Linux kernel module or the ``librbd`` library.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take your point, but I'd like to beef up the paragraph that defines block storage and completely rewrite the last sentence. I'll chew on it and make sure you guys are satisfied before we merge anything here.

Copy link
Contributor

@anthonyeleven anthonyeleven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with Igor's suggestion

@zdover23 zdover23 force-pushed the wip-doc-2022-11-24-rbd-front-matter branch 2 times, most recently from cd5cf02 to 745455e Compare December 1, 2022 03:15
Copy link
Contributor

@anthonyeleven anthonyeleven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion

RBD (the Ceph block device) is a service that provides thin-provisioned,
resizable block devices that offer snapshotting and cloning as features.

A block is a sequence of bytes (often 512). Block-based storage interfaces are
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might remove A block is a sequence of bytes (often 512). , I'm not sure that it adds value here.

Copy link
Contributor Author

@zdover23 zdover23 Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I've removed it.

In the main, I don't like this paragraph or the sentences in it. The first sentence points up the maturity and prevalence of the method of block-based storage and then lists physical media upon which block storage has been implemented. Even if I agreed with the inclusion in this introduction to RBD of this idea regarding the pervasiveness of block devices, I question the value in 2022 of emphasizing the maturity of the block-based storage model. I think it puts the text in danger of pointing out what is taken by most people nowadays as a matter of course and therefore unremarkable. And I object outright to the use of the adjective "common" where "prevalent" is meant, but that's just because "common" has a potentially negative connotation if it is accidentally read as referring to class and not read as referring to frequency of use or implementation. It's a very nit-picky semantic distinction, but if we can avoid this kind of ambiguity, why shouldn't we? I might recast this sentence charitably by writing "Block-based storage interfaces are used to store data on HDDs, SSDs, CDs, floppy disks, and tape. The block storage model, having existed for decades, is mature and reliable. Block storage is perhaps the most prevalent form of storage in the early twenty-first century.".

The sentence that follows is a real howler:

The ubiquity of block device interfaces is a perfect fit for interacting with mass data storage including Ceph.

I think that this means to say nothing more grand than that Ceph can work with block device interfaces. But it includes this three-pipe problem of a phrasal verb, "is a perfect fit for interacting with", which seems to me the kind of thing that no one could have actually intended to write. Admittedly, I can think of situations in which the phrase "perfect fit" is appropriate: This shirt is a perfect fit! or This couch is a perfect fit, right between the piano and the door!. But I remain adamant that the sentence as it is written, in which the ubiquity of block device interfaces is identified as a perfect fit for interacting with mass data storage including Ceph, cannot be what was meant. That's just not how the semantics of the word "fit" work. This sentence sounds a bit to me like a Ceph salesman slapping the side-panel of a brand-new, shiny, red Ceph fresh from the factory and on the showroom floor and saying "the ubiquity of block device interfaces is a perfect fit for interacting with mass data storage including this!". I think that this kind of expression should cause all native English speakers to pause and then to ask "Is the ubiquity a perfect fit?" and then to ask "Is the ubiquity a perfect fit?" and then to ask "Is the ubiquity a perfect fit?".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You aren't wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anthonyeleven I'll leave this PR open for another week while I gather my thoughts on the matter and make sure that I'm not putting any of the wrong technical feet forward, and then I'll alter the paragraph currently under discussion so that it reads as I have suggested (or reads in maybe a yet better way that documentation-writing collaborators or a future better-informed me will write).

@zdover23 zdover23 force-pushed the wip-doc-2022-11-24-rbd-front-matter branch from 745455e to 098fc88 Compare December 1, 2022 05:18
@zdover23 zdover23 force-pushed the wip-doc-2022-11-24-rbd-front-matter branch from 098fc88 to 28f0e63 Compare December 1, 2022 15:32
Swap the order of the first two paragraphs; make minor improvements.

Signed-off-by: Zac Dover <zac.dover@gmail.com>
@zdover23 zdover23 force-pushed the wip-doc-2022-11-24-rbd-front-matter branch from 28f0e63 to a1af6b3 Compare December 5, 2022 17:44
.. index:: Ceph Block Device; introduction

A block is a sequence of bytes (often 512).
**rbd** is a utility that can manipulate rados block device (RBD) images. It is
Copy link
Contributor

@idryomov idryomov Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • rbd utility (CLI tool) is not used by Linux kernel or QEMU/KVM. Users may use it to create and manipulate RBD images but it's not the only way to perform these tasks.
  • RADOS should be capitalized.

A block is a sequence of bytes (often 512).
**rbd** is a utility that can manipulate rados block device (RBD) images. It is
used by the Linux rbd driver and the rbd storage driver for QEMU/KVM. RBD
images are simple block devices that are striped over objects and stored in a
Copy link
Contributor

@idryomov idryomov Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • RBD image are not block devices -- it's just possible to "conjure" a block device backed by an RBD image.
  • Striping is already mentioned below. Mentioning it twice in just a few sentences seems overkill to me.

Copy link
Contributor

@idryomov idryomov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a docs guy but I'm really missing the point of this last push. The previous version that I left a "by means of a Linux kernel module" suggestion on was perfectly fine (except for that nit), at least to me. This version introduces factual errors.

@zdover23
Copy link
Contributor Author

zdover23 commented Dec 6, 2022

I'm not a docs guy but I'm really missing the point of this last push. The previous version that I left a "by means of a Linux kernel module" suggestion on was perfectly fine (except for that nit), at least to me. This version introduces factual errors.

@idryomov This wasn't meant to be a definitive and final addition to the docs. This most recent push was just an instance of me, taking notes, from here: https://docs.ceph.com/en/quincy/man/8/rbd/

I'll take this opportunity to ask you if that rbd-related sentence is indeed an inaccurate description of the rbd command-line tool. If it is, then I'll have to make a new PR to fix it.

I would like to reassure you that this PR will not be merged until you are satisfied with it.

@idryomov
Copy link
Contributor

idryomov commented Dec 6, 2022

@idryomov This wasn't meant to be a definitive and final addition to the docs. This most recent push was just an instance of me, taking notes, from here: https://docs.ceph.com/en/quincy/man/8/rbd/

That needs to be better communicated. Perhaps make this a draft PR for now?

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request

I'll take this opportunity to ask you if that rbd-related sentence is indeed an inaccurate description of the rbd command-line tool. If it is, then I'll have to make a new PR to fix it.

The original sentence in the man page is as follows:

rbd is a utility for manipulating rados block device (RBD) images, used by
the Linux rbd driver and the rbd storage driver for QEMU/KVM. 

Here ", used by the ..." can be interpreted to refer to RBD images, not the utility -- which would be more or less correct. But in your notes this sentence is split into two:

rbd is a utility that can manipulate rados block device (RBD) images. It is
used by the Linux rbd driver and the rbd storage driver for QEMU/KVM.

Here "it is used by the ..." clearly refers to the utility itself -- which is wrong.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Jul 18, 2023
@github-actions
Copy link

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

@github-actions github-actions bot closed this Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants