"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

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:

diff --git a/src/storage_backend.c b/src/storage_backend.c ... @@ -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);

How about S_IRWXUGO: target->perms.mode = sb.st_mode & S_IRWXUGO;

+ 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?

+ /* Ignore failure */ + } + return BACKING_STORE_OK; +} ... @@ -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;

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. ...

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]);

How about using virAsprintf in place of VIR_ALLOC_N + strcpy + strcat?

Hi, I'm happy to use this new feature in my code ;) BTW,

<owner>0744</owner> <group>0744</group>

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@redhat.com>:

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 @@ ... <target> <path>/var/lib/virt/images/sparse.img</path> + <format>qcow2</format> <permissions> <owner>0744</owner> <group>0744</group> <mode>0744</mode> <label>virt_image_t</label> </permissions> - </target> - </volume></pre> + </target></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> + ... + <backingStore> + <path>/var/lib/virt/images/master.img</path> + <format>raw</format> + <permissions> + <owner>0744</owner> + <group>0744</group> + <mode>0744</mode> + <label>virt_image_t</label> + </permissions> + </backingStore> + </volume></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 @@ ... <target> <path>/var/lib/virt/images/sparse.img</path> + <format>qcow2</format> <permissions> <owner>0744</owner> <group>0744</group> <mode>0744</mode> <label>virt_image_t</label> </permissions> - </target> - </volume></pre> + </target></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> + ... + <backingStore> + <path>/var/lib/virt/images/master.img</path> + <format>raw</format> + <permissions> + <owner>0744</owner> + <group>0744</group> + <mode>0744</mode> + <label>virt_image_t</label> + </permissions> + </backingStore> + </volume></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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list