[libvirt] [PATCHv2 0/9] Snapshots on gluster
Peter Krempa (9): storage: Add new argument for createVol backend API storage: gluster: Introduce dummy functions for creating a volume storage: Add internal API to create temporary storage pools and vols snapshot: Add support for specifying snapshot disk backing type snapshot: Test snapshot disk type specification storage: Implement ephemeral storage APIs in the generic storage driver storage: gluster: Support conversion of gluster volumes to temp volumes qemu: snapshot: Switch snapshot file deletion to the new storage API qemu: snapshot: Add support for external active snapshots on gluster docs/formatsnapshot.html.in | 15 + docs/hvsupport.pl | 3 + docs/schemas/domainsnapshot.rng | 76 +++- src/Makefile.am | 3 +- src/check-aclrules.pl | 3 + src/check-drivername.pl | 1 + src/conf/snapshot_conf.c | 42 ++- src/conf/snapshot_conf.h | 15 +- src/driver.h | 13 + src/libvirt_internal.c | 70 ++++ src/libvirt_internal.h | 22 ++ src/libvirt_private.syms | 3 + src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 9 + src/qemu/qemu_conf.c | 3 - src/qemu/qemu_driver.c | 227 ++++++++++-- src/storage/storage_backend.h | 3 +- src/storage/storage_backend_disk.c | 3 +- src/storage/storage_backend_fs.c | 30 +- src/storage/storage_backend_gluster.c | 119 +++++- src/storage/storage_backend_logical.c | 6 +- src/storage/storage_backend_rbd.c | 3 +- src/storage/storage_backend_sheepdog.c | 3 +- src/storage/storage_driver.c | 405 ++++++++++++++++++++- tests/domainsnapshotxml2xmlin/disk_snapshot.xml | 18 + .../disk_driver_name_null.xml | 2 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 22 +- .../disk_snapshot_redefine.xml | 6 +- 28 files changed, 1007 insertions(+), 120 deletions(-) create mode 100644 src/libvirt_internal.c -- 1.8.5.2
The new argument will be used when creating definitions for volumes that already exist for the temporary storage volume APIs. The argument will modify the expectations about the storage files and suppress some error reports. --- Notes: Version 2: - add commit message lost in a botched rebase - always initialize volume type for filesystem volumes src/storage/storage_backend.h | 3 ++- src/storage/storage_backend_disk.c | 3 ++- src/storage/storage_backend_fs.c | 30 +++++++++++++++++++++++------- src/storage/storage_backend_logical.c | 6 ++++-- src/storage/storage_backend_rbd.c | 3 ++- src/storage/storage_backend_sheepdog.c | 3 ++- src/storage/storage_driver.c | 4 ++-- 7 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 378bc4d..34630fe 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -54,7 +54,8 @@ typedef int (*virStorageBackendBuildVol)(virConnectPtr conn, unsigned int flags); typedef int (*virStorageBackendCreateVol)(virConnectPtr conn, virStoragePoolObjPtr pool, - virStorageVolDefPtr vol); + virStorageVolDefPtr vol, + bool internal); typedef int (*virStorageBackendRefreshVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol); diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index aa3b72f..a77c298 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -618,7 +618,8 @@ virStorageBackendDiskPartBoundries(virStoragePoolObjPtr pool, static int virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, - virStorageVolDefPtr vol) + virStorageVolDefPtr vol, + bool internal ATTRIBUTE_UNUSED) { if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index fa11e2f..2594cde 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -996,8 +996,10 @@ virStorageBackendFileSystemDelete(virConnectPtr conn ATTRIBUTE_UNUSED, static int virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) + virStorageVolDefPtr vol, + bool internal) { + struct stat st; vol->type = VIR_STORAGE_VOL_FILE; @@ -1007,15 +1009,29 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED, vol->name) == -1) return -1; - if (virFileExists(vol->target.path)) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("volume target path '%s' already exists"), - vol->target.path); - return -1; + if (internal) { + vol->type = VIR_STORAGE_VOL_FILE; + + if (stat(vol->target.path, &st) == 0) { + if (S_ISDIR(st.st_mode)) + vol->type = VIR_STORAGE_VOL_DIR; + else if (S_ISBLK(st.st_mode)) + vol->type = VIR_STORAGE_VOL_BLOCK; + } + } else { + if (virFileExists(vol->target.path)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("volume target path '%s' already exists"), + vol->target.path); + return -1; + } } VIR_FREE(vol->key); - return VIR_STRDUP(vol->key, vol->target.path); + if (VIR_STRDUP(vol->key, vol->target.path) < 0) + return -1; + + return 0; } static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 039d962..2e2560f 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -784,7 +784,8 @@ error: static int virStorageBackendLogicalCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) + virStorageVolDefPtr vol, + bool internal) { if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -800,7 +801,8 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, vol->name) == -1) return -1; - if (virFileExists(vol->target.path)) { + if (!internal && + virFileExists(vol->target.path)) { virReportError(VIR_ERR_OPERATION_INVALID, _("volume target path '%s' already exists"), vol->target.path); diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index c5f0bc5..75425f4 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -439,7 +439,8 @@ cleanup: static int virStorageBackendRBDCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) + virStorageVolDefPtr vol, + bool internal ATTRIBUTE_UNUSED) { vol->type = VIR_STORAGE_VOL_NETWORK; diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index a6981ce..705451b 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -152,7 +152,8 @@ virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, static int virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, - virStorageVolDefPtr vol) + virStorageVolDefPtr vol, + bool internal ATTRIBUTE_UNUSED) { if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index bb13e8e..a6bc801 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1643,7 +1643,7 @@ storageVolCreateXML(virStoragePoolPtr obj, /* Wipe any key the user may have suggested, as volume creation * will generate the canonical key. */ VIR_FREE(voldef->key); - if (backend->createVol(obj->conn, pool, voldef) < 0) { + if (backend->createVol(obj->conn, pool, voldef, false) < 0) { goto cleanup; } @@ -1822,7 +1822,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, * Wipe any key the user may have suggested, as volume creation * will generate the canonical key. */ VIR_FREE(newvol->key); - if (backend->createVol(obj->conn, pool, newvol) < 0) { + if (backend->createVol(obj->conn, pool, newvol, false) < 0) { goto cleanup; } -- 1.8.5.2
On 01/16/2014 06:14 AM, Peter Krempa wrote:
The new argument will be used when creating definitions for volumes that already exist for the temporary storage volume APIs. The argument will modify the expectations about the storage files and suppress some error reports. ---
Notes: Version 2: - add commit message lost in a botched rebase - always initialize volume type for filesystem volumes
VIR_FREE(vol->key); - return VIR_STRDUP(vol->key, vol->target.path); + if (VIR_STRDUP(vol->key, vol->target.path) < 0) + return -1; + + return 0;
This changes the result from 1 to 0, but looks safe. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The temporary pool code will need to initialize some fields for the temporary gluster volumes. This is done by the createVol function of the storage backend. This patch implements only the metadata setting. Attempts to create a regular volume yield an error. --- src/storage/storage_backend_gluster.c | 119 ++++++++++++++++++++++++++++------ 1 file changed, 99 insertions(+), 20 deletions(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index c73cf8a..8d57098 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -178,6 +178,51 @@ virStorageBackendGlusterReadHeader(glfs_fd_t *fd, return nread; } + +static int +virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state, + virStorageVolDefPtr vol, + const char *name) +{ + int ret = -1; + char *tmp; + + VIR_FREE(vol->key); + VIR_FREE(vol->target.path); + + vol->type = VIR_STORAGE_VOL_NETWORK; + vol->target.format = VIR_STORAGE_FILE_RAW; + + if (name) { + VIR_FREE(vol->name); + if (VIR_STRDUP(vol->name, name) < 0) + goto cleanup; + } + + if (virAsprintf(&vol->key, "%s%s%s", state->volname, state->dir, + vol->name) < 0) + goto cleanup; + + tmp = state->uri->path; + if (virAsprintf(&state->uri->path, "/%s", vol->key) < 0) { + state->uri->path = tmp; + goto cleanup; + } + if (!(vol->target.path = virURIFormat(state->uri))) { + VIR_FREE(state->uri->path); + state->uri->path = tmp; + goto cleanup; + } + VIR_FREE(state->uri->path); + state->uri->path = tmp; + + ret = 0; + +cleanup: + return ret; +} + + /* Populate *volptr for the given name and stat information, or leave * it NULL if the entry should be skipped (such as "."). Return 0 on * success, -1 on failure. */ @@ -187,7 +232,6 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, struct stat *st, virStorageVolDefPtr *volptr) { - char *tmp; int ret = -1; virStorageVolDefPtr vol = NULL; glfs_fd_t *fd = NULL; @@ -220,25 +264,9 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, &vol->capacity) < 0) goto cleanup; - if (VIR_STRDUP(vol->name, name) < 0) - goto cleanup; - if (virAsprintf(&vol->key, "%s%s%s", state->volname, state->dir, - vol->name) < 0) + if (virStorageBackendGlusterSetMetadata(state, vol, name) < 0) goto cleanup; - tmp = state->uri->path; - if (virAsprintf(&state->uri->path, "/%s", vol->key) < 0) { - state->uri->path = tmp; - goto cleanup; - } - if (!(vol->target.path = virURIFormat(state->uri))) { - VIR_FREE(state->uri->path); - state->uri->path = tmp; - goto cleanup; - } - VIR_FREE(state->uri->path); - state->uri->path = tmp; - if (S_ISDIR(st->st_mode)) { vol->type = VIR_STORAGE_VOL_NETDIR; vol->target.format = VIR_STORAGE_FILE_DIR; @@ -248,8 +276,6 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, goto cleanup; } - vol->type = VIR_STORAGE_VOL_NETWORK; - vol->target.format = VIR_STORAGE_FILE_RAW; /* No need to worry about O_NONBLOCK - gluster doesn't allow creation * of fifos, so there's nothing it would protect us from. */ if (!(fd = glfs_open(state->vol, name, O_RDONLY | O_NOCTTY))) { @@ -444,10 +470,63 @@ cleanup: } +static int +virStorageBackendGlusterVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + bool internal) +{ + virStorageBackendGlusterStatePtr state = NULL; + struct stat st; + int ret = -1; + + if (!internal) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("gluster pool backend doesn't " + "yet support volume creation")); + goto cleanup; + } + + if (!(state = virStorageBackendGlusterOpen(pool))) + goto cleanup; + + if (virStorageBackendGlusterSetMetadata(state, vol, NULL) < 0) + goto cleanup; + + if (glfs_stat(state->vol, vol->name, &st) == 0 && + S_ISDIR(st.st_mode)) { + vol->type = VIR_STORAGE_VOL_NETDIR; + vol->target.format = VIR_STORAGE_FILE_DIR; + } + + ret = 0; + +cleanup: + virStorageBackendGlusterClose(state); + return ret; +} + + +static int +virStorageBackendGlusterVolBuild(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, + virStorageVolDefPtr vol ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(0, -1); + + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("gluster pool backend doesn't yet support volume building")); + return -1; +} + + virStorageBackend virStorageBackendGluster = { .type = VIR_STORAGE_POOL_GLUSTER, .refreshPool = virStorageBackendGlusterRefreshPool, + .createVol = virStorageBackendGlusterVolCreate, + .buildVol = virStorageBackendGlusterVolBuild, .deleteVol = virStorageBackendGlusterVolDelete, }; -- 1.8.5.2
On 01/16/2014 06:14 AM, Peter Krempa wrote:
The temporary pool code will need to initialize some fields for the temporary gluster volumes. This is done by the createVol function of the storage backend. This patch implements only the metadata setting. Attempts to create a regular volume yield an error.
It will be nice to allow creation of volumes, but I agree that it can come later.
+ +static int +virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state, + virStorageVolDefPtr vol, + const char *name)
Thanks for the refactor; this looks better than v1.
+static int +virStorageBackendGlusterVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + bool internal) +{ + virStorageBackendGlusterStatePtr state = NULL; + struct stat st; + int ret = -1; + + if (!internal) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("gluster pool backend doesn't " + "yet support volume creation")); + goto cleanup; + } ... + + if (glfs_stat(state->vol, vol->name, &st) == 0 && + S_ISDIR(st.st_mode)) { + vol->type = VIR_STORAGE_VOL_NETDIR; + vol->target.format = VIR_STORAGE_FILE_DIR; + }
Shouldn't we error out if S_ISDIR and internal? That is, the only time we should be using this function internally is for a regular file, not a directory. ACK with that addressed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/27/14 18:49, Eric Blake wrote:
On 01/16/2014 06:14 AM, Peter Krempa wrote:
The temporary pool code will need to initialize some fields for the temporary gluster volumes. This is done by the createVol function of the storage backend. This patch implements only the metadata setting. Attempts to create a regular volume yield an error.
It will be nice to allow creation of volumes, but I agree that it can come later.
+ +static int +virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state, + virStorageVolDefPtr vol, + const char *name)
Thanks for the refactor; this looks better than v1.
+static int +virStorageBackendGlusterVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + bool internal) +{ + virStorageBackendGlusterStatePtr state = NULL; + struct stat st; + int ret = -1; + + if (!internal) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("gluster pool backend doesn't " + "yet support volume creation")); + goto cleanup; + } ... + + if (glfs_stat(state->vol, vol->name, &st) == 0 && + S_ISDIR(st.st_mode)) { + vol->type = VIR_STORAGE_VOL_NETDIR; + vol->target.format = VIR_STORAGE_FILE_DIR; + }
Shouldn't we error out if S_ISDIR and internal? That is, the only time we should be using this function internally is for a regular file, not a directory.
Well, currently there isn't any planned use-case regarding the use of this part of code on directories. There may be a need for this in the future, but it can always be removed.
ACK with that addressed.
If a VM driver wants to access stuff provided by a storage driver the volume needs to be a part of a storage pool. As this wasn't designed in from the beginning we need a way to convert generic domain disk and snapshot disk definitions into temporary pools and volumes. This patch allows that by adding private storage driver APIs that can be used to obtain a pool and vol definition. --- docs/hvsupport.pl | 3 +++ src/Makefile.am | 3 ++- src/check-drivername.pl | 1 + src/driver.h | 13 +++++++++ src/libvirt_internal.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_internal.h | 22 +++++++++++++++ src/libvirt_private.syms | 3 +++ 7 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 src/libvirt_internal.c diff --git a/docs/hvsupport.pl b/docs/hvsupport.pl index f8483f9..4f72cc1 100755 --- a/docs/hvsupport.pl +++ b/docs/hvsupport.pl @@ -176,6 +176,9 @@ $apis{virDomainMigratePerform3Params} = "1.1.0"; $apis{virDomainMigrateFinish3Params} = "1.1.0"; $apis{virDomainMigrateConfirm3Params} = "1.1.0"; +$apis{virStorageEphemeralFree} = "1.2.2"; +$apis{virStorageEphemeralFromDiskDef} = "1.2.2"; +$apis{virStorageEphemeralFromSnapshotDiskDef} = "1.2.2"; # Now we want to get the mapping between public APIs diff --git a/src/Makefile.am b/src/Makefile.am index 57e163f..c0c535d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -176,7 +176,8 @@ DRIVER_SOURCES = \ $(DATATYPES_SOURCES) \ fdstream.c fdstream.h \ $(NODE_INFO_SOURCES) \ - libvirt.c libvirt_internal.h \ + libvirt.c \ + libvirt_internal.c libvirt_internal.h \ locking/lock_manager.c locking/lock_manager.h \ locking/lock_driver.h \ locking/lock_driver_nop.h locking/lock_driver_nop.c \ diff --git a/src/check-drivername.pl b/src/check-drivername.pl index 5c8de0a..50b3480 100755 --- a/src/check-drivername.pl +++ b/src/check-drivername.pl @@ -50,6 +50,7 @@ while (<DRVFILE>) { next if $drv =~ /virDrvState/; next if $drv =~ /virDrvDomainMigrate(Prepare|Perform|Confirm|Begin|Finish)/; + next if $drv =~ /virDrvStorageEphemeral/; my $sym = $drv; $sym =~ s/virDrv/vir/; diff --git a/src/driver.h b/src/driver.h index 5f4cd8d..b72c5e9 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1754,6 +1754,16 @@ typedef int typedef int (*virDrvStoragePoolIsPersistent)(virStoragePoolPtr pool); +typedef void +(*virDrvStorageEphemeralFree)(virStorageEphemeralPtr def); + +typedef virStorageEphemeralPtr +(*virDrvStorageEphemeralFromDiskDef)(virConnectPtr conn, + virDomainDiskDefPtr def); + +typedef virStorageEphemeralPtr +(*virDrvStorageEphemeralFromSnapshotDiskDef)(virConnectPtr conn, + virDomainSnapshotDiskDefPtr def); typedef struct _virStorageDriver virStorageDriver; @@ -1813,6 +1823,9 @@ struct _virStorageDriver { virDrvStorageVolResize storageVolResize; virDrvStoragePoolIsActive storagePoolIsActive; virDrvStoragePoolIsPersistent storagePoolIsPersistent; + virDrvStorageEphemeralFree storageEphemeralFree; + virDrvStorageEphemeralFromDiskDef storageEphemeralFromDiskDef; + virDrvStorageEphemeralFromSnapshotDiskDef storageEphemeralFromSnapshotDiskDef; }; # ifdef WITH_LIBVIRTD diff --git a/src/libvirt_internal.c b/src/libvirt_internal.c new file mode 100644 index 0000000..69fa85f --- /dev/null +++ b/src/libvirt_internal.c @@ -0,0 +1,70 @@ +/* + * libvirt_internal.c: internally exported APIs, not for public use + * + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> +#include "libvirt_internal.h" + +#include "datatypes.h" +#include "viralloc.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +void +virStorageEphemeralFree(virStorageEphemeralPtr def) +{ + if (!def) + return; + + if (!def->pool) { + VIR_FREE(def); + return; + } + + if (def->pool->conn->storageDriver->storageEphemeralFree) { + def->pool->conn->storageDriver->storageEphemeralFree(def); + return; + } + + return; +} + + +virStorageEphemeralPtr +virStorageEphemeralFromDiskDef(virConnectPtr conn, + virDomainDiskDefPtr def) +{ + if (conn->storageDriver->storageEphemeralFromDiskDef) + return conn->storageDriver->storageEphemeralFromDiskDef(conn, def); + + virReportUnsupportedError(); + return NULL; +} + + +virStorageEphemeralPtr +virStorageEphemeralFromSnapshotDiskDef(virConnectPtr conn, + virDomainSnapshotDiskDefPtr def) +{ + if (conn->storageDriver->storageEphemeralFromSnapshotDiskDef) + return conn->storageDriver->storageEphemeralFromSnapshotDiskDef(conn, def); + + virReportUnsupportedError(); + return NULL; +} diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 115d8d1..0db46d8 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -27,6 +27,9 @@ # include "internal.h" +# include "conf/domain_conf.h" +# include "conf/snapshot_conf.h" + typedef void (*virStateInhibitCallback)(bool inhibit, void *opaque); @@ -280,4 +283,23 @@ int virDomainMigrateConfirm3Params(virDomainPtr domain, int cookieinlen, unsigned int flags, int cancelled); + +/* storage related internal stuff */ +typedef struct _virStorageEphemeral virStorageEphemeral; +typedef virStorageEphemeral *virStorageEphemeralPtr; + +struct _virStorageEphemeral +{ + bool existing; + virStorageVolPtr vol; + virStoragePoolPtr pool; +}; + +void virStorageEphemeralFree(virStorageEphemeralPtr def); + +virStorageEphemeralPtr virStorageEphemeralFromDiskDef(virConnectPtr conn, + virDomainDiskDefPtr def); + +virStorageEphemeralPtr virStorageEphemeralFromSnapshotDiskDef(virConnectPtr conn, + virDomainSnapshotDiskDefPtr def); #endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 91d1304..921f262 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -813,6 +813,9 @@ virRegisterNodeDeviceDriver; virRegisterNWFilterDriver; virRegisterSecretDriver; virRegisterStorageDriver; +virStorageEphemeralFree; +virStorageEphemeralFromDiskDef; +virStorageEphemeralFromSnapshotDiskDef; # locking/domain_lock.h -- 1.8.5.2
On 01/16/2014 06:14 AM, Peter Krempa wrote:
If a VM driver wants to access stuff provided by a storage driver the volume needs to be a part of a storage pool. As this wasn't designed in from the beginning we need a way to convert generic domain disk and snapshot disk definitions into temporary pools and volumes. This patch allows that by adding private storage driver APIs that can be used to obtain a pool and vol definition. --- docs/hvsupport.pl | 3 +++ src/Makefile.am | 3 ++- src/check-drivername.pl | 1 + src/driver.h | 13 +++++++++ src/libvirt_internal.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_internal.h | 22 +++++++++++++++ src/libvirt_private.syms | 3 +++ 7 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 src/libvirt_internal.c
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Add support for specifying various types when doing snapshots. This will later allow to do snapshots on network backed volumes. Disks of type 'volume' are not supported by snapshots (yet). --- Notes: Version 2: - always format disk type and fix fallout docs/formatsnapshot.html.in | 15 +++++ docs/schemas/domainsnapshot.rng | 76 ++++++++++++++++++---- src/conf/snapshot_conf.c | 42 +++++++----- src/conf/snapshot_conf.h | 15 +++-- src/qemu/qemu_conf.c | 3 - src/qemu/qemu_driver.c | 58 +++++++++++------ .../disk_driver_name_null.xml | 2 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 4 +- .../disk_snapshot_redefine.xml | 6 +- 9 files changed, 157 insertions(+), 64 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 76689cb..c2cd18c 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -170,6 +170,21 @@ snapshots, the original file name becomes the read-only snapshot, and the new file name contains the read-write delta of all disk changes since the snapshot. + + <span class="since">Since 1.2.2</span> the <code>disk</code> element + supports an optional attribute <code>type</code> if the + <code>snapshot</code> attribute is set to <code>external</code>. + This attribute specifies the snapshot target storage type and allows + to overwrite the default <code>file</code>type. The <code>type</code> + attribute along with the format of the <code>source</code> + sub-element is identical to the <code>source</code> element used in + domain disk definitions. See the + <a href="formatdomain.html#elementsDisks">disk devices</a> section + documentation for further information. + + Libvirt currently supports the <code>type</code> element in the qemu + driver and supported values are <code>file</code> and + <code>block</code> <span class="since">(since 1.2.2)</span>. </dd> </dl> </dd> diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 169fcfb..de9e788 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -123,19 +123,73 @@ <value>external</value> </attribute> </optional> - <interleave> - <ref name='disksnapshotdriver'/> - <optional> - <element name='source'> + <choice> + <group> + <optional> + <attribute name='type'> + <value>file</value> + </attribute> + </optional> + <interleave> <optional> - <attribute name='file'> - <ref name='absFilePath'/> - </attribute> + <element name='source'> + <optional> + <attribute name='file'> + <ref name='absFilePath'/> + </attribute> + </optional> + <empty/> + </element> </optional> - <empty/> - </element> - </optional> - </interleave> + <ref name='disksnapshotdriver'/> + </interleave> + </group> + <group> + <attribute name='type'> + <value>block</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="dev"> + <ref name="absFilePath"/> + </attribute> + <empty/> + </element> + </optional> + <ref name='disksnapshotdriver'/> + </interleave> + </group> + <group> + <attribute name="type"> + <value>dir</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="dir"> + <ref name="absFilePath"/> + </attribute> + <empty/> + </element> + </optional> + <ref name='disksnapshotdriver'/> + </interleave> + </group> + <group> + <attribute name="type"> + <value>network</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <ref name='diskSourceNetwork'/> + </element> + </optional> + <ref name='disksnapshotdriver'/> + </interleave> + </group> + </choice> </group> </choice> </element> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index fb0b4cc..b5b522c 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -108,6 +108,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, { int ret = -1; char *snapshot = NULL; + char *type = NULL; xmlNodePtr cur; def->name = virXMLPropString(node, "name"); @@ -128,7 +129,16 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, } } - def->type = -1; + if ((type = virXMLPropString(node, "type"))) { + if ((def->type = virDomainDiskTypeFromString(type)) < 0 || + def->type == VIR_DOMAIN_DISK_TYPE_VOLUME) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk snapshot type '%s'"), type); + goto cleanup; + } + } else { + def->type = VIR_DOMAIN_DISK_TYPE_FILE; + } for (cur = node->children; cur; cur = cur->next) { if (cur->type != XML_ELEMENT_NODE) @@ -137,17 +147,12 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, if (!def->file && xmlStrEqual(cur->name, BAD_CAST "source")) { - int backingtype = def->type; - - if (backingtype < 0) - backingtype = VIR_DOMAIN_DISK_TYPE_FILE; - if (virDomainDiskSourceDefParse(cur, - backingtype, + def->type, &def->file, - NULL, - NULL, - NULL, + &def->protocol, + &def->nhosts, + &def->hosts, NULL) < 0) goto cleanup; @@ -174,6 +179,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, ret = 0; cleanup: VIR_FREE(snapshot); + VIR_FREE(type); if (ret < 0) virDomainSnapshotDiskDefClear(def); return ret; @@ -532,7 +538,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, goto cleanup; disk->index = i; disk->snapshot = def->dom->disks[i]->snapshot; - disk->type = -1; + disk->type = VIR_DOMAIN_DISK_TYPE_FILE; if (!disk->snapshot) disk->snapshot = default_snapshot; } @@ -550,8 +556,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, const char *tmp; struct stat sb; - if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE && - disk->type != -1) { + if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot generate external snapshot name " "for disk '%s' on a '%s' device"), @@ -614,15 +619,12 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " snapshot='%s'", virDomainSnapshotLocationTypeToString(disk->snapshot)); - if (type < 0) - type = VIR_DOMAIN_DISK_TYPE_FILE; - if (!disk->file && disk->format == 0) { virBufferAddLit(buf, "/>\n"); return; } - virBufferAddLit(buf, ">\n"); + virBufferAsprintf(buf, " type='%s'>\n", virDomainDiskTypeToString(type)); if (disk->format > 0) virBufferEscapeString(buf, " <driver type='%s'/>\n", @@ -630,7 +632,11 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, virDomainDiskSourceDefFormatInternal(buf, type, disk->file, - 0, 0, 0, NULL, 0, NULL, NULL, 0); + 0, + disk->protocol, + disk->nhosts, + disk->hosts, + 0, NULL, NULL, 0); virBufferAddLit(buf, " </disk>\n"); } diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 241d63c..bcd92dc 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -48,12 +48,15 @@ enum virDomainSnapshotState { typedef struct _virDomainSnapshotDiskDef virDomainSnapshotDiskDef; typedef virDomainSnapshotDiskDef *virDomainSnapshotDiskDefPtr; struct _virDomainSnapshotDiskDef { - char *name; /* name matching the <target dev='...' of the domain */ - int index; /* index within snapshot->dom->disks that matches name */ - int snapshot; /* enum virDomainSnapshotLocation */ - int type; /* enum virDomainDiskType */ - char *file; /* new source file when snapshot is external */ - int format; /* enum virStorageFileFormat */ + char *name; /* name matching the <target dev='...' of the domain */ + int index; /* index within snapshot->dom->disks that matches name */ + int snapshot; /* enum virDomainSnapshotLocation */ + int type; /* enum virDomainDiskType */ + char *file; /* new source file when snapshot is external */ + int format; /* enum virStorageFileFormat */ + int protocol; /* network source protocol */ + size_t nhosts; /* network source hosts count */ + virDomainDiskHostDefPtr hosts; /* network source hosts */ }; /* Stores the complete snapshot metadata */ diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 4378791..c102ddc 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1473,9 +1473,6 @@ cleanup: int qemuSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def) { - if (def->type == -1) - return VIR_DOMAIN_DISK_TYPE_FILE; - return def->type; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ebb77dc..7f01014 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12207,33 +12207,47 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, } if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0 || - VIR_STRDUP(source, snap->file) < 0 || (persistDisk && VIR_STRDUP(persistSource, source) < 0)) goto cleanup; - /* create the stub file and set selinux labels; manipulate disk in - * place, in a way that can be reverted on failure. */ - if (!reuse) { - fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT, - &need_unlink, NULL); - if (fd < 0) - goto cleanup; - VIR_FORCE_CLOSE(fd); - } - /* XXX Here, we know we are about to alter disk->backingChain if - * successful, so we nuke the existing chain so that future - * commands will recompute it. Better would be storing the chain - * ourselves rather than reprobing, but this requires modifying - * domain_conf and our XML to fully track the chain across - * libvirtd restarts. */ + * successful, so we nuke the existing chain so that future commands will + * recompute it. Better would be storing the chain ourselves rather than + * reprobing, but this requires modifying domain_conf and our XML to fully + * track the chain across libvirtd restarts. */ virStorageFileFreeMetadata(disk->backingChain); disk->backingChain = NULL; - if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source, - VIR_DISK_CHAIN_READ_WRITE) < 0) { - qemuDomainPrepareDiskChainElement(driver, vm, disk, source, - VIR_DISK_CHAIN_NO_ACCESS); + switch (snap->type) { + case VIR_DOMAIN_DISK_TYPE_BLOCK: + reuse = true; + /* fallthrough */ + case VIR_DOMAIN_DISK_TYPE_FILE: + if (VIR_STRDUP(source, snap->file) < 0) + goto cleanup; + + /* create the stub file and set selinux labels; manipulate disk in + * place, in a way that can be reverted on failure. */ + if (!reuse) { + fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT, + &need_unlink, NULL); + if (fd < 0) + goto cleanup; + VIR_FORCE_CLOSE(fd); + } + + if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source, + VIR_DISK_CHAIN_READ_WRITE) < 0) { + qemuDomainPrepareDiskChainElement(driver, vm, disk, source, + VIR_DISK_CHAIN_NO_ACCESS); + goto cleanup; + } + break; + + default: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("snapshots are not supported on '%s' volumes"), + virDomainDiskTypeToString(snap->type)); goto cleanup; } @@ -12252,11 +12266,13 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, disk->src = source; source = NULL; disk->format = format; + disk->type = snap->type; if (persistDisk) { VIR_FREE(persistDisk->src); persistDisk->src = persistSource; persistSource = NULL; persistDisk->format = format; + persistDisk->type = snap->type; } cleanup: @@ -12298,11 +12314,13 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, disk->src = source; source = NULL; disk->format = origdisk->format; + disk->type = origdisk->type; if (persistDisk) { VIR_FREE(persistDisk->src); persistDisk->src = persistSource; persistSource = NULL; persistDisk->format = origdisk->format; + persistDisk->type = origdisk->type; } cleanup: diff --git a/tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml b/tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml index 41961f1..ddd350d 100644 --- a/tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml +++ b/tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml @@ -2,7 +2,7 @@ <name>asdf</name> <description>adsf</description> <disks> - <disk name='vda' snapshot='external'> + <disk name='vda' snapshot='external' type='file'> <source file='/tmp/foo'/> </disk> </disks> diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml index 1a1fc02..0ea7129 100644 --- a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml @@ -5,10 +5,10 @@ <disk name='/dev/HostVG/QEMUGuest1'/> <disk name='hdb' snapshot='no'/> <disk name='hdc' snapshot='internal'/> - <disk name='hdd' snapshot='external'> + <disk name='hdd' snapshot='external' type='file'> <driver type='qed'/> </disk> - <disk name='hde' snapshot='external'> + <disk name='hde' snapshot='external' type='file'> <source file='/path/to/new'/> </disk> </disks> diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot_redefine.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot_redefine.xml index 5f42bf5..c267db5 100644 --- a/tests/domainsnapshotxml2xmlout/disk_snapshot_redefine.xml +++ b/tests/domainsnapshotxml2xmlout/disk_snapshot_redefine.xml @@ -11,15 +11,15 @@ <disk name='hda' snapshot='no'/> <disk name='hdb' snapshot='no'/> <disk name='hdc' snapshot='internal'/> - <disk name='hdd' snapshot='external'> + <disk name='hdd' snapshot='external' type='file'> <driver type='qed'/> <source file='/path/to/generated4'/> </disk> - <disk name='hde' snapshot='external'> + <disk name='hde' snapshot='external' type='file'> <driver type='qcow2'/> <source file='/path/to/new'/> </disk> - <disk name='hdf' snapshot='external'> + <disk name='hdf' snapshot='external' type='file'> <driver type='qcow2'/> <source file='/path/to/generated5'/> </disk> -- 1.8.5.2
On 01/16/2014 06:14 AM, Peter Krempa wrote:
Add support for specifying various types when doing snapshots. This will later allow to do snapshots on network backed volumes. Disks of type 'volume' are not supported by snapshots (yet). ---
Notes: Version 2: - always format disk type and fix fallout
+ <group> + <attribute name="type"> + <value>dir</value>
I guess this one is here for symmetry, although we don't really allow taking a snapshot of a directory mount, let alone a directory that is a snapshot of a file. What if you omit this branch of the <choice>...
- def->type = -1; + if ((type = virXMLPropString(node, "type"))) { + if ((def->type = virDomainDiskTypeFromString(type)) < 0 || + def->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
...as well as explicitly reject directory types here? But overall looks okay to me. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/27/14 23:12, Eric Blake wrote:
On 01/16/2014 06:14 AM, Peter Krempa wrote:
Add support for specifying various types when doing snapshots. This will later allow to do snapshots on network backed volumes. Disks of type 'volume' are not supported by snapshots (yet). ---
Notes: Version 2: - always format disk type and fix fallout
+ <group> + <attribute name="type"> + <value>dir</value>
I guess this one is here for symmetry, although we don't really allow taking a snapshot of a directory mount, let alone a directory that is a snapshot of a file. What if you omit this branch of the <choice>...
- def->type = -1; + if ((type = virXMLPropString(node, "type"))) { + if ((def->type = virDomainDiskTypeFromString(type)) < 0 || + def->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
...as well as explicitly reject directory types here?
But overall looks okay to me. ACK.
I've fixed your comments and squashed the patch adding tests into this one and pushed the result. I've pushed this one separately as I found a few bugs in the rest of the series that will need a repost of the rest. Peter
Amend tests to check parsing of the various new disk types that can now be specified. --- tests/domainsnapshotxml2xmlin/disk_snapshot.xml | 18 ++++++++++++++++++ tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/tests/domainsnapshotxml2xmlin/disk_snapshot.xml b/tests/domainsnapshotxml2xmlin/disk_snapshot.xml index ee6b46a..aa1522a 100644 --- a/tests/domainsnapshotxml2xmlin/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlin/disk_snapshot.xml @@ -12,5 +12,23 @@ <disk name='hde' snapshot='external'> <source file='/path/to/new'/> </disk> + <disk name='hde' snapshot='external' type='file'> + <source file='/path/to/new2'/> + </disk> + <disk name='hdf' snapshot='external' type='block'> + <source dev='/path/to/new3'/> + </disk> + <disk name='hdg' snapshot='external' type='network'> + <source protocol='gluster' name='volume/path'> + <host name='host' port='1234'/> + </source> + </disk> + <disk name='hdh' snapshot='external' type='network'> + <source protocol='rbd' name='name'> + <host name='host' port='1234'/> + <host name='host2' port='1234' transport='rdma'/> + <host name='host3' port='1234'/> + </source> + </disk> </disks> </domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml index 0ea7129..c2e77d7 100644 --- a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml @@ -11,5 +11,23 @@ <disk name='hde' snapshot='external' type='file'> <source file='/path/to/new'/> </disk> + <disk name='hde' snapshot='external' type='file'> + <source file='/path/to/new2'/> + </disk> + <disk name='hdf' snapshot='external' type='block'> + <source dev='/path/to/new3'/> + </disk> + <disk name='hdg' snapshot='external' type='network'> + <source protocol='gluster' name='volume/path'> + <host name='host' port='1234'/> + </source> + </disk> + <disk name='hdh' snapshot='external' type='network'> + <source protocol='rbd' name='name'> + <host name='host' port='1234'/> + <host name='host2' port='1234' transport='rdma'/> + <host name='host3' port='1234'/> + </source> + </disk> </disks> </domainsnapshot> -- 1.8.5.2
On 01/16/2014 06:14 AM, Peter Krempa wrote:
Amend tests to check parsing of the various new disk types that can now be specified. --- tests/domainsnapshotxml2xmlin/disk_snapshot.xml | 18 ++++++++++++++++++ tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+)
Worth squashing into the patch that actually added this? ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This patch implements the APIs for creating temporary storage pools for the generic storage driver. --- src/check-aclrules.pl | 3 + src/storage/storage_driver.c | 253 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 256 insertions(+) diff --git a/src/check-aclrules.pl b/src/check-aclrules.pl index 057517e..7174868 100755 --- a/src/check-aclrules.pl +++ b/src/check-aclrules.pl @@ -73,6 +73,9 @@ my %implwhitelist = ( "xenUnifiedDomainIsPersistent" => 1, "xenUnifiedDomainIsUpdated" => 1, "xenUnifiedDomainOpenConsole" => 1, + "storageEphemeralFree" => 1, # internal API, no RPC + "storageEphemeralFromDiskDef" => 1, # internal API, no RPC + "storageEphemeralFromSnapshotDiskDef" => 1, # internal API, no RPC ); my %filterimplwhitelist = ( "xenUnifiedConnectListDomains" => 1, diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index a6bc801..c82c620 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -50,6 +50,8 @@ #include "virstring.h" #include "viraccessapicheck.h" +#include "dirname.h" + #define VIR_FROM_THIS VIR_FROM_STORAGE static virStorageDriverStatePtr driverState; @@ -2586,6 +2588,253 @@ cleanup: return ret; } + +static void +storageEphemeralFree(virStorageEphemeralPtr def) +{ + virConnectPtr conn; + virStorageDriverStatePtr driver; + virStoragePoolObjPtr pool; + + if (!def) + return; + + if (!def->existing && + def->pool) { + conn = def->pool->conn; + driver = conn->storagePrivateData; + + if ((pool = virStoragePoolObjFindByUUID(&driver->pools, + def->pool->uuid))) { + virStoragePoolObjRemove(&driver->pools, pool); + } + } + + if (def->vol) + virStorageVolFree(def->vol); + + if (def->pool) + virStoragePoolFree(def->pool); +} + + +static void +storageEphemeralGenerateUniquePoolID(virStoragePoolDefPtr def) +{ + virUUIDGenerate(def->uuid); + virUUIDFormat(def->uuid, def->name); +} + + +static virStoragePoolPtr +storageEphemeralCreateDirPool(virConnectPtr conn, + const char *source) +{ + virStorageDriverStatePtr driver = conn->storagePrivateData; + virStoragePoolDefPtr def = NULL; + virStoragePoolObjPtr pool = NULL; + virStorageBackendPtr backend; + virStoragePoolPtr ret = NULL; + + if (VIR_ALLOC(def) < 0) + goto cleanup; + + if (VIR_ALLOC_N(def->name, VIR_UUID_STRING_BUFLEN) < 0) + goto cleanup; + + if (VIR_STRDUP(def->target.path, source) < 0) + goto cleanup; + + if (VIR_STRDUP(def->source.dir, def->target.path) < 0) + goto cleanup; + + def->type = VIR_STORAGE_POOL_DIR; + + storageDriverLock(driver); + + /* generate a unique name */ + do { + if (pool) { + virStoragePoolObjUnlock(pool); + pool = NULL; + } + + storageEphemeralGenerateUniquePoolID(def); + + if ((pool = virStoragePoolObjFindByUUID(&driver->pools, def->uuid))) + continue; + + pool = virStoragePoolObjFindByName(&driver->pools, def->name); + } while (pool); + + if (!(backend = virStorageBackendForType(def->type))) + goto cleanup; + + if (!(pool = virStoragePoolObjAssignDef(&driver->pools, def))) + goto cleanup; + def = NULL; + + if (backend->startPool && + backend->startPool(conn, pool) < 0) { + virStoragePoolObjRemove(&driver->pools, pool); + pool = NULL; + goto cleanup; + } + + pool->active = 1; + pool->internal = true; + + ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid, + NULL, NULL); + +cleanup: + virStoragePoolDefFree(def); + if (pool) + virStoragePoolObjUnlock(pool); + storageDriverUnlock(driver); + return ret; +} + + +static virStorageVolPtr +storageEphemeralCreateVol(virStoragePoolPtr obj, + const char *source) +{ + + virStorageDriverStatePtr driver = obj->conn->storagePrivateData; + virStoragePoolObjPtr pool; + virStorageBackendPtr backend; + virStorageVolDefPtr vol = NULL; + virStorageVolPtr ret = NULL; + + storageDriverLock(driver); + pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); + storageDriverUnlock(driver); + + if (!pool) { + virReportError(VIR_ERR_NO_STORAGE_POOL, + _("no storage pool with matching uuid %s"), obj->uuid); + goto cleanup; + } + + if ((backend = virStorageBackendForType(pool->def->type)) == NULL) + goto cleanup; + + if (VIR_ALLOC(vol) < 0) + goto cleanup; + + if (VIR_STRDUP(vol->name, source) < 0) + goto cleanup; + + if (backend->createVol(obj->conn, pool, vol, true) < 0) + goto cleanup; + + if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) + goto cleanup; + + if (!(ret = virGetStorageVol(obj->conn, pool->def->name, + pool->volumes.objs[pool->volumes.count - 1]->name, + pool->volumes.objs[pool->volumes.count - 1]->key, + NULL, NULL))) { + vol = pool->volumes.objs[pool->volumes.count--]; + goto cleanup; + } + +cleanup: + virStorageVolDefFree(vol); + if (pool) + virStoragePoolObjUnlock(pool); + return ret; +} + + +static virStorageEphemeralPtr +storageEphemeralCreate(virConnectPtr conn, + int type, + const char *source, + virDomainDiskSourcePoolDefPtr srcpool) +{ + virStorageEphemeralPtr ret = NULL; + char *dirname = NULL; + char *filename = NULL; + + if (VIR_ALLOC(ret) < 0) + goto error; + + switch ((enum virDomainDiskType) type) { + case VIR_DOMAIN_DISK_TYPE_LAST: + case VIR_DOMAIN_DISK_TYPE_BLOCK: + case VIR_DOMAIN_DISK_TYPE_FILE: + case VIR_DOMAIN_DISK_TYPE_DIR: + if (!(dirname = mdir_name(source))) { + virReportOOMError(); + goto error; + } + + if (VIR_STRDUP(filename, last_component(source)) < 1) + goto error; + + if (!(ret->pool = storageEphemeralCreateDirPool(conn, dirname))) + goto error; + + if (!(ret->vol = storageEphemeralCreateVol(ret->pool, filename))) + goto error; + + break; + + case VIR_DOMAIN_DISK_TYPE_NETWORK: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("ephemeral network based volumes are not yet supported")); + goto error; + break; + + case VIR_DOMAIN_DISK_TYPE_VOLUME: + ret->existing = true; + + if (!(ret->pool = virStoragePoolLookupByName(conn, srcpool->pool))) + goto error; + + if (!(ret->vol = virStorageVolLookupByName(ret->pool, srcpool->volume))) + goto error; + + break; + } + +cleanup: + VIR_FREE(dirname); + VIR_FREE(filename); + + return ret; + +error: + storageEphemeralFree(ret); + ret = NULL; + goto cleanup; +} + + +static virStorageEphemeralPtr +storageEphemeralFromDiskDef(virConnectPtr conn, + virDomainDiskDefPtr def) +{ + return storageEphemeralCreate(conn, + def->type, + def->src, + def->srcpool); + +} + + +static virStorageEphemeralPtr +storageEphemeralFromSnapshotDiskDef(virConnectPtr conn, + virDomainSnapshotDiskDefPtr def) +{ + return storageEphemeralCreate(conn, + def->type, + def->file, + NULL); +} + static virStorageDriver storageDriver = { .name = "storage", .storageOpen = storageOpen, /* 0.4.0 */ @@ -2632,6 +2881,10 @@ static virStorageDriver storageDriver = { .storagePoolIsActive = storagePoolIsActive, /* 0.7.3 */ .storagePoolIsPersistent = storagePoolIsPersistent, /* 0.7.3 */ + + .storageEphemeralFree = storageEphemeralFree, /* 1.2.2 */ + .storageEphemeralFromDiskDef = storageEphemeralFromDiskDef, /* 1.2.2 */ + .storageEphemeralFromSnapshotDiskDef = storageEphemeralFromSnapshotDiskDef, /* 1.2.2 */ }; -- 1.8.5.2
On 01/16/2014 06:14 AM, Peter Krempa wrote:
This patch implements the APIs for creating temporary storage pools for the generic storage driver. --- src/check-aclrules.pl | 3 + src/storage/storage_driver.c | 253 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 256 insertions(+)
Didn't spot anything obviously wrong. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Add code to convert a regular guest volume into a ephemeral pool. --- src/storage/storage_driver.c | 154 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 151 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index c82c620..00bf8a5 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2627,6 +2627,107 @@ storageEphemeralGenerateUniquePoolID(virStoragePoolDefPtr def) static virStoragePoolPtr +storageEphemeralCreateGlusterPool(virConnectPtr conn, + const char *source, + virDomainDiskHostDefPtr host) +{ + virStorageDriverStatePtr driver = conn->storagePrivateData; + virStoragePoolDefPtr def = NULL; + virStoragePoolObjPtr pool = NULL; + virStorageBackendPtr backend; + virStoragePoolPtr ret = NULL; + + char *volume = NULL; + char *path = NULL; + char *tmp; + + if (VIR_STRDUP(volume, source) < 0) + goto cleanup; + + if ((tmp = strchr(volume, '/'))) + *tmp++ = '\0'; + + if (tmp && virAsprintf(&path, "/%s", tmp) < 0) + goto cleanup; + + if (VIR_ALLOC(def) < 0) + goto cleanup; + + if (VIR_ALLOC_N(def->source.hosts, 1)) + goto cleanup; + + def->source.nhost = 1; + + if (VIR_STRDUP(def->source.hosts->name, host->name) < 0) + goto cleanup; + + if (host->port && + virStrToLong_i(host->port, NULL, 10, &def->source.hosts->port) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("failed to parse port number '%s'"), + host->port); + goto cleanup; + } + + if (VIR_ALLOC_N(def->name, VIR_UUID_STRING_BUFLEN) < 0) + goto cleanup; + + def->source.name = volume; + volume = NULL; + + def->source.dir = path; + path = NULL; + + def->type = VIR_STORAGE_POOL_GLUSTER; + + storageDriverLock(driver); + + /* generate a unique name */ + do { + if (pool) { + virStoragePoolObjUnlock(pool); + pool = NULL; + } + + storageEphemeralGenerateUniquePoolID(def); + + if ((pool = virStoragePoolObjFindByUUID(&driver->pools, def->uuid))) + continue; + + pool = virStoragePoolObjFindByName(&driver->pools, def->name); + } while (pool); + + if (!(backend = virStorageBackendForType(def->type))) + goto cleanup; + + if (!(pool = virStoragePoolObjAssignDef(&driver->pools, def))) + goto cleanup; + def = NULL; + + if (backend->startPool && + backend->startPool(conn, pool) < 0) { + virStoragePoolObjRemove(&driver->pools, pool); + pool = NULL; + goto cleanup; + } + + pool->active = 1; + pool->internal = true; + + ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid, + NULL, NULL); + +cleanup: + virStoragePoolDefFree(def); + if (pool) + virStoragePoolObjUnlock(pool); + storageDriverUnlock(driver); + return ret; +} + + + +static virStoragePoolPtr storageEphemeralCreateDirPool(virConnectPtr conn, const char *source) { @@ -2752,6 +2853,9 @@ static virStorageEphemeralPtr storageEphemeralCreate(virConnectPtr conn, int type, const char *source, + int protocol, + size_t nhosts, + virDomainDiskHostDefPtr hosts, virDomainDiskSourcePoolDefPtr srcpool) { virStorageEphemeralPtr ret = NULL; @@ -2783,9 +2887,47 @@ storageEphemeralCreate(virConnectPtr conn, break; case VIR_DOMAIN_DISK_TYPE_NETWORK: - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("ephemeral network based volumes are not yet supported")); - goto error; + switch ((enum virDomainDiskProtocol) protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + if (nhosts != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Gluster disk supports only 1 source")); + goto error; + } + + if (!(dirname = mdir_name(source))) { + virReportOOMError(); + goto error; + } + + if (VIR_STRDUP(filename, last_component(source)) < 1) + goto error; + + if (!(ret->pool = storageEphemeralCreateGlusterPool(conn, dirname, hosts))) + goto error; + + if (!(ret->vol = storageEphemeralCreateVol(ret->pool, filename))) + goto error; + + break; + + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: + case VIR_DOMAIN_DISK_PROTOCOL_HTTP: + case VIR_DOMAIN_DISK_PROTOCOL_HTTPS: + case VIR_DOMAIN_DISK_PROTOCOL_FTP: + case VIR_DOMAIN_DISK_PROTOCOL_FTPS: + case VIR_DOMAIN_DISK_PROTOCOL_TFTP: + case VIR_DOMAIN_DISK_PROTOCOL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("ephemeral network based volumes using '%s' " + "protocol are not yet supported"), + virDomainDiskProtocolTypeToString(protocol)); + goto error; + break; + } break; case VIR_DOMAIN_DISK_TYPE_VOLUME: @@ -2820,6 +2962,9 @@ storageEphemeralFromDiskDef(virConnectPtr conn, return storageEphemeralCreate(conn, def->type, def->src, + def->protocol, + def->nhosts, + def->hosts, def->srcpool); } @@ -2832,6 +2977,9 @@ storageEphemeralFromSnapshotDiskDef(virConnectPtr conn, return storageEphemeralCreate(conn, def->type, def->file, + def->protocol, + def->nhosts, + def->hosts, NULL); } -- 1.8.5.2
On 01/16/2014 06:14 AM, Peter Krempa wrote:
Add code to convert a regular guest volume into a ephemeral pool.
s/a ephemeral/an ephemeral/ ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Use the new storage conversion APIs to delete garbage left behind after a failed snapshot attempt using the storage driver. --- src/qemu/qemu_driver.c | 60 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f01014..ede671a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12182,7 +12182,8 @@ cleanup: /* The domain is expected to hold monitor lock. */ static int -qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, +qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainSnapshotDiskDefPtr snap, virDomainDiskDefPtr disk, @@ -12199,6 +12200,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, int ret = -1; int fd = -1; bool need_unlink = false; + virStorageEphemeralPtr temppool = NULL; if (snap->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -12218,6 +12220,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, virStorageFileFreeMetadata(disk->backingChain); disk->backingChain = NULL; + if (!(temppool = virStorageEphemeralFromSnapshotDiskDef(conn, snap))) + goto cleanup; + switch (snap->type) { case VIR_DOMAIN_DISK_TYPE_BLOCK: reuse = true; @@ -12276,8 +12281,16 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, } cleanup: - if (need_unlink && unlink(source)) - VIR_WARN("unable to unlink just-created %s", source); + if (need_unlink) { + if (virStorageVolDelete(temppool->vol, 0) < 0) { + char *path = virStorageVolGetPath(temppool->vol); + VIR_WARN("unable to remove just-created snapshot storage '%s'", + NULLSTR(path)); + VIR_FREE(path); + } + } + + virStorageEphemeralFree(temppool); VIR_FREE(device); VIR_FREE(source); VIR_FREE(persistSource); @@ -12288,7 +12301,8 @@ cleanup: * counterpart to qemuDomainSnapshotCreateSingleDiskActive, called * only on a failed transaction. */ static void -qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, +qemuDomainSnapshotUndoSingleDiskActive(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr origdisk, virDomainDiskDefPtr disk, @@ -12297,17 +12311,33 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, { char *source = NULL; char *persistSource = NULL; - struct stat st; + + virStorageEphemeralPtr temppool = NULL; if (VIR_STRDUP(source, origdisk->src) < 0 || (persistDisk && VIR_STRDUP(persistSource, source) < 0)) goto cleanup; - qemuDomainPrepareDiskChainElement(driver, vm, disk, disk->src, - VIR_DISK_CHAIN_NO_ACCESS); - if (need_unlink && stat(disk->src, &st) == 0 && - S_ISREG(st.st_mode) && unlink(disk->src) < 0) - VIR_WARN("Unable to remove just-created %s", disk->src); + switch (origdisk->type) { + case VIR_DOMAIN_DISK_TYPE_BLOCK: + case -1: /* type was not provided in snapshot conf */ + case VIR_DOMAIN_DISK_TYPE_FILE: + qemuDomainPrepareDiskChainElement(driver, vm, disk, disk->src, + VIR_DISK_CHAIN_NO_ACCESS); + break; + + default: + break; + } + + temppool = virStorageEphemeralFromDiskDef(conn, disk); + + if (need_unlink && temppool && + virStorageVolDelete(temppool->vol, 0) < 0) { + char *path = virStorageVolGetPath(temppool->vol); + VIR_WARN("Unable to remove just-created '%s'", NULLSTR(path)); + VIR_FREE(path); + } /* Update vm in place to match changes. */ VIR_FREE(disk->src); @@ -12324,13 +12354,15 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, } cleanup: + virStorageEphemeralFree(temppool); VIR_FREE(source); VIR_FREE(persistSource); } /* The domain is expected to be locked and active. */ static int -qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, +qemuDomainSnapshotCreateDiskActive(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainSnapshotObjPtr snap, unsigned int flags, @@ -12383,7 +12415,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, } } - ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, + ret = qemuDomainSnapshotCreateSingleDiskActive(conn, driver, vm, &snap->def->disks[i], vm->def->disks[i], persistDisk, actions, @@ -12412,7 +12444,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, persistDisk = vm->newDef->disks[indx]; } - qemuDomainSnapshotUndoSingleDiskActive(driver, vm, + qemuDomainSnapshotUndoSingleDiskActive(conn, driver, vm, snap->def->dom->disks[i], vm->def->disks[i], persistDisk, @@ -12555,7 +12587,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, * * Next we snapshot the disks. */ - if ((ret = qemuDomainSnapshotCreateDiskActive(driver, vm, snap, flags, + if ((ret = qemuDomainSnapshotCreateDiskActive(conn, driver, vm, snap, flags, QEMU_ASYNC_JOB_SNAPSHOT)) < 0) goto endjob; -- 1.8.5.2
On 01/16/2014 06:14 AM, Peter Krempa wrote:
Use the new storage conversion APIs to delete garbage left behind after a failed snapshot attempt using the storage driver. --- src/qemu/qemu_driver.c | 60 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 14 deletions(-)
+ switch (origdisk->type) { + case VIR_DOMAIN_DISK_TYPE_BLOCK: + case -1: /* type was not provided in snapshot conf */
Is this case leftover that can be removed after your earlier refactoring? ACK with that fixed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Add support for gluster backed images as sources for snapshots in the qemu driver. This will also simplify adding further network backed volumes as sources for snapshot in case qemu will support them. --- docs/formatsnapshot.html.in | 4 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 9 ++++ src/qemu/qemu_driver.c | 113 +++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 118 insertions(+), 10 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index c2cd18c..d990c39 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -183,8 +183,8 @@ documentation for further information. Libvirt currently supports the <code>type</code> element in the qemu - driver and supported values are <code>file</code> and - <code>block</code> <span class="since">(since 1.2.2)</span>. + driver and supported values are <code>file</code>, <code>block</code> + and <code>gluster</code> <span class="since">(since 1.2.2)</span>. </dd> </dl> </dd> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 81486df..38e27d1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3822,7 +3822,7 @@ cleanup: } -static int +int qemuGetDriveSourceString(int type, const char *src, int protocol, diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index de7683d..06e0dee 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -312,4 +312,13 @@ qemuParseKeywords(const char *str, int *retnkeywords, int allowEmptyValue); +int qemuGetDriveSourceString(int type, + const char *src, + int protocol, + size_t nhosts, + virDomainDiskHostDefPtr hosts, + const char *username, + const char *secret, + char **path); + #endif /* __QEMU_COMMAND_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ede671a..b3e7cb6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11504,6 +11504,24 @@ cleanup: return ret; } + +static int +qemuDomainSnapshotDiskGetSourceString(virDomainSnapshotDiskDefPtr disk, + char **source) +{ + *source = NULL; + + return qemuGetDriveSourceString(qemuSnapshotDiskGetActualType(disk), + disk->file, + disk->protocol, + disk->nhosts, + disk->hosts, + NULL, + NULL, + source); +} + + typedef enum { VIR_DISK_CHAIN_NO_ACCESS, VIR_DISK_CHAIN_READ_ONLY, @@ -11884,6 +11902,29 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d return 0; case VIR_DOMAIN_DISK_TYPE_NETWORK: + switch ((enum virDomainDiskProtocol) disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + return 0; + + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: + case VIR_DOMAIN_DISK_PROTOCOL_HTTP: + case VIR_DOMAIN_DISK_PROTOCOL_HTTPS: + case VIR_DOMAIN_DISK_PROTOCOL_FTP: + case VIR_DOMAIN_DISK_PROTOCOL_FTPS: + case VIR_DOMAIN_DISK_PROTOCOL_TFTP: + case VIR_DOMAIN_DISK_PROTOCOL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("external active snapshots are not supported on " + "'network' disks using '%s' protocol"), + virDomainDiskProtocolTypeToString(disk->protocol)); + return -1; + + } + break; + case VIR_DOMAIN_DISK_TYPE_DIR: case VIR_DOMAIN_DISK_TYPE_VOLUME: case VIR_DOMAIN_DISK_TYPE_LAST: @@ -12194,6 +12235,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, qemuDomainObjPrivatePtr priv = vm->privateData; char *device = NULL; char *source = NULL; + char *newsource = NULL; + virDomainDiskHostDefPtr newhosts = NULL; + virDomainDiskHostDefPtr persistHosts = NULL; int format = snap->format; const char *formatStr = NULL; char *persistSource = NULL; @@ -12208,8 +12252,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, return -1; } - if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0 || - (persistDisk && VIR_STRDUP(persistSource, source) < 0)) + if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0) goto cleanup; /* XXX Here, we know we are about to alter disk->backingChain if @@ -12223,13 +12266,21 @@ qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, if (!(temppool = virStorageEphemeralFromSnapshotDiskDef(conn, snap))) goto cleanup; + if (qemuDomainSnapshotDiskGetSourceString(snap, &source) < 0) + goto cleanup; + + if (VIR_STRDUP(newsource, snap->file) < 0) + goto cleanup; + + if (persistDisk && + VIR_STRDUP(persistSource, snap->file) < 0) + goto cleanup; + switch (snap->type) { case VIR_DOMAIN_DISK_TYPE_BLOCK: reuse = true; /* fallthrough */ case VIR_DOMAIN_DISK_TYPE_FILE: - if (VIR_STRDUP(source, snap->file) < 0) - goto cleanup; /* create the stub file and set selinux labels; manipulate disk in * place, in a way that can be reverted on failure. */ @@ -12249,6 +12300,27 @@ qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, } break; + case VIR_DOMAIN_DISK_TYPE_NETWORK: + switch (snap->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: + if (!(newhosts = virDomainDiskHostDefCopy(snap->nhosts, snap->hosts))) + goto cleanup; + + if (persistDisk && + !(persistHosts = virDomainDiskHostDefCopy(snap->nhosts, snap->hosts))) + goto cleanup; + + break; + + default: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("snapshots on volumes using '%s' protocol " + "are not supported"), + virDomainDiskProtocolTypeToString(snap->protocol)); + goto cleanup; + } + break; + default: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("snapshots are not supported on '%s' volumes"), @@ -12267,17 +12339,33 @@ qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, /* Update vm in place to match changes. */ need_unlink = false; + VIR_FREE(disk->src); - disk->src = source; - source = NULL; + virDomainDiskHostDefFree(disk->nhosts, disk->hosts); + + disk->src = newsource; disk->format = format; disk->type = snap->type; + disk->protocol = snap->protocol; + disk->nhosts = snap->nhosts; + disk->hosts = newhosts; + + newsource = NULL; + newhosts = NULL; + if (persistDisk) { VIR_FREE(persistDisk->src); + virDomainDiskHostDefFree(persistDisk->nhosts, persistDisk->hosts); + persistDisk->src = persistSource; - persistSource = NULL; persistDisk->format = format; persistDisk->type = snap->type; + persistDisk->protocol = snap->protocol; + persistDisk->nhosts = snap->nhosts; + persistDisk->hosts = persistHosts; + + persistSource = NULL; + persistHosts = NULL; } cleanup: @@ -12293,7 +12381,10 @@ cleanup: virStorageEphemeralFree(temppool); VIR_FREE(device); VIR_FREE(source); + VIR_FREE(newsource); VIR_FREE(persistSource); + virDomainDiskHostDefFree(snap->nhosts, newhosts); + virDomainDiskHostDefFree(snap->nhosts, persistHosts); return ret; } @@ -12345,12 +12436,20 @@ qemuDomainSnapshotUndoSingleDiskActive(virConnectPtr conn, source = NULL; disk->format = origdisk->format; disk->type = origdisk->type; + disk->protocol = origdisk->protocol; + virDomainDiskHostDefFree(disk->nhosts, disk->hosts); + disk->nhosts = origdisk->nhosts; + disk->hosts = virDomainDiskHostDefCopy(origdisk->nhosts, origdisk->hosts); if (persistDisk) { VIR_FREE(persistDisk->src); persistDisk->src = persistSource; persistSource = NULL; persistDisk->format = origdisk->format; persistDisk->type = origdisk->type; + persistDisk->protocol = origdisk->protocol; + virDomainDiskHostDefFree(persistDisk->nhosts, persistDisk->hosts); + persistDisk->nhosts = origdisk->nhosts; + persistDisk->hosts = virDomainDiskHostDefCopy(origdisk->nhosts, origdisk->hosts); } cleanup: -- 1.8.5.2
On 01/16/2014 06:14 AM, Peter Krempa wrote:
Add support for gluster backed images as sources for snapshots in the qemu driver. This will also simplify adding further network backed volumes as sources for snapshot in case qemu will support them. --- docs/formatsnapshot.html.in | 4 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 9 ++++ src/qemu/qemu_driver.c | 113 +++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 118 insertions(+), 10 deletions(-)
Libvirt currently supports the <code>type</code> element in the qemu - driver and supported values are <code>file</code> and - <code>block</code> <span class="since">(since 1.2.2)</span>. + driver and supported values are <code>file</code>, <code>block</code> + and <code>gluster</code> <span class="since">(since 1.2.2)</span>.
Rather, type <code>network</code> with a protocol of gluster.
+ + return qemuGetDriveSourceString(qemuSnapshotDiskGetActualType(disk),
qemuSnapshotDiskGetActualType(disk) is now just a fancy wrapper around disk->type, since 4/9. Do we even need that function, or can we just delete it and simplify all callers? ACK with those fixes. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/27/14 23:40, Eric Blake wrote:
On 01/16/2014 06:14 AM, Peter Krempa wrote:
Add support for gluster backed images as sources for snapshots in the qemu driver. This will also simplify adding further network backed volumes as sources for snapshot in case qemu will support them. --- docs/formatsnapshot.html.in | 4 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 9 ++++ src/qemu/qemu_driver.c | 113 +++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 118 insertions(+), 10 deletions(-)
Libvirt currently supports the <code>type</code> element in the qemu - driver and supported values are <code>file</code> and - <code>block</code> <span class="since">(since 1.2.2)</span>. + driver and supported values are <code>file</code>, <code>block</code> + and <code>gluster</code> <span class="since">(since 1.2.2)</span>.
Rather, type <code>network</code> with a protocol of gluster.
+ + return qemuGetDriveSourceString(qemuSnapshotDiskGetActualType(disk),
qemuSnapshotDiskGetActualType(disk) is now just a fancy wrapper around disk->type, since 4/9. Do we even need that function, or can we just delete it and simplify all callers?
Currently the function isn't useful. I'd like to keep it in case we will support snapshots on type='volume' in the future.
ACK with those fixes.
On 01/16/14 14:14, Peter Krempa wrote:
Peter Krempa (9): storage: Add new argument for createVol backend API storage: gluster: Introduce dummy functions for creating a volume storage: Add internal API to create temporary storage pools and vols snapshot: Add support for specifying snapshot disk backing type snapshot: Test snapshot disk type specification storage: Implement ephemeral storage APIs in the generic storage driver storage: gluster: Support conversion of gluster volumes to temp volumes qemu: snapshot: Switch snapshot file deletion to the new storage API qemu: snapshot: Add support for external active snapshots on gluster
Ping?
participants (2)
-
Eric Blake -
Peter Krempa