2:47 p.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org