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