10:43 a.m.
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
10:43 a.m.
New subject: [libvirt] [PATCHv8 1/2] qemu: conf Implement domain RBD storage pool support
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(a)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
7:58 a.m.
New subject: [libvirt] [PATCHv8 1/2] qemu: conf Implement domain RBD storage pool support
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(a)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
10:43 a.m.
New subject: [libvirt] [PATCHv8 2/2] domain: conf: Fix secret type checking for network volumes
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(a)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
7:58 a.m.
New subject: [libvirt] [PATCHv8 2/2] domain: conf: Fix secret type checking for network volumes
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(a)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
3915
days inactive
3917
days old
4 comments
2 participants
participants (2)
-
Adam Walters -
Ján Tomko