On 07/22/2014 07:08 PM, Roman Bogorodskiy wrote:
Implement ZFS storage backend driver. Currently supported only on FreeBSD because of ZFS limitations on Linux.
Features supported:
- pool-start, pool-stop - pool-info - vol-list - vol-create / vol-delete
Pool definition looks like that:
<pool type='zfs'> <name>myzfspool</name> <source> <name>actualpoolname</name> </source> </pool>
The 'actualpoolname' value is a name of the pool on the system, such as shown by 'zpool list' command. Target makes no sense here because volumes path is always /dev/zvol/$poolname/$volname.
Users has to create a pool on his own, this driver doesn't support pool creation currently.
A volume could be used with Qemu by adding an entry like this:
<disk type='volume' device='disk'> <driver name='qemu' type='raw'/> <source pool='myzfspool' volume='vol5'/> <target dev='hdc' bus='ide'/> </disk> ---
Looks good to me, just a few nits below:
configure.ac | 43 +++++ include/libvirt/libvirt.h.in | 1 + po/POTFILES.in | 1 + src/Makefile.am | 8 + src/conf/storage_conf.c | 11 +- src/conf/storage_conf.h | 4 +- src/qemu/qemu_conf.c | 1 + src/storage/storage_backend.c | 6 + src/storage/storage_backend_zfs.c | 344 ++++++++++++++++++++++++++++++++++++++ src/storage/storage_backend_zfs.h | 29 ++++ src/storage/storage_driver.c | 1 + tools/virsh-pool.c | 3 + 12 files changed, 449 insertions(+), 3 deletions(-) create mode 100644 src/storage/storage_backend_zfs.c create mode 100644 src/storage/storage_backend_zfs.h
Missing additions to docs/schemas and docs/storage.html.in
diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c new file mode 100644 index 0000000..1079bc7 --- /dev/null +++ b/src/storage/storage_backend_zfs.c @@ -0,0 +1,344 @@ +/* + * storage_backend_zfs.c: storage backend for ZFS handling + * + * Copyright (C) 2014 Roman Bogorodskiy + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include "viralloc.h" +#include "virerror.h" +#include "virfile.h" +#include "storage_backend_zfs.h" +#include "virlog.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +VIR_LOG_INIT("storage.storage_backend_zfs"); + +/* + * Some common flags of zfs and zpool commands we use: + * -H -- don't print headers and separate fields by tab + * -p -- show exact numbers instead of human-readable, i.e. + * for size, show just a number instead of 2G etc + */ + + +static int +virStorageBackendZFSCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, + bool *isActive) +{ + char *devpath; + + if (virAsprintf(&devpath, "/dev/zvol/%s", + pool->def->source.name) == -1) + return -1; + *isActive = virFileIsDir(devpath); + VIR_FREE(devpath); + + return 0; +} +
+static int +virStorageBackendZFSStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) +{ + return 0; +}
This doesn't need to be implemented if it doesn't do anything (same for StopPool)
+static int +virStorageBackendZFSRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) +{ + virCommandPtr cmd = NULL; + char *zpool_props = NULL; + char **lines = NULL; + size_t i; + + /** + * $ zpool get -Hp health,size,free,allocated test + * test health ONLINE - + * test size 199715979264 - + * test free 198899976704 - + * test allocated 816002560 - + * $ + * + * Here we just provide a list of properties we want to see + */ + cmd = virCommandNewArgList(ZPOOL, + "get", "-Hp", + "health,size,free,allocated", + pool->def->source.name, + NULL); + virCommandSetOutputBuffer(cmd, &zpool_props); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + if (!(lines = virStringSplit(zpool_props, "\n", 0))) + goto cleanup; + + for (i = 0; lines[i]; i++) { + char **tokens; + size_t count; + char *prop_name; + + if (STREQ(lines[i], "")) + continue; + + if (!(tokens = virStringSplitCount(lines[i], "\t", 0, &count))) + goto cleanup; +
+ if (count != 4) + continue;
This leaks tokens.
+ + prop_name = tokens[1]; + + if (STREQ(prop_name, "free") || STREQ(prop_name, "size") || + STREQ(prop_name, "allocated")) { + unsigned long long value; + if (virStrToLong_ull(tokens[2], NULL, 10, &value) < 0) + goto cleanup; + + if (STREQ(prop_name, "free")) + pool->def->available = value; + else if (STREQ(prop_name, "size")) + pool->def->capacity = value; + else if (STREQ(prop_name, "allocated")) + pool->def->allocation = value; + } + } + + /* Obtain a list of volumes */ + if (virStorageBackendZFSFindVols(pool) < 0) + goto cleanup; + + cleanup: + virCommandFree(cmd); + virStringFreeList(lines); + VIR_FREE(zpool_props); + + return 0; +} +
+ +static int +virStorageBackendZFSCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol) +{ + virCommandPtr cmd = NULL; + int ret = -1; + + vol->type = VIR_STORAGE_VOL_BLOCK; + + if (vol->target.path != NULL) { + /* A target path passed to CreateVol has no meaning */ + VIR_FREE(vol->target.path); + } + + if (virAsprintf(&vol->target.path, "/dev/zvol/%s/%s", + pool->def->source.name, vol->name) == -1) + return -1; + + /** + * $ zfs create -o volmode=dev -V 10240K test/volname + * + * -o volmode=dev -- we want to get volumes exposed as cdev + * devices. If we don't specify that zfs + * will lookup vfs.zfs.vol.mode sysctl value + * -V -- tells to create a volume with the specified size + */ + cmd = virCommandNewArgList(ZFS, "create", "-o", "volmode=dev", + "-V", NULL); + virCommandAddArgFormat(cmd, "%lluK", + VIR_DIV_UP(vol->target.capacity, 1024)); + virCommandAddArgFormat(cmd, "%s/%s", + pool->def->source.name, vol->name); + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + if (virAsprintf(&vol->key, "%s/%s", + pool->def->source.name, vol->name) == -1) + goto cleanup;
I think we should fill out the key before trying to create it. Also, is source.name/vol.name unique enough, wouldn't the full target.path be better? Jan