[libvirt] [PATCHv2 0/3] Rebase of patch to add support for RBD storage pools

This series of patches adds support for RBD storage pools to the latest version of libvirt. Included in this is a simple structure to build additional network storage pool support off of (really a couple of if/else blocks, so nothing fancy). This code passes check, check-syntax, and the valgrind tests. Additionally, the code is tested working in my environment. In fact, the machine I am sending this email from is a VM being run under qemu+libvirt with backend storage being an RBD storage pool using this code. Unfortunately, I only have the one RBD pool and libvirt host, so the code could use additional testing. If anyone finds any problems with my code, please let me know. I will fix them and resubmit the patch as needed. Hopefully someone else out there finds this support as useful as I do. While not many libvirt management clients utilize the storage API today, it does look to be the better method to interact with storage, and thus likely to be the best way to go in the future. I figure that additional storage backend support may well help to get others using the API instead of duplicating configuration options across every VM. Adam Walters (3): qemu: config: Add qemuAddRBDPoolSourceHost helper function qemu: conf: Parse RBD storage pool from domain xml qemu: command: Handle RBD storage pools src/qemu/qemu_command.c | 11 +++++++++ src/qemu/qemu_conf.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) -- 1.8.4.2

Add a function to grab the RBD hosts from a pool definition and add them to the disk definition. This function allows the addition of RBD storage pool functionality. --- 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 557ccc5..e2154c6 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

Modify switch statement in qemuTranslateDiskSourcePool to parse the storage pool definition for RBD pools instead of throwing an error. I do throw errors for any other usage case of VIR_STORAGE_VOL_NETWORK, as my code does not cover their use cases currently. Once I have the ability to test such usage cases, I will revisit those possibilities. --- src/qemu/qemu_conf.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e2154c6..024d40f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1416,6 +1416,32 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, break; case VIR_STORAGE_VOL_NETWORK: + if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0))) + goto cleanup; + + if (!(pooldef = virStoragePoolDefParseString(poolxml))) + goto cleanup; + + def->srcpool->pooltype = pooldef->type; + if (pooldef->type == VIR_STORAGE_POOL_RBD) { + if (!def->srcpool->mode) + def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT; + + if (qemuAddRBDPoolSourceHost(def, pooldef) < 0) + goto cleanup; + + if (!(def->src = virStorageVolGetPath(vol))) + goto cleanup; + + if (qemuTranslateDiskSourcePoolAuth(def, pooldef) < 0) + goto cleanup; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unsupported network volume type")); + goto cleanup; + } + + break; case VIR_STORAGE_VOL_NETDIR: case VIR_STORAGE_VOL_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 1.8.4.2

This patch modifies the switch statement in qemuBuildVolumeString to format and pass the needed argument to handle a volume from an RBD storage pool properly. If the volume is a VIR_STORAGE_VOL_NETWORK but is not from an RBD pool, I goto cleanup, just like the other unimplemented volume types. Currently, the code only supports RBD volumes, as I lack the facilities to test other network storage pools. Once I have the ability to test those, I will revisit the possibility of implementing additional types. --- src/qemu/qemu_command.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 763417f..4102568 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3825,6 +3825,17 @@ qemuBuildVolumeString(virConnectPtr conn, } break; case VIR_STORAGE_VOL_NETWORK: + if (disk->srcpool->pooltype == VIR_STORAGE_POOL_RBD) { + virBufferAddLit(opt, "file="); + + if (qemuBuildRBDString(conn, disk, opt) < 0) + goto cleanup; + + virBufferAddChar(opt, ','); + } else { + goto cleanup; + } + break; case VIR_STORAGE_VOL_NETDIR: case VIR_STORAGE_VOL_LAST: /* Keep the compiler quiet, qemuTranslateDiskSourcePool already -- 1.8.4.2

On 12/02/13 22:36, Adam Walters wrote:
This patch modifies the switch statement in qemuBuildVolumeString to format and pass the needed argument to handle a volume from an RBD storage pool properly. If the volume is a VIR_STORAGE_VOL_NETWORK but is not from an RBD pool, I goto cleanup, just like the other unimplemented volume types. Currently, the code only supports RBD volumes, as I lack the facilities to test other network storage pools. Once I have the ability to test those, I will revisit the possibility of implementing additional types. --- src/qemu/qemu_command.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 763417f..4102568 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3825,6 +3825,17 @@ qemuBuildVolumeString(virConnectPtr conn, } break; case VIR_STORAGE_VOL_NETWORK: + if (disk->srcpool->pooltype == VIR_STORAGE_POOL_RBD) { + virBufferAddLit(opt, "file="); + + if (qemuBuildRBDString(conn, disk, opt) < 0) + goto cleanup; + + virBufferAddChar(opt, ','); + } else { + goto cleanup; + } + break; case VIR_STORAGE_VOL_NETDIR: case VIR_STORAGE_VOL_LAST: /* Keep the compiler quiet, qemuTranslateDiskSourcePool already
I'm about to push patch [1] that will kill the whole qemuBuildVolumeString function. You will need to adapt your patch to do the usefull stuff in qemuTranslateDiskSourcePool present in src/qemu/qemu_conf.c Sorry for the inconvenience. Peter [1]: http://www.redhat.com/archives/libvir-list/2013-December/msg00064.html
participants (2)
-
Adam Walters
-
Peter Krempa