diff mbox

[V2,3/4] xenconfig: support xl<->xml conversion of rbd disk devices

Message ID 1455755625-13329-4-git-send-email-jfehlig@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jim Fehlig Feb. 18, 2016, 12:33 a.m. UTC
The target= setting in xl disk configuration can be used to encode
meta info that is meaningful to a backend. Leverage this fact to
support qdisk network disk types such as rbd. E.g. <disk> config
such as

   <disk type='network' device='disk'>
     <driver name='qemu' type='raw'/>
     <source protocol='rbd' name='pool/image'>
       <host name='mon1.example.org' port='6321'/>
       <host name='mon2.example.org' port='6322'/>
       <host name='mon3.example.org' port='6322'/>
     </source>
     <target dev='hdb' bus='ide'/>
     <address type='drive' controller='0' bus='0' target='0' unit='1'/>
   </disk>

can be converted to the following xl config (and vice versa)

  disk = [ "format=raw,vdev=hdb,access=rw,backendtype=qdisk,
            target=rbd:pool/image:auth_supported=none:mon_host=mon1.example.org\\:6321\\;mon2.example.org\\:6322\\;mon3.example.org\\:6322"
         ]

Note that in xl disk config, a literal backslash in target= must
be escaped with a backslash. Conversion of <auth> config is not
handled in this patch, but can be done in a follow-up patch.

Also add a test for the conversions.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---

v2:
Change commit msg, test, and code to escape literal backslash
with a backslash.

 src/xenconfig/xen_xl.c                           | 153 +++++++++++++++++++++--
 tests/xlconfigdata/test-rbd-multihost-noauth.cfg |  26 ++++
 tests/xlconfigdata/test-rbd-multihost-noauth.xml |  51 ++++++++
 tests/xlconfigtest.c                             |   1 +
 4 files changed, 224 insertions(+), 7 deletions(-)

Comments

Ján Tomko Feb. 22, 2016, 2:22 p.m. UTC | #1
On Wed, Feb 17, 2016 at 05:33:44PM -0700, Jim Fehlig wrote:
> The target= setting in xl disk configuration can be used to encode
> meta info that is meaningful to a backend. Leverage this fact to
> support qdisk network disk types such as rbd. E.g. <disk> config
> such as
> 
>    <disk type='network' device='disk'>
>      <driver name='qemu' type='raw'/>
>      <source protocol='rbd' name='pool/image'>
>        <host name='mon1.example.org' port='6321'/>
>        <host name='mon2.example.org' port='6322'/>
>        <host name='mon3.example.org' port='6322'/>
>      </source>
>      <target dev='hdb' bus='ide'/>
>      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
>    </disk>
> 
> can be converted to the following xl config (and vice versa)
> 
>   disk = [ "format=raw,vdev=hdb,access=rw,backendtype=qdisk,
>             target=rbd:pool/image:auth_supported=none:mon_host=mon1.example.org\\:6321\\;mon2.example.org\\:6322\\;mon3.example.org\\:6322"
>          ]
> 
> Note that in xl disk config, a literal backslash in target= must
> be escaped with a backslash. Conversion of <auth> config is not
> handled in this patch, but can be done in a follow-up patch.
> 
> Also add a test for the conversions.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
> 
> v2:
> Change commit msg, test, and code to escape literal backslash
> with a backslash.
> 
>  src/xenconfig/xen_xl.c                           | 153 +++++++++++++++++++++--
>  tests/xlconfigdata/test-rbd-multihost-noauth.cfg |  26 ++++
>  tests/xlconfigdata/test-rbd-multihost-noauth.xml |  51 ++++++++
>  tests/xlconfigtest.c                             |   1 +
>  4 files changed, 224 insertions(+), 7 deletions(-)

> +xenFormatXLDiskSrcNet(virStorageSourcePtr src)
> +{
> +    char *ret = NULL;
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    size_t i;
> +
> +    switch ((virStorageNetProtocol) src->protocol) {
> +    case VIR_STORAGE_NET_PROTOCOL_NBD:
> +    case VIR_STORAGE_NET_PROTOCOL_HTTP:
> +    case VIR_STORAGE_NET_PROTOCOL_HTTPS:
> +    case VIR_STORAGE_NET_PROTOCOL_FTP:
> +    case VIR_STORAGE_NET_PROTOCOL_FTPS:
> +    case VIR_STORAGE_NET_PROTOCOL_TFTP:
> +    case VIR_STORAGE_NET_PROTOCOL_ISCSI:
> +    case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
> +    case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
> +    case VIR_STORAGE_NET_PROTOCOL_LAST:
> +    case VIR_STORAGE_NET_PROTOCOL_NONE:
> +        virReportError(VIR_ERR_NO_SUPPORT,
> +                       _("Unsupported network block protocol '%s'"),
> +                       virStorageNetProtocolTypeToString(src->protocol));
> +        goto cleanup;
> +
> +    case VIR_STORAGE_NET_PROTOCOL_RBD:
> +        if (strchr(src->path, ':')) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("':' not allowed in RBD source volume name '%s'"),
> +                           src->path);
> +            goto cleanup;
> +        }
> +
> +        virBufferStrcat(&buf, "rbd:", src->path, NULL);
> +
> +        virBufferAddLit(&buf, ":auth_supported=none");
> +
> +        if (src->nhosts > 0) {
> +            virBufferAddLit(&buf, ":mon_host=");
> +            for (i = 0; i < src->nhosts; i++) {
> +                if (i)
> +                    virBufferAddLit(&buf, "\\\\;");
> +

You could add the separator unconditionally

> +                /* assume host containing : is ipv6 */
> +                if (strchr(src->hosts[i].name, ':'))
> +                    virBufferEscape(&buf, '\\', ":", "[%s]",
> +                                    src->hosts[i].name);
> +                else
> +                    virBufferAsprintf(&buf, "%s", src->hosts[i].name);
> +
> +                if (src->hosts[i].port)
> +                    virBufferAsprintf(&buf, "\\\\:%s", src->hosts[i].port);
> +            }

And use virBufferTrim after the cycle.

ACK either way.

Jan
diff mbox

Patch

diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index f3e8b55..585ef9b 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -246,6 +246,32 @@  xenParseXLSpice(virConfPtr conf, virDomainDefPtr def)
     return -1;
 }
 
+
+static int
+xenParseXLDiskSrc(virDomainDiskDefPtr disk, char *srcstr)
+{
+    char *tmpstr = NULL;
+    int ret = -1;
+
+    if (STRPREFIX(srcstr, "rbd:")) {
+        tmpstr = virStringReplace(srcstr, "\\\\", "\\");
+
+        virDomainDiskSetType(disk, VIR_STORAGE_TYPE_NETWORK);
+        disk->src->protocol = VIR_STORAGE_NET_PROTOCOL_RBD;
+        ret = virStorageSourceParseRBDColonString(tmpstr, disk->src);
+    } else {
+        if (virDomainDiskSetSource(disk, srcstr) < 0)
+            goto cleanup;
+
+        ret = 0;
+    }
+
+ cleanup:
+    VIR_FREE(tmpstr);
+    return ret;
+}
+
+
 /*
  * For details on xl disk config syntax, see
  * docs/misc/xl-disk-configuration.txt in the Xen sources.  The important
@@ -311,12 +337,12 @@  xenParseXLDisk(virConfPtr conf, virDomainDefPtr def)
             if (!(disk = virDomainDiskDefNew(NULL)))
                 goto fail;
 
+            if (xenParseXLDiskSrc(disk, libxldisk->pdev_path) < 0)
+                goto fail;
+
             if (VIR_STRDUP(disk->dst, libxldisk->vdev) < 0)
                 goto fail;
 
-            if (virDomainDiskSetSource(disk, libxldisk->pdev_path) < 0)
-                goto fail;
-
             disk->src->readonly = !libxldisk->readwrite;
             disk->removable = libxldisk->removable;
 
@@ -358,7 +384,8 @@  xenParseXLDisk(virConfPtr conf, virDomainDefPtr def)
                 case LIBXL_DISK_BACKEND_UNKNOWN:
                     if (virDomainDiskSetDriver(disk, "qemu") < 0)
                         goto fail;
-                    virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
+                    if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NONE)
+                        virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
                     break;
 
                 case LIBXL_DISK_BACKEND_TAP:
@@ -578,14 +605,115 @@  xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
 }
 
 
+static char *
+xenFormatXLDiskSrcNet(virStorageSourcePtr src)
+{
+    char *ret = NULL;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    size_t i;
+
+    switch ((virStorageNetProtocol) src->protocol) {
+    case VIR_STORAGE_NET_PROTOCOL_NBD:
+    case VIR_STORAGE_NET_PROTOCOL_HTTP:
+    case VIR_STORAGE_NET_PROTOCOL_HTTPS:
+    case VIR_STORAGE_NET_PROTOCOL_FTP:
+    case VIR_STORAGE_NET_PROTOCOL_FTPS:
+    case VIR_STORAGE_NET_PROTOCOL_TFTP:
+    case VIR_STORAGE_NET_PROTOCOL_ISCSI:
+    case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
+    case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
+    case VIR_STORAGE_NET_PROTOCOL_LAST:
+    case VIR_STORAGE_NET_PROTOCOL_NONE:
+        virReportError(VIR_ERR_NO_SUPPORT,
+                       _("Unsupported network block protocol '%s'"),
+                       virStorageNetProtocolTypeToString(src->protocol));
+        goto cleanup;
+
+    case VIR_STORAGE_NET_PROTOCOL_RBD:
+        if (strchr(src->path, ':')) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("':' not allowed in RBD source volume name '%s'"),
+                           src->path);
+            goto cleanup;
+        }
+
+        virBufferStrcat(&buf, "rbd:", src->path, NULL);
+
+        virBufferAddLit(&buf, ":auth_supported=none");
+
+        if (src->nhosts > 0) {
+            virBufferAddLit(&buf, ":mon_host=");
+            for (i = 0; i < src->nhosts; i++) {
+                if (i)
+                    virBufferAddLit(&buf, "\\\\;");
+
+                /* assume host containing : is ipv6 */
+                if (strchr(src->hosts[i].name, ':'))
+                    virBufferEscape(&buf, '\\', ":", "[%s]",
+                                    src->hosts[i].name);
+                else
+                    virBufferAsprintf(&buf, "%s", src->hosts[i].name);
+
+                if (src->hosts[i].port)
+                    virBufferAsprintf(&buf, "\\\\:%s", src->hosts[i].port);
+            }
+        }
+
+        if (virBufferCheckError(&buf) < 0)
+            goto cleanup;
+
+        ret = virBufferContentAndReset(&buf);
+        break;
+    }
+
+ cleanup:
+    virBufferFreeAndReset(&buf);
+
+    return ret;
+}
+
+
+static int
+xenFormatXLDiskSrc(virStorageSourcePtr src, char **srcstr)
+{
+    int actualType = virStorageSourceGetActualType(src);
+
+    *srcstr = NULL;
+
+    if (virStorageSourceIsEmpty(src))
+        return 0;
+
+    switch ((virStorageType) actualType) {
+    case VIR_STORAGE_TYPE_BLOCK:
+    case VIR_STORAGE_TYPE_FILE:
+    case VIR_STORAGE_TYPE_DIR:
+        if (VIR_STRDUP(*srcstr, src->path) < 0)
+            return -1;
+        break;
+
+    case VIR_STORAGE_TYPE_NETWORK:
+        if (!(*srcstr = xenFormatXLDiskSrcNet(src)))
+            return -1;
+        break;
+
+    case VIR_STORAGE_TYPE_VOLUME:
+    case VIR_STORAGE_TYPE_NONE:
+    case VIR_STORAGE_TYPE_LAST:
+        break;
+    }
+
+    return 0;
+}
+
+
 static int
 xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     virConfValuePtr val, tmp;
-    const char *src = virDomainDiskGetSource(disk);
     int format = virDomainDiskGetFormat(disk);
     const char *driver = virDomainDiskGetDriver(disk);
+    char *target = NULL;
 
     /* format */
     virBufferAddLit(&buf, "format=");
@@ -637,8 +765,18 @@  xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk)
     if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
         virBufferAddLit(&buf, "devtype=cdrom,");
 
-    /* target */
-    virBufferAsprintf(&buf, "target=%s", src);
+    /*
+     * target
+     * From $xensrc/docs/misc/xl-disk-configuration.txt:
+     * When this parameter is specified by name, ie with the "target="
+     * syntax in the configuration file, it consumes the whole rest of the
+     * <diskspec> including trailing whitespaces.  Therefore in that case
+     * it must come last.
+     */
+    if (xenFormatXLDiskSrc(disk->src, &target) < 0)
+        goto cleanup;
+
+    virBufferAsprintf(&buf, "target=%s", target);
 
     if (virBufferCheckError(&buf) < 0)
         goto cleanup;
@@ -658,6 +796,7 @@  xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk)
     return 0;
 
  cleanup:
+    VIR_FREE(target);
     virBufferFreeAndReset(&buf);
     return -1;
 }
diff --git a/tests/xlconfigdata/test-rbd-multihost-noauth.cfg b/tests/xlconfigdata/test-rbd-multihost-noauth.cfg
new file mode 100644
index 0000000..cfe00e5
--- /dev/null
+++ b/tests/xlconfigdata/test-rbd-multihost-noauth.cfg
@@ -0,0 +1,26 @@ 
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+pae = 1
+acpi = 1
+apic = 1
+hap = 0
+viridian = 0
+rtc_timeoffset = 0
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+device_model = "/usr/lib/xen/bin/qemu-dm"
+sdl = 0
+vnc = 1
+vncunused = 1
+vnclisten = "127.0.0.1"
+vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ]
+parallel = "none"
+serial = "none"
+builder = "hvm"
+boot = "d"
+disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2", "format=raw,vdev=hdb,access=rw,backendtype=qdisk,target=rbd:pool/image:auth_supported=none:mon_host=mon1.example.org\\:6321\\;mon2.example.org\\:6322\\;mon3.example.org\\:6322" ]
diff --git a/tests/xlconfigdata/test-rbd-multihost-noauth.xml b/tests/xlconfigdata/test-rbd-multihost-noauth.xml
new file mode 100644
index 0000000..720265e
--- /dev/null
+++ b/tests/xlconfigdata/test-rbd-multihost-noauth.xml
@@ -0,0 +1,51 @@ 
+<domain type='xen'>
+  <name>XenGuest2</name>
+  <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>592896</memory>
+  <currentMemory unit='KiB'>403456</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='xenfv'>hvm</type>
+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
+    <boot dev='cdrom'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='variable' adjustment='0' basis='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
+    <disk type='block' device='disk'>
+      <driver name='phy' type='raw'/>
+      <source dev='/dev/HostVG/XenGuest2'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source protocol='rbd' name='pool/image'>
+        <host name='mon1.example.org' port='6321'/>
+        <host name='mon2.example.org' port='6322'/>
+        <host name='mon3.example.org' port='6322'/>
+      </source>
+      <target dev='hdb' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='00:16:3e:66:92:9c'/>
+      <source bridge='xenbr1'/>
+      <script path='vif-bridge'/>
+      <model type='e1000'/>
+    </interface>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'>
+      <listen type='address' address='127.0.0.1'/>
+    </graphics>
+  </devices>
+</domain>
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index 13b99f2..2668a76 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -217,6 +217,7 @@  mymain(void)
     DO_TEST("paravirt-cmdline");
     DO_TEST_FORMAT("paravirt-cmdline-extra-root");
     DO_TEST_FORMAT("paravirt-cmdline-bogus-extra-root");
+    DO_TEST("rbd-multihost-noauth");
 
 #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
     DO_TEST("fullvirt-multiusb");