[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: Support storage copy on write volumes



On Thu, Jan 22, 2009 at 07:15:19PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange redhat com> wrote:
> > This is a follow up on Miloslav's proposal to add copy on write
> > support to the storage APIs, changing the XML to that described
> > here:
> >
> >   http://www.redhat.com/archives/libvir-list/2009-January/msg00231.html
> >
> > In addition to the original QCOW/VMDK support, I have done an impl which
> > can extract the backing store for LVM volumes
> 

> > -    vol->target.perms.mode = sb.st_mode;
> > -    vol->target.perms.uid = sb.st_uid;
> > -    vol->target.perms.gid = sb.st_gid;
> > +    target->perms.mode = sb.st_mode & (0x200-1);
> 
> How about S_IRWXUGO:
> 
>        target->perms.mode = sb.st_mode & S_IRWXUGO;

Included that change.

> 
> > +    target->perms.uid = sb.st_uid;
> > +    target->perms.gid = sb.st_gid;
> ...
> > diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
> ...
> > +static int
> > +cowGetBackingStore(virConnectPtr conn,
> > +                   char **res,
> > +                   const unsigned char *buf,
> > +                   size_t buf_size)
> > +{
> > +    size_t len;
> > +
> > +    *res = NULL;
> > +    if (buf_size < 4+4+1024)
> > +        return BACKING_STORE_INVALID;
> > +    if (buf[4+4] == '\0') /* cow_header_v2.backing_file[0] */
> > +        return BACKING_STORE_OK;
> > +
> > +    len = 1024;
> > +    if (VIR_ALLOC_N(*res, len + 1) < 0) {
> > +        virStorageReportError(conn, VIR_ERR_NO_MEMORY, _("backing store path"));
> > +        return BACKING_STORE_ERROR;
> > +    }
> > +    memcpy(*res, buf + 4+4, len); /* cow_header_v2.backing_file */
> > +    (*res)[len] = '\0';
> > +    if (VIR_REALLOC_N(*res, strlen(*res) + 1) < 0) {
> 
> Is this just-copied 1024-byte block of data guaranteed
> not to contain any NUL bytes?  Or maybe you just want that
> NUL-terminated string?

I changed that as suggested in your later mail

http://www.redhat.com/archives/libvir-list/2009-January/msg00621.html

> >
> > -        imgargv[0] = QEMU_IMG;
> > -        imgargv[1] = "create";
> > -        imgargv[2] = "-f";
> > -        imgargv[3] = type;
> > -        imgargv[4] = vol->target.path;
> > -        imgargv[5] = size;
> > -        imgargv[6] = NULL;
> > +        i = 0;
> > +        imgargv[i++] = QEMU_IMG;
> > +        imgargv[i++] = "create";
> > +        imgargv[i++] = "-f";
> > +        imgargv[i++] = type;
> > +        if (vol->backingStore.path != NULL) {
> > +            imgargv[i++] = "-b";
> > +            imgargv[i++] = vol->backingStore.path;
> > +        }
> > +        imgargv[i++] = vol->target.path;
> > +        imgargv[i++] = size;
> > +        imgargv[i++] = NULL;
> 
> Add an assertion like
> 
>    assert (i < ARRAY_CARDINALITY (imgargv));
> 
> Otherwise, it's far too easy to forget to update the "9"
> above when adding a new option here.

I changed this so we just declare the array and its members
in one go, avoiding need to explicit give array sizing.

> > diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
> > --- a/src/storage_backend_logical.c
> > +++ b/src/storage_backend_logical.c
> > @@ -116,8 +116,22 @@ virStorageBackendLogicalMakeVol(virConne
> >          strcat(vol->target.path, vol->name);
> >      }
> >
> > +    if (groups[1] && !STREQ(groups[1], "")) {
> > +        if (VIR_ALLOC_N(vol->backingStore.path, strlen(pool->def->target.path) +
> > +                        1 + strlen(groups[1]) + 1) < 0) {
> > +            virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("volume"));
> > +            return -1;
> > +        }
> > +        strcpy(vol->backingStore.path, pool->def->target.path);
> > +        strcat(vol->backingStore.path, "/");
> > +        strcat(vol->backingStore.path, groups[1]);
> 
> How about using virAsprintf in place of VIR_ALLOC_N + strcpy + strcat?

Did that, and likewise the similar allocation  a little above this chunk

 docs/formatstorage.html       |   45 +++++
 docs/formatstorage.html.in    |   52 ++++++
 src/libvirt_private.syms      |    1 
 src/storage_backend.c         |  140 ++++++++++-------
 src/storage_backend.h         |   13 +
 src/storage_backend_fs.c      |  335 ++++++++++++++++++++++++++++++++++++------
 src/storage_backend_iscsi.c   |    6 
 src/storage_backend_logical.c |   68 +++++---
 src/storage_conf.c            |  101 +++++++++---
 src/storage_conf.h            |    1 
 src/virterror.c               |    2 
 11 files changed, 607 insertions(+), 157 deletions(-)


Daniel

diff --git a/docs/formatstorage.html b/docs/formatstorage.html
--- a/docs/formatstorage.html
+++ b/docs/formatstorage.html
@@ -131,6 +131,8 @@
                 <a href="#StorageVolFirst">General metadata</a>
               </li><li>
                 <a href="#StorageVolTarget">Target elements</a>
+              </li><li>
+                <a href="#StorageVolBacking">Backing store elements</a>
               </li></ul>
           </li><li>
             <a href="#examples">Example configuration</a>
@@ -328,14 +330,14 @@
         ...
 	&lt;target&gt;
           &lt;path&gt;/var/lib/virt/images/sparse.img&lt;/path&gt;
+          &lt;format&gt;qcow2&lt;/format&gt;
           &lt;permissions&gt;
             &lt;owner&gt;0744&lt;/owner&gt;
             &lt;group&gt;0744&lt;/group&gt;
             &lt;mode&gt;0744&lt;/mode&gt;
             &lt;label&gt;virt_image_t&lt;/label&gt;
           &lt;/permissions&gt;
-	&lt;/target&gt;
-      &lt;/volume&gt;</pre>
+	&lt;/target&gt;</pre>
         <dl><dt><code>path</code></dt><dd>Provides the location at which the volume can be accessed on
 	the local filesystem, as an absolute path. This is a readonly
 	attribute, so shouldn't be specified when creating a volume.
@@ -356,6 +358,45 @@
 	contains the MAC (eg SELinux) label string.
 	<span class="since">Since 0.4.1</span>
       </dd></dl>
+        <h3>
+          <a name="StorageVolBacking" id="StorageVolBacking">Backing store elements</a>
+        </h3>
+        <p>
+      A single <code>backingStore</code> element is contained within the top level
+      <code>volume</code> element. This tag is used to describe the optional copy
+      on write, backing store for the storage volume. It can contain the following
+      child elements:
+    </p>
+        <pre>
+        ...
+        &lt;backingStore&gt;
+          &lt;path&gt;/var/lib/virt/images/master.img&lt;/path&gt;
+          &lt;format&gt;raw&lt;/format&gt;
+          &lt;permissions&gt;
+            &lt;owner&gt;0744&lt;/owner&gt;
+            &lt;group&gt;0744&lt;/group&gt;
+            &lt;mode&gt;0744&lt;/mode&gt;
+            &lt;label&gt;virt_image_t&lt;/label&gt;
+          &lt;/permissions&gt;
+        &lt;/backingStore&gt;
+      &lt;/volume&gt;</pre>
+        <dl><dt><code>path</code></dt><dd>Provides the location at which the backing store can be accessed on
+	the local filesystem, as an absolute path. If omitted, there is no
+        backing store for this volume.
+	<span class="since">Since 0.6.0</span></dd><dt><code>format</code></dt><dd>Provides information about the pool specific backing store format.
+	For disk pools it will provide the partition type. For filesystem
+	or directory pools it will provide the file format type, eg cow,
+	qcow, vmdk, raw. Consult the pool-specific docs for the list of valid
+        values. Most file formats require a backing store of the same format,
+        however, the qcow2 format allows a different backing store format.
+        <span class="since">Since 0.6.0</span></dd><dt><code>permissions</code></dt><dd>Provides information about the permissions of the backing file.
+        It contains 4 child elements. The
+	<code>mode</code> element contains the octal permission set. The
+	<code>owner</code> element contains the numeric user ID. The <code>group</code>
+	element contains the numeric group ID. The <code>label</code> element
+	contains the MAC (eg SELinux) label string.
+	<span class="since">Since 0.6.0</span>
+      </dd></dl>
         <h2>
           <a name="examples" id="examples">Example configuration</a>
         </h2>
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -234,14 +234,14 @@
         ...
 	&lt;target&gt;
           &lt;path&gt;/var/lib/virt/images/sparse.img&lt;/path&gt;
+          &lt;format&gt;qcow2&lt;/format&gt;
           &lt;permissions&gt;
             &lt;owner&gt;0744&lt;/owner&gt;
             &lt;group&gt;0744&lt;/group&gt;
             &lt;mode&gt;0744&lt;/mode&gt;
             &lt;label&gt;virt_image_t&lt;/label&gt;
           &lt;/permissions&gt;
-	&lt;/target&gt;
-      &lt;/volume&gt;</pre>
+	&lt;/target&gt;</pre>
 
     <dl>
       <dt><code>path</code></dt>
@@ -271,6 +271,54 @@
       </dd>
     </dl>
 
+    <h3><a name="StorageVolBacking">Backing store elements</a></h3>
+
+    <p>
+      A single <code>backingStore</code> element is contained within the top level
+      <code>volume</code> element. This tag is used to describe the optional copy
+      on write, backing store for the storage volume. It can contain the following
+      child elements:
+    </p>
+
+    <pre>
+        ...
+        &lt;backingStore&gt;
+          &lt;path&gt;/var/lib/virt/images/master.img&lt;/path&gt;
+          &lt;format&gt;raw&lt;/format&gt;
+          &lt;permissions&gt;
+            &lt;owner&gt;0744&lt;/owner&gt;
+            &lt;group&gt;0744&lt;/group&gt;
+            &lt;mode&gt;0744&lt;/mode&gt;
+            &lt;label&gt;virt_image_t&lt;/label&gt;
+          &lt;/permissions&gt;
+        &lt;/backingStore&gt;
+      &lt;/volume&gt;</pre>
+
+    <dl>
+      <dt><code>path</code></dt>
+      <dd>Provides the location at which the backing store can be accessed on
+	the local filesystem, as an absolute path. If omitted, there is no
+        backing store for this volume.
+	<span class="since">Since 0.6.0</span></dd>
+      <dt><code>format</code></dt>
+      <dd>Provides information about the pool specific backing store format.
+	For disk pools it will provide the partition type. For filesystem
+	or directory pools it will provide the file format type, eg cow,
+	qcow, vmdk, raw. Consult the pool-specific docs for the list of valid
+        values. Most file formats require a backing store of the same format,
+        however, the qcow2 format allows a different backing store format.
+        <span class="since">Since 0.6.0</span></dd>
+      <dt><code>permissions</code></dt>
+      <dd>Provides information about the permissions of the backing file.
+        It contains 4 child elements. The
+	<code>mode</code> element contains the octal permission set. The
+	<code>owner</code> element contains the numeric user ID. The <code>group</code>
+	element contains the numeric group ID. The <code>label</code> element
+	contains the MAC (eg SELinux) label string.
+	<span class="since">Since 0.6.0</span>
+      </dd>
+    </dl>
+
     <h2><a name="examples">Example configuration</a></h2>
 
     <p>
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -259,6 +259,7 @@ virStoragePoolFormatDiskTypeToString;
 virStoragePoolFormatFileSystemTypeToString;
 virStoragePoolFormatFileSystemNetTypeToString;
 virStorageVolFormatFileSystemTypeToString;
+virStorageVolFormatFileSystemTypeFromString;
 virStoragePoolTypeFromString;
 virStoragePoolObjLock;
 virStoragePoolObjUnlock;
diff --git a/src/storage_backend.c b/src/storage_backend.c
--- a/src/storage_backend.c
+++ b/src/storage_backend.c
@@ -99,27 +99,51 @@ virStorageBackendForType(int type) {
 
 
 int
+virStorageBackendUpdateVolTargetInfo(virConnectPtr conn,
+                                     virStorageVolTargetPtr target,
+                                     unsigned long long *allocation,
+                                     unsigned long long *capacity)
+{
+    int ret, fd;
+
+    if ((fd = open(target->path, O_RDONLY)) < 0) {
+        virReportSystemError(conn, errno,
+                             _("cannot open volume '%s'"),
+                             target->path);
+        return -1;
+    }
+
+    ret = virStorageBackendUpdateVolTargetInfoFD(conn,
+                                                 target,
+                                                 fd,
+                                                 allocation,
+                                                 capacity);
+
+    close(fd);
+
+    return ret;
+}
+
+int
 virStorageBackendUpdateVolInfo(virConnectPtr conn,
                                virStorageVolDefPtr vol,
                                int withCapacity)
 {
-    int ret, fd;
+    int ret;
 
-    if ((fd = open(vol->target.path, O_RDONLY)) < 0) {
-        virReportSystemError(conn, errno,
-                             _("cannot open volume '%s'"),
-                             vol->target.path);
-        return -1;
-    }
+    if ((ret = virStorageBackendUpdateVolTargetInfo(conn,
+                                                    &vol->target,
+                                                    &vol->allocation,
+                                                    withCapacity ? &vol->capacity : NULL)) < 0)
+        return ret;
 
-    ret = virStorageBackendUpdateVolInfoFD(conn,
-                                           vol,
-                                           fd,
-                                           withCapacity);
+    if (vol->backingStore.path &&
+        (ret = virStorageBackendUpdateVolTargetInfo(conn,
+                                                    &vol->backingStore,
+                                                    NULL, NULL)) < 0)
+        return ret;
 
-    close(fd);
-
-    return ret;
+    return 0;
 }
 
 struct diskType {
@@ -154,10 +178,11 @@ static struct diskType const disk_types[
 };
 
 int
-virStorageBackendUpdateVolInfoFD(virConnectPtr conn,
-                                 virStorageVolDefPtr vol,
-                                 int fd,
-                                 int withCapacity)
+virStorageBackendUpdateVolTargetInfoFD(virConnectPtr conn,
+                                       virStorageVolTargetPtr target,
+                                       int fd,
+                                       unsigned long long *allocation,
+                                       unsigned long long *capacity)
 {
     struct stat sb;
 #if HAVE_SELINUX
@@ -167,7 +192,7 @@ virStorageBackendUpdateVolInfoFD(virConn
     if (fstat(fd, &sb) < 0) {
         virReportSystemError(conn, errno,
                              _("cannot stat file '%s'"),
-                             vol->target.path);
+                             target->path);
         return -1;
     }
 
@@ -176,38 +201,41 @@ virStorageBackendUpdateVolInfoFD(virConn
         !S_ISBLK(sb.st_mode))
         return -2;
 
-    if (S_ISREG(sb.st_mode)) {
+    if (allocation) {
+        if (S_ISREG(sb.st_mode)) {
 #ifndef __MINGW32__
-        vol->allocation = (unsigned long long)sb.st_blocks *
-            (unsigned long long)sb.st_blksize;
+            *allocation = (unsigned long long)sb.st_blocks *
+                (unsigned long long)sb.st_blksize;
 #else
-        vol->allocation = sb.st_size;
+            *allocation = sb.st_size;
 #endif
-        /* Regular files may be sparse, so logical size (capacity) is not same
-         * as actual allocation above
-         */
-        if (withCapacity)
-            vol->capacity = sb.st_size;
-    } else {
-        off_t end;
-        /* XXX this is POSIX compliant, but doesn't work for for CHAR files,
-         * only BLOCK. There is a Linux specific ioctl() for getting
-         * size of both CHAR / BLOCK devices we should check for in
-         * configure
-         */
-        end = lseek(fd, 0, SEEK_END);
-        if (end == (off_t)-1) {
-            virReportSystemError(conn, errno,
-                                 _("cannot seek to end of file '%s'"),
-                                 vol->target.path);
-            return -1;
+            /* Regular files may be sparse, so logical size (capacity) is not same
+             * as actual allocation above
+             */
+            if (capacity)
+                *capacity = sb.st_size;
+        } else {
+            off_t end;
+            /* XXX this is POSIX compliant, but doesn't work for for CHAR files,
+             * only BLOCK. There is a Linux specific ioctl() for getting
+             * size of both CHAR / BLOCK devices we should check for in
+             * configure
+             */
+            end = lseek(fd, 0, SEEK_END);
+            if (end == (off_t)-1) {
+                virReportSystemError(conn, errno,
+                                     _("cannot seek to end of file '%s'"),
+                                     target->path);
+                return -1;
+            }
+            *allocation = end;
+            if (capacity)
+                *capacity = end;
         }
-        vol->allocation = end;
-        if (withCapacity) vol->capacity = end;
     }
 
     /* make sure to set the target format "unknown" to begin with */
-    vol->target.format = VIR_STORAGE_POOL_DISK_UNKNOWN;
+    target->format = VIR_STORAGE_POOL_DISK_UNKNOWN;
 
     if (S_ISBLK(sb.st_mode)) {
         off_t start;
@@ -219,14 +247,14 @@ virStorageBackendUpdateVolInfoFD(virConn
         if (start < 0) {
             virReportSystemError(conn, errno,
                                  _("cannot seek to beginning of file '%s'"),
-                                 vol->target.path);
+                                 target->path);
             return -1;
         }
         bytes = saferead(fd, buffer, sizeof(buffer));
         if (bytes < 0) {
             virReportSystemError(conn, errno,
                                  _("cannot read beginning of file '%s'"),
-                                 vol->target.path);
+                                 target->path);
             return -1;
         }
 
@@ -235,38 +263,38 @@ virStorageBackendUpdateVolInfoFD(virConn
                 continue;
             if (memcmp(buffer+disk_types[i].offset, &disk_types[i].magic,
                 disk_types[i].length) == 0) {
-                vol->target.format = disk_types[i].part_table_type;
+                target->format = disk_types[i].part_table_type;
                 break;
             }
         }
     }
 
-    vol->target.perms.mode = sb.st_mode;
-    vol->target.perms.uid = sb.st_uid;
-    vol->target.perms.gid = sb.st_gid;
+    target->perms.mode = sb.st_mode & S_IRWXUGO;
+    target->perms.uid = sb.st_uid;
+    target->perms.gid = sb.st_gid;
 
-    VIR_FREE(vol->target.perms.label);
+    VIR_FREE(target->perms.label);
 
 #if HAVE_SELINUX
     if (fgetfilecon(fd, &filecon) == -1) {
         if (errno != ENODATA && errno != ENOTSUP) {
             virReportSystemError(conn, errno,
                                  _("cannot get file context of '%s'"),
-                                 vol->target.path);
+                                 target->path);
             return -1;
         } else {
-            vol->target.perms.label = NULL;
+            target->perms.label = NULL;
         }
     } else {
-        vol->target.perms.label = strdup(filecon);
-        if (vol->target.perms.label == NULL) {
+        target->perms.label = strdup(filecon);
+        if (target->perms.label == NULL) {
             virReportOOMError(conn);
             return -1;
         }
         freecon(filecon);
     }
 #else
-    vol->target.perms.label = NULL;
+    target->perms.label = NULL;
 #endif
 
     return 0;
diff --git a/src/storage_backend.h b/src/storage_backend.h
--- a/src/storage_backend.h
+++ b/src/storage_backend.h
@@ -64,10 +64,15 @@ int virStorageBackendUpdateVolInfo(virCo
                                    virStorageVolDefPtr vol,
                                    int withCapacity);
 
-int virStorageBackendUpdateVolInfoFD(virConnectPtr conn,
-                                     virStorageVolDefPtr vol,
-                                     int fd,
-                                     int withCapacity);
+int virStorageBackendUpdateVolTargetInfo(virConnectPtr conn,
+                                         virStorageVolTargetPtr target,
+                                         unsigned long long *allocation,
+                                         unsigned long long *capacity);
+int virStorageBackendUpdateVolTargetInfoFD(virConnectPtr conn,
+                                           virStorageVolTargetPtr target,
+                                           int fd,
+                                           unsigned long long *allocation,
+                                           unsigned long long *capacity);
 
 void virStorageBackendWaitForDevices(virConnectPtr conn);
 
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -49,6 +49,19 @@ enum lv_endian {
     LV_BIG_ENDIAN         /* 4321 */
 };
 
+enum {
+    BACKING_STORE_OK,
+    BACKING_STORE_INVALID,
+    BACKING_STORE_ERROR,
+};
+
+static int cowGetBackingStore(virConnectPtr, char **,
+                              const unsigned char *, size_t);
+static int qcowXGetBackingStore(virConnectPtr, char **,
+                                const unsigned char *, size_t);
+static int vmdk4GetBackingStore(virConnectPtr, char **,
+                                const unsigned char *, size_t);
+
 /* Either 'magic' or 'extension' *must* be provided */
 struct FileTypeInfo {
     int type;           /* One of the constants above */
@@ -65,85 +78,228 @@ struct FileTypeInfo {
                            * -1 to use st_size as capacity */
     int sizeBytes;        /* Number of bytes for size field */
     int sizeMultiplier;   /* A scaling factor if size is not in bytes */
+                          /* Store a COW base image path (possibly relative),
+                           * or NULL if there is no COW base image, to RES;
+                           * return BACKING_STORE_* */
+    int (*getBackingStore)(virConnectPtr conn, char **res,
+                           const unsigned char *buf, size_t buf_size);
 };
 const struct FileTypeInfo const fileTypeInfo[] = {
     /* Bochs */
     /* XXX Untested
     { VIR_STORAGE_VOL_BOCHS, "Bochs Virtual HD Image", NULL,
       LV_LITTLE_ENDIAN, 64, 0x20000,
-      32+16+16+4+4+4+4+4, 8, 1 },*/
+      32+16+16+4+4+4+4+4, 8, 1, NULL },*/
     /* CLoop */
     /* XXX Untested
     { VIR_STORAGE_VOL_CLOOP, "#!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n", NULL,
       LV_LITTLE_ENDIAN, -1, 0,
-      -1, 0, 0 }, */
+      -1, 0, 0, NULL }, */
     /* Cow */
     { VIR_STORAGE_VOL_FILE_COW, "OOOM", NULL,
       LV_BIG_ENDIAN, 4, 2,
-      4+4+1024+4, 8, 1 },
+      4+4+1024+4, 8, 1, cowGetBackingStore },
     /* DMG */
     /* XXX QEMU says there's no magic for dmg, but we should check... */
     { VIR_STORAGE_VOL_FILE_DMG, NULL, ".dmg",
       0, -1, 0,
-      -1, 0, 0 },
+      -1, 0, 0, NULL },
     /* XXX there's probably some magic for iso we can validate too... */
     { VIR_STORAGE_VOL_FILE_ISO, NULL, ".iso",
       0, -1, 0,
-      -1, 0, 0 },
+      -1, 0, 0, NULL },
     /* Parallels */
     /* XXX Untested
     { VIR_STORAGE_VOL_FILE_PARALLELS, "WithoutFreeSpace", NULL,
       LV_LITTLE_ENDIAN, 16, 2,
-      16+4+4+4+4, 4, 512 },
+      16+4+4+4+4, 4, 512, NULL },
     */
     /* QCow */
     { VIR_STORAGE_VOL_FILE_QCOW, "QFI", NULL,
       LV_BIG_ENDIAN, 4, 1,
-      4+4+8+4+4, 8, 1 },
+      4+4+8+4+4, 8, 1, qcowXGetBackingStore },
     /* QCow 2 */
     { VIR_STORAGE_VOL_FILE_QCOW2, "QFI", NULL,
       LV_BIG_ENDIAN, 4, 2,
-      4+4+8+4+4, 8, 1 },
+      4+4+8+4+4, 8, 1, qcowXGetBackingStore },
     /* VMDK 3 */
     /* XXX Untested
     { VIR_STORAGE_VOL_FILE_VMDK, "COWD", NULL,
       LV_LITTLE_ENDIAN, 4, 1,
-      4+4+4, 4, 512 },
+      4+4+4, 4, 512, NULL },
     */
     /* VMDK 4 */
     { VIR_STORAGE_VOL_FILE_VMDK, "KDMV", NULL,
       LV_LITTLE_ENDIAN, 4, 1,
-      4+4+4, 8, 512 },
+      4+4+4, 8, 512, vmdk4GetBackingStore },
     /* Connectix / VirtualPC */
     /* XXX Untested
     { VIR_STORAGE_VOL_FILE_VPC, "conectix", NULL,
       LV_BIG_ENDIAN, -1, 0,
-      -1, 0, 0},
+      -1, 0, 0, NULL},
     */
 };
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
+static int
+cowGetBackingStore(virConnectPtr conn,
+                   char **res,
+                   const unsigned char *buf,
+                   size_t buf_size)
+{
+#define COW_FILENAME_MAXLEN 1024
+    *res = NULL;
+    if (buf_size < 4+4+ COW_FILENAME_MAXLEN)
+        return BACKING_STORE_INVALID;
+    if (buf[4+4] == '\0') /* cow_header_v2.backing_file[0] */
+        return BACKING_STORE_OK;
+
+    *res = strndup ((const char*)buf + 4+4, COW_FILENAME_MAXLEN);
+    if (*res == NULL) {
+        virReportOOMError(conn);
+        return BACKING_STORE_ERROR;
+    }
+    return BACKING_STORE_OK;
+}
+
+static int
+qcowXGetBackingStore(virConnectPtr conn,
+                     char **res,
+                     const unsigned char *buf,
+                     size_t buf_size)
+{
+    unsigned long long offset;
+    unsigned long size;
+
+    *res = NULL;
+    if (buf_size < 4+4+8+4)
+        return BACKING_STORE_INVALID;
+    offset = (((unsigned long long)buf[4+4] << 56)
+              | ((unsigned long long)buf[4+4+1] << 48)
+              | ((unsigned long long)buf[4+4+2] << 40)
+              | ((unsigned long long)buf[4+4+3] << 32)
+              | ((unsigned long long)buf[4+4+4] << 24)
+              | ((unsigned long long)buf[4+4+5] << 16)
+              | ((unsigned long long)buf[4+4+6] << 8)
+              | buf[4+4+7]); /* QCowHeader.backing_file_offset */
+    if (offset > buf_size)
+        return BACKING_STORE_INVALID;
+    size = ((buf[4+4+8] << 24)
+            | (buf[4+4+8+1] << 16)
+            | (buf[4+4+8+2] << 8)
+            | buf[4+4+8+3]); /* QCowHeader.backing_file_size */
+    if (size == 0)
+        return BACKING_STORE_OK;
+    if (offset + size > buf_size || offset + size < offset)
+        return BACKING_STORE_INVALID;
+    if (size + 1 == 0)
+        return BACKING_STORE_INVALID;
+    if (VIR_ALLOC_N(*res, size + 1) < 0) {
+        virStorageReportError(conn, VIR_ERR_NO_MEMORY, _("backing store path"));
+        return BACKING_STORE_ERROR;
+    }
+    memcpy(*res, buf + offset, size);
+    (*res)[size] = '\0';
+    return BACKING_STORE_OK;
+}
+
+
+static int
+vmdk4GetBackingStore(virConnectPtr conn,
+                     char **res,
+                     const unsigned char *buf,
+                     size_t buf_size)
+{
+    static const char prefix[] = "parentFileNameHint=\"";
+
+    char desc[20*512 + 1], *start, *end;
+    size_t len;
+
+    *res = NULL;
+
+    if (buf_size <= 0x200)
+        return BACKING_STORE_INVALID;
+    len = buf_size - 0x200;
+    if (len > sizeof(desc) - 1)
+        len = sizeof(desc) - 1;
+    memcpy(desc, buf + 0x200, len);
+    desc[len] = '\0';
+    start = strstr(desc, prefix);
+    if (start == NULL)
+        return BACKING_STORE_OK;
+    start += strlen(prefix);
+    end = strchr(start, '"');
+    if (end == NULL)
+        return BACKING_STORE_INVALID;
+    if (end == start)
+        return BACKING_STORE_OK;
+    *end = '\0';
+    *res = strdup(start);
+    if (*res == NULL) {
+        virStorageReportError(conn, VIR_ERR_NO_MEMORY, _("backing store path"));
+        return BACKING_STORE_ERROR;
+    }
+    return BACKING_STORE_OK;
+}
+
+/**
+ * Return an absolute path corresponding to PATH, which is absolute or relative
+ * to the directory containing BASE_FILE, or NULL on error
+ */
+static char *absolutePathFromBaseFile(const char *base_file, const char *path)
+{
+    size_t base_size, path_size;
+    char *res, *p;
+
+    if (*path == '/')
+        return strdup(path);
+
+    base_size = strlen(base_file) + 1;
+    path_size = strlen(path) + 1;
+    if (VIR_ALLOC_N(res, base_size - 1 + path_size) < 0)
+        return NULL;
+    memcpy(res, base_file, base_size);
+    p = strrchr(res, '/');
+    if (p != NULL)
+        p++;
+    else
+        p = res;
+    memcpy(p, path, path_size);
+    if (VIR_REALLOC_N(res, (p + path_size) - res) < 0) {
+        /* Ignore failure */
+    }
+    return res;
+}
+
 
 
 /**
  * Probe the header of a file to determine what type of disk image
  * it is, and info about its capacity if available.
  */
-static int virStorageBackendProbeFile(virConnectPtr conn,
-                                      virStorageVolDefPtr def) {
+static int virStorageBackendProbeTarget(virConnectPtr conn,
+                                        virStorageVolTargetPtr target,
+                                        char **backingStore,
+                                        unsigned long long *allocation,
+                                        unsigned long long *capacity) {
     int fd;
-    unsigned char head[4096];
+    unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */
     int len, i, ret;
 
-    if ((fd = open(def->target.path, O_RDONLY)) < 0) {
+    if (backingStore)
+        *backingStore = NULL;
+
+    if ((fd = open(target->path, O_RDONLY)) < 0) {
         virReportSystemError(conn, errno,
                              _("cannot open volume '%s'"),
-                             def->target.path);
+                             target->path);
         return -1;
     }
 
-    if ((ret = virStorageBackendUpdateVolInfoFD(conn, def, fd, 1)) < 0) {
+    if ((ret = virStorageBackendUpdateVolTargetInfoFD(conn, target, fd,
+                                                      allocation,
+                                                      capacity)) < 0) {
         close(fd);
         return ret; /* Take care to propagate ret, it is not always -1 */
     }
@@ -151,7 +307,7 @@ static int virStorageBackendProbeFile(vi
     if ((len = read(fd, head, sizeof(head))) < 0) {
         virReportSystemError(conn, errno,
                              _("cannot read header '%s'"),
-                             def->target.path);
+                             target->path);
         close(fd);
         return -1;
     }
@@ -191,9 +347,9 @@ static int virStorageBackendProbeFile(vi
         }
 
         /* Optionally extract capacity from file */
-        if (fileTypeInfo[i].sizeOffset != -1) {
+        if (fileTypeInfo[i].sizeOffset != -1 && capacity) {
             if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) {
-                def->capacity =
+                *capacity =
                     ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7] << 56) |
                     ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 48) |
                     ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 40) |
@@ -203,7 +359,7 @@ static int virStorageBackendProbeFile(vi
                     ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 8) |
                     ((unsigned long long)head[fileTypeInfo[i].sizeOffset]);
             } else {
-                def->capacity =
+                *capacity =
                     ((unsigned long long)head[fileTypeInfo[i].sizeOffset] << 56) |
                     ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 48) |
                     ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 40) |
@@ -214,13 +370,37 @@ static int virStorageBackendProbeFile(vi
                     ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]);
             }
             /* Avoid unlikely, but theoretically possible overflow */
-            if (def->capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier))
+            if (*capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier))
                 continue;
-            def->capacity *= fileTypeInfo[i].sizeMultiplier;
+            *capacity *= fileTypeInfo[i].sizeMultiplier;
         }
 
         /* Validation passed, we know the file format now */
-        def->target.format = fileTypeInfo[i].type;
+        target->format = fileTypeInfo[i].type;
+        if (fileTypeInfo[i].getBackingStore != NULL && backingStore) {
+            char *base;
+
+            switch (fileTypeInfo[i].getBackingStore(conn, &base, head, len)) {
+            case BACKING_STORE_OK:
+                break;
+
+            case BACKING_STORE_INVALID:
+                continue;
+
+            case BACKING_STORE_ERROR:
+                return -1;
+            }
+            if (base != NULL) {
+                *backingStore
+                    = absolutePathFromBaseFile(target->path, base);
+                VIR_FREE(base);
+                if (*backingStore == NULL) {
+                    virStorageReportError(conn, VIR_ERR_NO_MEMORY,
+                                          _("backing store path"));
+                    return -1;
+                }
+            }
+        }
         return 0;
     }
 
@@ -229,15 +409,15 @@ static int virStorageBackendProbeFile(vi
         if (fileTypeInfo[i].extension == NULL)
             continue;
 
-        if (!virFileHasSuffix(def->target.path, fileTypeInfo[i].extension))
+        if (!virFileHasSuffix(target->path, fileTypeInfo[i].extension))
             continue;
 
-        def->target.format = fileTypeInfo[i].type;
+        target->format = fileTypeInfo[i].type;
         return 0;
     }
 
     /* All fails, so call it a raw file */
-    def->target.format = VIR_STORAGE_VOL_FILE_RAW;
+    target->format = VIR_STORAGE_VOL_FILE_RAW;
     return 0;
 }
 
@@ -636,6 +816,7 @@ virStorageBackendFileSystemRefresh(virCo
 
     while ((ent = readdir(dir)) != NULL) {
         int ret;
+        char *backingStore;
 
         if (VIR_ALLOC(vol) < 0)
             goto no_memory;
@@ -655,7 +836,11 @@ virStorageBackendFileSystemRefresh(virCo
         if ((vol->key = strdup(vol->target.path)) == NULL)
             goto no_memory;
 
-        if ((ret = virStorageBackendProbeFile(conn, vol) < 0)) {
+        if ((ret = virStorageBackendProbeTarget(conn,
+                                                &vol->target,
+                                                &backingStore,
+                                                &vol->allocation,
+                                                &vol->capacity) < 0)) {
             if (ret == -1)
                 goto no_memory;
             else {
@@ -667,6 +852,48 @@ virStorageBackendFileSystemRefresh(virCo
             }
         }
 
+        if (backingStore != NULL) {
+            if (vol->target.format == VIR_STORAGE_VOL_FILE_QCOW2 &&
+                STRPREFIX("fmt:", backingStore)) {
+                char *fmtstr = backingStore + 4;
+                char *path = strchr(fmtstr, ':');
+                if (!path) {
+                    VIR_FREE(backingStore);
+                } else {
+                    *path = '\0';
+                    if ((vol->backingStore.format =
+                         virStorageVolFormatFileSystemTypeFromString(fmtstr)) < 0) {
+                        VIR_FREE(backingStore);
+                    } else {
+                        memmove(backingStore, path, strlen(path) + 1);
+                        vol->backingStore.path = backingStore;
+
+                        if (virStorageBackendUpdateVolTargetInfo(conn,
+                                                                 &vol->backingStore,
+                                                                 NULL,
+                                                                 NULL) < 0)
+                            VIR_FREE(vol->backingStore);
+                    }
+                }
+            } else {
+                vol->backingStore.path = backingStore;
+
+                if ((ret = virStorageBackendProbeTarget(conn,
+                                                        &vol->backingStore,
+                                                        NULL, NULL, NULL)) < 0) {
+                    if (ret == -1)
+                        goto no_memory;
+                    else {
+                        /* Silently ignore non-regular files,
+                         * eg '.' '..', 'lost+found' */
+                        VIR_FREE(vol->backingStore);
+                    }
+                }
+            }
+        }
+
+
+
         if (VIR_REALLOC_N(pool->volumes.objs,
                           pool->volumes.count+1) < 0)
             goto no_memory;
@@ -836,28 +1063,48 @@ virStorageBackendFileSystemVolCreate(vir
         }
     } else {
 #if HAVE_QEMU_IMG
-        const char *type;
+        const char *type = virStorageVolFormatFileSystemTypeToString(vol->target.format);
+        const char *backingType = vol->backingStore.path ?
+            virStorageVolFormatFileSystemTypeToString(vol->backingStore.format) : NULL;
         char size[100];
-        const char *imgargv[7];
+        const char **imgargv;
+        const char *imgargvnormal[] = {
+            QEMU_IMG, "create", "-f", type, vol->target.path, size, NULL,
+        };
+        /* XXX including "backingType" here too, once QEMU accepts
+         * the patches to specify it. It'll probably be -F backingType */
+        const char *imgargvbacking[] = {
+            QEMU_IMG, "create", "-f", type, "-b", vol->backingStore.path, vol->target.path, size, NULL,
+        };
 
-        if ((type = virStorageVolFormatFileSystemTypeToString(vol->target.format)) == NULL) {
+        if (type == NULL) {
             virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
                                   _("unknown storage vol type %d"),
                                   vol->target.format);
             return -1;
         }
+        if (vol->backingStore.path == NULL) {
+            imgargv = imgargvnormal;
+        } else {
+            if (backingType == NULL) {
+                virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                                      _("unknown storage vol backing store type %d"),
+                                      vol->backingStore.format);
+                return -1;
+            }
+            if (access(vol->backingStore.path, R_OK) != 0) {
+                virReportSystemError(conn, errno,
+                                     _("inaccessible backing store volume %s"),
+                                     vol->backingStore.path);
+                return -1;
+            }
+
+            imgargv = imgargvbacking;
+        }
 
         /* Size in KB */
         snprintf(size, sizeof(size), "%llu", vol->capacity/1024);
 
-        imgargv[0] = QEMU_IMG;
-        imgargv[1] = "create";
-        imgargv[2] = "-f";
-        imgargv[3] = type;
-        imgargv[4] = vol->target.path;
-        imgargv[5] = size;
-        imgargv[6] = NULL;
-
         if (virRun(conn, imgargv, NULL) < 0) {
             unlink(vol->target.path);
             return -1;
@@ -884,6 +1131,12 @@ virStorageBackendFileSystemVolCreate(vir
                                   vol->target.format);
             return -1;
         }
+        if (vol->target.backingStore != NULL) {
+            virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
+                                  _("copy-on-write image not supported with "
+                                    "qcow-create"));
+            return -1;
+        }
 
         /* Size in MB - yes different units to qemu-img :-( */
         snprintf(size, sizeof(size), "%llu", vol->capacity/1024/1024);
@@ -934,7 +1187,9 @@ virStorageBackendFileSystemVolCreate(vir
     }
 
     /* Refresh allocation / permissions info, but not capacity */
-    if (virStorageBackendUpdateVolInfoFD(conn, vol, fd, 0) < 0) {
+    if (virStorageBackendUpdateVolTargetInfoFD(conn, &vol->target, fd,
+                                               &vol->allocation,
+                                               NULL) < 0) {
         unlink(vol->target.path);
         close(fd);
         return -1;
diff --git a/src/storage_backend_iscsi.c b/src/storage_backend_iscsi.c
--- a/src/storage_backend_iscsi.c
+++ b/src/storage_backend_iscsi.c
@@ -228,7 +228,11 @@ virStorageBackendISCSINewLun(virConnectP
 
     VIR_FREE(devpath);
 
-    if (virStorageBackendUpdateVolInfoFD(conn, vol, fd, 1) < 0)
+    if (virStorageBackendUpdateVolTargetInfoFD(conn,
+                                               &vol->target,
+                                               fd,
+                                               &vol->allocation,
+                                               &vol->capacity) < 0)
         goto cleanup;
 
     /* XXX use unique iSCSI id instead */
diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
--- a/src/storage_backend_logical.c
+++ b/src/storage_backend_logical.c
@@ -106,18 +106,27 @@ virStorageBackendLogicalMakeVol(virConne
     }
 
     if (vol->target.path == NULL) {
-        if (VIR_ALLOC_N(vol->target.path, strlen(pool->def->target.path) +
-                        1 + strlen(vol->name) + 1) < 0) {
-            virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("volume"));
+        if (virAsprintf(&vol->target.path, "%s/%s",
+                        pool->def->target.path, vol->name) < 0) {
+            virReportOOMError(conn);
+            virStorageVolDefFree(vol);
             return -1;
         }
-        strcpy(vol->target.path, pool->def->target.path);
-        strcat(vol->target.path, "/");
-        strcat(vol->target.path, vol->name);
+    }
+
+    if (groups[1] && !STREQ(groups[1], "")) {
+        if (virAsprintf(&vol->backingStore.path, "%s/%s",
+                        pool->def->target.path, groups[1]) < 0) {
+            virReportOOMError(conn);
+            virStorageVolDefFree(vol);
+            return -1;
+        }
+
+        vol->backingStore.format = VIR_STORAGE_POOL_LOGICAL_LVM2;
     }
 
     if (vol->key == NULL &&
-        (vol->key = strdup(groups[1])) == NULL) {
+        (vol->key = strdup(groups[2])) == NULL) {
         virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("volume"));
         return -1;
     }
@@ -134,22 +143,22 @@ virStorageBackendLogicalMakeVol(virConne
     }
 
     if ((vol->source.extents[vol->source.nextent].path =
-         strdup(groups[2])) == NULL) {
+         strdup(groups[3])) == NULL) {
         virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("extents"));
         return -1;
     }
 
-    if (virStrToLong_ull(groups[3], NULL, 10, &offset) < 0) {
+    if (virStrToLong_ull(groups[4], NULL, 10, &offset) < 0) {
         virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("malformed volume extent offset value"));
         return -1;
     }
-    if (virStrToLong_ull(groups[4], NULL, 10, &length) < 0) {
+    if (virStrToLong_ull(groups[5], NULL, 10, &length) < 0) {
         virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("malformed volume extent length value"));
         return -1;
     }
-    if (virStrToLong_ull(groups[5], NULL, 10, &size) < 0) {
+    if (virStrToLong_ull(groups[6], NULL, 10, &size) < 0) {
         virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("malformed volume extent size value"));
         return -1;
@@ -168,14 +177,14 @@ virStorageBackendLogicalFindLVs(virConne
                                 virStorageVolDefPtr vol)
 {
     /*
-     *  # lvs --separator , --noheadings --units b --unbuffered --nosuffix --options "lv_name,uuid,devices,seg_size,vg_extent_size" VGNAME
-     *  RootLV,06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky,/dev/hda2(0),5234491392,33554432
-     *  SwapLV,oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M,/dev/hda2(156),1040187392,33554432
-     *  Test2,3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR,/dev/hda2(219),1073741824,33554432
-     *  Test3,UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht,/dev/hda2(251),2181038080,33554432
-     *  Test3,UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht,/dev/hda2(187),1040187392,33554432
+     *  # lvs --separator , --noheadings --units b --unbuffered --nosuffix --options "lv_name,origin,uuid,devices,seg_size,vg_extent_size" VGNAME
+     *  RootLV,,06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky,/dev/hda2(0),5234491392,33554432
+     *  SwapLV,,oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M,/dev/hda2(156),1040187392,33554432
+     *  Test2,,3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR,/dev/hda2(219),1073741824,33554432
+     *  Test3,,UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht,/dev/hda2(251),2181038080,33554432
+     *  Test3,Test2,UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht,/dev/hda2(187),1040187392,33554432
      *
-     * Pull out name & uuid, device, device extent start #, segment size, extent size.
+     * Pull out name, origin, & uuid, device, device extent start #, segment size, extent size.
      *
      * NB can be multiple rows per volume if they have many extents
      *
@@ -185,15 +194,15 @@ virStorageBackendLogicalFindLVs(virConne
      *    not a suitable separator (rhbz 470693).
      */
     const char *regexes[] = {
-        "^\\s*(\\S+),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$"
+        "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$"
     };
     int vars[] = {
-        6
+        7
     };
     const char *prog[] = {
         LVS, "--separator", ",", "--noheadings", "--units", "b",
         "--unbuffered", "--nosuffix", "--options",
-        "lv_name,uuid,devices,seg_size,vg_extent_size",
+        "lv_name,origin,uuid,devices,seg_size,vg_extent_size",
         pool->def->source.name, NULL
     };
 
@@ -565,10 +574,25 @@ virStorageBackendLogicalCreateVol(virCon
 {
     int fd = -1;
     char size[100];
-    const char *cmdargv[] = {
+    const char *cmdargvnew[] = {
         LVCREATE, "--name", vol->name, "-L", size,
         pool->def->target.path, NULL
     };
+    const char *cmdargvsnap[] = {
+        LVCREATE, "--name", vol->name, "-L", size,
+        "-s", vol->backingStore.path, NULL
+    };
+    const char **cmdargv = cmdargvnew;
+
+    if (vol->backingStore.path) {
+        if (vol->backingStore.format !=
+            VIR_STORAGE_POOL_LOGICAL_LVM2) {
+            virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+                                  _("LVM snapshots must be backed by another LVM volume"));
+            return -1;
+        }
+        cmdargv = cmdargvsnap;
+    }
 
     snprintf(size, sizeof(size)-1, "%lluK", vol->capacity/1024);
     size[sizeof(size)-1] = '\0';
diff --git a/src/storage_conf.c b/src/storage_conf.c
--- a/src/storage_conf.c
+++ b/src/storage_conf.c
@@ -249,6 +249,8 @@ virStorageVolDefFree(virStorageVolDefPtr
 
     VIR_FREE(def->target.path);
     VIR_FREE(def->target.perms.label);
+    VIR_FREE(def->backingStore.path);
+    VIR_FREE(def->backingStore.perms.label);
     VIR_FREE(def);
 }
 
@@ -998,6 +1000,28 @@ virStorageVolDefParseDoc(virConnectPtr c
     if (virStorageVolDefParsePerms(conn, ctxt, &ret->target.perms) < 0)
         goto cleanup;
 
+
+
+    ret->backingStore.path = virXPathString(conn, "string(/volume/backingStore/path)", ctxt);
+    if (options->formatFromString) {
+        char *format = virXPathString(conn, "string(/volume/backingStore/format/@type)", ctxt);
+        if (format == NULL)
+            ret->backingStore.format = options->defaultFormat;
+        else
+            ret->backingStore.format = (options->formatFromString)(format);
+
+        if (ret->backingStore.format < 0) {
+            virStorageReportError(conn, VIR_ERR_XML_ERROR,
+                                  _("unknown volume format type %s"), format);
+            VIR_FREE(format);
+            goto cleanup;
+        }
+        VIR_FREE(format);
+    }
+
+    if (virStorageVolDefParsePerms(conn, ctxt, &ret->backingStore.perms) < 0)
+        goto cleanup;
+
     return ret;
 
  cleanup:
@@ -1069,6 +1093,47 @@ virStorageVolDefParse(virConnectPtr conn
 }
 
 
+static int
+virStorageVolTargetDefFormat(virConnectPtr conn,
+                             virStorageVolOptionsPtr options,
+                             virBufferPtr buf,
+                             virStorageVolTargetPtr def,
+                             const char *type) {
+    virBufferVSprintf(buf, "  <%s>\n", type);
+
+    if (def->path)
+        virBufferVSprintf(buf,"    <path>%s</path>\n", def->path);
+
+    if (options->formatToString) {
+        const char *format = (options->formatToString)(def->format);
+        if (!format) {
+            virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                                  _("unknown volume format number %d"),
+                                  def->format);
+            return -1;
+        }
+        virBufferVSprintf(buf,"    <format type='%s'/>\n", format);
+    }
+
+    virBufferAddLit(buf,"    <permissions>\n");
+    virBufferVSprintf(buf,"      <mode>0%o</mode>\n",
+                      def->perms.mode);
+    virBufferVSprintf(buf,"      <owner>%d</owner>\n",
+                      def->perms.uid);
+    virBufferVSprintf(buf,"      <group>%d</group>\n",
+                      def->perms.gid);
+
+
+    if (def->perms.label)
+        virBufferVSprintf(buf,"      <label>%s</label>\n",
+                          def->perms.label);
+
+    virBufferAddLit(buf,"    </permissions>\n");
+
+    virBufferVSprintf(buf, "  </%s>\n", type);
+
+    return 0;
+}
 
 char *
 virStorageVolDefFormat(virConnectPtr conn,
@@ -1116,37 +1181,15 @@ virStorageVolDefFormat(virConnectPtr con
     virBufferVSprintf(&buf,"  <allocation>%llu</allocation>\n",
                       def->allocation);
 
-    virBufferAddLit(&buf, "  <target>\n");
+    if (virStorageVolTargetDefFormat(conn, options, &buf,
+                                     &def->target, "target") < 0)
+        goto cleanup;
 
-    if (def->target.path)
-        virBufferVSprintf(&buf,"    <path>%s</path>\n", def->target.path);
+    if (def->backingStore.path &&
+        virStorageVolTargetDefFormat(conn, options, &buf,
+                                     &def->backingStore, "backingStore") < 0)
+        goto cleanup;
 
-    if (options->formatToString) {
-        const char *format = (options->formatToString)(def->target.format);
-        if (!format) {
-            virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                                  _("unknown volume format number %d"),
-                                  def->target.format);
-            goto cleanup;
-        }
-        virBufferVSprintf(&buf,"    <format type='%s'/>\n", format);
-    }
-
-    virBufferAddLit(&buf,"    <permissions>\n");
-    virBufferVSprintf(&buf,"      <mode>0%o</mode>\n",
-                      def->target.perms.mode);
-    virBufferVSprintf(&buf,"      <owner>%d</owner>\n",
-                      def->target.perms.uid);
-    virBufferVSprintf(&buf,"      <group>%d</group>\n",
-                      def->target.perms.gid);
-
-
-    if (def->target.perms.label)
-        virBufferVSprintf(&buf,"      <label>%s</label>\n",
-                          def->target.perms.label);
-
-    virBufferAddLit(&buf,"    </permissions>\n");
-    virBufferAddLit(&buf, "  </target>\n");
     virBufferAddLit(&buf,"</volume>\n");
 
     if (virBufferError(&buf))
diff --git a/src/storage_conf.h b/src/storage_conf.h
--- a/src/storage_conf.h
+++ b/src/storage_conf.h
@@ -89,6 +89,7 @@ struct _virStorageVolDef {
 
     virStorageVolSource source;
     virStorageVolTarget target;
+    virStorageVolTarget backingStore;
 };
 
 typedef struct _virStorageVolDefList virStorageVolDefList;
diff --git a/src/virterror.c b/src/virterror.c
--- a/src/virterror.c
+++ b/src/virterror.c
@@ -1059,7 +1059,7 @@ void virReportSystemErrorFull(virConnect
         }
     }
 
-    if (!msgDetailBuf)
+    if (!msgDetail)
         msgDetail = errnoDetail;
 
     virRaiseError(conn, NULL, NULL, domcode, VIR_ERR_SYSTEM_ERROR, VIR_ERR_ERROR,


-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]