[libvirt] [PATCH 0/8] Web page updates/changes
I was [t]asked to make updates to the api webpage including an example of a code path from application into the driver. As I started on the task I found that the api webpage first needed a couple of updates just to fill in what seems to have been missing information. As I went through the various pages I realized that the 'better' location for the example code path seemed to be the internals page and thus I added it there. My simple example (hah!) was the virConnectOpen code path - I figured for a intro- duction to the internals web page - it'd be a good place to start. While I was doing this I used the hellolibvirt example a bit and based on information found on the 'api' page I added some more calls into the example to print network and disk information as well as more domain data. Finally while going through the pages I found a couple of places where drv pages weren't referenced although they were available, so I updated those (phyp and parallels). Hopefully I've figured things out correctly, but I'm sure if I have something a bit wrong someone will correct me. Making the figure was quite frustrating as xfig is a bit less forgiving than say office writer. John Ferlan (8): hellolibvirt: Update hellolibvirt example api: Reword objects exposed section api: Reword and clean lists for object description api: Complete list of function and naming conventions api: Add text and references for drivers section api: Add text and references for daemon Add references for phyp and parallels internals: Update to include RPC and Lock links and add new data docs/api.html.in | 261 +++++++++++++++++++++++++---------- docs/drivers.html.in | 1 + docs/index.html.in | 6 + docs/internals.html.in | 102 +++++++++++++- docs/libvirt-virConnect-example.fig | 58 ++++++++ docs/libvirt-virConnect-example.png | Bin 0 -> 10464 bytes docs/sitemap.html.in | 8 ++ examples/hellolibvirt/hellolibvirt.c | 201 +++++++++++++++++++++++---- 8 files changed, 530 insertions(+), 107 deletions(-) create mode 100644 docs/libvirt-virConnect-example.fig create mode 100644 docs/libvirt-virConnect-example.png -- 1.7.11.7
Add a list of active domains, list of active/inactive networks, and list of active/inactive storage pools --- examples/hellolibvirt/hellolibvirt.c | 201 ++++++++++++++++++++++++++++++----- 1 file changed, 173 insertions(+), 28 deletions(-) diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c index 234637e..f191782 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -85,65 +85,200 @@ out: return ret; } - +typedef int (*virFunction)(virConnectPtr conn, + char **nameList, + int maxnames); static int -showDomains(virConnectPtr conn) +listObject(virConnectPtr conn, int maxnames, const char *objNameStr, + virFunction fcn) { - int ret = 0, i, numNames, numInactiveDomains, numActiveDomains; + int ret = 0, i, numNames; char **nameList = NULL; - numActiveDomains = virConnectNumOfDomains(conn); - if (-1 == numActiveDomains) { + nameList = malloc(sizeof(*nameList) * maxnames); + + if (NULL == nameList) { ret = 1; - printf("Failed to get number of active domains\n"); + printf("Could not allocate memory for list of %s\n", objNameStr); + goto out; + } + + numNames = (*fcn)(conn, nameList, maxnames); + + if (-1 == numNames) { + ret = 1; + printf("Could not get list of %s from hypervisor\n", objNameStr); showError(conn); goto out; } - numInactiveDomains = virConnectNumOfDefinedDomains(conn); - if (-1 == numInactiveDomains) { + printf(" %s: \n", objNameStr); + for (i = 0; i < numNames; i++) { + printf("\t%s\n", *(nameList + i)); + /* The API documentation doesn't say so, but the names + * returned by are strdup'd and must be freed here. + */ + free(*(nameList + i)); + } + +out: + free(nameList); + return ret; +} + +static int +showNetworks(virConnectPtr conn) +{ + int ret = 0, numInactiveNetworks, numActiveNetworks; + + numActiveNetworks = virConnectNumOfNetworks(conn); + if (-1 == numActiveNetworks) { ret = 1; - printf("Failed to get number of inactive domains\n"); + printf("Failed to get number of active networks\n"); showError(conn); goto out; } - printf("There are %d active and %d inactive domains\n", - numActiveDomains, numInactiveDomains); + numInactiveNetworks = virConnectNumOfDefinedNetworks(conn); + if (-1 == numInactiveNetworks) { + ret = 1; + printf("Failed to get number of inactive networks\n"); + showError(conn); + goto out; + } - nameList = malloc(sizeof(*nameList) * numInactiveDomains); + printf("There are %d active and %d inactive networks\n", + numActiveNetworks, numInactiveNetworks); - if (NULL == nameList) { + if (numActiveNetworks) { + ret = listObject(conn, numActiveNetworks, "active networks", + virConnectListNetworks); + if (ret != 0) + goto out; + } + + if (numInactiveNetworks) { + ret = listObject(conn, numInactiveNetworks, "defined networks", + virConnectListDefinedNetworks); + if (ret != 0) + goto out; + } + +out: + return ret; +} + +static int +showStoragePools(virConnectPtr conn) +{ + int ret = 0, numInactiveStoragePools, numActiveStoragePools; + + numActiveStoragePools = virConnectNumOfStoragePools(conn); + if (-1 == numActiveStoragePools) { + ret = 1; + printf("Failed to get number of active storage pools\n"); + showError(conn); + goto out; + } + + numInactiveStoragePools = virConnectNumOfDefinedStoragePools(conn); + if (-1 == numInactiveStoragePools) { ret = 1; - printf("Could not allocate memory for list of inactive domains\n"); + printf("Failed to get number of inactive storage pools\n"); + showError(conn); goto out; } - numNames = virConnectListDefinedDomains(conn, - nameList, - numInactiveDomains); + printf("There are %d active and %d inactive storage pools\n", + numActiveStoragePools, numInactiveStoragePools); - if (-1 == numNames) { + if (numActiveStoragePools) { + ret = listObject(conn, numActiveStoragePools, "active storage pools", + virConnectListStoragePools); + if (ret != 0) + goto out; + } + + if (numInactiveStoragePools) { + ret = listObject(conn, numInactiveStoragePools, + "inactive storage pools", + virConnectListDefinedStoragePools); + if (ret != 0) + goto out; + } + +out: + return ret; +} + +static int +showDomains(virConnectPtr conn) +{ + int ret = 0, numInactiveDomains, numActiveDomains; + + numActiveDomains = virConnectNumOfDomains(conn); + if (-1 == numActiveDomains) { ret = 1; - printf("Could not get list of defined domains from hypervisor\n"); + printf("Failed to get number of active domains\n"); showError(conn); goto out; } - if (numNames > 0) { - printf("Inactive domains:\n"); + numInactiveDomains = virConnectNumOfDefinedDomains(conn); + if (-1 == numInactiveDomains) { + ret = 1; + printf("Failed to get number of inactive domains\n"); + showError(conn); + goto out; } - for (i = 0 ; i < numNames ; i++) { - printf(" %s\n", *(nameList + i)); - /* The API documentation doesn't say so, but the names - * returned by virConnectListDefinedDomains are strdup'd and - * must be freed here. */ - free(*(nameList + i)); + printf("There are %d active and %d inactive domains\n", + numActiveDomains, numInactiveDomains); + + if (numActiveDomains) { + int nIDs, i; + int *IDs; + + /* Slightly different than the others... */ + + IDs = malloc(sizeof(*IDs) * numActiveDomains); + + if (NULL == IDs) { + ret = 1; + printf("Could not allocate memory for list of active domains\n"); + goto out; + } + + nIDs = virConnectListDomains(conn, IDs, numActiveDomains); + if (-1 == nIDs) { + ret = 1; + printf("Could not get list of active domains from hypervisor\n"); + showError(conn); + goto out; + } + + printf(" active domains: \n"); + for (i = 0; i < nIDs; i++) { + virDomainPtr dom = virDomainLookupByID(conn, *(IDs + i)); + if (dom) { + printf("\tID=%d Name=%s\n", *(IDs + i), virDomainGetName(dom)); + virDomainFree(dom); + } else { + printf("\tID=%d\n", *(IDs + i)); + } + } + + free(IDs); + } + + if (numInactiveDomains) { + ret = listObject(conn, numInactiveDomains, "defined domains", + virConnectListDefinedDomains); + if (ret != 0) + goto out; } out: - free(nameList); return ret; } @@ -191,6 +326,16 @@ main(int argc, char *argv[]) goto disconnect; } + if (0 != showNetworks(conn)) { + ret = 1; + goto disconnect; + } + + if (0 != showStoragePools(conn)) { + ret = 1; + goto disconnect; + } + disconnect: if (0 != virConnectClose(conn)) { printf("Failed to disconnect from hypervisor\n"); -- 1.7.11.7
--- docs/api.html.in | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/docs/api.html.in b/docs/api.html.in index f596aa9..0f01aed 100644 --- a/docs/api.html.in +++ b/docs/api.html.in @@ -8,26 +8,28 @@ <ul id="toc"></ul> - <h2><a name="Objects">Objects exposed</a></h2> - <p> As defined in the <a href="goals.html">goals section</a>, libvirt - API need to expose all the resources needed to manage the virtualization - support of recent operating systems. The first object manipulated though - the API is <code>virConnectPtr</code> which represent a connection to - an hypervisor. Any application using libvirt is likely to start using the + <h2><a name="Objects">Objects Exposed</a></h2> + <p> As defined in the <a href="goals.html">goals section</a>, the libvirt + API is designed to expose all the resources needed to manage the + virtualization support of recent operating systems. The first object + manipulated through the API is the <code>virConnectPtr</code>, which + represents the connection to a hypervisor. Any application using libvirt + is likely to start using the API by calling one of <a href="html/libvirt-libvirt.html#virConnectOpen" >the virConnectOpen functions</a>. You will note that those functions take - a name argument which is actually an URI to select the right hypervisor to - open, this is needed to allow remote connections and also select between - different possible hypervisors (for example on a Linux system it may be - possible to use both KVM and LinuxContainers on the same node). A NULL - name will default to a preselected hypervisor but it's probably not a + a name argument which is actually a <a href="uri.html">connection URI</a> + to select the right hypervisor to open. + A URI is needed to allow remote connections and also select between + different possible hypervisors. For example, on a Linux system it may be + possible to use both KVM and LinuxContainers on the same node. A NULL + name will default to a preselected hypervisor, but it's probably not a wise thing to do in most cases. See the <a href="uri.html">connection URI</a> page for a full descriptions of the values allowed.</p> - <p> Once the application obtained a <code class='docref'>virConnectPtr</code> - connection to the - hypervisor it can then use it to manage domains and related resources - available for virtualization like storage and networking. All those are - exposed as first class objects, and connected to the hypervisor connection + <p> Once the application obtains a <code class='docref'>virConnectPtr</code> + connection to the hypervisor it can then use it to manage the hypervisor's + available domains and related virtualization + resources, such as storage and networking. All those are + exposed as first class objects and connected to the hypervisor connection (and the node or cluster where it is available).</p> <p class="image"> <img alt="first class objects exposed by the API" -- 1.7.11.7
--- docs/api.html.in | 67 +++++++++++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/docs/api.html.in b/docs/api.html.in index 0f01aed..09fe4f0 100644 --- a/docs/api.html.in +++ b/docs/api.html.in @@ -37,43 +37,56 @@ </p> <p> The figure above shows the five main objects exported by the API:</p> <ul> - <li>virConnectPtr: represent a connection to an hypervisor.</li> - <li>virDomainPtr: represent one domain either active or defined (i.e. - existing as permanent config file and storage but not currently running - on that node). The function <code class='docref'>virConnectListDomains</code> - allows to list all the IDs for the domains active on this hypervisor.</li> - <li>virNetworkPtr: represent one network either active or defined (i.e. - existing as permanent config file and storage but not currently activated. - The function <code class='docref'>virConnectListNetworks</code> - allows to list all the virtualization networks activated on this node.</li> - <li>virStorageVolPtr: represent one storage volume, usually this is used + <li><code class='docref'>virConnectPtr</code> + <p>Represents the connection to a hypervisor. Use one of the + <a href="html/libvirt-libvirt.html#virConnectOpen">virConnectOpen</a> + functions to obtain connection to the hypervisor which is then used + as a parameter to other connection API's.</p></li> + <li><code class='docref'>virDomainPtr</code> + <p>Represents one domain either active or defined (i.e. existing as + permanent config file and storage but not currently running on that + node). The function <code class='docref'>virConnectListAllDomains</code> + lists all the domains for the hypervisor.</p></li> + <li><code class='docref'>virNetworkPtr</code> + <p>Represents one network either active or defined (i.e. existing + as permanent config file and storage but not currently activated). + The function <code class='docref'>virConnectListAllNetworks</code> + lists all the virtualization networks for the hypervisor.</p></li> + <li><code class='docref'>virStorageVolPtr</code> + <p>Represents one storage volume generally used as a block device available to one of the domains. The function - <code class="docref">virStorageVolLookupByPath</code> allows to find - the object based on its path on the node.</li> - <li>virStoragePoolPtr: represent a storage pool, i.e. a logical area - which can be used to allocate and store storage volumes. The function - <code class="docref">virStoragePoolLookupByVolume</code> allows to find - the storage pool containing a given storage volume.</li> + <code class="docref">virStorageVolLookupByPath</code> finds + the storage volume object based on its path on the node.</p></li> + <li><code class='docref'>virStoragePoolPtr</code> + <p>Represents a storage pool, which is a logical area + used to allocate and store storage volumes. The function + <code class='docref'>virConnectListAllStoragePools</code> lists + all of the virtualization storage pools on the hypervisor. The function + <code class="docref">virStoragePoolLookupByVolume</code> finds + the storage pool containing a given storage volume.</p></li> </ul> - <p> Most object manipulated by the library can also be represented using + <p> Most objects manipulated by the library can also be represented using XML descriptions. This is used primarily to create those object, but is also helpful to modify or save their description back.</p> - <p> Domains, network and storage pools can be either <code>active</code> + <p> Domains, networks, and storage pools can be either <code>active</code> i.e. either running or available for immediate use, or <code>defined</code> in which case they are inactive but there is a permanent definition available in the system for them. Based on this - thay can be activated dynamically in order to be used.</p> - <p> Most kind of object can also be named in various ways:</p> + they can be activated dynamically in order to be used.</p> + <p> Most objects can also be named in various ways:</p> <ul> - <li>by their <code>name</code>, an user friendly identifier but - whose unicity cannot be guaranteed between two nodes.</li> - <li>by their <code>ID</code>, which is a runtime unique identifier - provided by the hypervisor for one given activation of the object, - but it becomes invalid once the resource is deactivated.</li > - <li>by their <code>UUID</code>, a 16 bytes unique identifier + <li><code>name</code> + <p>A user friendly identifier but whose uniqueness + cannot be guaranteed between two nodes.</p></li> + <li><code>ID</code> + <p>A runtime unique identifier + provided by the hypervisor for one given activation of the object; + however, it becomes invalid once the resource is deactivated.</p></li > + <li><code>UUID</code> + <p> A 16 byte unique identifier as defined in <a href="http://www.ietf.org/rfc/rfc4122.txt">RFC 4122</a>, which is guaranteed to be unique for long term usage and across a - set of nodes.</li> + set of nodes.</p></li> </ul> <h2><a name="Functions">Functions and naming -- 1.7.11.7
--- docs/api.html.in | 92 +++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 64 insertions(+), 28 deletions(-) diff --git a/docs/api.html.in b/docs/api.html.in index 09fe4f0..9855b39 100644 --- a/docs/api.html.in +++ b/docs/api.html.in @@ -89,38 +89,74 @@ set of nodes.</p></li> </ul> - <h2><a name="Functions">Functions and naming - conventions</a></h2> + <h2><a name="Functions">Functions and Naming Conventions</a></h2> <p> The naming of the functions present in the library is usually - made of a prefix describing the object associated to the function + composed by a prefix describing the object associated to the function and a verb describing the action on that object.</p> - <p> For each first class object you will find apis + <p> For each first class object you will find APIs for the following actions:</p> <ul> - <li><b>Lookup</b>:...LookupByName,</li> - <li><b>Enumeration</b>:virConnectList... and virConnectNumOf...: - those are used to enumerate a set of object available to an given - hypervisor connection like: - <code class='docref'>virConnectListDomains</code>, - <code class='docref'>virConnectNumOfDomains</code>, - <code class='docref'>virConnectListNetworks</code>, - <code class='docref'>virConnectListStoragePools</code>, etc.</li> - <li><b>Description</b>: ...GetInfo: those are generic accessor providing - a set of informations about an object, they are - <code class='docref'>virNodeGetInfo</code>, - <code class='docref'>virDomainGetInfo</code>, - <code class='docref'>virStoragePoolGetInfo</code>, - <code class='docref'>virStorageVolGetInfo</code>.</li> - <li><b>Accessors</b>: ...Get... and ...Set...: those are more specific - accessors to query or modify the given object, like - <code class='docref'>virConnectGetType</code>, - <code class='docref'>virDomainGetMaxMemory</code>, - <code class='docref'>virDomainSetMemory</code>, - <code class='docref'>virDomainGetVcpus</code>, - <code class='docref'>virStoragePoolSetAutostart</code>, - <code class='docref'>virNetworkGetBridgeName</code>, etc.</li> - <li><b>Creation</b>: </li> - <li><b>Destruction</b>: ... </li> + <li><b>Lookup</b> [...LookupBy...] + <p>Used to perform lookups on objects by some type of identifier, + such as:</p> + <ul> + <li><code class='docref'>virDomainLookupByID</code></li> + <li><code class='docref'>virDomainLookupByName</code></li> + <li><code class='docref'>virDomainLookupByUUID</code></li> + <li><code class='docref'>virDomainLookupByUUIDString</code></li> + </ul> + </li> + <li><b>Enumeration</b> [virConnectList..., virConnectNumOf...] + <p>Used to enumerate a set of object available to an given + hypervisor connection such as:</p> + <ul> + <li><code class='docref'>virConnectListDomains</code></li> + <li><code class='docref'>virConnectNumOfDomains</code></li> + <li><code class='docref'>virConnectListNetworks</code></li> + <li><code class='docref'>virConnectListStoragePools</code></li> + </ul> + </li> + <li><b>Description</b> [...GetInfo] + <p>Generic accessor providing a set of generic information about an + object, such as: </p> + <ul> + <li><code class='docref'>virNodeGetInfo</code></li> + <li><code class='docref'>virDomainGetInfo</code></li> + <li><code class='docref'>virStoragePoolGetInfo</code></li> + <li><code class='docref'>virStorageVolGetInfo</code></li> + </ul> + </li> + <li><b>Accessors</b> [...Get..., ...Set...] + <p>Specific accessors used to query or modify data for the given object, + such as: </p> + <ul> + <li><code class='docref'>virConnectGetType</code></li> + <li><code class='docref'>virDomainGetMaxMemory</code></li> + <li><code class='docref'>virDomainSetMemory</code></li> + <li><code class='docref'>virDomainGetVcpus</code></li> + <li><code class='docref'>virStoragePoolSetAutostart</code></li> + <li><code class='docref'>virNetworkGetBridgeName</code></li> + </ul> + </li> + <li><b>Creation</b> [...Create, ...CreateXML] + <p>Used to create and start objects. The ...CreateXML APIs will create + the object based on an XML description, while the ...Create APIs will + create the object based on existing object pointer, such as: </p> + <ul> + <li><code class='docref'>virDomainCreate</code></li> + <li><code class='docref'>virDomainCreateXML</code></li> + <li><code class='docref'>virNetworkCreate</code></li> + <li><code class='docref'>virNetworkCreateXML</code></li> + </ul> + </li> + <li><b>Destruction</b> [...Destroy] + <p>Used to shutdown or deactivate and destroy objects, such as: </p> + <ul> + <li><code class='docref'>virDomainDestroy</code></li> + <li><code class='docref'>virNetworkDestroy</code></li> + <li><code class='docref'>virStoragePoolDestroy</code></li> + </ul> + </li> </ul> <p> For more in-depth details of the storage related APIs see <a href="storage.html">the storage management page</a>. -- 1.7.11.7
--- docs/api.html.in | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/docs/api.html.in b/docs/api.html.in index 9855b39..51d6527 100644 --- a/docs/api.html.in +++ b/docs/api.html.in @@ -161,14 +161,31 @@ <p> For more in-depth details of the storage related APIs see <a href="storage.html">the storage management page</a>. </p> - <h2><a name="Driver">The libvirt drivers</a></h2> - <p></p> + <h2><a name="Drivers">The libvirt Drivers</a></h2> + <p>Drivers are the basic building block for libvirt functionality + to support the capability to handle specific hypervisor driver calls. + Drivers are discovered and registered during connection processing as + part of the <code class='docref'>virInitialize</code> API. Each driver + has a registration API which loads up the driver specific function + references for the libvirt APIs to call. The following is a simplistic + view of the hypervisor driver mechanism. Consider the stacked list of + drivers as a series of modules that can be plugged into the architecture + depending on how libvirt is configured to be built.</p> <p class="image"> <img alt="The libvirt driver architecture" src="libvirt-driver-arch.png"/> </p> - <h2><a name="Remote">Daemon and remote access</a></h2> - <p></p> + <p>The driver architecture is also used to support other virtualization + components such as storage, storage pools, host device, networking, + network interfaces, and network filters.</p> + <p>See the <a href="drivers.html">libvirt drivers</a> page for more + information on hypervisor and storage specific drivers.</p> + <p>Not all drivers support every virtualization function possible. + The <a href="hvsupport.html">libvirt API support matrix</a> lists + the various functions and support found in each driver by the version + support was added into libvirt. + </p> + <h2><a name="Remote">Daemon and Remote Access</a></h2> <p class="image"> <img alt="The libvirt daemon and remote architecture" src="libvirt-daemon-arch.png"/> -- 1.7.11.7
--- docs/api.html.in | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/docs/api.html.in b/docs/api.html.in index 51d6527..12dfaee 100644 --- a/docs/api.html.in +++ b/docs/api.html.in @@ -186,9 +186,52 @@ support was added into libvirt. </p> <h2><a name="Remote">Daemon and Remote Access</a></h2> + <p>Access to libvirt drivers is primarily handled by the libvirtd + daemon through the <a href="remote.html">remote</a> driver via an + <a href="internals/rpc.html">RPC</a>. Some hypervisors do support + client-side connections and responses, such as Test, OpenVZ, VMware, + Power VM (phyp), VirtualBox (vbox), ESX, Hyper-V, Xen, and Parallels. + The libvirtd daemon service is started on the host at system boot + time and can also be restarted at any time by a properly privileged + user, such as root. The libvirtd daemon uses the same libvirt API + <code class='docref'>virInitialize</code> sequence as applications + for client-side driver registrations, but then extends the registered + driver list to encompass all known drivers supported for all driver + types supported on the host. </p> + <p>The libvirt client <a href="apps.html">applications</a> use a + <a href="uri.html">URI</a> to obtain the <code>virConnectPtr</code>. + The <code>virConnectPtr</code> keeps track of the driver connection + plus a variety of other connections (network, interface, storage, etc.). + The <code>virConnectPtr</code> is then used as a parameter to other + virtualization <a href="#Functions">functions</a>. Depending upon the + driver being used, calls will be routed through the remote driver to + the libvirtd daemon. The daemon will reference the connection specific + driver in order to retreive the requested information and then pass + back status and/or data through the connection back to the application. + The application can then decide what to do with that data, such as + display, write log data, etc. <a href="migration.html">Migration</a> + is an example of many facets of the architecture in use.</p> + <p class="image"> <img alt="The libvirt daemon and remote architecture" src="libvirt-daemon-arch.png"/> </p> + <p> + The key takeaway from the above diagram is that there is a remote driver + which handles transactions for a majority of the drivers. The libvirtd + daemon running on the host will receive transaction requests from the + remote driver and will then query the hypervisor driver as specified in + the <code>virConnectPtr</code> in order to fetch the data. The data will + then be returned through the remote driver to the client application + for processing. + </p> + <p>If you are interested in contributing to libvirt, read the + <a href="http://wiki.libvirt.org/page/FAQ">FAQ</a> and + <a href="hacking.html">hacking</a> guidelines to gain an understanding + of basic rules and guidelines. In order to add new API functionality + follow the instructions regarding + <a href="api_extension.html">implementing a new API in libvirt</a>. + </p> + </body> </html> -- 1.7.11.7
--- docs/drivers.html.in | 1 + docs/index.html.in | 6 ++++++ docs/sitemap.html.in | 8 ++++++++ 3 files changed, 15 insertions(+) diff --git a/docs/drivers.html.in b/docs/drivers.html.in index 307d286..3d79ed5 100644 --- a/docs/drivers.html.in +++ b/docs/drivers.html.in @@ -30,6 +30,7 @@ <li><strong><a href="drvxen.html">Xen</a></strong></li> <li><strong><a href="drvhyperv.html">Microsoft Hyper-V</a></strong></li> <li><strong><a href="drvphyp.html">IBM PowerVM (phyp)</a></strong></li> + <li><strong><a href="drvparallels.html">Parallels</a></strong></li> </ul> <h2><a name="storage">Storage drivers</a></h2> diff --git a/docs/index.html.in b/docs/index.html.in index c84eb1f..038986d 100644 --- a/docs/index.html.in +++ b/docs/index.html.in @@ -63,6 +63,12 @@ The <a href="http://libvirt.org/drvhyperv.html">Microsoft Hyper-V</a> hypervisor </li> <li> + The <a href="http://libvirt.org/drvphyp.html">IBM PowerVM</a> hypervisor + </li> + <li> + The <a href="http://libvirt.org/drvparallels.html">Parallels</a> hypervisor + </li> + <li> Virtual networks using bridging, NAT, VEPA and VN-LINK. </li> <li> diff --git a/docs/sitemap.html.in b/docs/sitemap.html.in index 206040c..afabf2d 100644 --- a/docs/sitemap.html.in +++ b/docs/sitemap.html.in @@ -216,6 +216,14 @@ <a href="drvhyperv.html">Microsoft Hyper-V</a> <span>Driver for Microsoft Hyper-V</span> </li> + <li> + <a href="drvphyp.html">IBM PowerVM</a> + <span>Driver for IBM PowerVM</span> + </li> + <li> + <a href="drvparallels.html">Parallels</a> + <span>Driver for Parallels Cloud Server</span> + </li> </ul> </li> <li> -- 1.7.11.7
Added a picture and explanation describing the virConnectOpen processing at a "higher" level, but with some source code references. --- docs/internals.html.in | 102 ++++++++++++++++++++++++++++++++++-- docs/libvirt-virConnect-example.fig | 58 ++++++++++++++++++++ docs/libvirt-virConnect-example.png | Bin 0 -> 10464 bytes 3 files changed, 156 insertions(+), 4 deletions(-) create mode 100644 docs/libvirt-virConnect-example.fig create mode 100644 docs/libvirt-virConnect-example.png diff --git a/docs/internals.html.in b/docs/internals.html.in index 5689998..90143ef 100644 --- a/docs/internals.html.in +++ b/docs/internals.html.in @@ -9,12 +9,106 @@ </p> <ul> - <li>Introduction to basic rules and guidelines for <a href="hacking.html">hacking</a> - on libvirt code</li> + <li>Introduction to basic rules and guidelines for + <a href="hacking.html">hacking</a> on libvirt code</li> <li>Guide to adding <a href="api_extension.html">public APIs</a></li> - <li>Approach for <a href="internals/command.html">spawning commands</a> from - libvirt driver code</li> + <li>Approach for <a href="internals/command.html">spawning commands</a> + from libvirt driver code</li> + <li>The libvirt <a href="internals/rpc.html">RPC infrastructure</a></li> + <li>The <a href="internals/locking.html">Resource Lock Manager</a></li> </ul> + <p>Before adding new code it will be important to get a basic understanding + of the many elements involved with making any call or change to the libvirt + code. The architecture <a href="goals.html">goals</a> must be adhered to + when submitting new code. Understanding the many places that need to be + touched and the interactions between various subsystems within libvirt + will directly correlate to the ability to be successful in getting new + code accepted.</p> + <p>The following diagram depicts code flow from a client application, in + this case the libvirt provided <code>virsh</code> command through the + various layers to elicit a response from some chosen hypervisor. + </p> + + <p class="image"> + <img alt="virConnectOpen calling sequence" + src="libvirt-virConnect-example.png"/> + </p> + <ul> + <li>"virsh -c qemu:///system list --all" + <p>After the virsh code processes the input arguments, it eventually + will make a call to open the connection using a default set of + authentication credentials (virConnectAuthDefault). </p></li> + <li>virConnectOpenAuth() + <p>Each of the virConnectOpen APIs will first call virInitialize() and + then revector through the local "do_open():" call.</p> + <ul> + <li>virInitialize() + <p>Calls the registration API for each of the drivers with + client-side only capabilities and then call the remoteRegister() + API last. This ensures the virDriverTab[] tries local drivers + first before using the remote driver.</p></li> + <li>Loop through virDriverTab[] entries trying to call their + respective "open" entry point (in our case remoteOpen())</li> + <li>After successful return from the virDriverTab[] open() + API, attempt to find and open other drivers (network, interface, + storage, etc.)</li> + </ul> + </li> + <li>remoteOpen() + <p>After a couple of URI checks, a call to doRemoteOpen() is made</p> + <ul> + <li>Determine network transport and host/port to use from URI + <p>The transport will be either tls, unix, ssh, libssh2, ext, + or tcp with the default of tls. Decode the host/port if provided + or default to "localhost".</p></li> + <li>virNetClientRegisterAsyncIO() + <p>Register an I/O callback mechanism to get returned data via + virNetClientIncomingEvent()</p></li> + <li>"call(...REMOTE_PROC_OPEN...)" + <p>Eventually routes into virNetClientProgramCall() which will + call virNetClientSendWithReply() and eventually uses + virNetClientIO()to send the message to libvirtd and + then waits for a response using virNetClientIOEventLoop()</p></li> + <li>virNetClientIncomingEvent() + <p>Receives the returned packet and processes through + virNetClientIOUpdateCallback()</p></li> + </ul> + </li> + <li>libvirtd Daemon + <p></p> + <ul> + <li>Daemon Startup + <p>The daemon initialization processing will declare itself + as a server via a virNetServerNew() call, then use + virDriverLoadModule() to find/load all known drivers, + set up an RPC server program using the <code>remoteProcs[]</code> + table via a virNetServerProgramNew() call. The table is the + corollary to the <code>remote_procedure</code> enum list in + the client. It lists all the functions to be called in + the same order. Once RPC is set up, networking server sockets + are opened, the various driver state initialization routines + are run from the <code>virStateDriverTab[]</code>, the network + links are enabled, and the daemon waits for work.</p></li> + <li>RPC + <p>When a message is received, the <code>remoteProcs[]</code> + table is referenced for the 'REMOTE_PROC_OPEN' call entry. + This results in remoteDispatchOpen() being called via the + virNetServerProgramDispatchCall().</p></li> + <li>remoteDispatchOpen() + <p>The API will read the argument passed picking out the + <code>name</code> of the driver to be opened. The code + will then call virConnectOpen() or virConnectOpenReadOnly() + depending on the argument <code>flags</code>.</p></li> + <li>virConnectOpen() or virConnectOpenReadOnly() + <p>Just like the client except that upon entry the URI + is what was passed from the client and will be found + and opened to process the data.</p> + <p>The returned structure data is returned via the + virNetServer interfaces to the remote driver which then + returns it to the client application.</p></li> + </ul> + </li> + </ul> </body> </html> diff --git a/docs/libvirt-virConnect-example.fig b/docs/libvirt-virConnect-example.fig new file mode 100644 index 0000000..2936f22 --- /dev/null +++ b/docs/libvirt-virConnect-example.fig @@ -0,0 +1,58 @@ +#FIG 3.2 Produced by xfig version 3.2.5b +Landscape +Center +Inches +Letter +100.00 +Single +-2 +1200 2 +2 2 0 2 0 7 50 -1 -1 0.000 0 0 -1 0 0 5 + 450 375 4575 375 4575 1725 450 1725 450 375 +2 2 0 2 0 7 50 -1 -1 0.000 0 0 -1 0 0 5 + 1125 2475 4950 2475 4950 3600 1125 3600 1125 2475 +2 1 0 1 0 7 50 -1 -1 0.000 0 0 -1 1 0 2 + 1 0 1.00 60.00 120.00 + 1725 1725 2175 2475 +2 2 0 1 0 7 50 -1 -1 0.000 0 0 -1 0 0 5 + 3150 5700 6525 5700 6525 6900 3150 6900 3150 5700 +2 2 0 1 0 7 50 -1 -1 0.000 0 0 -1 0 0 5 + 7875 6825 10125 6825 10125 7725 7875 7725 7875 6825 +2 2 0 2 0 7 50 -1 -1 0.000 0 0 -1 0 0 5 + 2550 4725 10350 4725 10350 7800 2550 7800 2550 4725 +2 2 0 1 0 7 50 -1 -1 0.000 0 0 7 0 0 5 + 8850 1950 11550 1950 11550 3360 8850 3360 8850 1950 +2 1 0 1 0 7 50 -1 -1 0.000 0 0 -1 1 0 2 + 1 0 1.00 60.00 120.00 + 3975 3600 5025 4425 +2 1 0 1 0 7 50 -1 -1 0.000 0 0 -1 1 0 2 + 1 0 1.00 60.00 120.00 + 8925 3225 5400 4425 +2 1 0 1 0 7 50 -1 -1 0.000 0 0 -1 1 0 2 + 1 0 1.00 60.00 120.00 + 5625 6900 7875 7425 +2 1 0 1 0 7 50 -1 -1 0.000 0 0 -1 1 0 3 + 1 0 1.00 60.00 120.00 + 11400 3375 11400 7575 10125 7575 +2 2 0 2 0 7 50 -1 -1 0.000 0 0 -1 0 0 5 + 8400 975 12450 975 12450 4125 8400 4125 8400 975 +2 1 0 1 0 7 50 -1 -1 0.000 0 0 -1 1 0 6 + 1 0 1.00 60.00 120.00 + 10125 7125 10725 7125 10725 4425 7725 4425 7725 2700 8850 2700 +4 0 0 50 -1 16 12 0.0000 4 180 2430 1350 2895 virConnectOpenReadOnly(uri)\001 +4 0 0 50 -1 16 12 0.0000 4 180 3240 1350 3090 virConnectOpenAuth(uri, auth, flags)\001 +4 0 0 50 -1 0 12 0.0000 4 165 1350 3300 5850 virConnectOpen:\001 +4 0 0 50 -1 0 12 0.0000 4 165 2070 3300 6045 virConnectOpenReadOnly:\001 +4 0 0 50 -1 0 12 0.0000 4 165 1710 3300 6240 virConnectOpenAuth:\001 +4 0 0 50 -1 0 12 0.0000 4 180 900 3975 6600 do_open():\001 +4 0 0 50 -1 0 14 0.0000 4 135 1260 8025 7125 Rremote driver\001 +4 0 0 50 -1 16 24 0.0000 4 135 630 5025 4650 libvirt\001 +4 0 0 50 -1 0 14 0.0000 4 180 1890 9000 2175 remoteDispatchOpen():\001 +4 0 0 50 -1 0 12 0.0000 4 45 270 9300 2475 ...\001 +4 0 0 50 -1 0 12 0.0000 4 180 1440 9300 2670 virConnectOpen()\001 +4 0 0 50 -1 0 12 0.0000 4 180 2160 9300 2865 virConnectOpenReadOnly()\001 +4 0 0 50 -1 0 12 0.0000 4 45 270 9300 3060 ...\001 +4 0 0 50 -1 0 12 0.0000 4 180 1080 8250 7350 remoteOpen()\001 +4 0 0 50 -1 16 16 0.0000 4 165 3240 600 1050 "virsh -c qemu:///system list --all"\001 +4 0 0 50 -1 16 12 0.0000 4 180 1710 1350 2700 virConnectOpen(uri)\001 +4 0 0 50 -1 16 24 0.0000 4 135 720 9750 825 libvirtd\001 diff --git a/docs/libvirt-virConnect-example.png b/docs/libvirt-virConnect-example.png new file mode 100644 index 0000000000000000000000000000000000000000..ae9c65ba01ef574c59f11d75c7d959b53a02ee2b GIT binary patch literal 10464 zcmeHsc|4Tu+xIn72%*g=B1I_~QkLv{OpM)NERl8W*>@6JL?Oi3Dnl8AnlXgQP*lQb z#*!@}H1;)FdoTBW|K892Jn!>)Kfm{n_kG?!?my;popa539_KN~c^t=i9N+I<V?$jo z4t@>*0Jso(+NJ=oM+5*MOZ%YUn-GIUY4E}Br)M1i0Q(JhKaiM^```>fFi^)T(9G90 zFvQW{1<*A%5Y~0~4G0Wy^2hiE3M)unkn<?sFAD%C4kNTR%tLc$hd%dv*7CD<2CFRZ z_`cqx+Gn0SsG%A!ZRlE3u+KXTyLWhaehmT$QwwO5%iGZv<*b1pd3b@=o&CQrJ5o{6 zPl3v*0{nRZSg=Zi0qRE(oPhAXA{O9)4jl^IIfa1$+NmS}c&dj6025I-5PNoa+9Nef zO8}^qSI+m`aIfoow772PeIh2SyqfT1X;>W#1L~XBHdm=V%NPbU_Gbr%jou#^)Biu^ ztO!BXBNASIXF9wHJi9&h>g}~pB8$XluQFSudhZR9Fc4~3D|6B;%hzM##fhyqGNxY_ z*3ay|e(@?9F+4Ay_omL`+`v<E_@_iK20z>GCg;1~zd$leUVXuJXxrT;fa?0?YsS5( zufyQyn!DE%2xma$YtGq)@(6=+n&~~y0`IYn1z!E@Us9uzEzj879Slw*0VCJ@ge@4j z58DV(2qz!wxe&=@kzJkWzh3dXYaXT@o&R1c44NU=Ir8|o%NwPUJOt)D6x=>RJA-hP zZFg0^mjGR1JCR|**JB^H$3i?RreQc$?0lo=`Y51l8x&Kv6h2giS1@|a8p~;$;hLg0 zbsJe4&Ql^-jhz_QuJH39YqkEK{j@UsX5Hr6^U}>hz}Phx77)_G6;dt$6lsR@RH?{G z3$U8+`1T4BPXZUru%wg%PPbb#tyyw1$5Sn>%@u0OU}-IdIT>=KG1F9JQf@t4dF_&m zXO)XK&>Bp}xESe;y$;^!?Y1Gg&^r6nX7SEXnuvX`vg%<R)`GIKgc$TqFN_kox>AiH zc-g=j?MyyT4_WviWHl5&!&YotohYsb2PzR3bbccR2A_?t8?D2JSQlP|*ZWOEC>%EL z$V}EHiu0nG+&wX7z2E3p86-<9=Qf$~*w7}S=Pzuu^c8B-3|OgYT4SUsqX_+%!rNwA z#43qArSooh>!H-+e$HxL^E9ug-m5TDt^9z(i3d1YsJhl>I*R`)Ipu)j`U=@M2y><W z+qkd(Cee6cqk3dZ#V>4fh->(I4e#*5=PR4r{k9P`RrvAEHM=`0Jcc<LG4&6T%QDq3 zru~9qik*mZ6c(}hn?olaEa_ff-80q(!Md|2TD;93cc=5yHn4Ak-m^K$uBY|N&;oDB zycjIdr}&TW?ov5FX42=K6Xcf*10)mzC+Q+i-8?Y=NXlj=zu{K0N7t3G7)iTMORFN= z=oO)wsj<&GMXZHNTyqB-#_E;qMCV9$uQLf582_t=mNI7^6D$RzgJ)?Xz8ZmN6(+}S z+|Gb5gnGAN1dqG(-k1;yXSM82>p=bPy|s$LyXsH#9gAQ*#Z!ukeQuo3^Qj_B0x7@9 zwHJR&=Z8c`eT5ChOL13>zvYol^t~r$um{j)4{MT<zwup_AE4goC*>QpL1XdmjGPEl z$uK$@P=5eM)_)o0|7J4yZ=N<zTcE9NQpiF8KwF+%Uf+stz~gVcoL}=41Aum>%HG{H z7NuH1n_pwZZ~!u5fBxGYBO(mw%q?0y;m8TN>;IVtwsm%2KKwKP<uJ-v0MaRp%RU=G z{qUdJ@%-*8zuesk<Fszxnu;gZ(`%b{b_ZjtJG0d_O5B_gc<nFH%h_6z*4xE?l-s*m zlfawUvInRG6T|M8pqwfoHtPPJitFymz^xhIc_#by$KP_zuB;C}LdiZwjr~+KaRujA zYRH)ylnjb`JD=}U-rp`~`{gQAr3}Y&aAYy%_@qe3!Mabj-l1bMU|Rjr(&>D0I$yv{ zN6{rWzvB|tJJ+php^PbBe7X%KJrIvmG|GZ!p`JUxZqo1mkcoP}>;-4I5-vrU%o|{P zSV4s(<#Ru{iEV1|R~CPBm7@iYx*g&d=e$;Gqj&@DP|HqSc;xzM2G#Y5jQ%(ir;k-0 zn3Z)dyIE9~T*=}`SlQa%wtRb00G2N|>#Q3%%Pn2E2TA5HxQ8qs7HI0xoj(a|_YbLA z4F3$M<aPTMWv^gZkEpS>NXZyK_C*8R;|eMe-m7eIq=PeKJ6x%Dl8pNJarzKwamv*? zisZS-s&hX=qp#|kb+<KCoq4jkT(u%Jw;1s0Kx~q;?_t?`D-{VhDY;y@O_-yT;zo2( zGV@Fu*(IE_bA{v0jSa6L{mExDr=hFeXT7?-c*ng%%8&5<JWP%axu>*2`BsxdG0|!0 z)-%hYwYWbg%E5ul14vD+bf0z{S(>#4GwTK(lhWY#!Ox}q*z$_Vlis`e9s4eo++FkW z%22u-K}*IH0<fg@m^8tFO6@-#Nw(0M6ZimLCc8JMw|fs1Fik^Ua{6-UH4uBVsQd9_ z7U2HiYf?6I_QsGnAMnea0p7ME7+EJ{W#EDcrb^Tc0+^+toV5N}@t}xTqYVnMrlI!n z2|<AZo1yV<VYBMAfm5S;lLz1tQbO|MC;OArdftCbtUlEGZ7;*D?N@);ZTD~<$ltH- zHU1J7z)k$Yor2^i^&ZB(Bz~^x5s%maWjLlg#?5DGQSr&u)#1~_iX+~c+~IE<!!~;H z<+j;^**1dD?V`eR_*R8du3;C6K^B4?W%M@{5B0wRu52{M=!cbkxhjb#)U=I~f~sfT zjXW8pTR|5iE6cf-Fd3?rzsU<k*3inOZBmQVlE;kR<|)@tuGb!A<r$t6pk0$4muhdx z4)<p8tGSKE5T!T@MqZxANWg6-yuyP=P1%VGmiR|LO{UFLo*CO4F7g42Lq^9(Qd^wt z!ha2|1gx!9VB$U&S7UPdca&5%4fJhFXH8j(StK(G2v^85@3$^f$QX;Andj?G63wZ! zrjko}R{La#!$x?E9PeHU&W@{RdrR%dQ=mW4?RTA49cpr-Tg{5q+|>|LUwb$zG8<$B z+@`9Ko}~xqJeiqrM>BIptL*L^=LT~m2U{qi=Gk>J(lfn#Ca5wpXIcp(*mex#HaurE zY5L%6ihpn!XDq*L6z8)EH7uiG)aG_V#&qG~h5YbYm967DddYdyNYyN^81*vP_<N4d zD(08sfgEn-0>k6>$Jg@;B(ZrH>kaYJc22>zttqv~v<{Vy70Et7L3!MfJ^M}d7X}-p z(t27dO665EtcRsD3n%?zvMik0fS<h6v=Ufj5e%T(t;$<HBK=FqniyhVu7)8air(*h zuJyQAkGRZz9D}pLT=ip?sLCNhOgy7>Kc$XV{VNnH9^+;jTpu(Ndh*fGS$b>lC1mi2 zIn8=Ow^!9K0@EL~HhMT$WEfU7Kb!I1EPX#KIJH$S2Nz%cb(2-`>ACUC5fyxCJ#S~H z@2|12jt)-kGgH+h*Q#z8lAZdeq+H9+xxL^E4XWrVTREn-S&&JZYPcjHw#;GS@hix~ ze}I57QF?<OYeP=-DT=B0(4~ta%aqK*XS*85zDk8%HhSlfp_!erC#8haW_Mz0^G!j| znK8EWf+zg^cYV_L)OFr=dB<V-RzGy%>xqWw^WC=9(T2z=*@{ULRU|8JsFBf65|7Ed z-@9#hG@T(H6_*~dOi+zE>{`2%u_xCnJn-s({*>>l$ATYzuh$XIMw!?LE8@Juhc-Oc z^@-j?B-}at>elyQ!&*9-;2Fa3#P-bW?8C_St#AnyH5eBA&bp)iv)_t)T9DC`GgsXb zj>tVR#y1UQsn|OYXE9g4#w-GpM|lL9FFhH)%`YoPln^|_w&B>~^!fJu3o1edZa4zU zR}L5X3*PI9D!}Cp2W^3_h+lHkZI2)Z2LG0byhy*B5P9Jf6K}BrA8hyCx>#238B|@Q zo35GnNuZ{dUB624=zZ~F>)01LBD7*uLAMKWR-U1TeBj#G?5Sz+WDD0<JnnYMET`M` zl2@tQ6Yf3_WrGTYrw5bk>l!&jZ4KWn8FigII`LJ(7_&CBwSrw{+&@M8DFIRK_!6jV zmQ6pQ7U|t???Mddp`UYj&;6NpVM+-@n7H5F>}D1mQ%A?4Gl;(VpS@@jK_6!&e}5RI z?Ht@Q*jYT0jw@)d<4I%odrqZy7!O~u!1Qxk`<y%L6&<BwM)T>Jx{MK=zo}^dfpqab z=L?C_?%G%5UQ=fW&HWW0TvMicZ`&`&GjH$`Q<|Fe(s04|aN(}r_fU2PQi_J?TdmdN z*eqgu5nDjp%(RJQ7;`!mPOdD!)jIx2#AN%-jFqf<K*C{-D29pS^=~@Ceck7<S1$8j z#rM$#yYXd{Q}^gPLIH6<R59L%x~`#okjN5=ZPWE5N}I1BKDROF7d`Tb4gRhbpRj(F zrG$IX&>}bIRL$oe1CXG*0xT`>i+WK)4O%T=`8qmh60$v4UVfgI?XzTZ7vF-EtK@vp zXPF^DAN$E2G`AX!R@TB-ARi2?$BGCAbVjr#N~Dr%T1=G|-kceIA7cXf(ZxtZt&48w zT_#+0Nf5VF8o^92Y!L#uotiUuNOfP9w${RiA)$9mP7~s8n0L~3N=v%GeHV{PF99r2 zzL<PG|HGfN1$Q{~y#VuYG#yO|byOUvn*2n%dZQO*n~@wi1zGT?d>wa$nH9^6Jbv?- z>~>_%IE~WRZ`Bm|M+Rmv$4nE%aRX@tzuIyfn%4kKNs)3Ybx$=<7Bs9yA|p)0;6zI5 zkV1k*k${Dj@Af;fp&u-<#*{OJoQVQKzLa801T#z}uf|+cuKf{SW}uuoieVo|!@Hl< zPk9LqNlTVqOgIv}<FV>7MJCaR({S@*{}WJZ+u?8?PRo~q;9byE{5-JoB3|}|@B4m} zn?ph6*B0U}20bW1J+i_Zee31TZ=uimXN1mWNOqUzMlBmm)~_$j;!CqAijG092+mj} z=oD=4A3PAD<m561@?!_chV6+KLxg4go3(|eRY@-7o(<EZ=x_`rv}P`X=X`@`pSFe2 z8)BVmUlXxpdBZg$r`b{==SUeho-I6!du8kLg+mo%as<Il!)!JqTtebYuZq!?jVfGf zFLRWE^YE|;XjTs%EgtJAZk4d}%r2)?s#$`b=dgvGCY(DAxcYQ?l(L6R<SRLc5IXvD zQJ&WDlG%yDWy(b2zx?E(z|<mfaWx)e$k1u)4}75!axD~)TrB^pl`;^*=m_xtxM-th zZtrx~6=IB_*H#SVclg@8922U?0JfT7(e+<+^M1NaO5PLQa+SR!SCl*RZiBsjO@3^% z8CY-Hy22_0`IeNyvdfJT*Bk-1(7D>w(FCw>ZsIzo|L5qb`#UIT&6d3qN5Lhe&!P^m zw;t|Fxm@JPnJHH(zZu=|U<&}(&OAvv-~m3pF6=EUkyBF-0Od{>!>z8vy3$F2RZ-Fb zZm>Lhe7LfZ7Zms#d@3L~M`rg*fMpP*{!uK8*h3QVRfigP3EZec*rD;XC_Gl|aODU( z4Mx4Rt5BRL_{)Q*2RVH}tRfT&;hbaIBRrK&0b?y{9JbDc?*G_9o>n*Ry;tH#b8;GN z=I7@2{+;pDkzG%u(qKH&xuG&Xi+YSyUe#}%e2667WBk>*_d*j%Nu3U;A1Ou6jD0pM z9Y-ZX4<VA+E_x*iv0E+ZrNL;<Utp92l_Pzk@O%TKQ`M&(8uj(i%DI)zgnd>Ea^M8F zFR(07&ni*4ePbnD@}vVK4JP$eEHz-C6;+QuYPTYKsI`RiaVjZPQ01;n$w5x5Q<z25 z@IwlQR4598uFtA$R)xZhjp(D@`v;!p$Fj@_V-|xGy-m(IKy>xc3+q1+qM%SDIE-n6 zoq%w<Bk6I!kCIo>P}l)&dRzlI6bFak+x;qcY2d1oRFbff(daU$z`7{hsz_|{*j|Yd z%`{k9XZn|O5YC!C2+kgM8^ME>u{Nos!h4Q3@?s8<HBpQP{4IG70cD*tpvQTyh<=T~ zB2mbh2J<@OWv54lZk)n23MG2y8Bn2$r!kF&;82|k4Hv^So(w-E0&eD{hxU7v{|>dc zEakp0OlIW+Eu)Bq>jy$vlRA|Wa&MoJO8ixH=l%S5NKhRvQMfze3;MJJq~a7t!zt0* zh24s(gg|%ZJKBgqI731Acv}Bj%!CR(s)s)C!`o4x3I&ZQ+#`_a&29zgi^BJ<bvfEV zVN`Jhr{Pjj{=4_pbO_kLHHL-HBf9ReRDzqAOugKd@CCFDr$NLaUu^AOiP$kwcufmQ zY!S69(p`@pcb*F~u~nH-4j#xIVN7G){ktANuR%D4<BC|avV(<-+tEymd&F}}n)E=w zFLaBCcGm>i{p8bNwwN}Rw=rWI2i(?QSj6D0SdGtM8dr<k%@Y8UQs;h`lv%5c49Li3 z1gDSasqy{HDO8rczc$h~k#PLPD0rNH+=WJ?L!xoNgs#%#e2%c{;MiLK0LT~`PWm81 zrt8di0V=URImH3!W-w?al0bLc*lj5)s-HB%{;K+9``7%H50x7@UQ8?0>M4>_$ekfb zK6Q`9K^UX4#sPWmzt;jkrvtc(!bAC-2tbD@rtuOEX)IK~$4~Vy;IHD6eRgP&4v=p| zk8^8feUT%n1MsJjMn3L67YoK)Cxx``QRQabdy=Xk3jZ<i>vGeL6^nHtG~+G-UH@G! z_H-I7(09LqO+nC?;O-@uy2o#=&e*!wQ#)33pIGdambAMjEuMkESK?&Nl<?|@TO#UU zj{jGavLf`e(aH&M4IB*7pid?G9sxues1Z-r4*WZq<bN{^@xR4O|DWp?Q(g{K|J=oU zp=H0eqb)5V4yi@W93|3oZ4V$L6Qb~bFi|H&$|=d(XM8X2es~75?-b_qS5v=ZP3<cc zma=+nuN)Nkp;VtM?*dA#3l-qQ1|oKHmdtplaGV^RJG(e@ado8`L|e9_@d{^VKVjk6 zzbfUk#8+EOUi~e=V|sD3iB{7Gxg+gymVNWr7$4G*zKqMVTOkiK$>sw%4g*eGaW$Ld zCVoihC2#a`Q-*z2jv9(#6xJ#;9l66D+}Hex*B%~&kpuJ)=-i?+>9dmS4(v8WKERwF z@xeqZzVyCGpFx0VYZ1g}y0X$=a1?q9@GV3pEgP`q8;NmCy~+#Bpt}&w5_>VSbS2fI z>YG*s?7>B#Xz<E&9?PKxrnh`fYm3BO8@Aa~c57kydqiNmhfK-ZfeE2?3$~G60AlwF z-FYU<8B4x@r-Z_%>R7vTy(Py#Q-}+rtg0RY+A-qW;bJ1sB<9eF_%ze!2(VVP_RuV> zDUf16#jbu6wAErf6G`Jom)g5T=cp#Xw7rd6!xc9HGDXOwXUdM@s2vAXzsIVpnTOB; zlKE}xsA5?@B4AjIOw#h`$IDf1lu6qBFxJ0i7rfXhW>6xpisUP3eufh}BG~s5BMa2T z7d0bB6TNh86+(-)tXr+``6vb7(Le)3pFta8bBw-%MY4ZxDMo0(oHip4LKU`#SlHyc z$_6{bY)sr$Fafj5;Y4U%Z4=i$FER@0m=yCpQSOl&YV%A_&UJNKTEKK6(L25@@x%@` z@_~<O^2CZz)8M5yQ${MTkQE(z9NQrx;G+p%K+0$GlwGxea5rt`+3x$FgzPfd$zOG& z%|2o|WQ+wz8bMpxaByLYYNl9Cn79N!*1Q^}WQd;v<#w)k)ZTW5>AWl1UgaJcK`nSb zc?<T~63~B6>Kk3m|0?1!P0%L(3o*cceeF>Nwk@qT@&nItef0Kg@Z+&c4A9vaV$!so zN2+%!N7gE=N6ywC{jH?;cQl~IDpanZ`!ONp#SXBdgN5DhB3X71_SHxCX}-V4Kl3W- zHh8}NaM@{=5r`<<@R0o6X6O~nQam#-J`;<E6s|pdCa3adV$taF4zf#%g=&Ujj4C(u z?jB;l+sW(2>_HpfnS%H|BR(rWD~*SXX^WL&1`RjmlX^iJ_CHkOU+YG9L%<XJgI<aI z?cNS8g#UYl%m0!<7SDpb&gQngT@p=j3ZM$=p*z2<Z3-5c!~G%kAU+94I`#Y!o=q?+ zcKdZLv{PNK-9XHcL7Vb=`$$^UMc{dHOkO+6{_~U<pnO*jE;PEfi90#5R^k7a?nRdN zMT({T7VN#4H%lBexeQ0OK|=++(Sg2I8lKayZprW_U!rZWd7}32-q6PS&!G<IDYpZq z1Y*U_Ork<Bk_BT0Ntne0<ErocpSE_$?WgA0>04Qklr8kBZ?>{PLIFgQ*d6ztt4^rk zcTpSH$z44gz=9}PC#89>iSI4E)u$%{1HiBThCdJjwLmCbWb~w#9<-4C|I}Q~0Xz_) zQ=9vPHbzYxXkclne0w>t*Lhr*9<lH5B$u>d5^oC^K=nz3oxalhrc=UZb3btWBnA?b zt%Jubq}2lCG*WCDruMH9+LVuqN`qCtH=WwpXaDY3bb~5-MUM_mxk$qEF>&uG^%@CD zg}2pKU!AN6@no6e^hI)X!*H3)4+xhIrMKXHIw@9N@mO}adgU#GD11az@z}hto+e|b zX3?8}_?e7!$#SIs{QL!+Z78~+`)V<ZAwCBc@zXNzfi}3(^M`YNk7?6mT>8v3O|;K> zG8=h(M`^lF;z)j;Bhx5>%hoPSPR<adx_Z-VJIs9$;xa08m%LZdr5{r+V#_|U-9M~{ zrc#df^TG#?XY>ydQ0WCIPPMcJy2<bHAdRyK&KfDLy3zgZY6pkh5)yHT-+A-REQ5eI z@O%~f%h1-Y!ajqjx9sx{nIPYWcp?DBgB*e#{bSAB(A8_-GSPkI)ke+sc~zGsWz~fm zeqJCriz~fRwaKoFLd&seym-l3z4<UT=<Qp`V2`KLF~PIp)l9+2uLIllaDEU8&2P!L zPmKZ<Oh?#J!7_RTzJC_l0pj+)-**n~)*V3J{oTYoBBgjt>d29Rf^ZW7JFr`TF`bpo z);sKx_jq4lE+}F#))yPoNGMF-W-L0$oOi6jm98c2b`ufGN<qmX{wD$uKVb{ZBqRUd zYqTa+>pVw^OLmszcwFBfJ%$NQpkFq^aJg(x`2{UGHC($M5&Fp}<6}AA7T2UzEbDpW zB7(CJC&o34jsiw|CV5>a5hDE_F2UyP<sfgmH{3wXX_><}+<EoQp%HGdc}ZUx_*hf* z-doWN|ArcP)iGHS!b~V)@w+Lcbc{#|vY9k`Z^?D({w;iq0#QMNa`;`bc$9w_x8^!^ zwebn$>$BWi4!$0hU+SsJ*4RwDjd_As=|G;mbHSwdXOMEGQp&hz!)KHCrwrRnho0Om zcKxo6cb!)3dVHqK)3XVTQy|K!*0Eb)sfSpd;s!Qhis_WHiU}BMw<(CtKY|X4hu1vW ztsDTV6FB>CeD8lqKe0Agl?d#cngSRw*fHY>q@fyixz!|T6I}Ik<lD?&-7H{~&Rvhp zbm4#P+Bdzq9e&)IZ+A)-CZ6ust+e~A{#%QA+y5AX|A}XwTLKo2&r$Y<>;`STfK(&b zUtG?=$f~0y-CTR@AcacTM9xBhxGkQ67y?c=`%oarqXP&XL+x@+hq$}{x0(2T!+$aH zJsMnq@B!)tYQv;fv!*7RCBlaWg7(6G_G|zU`3Yp5%t5wLz4@QPKSub+6aI;Uf1==@ dDEKD|{)vMBzoTFrs_Vk~tt;K-<ln2R{{d$tIotpM literal 0 HcmV?d00001 -- 1.7.11.7
On 19.02.2013 03:14, John Ferlan wrote:
Added a picture and explanation describing the virConnectOpen processing at a "higher" level, but with some source code references. --- docs/internals.html.in | 102 ++++++++++++++++++++++++++++++++++-- docs/libvirt-virConnect-example.fig | 58 ++++++++++++++++++++ docs/libvirt-virConnect-example.png | Bin 0 -> 10464 bytes 3 files changed, 156 insertions(+), 4 deletions(-) create mode 100644 docs/libvirt-virConnect-example.fig create mode 100644 docs/libvirt-virConnect-example.png
diff --git a/docs/libvirt-virConnect-example.fig b/docs/libvirt-virConnect-example.fig new file mode 100644 index 0000000..2936f22 --- /dev/null +++ b/docs/libvirt-virConnect-example.fig @@ -0,0 +1,58 @@ +#FIG 3.2 Produced by xfig version 3.2.5b +Landscape +Center +Inches +Letter
Trailing whitespace. I know it wasn't added by you, but I guess we still can drop it, can't we? Michal
On 19.02.2013 03:14, John Ferlan wrote:
Add a list of active domains, list of active/inactive networks, and list of active/inactive storage pools --- examples/hellolibvirt/hellolibvirt.c | 201 ++++++++++++++++++++++++++++++----- 1 file changed, 173 insertions(+), 28 deletions(-)
diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c index 234637e..f191782 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -85,65 +85,200 @@ out: return ret; }
- +typedef int (*virFunction)(virConnectPtr conn, + char **nameList, + int maxnames); static int -showDomains(virConnectPtr conn) +listObject(virConnectPtr conn, int maxnames, const char *objNameStr, + virFunction fcn) { - int ret = 0, i, numNames, numInactiveDomains, numActiveDomains; + int ret = 0, i, numNames; char **nameList = NULL;
- numActiveDomains = virConnectNumOfDomains(conn); - if (-1 == numActiveDomains) { + nameList = malloc(sizeof(*nameList) * maxnames); + + if (NULL == nameList) {
If blue is the sky .... These Yoda conditions should really be made inverted. But there are some even outside of the hellolibvirt.c.
ret = 1; - printf("Failed to get number of active domains\n"); + printf("Could not allocate memory for list of %s\n", objNameStr); + goto out; + } + + numNames = (*fcn)(conn, nameList, maxnames); + + if (-1 == numNames) { + ret = 1; + printf("Could not get list of %s from hypervisor\n", objNameStr); showError(conn); goto out; }
- numInactiveDomains = virConnectNumOfDefinedDomains(conn); - if (-1 == numInactiveDomains) { + printf(" %s: \n", objNameStr); + for (i = 0; i < numNames; i++) { + printf("\t%s\n", *(nameList + i)); + /* The API documentation doesn't say so, but the names + * returned by are strdup'd and must be freed here. + */
The docs should be fixed then.
+ free(*(nameList + i)); + } + +out: + free(nameList); + return ret; +}
I wonder if we should advise users to use the other list APIs that are around for a while (virConnectListAll. I guess it all boils down to question if the hellolibvirt binary is supposed to work with ancient libvirts or is just an example shipped within a release. In case it is supposed to work with prehistoric versions, we must add a fallback code. If we are satisfied with the example working with current libvirt, there's no need for fallbacking code then. Michal
On 19.02.2013 03:14, John Ferlan wrote:
I was [t]asked to make updates to the api webpage including an example of a code path from application into the driver. As I started on the task I found that the api webpage first needed a couple of updates just to fill in what seems to have been missing information. As I went through the various pages I realized that the 'better' location for the example code path seemed to be the internals page and thus I added it there. My simple example (hah!) was the virConnectOpen code path - I figured for a intro- duction to the internals web page - it'd be a good place to start.
While I was doing this I used the hellolibvirt example a bit and based on information found on the 'api' page I added some more calls into the example to print network and disk information as well as more domain data.
Finally while going through the pages I found a couple of places where drv pages weren't referenced although they were available, so I updated those (phyp and parallels).
Hopefully I've figured things out correctly, but I'm sure if I have something a bit wrong someone will correct me. Making the figure was quite frustrating as xfig is a bit less forgiving than say office writer.
John Ferlan (8): hellolibvirt: Update hellolibvirt example api: Reword objects exposed section api: Reword and clean lists for object description api: Complete list of function and naming conventions api: Add text and references for drivers section api: Add text and references for daemon Add references for phyp and parallels internals: Update to include RPC and Lock links and add new data
docs/api.html.in | 261 +++++++++++++++++++++++++---------- docs/drivers.html.in | 1 + docs/index.html.in | 6 + docs/internals.html.in | 102 +++++++++++++- docs/libvirt-virConnect-example.fig | 58 ++++++++ docs/libvirt-virConnect-example.png | Bin 0 -> 10464 bytes docs/sitemap.html.in | 8 ++ examples/hellolibvirt/hellolibvirt.c | 201 +++++++++++++++++++++++---- 8 files changed, 530 insertions(+), 107 deletions(-) create mode 100644 docs/libvirt-virConnect-example.fig create mode 100644 docs/libvirt-virConnect-example.png
ACK to the 2-8 patches. The first one needs a bit discussion IMO. Michal
On Tue, Feb 19, 2013 at 11:26:34AM +0100, Michal Privoznik wrote:
On 19.02.2013 03:14, John Ferlan wrote:
Add a list of active domains, list of active/inactive networks, and list of active/inactive storage pools --- examples/hellolibvirt/hellolibvirt.c | 201 ++++++++++++++++++++++++++++++----- 1 file changed, 173 insertions(+), 28 deletions(-)
diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c index 234637e..f191782 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -85,65 +85,200 @@ out: return ret; }
- +typedef int (*virFunction)(virConnectPtr conn, + char **nameList, + int maxnames); static int -showDomains(virConnectPtr conn) +listObject(virConnectPtr conn, int maxnames, const char *objNameStr, + virFunction fcn) { - int ret = 0, i, numNames, numInactiveDomains, numActiveDomains; + int ret = 0, i, numNames; char **nameList = NULL;
- numActiveDomains = virConnectNumOfDomains(conn); - if (-1 == numActiveDomains) { + nameList = malloc(sizeof(*nameList) * maxnames); + + if (NULL == nameList) {
If blue is the sky .... These Yoda conditions should really be made inverted. But there are some even outside of the hellolibvirt.c.
Yeah, this was my coding style from before libvirt, but it's clearly not libvirt standard and they should be removed. Dave
ret = 1; - printf("Failed to get number of active domains\n"); + printf("Could not allocate memory for list of %s\n", objNameStr); + goto out; + } + + numNames = (*fcn)(conn, nameList, maxnames); + + if (-1 == numNames) { + ret = 1; + printf("Could not get list of %s from hypervisor\n", objNameStr); showError(conn); goto out; }
- numInactiveDomains = virConnectNumOfDefinedDomains(conn); - if (-1 == numInactiveDomains) { + printf(" %s: \n", objNameStr); + for (i = 0; i < numNames; i++) { + printf("\t%s\n", *(nameList + i)); + /* The API documentation doesn't say so, but the names + * returned by are strdup'd and must be freed here. + */
The docs should be fixed then.
+ free(*(nameList + i)); + } + +out: + free(nameList); + return ret; +}
I wonder if we should advise users to use the other list APIs that are around for a while (virConnectListAll. I guess it all boils down to question if the hellolibvirt binary is supposed to work with ancient libvirts or is just an example shipped within a release. In case it is supposed to work with prehistoric versions, we must add a fallback code. If we are satisfied with the example working with current libvirt, there's no need for fallbacking code then.
Michal
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Feb 19, 2013 at 11:26:34AM +0100, Michal Privoznik wrote:
On 19.02.2013 03:14, John Ferlan wrote:
Add a list of active domains, list of active/inactive networks, and list of active/inactive storage pools --- examples/hellolibvirt/hellolibvirt.c | 201 ++++++++++++++++++++++++++++++----- 1 file changed, 173 insertions(+), 28 deletions(-)
diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c index 234637e..f191782 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -85,65 +85,200 @@ out: return ret; }
- +typedef int (*virFunction)(virConnectPtr conn, + char **nameList, + int maxnames); static int -showDomains(virConnectPtr conn) +listObject(virConnectPtr conn, int maxnames, const char *objNameStr, + virFunction fcn) { - int ret = 0, i, numNames, numInactiveDomains, numActiveDomains; + int ret = 0, i, numNames; char **nameList = NULL;
- numActiveDomains = virConnectNumOfDomains(conn); - if (-1 == numActiveDomains) { + nameList = malloc(sizeof(*nameList) * maxnames); + + if (NULL == nameList) {
If blue is the sky .... These Yoda conditions should really be made inverted. But there are some even outside of the hellolibvirt.c.
ret = 1; - printf("Failed to get number of active domains\n"); + printf("Could not allocate memory for list of %s\n", objNameStr); + goto out; + } + + numNames = (*fcn)(conn, nameList, maxnames); + + if (-1 == numNames) { + ret = 1; + printf("Could not get list of %s from hypervisor\n", objNameStr); showError(conn); goto out; }
- numInactiveDomains = virConnectNumOfDefinedDomains(conn); - if (-1 == numInactiveDomains) { + printf(" %s: \n", objNameStr); + for (i = 0; i < numNames; i++) { + printf("\t%s\n", *(nameList + i)); + /* The API documentation doesn't say so, but the names + * returned by are strdup'd and must be freed here. + */
The docs should be fixed then.
+ free(*(nameList + i)); + } + +out: + free(nameList); + return ret; +}
I wonder if we should advise users to use the other list APIs that are around for a while (virConnectListAll. I guess it all boils down to question if the hellolibvirt binary is supposed to work with ancient libvirts or is just an example shipped within a release. In case it is supposed to work with prehistoric versions, we must add a fallback code. If we are satisfied with the example working with current libvirt, there's no need for fallbacking code then.
I originally wrote hellolibvirt because at the time (prehistoric versions!) there was no good example of a trivial use of the library. I think it's not a bad idea to keep it updated with modern code since I'm guessing anybody looking at it probably intends to develop against the version from which they got it. I think it should illustrate the most basic use of the API, so it should demonstrate best practices for the functionality it implements, in this case, use the new API that lists all domains rather than the two older calls and don't bother with the fallback case. Dave
On 02/19/13 15:58, Dave Allan wrote:
On Tue, Feb 19, 2013 at 11:26:34AM +0100, Michal Privoznik wrote:
On 19.02.2013 03:14, John Ferlan wrote:
Add a list of active domains, list of active/inactive networks, and list of active/inactive storage pools --- examples/hellolibvirt/hellolibvirt.c | 201 ++++++++++++++++++++++++++++++----- 1 file changed, 173 insertions(+), 28 deletions(-)
I wonder if we should advise users to use the other list APIs that are around for a while (virConnectListAll. I guess it all boils down to question if the hellolibvirt binary is supposed to work with ancient libvirts or is just an example shipped within a release. In case it is supposed to work with prehistoric versions, we must add a fallback code. If we are satisfied with the example working with current libvirt, there's no need for fallbacking code then.
I originally wrote hellolibvirt because at the time (prehistoric versions!) there was no good example of a trivial use of the library. I think it's not a bad idea to keep it updated with modern code since I'm guessing anybody looking at it probably intends to develop against the version from which they got it. I think it should illustrate the most basic use of the API, so it should demonstrate best practices for the functionality it implements, in this case, use the new API that lists all domains rather than the two older calls and don't bother with the fallback case.
I agree. Additionally for the fallback case there's "example" code in the virsh source tree.
Dave
On 02/19/2013 05:26 AM, Michal Privoznik wrote:
On 19.02.2013 03:14, John Ferlan wrote:
ACK to the 2-8 patches. The first one needs a bit discussion IMO.
Michal
Thanks - I'll fix the whitespace in 8/8 too - I remember looking a few other examples using gitk and saw that the only changes were whitespace removal. I'll work on the changes that have been discussed and resubmit 1/8... Of course I'm still plowing through the plethora of mail generated on this list over the last 2 days. I agree that the style in there was a bit dated, but I went with the try to follow the existing style model rather than rewrite the world model. w/r/t: the API documentation - I just cut-n-paste'd comments/code - I thought to look up the documentation when I saw the comment, but probably got distracted by the time it came to actually look it up. As for using newer interfaces over older ones - that's fine too, I was merely going for different ways to extract the data. I agree that it's probably best to go with the newer ones. I did wonder while making the changes - does it make sense to "install" the example somewhere? I see a bunch of examples in my '/usr/share/doc/*/examples' directories. Not that I'd know how to do this mind you, but I supposed I'd have to be willing to learn now wouldn't I? John
On 02/19/2013 08:17 AM, Peter Krempa wrote:
I originally wrote hellolibvirt because at the time (prehistoric versions!) there was no good example of a trivial use of the library. I think it's not a bad idea to keep it updated with modern code since I'm guessing anybody looking at it probably intends to develop against the version from which they got it. I think it should illustrate the most basic use of the API, so it should demonstrate best practices for the functionality it implements, in this case, use the new API that lists all domains rather than the two older calls and don't bother with the fallback case.
I agree.
Additionally for the fallback case there's "example" code in the virsh source tree.
Likewise - letting hellolibvirt be best-practices only, and then adding a comment that points to virsh as a more complicated example that shows how to do fallback to older libvirt, seems reasonable. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 02/19/2013 03:26 AM, Michal Privoznik wrote:
On 19.02.2013 03:14, John Ferlan wrote:
Added a picture and explanation describing the virConnectOpen processing at a "higher" level, but with some source code references. ---
+++ b/docs/libvirt-virConnect-example.fig @@ -0,0 +1,58 @@ +#FIG 3.2 Produced by xfig version 3.2.5b +Landscape +Center +Inches +Letter
Trailing whitespace. I know it wasn't added by you, but I guess we still can drop it, can't we?
I think we exempted .fig from whitespace checking, precisely because it is xfig that is the (broken?) tool generating them in the first place. It becomes a question of whether it costs more time to clean up after xfig every time we touch an image, or if it is simpler to just let sleeping dogs lie. I'm okay if we don't bother in this case. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Update the code to be more in line with how code looks elsewhere in libvirt. Allow listing of domains, networks, storage pools, and network interfaces. Update the function prototypes in libvirt.c to include a message about the client needing to free() returned name fields. Fix the all domains example flags values. --- examples/hellolibvirt/hellolibvirt.c | 131 ++++++++++++++++++++++------------- src/libvirt.c | 21 +++--- 2 files changed, 91 insertions(+), 61 deletions(-) diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c index 234637e..85e5ae6 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -15,7 +15,7 @@ showError(virConnectPtr conn) virErrorPtr err; err = malloc(sizeof(*err)); - if (NULL == err) { + if (!err) { printf("Could not allocate memory for error data\n"); goto out; } @@ -56,14 +56,14 @@ showHypervisorInfo(virConnectPtr conn) * to fail if, for example, there is no connection to a * hypervisor, so check what it returns. */ hvType = virConnectGetType(conn); - if (NULL == hvType) { + if (!hvType) { ret = 1; printf("Failed to get hypervisor type\n"); showError(conn); goto out; } - if (0 != virConnectGetVersion(conn, &hvVer)) { + if (virConnectGetVersion(conn, &hvVer) < 0) { ret = 1; printf("Failed to get hypervisor version\n"); showError(conn); @@ -85,73 +85,101 @@ out: return ret; } +/* Turn off the message in order to compile here + * + * The virConnectListAll*(), vir*IsActive(), vir*GetName(), and vir*Free() + * functions require specific types which aren't possible to provide in + * this tabular manner. + */ +#pragma GCC diagnostic ignored "-Wstrict-prototypes" +typedef struct _objectTable objectTable; +struct _objectTable { + const char *objName; + int (*activeFcn)(virConnectPtr); + int (*inactiveFcn)(virConnectPtr); + int (*allFcn)(); + int (*isActiveFcn)(); + const char *(*nameFcn)(); + int (*freeFcn)(); + unsigned int flags; +}; + +objectTable fcnTable[] = { + {"domains", + virConnectNumOfDomains, virConnectNumOfDefinedDomains, + virConnectListAllDomains, virDomainIsActive, + virDomainGetName, virDomainFree, + VIR_CONNECT_LIST_DOMAINS_ACTIVE | + VIR_CONNECT_LIST_DOMAINS_INACTIVE}, + {"networks", + virConnectNumOfNetworks, virConnectNumOfDefinedNetworks, + virConnectListAllNetworks, virNetworkIsActive, + virNetworkGetName, virNetworkFree, + VIR_CONNECT_LIST_NETWORKS_ACTIVE | + VIR_CONNECT_LIST_NETWORKS_INACTIVE}, + {"storage pools", + virConnectNumOfStoragePools, virConnectNumOfDefinedStoragePools, + virConnectListAllStoragePools, virStoragePoolIsActive, + virStoragePoolGetName, virStoragePoolFree, + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE}, + {"interfaces", + virConnectNumOfInterfaces, virConnectNumOfDefinedInterfaces, + virConnectListAllInterfaces, virInterfaceIsActive, + virInterfaceGetName, virInterfaceFree, + VIR_CONNECT_LIST_INTERFACES_ACTIVE | + VIR_CONNECT_LIST_INTERFACES_INACTIVE}, + {NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0} +}; static int -showDomains(virConnectPtr conn) +showObject(virConnectPtr conn, objectTable entry) { - int ret = 0, i, numNames, numInactiveDomains, numActiveDomains; - char **nameList = NULL; + int ret = 0, numActive, numInactive, numObjects, i; + void **objects = NULL; - numActiveDomains = virConnectNumOfDomains(conn); - if (-1 == numActiveDomains) { + if ((numActive = (*entry.activeFcn)(conn)) < 0) { ret = 1; - printf("Failed to get number of active domains\n"); + printf("Failed to get number of active %s\n", entry.objName); showError(conn); goto out; } - numInactiveDomains = virConnectNumOfDefinedDomains(conn); - if (-1 == numInactiveDomains) { + if ((numInactive = (*entry.inactiveFcn)(conn)) < 0) { ret = 1; - printf("Failed to get number of inactive domains\n"); + printf("Failed to get number of inactive %s\n", entry.objName); showError(conn); goto out; } - printf("There are %d active and %d inactive domains\n", - numActiveDomains, numInactiveDomains); + printf("There are %d active and %d inactive %s:\n", + numActive, numInactive, entry.objName); - nameList = malloc(sizeof(*nameList) * numInactiveDomains); - - if (NULL == nameList) { + if ((numObjects = (*entry.allFcn)(conn, &objects, entry.flags)) < 0) { ret = 1; - printf("Could not allocate memory for list of inactive domains\n"); - goto out; - } - - numNames = virConnectListDefinedDomains(conn, - nameList, - numInactiveDomains); - - if (-1 == numNames) { - ret = 1; - printf("Could not get list of defined domains from hypervisor\n"); + printf("Failed to get all %s\n", entry.objName); showError(conn); goto out; } - - if (numNames > 0) { - printf("Inactive domains:\n"); - } - - for (i = 0 ; i < numNames ; i++) { - printf(" %s\n", *(nameList + i)); - /* The API documentation doesn't say so, but the names - * returned by virConnectListDefinedDomains are strdup'd and - * must be freed here. */ - free(*(nameList + i)); + for (i = 0; i < numObjects; i++) { + int active = (*entry.isActiveFcn)(objects[i]); + /* Sure the name could be longer, but for a quick example this works */ + printf("\t%8s (%s)\n", + (*entry.nameFcn)(objects[i]), + (active == 1 ? "active" : "non-active")); + + /* We have to free the entry */ + (*entry.freeFcn)(objects[i]); } - + free(objects); out: - free(nameList); return ret; } - int main(int argc, char *argv[]) { - int ret = 0; + int ret = 0, i; virConnectPtr conn; char *uri; @@ -163,7 +191,7 @@ main(int argc, char *argv[]) * except, possibly, the URI of the hypervisor. */ conn = virConnectOpenAuth(uri, virConnectAuthPtrDefault, 0); - if (NULL == conn) { + if (!conn) { ret = 1; printf("No connection to hypervisor\n"); showError(conn); @@ -171,7 +199,7 @@ main(int argc, char *argv[]) } uri = virConnectGetURI(conn); - if (NULL == uri) { + if (!uri) { ret = 1; printf("Failed to get URI for hypervisor connection\n"); showError(conn); @@ -181,18 +209,21 @@ main(int argc, char *argv[]) printf("Connected to hypervisor at \"%s\"\n", uri); free(uri); - if (0 != showHypervisorInfo(conn)) { + if (showHypervisorInfo(conn) < 0) { ret = 1; goto disconnect; } - if (0 != showDomains(conn)) { - ret = 1; - goto disconnect; + i = 0; + while (fcnTable[i].objName) { + ret = showObject(conn, fcnTable[i]); + if (ret != 0) + goto disconnect; + i++; } disconnect: - if (0 != virConnectClose(conn)) { + if (virConnectClose(conn) < 0) { printf("Failed to disconnect from hypervisor\n"); showError(conn); ret = 1; diff --git a/src/libvirt.c b/src/libvirt.c index 1e78500..e717f16 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8304,19 +8304,18 @@ error: * VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT, for filtering based on whether * a domain has snapshots. * - * Returns the number of domains found or -1 and sets domains to NULL in case - * of error. On success, the array stored into @doms is guaranteed to have an + * Returns the number of domains found or -1 and sets domains to NULL in case of + * error. On success, the array stored into @domains is guaranteed to have an * extra allocated element set to NULL but not included in the return count, to * make iteration easier. The caller is responsible for calling virDomainFree() - * on each array element, then calling free() on @doms. + * on each array element, then calling free() on @domains. * * Example of usage: * virDomainPtr *domains; - * virDomainPtr dom; * int i; * int ret; - * unsigned int flags = VIR_CONNECT_LIST_RUNNING | - * VIR_CONNECT_LIST_PERSISTENT; + * unsigned int flags = VIR_CONNECT_LIST_DOMAINS_RUNNING | + * VIR_CONNECT_LIST_DOMAINS_PERSISTENT; * * ret = virConnectListAllDomains(conn, &domains, flags); * if (ret < 0) @@ -10226,7 +10225,7 @@ error: * this command is inherently racy; a network can be started between a call * to virConnectNumOfNetworks() and this call; you are only guaranteed that * all currently active networks were listed if the return is less than - * @maxnames. + * @maxnames. The client must call free() on each returned name. */ int virConnectListNetworks(virConnectPtr conn, char **const names, int maxnames) @@ -11208,7 +11207,7 @@ error: * this command is inherently racy; a interface can be started between a call * to virConnectNumOfInterfaces() and this call; you are only guaranteed that * all currently active interfaces were listed if the return is less than - * @maxnames. + * @maxnames. The client must call free() on each returned name. */ int virConnectListInterfaces(virConnectPtr conn, char **const names, int maxnames) @@ -12076,7 +12075,7 @@ error: * this command is inherently racy; a pool can be started between a call to * virConnectNumOfStoragePools() and this call; you are only guaranteed * that all currently active pools were listed if the return is less than - * @maxnames. + * @maxnames. The client must call free() on each returned name. */ int virConnectListStoragePools(virConnectPtr conn, @@ -18285,7 +18284,7 @@ error: * the results, see virDomainListAllSnapshots(). * * Returns the number of domain snapshots found or -1 in case of error. - * The caller is responsible for freeing each member of the array. + * The caller is responsible to call free() for each member of the array. */ int virDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen, @@ -18534,7 +18533,7 @@ error: * the results, see virDomainSnapshotListAllChildren(). * * Returns the number of domain snapshots found or -1 in case of error. - * The caller is responsible for freeing each member of the array. + * The caller is responsible to call free() for each member of the array. */ int virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, -- 1.7.11.7
On Wed, Feb 20, 2013 at 12:38:38PM -0500, John Ferlan wrote:
Update the code to be more in line with how code looks elsewhere in libvirt. Allow listing of domains, networks, storage pools, and network interfaces.
I like the changes to make the style more in line with the rest of the codebase, however, I really think that this example code should be about a minimal use of the API to list a few domains and that's all, so I'm not in favor of extending it to list other kinds of objects. I think people can find the details of how to do that kind of thing in the API docs. Dave
Update the function prototypes in libvirt.c to include a message about the client needing to free() returned name fields. Fix the all domains example flags values. --- examples/hellolibvirt/hellolibvirt.c | 131 ++++++++++++++++++++++------------- src/libvirt.c | 21 +++--- 2 files changed, 91 insertions(+), 61 deletions(-)
diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c index 234637e..85e5ae6 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -15,7 +15,7 @@ showError(virConnectPtr conn) virErrorPtr err;
err = malloc(sizeof(*err)); - if (NULL == err) { + if (!err) { printf("Could not allocate memory for error data\n"); goto out; } @@ -56,14 +56,14 @@ showHypervisorInfo(virConnectPtr conn) * to fail if, for example, there is no connection to a * hypervisor, so check what it returns. */ hvType = virConnectGetType(conn); - if (NULL == hvType) { + if (!hvType) { ret = 1; printf("Failed to get hypervisor type\n"); showError(conn); goto out; }
- if (0 != virConnectGetVersion(conn, &hvVer)) { + if (virConnectGetVersion(conn, &hvVer) < 0) { ret = 1; printf("Failed to get hypervisor version\n"); showError(conn); @@ -85,73 +85,101 @@ out: return ret; }
+/* Turn off the message in order to compile here + * + * The virConnectListAll*(), vir*IsActive(), vir*GetName(), and vir*Free() + * functions require specific types which aren't possible to provide in + * this tabular manner. + */ +#pragma GCC diagnostic ignored "-Wstrict-prototypes" +typedef struct _objectTable objectTable; +struct _objectTable { + const char *objName; + int (*activeFcn)(virConnectPtr); + int (*inactiveFcn)(virConnectPtr); + int (*allFcn)(); + int (*isActiveFcn)(); + const char *(*nameFcn)(); + int (*freeFcn)(); + unsigned int flags; +}; + +objectTable fcnTable[] = { + {"domains", + virConnectNumOfDomains, virConnectNumOfDefinedDomains, + virConnectListAllDomains, virDomainIsActive, + virDomainGetName, virDomainFree, + VIR_CONNECT_LIST_DOMAINS_ACTIVE | + VIR_CONNECT_LIST_DOMAINS_INACTIVE}, + {"networks", + virConnectNumOfNetworks, virConnectNumOfDefinedNetworks, + virConnectListAllNetworks, virNetworkIsActive, + virNetworkGetName, virNetworkFree, + VIR_CONNECT_LIST_NETWORKS_ACTIVE | + VIR_CONNECT_LIST_NETWORKS_INACTIVE}, + {"storage pools", + virConnectNumOfStoragePools, virConnectNumOfDefinedStoragePools, + virConnectListAllStoragePools, virStoragePoolIsActive, + virStoragePoolGetName, virStoragePoolFree, + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE}, + {"interfaces", + virConnectNumOfInterfaces, virConnectNumOfDefinedInterfaces, + virConnectListAllInterfaces, virInterfaceIsActive, + virInterfaceGetName, virInterfaceFree, + VIR_CONNECT_LIST_INTERFACES_ACTIVE | + VIR_CONNECT_LIST_INTERFACES_INACTIVE}, + {NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0} +};
static int -showDomains(virConnectPtr conn) +showObject(virConnectPtr conn, objectTable entry) { - int ret = 0, i, numNames, numInactiveDomains, numActiveDomains; - char **nameList = NULL; + int ret = 0, numActive, numInactive, numObjects, i; + void **objects = NULL;
- numActiveDomains = virConnectNumOfDomains(conn); - if (-1 == numActiveDomains) { + if ((numActive = (*entry.activeFcn)(conn)) < 0) { ret = 1; - printf("Failed to get number of active domains\n"); + printf("Failed to get number of active %s\n", entry.objName); showError(conn); goto out; }
- numInactiveDomains = virConnectNumOfDefinedDomains(conn); - if (-1 == numInactiveDomains) { + if ((numInactive = (*entry.inactiveFcn)(conn)) < 0) { ret = 1; - printf("Failed to get number of inactive domains\n"); + printf("Failed to get number of inactive %s\n", entry.objName); showError(conn); goto out; }
- printf("There are %d active and %d inactive domains\n", - numActiveDomains, numInactiveDomains); + printf("There are %d active and %d inactive %s:\n", + numActive, numInactive, entry.objName);
- nameList = malloc(sizeof(*nameList) * numInactiveDomains); - - if (NULL == nameList) { + if ((numObjects = (*entry.allFcn)(conn, &objects, entry.flags)) < 0) { ret = 1; - printf("Could not allocate memory for list of inactive domains\n"); - goto out; - } - - numNames = virConnectListDefinedDomains(conn, - nameList, - numInactiveDomains); - - if (-1 == numNames) { - ret = 1; - printf("Could not get list of defined domains from hypervisor\n"); + printf("Failed to get all %s\n", entry.objName); showError(conn); goto out; } - - if (numNames > 0) { - printf("Inactive domains:\n"); - } - - for (i = 0 ; i < numNames ; i++) { - printf(" %s\n", *(nameList + i)); - /* The API documentation doesn't say so, but the names - * returned by virConnectListDefinedDomains are strdup'd and - * must be freed here. */ - free(*(nameList + i)); + for (i = 0; i < numObjects; i++) { + int active = (*entry.isActiveFcn)(objects[i]); + /* Sure the name could be longer, but for a quick example this works */ + printf("\t%8s (%s)\n", + (*entry.nameFcn)(objects[i]), + (active == 1 ? "active" : "non-active")); + + /* We have to free the entry */ + (*entry.freeFcn)(objects[i]); } - + free(objects); out: - free(nameList); return ret; }
- int main(int argc, char *argv[]) { - int ret = 0; + int ret = 0, i; virConnectPtr conn; char *uri;
@@ -163,7 +191,7 @@ main(int argc, char *argv[]) * except, possibly, the URI of the hypervisor. */ conn = virConnectOpenAuth(uri, virConnectAuthPtrDefault, 0);
- if (NULL == conn) { + if (!conn) { ret = 1; printf("No connection to hypervisor\n"); showError(conn); @@ -171,7 +199,7 @@ main(int argc, char *argv[]) }
uri = virConnectGetURI(conn); - if (NULL == uri) { + if (!uri) { ret = 1; printf("Failed to get URI for hypervisor connection\n"); showError(conn); @@ -181,18 +209,21 @@ main(int argc, char *argv[]) printf("Connected to hypervisor at \"%s\"\n", uri); free(uri);
- if (0 != showHypervisorInfo(conn)) { + if (showHypervisorInfo(conn) < 0) { ret = 1; goto disconnect; }
- if (0 != showDomains(conn)) { - ret = 1; - goto disconnect; + i = 0; + while (fcnTable[i].objName) { + ret = showObject(conn, fcnTable[i]); + if (ret != 0) + goto disconnect; + i++; }
disconnect: - if (0 != virConnectClose(conn)) { + if (virConnectClose(conn) < 0) { printf("Failed to disconnect from hypervisor\n"); showError(conn); ret = 1; diff --git a/src/libvirt.c b/src/libvirt.c index 1e78500..e717f16 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8304,19 +8304,18 @@ error: * VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT, for filtering based on whether * a domain has snapshots. * - * Returns the number of domains found or -1 and sets domains to NULL in case - * of error. On success, the array stored into @doms is guaranteed to have an + * Returns the number of domains found or -1 and sets domains to NULL in case of + * error. On success, the array stored into @domains is guaranteed to have an * extra allocated element set to NULL but not included in the return count, to * make iteration easier. The caller is responsible for calling virDomainFree() - * on each array element, then calling free() on @doms. + * on each array element, then calling free() on @domains. * * Example of usage: * virDomainPtr *domains; - * virDomainPtr dom; * int i; * int ret; - * unsigned int flags = VIR_CONNECT_LIST_RUNNING | - * VIR_CONNECT_LIST_PERSISTENT; + * unsigned int flags = VIR_CONNECT_LIST_DOMAINS_RUNNING | + * VIR_CONNECT_LIST_DOMAINS_PERSISTENT; * * ret = virConnectListAllDomains(conn, &domains, flags); * if (ret < 0) @@ -10226,7 +10225,7 @@ error: * this command is inherently racy; a network can be started between a call * to virConnectNumOfNetworks() and this call; you are only guaranteed that * all currently active networks were listed if the return is less than - * @maxnames. + * @maxnames. The client must call free() on each returned name. */ int virConnectListNetworks(virConnectPtr conn, char **const names, int maxnames) @@ -11208,7 +11207,7 @@ error: * this command is inherently racy; a interface can be started between a call * to virConnectNumOfInterfaces() and this call; you are only guaranteed that * all currently active interfaces were listed if the return is less than - * @maxnames. + * @maxnames. The client must call free() on each returned name. */ int virConnectListInterfaces(virConnectPtr conn, char **const names, int maxnames) @@ -12076,7 +12075,7 @@ error: * this command is inherently racy; a pool can be started between a call to * virConnectNumOfStoragePools() and this call; you are only guaranteed * that all currently active pools were listed if the return is less than - * @maxnames. + * @maxnames. The client must call free() on each returned name. */ int virConnectListStoragePools(virConnectPtr conn, @@ -18285,7 +18284,7 @@ error: * the results, see virDomainListAllSnapshots(). * * Returns the number of domain snapshots found or -1 in case of error. - * The caller is responsible for freeing each member of the array. + * The caller is responsible to call free() for each member of the array. */ int virDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen, @@ -18534,7 +18533,7 @@ error: * the results, see virDomainSnapshotListAllChildren(). * * Returns the number of domain snapshots found or -1 in case of error. - * The caller is responsible for freeing each member of the array. + * The caller is responsible to call free() for each member of the array. */ int virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, -- 1.7.11.7
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 02/20/2013 08:05 PM, Dave Allan wrote:
On Wed, Feb 20, 2013 at 12:38:38PM -0500, John Ferlan wrote:
Update the code to be more in line with how code looks elsewhere in libvirt. Allow listing of domains, networks, storage pools, and network interfaces.
I like the changes to make the style more in line with the rest of the codebase, however, I really think that this example code should be about a minimal use of the API to list a few domains and that's all, so I'm not in favor of extending it to list other kinds of objects. I think people can find the details of how to do that kind of thing in the API docs.
Dave
Update the function prototypes in libvirt.c to include a message about the client needing to free() returned name fields. Fix the all domains example flags values. --- examples/hellolibvirt/hellolibvirt.c | 131 ++++++++++++++++++++++------------- src/libvirt.c | 21 +++--- 2 files changed, 91 insertions(+), 61 deletions(-)
Any other (strong) opinions one way or another? Should I remove the hellolibvirt.c for now? Adding network, volume groups, and interfaces was (mostly) trivial once I got past the set up the structure to handle multiple types. It certainly shows how similar the API's are for various objects and how trivial it is to gather generic information for each. Any thoughts on the libvirt.c changes which are primarily documentation of the need to free the 'names' returned. That was a result of a review comment which noted that the comment in hellolibvirt.c if true should be rectified. I forgot to put the v2->v1 differences marker when generating the patch to call this one out specifically. Thanks, John
On 02/22/2013 07:46 AM, John Ferlan wrote:
On 02/20/2013 08:05 PM, Dave Allan wrote:
Update the code to be more in line with how code looks elsewhere in libvirt. Allow listing of domains, networks, storage pools, and network interfaces. I like the changes to make the style more in line with the rest of the codebase, however, I really think that this example code should be about a minimal use of the API to list a few domains and that's all, so I'm not in favor of extending it to list other kinds of objects. I
On Wed, Feb 20, 2013 at 12:38:38PM -0500, John Ferlan wrote: think people can find the details of how to do that kind of thing in the API docs.
Dave
Update the function prototypes in libvirt.c to include a message about the client needing to free() returned name fields. Fix the all domains example flags values. --- examples/hellolibvirt/hellolibvirt.c | 131 ++++++++++++++++++++++------------- src/libvirt.c | 21 +++--- 2 files changed, 91 insertions(+), 61 deletions(-) Any other (strong) opinions one way or another? Should I remove the hellolibvirt.c for now?
Adding network, volume groups, and interfaces was (mostly) trivial once I got past the set up the structure to handle multiple types. It certainly shows how similar the API's are for various objects and how trivial it is to gather generic information for each.
That's a neat idea, but I think putting the actual functions being called into a table and calling them indirectly obscures what's trying to be exhibited. Aside from that, the virInterface API *still* isn't available on all platforms, so I don't think that's a good thing to have in a "basic" example program - we would get too many noobs asking why the example program is "broken". I think it would be better to just provide simple examples for domain, with a comment somewhere that says "storage pool, network, and interface APIs work in a similar fashion" or something like that. (And maybe you could have this more complicated example called hellolibvirt2.c or something)
Any thoughts on the libvirt.c changes which are primarily documentation of the need to free the 'names' returned. That was a result of a review comment which noted that the comment in hellolibvirt.c if true should be rectified. I forgot to put the v2->v1 differences marker when generating the patch to call this one out specifically.
Those changes seem fine to me, although maybe they should be in a separate patch, since they aren't really part of updating hellolibvirt.
Update the function prototypes to include a message about the client needing to free() returned name fields. Fix the all domains example flags values. v3->v2 diff: Separted this out to it's own patch. --- src/libvirt.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 934997a..8447c0e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8304,19 +8304,18 @@ error: * VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT, for filtering based on whether * a domain has snapshots. * - * Returns the number of domains found or -1 and sets domains to NULL in case - * of error. On success, the array stored into @doms is guaranteed to have an + * Returns the number of domains found or -1 and sets domains to NULL in case of + * error. On success, the array stored into @domains is guaranteed to have an * extra allocated element set to NULL but not included in the return count, to * make iteration easier. The caller is responsible for calling virDomainFree() - * on each array element, then calling free() on @doms. + * on each array element, then calling free() on @domains. * * Example of usage: * virDomainPtr *domains; - * virDomainPtr dom; * int i; * int ret; - * unsigned int flags = VIR_CONNECT_LIST_RUNNING | - * VIR_CONNECT_LIST_PERSISTENT; + * unsigned int flags = VIR_CONNECT_LIST_DOMAINS_RUNNING | + * VIR_CONNECT_LIST_DOMAINS_PERSISTENT; * * ret = virConnectListAllDomains(conn, &domains, flags); * if (ret < 0) @@ -10226,7 +10225,7 @@ error: * this command is inherently racy; a network can be started between a call * to virConnectNumOfNetworks() and this call; you are only guaranteed that * all currently active networks were listed if the return is less than - * @maxnames. + * @maxnames. The client must call free() on each returned name. */ int virConnectListNetworks(virConnectPtr conn, char **const names, int maxnames) @@ -11208,7 +11207,7 @@ error: * this command is inherently racy; a interface can be started between a call * to virConnectNumOfInterfaces() and this call; you are only guaranteed that * all currently active interfaces were listed if the return is less than - * @maxnames. + * @maxnames. The client must call free() on each returned name. */ int virConnectListInterfaces(virConnectPtr conn, char **const names, int maxnames) @@ -12076,7 +12075,7 @@ error: * this command is inherently racy; a pool can be started between a call to * virConnectNumOfStoragePools() and this call; you are only guaranteed * that all currently active pools were listed if the return is less than - * @maxnames. + * @maxnames. The client must call free() on each returned name. */ int virConnectListStoragePools(virConnectPtr conn, @@ -18440,7 +18439,7 @@ error: * the results, see virDomainListAllSnapshots(). * * Returns the number of domain snapshots found or -1 in case of error. - * The caller is responsible for freeing each member of the array. + * The caller is responsible to call free() for each member of the array. */ int virDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen, @@ -18689,7 +18688,7 @@ error: * the results, see virDomainSnapshotListAllChildren(). * * Returns the number of domain snapshots found or -1 in case of error. - * The caller is responsible for freeing each member of the array. + * The caller is responsible to call free() for each member of the array. */ int virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, -- 1.7.11.7
v3->v2 difference: Reduced the amount of change to a few comments and adjusting the (NULL == xxx) and (-1 == xxx) checks Since these are just documentation changes - once ACK'd is it OK to push now or should I wait for after the freeze? Tks, John --- examples/hellolibvirt/hellolibvirt.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c index 234637e..335a75e 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -1,5 +1,6 @@ /* This file contains trivial example code to connect to the running - * hypervisor and gather a few bits of information. */ + * hypervisor and gather a few bits of information about domains. + * Similar API's exist for storage pools, networks, and interfaces. */ #include <config.h> @@ -15,7 +16,7 @@ showError(virConnectPtr conn) virErrorPtr err; err = malloc(sizeof(*err)); - if (NULL == err) { + if (!err) { printf("Could not allocate memory for error data\n"); goto out; } @@ -56,7 +57,7 @@ showHypervisorInfo(virConnectPtr conn) * to fail if, for example, there is no connection to a * hypervisor, so check what it returns. */ hvType = virConnectGetType(conn); - if (NULL == hvType) { + if (!hvType) { ret = 1; printf("Failed to get hypervisor type\n"); showError(conn); @@ -93,7 +94,7 @@ showDomains(virConnectPtr conn) char **nameList = NULL; numActiveDomains = virConnectNumOfDomains(conn); - if (-1 == numActiveDomains) { + if (numActiveDomains == -1) { ret = 1; printf("Failed to get number of active domains\n"); showError(conn); @@ -101,7 +102,7 @@ showDomains(virConnectPtr conn) } numInactiveDomains = virConnectNumOfDefinedDomains(conn); - if (-1 == numInactiveDomains) { + if (numInactiveDomains == -1) { ret = 1; printf("Failed to get number of inactive domains\n"); showError(conn); @@ -113,17 +114,20 @@ showDomains(virConnectPtr conn) nameList = malloc(sizeof(*nameList) * numInactiveDomains); - if (NULL == nameList) { + if (!nameList) { ret = 1; printf("Could not allocate memory for list of inactive domains\n"); goto out; } + /* The virConnectListDomains() will return a list of the active domains. + * Alternatively the virConnectListAllDomains() API would return a list + * of both active and inactive domains */ numNames = virConnectListDefinedDomains(conn, nameList, numInactiveDomains); - if (-1 == numNames) { + if (numNames == -1) { ret = 1; printf("Could not get list of defined domains from hypervisor\n"); showError(conn); @@ -136,9 +140,7 @@ showDomains(virConnectPtr conn) for (i = 0 ; i < numNames ; i++) { printf(" %s\n", *(nameList + i)); - /* The API documentation doesn't say so, but the names - * returned by virConnectListDefinedDomains are strdup'd and - * must be freed here. */ + /* must free the returned named per the API documentation */ free(*(nameList + i)); } @@ -163,7 +165,7 @@ main(int argc, char *argv[]) * except, possibly, the URI of the hypervisor. */ conn = virConnectOpenAuth(uri, virConnectAuthPtrDefault, 0); - if (NULL == conn) { + if (!conn) { ret = 1; printf("No connection to hypervisor\n"); showError(conn); @@ -171,7 +173,7 @@ main(int argc, char *argv[]) } uri = virConnectGetURI(conn); - if (NULL == uri) { + if (!uri) { ret = 1; printf("Failed to get URI for hypervisor connection\n"); showError(conn); -- 1.7.11.7
On 02/26/2013 07:18 AM, John Ferlan wrote:
Update the function prototypes to include a message about the client needing to free() returned name fields. Fix the all domains example flags values.
v3->v2 diff: Separted this out to it's own patch.
s/Separted/Separated/ then again, the information about v3->v2 diff belongs after a --- line, so that 'git am' won't put it into git history (if someone else is applying the patch). If you are pushing the patch yourself, then be sure to edit the commit message to drop version history at that time (it's useful on list, but doesn't need to be part of the permanent logs).
--- src/libvirt.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
ACK. Let's get this into 1.0.3. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 02/26/2013 07:20 AM, John Ferlan wrote: I would have put '---' here, since...
v3->v2 difference: Reduced the amount of change to a few comments and adjusting the (NULL == xxx) and (-1 == xxx) checks
Since these are just documentation changes - once ACK'd is it OK to push now or should I wait for after the freeze?
Tks,
...this information isn't useful in the final git log, but makes sense in reviewing. This patch is safe for 1.0.3 (as you point out, it is a doc improvement, and can't cause any bugs in libvirtd)
John --- examples/hellolibvirt/hellolibvirt.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)
@@ -93,7 +94,7 @@ showDomains(virConnectPtr conn) char **nameList = NULL;
numActiveDomains = virConnectNumOfDomains(conn); - if (-1 == numActiveDomains) { + if (numActiveDomains == -1) { ret = 1; printf("Failed to get number of active domains\n"); showError(conn);
virConnectNumOfDomains is inherently racy. Wouldn't it be better to just drop this section of our example?
@@ -101,7 +102,7 @@ showDomains(virConnectPtr conn) }
numInactiveDomains = virConnectNumOfDefinedDomains(conn); - if (-1 == numInactiveDomains) { + if (numInactiveDomains == -1) { ret = 1; printf("Failed to get number of inactive domains\n"); showError(conn);
Same question.
@@ -113,17 +114,20 @@ showDomains(virConnectPtr conn)
nameList = malloc(sizeof(*nameList) * numInactiveDomains);
- if (NULL == nameList) { + if (!nameList) { ret = 1; printf("Could not allocate memory for list of inactive domains\n"); goto out; }
+ /* The virConnectListDomains() will return a list of the active domains. + * Alternatively the virConnectListAllDomains() API would return a list + * of both active and inactive domains */ numNames = virConnectListDefinedDomains(conn, nameList, numInactiveDomains);
I think it would be better to update the example to JUST use virConnectListAllDomains(), and completely avoid virConnectListDefinedDomains.
- if (-1 == numNames) { + if (numNames == -1) { ret = 1; printf("Could not get list of defined domains from hypervisor\n"); showError(conn); @@ -136,9 +140,7 @@ showDomains(virConnectPtr conn)
for (i = 0 ; i < numNames ; i++) { printf(" %s\n", *(nameList + i)); - /* The API documentation doesn't say so, but the names - * returned by virConnectListDefinedDomains are strdup'd and - * must be freed here. */ + /* must free the returned named per the API documentation */ free(*(nameList + i));
Pre-existing, but '*(nameList + i)' looks odd; 'nameList[i]' looks nicer. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 02/26/2013 07:54 PM, Eric Blake wrote:
On 02/26/2013 07:20 AM, John Ferlan wrote:
I would have put '---' here, since...
The information was only in the email not part of the git history. In the future I'll remember to put that after the '---'.
examples/hellolibvirt/hellolibvirt.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)
@@ -93,7 +94,7 @@ showDomains(virConnectPtr conn) char **nameList = NULL;
numActiveDomains = virConnectNumOfDomains(conn); - if (-1 == numActiveDomains) { + if (numActiveDomains == -1) { ret = 1; printf("Failed to get number of active domains\n"); showError(conn);
virConnectNumOfDomains is inherently racy. Wouldn't it be better to just drop this section of our example?
Considering the back and forth I decided to make "less" change because it's just an example, but I suppose I took too much off the top. So below are proposed adjustments.
@@ -101,7 +102,7 @@ showDomains(virConnectPtr conn) }
numInactiveDomains = virConnectNumOfDefinedDomains(conn); - if (-1 == numInactiveDomains) { + if (numInactiveDomains == -1) { ret = 1; printf("Failed to get number of inactive domains\n"); showError(conn);
Same question.
@@ -113,17 +114,20 @@ showDomains(virConnectPtr conn)
nameList = malloc(sizeof(*nameList) * numInactiveDomains);
- if (NULL == nameList) { + if (!nameList) { ret = 1; printf("Could not allocate memory for list of inactive domains\n"); goto out; }
+ /* The virConnectListDomains() will return a list of the active domains. + * Alternatively the virConnectListAllDomains() API would return a list + * of both active and inactive domains */ numNames = virConnectListDefinedDomains(conn, nameList, numInactiveDomains);
I think it would be better to update the example to JUST use virConnectListAllDomains(), and completely avoid virConnectListDefinedDomains.
- if (-1 == numNames) { + if (numNames == -1) { ret = 1; printf("Could not get list of defined domains from hypervisor\n"); showError(conn); @@ -136,9 +140,7 @@ showDomains(virConnectPtr conn)
for (i = 0 ; i < numNames ; i++) { printf(" %s\n", *(nameList + i)); - /* The API documentation doesn't say so, but the names - * returned by virConnectListDefinedDomains are strdup'd and - * must be freed here. */ + /* must free the returned named per the API documentation */ free(*(nameList + i));
Pre-existing, but '*(nameList + i)' looks odd; 'nameList[i]' looks nicer.
Here's the differences from the v3 to what seems to be a happy medium. The virConnectNum* functions are still used just to "show" they exist and how to use them. There's a comment before the usage. diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellol index 335a75e..26dd67f 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -91,8 +91,14 @@ static int showDomains(virConnectPtr conn) { int ret = 0, i, numNames, numInactiveDomains, numActiveDomains; - char **nameList = NULL; - + int flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE | + VIR_CONNECT_LIST_DOMAINS_INACTIVE; + virDomainPtr *nameList = NULL; + * the current call. A domain could be started or stopped and any + * assumptions made purely on these return values could result in + * unexpected results */ numActiveDomains = virConnectNumOfDomains(conn); if (numActiveDomains == -1) { ret = 1; @@ -112,40 +118,24 @@ showDomains(virConnectPtr conn) printf("There are %d active and %d inactive domains\n", numActiveDomains, numInactiveDomains); - nameList = malloc(sizeof(*nameList) * numInactiveDomains); - - if (!nameList) { - ret = 1; - printf("Could not allocate memory for list of inactive domains\n"); - goto out; - } - - /* The virConnectListDomains() will return a list of the active domains. - * Alternatively the virConnectListAllDomains() API would return a list - * of both active and inactive domains */ - numNames = virConnectListDefinedDomains(conn, - nameList, - numInactiveDomains); - - if (numNames == -1) { - ret = 1; - printf("Could not get list of defined domains from hypervisor\n"); - showError(conn); - goto out; - } - - if (numNames > 0) { - printf("Inactive domains:\n"); - } - - for (i = 0 ; i < numNames ; i++) { - printf(" %s\n", *(nameList + i)); + /* Return a list of all active and inactive domains. Using this API + * instead of virConnectListDomains() and virConnectListDefinedDomains() + * is preferred since it "solves" an inherit race between separated API + * calls if domains are started or stopped between calls */ + numNames = virConnectListAllDomains(conn, + &nameList, + flags); + for (i = 0; i < numNames; i++) { + int active = virDomainIsActive(nameList[i]); + printf(" %8s (%s)\n", + virDomainGetName(nameList[i]), + (active == 1 ? "active" : "non-active")); /* must free the returned named per the API documentation */ - free(*(nameList + i)); + virDomainFree(nameList[i]); } + free(nameList); out: - free(nameList); return ret; } This changes the output from: Attempting to connect to hypervisor Connected to hypervisor at "qemu:///system" Hypervisor: "QEMU" version: 0.32.656 There are 0 active and 2 inactive domains Inactive domains: foo bar Disconnected from hypervisor to Attempting to connect to hypervisor Connected to hypervisor at "qemu:///system" Hypervisor: "QEMU" version: 0.32.656 There are 0 active and 2 inactive domains foo (non-active) bar (non-active) Disconnected from hypervisor John
On 02/27/2013 05:28 AM, John Ferlan wrote:
On 02/26/2013 07:54 PM, Eric Blake wrote:
On 02/26/2013 07:20 AM, John Ferlan wrote:
I would have put '---' here, since...
The information was only in the email not part of the git history. In the future I'll remember to put that after the '---'.
It makes the most difference on projects where you don't have push rights, because the maintainer with push rights will use 'git am' to snarf in your email message, which discards comments after ---. Once you have push rights yourself, 'git am' is no longer involved in the loop, so there is nothing in the workflow to strip the comments, at which point it doesn't matter where you injected the comments into your email, other than the consistency factor. At any rate, it's always nice to know how to make the most use of a workflow that simplifies review time, which includes being consistent by injecting your email-only comments after ---.
Here's the differences from the v3 to what seems to be a happy medium.
The virConnectNum* functions are still used just to "show" they exist and how to use them. There's a comment before the usage.
diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellol index 335a75e..26dd67f 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -91,8 +91,14 @@ static int showDomains(virConnectPtr conn) { int ret = 0, i, numNames, numInactiveDomains, numActiveDomains; - char **nameList = NULL; - + int flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE | + VIR_CONNECT_LIST_DOMAINS_INACTIVE;
Technically, a flags of 0 will give the same result (as the combination of ACTIVE|INACTIVE covers all domains); but this is reasonable for the example program (easier to see what to modify to get just half the set).
+ virDomainPtr *nameList = NULL; + * the current call. A domain could be started or stopped and any + * assumptions made purely on these return values could result in + * unexpected results */ numActiveDomains = virConnectNumOfDomains(conn); if (numActiveDomains == -1) { ret = 1;
+ /* Return a list of all active and inactive domains. Using this API + * instead of virConnectListDomains() and virConnectListDefinedDomains() + * is preferred since it "solves" an inherit race between separated API + * calls if domains are started or stopped between calls */ + numNames = virConnectListAllDomains(conn, + &nameList, + flags); + for (i = 0; i < numNames; i++) { + int active = virDomainIsActive(nameList[i]); + printf(" %8s (%s)\n", + virDomainGetName(nameList[i]), + (active == 1 ? "active" : "non-active")); /* must free the returned named per the API documentation */ - free(*(nameList + i)); + virDomainFree(nameList[i]); } + free(nameList);
out: - free(nameList); return ret;
ACK.
This changes the output from:
Attempting to connect to hypervisor Connected to hypervisor at "qemu:///system" Hypervisor: "QEMU" version: 0.32.656 There are 0 active and 2 inactive domains Inactive domains: foo bar Disconnected from hypervisor
to
Attempting to connect to hypervisor Connected to hypervisor at "qemu:///system" Hypervisor: "QEMU" version: 0.32.656 There are 0 active and 2 inactive domains foo (non-active) bar (non-active) Disconnected from hypervisor
This might be worth putting in the commit message. At any rate, since this is a doc-only change, I'm comfortable with you pushing it before the 1.0.3 release. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 02/18/2013 09:14 PM, John Ferlan wrote:
docs/api.html.in | 261 +++++++++++++++++++++++++---------- docs/drivers.html.in | 1 + docs/index.html.in | 6 + docs/internals.html.in | 102 +++++++++++++- docs/libvirt-virConnect-example.fig | 58 ++++++++ docs/libvirt-virConnect-example.png | Bin 0 -> 10464 bytes docs/sitemap.html.in | 8 ++ examples/hellolibvirt/hellolibvirt.c | 201 +++++++++++++++++++++++---- 8 files changed, 530 insertions(+), 107 deletions(-) create mode 100644 docs/libvirt-virConnect-example.fig create mode 100644 docs/libvirt-virConnect-example.png
pushed. thanks. John
participants (6)
-
Dave Allan -
Eric Blake -
John Ferlan -
Laine Stump -
Michal Privoznik -
Peter Krempa