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

Eric Blake eblake at redhat.com
Tue Oct 26 19:47:12 UTC 2010


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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org




More information about the libvir-list mailing list