[libvirt] [PATCHv8 0/2] Implement RBD storage pool support
This is an updated version of my RBD storage pool support patch. Relevant changes since v7 are: - Fixed bug where I compared disk->srcpool->mode to VIR_DOMAIN_DISK_TYPE_NETWORK In response to a couple of items Ján mentioned, I decided not to pursue adding anything to qemuxml2argvtest. The reason I decided not to pursue this is there is not really any need. In comparison with other volume types, RBD already has the same test coverage. Namely, a base set of test-cases without using storage pools, plus a generic storage pool test. In addition, I also attempted to implement a proper check for expected vs actual secret usage, but at that particular point in the code, Ján was correct that the pool definition has not yet been parsed. Thus, I had to keep my bypass. Adam Walters (2): qemu: conf Implement domain RBD storage pool support domain: conf: Fix secret type checking for network volumes src/conf/domain_conf.c | 9 ++++++-- src/qemu/qemu_conf.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 3 deletions(-) -- 1.8.5.3
This patch adds a helper function, qemuAddRBDPoolSourceHost, and implements the usage of this function to allow RBD storage pools in QEMU domain XML. The new function grabs RBD monitor hosts from the storage pool definition and applies them to the domain's disk definition at runtime. This function is used by my modifications to qemuTranslateDiskSourcePool similar to the function used by the iSCSI code. My modifications to qemuTranslateDiskSourcePool is based heavily on the existing iSCSI code, but modified to support RBD install. It will place all relevant information into the domain's disk definition at runtime to allow access to a RBD storage pool. Signed-off-by: Adam Walters <adam@pandorasboxen.com> --- src/qemu/qemu_conf.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2c397b0..629ac62 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1251,6 +1251,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) { @@ -1433,8 +1472,29 @@ 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' file volume")); + goto cleanup; + } + + 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_STORAGE_POOL_MPATH: case VIR_STORAGE_POOL_SHEEPDOG: case VIR_STORAGE_POOL_GLUSTER: case VIR_STORAGE_POOL_LAST: -- 1.8.5.3
The subject would IMO be better as: qemu: support RBD storage pools in 'volume' type disks On 03/03/2014 05:43 PM, Adam Walters wrote:
This patch adds a helper function, qemuAddRBDPoolSourceHost, and implements the usage of this function to allow RBD storage pools in QEMU domain XML.
The new function grabs RBD monitor hosts from the storage pool definition and applies them to the domain's disk definition at runtime. This function is used by my modifications to qemuTranslateDiskSourcePool similar to the function used by the iSCSI code.
My modifications to qemuTranslateDiskSourcePool is based heavily on the existing iSCSI code, but modified to support RBD install. It will place all relevant information into the domain's disk definition at runtime to allow access to a RBD storage pool.
Signed-off-by: Adam Walters <adam@pandorasboxen.com> --- src/qemu/qemu_conf.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2c397b0..629ac62 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1251,6 +1251,45 @@ cleanup: }
static int +qemuAddRBDPoolSourceHost(virDomainDiskDefPtr def, + virStoragePoolDefPtr pooldef) +{ + int ret = -1; + size_t i = 0; + char **tokens = NULL;
Unused variable. The rest looks good to me. Jan
This patch fixes the secret type checking done in the virDomainDiskDefParseXML function. Previously, it would not allow any volumes that utilized a secret. This patch is a simple bypass of the checking code for volumes. Signed-off-by: Adam Walters <adam@pandorasboxen.com> --- src/conf/domain_conf.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1d5cc14..be6742a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5494,7 +5494,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, cur = cur->next; } - if (auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage) { + /* Bypass this check for volumes. On libvirtd start, pool definitions are not + * yet parsed, and thus we can't expect to have a valid expected_secret_usage. + */ + if (auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage && + def->type != VIR_DOMAIN_DISK_TYPE_VOLUME) { virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid secret type '%s'"), virSecretUsageTypeTypeToString(auth_secret_usage)); @@ -18442,7 +18446,8 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK || (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && disk->srcpool && - disk->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT)) + (disk->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT || + disk->srcpool->actualtype == VIR_DOMAIN_DISK_TYPE_NETWORK))) return 0; if (iter(disk, disk->src, 0, opaque) < 0) -- 1.8.5.3
On 03/03/2014 05:43 PM, Adam Walters wrote:
This patch fixes the secret type checking done in the virDomainDiskDefParseXML function. Previously, it would not allow any volumes that utilized a secret. This patch is a simple bypass of the checking code for volumes.
Signed-off-by: Adam Walters <adam@pandorasboxen.com> --- src/conf/domain_conf.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1d5cc14..be6742a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5494,7 +5494,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, cur = cur->next; }
- if (auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage) { + /* Bypass this check for volumes. On libvirtd start, pool definitions are not + * yet parsed, and thus we can't expect to have a valid expected_secret_usage. + */
The storage driver is initialized before QEMU driver. When a domain definition with disk type='volume' is parsed, we don't check if the pool or the volume exists, so we shouldn't check the secret either.
+ if (auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage && + def->type != VIR_DOMAIN_DISK_TYPE_VOLUME) { virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid secret type '%s'"), virSecretUsageTypeTypeToString(auth_secret_usage));
If there is a secret in the pool definition, the secret specified here will get overwritten in qemuTranslateDiskSourcePoolAuth. (which is where the secret type check should be IMO)
@@ -18442,7 +18446,8 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK || (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && disk->srcpool && - disk->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT)) + (disk->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT || + disk->srcpool->actualtype == VIR_DOMAIN_DISK_TYPE_NETWORK)))
This changes the behavior for ISCSI volumees with MODE_HOST or MODE_DEFAULT. I think you need to check pooltype, since the default for iscsi (HOST) is different for rbd (DIRECT)
return 0;
if (iter(disk, disk->src, 0, opaque) < 0)
Jan
participants (2)
-
Adam Walters -
Ján Tomko