[Libguestfs] [PATCH 5/7] New APIs: add-domain and add-libvirt-dom.
Daniel P. Berrange
berrange at redhat.com
Wed Nov 10 11:58:40 UTC 2010
On Wed, Nov 10, 2010 at 11:46:31AM +0000, Richard W.M. Jones wrote:
> diff --git a/src/guestfs.h b/src/guestfs.h
> index 0f4f9fd..be7398a 100644
> --- a/src/guestfs.h
> +++ b/src/guestfs.h
> @@ -79,6 +79,11 @@ extern void guestfs_set_private (guestfs_h *g, const char *key, void *data);
> extern void *guestfs_get_private (guestfs_h *g, const char *key);
>
> /*--- Structures and actions ---*/
> +
> +/* Hack to avoid needing to include <libvirt.h> */
> +typedef struct _virDomain virDomain;
> +typedef virDomain *virDomainPtr;
> +
> #include <rpc/types.h>
> #include <rpc/xdr.h>
> #include <guestfs-structs.h>
Surely this will break any application whose code does
#include <libvirt/libvirt.h>
#include <guestfs.h>
because they'll get a duplicated definition of virDomain from
both headers.
IMHO, this hack is a waste of time and it is better to just
#include <libvirt/libvirt.h> from the header files instead.
A build time dependancy on the libvirt client is really not
a burden, when you consider that weight of the entire build
chain you already have to pull in. Even with the build time
dep, you can still have the dep be optional at runtime via
your use of dlopen().
> diff --git a/src/virt.c b/src/virt.c
> new file mode 100644
> index 0000000..5816f61
> --- /dev/null
> +++ b/src/virt.c
> @@ -0,0 +1,287 @@
> +/* libguestfs
> + * Copyright (C) 2010 Red Hat Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <config.h>
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <assert.h>
> +
> +#ifdef HAVE_LIBVIRT
> +#include <libvirt/libvirt.h>
> +#include <libvirt/virterror.h>
> +#endif
> +
> +#ifdef HAVE_LIBXML2
> +#include <libxml/xpath.h>
> +#include <libxml/parser.h>
> +#include <libxml/tree.h>
> +#endif
> +
> +#include "guestfs.h"
> +#include "guestfs-internal.h"
> +#include "guestfs-internal-actions.h"
> +#include "guestfs_protocol.h"
> +
> +#if defined(HAVE_LIBVIRT) && defined(HAVE_LIBXML2)
> +
> +static void init_libxml2 (void) __attribute__((constructor));
> +
> +static void
> +init_libxml2 (void)
> +{
> + /* I am told that you don't really need to call virInitialize ... */
> +
> + xmlInitParser ();
> + LIBXML_TEST_VERSION;
> +}
> +
> +int
> +guestfs__add_domain (guestfs_h *g, const char *domain_name,
> + const struct guestfs_add_domain_argv *optargs)
> +{
> + virErrorPtr err;
> + virConnectPtr conn = NULL;
> + virDomainPtr dom = NULL;
> + int r = -1;
> + const char *libvirturi;
> + int readonly;
> + const char *iface;
> + struct guestfs_add_libvirt_dom_argv optargs2 = { .bitmask = 0 };
> +
> + libvirturi = optargs->bitmask & GUESTFS_ADD_DOMAIN_LIBVIRTURI_BITMASK
> + ? optargs->libvirturi : NULL;
> + readonly = optargs->bitmask & GUESTFS_ADD_DOMAIN_READONLY_BITMASK
> + ? optargs->readonly : 0;
> + iface = optargs->bitmask & GUESTFS_ADD_DOMAIN_IFACE_BITMASK
> + ? optargs->iface : NULL;
> +
> + /* Connect to libvirt, find the domain. */
> + conn = virConnectOpenReadOnly (libvirturi);
> + if (!conn) {
> + err = virGetLastError ();
> + error (g, _("could not connect to libvirt (code %d, domain %d): %s"),
> + err->code, err->domain, err->message);
> + goto cleanup;
> + }
> +
> + dom = virDomainLookupByName (conn, domain_name);
> + if (!dom) {
> + err = virConnGetLastError (conn);
> + error (g, _("no libvirt domain called '%s': %s"),
> + domain_name, err->message);
> + goto cleanup;
> + }
> +
> + if (readonly) {
> + optargs2.bitmask |= GUESTFS_ADD_LIBVIRT_DOM_READONLY_BITMASK;
> + optargs2.readonly = readonly;
> + }
> + if (iface) {
> + optargs2.bitmask |= GUESTFS_ADD_LIBVIRT_DOM_IFACE_BITMASK;
> + optargs2.iface = iface;
> + }
> +
> + r = guestfs__add_libvirt_dom (g, dom, &optargs2);
> +
> + cleanup:
> + if (dom) virDomainFree (dom);
> + if (conn) virConnectClose (conn);
> +
> + return r;
> +}
> +
> +int
> +guestfs__add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
> + const struct guestfs_add_libvirt_dom_argv *optargs)
> +{
> + int r = -1, nr_added = 0, i;
> + virErrorPtr err;
> + virConnectPtr conn = virDomainGetConnect (dom);
> + xmlDocPtr doc = NULL;
> + xmlXPathContextPtr xpathCtx = NULL;
> + xmlXPathObjectPtr xpathObj = NULL;
> + char *xml = NULL;
> + int readonly;
> + const char *iface;
> + int cmdline_pos;
> +
> + cmdline_pos = guestfs___checkpoint_cmdline (g);
> +
> + readonly = optargs->bitmask & GUESTFS_ADD_LIBVIRT_DOM_READONLY_BITMASK
> + ? optargs->readonly : 0;
> + iface = optargs->bitmask & GUESTFS_ADD_LIBVIRT_DOM_IFACE_BITMASK
> + ? optargs->iface : NULL;
> +
> + if (!readonly) {
> + virDomainInfo info;
> + if (virDomainGetInfo (dom, &info) == -1) {
> + err = virConnGetLastError (conn);
> + error (g, _("error getting domain info: %s"), err->message);
> + goto cleanup;
> + }
> + if (info.state != VIR_DOMAIN_SHUTOFF) {
> + error (g, _("error: domain is a live virtual machine.\nYou must use readonly access because write access to a running virtual machine\ncan cause disk corruption."));
> + goto cleanup;
> + }
> + }
> +
> + /* Domain XML. */
> + xml = virDomainGetXMLDesc (dom, 0);
> +
> + if (!xml) {
> + err = virConnGetLastError (conn);
> + error (g, _("error reading libvirt XML information: %s"),
> + err->message);
> + goto cleanup;
> + }
> +
> + /* Now the horrible task of parsing out the fields we need from the XML.
> + * http://www.xmlsoft.org/examples/xpath1.c
> + */
> + doc = xmlParseMemory (xml, strlen (xml));
> + if (doc == NULL) {
> + error (g, _("unable to parse XML information returned by libvirt"));
> + goto cleanup;
> + }
> +
> + xpathCtx = xmlXPathNewContext (doc);
> + if (xpathCtx == NULL) {
> + error (g, _("unable to create new XPath context"));
> + goto cleanup;
> + }
> +
> + /* This gives us a set of all the <disk> nodes. */
> + xpathObj = xmlXPathEvalExpression (BAD_CAST "//devices/disk", xpathCtx);
> + if (xpathObj == NULL) {
> + error (g, _("unable to evaluate XPath expression"));
> + goto cleanup;
> + }
> +
> + xmlNodeSetPtr nodes = xpathObj->nodesetval;
> + for (i = 0; i < nodes->nodeNr; ++i) {
> + xmlXPathObjectPtr xpfilename;
> + xmlXPathObjectPtr xpformat;
> +
> + /* Change the context to the current <disk> node.
> + * DV advises to reset this before each search since older versions of
> + * libxml2 might overwrite it.
> + */
> + xpathCtx->node = nodes->nodeTab[i];
> +
> + /* Filename can be in <source dev=..> or <source file=..> attribute. */
> + xpfilename = xmlXPathEvalExpression (BAD_CAST "./source/@dev", xpathCtx);
> + if (xpfilename == NULL ||
> + xpfilename->nodesetval == NULL ||
> + xpfilename->nodesetval->nodeNr == 0) {
> + xmlXPathFreeObject (xpfilename);
> + xpathCtx->node = nodes->nodeTab[i];
> + xpfilename = xmlXPathEvalExpression (BAD_CAST "./source/@file", xpathCtx);
> + if (xpfilename == NULL ||
> + xpfilename->nodesetval == NULL ||
> + xpfilename->nodesetval->nodeNr == 0) {
> + xmlXPathFreeObject (xpfilename);
> + continue; /* disk filename not found, skip this */
> + }
> + }
> +
> + assert (xpfilename->nodesetval->nodeTab[0]);
> + assert (xpfilename->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
> + xmlAttrPtr attr = (xmlAttrPtr) xpfilename->nodesetval->nodeTab[0];
> + char *filename = (char *) xmlNodeListGetString (doc, attr->children, 1);
> +
> + /* Get the disk format (may not be set). */
> + xpathCtx->node = nodes->nodeTab[i];
> + xpformat = xmlXPathEvalExpression (BAD_CAST "./driver/@type", xpathCtx);
> + char *format = NULL;
> + if (xpformat != NULL &&
> + xpformat->nodesetval &&
> + xpformat->nodesetval->nodeNr > 0) {
> + assert (xpformat->nodesetval->nodeTab[0]);
> + assert (xpformat->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
> + attr = (xmlAttrPtr) xpformat->nodesetval->nodeTab[0];
> + format = (char *) xmlNodeListGetString (doc, attr->children, 1);
> + }
> +
> + /* Add the disk, with optional format. */
> + struct guestfs_add_drive_opts_argv optargs2 = { .bitmask = 0 };
> + if (readonly) {
> + optargs2.bitmask |= GUESTFS_ADD_DRIVE_OPTS_READONLY_BITMASK;
> + optargs2.readonly = readonly;
> + }
> + if (format) {
> + optargs2.bitmask |= GUESTFS_ADD_DRIVE_OPTS_FORMAT_BITMASK;
> + optargs2.format = format;
> + }
> + if (iface) {
> + optargs2.bitmask |= GUESTFS_ADD_DRIVE_OPTS_IFACE_BITMASK;
> + optargs2.iface = iface;
> + }
> +
> + int t = guestfs__add_drive_opts (g, filename, &optargs2);
> +
> + xmlFree (filename);
> + xmlFree (format);
> + xmlXPathFreeObject (xpfilename);
> + xmlXPathFreeObject (xpformat);
> +
> + if (t == -1)
> + goto cleanup;
> +
> + nr_added++;
> + }
> +
> + if (nr_added == 0) {
> + error (g, _("libvirt domain has no disks"));
> + goto cleanup;
> + }
> +
> + /* Successful. */
> + r = nr_added;
> +
> + cleanup:
> + if (r == -1) guestfs___rollback_cmdline (g, cmdline_pos);
> + free (xml);
> + if (xpathObj) xmlXPathFreeObject (xpathObj);
> + if (xpathCtx) xmlXPathFreeContext (xpathCtx);
> + if (doc) xmlFreeDoc (doc);
> +
> + return r;
> +}
> +
> +#else /* no libvirt or libxml2 at compile time */
> +
> +#define NOT_IMPL(r) \
> + error (g, _("add-domain APIs not available since this version of libguestfs was compiled without libvirt or libxml2")); \
> + return r
> +
> +int
> +guestfs__add_domain (guestfs_h *g, const char *dom,
> + const struct guestfs_add_domain_argv *optargs)
> +{
> + NOT_IMPL(-1);
> +}
> +
> +int
> +guestfs__add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
> + const struct guestfs_add_libvirt_dom_argv *optargs)
> +{
> + NOT_IMPL(-1);
> +}
> +
> +#endif /* no libvirt or libxml2 at compile time */
Would it not be better to remove the public API symbol entirely if building
without libvirt support. That way any application code which has a dependancy
on a libvirt enabled libguestfs, will get a clear compile time error instead
of appearing to build fine and then failing at some random point while
running.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
More information about the Libguestfs
mailing list