[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] docs: revamp api_extension example, using vcpu patch series



On 10/25/2010 06:29 PM, Stefan Berger wrote:
On 10/20/2010 12:19 PM, Eric Blake wrote:
Complete patch is compressed and attached, because it is so large. But
the bulk of it consisted mainly of running 'git format-patch -15
b0137887' to pick up the top of my vcpu series, then replacing the
contents of docs/api_extension/ with the new patches.

The meat of the patch is the overview html file, copied here if you
don't want to unzip the attachment:

Some suggestions below.

Thanks.  Unless documented below, I've taken your suggestions as-is.

<p>
This document walks you through the process of implementing a new
- API in libvirt. It uses as an example the addition of the node device
- create and destroy APIs.
+ API in libvirt. It uses as an example the addition of

... the addition of an API for separating maximum.. ?

+ separating maximum from current vcpu usage of a domain.
+ Remember that new API includes any new public functions, but it
that a new API
+ also includes addition of flags or extension of XML used through

may include addition of flags or extensions of XML used by

+ existing functions. The example in this document adds both
+ types of API, and not all extensions require quite as many patches.

I'd make a new sentence after the comma.

Maybe: Also, not all libvirt extensions require quite as many patches.

Here's how I reworded it:

      This document walks you through the process of implementing a new
      API in libvirt.  It uses as an example the addition of an API for
      separating maximum from current vcpu usage of a domain.
      Remember that new API consists of any new public functions, as
      well as the addition of flags or extensions of XML used by
      existing functions.  The example in this document adds both new
      functions and an XML extension.  Not all libvirt API additions
      require quite as many patches.


+ The example code used in this document was rebased several
+ times between the original list posting and the final committed
+ version.
I'd say that the implementer has to follow the tree and adapt the
patches as the tree changes.

I've replaced that sentence with:

      Also, you should follow the upstream tree, and rebase your
      series to adapt your patches to work with any other changes
      that were accepted upstream during your development.

+ <p>
+ Implementing the remote protocol is essentially a
+ straightforward exercise which is probably most easily
+ understood by referring to the existing code and the example
+ patch. It involves several related changes, followed by
+ regenerating derived files.</p>

Not sure whether the reader will know what 'derived files' are.

The derived files are mentioned later on; this is the <h2> overview of several <h3> steps. Here's how I re-worded that paragraph:

      Implementing the remote protocol is essentially a
      straightforward exercise which is probably most easily
      understood by referring to the existing code and the example
      patch.  It involves several related changes, including the
      regeneration of derived files, with further details below.

then later on, I listed the generated files:

    <p><code>
	daemon/remote_dispatch_args.h
	daemon/remote_dispatch_prototypes.h
	daemon/remote_dispatch_table.h
	src/remote/remote_protocol.c
	src/remote/remote_protocol.h
    </code></p>

+ <h2><a name="internaluseapi">Use the new API internally</a></h2>
+
+ <p>
+ Many times, a new API is merely an extension of existing
+ concepts that accomplished a subset of the same actions. When
This sound really abstract...

+ this is the case, it makes sense to share a common
+ implementation by making the older API become a trivial wrapper
+ around the new API, rather than duplicating the common code.
+ This step should not introduce any semantic differences for the
+ old API. This step is optional if the new API has no relation
+ to existing API.
is not necessary rather than 'optional'.

I hope this is less abstract:

      Sometimes, a new API serves as a superset of existing API, by
      adding more granularity in what can be managed.  When this is
      the case, it makes sense to share a common implementation by
      making the older API become a trivial wrapper around the new
      API, rather than duplicating the common code.  This step should
      not introduce any semantic differences for the old API, and is
      not necessary if the new API has no relation to existing API.

Now, all that's holding me up from pushing are server-side hooks:
Total 20 (delta 6), reused 16 (delta 3)
remote: docs/api_extension/0001-Step-1-of-15-add-to-xml.patch:45: space before tab in indent. remote: + minimum memory allocation for the guest. The units for this value are remote: docs/api_extension/0001-Step-1-of-15-add-to-xml.patch:46: space before tab in indent.
remote: + 	kilobytes (i.e. blocks of 1024 bytes)</dd>
remote: docs/api_extension/0001-Step-1-of-15-add-to-xml.patch:143: trailing whitespace.
remote: +--
...
remote: error: hook declined to update refs/heads/master

--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]