Re: [libvirt] [PATCH] Using qemu with sheepdog pool
On 01/22/2014 08:21 AM, joel SIMOES wrote:
From: Joel SIMOES <joel.simoes(a)laposte.net>
A bit more detail in the commit message body might be nice.
---
src/qemu/qemu_conf.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 65 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index ac53f6d..dfafcdc 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1185,6 +1185,56 @@ int qemuDriverAllocateID(virQEMUDriverPtr driver)
return virAtomicIntInc(&driver->nextvmid);
}
+
+static int
+qemuAddSheepPoolSourceHost(virDomainDiskDefPtr def,
+ virStoragePoolDefPtr pooldef)
+{
+ int ret = -1;
+ char **tokens = NULL;
+
+ /* Only support one host */
+ if (pooldef->source.nhost != 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Expected exactly 1 host for the storage pool"));
+ goto cleanup;
+ }
+
+ /* iscsi pool only supports one host */
Incorrect copy-and-paste?
+ def->nhosts = 1;
+
+ if (VIR_ALLOC_N(def->hosts, def->nhosts) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(def->hosts[0].name, pooldef->source.hosts[0].name) < 0)
+ goto cleanup;
+
+ if (virAsprintf(&def->hosts[0].port, "%d",
+ pooldef->source.hosts[0].port ?
+ pooldef->source.hosts[0].port :
+ 7000) < 0)
+ goto cleanup;
+
+
+
+
+
Too much whitespace, including trailing whitespace. Ah, _this_ is the
patch where you later posted an unthreaded cleanup patch; it's better to
squash that into this and post as a v2.
-
- if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI)
{
+
+ if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI
&& !(def->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT
&& pooldef->type == VIR_STORAGE_POOL_SHEEPDOG) ) {
Long line. I'd wrap it as:
if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI
&&
!(def->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT &&
pooldef->type == VIR_STORAGE_POOL_SHEEPDOG)) {
+
virReportError(VIR_ERR_XML_ERROR, "%s",
_("disk source mode is only valid when "
- "storage pool is of iscsi type"));
+ "storage pool is of iscsi type or only direct for sheepdog
"));
Trailing whitespace in the message printed to the user.
- case VIR_STORAGE_POOL_MPATH:
- case VIR_STORAGE_POOL_RBD:
case VIR_STORAGE_POOL_SHEEPDOG:
+ def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK;
+ // force direct mode
We prefer /**/ comments.
I think you're on the right track, and wish I knew more about sheepdog
to actually test these patches. But hopefully this review has been
helpful, and I look forward to seeing a cleaner v2.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)