[libvirt] [PATCHv3 0/3] Implement RBD storage pool support

I have re-written the RBD storage pool patch to work properly under the re-organized QEMU disk argument generation structure. The patch is largely the same as last time, just using the new switch statement and not modifying qemu_command.c anymore. Also of note in this set of patches is a fix to a bug in configure.ac introduced in a recent commit. I had to fix it in order to get my Archlinux package to compile on another machine so I could test the code before submission, so I figured that I would go ahead and share it with everyone, too. Adam Walters (3): qemu: conf: Add qemuAddRBDPoolSourceHost helper function qemu: conf: Implement RBD storage pool support configure: Resolve compile issue in configure.ac configure.ac | 1 - src/qemu/qemu_conf.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 2 deletions(-) -- 1.8.4.2

This helper function pulls the host definitions from the RBD storage pool definition, and applies it to the disk definition. --- src/qemu/qemu_conf.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 44e4320..060ac17 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1247,6 +1247,45 @@ cleanup: } static int +qemuAddRBDPoolSourceHost(virDomainDiskDefPtr def, + virStoragePoolDefPtr pooldef) +{ + int ret = -1; + size_t i = 0; + char **tokens = NULL; + + def->nhosts = pooldef->source.nhost; + + if (VIR_ALLOC_N(def->hosts, def->nhosts) < 0) + goto cleanup; + + for (i = 0; i < def->nhosts; i++) { + if (VIR_STRDUP(def->hosts[i].name, pooldef->source.hosts[i].name) < 0) + goto cleanup; + + if (virAsprintf(&def->hosts[i].port, "%d", + pooldef->source.hosts[i].port ? + pooldef->source.hosts[i].port : + 6789) < 0) + goto cleanup; + + /* Storage pools have not supported these 2 attributes yet, + * use the defaults. + */ + def->hosts[i].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + def->hosts[i].socket = NULL; + } + + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; + + ret = 0; + +cleanup: + virStringFreeList(tokens); + return ret; +} + +static int qemuTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def, virStoragePoolDefPtr pooldef) { -- 1.8.4.2

This implements RBD storage pool support in the qemuTranslateDiskSourcePool function. This support is working on my machine, but could probably use some additional testing. It is implemented very similarly to the ISCSI support. --- src/qemu/qemu_conf.c | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 060ac17..f3daf06 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1478,8 +1478,42 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, } break; - case VIR_STORAGE_POOL_MPATH: case VIR_STORAGE_POOL_RBD: + if (def->startupPolicy) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'startupPolicy' is only valid for " + "'file' type volume")); + goto cleanup; + } + + switch (def->srcpool->mode) { + case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DEFAULT: + case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_LAST: + def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT; + /* fallthrough */ + case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT: + def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; + + if (!(def->src = virStorageVolGetPath(vol))) + goto cleanup; + + if (qemuTranslateDiskSourcePoolAuth(def, pooldef) < 0) + goto cleanup; + + if (qemuAddRBDPoolSourceHost(def, pooldef) < 0) + goto cleanup; + break; + + case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST: + virReportError(VIR_ERR_XML_ERROR, "%s", + _("\"mode='host'\" is not valid for " + "'rbd' type volume")); + goto cleanup; + } + break; + + case VIR_STORAGE_POOL_MPATH: case VIR_STORAGE_POOL_SHEEPDOG: case VIR_STORAGE_POOL_GLUSTER: case VIR_STORAGE_POOL_LAST: -- 1.8.4.2

On 12/05/13 05:09, Adam Walters wrote:
This implements RBD storage pool support in the qemuTranslateDiskSourcePool function. This support is working on my machine, but could probably use some additional testing. It is implemented very similarly to the ISCSI support. --- src/qemu/qemu_conf.c | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 060ac17..f3daf06 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1478,8 +1478,42 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, } break;
- case VIR_STORAGE_POOL_MPATH: case VIR_STORAGE_POOL_RBD: + if (def->startupPolicy) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'startupPolicy' is only valid for " + "'file' type volume")); + goto cleanup; + } + + switch (def->srcpool->mode) {
the mode is checked and won't be anything else than _DEFAULT here, so this switch is not needed.
+ case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DEFAULT: + case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_LAST: + def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT; + /* fallthrough */ + case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT: + def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK; + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; + + if (!(def->src = virStorageVolGetPath(vol))) + goto cleanup; + + if (qemuTranslateDiskSourcePoolAuth(def, pooldef) < 0) + goto cleanup; + + if (qemuAddRBDPoolSourceHost(def, pooldef) < 0) + goto cleanup; + break; + + case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST: + virReportError(VIR_ERR_XML_ERROR, "%s", + _("\"mode='host'\" is not valid for " + "'rbd' type volume")); + goto cleanup; + } + break; + + case VIR_STORAGE_POOL_MPATH: case VIR_STORAGE_POOL_SHEEPDOG: case VIR_STORAGE_POOL_GLUSTER: case VIR_STORAGE_POOL_LAST:
I'll resend this patch with the switch removed so you can give it a try before pushing. Thanks for the patch and for fixing it after my changes. Peter

This patch resolves a compile issue caused by the removal of examples/domsuspend code in commit 5eb4b04211ea379c58f735dee6c2852c8b80da89. This issue is only seen in a fresh checkout, but causes the build and configure to fail. --- configure.ac | 1 - 1 file changed, 1 deletion(-) diff --git a/configure.ac b/configure.ac index 936e273..4465e01 100644 --- a/configure.ac +++ b/configure.ac @@ -2555,7 +2555,6 @@ AC_CONFIG_FILES([\ tests/Makefile \ examples/apparmor/Makefile \ examples/domain-events/events-c/Makefile \ - examples/domsuspend/Makefile \ examples/dominfo/Makefile \ examples/openauth/Makefile \ examples/hellolibvirt/Makefile \ -- 1.8.4.2

On Wed, Dec 04, 2013 at 11:09:14PM -0500, Adam Walters wrote:
This patch resolves a compile issue caused by the removal of examples/domsuspend code in commit 5eb4b04211ea379c58f735dee6c2852c8b80da89. This issue is only seen in a fresh checkout, but causes the build and configure to fail. --- configure.ac | 1 - 1 file changed, 1 deletion(-)
diff --git a/configure.ac b/configure.ac index 936e273..4465e01 100644 --- a/configure.ac +++ b/configure.ac @@ -2555,7 +2555,6 @@ AC_CONFIG_FILES([\ tests/Makefile \ examples/apparmor/Makefile \ examples/domain-events/events-c/Makefile \ - examples/domsuspend/Makefile \ examples/dominfo/Makefile \ examples/openauth/Makefile \ examples/hellolibvirt/Makefile \ -- 1.8.4.2
ACK pushing that one separately as a buildbreaker, thanks, Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veillard@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

On 12/05/13 09:14, Daniel Veillard wrote:
On Wed, Dec 04, 2013 at 11:09:14PM -0500, Adam Walters wrote:
This patch resolves a compile issue caused by the removal of examples/domsuspend code in commit 5eb4b04211ea379c58f735dee6c2852c8b80da89. This issue is only seen in a fresh checkout, but causes the build and configure to fail. --- configure.ac | 1 - 1 file changed, 1 deletion(-)
diff --git a/configure.ac b/configure.ac index 936e273..4465e01 100644 --- a/configure.ac +++ b/configure.ac @@ -2555,7 +2555,6 @@ AC_CONFIG_FILES([\ tests/Makefile \ examples/apparmor/Makefile \ examples/domain-events/events-c/Makefile \ - examples/domsuspend/Makefile \ examples/dominfo/Makefile \ examples/openauth/Makefile \ examples/hellolibvirt/Makefile \ -- 1.8.4.2
ACK pushing that one separately as a buildbreaker,
thanks,
Daniel
Hmmm a rerun configure didn't catch this unfortunately. I'm adding 'git clean -fxd' to my build command to avoid this in the future. Sorry for the mess. PEter
participants (3)
-
Adam Walters
-
Daniel Veillard
-
Peter Krempa