"Daniel P. Berrange" <berrange(a)redhat.com&gt; wrote: This looks fine, but I'd feel a lot better about it if there were a few tests of the new bits. A few questions/suggestions: ... How about S_IRWXUGO: target->perms.mode = sb.st_mode & S_IRWXUGO; ... ... 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? ... 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. ... How about using virAsprintf in place of VIR_ALLOC_N + strcpy + strcat?

"Daniel P. Berrange" <berrange(a)redhat.com&gt; wrote: ... ... All looks fine. ACK. You'll have noticed that the fix below is already committed.

Hi, I'm happy to use this new feature in my code ;) BTW, this example code makes me confused because it looks like a mode octal permission set not a numeric ID. I think it's better to use decimal digits something like '500' (ID of first user in fedora) to be friendly. Thanks ozaki-r 2009/1/21 Daniel P. Berrange <berrange(a)redhat.com&gt;: > 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 > > 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 | 336 +++++++++++++++++++++++++++++++++++++----- > src/storage_backend_iscsi.c | 6 > src/storage_backend_logical.c | 61 +++++-- > src/storage_conf.c | 101 +++++++++--- > src/storage_conf.h | 1 > 10 files changed, 609 insertions(+), 147 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;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;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 > @@ -258,6 +258,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 & (0x200-1); > + 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,234 @@ 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) > +{ > + 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) { > + /* Ignore failure */ > + } > + 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 +313,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 +353,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 +365,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 +376,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 +415,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 +822,7 @@ virStorageBackendFileSystemRefresh(virCo > > while ((ent = readdir(dir)) != NULL) { > int ret; > + char *backingStore; > > if (VIR_ALLOC(vol) < 0) > goto no_memory; > @@ -655,7 +842,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 +858,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; > @@ -837,8 +1070,10 @@ virStorageBackendFileSystemVolCreate(vir > } else { > #if HAVE_QEMU_IMG > const char *type; > + const char *backingType = NULL; > char size[100]; > - const char *imgargv[7]; > + const char *imgargv[9]; > + size_t i; > > if ((type = virStorageVolFormatFileSystemTypeToString(vol->target.format)) == NULL) { > virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, > @@ -846,17 +1081,36 @@ virStorageBackendFileSystemVolCreate(vir > vol->target.format); > return -1; > } > + if (vol->backingStore.path) { > + if ((backingType = virStorageVolFormatFileSystemTypeToString(vol->backingStore.format)) == 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; > + } > + } > > /* 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; > + 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; > > if (virRun(conn, imgargv, NULL) < 0) { > unlink(vol->target.path); > @@ -884,6 +1138,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 +1194,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 > @@ -226,7 +226,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 > @@ -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]); > + > + 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 +148,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 +182,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 +199,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 +579,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; > > > -- > |: 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 :| > > -- > Libvir-list mailing list > Libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list >