On Fri, Jun 16, 2017 at 04:41:20PM +0100, Stephen Finucane wrote: > On Fri, 2017-06-16 at 16:51 +0200, Kashyap Chamarthy wrote:
[...] > As requested, a couple of rST pointers below that will help you if/when you > switch to Sphinx. I've only focused on the design aspect, not the content. Thanks for the Sphinx / rST review. Eric Blake is reviewing the content, and probably others will chime in, too. [...] > > +Copyright (C) 2017 Red Hat Inc. > > + > > +This work is licensed under the terms of the GNU GPL, version 2 or > > +later. See the COPYING file in the top-level directory. > > + > > +--- > > + > > This information doesn't need to be output in the web version, IMO. If write > it > like a comment, it will only be visible in the source. See what we do in OVS > docs [1] for an example. > > [1] > https://raw.githubusercontent.com/openvswitch/ovs/master/Documentation/index.rst Yep, that's useful. Fixed in v3. [...] > > +NB: The file ``qapi/block-core.json`` in the QEMU source tree has the > > +canonical QEMU API (QAPI) schema documentation for the QMP primitives > > +discussed here. > > + > > You might consider using admonitions here and elsewhere. This would make sense > as a 'note' or 'important' directive: > > .. note:: > > The file ``qapi/block-core.json`` ... Yes, done. > > + > > +.. contents:: > > This can probably go if/when Sphinx is integrated - Sphinx includes a ToC in > the sidebar by default. Perhaps include a TODO to remove this? > > .. TODO(kashyap): Remove this when Sphinx is integrated Useful, done. [...] > > +(1) Directional: 'base' and 'top'. Given the simple disk image chain > > + above, image [A] can be referred to as 'base', and image [B] as > > + 'top'. (This terminology can be seen in in QAPI schema file, > > + block-core.json.) > > This looks really like a definition list, which is rST are written like so: > > term > > Detailed description of the term here... > > So this would become: > > Directional > > 'base' and 'top'. Given... Yeah, I tried it, but it's just creating needless blank lines for me in the HTML rendering. So I stuck with using numbers. [...] > > +NB: The base disk image can be raw format; however, all the overlay > > +files must be of QCOW2 format. > > .. important:: Yes, noted. [...] > > +Brief overview of live block QMP primitives > > +------------------------------------------- [...] > Definition list? Not quite. It's more a quick overview. > > + > > + > > +.. _`Interacting with a QEMU instance`: > > If you're not linking to this, you don't need to include this. The 'contents' > directive will automatically insert an anchor for each heading. Yeah, I actually did link to to further below, hence I retained it. [...] > > +The ``-blockdev`` command-line option, used above, is available from > > +QEMU 2.9 onwards. In the above invocation, notice the 'node-name' > > ``node-name``? Yes. > > > +parameter that is used to refer to the disk image a.qcow2 ('node-A') -- > > ``a.qcow2``? Perhaps. [...] > > +NB: In the event we have to repeat a certain QMP command, we will: for > > +the first occurrence of it, show the the ``qmp-shell`` invocation, > > +*and* the corresponding raw JSON QMP syntax; but for subsequent > > +invocations, present just the ``qmp-shell`` syntax, and omit the > > +equivalent JSON output. > > .. important:: Yeah, it's more of a ".. note::", will fix. [...] > > +Here, "node-A" is the name QEMU internally uses to refer to the base > > +image [A] -- it is the backing file, based on which the overlay image, > > +[B], is created. > > I guess you should probably use ``[A]`` here to preserve formatting Hmm, noted. [...] > > + > > +A note on points-in-time vs file names > > +-------------------------------------- > > + > > +In our disk disk image chain: > > + > > +:: > > repeated word and no need for ':\n\n::' - you can just use '::'. > > In our disk image chain:: > > ditto for the rest of the file Ah, indeed. I recall using it in the past, but forgot. Avoids needless blank lines. Will fix throughout. [...] > > + > > + > > +Live block streaming --- ``block-stream`` > > +----------------------------------------- > > + > > +The ``block-stream`` command allows you to do live copy data from backing > > +files into overlay images. > > + > > +Given our original example disk image chain from earlier: > > + > > +:: > > + > > + [A] <-- [B] <-- [C] <-- [D] > > + > > +The disk image chain can be shortened in one of the following different > > +ways (not an exhaustive list). > > + > > Maybe you should include an anchor here, so you can link to it below. Yes, makes sense. I know by "link to it below", you mean link to it in the next where I refer to these. [...] > > +[The invocation for rest of the cases, discussed in the previous > > +section, is omitted for brevity.] > > This looks like a: > > .. note:: Yes; will fix. [...] -- /kashyap
