[libvirt] [PATCHv4 0/7] gluster snapshots and storage file operations via storage driver

This series is a new approach to using storage driver as file operations backend and uses the implemented functions to make snapshots on gluster work. Most of the patches are different to the previous version due to the refactor. Peter Krempa (7): storage: gluster: Set volume metadata in a separate function storage: Add APIs for internal handling of files via the storage driver storage: Implement 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: Add support for external active snapshots on gluster docs/formatsnapshot.html.in | 5 +- docs/hvsupport.pl | 4 +- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/check-aclrules.pl | 1 + src/check-drivername.pl | 1 + src/driver.h | 30 ++++- src/libvirt_private.c | 176 ++++++++++++++++++++++++++++ src/libvirt_private.h | 61 ++++++++++ src/libvirt_private.syms | 9 ++ src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 9 ++ src/qemu/qemu_driver.c | 145 ++++++++++++++++++++--- src/storage/storage_backend.c | 42 +++++++ src/storage/storage_backend.h | 43 +++++++ src/storage/storage_backend_fs.c | 38 +++++++ src/storage/storage_backend_fs.h | 2 + src/storage/storage_backend_gluster.c | 209 ++++++++++++++++++++++++++++++---- src/storage/storage_backend_gluster.h | 1 + src/storage/storage_driver.c | 89 +++++++++++++++ 20 files changed, 828 insertions(+), 41 deletions(-) create mode 100644 src/libvirt_private.c create mode 100644 src/libvirt_private.h -- 1.8.5.3

Extract the metadata setting code into a separate function for future use. --- src/storage/storage_backend_gluster.c | 66 ++++++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index c73cf8a..d7a1553 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,24 +264,8 @@ 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) - 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; + if (virStorageBackendGlusterSetMetadata(state, vol, name) < 0) goto cleanup; - } - VIR_FREE(state->uri->path); - state->uri->path = tmp; if (S_ISDIR(st->st_mode)) { vol->type = VIR_STORAGE_VOL_NETDIR; @@ -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))) { -- 1.8.5.3

On 02/03/2014 09:54 AM, Peter Krempa wrote:
Extract the metadata setting code into a separate function for future use. --- src/storage/storage_backend_gluster.c | 66 ++++++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 20 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 02/07/14 00:23, Eric Blake wrote:
On 02/03/2014 09:54 AM, Peter Krempa wrote:
Extract the metadata setting code into a separate function for future use. --- src/storage/storage_backend_gluster.c | 66 ++++++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 20 deletions(-)
ACK.
Thanks, I've pushed this one since it's separate. Peter

Some remote filesystems are not accessible via the local filesystem calls, but libvirt needs means to do operations on such files. This patch adds internal APIs into the storage driver that will allow operations on various networked and other filesystems. --- docs/hvsupport.pl | 4 +- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/check-drivername.pl | 1 + src/driver.h | 30 +++++++- src/libvirt_private.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_private.h | 61 ++++++++++++++++ src/libvirt_private.syms | 9 +++ 8 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 src/libvirt_private.c create mode 100644 src/libvirt_private.h diff --git a/docs/hvsupport.pl b/docs/hvsupport.pl index f8483f9..bed04f9 100755 --- a/docs/hvsupport.pl +++ b/docs/hvsupport.pl @@ -206,6 +206,8 @@ while (defined($line = <FILE>)) { $api = "vir$name"; } elsif ($name =~ /\w+(Open|Close)/) { next; + } elsif ($name =~ /^StorageFile\w+/) { + next; } else { die "driver $name does not have a public API"; } @@ -256,7 +258,7 @@ foreach my $src (@srcs) { my $meth = $2; my $vers = $3; - next if $api eq "no" || $api eq "name"; + next if $api eq "no" || $api eq "name" || $api =~ /^storageFile\w+/; die "Method $meth in $src is missing version" unless defined $vers; diff --git a/po/POTFILES.in b/po/POTFILES.in index 0359b2f..9304942 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -50,6 +50,7 @@ src/interface/interface_backend_netcf.c src/interface/interface_backend_udev.c src/internal.h src/libvirt.c +src/libvirt_private.c src/libvirt-lxc.c src/libvirt-qemu.c src/locking/lock_daemon.c diff --git a/src/Makefile.am b/src/Makefile.am index ac2d1d9..d0b1db3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -173,6 +173,7 @@ DRIVER_SOURCES = \ fdstream.c fdstream.h \ $(NODE_INFO_SOURCES) \ libvirt.c libvirt_internal.h \ + libvirt_private.h libvirt_private.c \ 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..a98d7db 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 =~ /virDrvStorageFile/; my $sym = $drv; $sym =~ s/virDrv/vir/; diff --git a/src/driver.h b/src/driver.h index 5f4cd8d..20f944a 100644 --- a/src/driver.h +++ b/src/driver.h @@ -2,7 +2,7 @@ * driver.h: description of the set of interfaces provided by a * entry point to the virtualization engine * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-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 @@ -24,6 +24,8 @@ # include <unistd.h> +# include <sys/stat.h> + # include "internal.h" # include "libvirt_internal.h" # include "viruri.h" @@ -1755,6 +1757,25 @@ typedef int (*virDrvStoragePoolIsPersistent)(virStoragePoolPtr pool); +/* -------- Internal storage driver APIs ----------- */ +typedef struct _virStorageFile virStorageFile; +typedef virStorageFile *virStorageFilePtr; + +typedef int +(*virDrvStorageFileInit)(virStorageFilePtr file); + +typedef void +(*virDrvStorageFileDeinit)(virStorageFilePtr file); + +typedef int +(*virDrvStorageFileCreate)(virStorageFilePtr file); + +typedef int +(*virDrvStorageFileUnlink)(virStorageFilePtr file); + +typedef int +(*virDrvStorageFileStat)(virStorageFilePtr file, + struct stat *st); typedef struct _virStorageDriver virStorageDriver; typedef virStorageDriver *virStorageDriverPtr; @@ -1813,6 +1834,13 @@ struct _virStorageDriver { virDrvStorageVolResize storageVolResize; virDrvStoragePoolIsActive storagePoolIsActive; virDrvStoragePoolIsPersistent storagePoolIsPersistent; + + /* -------- Internal storage driver APIs ----------- */ + virDrvStorageFileInit storageFileInit; + virDrvStorageFileDeinit storageFileDeinit; + virDrvStorageFileCreate storageFileCreate; + virDrvStorageFileUnlink storageFileUnlink; + virDrvStorageFileStat storageFileStat; }; # ifdef WITH_LIBVIRTD diff --git a/src/libvirt_private.c b/src/libvirt_private.c new file mode 100644 index 0000000..3b103b2 --- /dev/null +++ b/src/libvirt_private.c @@ -0,0 +1,176 @@ +/** + * libvirt_private.c: internal driver APIs not exported via RPC + * + * 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/>. + * + * Author: Peter Krempa <pkrempa@redhat.com> + */ + +#include <config.h> + +#include "libvirt_private.h" + +#include "datatypes.h" +#include "internal.h" +#include "viralloc.h" + +#define VIR_FROM_THIS VIR_FROM_NONE +/* ----------- helpers for APIs cooperating with storage driver ------ */ +virStorageFilePtr +virStorageFileInitFromDiskDef(virConnectPtr conn, + virDomainDiskDefPtr disk) +{ + virStorageFilePtr file = NULL; + + if (!conn->storageDriver) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("can't initialize storage file: no storage driver")); + return NULL; + } + + if (VIR_ALLOC(file) < 0) + return NULL; + + file->driver = conn->storageDriver; + + file->path = disk->src; + file->type = disk->type; + file->protocol = disk->protocol; + file->nhosts = disk->nhosts; + if (!(file->hosts = virDomainDiskHostDefCopy(disk->nhosts, disk->hosts))) + goto error; + + if (file->driver->storageFileInit) { + if (file->driver->storageFileInit(file) < 0) + goto error; + } + + return file; + +error: + virDomainDiskHostDefFree(file->nhosts, file->hosts); + VIR_FREE(file); + return NULL; +} + + +virStorageFilePtr +virStorageFileInitFromSnapshotDef(virConnectPtr conn, + virDomainSnapshotDiskDefPtr disk) +{ + virStorageFilePtr file = NULL; + + if (!conn->storageDriver) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("can't initialize storage file: no storage driver")); + return NULL; + } + + if (VIR_ALLOC(file) < 0) + return NULL; + + file->driver = conn->storageDriver; + + file->path = disk->file; + file->type = disk->type; + file->protocol = disk->protocol; + file->nhosts = disk->nhosts; + if (!(file->hosts = virDomainDiskHostDefCopy(disk->nhosts, disk->hosts))) + goto error; + + if (file->driver->storageFileInit) { + if (file->driver->storageFileInit(file) < 0) + goto error; + } + + return file; + +error: + virDomainDiskHostDefFree(file->nhosts, file->hosts); + VIR_FREE(file); + return NULL; +} + + +void +virStorageFileFree(virStorageFilePtr file) +{ + if (!file) + return; + + if (file->driver->storageFileDeinit) + file->driver->storageFileDeinit(file); + + virDomainDiskHostDefFree(file->nhosts, file->hosts); + VIR_FREE(file); +} + +/* ----------- APIs cooperating with storage driver ------------------ */ + +/** + * virStorageFileCreate: Creates an empty storage file via storage driver + * + * @file: file structure pointing to the file + * + * Returns 0 on success, errno on failure. + */ +int +virStorageFileCreate(virStorageFilePtr file) +{ + if (file->driver->storageFileCreate) + return file->driver->storageFileCreate(file); + + return ENOSYS; +} + + +/** + * virStorageFileUnlink: Unlink storage file via storage driver + * + * @file: file structure poiniting to the file + * + * Unlinks the file described by the @file structure. + * + * Returns 0 on success, errno on failure. + */ +int +virStorageFileUnlink(virStorageFilePtr file) +{ + if (file->driver->storageFileUnlink) + return file->driver->storageFileUnlink(file); + + return ENOSYS; +} + + +/** + * 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, errno on failure. + */ +int +virStorageFileStat(virStorageFilePtr file, + struct stat *stat) +{ + if (file->driver->storageFileStat) + return file->driver->storageFileStat(file, stat); + + return ENOSYS; +} diff --git a/src/libvirt_private.h b/src/libvirt_private.h new file mode 100644 index 0000000..8b6dd30 --- /dev/null +++ b/src/libvirt_private.h @@ -0,0 +1,61 @@ +/** + * libvirt_private.h: internal driver APIs not exported via RPC + * + * 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/>. + * + * Author: Peter Krempa <pkrempa@redhat.com> + */ + +#ifndef __VIR_PRIVATE_H__ +# define __VIR_PRIVATE_H__ + +# include "driver.h" +# include "conf/domain_conf.h" +# include "conf/snapshot_conf.h" +# include "viruri.h" + +# include <sys/types.h> +# include <sys/stat.h> + +/* -------- APIs cooperating with storage driver ----------------------- */ +struct _virStorageFile { + virStorageDriverPtr driver; + void *priv; + + char *path; + int type; + int protocol; + + size_t nhosts; + virDomainDiskHostDefPtr hosts; +}; + + +virStorageFilePtr +virStorageFileInitFromDiskDef(virConnectPtr conn, + virDomainDiskDefPtr disk); +virStorageFilePtr +virStorageFileInitFromSnapshotDef(virConnectPtr conn, + virDomainSnapshotDiskDefPtr disk); +void virStorageFileFree(virStorageFilePtr file); + +int virStorageFileCreate(virStorageFilePtr file); +int virStorageFileUnlink(virStorageFilePtr file); +int virStorageFileStat(virStorageFilePtr file, + struct stat *stat); + +#endif /* #ifndef __VIR_PRIVATE_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1a8d088..63460a7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -818,6 +818,15 @@ virRegisterSecretDriver; virRegisterStorageDriver; +# libvirt_private.h +virStorageFileCreate; +virStorageFileFree; +virStorageFileInitFromDiskDef; +virStorageFileInitFromSnapshotDef; +virStorageFileStat; +virStorageFileUnlink; + + # locking/domain_lock.h virDomainLockDiskAttach; virDomainLockDiskDetach; -- 1.8.5.3

On 02/03/2014 09:54 AM, Peter Krempa wrote:
Some remote filesystems are not accessible via the local filesystem calls, but libvirt needs means to do operations on such files.
This patch adds internal APIs into the storage driver that will allow operations on various networked and other filesystems. --- docs/hvsupport.pl | 4 +- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/check-drivername.pl | 1 + src/driver.h | 30 +++++++- src/libvirt_private.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_private.h | 61 ++++++++++++++++ src/libvirt_private.syms | 9 +++ 8 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 src/libvirt_private.c create mode 100644 src/libvirt_private.h
+++ b/src/Makefile.am @@ -173,6 +173,7 @@ DRIVER_SOURCES = \ fdstream.c fdstream.h \ $(NODE_INFO_SOURCES) \ libvirt.c libvirt_internal.h \ + libvirt_private.h libvirt_private.c \ locking/lock_manager.c locking/lock_manager.h \
Alignment of \ looks off.
+virStorageFilePtr +virStorageFileInitFromDiskDef(virConnectPtr conn, + virDomainDiskDefPtr disk) +{
+ + if (file->driver->storageFileInit) { + if (file->driver->storageFileInit(file) < 0)
This could be joined with && instead of nested if.
+ + +virStorageFilePtr +virStorageFileInitFromSnapshotDef(virConnectPtr conn, + virDomainSnapshotDiskDefPtr disk)
+ + if (file->driver->storageFileInit) { + if (file->driver->storageFileInit(file) < 0) + goto error;
Same here.
+/** + * virStorageFileUnlink: Unlink storage file via storage driver + * + * @file: file structure poiniting to the file
s/poiniting/pointing/ Those are all minor. Wait for the review of the full series before pushing this, but I'm okay with ACK for htis one. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Implement the APIs added by the previous patch in the default storage driver used by qemu. --- src/check-aclrules.pl | 1 + src/storage/storage_backend.c | 37 ++++++++++++++++++ src/storage/storage_backend.h | 43 +++++++++++++++++++++ src/storage/storage_driver.c | 89 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+) diff --git a/src/check-aclrules.pl b/src/check-aclrules.pl index 057517e..7f34094 100755 --- a/src/check-aclrules.pl +++ b/src/check-aclrules.pl @@ -221,6 +221,7 @@ while (<>) { if ($api ne "no" && $api ne "name" && + $api !~ /^storageFile\w+/ && $table ne "virStateDriver" && !exists $acls{$impl} && !exists $whitelist{$api} && 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..5613285 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 "libvirt_private.h" typedef char * (*virStorageBackendFindPoolSources)(virConnectPtr conn, const char *srcSpec, @@ -189,4 +190,46 @@ 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); + +typedef struct _virStorageFilePriv virStorageFileBackendPriv; +typedef virStorageFileBackendPriv *virStorageFileBackendPrivPtr; +struct _virStorageFilePriv { + virStorageFileBackendPtr backend; + void *priv; +}; + + +struct _virStorageFileBackend { + int type; + int protocol; + + virStorageFileBackendInit backendInit; + virStorageFileBackendDeinit backendDeinit; + + 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 c83aa8a..75e6454 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2585,6 +2585,86 @@ cleanup: return ret; } + +static int +storageFileInit(virStorageFilePtr file) +{ + virStorageFileBackendPrivPtr priv = NULL; + + if (VIR_ALLOC(priv) < 0) + return -1; + + file->priv = priv; + + if (!(priv->backend = virStorageFileBackendForType(file->type, + file->protocol))) + goto error; + + if (priv->backend->backendInit) { + if (priv->backend->backendInit(file) < 0) + goto error; + } + + return 0; + +error: + file->priv = NULL; + VIR_FREE(priv); + return -1; +} + + +static void +storageFileDeinit(virStorageFilePtr file) +{ + virStorageFileBackendPrivPtr priv = file->priv; + + if (priv->backend && + priv->backend->backendDeinit) + priv->backend->backendDeinit(file); + + VIR_FREE(priv); + file->priv = NULL; +} + + +static int +storageFileCreate(virStorageFilePtr file) +{ + virStorageFileBackendPrivPtr priv = file->priv; + + if (!priv->backend->storageFileCreate) + return ENOSYS; + + return priv->backend->storageFileCreate(file); +} + + +static int +storageFileUnlink(virStorageFilePtr file) +{ + virStorageFileBackendPrivPtr priv = file->priv; + + if (!priv->backend->storageFileUnlink) + return ENOSYS; + + return priv->backend->storageFileUnlink(file); +} + + +static int +storageFileStat(virStorageFilePtr file, + struct stat *st) +{ + virStorageFileBackendPrivPtr priv = file->priv; + + if (!(priv->backend->storageFileStat)) + return ENOSYS; + + return priv->backend->storageFileStat(file, st); +} + + static virStorageDriver storageDriver = { .name = "storage", .storageOpen = storageOpen, /* 0.4.0 */ @@ -2631,6 +2711,15 @@ static virStorageDriver storageDriver = { .storagePoolIsActive = storagePoolIsActive, /* 0.7.3 */ .storagePoolIsPersistent = storagePoolIsPersistent, /* 0.7.3 */ + + /* ----- internal APIs ----- */ + .storageFileInit = storageFileInit, + .storageFileDeinit = storageFileDeinit, + + .storageFileUnlink = storageFileUnlink, + .storageFileStat = storageFileStat, + .storageFileCreate = storageFileCreate, + }; -- 1.8.5.3

On 02/03/2014 09:54 AM, Peter Krempa wrote:
Implement the APIs added by the previous patch in the default storage driver used by qemu. --- src/check-aclrules.pl | 1 + src/storage/storage_backend.c | 37 ++++++++++++++++++ src/storage/storage_backend.h | 43 +++++++++++++++++++++ src/storage/storage_driver.c | 89 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+)
+static int +storageFileInit(virStorageFilePtr file) +{ + virStorageFileBackendPrivPtr priv = NULL; + + if (VIR_ALLOC(priv) < 0) + return -1; + + file->priv = priv; + + if (!(priv->backend = virStorageFileBackendForType(file->type, + file->protocol))) + goto error; + + if (priv->backend->backendInit) { + if (priv->backend->backendInit(file) < 0) + goto error; + } + + return 0;
So if there's no backendInit callback function, we just succeed? Shouldn't that error out? Or is backendInit just optional, and the important thing is wheterh priv->backend was successfully looked up.
+static int +storageFileUnlink(virStorageFilePtr file) +{ + virStorageFileBackendPrivPtr priv = file->priv; + + if (!priv->backend->storageFileUnlink) + return ENOSYS;
Returning a positive errno value? Don't you want a negative here? Any documentation on the fact that these functions are returning errno values instead of -1, and with errno itself left indeterminate? Or maybe make this 'errno = ENOSYS; return -1;'?
+ + return priv->backend->storageFileUnlink(file); +} + + +static int +storageFileStat(virStorageFilePtr file, + struct stat *st) +{ + virStorageFileBackendPrivPtr priv = file->priv; + + if (!(priv->backend->storageFileStat)) + return ENOSYS;
Same issue about failure returns.
+ + return priv->backend->storageFileStat(file, st); +} + + static virStorageDriver storageDriver = { .name = "storage", .storageOpen = storageOpen, /* 0.4.0 */ @@ -2631,6 +2711,15 @@ static virStorageDriver storageDriver = {
.storagePoolIsActive = storagePoolIsActive, /* 0.7.3 */ .storagePoolIsPersistent = storagePoolIsPersistent, /* 0.7.3 */ + + /* ----- internal APIs ----- */ + .storageFileInit = storageFileInit, + .storageFileDeinit = storageFileDeinit, + + .storageFileUnlink = storageFileUnlink, + .storageFileStat = storageFileStat, + .storageFileCreate = storageFileCreate, + };
I can see where you're going with this, but am not quite sure it is ready to merge without addressing my comments above. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 02/10/14 23:09, Eric Blake wrote:
On 02/03/2014 09:54 AM, Peter Krempa wrote:
Implement the APIs added by the previous patch in the default storage driver used by qemu. --- src/check-aclrules.pl | 1 + src/storage/storage_backend.c | 37 ++++++++++++++++++ src/storage/storage_backend.h | 43 +++++++++++++++++++++ src/storage/storage_driver.c | 89 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+)
+static int +storageFileInit(virStorageFilePtr file) +{ + virStorageFileBackendPrivPtr priv = NULL; + + if (VIR_ALLOC(priv) < 0) + return -1; + + file->priv = priv; + + if (!(priv->backend = virStorageFileBackendForType(file->type, + file->protocol))) + goto error; + + if (priv->backend->backendInit) { + if (priv->backend->backendInit(file) < 0) + goto error; + } + + return 0;
So if there's no backendInit callback function, we just succeed? Shouldn't that error out? Or is backendInit just optional, and the important thing is wheterh priv->backend was successfully looked up.
The backendInit function is optional. It's needed for backends that need to open connections or so. The regular local file backend doesn't do anything special in this case so it is deliberately left to do nothing.
+static int +storageFileUnlink(virStorageFilePtr file) +{ + virStorageFileBackendPrivPtr priv = file->priv; + + if (!priv->backend->storageFileUnlink) + return ENOSYS;
Returning a positive errno value? Don't you want a negative here? Any documentation on the fact that these functions are returning errno values instead of -1, and with errno itself left indeterminate? Or maybe make this 'errno = ENOSYS; return -1;'?
Well the behavior is documented in "libvirt_private.c" in the previous patch to return positive errnos, but I think that returning -1 and setting errno to mirror the usual approach from libc is better so I'll change it.
+ + return priv->backend->storageFileUnlink(file); +} + + +static int +storageFileStat(virStorageFilePtr file, + struct stat *st) +{ + virStorageFileBackendPrivPtr priv = file->priv; + + if (!(priv->backend->storageFileStat)) + return ENOSYS;
Same issue about failure returns.
+ + return priv->backend->storageFileStat(file, st); +} + + static virStorageDriver storageDriver = { .name = "storage", .storageOpen = storageOpen, /* 0.4.0 */ @@ -2631,6 +2711,15 @@ static virStorageDriver storageDriver = {
.storagePoolIsActive = storagePoolIsActive, /* 0.7.3 */ .storagePoolIsPersistent = storagePoolIsPersistent, /* 0.7.3 */ + + /* ----- internal APIs ----- */ + .storageFileInit = storageFileInit, + .storageFileDeinit = storageFileDeinit, + + .storageFileUnlink = storageFileUnlink, + .storageFileStat = storageFileStat, + .storageFileCreate = storageFileCreate, + };
I can see where you're going with this, but am not quite sure it is ready to merge without addressing my comments above.
Peter

Implement the "stat" and "unlink" function for "file" volumes and "stat" for "block volumes using the regular system calls. --- src/storage/storage_backend.c | 2 ++ src/storage/storage_backend_fs.c | 38 ++++++++++++++++++++++++++++++++++++++ src/storage/storage_backend_fs.h | 2 ++ 3 files changed, 42 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index b59b5b7..da81da9 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -123,6 +123,8 @@ static virStorageBackendPtr backends[] = { static virStorageFileBackendPtr fileBackends[] = { + &virStorageFileBackendFile, + &virStorageFileBackendBlock, NULL }; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index fa11e2f..7e55196 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1327,4 +1327,42 @@ virStorageBackend virStorageBackendNetFileSystem = { .deleteVol = virStorageBackendFileSystemVolDelete, .resizeVol = virStorageBackendFileSystemVolResize, }; + + +static int +virStorageFileBackendFileUnlink(virStorageFilePtr file) +{ + if (unlink(file->path) < 0) + return errno; + + return 0; +} + + +static int +virStorageFileBackendFileStat(virStorageFilePtr file, + struct stat *st) +{ + if (stat(file->path, st) < 0) + return errno; + + return 0; +} + + +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

Implement storage backend functions to deal with gluster volumes and implement the "stat" and "unlink" backend APIs. --- src/storage/storage_backend.c | 3 + src/storage/storage_backend_gluster.c | 143 ++++++++++++++++++++++++++++++++++ src/storage/storage_backend_gluster.h | 1 + 3 files changed, 147 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index da81da9..1253343 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -125,6 +125,9 @@ static virStorageBackendPtr backends[] = { static virStorageFileBackendPtr fileBackends[] = { &virStorageFileBackendFile, &virStorageFileBackendBlock, +#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..c5ff07a 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -477,3 +477,146 @@ 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->path, file->hosts[0].name); + virStorageFileBackendPrivPtr backPriv = file->priv; + virStorageFileBackendGlusterPrivPtr priv = backPriv->priv; + + glfs_fini(priv->vol); + VIR_FREE(priv->volname); + + VIR_FREE(priv); + backPriv->priv = NULL; +} + +static int +virStorageFileBackendGlusterInit(virStorageFilePtr file) +{ + virStorageFileBackendPrivPtr backPriv = file->priv; + 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, file->path, hostname); + + 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; + } + + backPriv->priv = priv; + + return 0; + +error: + VIR_FREE(priv->volname); + glfs_fini(priv->vol); + + return -1; +} + + +static int +virStorageFileBackendGlusterUnlink(virStorageFilePtr file) +{ + virStorageFileBackendPrivPtr backPriv = file->priv; + virStorageFileBackendGlusterPrivPtr priv = backPriv->priv; + int ret = 0; + + if (glfs_unlink(priv->vol, priv->path) < 0) + ret = errno; + + VIR_DEBUG("removing storage file %p(%s@%s): %d", + file, file->path, file->hosts[0].name, ret); + return ret; +} + + +static int +virStorageFileBackendGlusterStat(virStorageFilePtr file, + struct stat *st) +{ + virStorageFileBackendPrivPtr backPriv = file->priv; + virStorageFileBackendGlusterPrivPtr priv = backPriv->priv; + int ret = 0; + + if (glfs_stat(priv->vol, priv->path, st) < 0) + ret = errno; + + VIR_DEBUG("stat of gluster storage file %p(%s@%s): %d", + file, file->path, file->hosts[0].name, ret); + 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

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. --- src/qemu/qemu_driver.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0128356..9f61b4b 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 "libvirt_private.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -12603,7 +12604,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, @@ -12621,6 +12623,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", @@ -12640,6 +12643,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, virStorageFileFreeMetadata(disk->backingChain); disk->backingChain = NULL; + if (!(snapfile = virStorageFileInitFromSnapshotDef(conn, snap))) + goto cleanup; + switch (snap->type) { case VIR_DOMAIN_DISK_TYPE_BLOCK: reuse = true; @@ -12715,8 +12721,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); @@ -12727,7 +12734,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, @@ -12736,16 +12744,20 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, { char *source = NULL; char *persistSource = NULL; + virStorageFilePtr diskfile = NULL; struct stat st; + diskfile = virStorageFileInitFromDiskDef(conn, 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. */ @@ -12763,13 +12775,15 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, } cleanup: + virStorageFileFree(diskfile); 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, @@ -12817,7 +12831,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, persistDisk = vm->newDef->disks[indx]; } - ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, + ret = qemuDomainSnapshotCreateSingleDiskActive(conn, driver, vm, &snap->def->disks[i], vm->def->disks[i], persistDisk, actions, @@ -12863,7 +12877,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, } - qemuDomainSnapshotUndoSingleDiskActive(driver, vm, + qemuDomainSnapshotUndoSingleDiskActive(conn, driver, vm, snap->def->dom->disks[i], vm->def->disks[i], persistDisk, @@ -13005,7 +13019,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.3

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 | 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 5b94de1..7406317 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 9f61b4b..a490b72 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11906,6 +11906,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, @@ -12303,6 +12321,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: @@ -12617,6 +12658,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; @@ -12631,8 +12675,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 @@ -12646,13 +12689,21 @@ qemuDomainSnapshotCreateSingleDiskActive(virConnectPtr conn, if (!(snapfile = virStorageFileInitFromSnapshotDef(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. */ @@ -12672,6 +12723,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"), @@ -12707,17 +12779,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: @@ -12726,7 +12814,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; } @@ -12766,12 +12857,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.3
participants (2)
-
Eric Blake
-
Peter Krempa