[libvirt] [PATCHv6 0/8] Gluster snapshot series

This version was tweaked according to Dan's and Eric's feedback. Peter Krempa (8): conf: Move qemuDiskGetActualType to virDomainDiskGetActualType conf: Move qemuSnapshotDiskGetActualType to virDomainSnapshotDiskGetActualType storage: Add file storage APIs in the default storage driver storage: add file functions for local and block files storage: Add storage file backends for gluster qemu: Switch snapshot deletion to the new API functions qemu: snapshot: Use new APIs to detect presence of existing storage files qemu: snapshot: Add support for external active snapshots on gluster cfg.mk | 1 + docs/formatsnapshot.html.in | 5 +- src/conf/domain_conf.c | 10 ++ src/conf/domain_conf.h | 1 + src/conf/snapshot_conf.c | 7 ++ src/conf/snapshot_conf.h | 2 + src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 8 +- src/qemu/qemu_command.h | 9 ++ src/qemu/qemu_conf.c | 17 --- src/qemu/qemu_conf.h | 4 - src/qemu/qemu_driver.c | 198 ++++++++++++++++++++++++++-------- src/storage/storage_backend.c | 44 ++++++++ src/storage/storage_backend.h | 41 +++++++ src/storage/storage_backend_fs.c | 48 +++++++++ src/storage/storage_backend_fs.h | 2 + src/storage/storage_backend_gluster.c | 139 ++++++++++++++++++++++++ src/storage/storage_backend_gluster.h | 1 + src/storage/storage_driver.c | 145 +++++++++++++++++++++++++ src/storage/storage_driver.h | 32 +++++- tests/Makefile.am | 3 + 21 files changed, 644 insertions(+), 75 deletions(-) -- 1.8.5.3

All the data for getting the actual type is present in the domain config. There is no need to have this function private to the qemu driver and it will be re-used later in other parts of libvirt --- Notes: Version 6: - new in series src/conf/domain_conf.c | 10 ++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 6 +++--- src/qemu/qemu_conf.c | 10 ---------- src/qemu/qemu_conf.h | 2 -- src/qemu/qemu_driver.c | 6 +++--- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f6065ed..fc9615f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1345,6 +1345,16 @@ error: } +int +virDomainDiskGetActualType(virDomainDiskDefPtr def) +{ + if (def->type == VIR_DOMAIN_DISK_TYPE_VOLUME && def->srcpool) + return def->srcpool->actualtype; + + return def->type; +} + + void virDomainControllerDefFree(virDomainControllerDefPtr def) { if (!def) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4895e81..24cbec3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2252,6 +2252,7 @@ void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def); void virDomainDiskHostDefFree(size_t nhosts, virDomainDiskHostDefPtr hosts); virDomainDiskHostDefPtr virDomainDiskHostDefCopy(size_t nhosts, virDomainDiskHostDefPtr hosts); +int virDomainDiskGetActualType(virDomainDiskDefPtr def); int virDomainDeviceFindControllerModel(virDomainDefPtr def, virDomainDeviceInfoPtr info, int controllerType); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0b28bac..aae6a3b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -198,6 +198,7 @@ virDomainDiskErrorPolicyTypeToString; virDomainDiskFindByBusAndDst; virDomainDiskGeometryTransTypeFromString; virDomainDiskGeometryTransTypeToString; +virDomainDiskGetActualType; virDomainDiskHostDefClear; virDomainDiskHostDefCopy; virDomainDiskHostDefFree; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e499d54..e9908f9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3869,7 +3869,7 @@ qemuDomainDiskGetSourceString(virConnectPtr conn, virDomainDiskDefPtr disk, char **source) { - int actualType = qemuDiskGetActualType(disk); + int actualType = virDomainDiskGetActualType(disk); char *secret = NULL; int ret = -1; @@ -3899,7 +3899,7 @@ qemuDomainDiskGetSourceString(virConnectPtr conn, goto cleanup; } - ret = qemuGetDriveSourceString(qemuDiskGetActualType(disk), + ret = qemuGetDriveSourceString(virDomainDiskGetActualType(disk), disk->src, disk->protocol, disk->nhosts, @@ -3927,7 +3927,7 @@ qemuBuildDriveStr(virConnectPtr conn, int idx = virDiskNameToIndex(disk->dst); int busid = -1, unitid = -1; char *source = NULL; - int actualType = qemuDiskGetActualType(disk); + int actualType = virDomainDiskGetActualType(disk); if (idx < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index ecaaf81..dc4ded3 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1304,16 +1304,6 @@ cleanup: int -qemuDiskGetActualType(virDomainDiskDefPtr def) -{ - if (def->type == VIR_DOMAIN_DISK_TYPE_VOLUME && def->srcpool) - return def->srcpool->actualtype; - - return def->type; -} - - -int qemuTranslateDiskSourcePool(virConnectPtr conn, virDomainDiskDefPtr def) { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 1f44a76..842b740 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -305,8 +305,6 @@ int qemuSetUnprivSGIO(virDomainDeviceDefPtr dev); int qemuDriverAllocateID(virQEMUDriverPtr driver); virDomainXMLOptionPtr virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver); -int qemuDiskGetActualType(virDomainDiskDefPtr def); - int qemuTranslateDiskSourcePool(virConnectPtr conn, virDomainDiskDefPtr def); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 59e018d..6ddbd21 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12260,7 +12260,7 @@ endjob: static int qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) { - int actualType = qemuDiskGetActualType(disk); + int actualType = virDomainDiskGetActualType(disk); switch ((enum virDomainDiskType) actualType) { case VIR_DOMAIN_DISK_TYPE_BLOCK: @@ -12304,7 +12304,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) static int qemuDomainSnapshotPrepareDiskExternalBackingActive(virDomainDiskDefPtr disk) { - int actualType = qemuDiskGetActualType(disk); + int actualType = virDomainDiskGetActualType(disk); if (actualType == VIR_DOMAIN_DISK_TYPE_BLOCK && disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { @@ -12448,7 +12448,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn, if (qemuTranslateDiskSourcePool(conn, disk) < 0) return -1; - actualType = qemuDiskGetActualType(disk); + actualType = virDomainDiskGetActualType(disk); switch ((enum virDomainDiskType) actualType) { case VIR_DOMAIN_DISK_TYPE_BLOCK: -- 1.8.5.3

On 02/13/2014 09:49 AM, Peter Krempa wrote:
All the data for getting the actual type is present in the domain config. There is no need to have this function private to the qemu driver and it will be re-used later in other parts of libvirt ---
Notes: Version 6: - new in series
src/conf/domain_conf.c | 10 ++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 6 +++--- src/qemu/qemu_conf.c | 10 ---------- src/qemu/qemu_conf.h | 2 -- src/qemu/qemu_driver.c | 6 +++--- 7 files changed, 18 insertions(+), 18 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

All the data for getting the actual type is present in the snapshot config. There is no need to have this function private to the qemu driver and it will be re-used later in other parts of libvirt --- Notes: Version 6: - new in series src/conf/snapshot_conf.c | 7 +++++++ src/conf/snapshot_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 7 ------- src/qemu/qemu_conf.h | 2 -- src/qemu/qemu_driver.c | 6 +++--- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index bb732a1..12b0930 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -1291,3 +1291,10 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, cleanup: return ret; } + + +int +virDomainSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def) +{ + return def->type; +} diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index bcd92dc..59c6d92 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -187,6 +187,8 @@ int virDomainSnapshotRedefinePrep(virDomainPtr domain, bool *update_current, unsigned int flags); +int virDomainSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def); + VIR_ENUM_DECL(virDomainSnapshotLocation) VIR_ENUM_DECL(virDomainSnapshotState) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index aae6a3b..67d20ed 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -659,6 +659,7 @@ virDomainSnapshotDefFormat; virDomainSnapshotDefFree; virDomainSnapshotDefIsExternal; virDomainSnapshotDefParseString; +virDomainSnapshotDiskGetActualType; virDomainSnapshotDropParent; virDomainSnapshotFindByName; virDomainSnapshotForEach; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index dc4ded3..2c397b0 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1465,13 +1465,6 @@ cleanup: int -qemuSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def) -{ - return def->type; -} - - -int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainSnapshotDiskDefPtr def) { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 842b740..c181dc2 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -308,8 +308,6 @@ virDomainXMLOptionPtr virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver); int qemuTranslateDiskSourcePool(virConnectPtr conn, virDomainDiskDefPtr def); -int qemuSnapshotDiskGetActualType(virDomainSnapshotDiskDefPtr def); - int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn, virDomainSnapshotDiskDefPtr def); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6ddbd21..aa2e178 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12321,7 +12321,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingActive(virDomainDiskDefPtr disk) static int qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr disk) { - int actualType = qemuSnapshotDiskGetActualType(disk); + int actualType = virDomainSnapshotDiskGetActualType(disk); switch ((enum virDomainDiskType) actualType) { case VIR_DOMAIN_DISK_TYPE_BLOCK: @@ -12345,7 +12345,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d static int qemuDomainSnapshotPrepareDiskExternalOverlayInactive(virDomainSnapshotDiskDefPtr disk) { - int actualType = qemuSnapshotDiskGetActualType(disk); + int actualType = virDomainSnapshotDiskGetActualType(disk); switch ((enum virDomainDiskType) actualType) { case VIR_DOMAIN_DISK_TYPE_BLOCK: @@ -12397,7 +12397,7 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn, return -1; } - actualType = qemuSnapshotDiskGetActualType(snapdisk); + actualType = virDomainSnapshotDiskGetActualType(snapdisk); switch ((enum virDomainDiskType) actualType) { case VIR_DOMAIN_DISK_TYPE_BLOCK: -- 1.8.5.3

On 02/13/2014 09:49 AM, Peter Krempa wrote:
All the data for getting the actual type is present in the snapshot config. There is no need to have this function private to the qemu driver and it will be re-used later in other parts of libvirt ---
Notes: Version 6: - new in series
src/conf/snapshot_conf.c | 7 +++++++ src/conf/snapshot_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 7 ------- src/qemu/qemu_conf.h | 2 -- src/qemu/qemu_driver.c | 6 +++--- 6 files changed, 13 insertions(+), 12 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Add APIs that will allow to use the storage driver to assist in operations on files even for remote filesystems without native representation as files in the host. --- Notes: Version 6: - rewrite due to change of exporting approach Version 5: - adapt to error reporting change src/storage/storage_backend.c | 37 +++++++++++ src/storage/storage_backend.h | 41 ++++++++++++ src/storage/storage_driver.c | 145 ++++++++++++++++++++++++++++++++++++++++++ src/storage/storage_driver.h | 32 +++++++++- tests/Makefile.am | 3 + 5 files changed, 257 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 19fb1f0..b59b5b7 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -121,6 +121,12 @@ static virStorageBackendPtr backends[] = { NULL }; + +static virStorageFileBackendPtr fileBackends[] = { + NULL +}; + + enum { TOOL_QEMU_IMG, TOOL_KVM_IMG, @@ -1152,6 +1158,37 @@ virStorageBackendForType(int type) } +virStorageFileBackendPtr +virStorageFileBackendForType(int type, + int protocol) +{ + size_t i; + + for (i = 0; fileBackends[i]; i++) { + if (fileBackends[i]->type == type) { + if (type == VIR_DOMAIN_DISK_TYPE_NETWORK && + fileBackends[i]->protocol != protocol) + continue; + + return fileBackends[i]; + } + } + + if (type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing storage backend for network files " + "using %s protocol"), + virDomainDiskProtocolTypeToString(protocol)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing storage backend for '%s' storage"), + virDomainDiskTypeToString(type)); + } + + return NULL; +} + + /* * Allows caller to silently ignore files with improper mode * diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 378bc4d..1c7ad1e 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -29,6 +29,7 @@ # include "internal.h" # include "storage_conf.h" # include "vircommand.h" +# include "storage_driver.h" typedef char * (*virStorageBackendFindPoolSources)(virConnectPtr conn, const char *srcSpec, @@ -189,4 +190,44 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, const char *create_tool, int imgformat); +/* ------- virStorageFile backends ------------ */ +typedef int +(*virStorageFileBackendInit)(virStorageFilePtr file); + +typedef void +(*virStorageFileBackendDeinit)(virStorageFilePtr file); + +typedef int +(*virStorageFileBackendCreate)(virStorageFilePtr file); + +typedef int +(*virStorageFileBackendUnlink)(virStorageFilePtr file); + +typedef int +(*virStorageFileBackendStat)(virStorageFilePtr file, + struct stat *st); + +typedef struct _virStorageFileBackend virStorageFileBackend; +typedef virStorageFileBackend *virStorageFileBackendPtr; + +virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol); + +struct _virStorageFileBackend { + int type; + int protocol; + + /* All storage file callbacks may be omitted if not implemented */ + + /* The following group of callbacks is expected to set a libvirt + * error on failure. */ + virStorageFileBackendInit backendInit; + virStorageFileBackendDeinit backendDeinit; + + /* The following group of callbacks is expected to set errno + * and return -1 on error. No libvirt error shall be reported */ + virStorageFileBackendCreate storageFileCreate; + virStorageFileBackendUnlink storageFileUnlink; + virStorageFileBackendStat storageFileStat; +}; + #endif /* __VIR_STORAGE_BACKEND_H__ */ diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 2f7b2e5..64451c5 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2589,6 +2589,7 @@ cleanup: return ret; } + static virStorageDriver storageDriver = { .name = "storage", .storageOpen = storageOpen, /* 0.4.0 */ @@ -2653,3 +2654,147 @@ int storageRegister(void) virRegisterStateDriver(&stateDriver); return 0; } + + +/* ----------- file handlers cooperating with storage driver --------------- */ +void +virStorageFileFree(virStorageFilePtr file) +{ + if (!file) + return; + + if (file->backend && + file->backend->backendDeinit) + file->backend->backendDeinit(file); + + VIR_FREE(file->path); + virDomainDiskHostDefFree(file->nhosts, file->hosts); + VIR_FREE(file); +} + + +static virStorageFilePtr +virStorageFileInitInternal(int type, + const char *path, + int protocol, + size_t nhosts, + virDomainDiskHostDefPtr hosts) +{ + virStorageFilePtr file = NULL; + + if (VIR_ALLOC(file) < 0) + return NULL; + + file->type = type; + file->protocol = protocol; + file->nhosts = nhosts; + + if (VIR_STRDUP(file->path, path) < 0) + goto error; + + if (!(file->hosts = virDomainDiskHostDefCopy(nhosts, hosts))) + goto error; + + if (!(file->backend = virStorageFileBackendForType(file->type, + file->protocol))) + goto error; + + if (file->backend->backendInit && + file->backend->backendInit(file) < 0) + goto error; + + return file; + +error: + VIR_FREE(file->path); + virDomainDiskHostDefFree(file->nhosts, file->hosts); + VIR_FREE(file); + return NULL; +} + + +virStorageFilePtr +virStorageFileInitFromDiskDef(virDomainDiskDefPtr disk) +{ + return virStorageFileInitInternal(virDomainDiskGetActualType(disk), + disk->src, + disk->protocol, + disk->nhosts, + disk->hosts); +} + + +virStorageFilePtr +virStorageFileInitFromSnapshotDef(virDomainSnapshotDiskDefPtr disk) +{ + return virStorageFileInitInternal(virDomainSnapshotDiskGetActualType(disk), + disk->file, + disk->protocol, + disk->nhosts, + disk->hosts); +} + + + +/** + * virStorageFileCreate: Creates an empty storage file via storage driver + * + * @file: file structure pointing to the file + * + * Returns 0 on success, -2 if the function isn't supported by the backend, + * -1 on other failure. Errno is set in case of failure. + */ +int +virStorageFileCreate(virStorageFilePtr file) +{ + if (!file->backend->storageFileCreate) { + errno = ENOSYS; + return -2; + } + + return file->backend->storageFileCreate(file); +} + + +/** + * virStorageFileUnlink: Unlink storage file via storage driver + * + * @file: file structure pointing to the file + * + * Unlinks the file described by the @file structure. + * + * Returns 0 on success, -2 if the function isn't supported by the backend, + * -1 on other failure. Errno is set in case of failure. + */ +int +virStorageFileUnlink(virStorageFilePtr file) +{ + if (!file->backend->storageFileUnlink) { + errno = ENOSYS; + return -2; + } + + return file->backend->storageFileUnlink(file); +} + + +/** + * virStorageFileStat: returns stat struct of a file via storage driver + * + * @file: file structure pointing to the file + * @stat: stat structure to return data + * + * Returns 0 on success, -2 if the function isn't supported by the backend, + * -1 on other failure. Errno is set in case of failure. +*/ +int +virStorageFileStat(virStorageFilePtr file, + struct stat *st) +{ + if (!(file->backend->storageFileStat)) { + errno = ENOSYS; + return -2; + } + + return file->backend->storageFileStat(file, st); +} diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index 8ccfd75..993bba5 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -1,7 +1,7 @@ /* * storage_driver.h: core driver for storage APIs * - * Copyright (C) 2006-2008 Red Hat, Inc. + * Copyright (C) 2006-2008, 2014 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -25,6 +25,36 @@ # define __VIR_STORAGE_DRIVER_H__ # include "storage_conf.h" +# include "conf/domain_conf.h" +# include "conf/snapshot_conf.h" + +typedef struct _virStorageFileBackend virStorageFileBackend; +typedef virStorageFileBackend *virStorageFileBackendPtr; + +typedef struct _virStorageFile virStorageFile; +typedef virStorageFile *virStorageFilePtr; +struct _virStorageFile { + virStorageFileBackendPtr backend; + void *priv; + + char *path; + int type; + int protocol; + + size_t nhosts; + virDomainDiskHostDefPtr hosts; +}; + +virStorageFilePtr +virStorageFileInitFromDiskDef(virDomainDiskDefPtr disk); +virStorageFilePtr +virStorageFileInitFromSnapshotDef(virDomainSnapshotDiskDefPtr disk); +void virStorageFileFree(virStorageFilePtr file); + +int virStorageFileCreate(virStorageFilePtr file); +int virStorageFileUnlink(virStorageFilePtr file); +int virStorageFileStat(virStorageFilePtr file, + struct stat *stat); int storageRegister(void); diff --git a/tests/Makefile.am b/tests/Makefile.am index 62a19cb..0718a69 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -433,6 +433,9 @@ qemu_LDADDS = ../src/libvirt_driver_qemu_impl.la if WITH_NETWORK qemu_LDADDS += ../src/libvirt_driver_network_impl.la endif WITH_NETWORK +if WITH_STORAGE +qemu_LDADDS += ../src/libvirt_driver_storage_impl.la +endif WITH_STORAGE if WITH_DTRACE_PROBES qemu_LDADDS += ../src/libvirt_qemu_probes.lo endif WITH_DTRACE_PROBES -- 1.8.5.3

On 02/13/2014 09:49 AM, Peter Krempa wrote:
Add APIs that will allow to use the storage driver to assist in operations on files even for remote filesystems without native representation as files in the host. ---
Notes: Version 6: - rewrite due to change of exporting approach
Version 5: - adapt to error reporting change
src/storage/storage_backend.c | 37 +++++++++++ src/storage/storage_backend.h | 41 ++++++++++++ src/storage/storage_driver.c | 145 ++++++++++++++++++++++++++++++++++++++++++ src/storage/storage_driver.h | 32 +++++++++- tests/Makefile.am | 3 + 5 files changed, 257 insertions(+), 1 deletion(-)
+struct _virStorageFileBackend { + int type; + int protocol; + + /* All storage file callbacks may be omitted if not implemented */ + + /* The following group of callbacks is expected to set a libvirt + * error on failure. */ + virStorageFileBackendInit backendInit; + virStorageFileBackendDeinit backendDeinit; + + /* The following group of callbacks is expected to set errno + * and return -1 on error. No libvirt error shall be reported */ + virStorageFileBackendCreate storageFileCreate; + virStorageFileBackendUnlink storageFileUnlink; + virStorageFileBackendStat storageFileStat;
Thanks; the comments really help. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Implement the "stat" and "unlink" function for "file" volumes and "stat" for "block" volumes using the regular system calls. --- Notes: Version 6: - tweaked error message style - made sure that libvirt compiles cleanly with --without-storage-fs (added ifdefs) Version 5: - added debug messages - adapted to error reporting changes Version 4: - adapt to change in error reporting Version 6: - tweaked error message style Version 5: - added debug messages - adapted to error reporting changes Version 4: - adapt to change in error reporting src/storage/storage_backend.c | 4 ++++ src/storage/storage_backend_fs.c | 48 ++++++++++++++++++++++++++++++++++++++++ src/storage/storage_backend_fs.h | 2 ++ 3 files changed, 54 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index b59b5b7..aa12c5a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -123,6 +123,10 @@ static virStorageBackendPtr backends[] = { static virStorageFileBackendPtr fileBackends[] = { +#if WITH_STORAGE_FS + &virStorageFileBackendFile, + &virStorageFileBackendBlock, +#endif NULL }; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index fa11e2f..4d69f74 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1327,4 +1327,52 @@ virStorageBackend virStorageBackendNetFileSystem = { .deleteVol = virStorageBackendFileSystemVolDelete, .resizeVol = virStorageBackendFileSystemVolResize, }; + + +static int +virStorageFileBackendFileUnlink(virStorageFilePtr file) +{ + int ret; + + ret = unlink(file->path); + /* preserve errno */ + + VIR_DEBUG("removing storage file %p(%s): ret=%d, errno=%d", + file, file->path, ret, errno); + + return ret; +} + + +static int +virStorageFileBackendFileStat(virStorageFilePtr file, + struct stat *st) +{ + int ret; + + ret = stat(file->path, st); + /* preserve errno */ + + VIR_DEBUG("stat of storage file %p(%s): ret=%d, errno=%d", + file, file->path, ret, errno); + + return ret; +} + + +virStorageFileBackend virStorageFileBackendFile = { + .type = VIR_DOMAIN_DISK_TYPE_FILE, + + .storageFileUnlink = virStorageFileBackendFileUnlink, + .storageFileStat = virStorageFileBackendFileStat, +}; + + +virStorageFileBackend virStorageFileBackendBlock = { + .type = VIR_DOMAIN_DISK_TYPE_BLOCK, + + .storageFileStat = virStorageFileBackendFileStat, +}; + + #endif /* WITH_STORAGE_FS */ diff --git a/src/storage/storage_backend_fs.h b/src/storage/storage_backend_fs.h index a519b38..347ea9b 100644 --- a/src/storage/storage_backend_fs.h +++ b/src/storage/storage_backend_fs.h @@ -38,4 +38,6 @@ typedef enum { } virStoragePoolProbeResult; extern virStorageBackend virStorageBackendDirectory; +extern virStorageFileBackend virStorageFileBackendFile; +extern virStorageFileBackend virStorageFileBackendBlock; #endif /* __VIR_STORAGE_BACKEND_FS_H__ */ -- 1.8.5.3

On 02/13/2014 09:49 AM, Peter Krempa wrote:
Implement the "stat" and "unlink" function for "file" volumes and "stat" for "block" volumes using the regular system calls. ---
Notes: Version 6: - tweaked error message style - made sure that libvirt compiles cleanly with --without-storage-fs (added ifdefs)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Implement storage backend functions to deal with gluster volumes and implement the "stat" and "unlink" backend APIs. --- Notes: Version 6: - tweaked error message style - tweak of private data access due to simplification of export hierarchy Version 5: - adapted to error reporting changes - tweaked error message src/storage/storage_backend.c | 3 + src/storage/storage_backend_gluster.c | 139 ++++++++++++++++++++++++++++++++++ src/storage/storage_backend_gluster.h | 1 + 3 files changed, 143 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index aa12c5a..f7b91ac 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -127,6 +127,9 @@ static virStorageFileBackendPtr fileBackends[] = { &virStorageFileBackendFile, &virStorageFileBackendBlock, #endif +#if WITH_STORAGE_GLUSTER + &virStorageFileBackendGluster, +#endif NULL }; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index d7a1553..53cef78 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -477,3 +477,142 @@ virStorageBackend virStorageBackendGluster = { .deleteVol = virStorageBackendGlusterVolDelete, }; + + +typedef struct _virStorageFileBackendGlusterPriv virStorageFileBackendGlusterPriv; +typedef virStorageFileBackendGlusterPriv *virStorageFileBackendGlusterPrivPtr; + +struct _virStorageFileBackendGlusterPriv { + glfs_t *vol; + char *volname; + char *path; +}; + + +static void +virStorageFileBackendGlusterDeinit(virStorageFilePtr file) +{ + VIR_DEBUG("deinitializing gluster storage file %p(%s/%s)", + file, file->hosts[0].name, file->path); + virStorageFileBackendGlusterPrivPtr priv = file->priv; + + glfs_fini(priv->vol); + VIR_FREE(priv->volname); + + VIR_FREE(priv); + file->priv = NULL; +} + +static int +virStorageFileBackendGlusterInit(virStorageFilePtr file) +{ + virStorageFileBackendGlusterPrivPtr priv = NULL; + virDomainDiskHostDefPtr host = &(file->hosts[0]); + const char *hostname = host->name; + int port = 0; + + VIR_DEBUG("initializing gluster storage file %p(%s/%s)", + file, hostname, file->path); + + if (VIR_ALLOC(priv) < 0) + return -1; + + if (VIR_STRDUP(priv->volname, file->path) < 0) + goto error; + + if (!(priv->path = strchr(priv->volname, '/'))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid path of gluster volume: '%s'"), + file->path); + goto error; + } + + *priv->path = '\0'; + priv->path++; + + if (host->port && + virStrToLong_i(host->port, NULL, 10, &port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse port number '%s'"), + host->port); + goto error; + } + + if (host->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) + hostname = host->socket; + + + if (!(priv->vol = glfs_new(priv->volname))) { + virReportOOMError(); + goto error; + } + + if (glfs_set_volfile_server(priv->vol, + virDomainDiskProtocolTransportTypeToString(host->transport), + hostname, port) < 0) { + virReportSystemError(errno, + _("failed to set gluster volfile server '%s'"), + hostname); + goto error; + } + + if (glfs_init(priv->vol) < 0) { + virReportSystemError(errno, + _("failed to initialize gluster connection to " + "server: '%s'"), hostname); + goto error; + } + + file->priv = priv; + + return 0; + +error: + VIR_FREE(priv->volname); + glfs_fini(priv->vol); + + return -1; +} + + +static int +virStorageFileBackendGlusterUnlink(virStorageFilePtr file) +{ + virStorageFileBackendGlusterPrivPtr priv = file->priv; + int ret; + + ret = glfs_unlink(priv->vol, priv->path); + /* preserve errno */ + + VIR_DEBUG("removing storage file %p(%s/%s): ret=%d, errno=%d", + file, file->hosts[0].name, file->path, ret, errno); + return ret; +} + + +static int +virStorageFileBackendGlusterStat(virStorageFilePtr file, + struct stat *st) +{ + virStorageFileBackendGlusterPrivPtr priv = file->priv; + int ret; + + ret = glfs_stat(priv->vol, priv->path, st); + /* preserve errno */ + + VIR_DEBUG("stat of storage file %p(%s/%s): ret=%d, errno=%d", + file, file->hosts[0].name, file->path, ret, errno); + return ret; +} + + +virStorageFileBackend virStorageFileBackendGluster = { + .type = VIR_DOMAIN_DISK_TYPE_NETWORK, + .protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER, + + .backendInit = virStorageFileBackendGlusterInit, + .backendDeinit = virStorageFileBackendGlusterDeinit, + + .storageFileUnlink = virStorageFileBackendGlusterUnlink, + .storageFileStat = virStorageFileBackendGlusterStat, +}; diff --git a/src/storage/storage_backend_gluster.h b/src/storage/storage_backend_gluster.h index b21bda7..6796016 100644 --- a/src/storage/storage_backend_gluster.h +++ b/src/storage/storage_backend_gluster.h @@ -25,5 +25,6 @@ # include "storage_backend.h" extern virStorageBackend virStorageBackendGluster; +extern virStorageFileBackend virStorageFileBackendGluster; #endif /* __VIR_STORAGE_BACKEND_GLUSTER_H__ */ -- 1.8.5.3

On 02/13/2014 09:49 AM, Peter Krempa wrote:
Implement storage backend functions to deal with gluster volumes and implement the "stat" and "unlink" backend APIs. ---
Notes: Version 6: - tweaked error message style - tweak of private data access due to simplification of export hierarchy
+ if (VIR_STRDUP(priv->volname, file->path) < 0) + goto error; + + if (!(priv->path = strchr(priv->volname, '/'))) {
Spurious double space ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Coverity has found a RESOURCE_LEAK... <...snip...>
+static int +virStorageFileBackendGlusterInit(virStorageFilePtr file) +{ + virStorageFileBackendGlusterPrivPtr priv = NULL; + virDomainDiskHostDefPtr host = &(file->hosts[0]); + const char *hostname = host->name; + int port = 0; + + VIR_DEBUG("initializing gluster storage file %p(%s/%s)", + file, hostname, file->path); + + if (VIR_ALLOC(priv) < 0) + return -1; +
We have 'priv' here... but anywhere through to "error:" if we fail, then priv isn't free'd.
+ if (VIR_STRDUP(priv->volname, file->path) < 0) + goto error; + + if (!(priv->path = strchr(priv->volname, '/'))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid path of gluster volume: '%s'"), + file->path); + goto error; + } + + *priv->path = '\0'; + priv->path++; + + if (host->port && + virStrToLong_i(host->port, NULL, 10, &port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse port number '%s'"), + host->port); + goto error; + } + + if (host->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) + hostname = host->socket; + + + if (!(priv->vol = glfs_new(priv->volname))) { + virReportOOMError(); + goto error; + } + + if (glfs_set_volfile_server(priv->vol, + virDomainDiskProtocolTransportTypeToString(host->transport), + hostname, port) < 0) { + virReportSystemError(errno, + _("failed to set gluster volfile server '%s'"), + hostname); + goto error; + } + + if (glfs_init(priv->vol) < 0) { + virReportSystemError(errno, + _("failed to initialize gluster connection to " + "server: '%s'"), hostname); + goto error; + } + + file->priv = priv; + + return 0; + +error: + VIR_FREE(priv->volname); + glfs_fini(priv->vol);
Adding the free here would be the salve for Coverity. John
+ + return -1; +} +

On 02/14/14 12:24, John Ferlan wrote: ...
+error: + VIR_FREE(priv->volname); + glfs_fini(priv->vol);
Adding the free here would be the salve for Coverity.
Thanks for reporting the issue. This is now fixed by commit commit ad95fa59572c99c26959e6808a1e779a4ffed6de Author: Peter Krempa <pkrempa@redhat.com> Date: Fri Feb 14 13:08:39 2014 +0100 storage: gluster: Don't leak private data when storage file init fails
John
Peter

Use the new storage driver APIs to delete snapshot backing files in case of failure instead of directly relying on "unlink". This will help us in the future when we will be adding network based storage without local representation in the host. --- Notes: Version 6: - simplified as the storage driver now doesn't require "conn" Version 5: - no change, wasn't reviewed yet cfg.mk | 1 + src/qemu/qemu_driver.c | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/cfg.mk b/cfg.mk index 207dfeb..56d7a5b 100644 --- a/cfg.mk +++ b/cfg.mk @@ -764,6 +764,7 @@ sc_prohibit_cross_inclusion: cpu/ | locking/ | network/ | rpc/ | security/) \ safe="($$dir|util|conf)";; \ xenapi/ | xenxs/ ) safe="($$dir|util|conf|xen)";; \ + qemu/ ) safe="($$dir|util|conf|cpu|network|locking|rpc|security|storage)";; \ *) safe="($$dir|util|conf|cpu|network|locking|rpc|security)";; \ esac; \ in_vc_files="^src/$$dir" \ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index aa2e178..5199bf8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -94,6 +94,7 @@ #include "virstring.h" #include "viraccessapicheck.h" #include "viraccessapicheckqemu.h" +#include "storage/storage_driver.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -12648,6 +12649,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, int ret = -1; int fd = -1; bool need_unlink = false; + virStorageFilePtr snapfile = NULL; if (snap->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -12667,6 +12669,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, virStorageFileFreeMetadata(disk->backingChain); disk->backingChain = NULL; + if (!(snapfile = virStorageFileInitFromSnapshotDef(snap))) + goto cleanup; + switch (snap->type) { case VIR_DOMAIN_DISK_TYPE_BLOCK: reuse = true; @@ -12742,8 +12747,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, } cleanup: - if (need_unlink && unlink(source)) + if (need_unlink && virStorageFileUnlink(snapfile)) VIR_WARN("unable to unlink just-created %s", source); + virStorageFileFree(snapfile); VIR_FREE(device); VIR_FREE(source); VIR_FREE(persistSource); @@ -12763,16 +12769,20 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, { char *source = NULL; char *persistSource = NULL; + virStorageFilePtr diskfile = NULL; struct stat st; + diskfile = virStorageFileInitFromDiskDef(disk); + 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) + if (need_unlink && diskfile && + virStorageFileStat(diskfile, &st) == 0 && S_ISREG(st.st_mode) && + virStorageFileUnlink(diskfile) < 0) VIR_WARN("Unable to remove just-created %s", disk->src); /* Update vm in place to match changes. */ @@ -12790,6 +12800,7 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, } cleanup: + virStorageFileFree(diskfile); VIR_FREE(source); VIR_FREE(persistSource); } -- 1.8.5.3

On 02/13/2014 09:49 AM, Peter Krempa wrote:
Use the new storage driver APIs to delete snapshot backing files in case of failure instead of directly relying on "unlink". This will help us in the future when we will be adding network based storage without local representation in the host. ---
Notes: Version 6: - simplified as the storage driver now doesn't require "conn"
+++ b/cfg.mk @@ -764,6 +764,7 @@ sc_prohibit_cross_inclusion: cpu/ | locking/ | network/ | rpc/ | security/) \ safe="($$dir|util|conf)";; \ xenapi/ | xenxs/ ) safe="($$dir|util|conf|xen)";; \ + qemu/ ) safe="($$dir|util|conf|cpu|network|locking|rpc|security|storage)";; \ *) safe="($$dir|util|conf|cpu|network|locking|rpc|security)";; \
We could probably add storage as safe for ALL drivers to use, not just qemu; but I'm okay with the approach you took. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Use the new storage driver based "stat" api to detect exiting files just as we did with local files. --- Notes: Version 6: - tweaked, as "conn" is not needed Version 5: - new in sereis src/qemu/qemu_driver.c | 58 ++++++++++++++++++++++---------------------------- 1 file changed, 26 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5199bf8..04cd13b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12367,7 +12367,6 @@ qemuDomainSnapshotPrepareDiskExternalOverlayInactive(virDomainSnapshotDiskDefPtr } - static int qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn, virDomainDiskDefPtr disk, @@ -12375,7 +12374,8 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn, bool active, bool reuse) { - int actualType; + virStorageFilePtr snapfile = NULL; + int ret = -1; struct stat st; if (qemuTranslateSnapshotDiskSourcePool(conn, snapdisk) < 0) @@ -12398,40 +12398,34 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn, return -1; } - actualType = virDomainSnapshotDiskGetActualType(snapdisk); + if (!(snapfile = virStorageFileInitFromSnapshotDef(snapdisk))) + return -1; - switch ((enum virDomainDiskType) actualType) { - case VIR_DOMAIN_DISK_TYPE_BLOCK: - case VIR_DOMAIN_DISK_TYPE_FILE: - if (stat(snapdisk->file, &st) < 0) { - if (errno != ENOENT) { - virReportSystemError(errno, - _("unable to stat for disk %s: %s"), - snapdisk->name, snapdisk->file); - return -1; - } else if (reuse) { - virReportSystemError(errno, - _("missing existing file for disk %s: %s"), - snapdisk->name, snapdisk->file); - return -1; - } - } else if (!S_ISBLK(st.st_mode) && st.st_size && !reuse) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("external snapshot file for disk %s already " - "exists and is not a block device: %s"), - snapdisk->name, snapdisk->file); - return -1; + if (virStorageFileStat(snapfile, &st) < 0) { + if (errno != ENOENT) { + virReportSystemError(errno, + _("unable to stat for disk %s: %s"), + snapdisk->name, snapdisk->file); + goto cleanup; + } else if (reuse) { + virReportSystemError(errno, + _("missing existing file for disk %s: %s"), + snapdisk->name, snapdisk->file); + goto cleanup; } - break; - - case VIR_DOMAIN_DISK_TYPE_NETWORK: - case VIR_DOMAIN_DISK_TYPE_DIR: - case VIR_DOMAIN_DISK_TYPE_VOLUME: - case VIR_DOMAIN_DISK_TYPE_LAST: - break; + } else if (!S_ISBLK(st.st_mode) && st.st_size && !reuse) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("external snapshot file for disk %s already " + "exists and is not a block device: %s"), + snapdisk->name, snapdisk->file); + goto cleanup; } - return 0; + ret = 0; + +cleanup: + virStorageFileFree(snapfile); + return ret; } -- 1.8.5.3

On 02/13/2014 09:49 AM, Peter Krempa wrote:
Use the new storage driver based "stat" api to detect exiting files just as we did with local files. ---
Notes: Version 6: - tweaked, as "conn" is not needed Version 5: - new in sereis
ACK -- 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. --- Notes: Version 6: - fixed after rename of qemuSnapshotDiskGetActualType Version 5: - no change, wasn't reviewed yet docs/formatsnapshot.html.in | 5 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 9 ++++ src/qemu/qemu_driver.c | 113 +++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 119 insertions(+), 10 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 44ed4fd..1f9a6c6 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -183,8 +183,9 @@ 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>network</code> with a protocol of <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 e9908f9..bb8756f 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 82ca4b3..ebb0b1d 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -313,4 +313,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 04cd13b..6ea837e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11933,6 +11933,24 @@ cleanup: return ret; } + +static int +qemuDomainSnapshotDiskGetSourceString(virDomainSnapshotDiskDefPtr disk, + char **source) +{ + *source = NULL; + + return qemuGetDriveSourceString(virDomainSnapshotDiskGetActualType(disk), + disk->file, + disk->protocol, + disk->nhosts, + disk->hosts, + NULL, + NULL, + source); +} + + typedef enum { VIR_DISK_CHAIN_NO_ACCESS, VIR_DISK_CHAIN_READ_ONLY, @@ -12330,6 +12348,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: @@ -12637,6 +12678,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, 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; @@ -12651,8 +12695,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, 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 @@ -12666,13 +12709,21 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, if (!(snapfile = virStorageFileInitFromSnapshotDef(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. */ @@ -12692,6 +12743,27 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, } 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"), @@ -12727,17 +12799,33 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, /* 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: @@ -12746,7 +12834,10 @@ cleanup: virStorageFileFree(snapfile); VIR_FREE(device); VIR_FREE(source); + VIR_FREE(newsource); VIR_FREE(persistSource); + virDomainDiskHostDefFree(snap->nhosts, newhosts); + virDomainDiskHostDefFree(snap->nhosts, persistHosts); return ret; } @@ -12785,12 +12876,20 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, 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.3

On 02/13/2014 09:49 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. ---
Notes: Version 6: - fixed after rename of qemuSnapshotDiskGetActualType
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 02/13/14 22:17, Eric Blake wrote:
On 02/13/2014 09:49 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. ---
Notes: Version 6: - fixed after rename of qemuSnapshotDiskGetActualType
ACK.
Series is now pushed with the little tweak requested in 5/8. Thanks for the reviews. Peter
participants (3)
-
Eric Blake
-
John Ferlan
-
Peter Krempa