Re: [PATCHv4 2/2] qemu: conf: Implement RBD storage pool support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I tested the patch, and it didn't work for me (already found the bug, though, and a new patch will be sent shortly). The bug is related to the cGroup code (specifically, the virDomainDiskDefForeachPath function in conf/domain_conf.c). The entire libvirt codebase does not yet utilize the actualtype value in a disk definition. cGroup creation thus fails in that function due to the source path (in the form of "rbd_pool/rbd_image_name" not existing on the file system).

A question prior to creating a new patch, though. Would you prefer I modify virDomainDiskDefForeachPath to also check def->srcpool->actualtype for equality against VIR_DOMAIN_DISK_TYPE_NETWORK, or bypass the check by setting the the access mode on the disk definition to direct (which would work around the bug). I'm sure there is more to the virDomainDiskDefForeachPath function (and other functions, too) that a simple equality check in one spot may not handle, but I'm not intimately familiar enough with the code to really know what will happen.

The only reason I had implemented the switch statement was to handle a possible future use case where it is decided that support for RBD devices through the kernel should be added, which might be considered "host mode." I probably should've asked that question before making a patch, but I figured it was probably better to try and handle possible future use cases, rather than make it work solely for what I currently use. Also, sorry for the delayed response. I had to make a trip out to Tallahassee yesterday for work, which took all day.


On Thu, Dec 5, 2013 at 6:13 AM, Peter Krempa <pkrempa@xxxxxxxxxx> wrote:
From: Adam Walters <adam@xxxxxxxxxxxxxxxxx>

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 | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 2d42c3b..a44c71f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1478,8 +1478,27 @@ 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;
+        }
+        def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK;
+        def->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD;
+
+        if (!(def->src = ""> +            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.4.3


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]