[Libguestfs] [PATCH v2] libvirt: read disk paths from pools (RHBZ#1366049)
Richard W.M. Jones
rjones at redhat.com
Thu Sep 22 15:44:03 UTC 2016
On Thu, Sep 22, 2016 at 05:40:25PM +0200, Pino Toscano wrote:
> A disk of type 'volume' is stored as
> <source pool='pool_name' volume='volume_name'/>
> and its real location is inside the 'volume_name', as 'pool_name': in
> this case, query libvirt for the actual path of the specified volume in
> the specified pool.
>
> Adjust the code so that:
> - for_each_disk gets the virConnectPtr, needed to do operations with
> libvirt
> - when extracting the disk filename depending on the type, the code
> snippet doing it can directly set 'filename', without setting an XPath
> result variable
>
> Only file-based volumes are supported for now; more types can be added
> (with proper testing) later on.
All looks good now, ACK.
Thanks, Rich.
> src/libvirt-domain.c | 134 +++++++++++++++++++++++++----
> tests/disks/test-qemu-drive-libvirt.sh | 16 ++++
> tests/disks/test-qemu-drive-libvirt.xml.in | 37 ++++++++
> 3 files changed, 172 insertions(+), 15 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index d54814f..4d4142d 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -40,8 +40,9 @@
> #if defined(HAVE_LIBVIRT)
>
> static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom);
> -static ssize_t for_each_disk (guestfs_h *g, xmlDocPtr doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, void *data), void *data);
> +static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, void *data), void *data);
> static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char **label_rtn, char **imagelabel_rtn);
> +static char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const char *pool_nane, const char *volume_name);
>
> static void
> ignore_errors (void *ignore, virErrorPtr ignore2)
> @@ -311,7 +312,7 @@ guestfs_impl_add_libvirt_dom (guestfs_h *g, void *domvp,
> * all disks are added or none are added.
> */
> ckp = guestfs_int_checkpoint_drives (g);
> - r = for_each_disk (g, doc, add_disk, &data);
> + r = for_each_disk (g, virDomainGetConnect (dom), doc, add_disk, &data);
> if (r == -1)
> guestfs_int_rollback_drives (g, ckp);
>
> @@ -466,6 +467,7 @@ libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc,
> */
> static ssize_t
> for_each_disk (guestfs_h *g,
> + virConnectPtr conn,
> xmlDocPtr doc,
> int (*f) (guestfs_h *g,
> const char *filename, const char *format,
> @@ -509,6 +511,8 @@ for_each_disk (guestfs_h *g,
> CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpprotocol = NULL;
> CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xphost = NULL;
> CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpusername = NULL;
> + CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppool = NULL;
> + CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpvolume = NULL;
> xmlAttrPtr attr;
> int readonly;
> int t;
> @@ -628,22 +632,66 @@ for_each_disk (guestfs_h *g,
> * TODO: secrets: ./auth/secret/@type,
> * ./auth/secret/@usage || ./auth/secret/@uuid
> */
> - } else
> - continue; /* type <> "file", "block", or "network", skip it */
> + } else if (STREQ (type, "volume")) { /* type = "volume", use source/@volume */
> + CLEANUP_FREE char *pool = NULL;
> + CLEANUP_FREE char *volume = NULL;
> +
> + xpathCtx->node = nodes->nodeTab[i];
>
> - assert (xpfilename);
> - assert (xpfilename->nodesetval);
> - if (xpfilename->nodesetval->nodeNr > 0) {
> - assert (xpfilename->nodesetval->nodeTab[0]);
> - assert (xpfilename->nodesetval->nodeTab[0]->type ==
> + /* Get the source pool. Required. */
> + xppool = xmlXPathEvalExpression (BAD_CAST "./source/@pool",
> + xpathCtx);
> + if (xppool == NULL ||
> + xppool->nodesetval == NULL ||
> + xppool->nodesetval->nodeNr == 0)
> + continue;
> + assert (xppool->nodesetval->nodeTab[0]);
> + assert (xppool->nodesetval->nodeTab[0]->type ==
> + XML_ATTRIBUTE_NODE);
> + attr = (xmlAttrPtr) xppool->nodesetval->nodeTab[0];
> + pool = (char *) xmlNodeListGetString (doc, attr->children, 1);
> +
> + /* Get the source volume. Required. */
> + xpvolume = xmlXPathEvalExpression (BAD_CAST "./source/@volume",
> + xpathCtx);
> + if (xpvolume == NULL ||
> + xpvolume->nodesetval == NULL ||
> + xpvolume->nodesetval->nodeNr == 0)
> + continue;
> + assert (xpvolume->nodesetval->nodeTab[0]);
> + assert (xpvolume->nodesetval->nodeTab[0]->type ==
> XML_ATTRIBUTE_NODE);
> - attr = (xmlAttrPtr) xpfilename->nodesetval->nodeTab[0];
> - filename = (char *) xmlNodeListGetString (doc, attr->children, 1);
> - debug (g, "disk[%zu]: filename: %s", i, filename);
> + attr = (xmlAttrPtr) xpvolume->nodesetval->nodeTab[0];
> + volume = (char *) xmlNodeListGetString (doc, attr->children, 1);
> +
> + debug (g, "disk[%zu]: pool: %s; volume: %s", i, pool, volume);
> +
> + filename = filename_from_pool (g, conn, pool, volume);
> + if (filename == NULL)
> + continue; /* filename_from_pool already called error() */
> + } else
> + continue; /* type is not handled above, skip it */
> +
> + /* Allow any of the code blocks above (handling a disk type)
> + * to directly get the filename (setting 'filename'), with no need
> + * for an XPath evaluation.
> + */
> + if (filename == NULL) {
> + assert (xpfilename);
> + assert (xpfilename->nodesetval);
> + if (xpfilename->nodesetval->nodeNr > 0) {
> + assert (xpfilename->nodesetval->nodeTab[0]);
> + assert (xpfilename->nodesetval->nodeTab[0]->type ==
> + XML_ATTRIBUTE_NODE);
> + attr = (xmlAttrPtr) xpfilename->nodesetval->nodeTab[0];
> + filename = (char *) xmlNodeListGetString (doc, attr->children, 1);
> + }
> + else
> + /* For network protocols (eg. nbd), name may be omitted. */
> + filename = safe_strdup (g, "");
> }
> - else
> - /* For network protocols (eg. nbd), name may be omitted. */
> - filename = safe_strdup (g, "");
> +
> + debug (g, "disk[%zu]: filename: %s", i, filename);
>
> /* Get the disk format (may not be set). */
> xpathCtx->node = nodes->nodeTab[i];
> @@ -784,6 +832,62 @@ get_domain_xml (guestfs_h *g, virDomainPtr dom)
> return doc;
> }
>
> +static char *
> +filename_from_pool (guestfs_h *g, virConnectPtr conn,
> + const char *pool_name, const char *volume_name)
> +{
> + char *filename = NULL;
> + virErrorPtr err;
> + virStoragePoolPtr pool = NULL;
> + virStorageVolPtr vol = NULL;
> + virStorageVolInfo info;
> + int ret;
> +
> + pool = virStoragePoolLookupByName (conn, pool_name);
> + if (pool == NULL) {
> + err = virGetLastError ();
> + error (g, _("no libvirt pool called '%s': %s"),
> + pool_name, err->message);
> + goto cleanup;
> + }
> +
> + vol = virStorageVolLookupByName (pool, volume_name);
> + if (vol == NULL) {
> + err = virGetLastError ();
> + error (g, _("no volume called '%s' in the libvirt pool '%s': %s"),
> + volume_name, pool_name, err->message);
> + goto cleanup;
> + }
> +
> + ret = virStorageVolGetInfo (vol, &info);
> + if (ret < 0) {
> + err = virGetLastError ();
> + error (g, _("cannot get information of the libvirt volume '%s': %s"),
> + volume_name, err->message);
> + goto cleanup;
> + }
> +
> + debug (g, "type of libvirt volume %s: %d", volume_name, info.type);
> +
> + /* Support only file-based volumes for now. */
> + if (info.type != VIR_STORAGE_VOL_FILE)
> + goto cleanup;
> +
> + filename = virStorageVolGetPath (vol);
> + if (filename == NULL) {
> + err = virGetLastError ();
> + error (g, _("cannot get the filename of the libvirt volume '%s': %s"),
> + volume_name, err->message);
> + goto cleanup;
> + }
> +
> + cleanup:
> + if (vol) virStorageVolFree (vol);
> + if (pool) virStoragePoolFree (pool);
> +
> + return filename;
> +}
> +
> #else /* no libvirt at compile time */
>
> #define NOT_IMPL(r) \
> diff --git a/tests/disks/test-qemu-drive-libvirt.sh b/tests/disks/test-qemu-drive-libvirt.sh
> index 215a99e..b2656ba 100755
> --- a/tests/disks/test-qemu-drive-libvirt.sh
> +++ b/tests/disks/test-qemu-drive-libvirt.sh
> @@ -47,6 +47,12 @@ export LIBGUESTFS_BACKEND=direct
> export LIBGUESTFS_HV="${abs_srcdir}/debug-qemu.sh"
> export DEBUG_QEMU_FILE="${abs_builddir}/test-qemu-drive-libvirt.out"
>
> +# Setup the fake pool.
> +pool_dir=tmp
> +rm -rf "$pool_dir"
> +mkdir "$pool_dir"
> +touch "$pool_dir/in-pool"
> +
> function check_output ()
> {
> if [ ! -f "$DEBUG_QEMU_FILE" ]; then
> @@ -104,8 +110,18 @@ check_output
> grep -sq -- '-drive file=sheepdog:volume,' "$DEBUG_QEMU_FILE" || fail
> rm "$DEBUG_QEMU_FILE"
>
> +# Local, stored in a pool.
> +
> +$guestfish -d pool1 run ||:
> +check_output
> +grep -sq -- "-drive file=$abs_builddir/tmp/in-pool" "$DEBUG_QEMU_FILE" || fail
> +rm "$DEBUG_QEMU_FILE"
> +
> # To do:
>
> # HTTP - curl not yet supported by libvirt
>
> # SSH.
> +
> +# Clean up.
> +rm -r "$pool_dir"
> diff --git a/tests/disks/test-qemu-drive-libvirt.xml.in b/tests/disks/test-qemu-drive-libvirt.xml.in
> index e8e6252..f0b7fe0 100644
> --- a/tests/disks/test-qemu-drive-libvirt.xml.in
> +++ b/tests/disks/test-qemu-drive-libvirt.xml.in
> @@ -132,4 +132,41 @@
> </devices>
> </domain>
>
> + <domain type='test' xmlns:test='http://libvirt.org/schemas/domain/test/1.0'>
> + <test:runstate>5</test:runstate> <!-- 5 == VIR_DOMAIN_SHUTOFF -->
> + <name>pool1</name>
> + <memory>1048576</memory>
> + <os>
> + <type>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <devices>
> + <disk type='volume' device='disk'>
> + <driver name='qemu'/>
> + <source pool='pool1' volume='in-pool'/>
> + <target dev='vda' bus='virtio'/>
> + </disk>
> + </devices>
> + </domain>
> +
> + <pool type='dir'>
> + <name>pool1</name>
> + <uuid>12345678-1234-1234-1234-1234567890ab</uuid>
> + <target>
> + <path>@abs_builddir@/tmp</path>
> + </target>
> +
> + <volume type='file'>
> + <name>in-pool</name>
> + <capacity unit='bytes'>1048576</capacity>
> + <key>@abs_builddir@/tmp/in-pool</key>
> + <source>
> + </source>
> + <target>
> + <path>@abs_builddir@/tmp/in-pool</path>
> + </target>
> + </volume>
> +
> + </pool>
> +
> </node>
> --
> 2.7.4
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
More information about the Libguestfs
mailing list