[libvirt] [PATCH V2 0/4] libxl: support qemu's network-based block backends
xl/libxl already supports qemu's network-based block backends such as nbd and rbd. libvirt has supported configuring network disks for long time too. This series marries the two in the libxl driver and in the xl<->xml converter. Only rbd supported is added in this series. Support for other backends such as nbd and iscsi can be added as a follow-up improvement. Patch 1 is super trivial and contains no functional changes. Patch 2 changes the xl disk configuration produced by the xml->xl converter to use the formal key=value syntax described in xl-disk-configuration.txt. Patch 3 adds support for converting rbd info between xl and xml config formats. Patch 4 adds support for rbd disks in the libxl driver. In V2: Change commit msg, test, and code in patch3 to escape literal backslashes with a backslash. Jim Fehlig (4): xenconfig: replace text 'xm' with 'xl' in xlconfigtest xenconfig: produce key=value disk config syntax in xl formatter xenconfig: support xl<->xml conversion of rbd disk devices libxl: add support for rbd qdisk src/libxl/libxl_conf.c | 192 ++++++++++++++++++++- src/xenconfig/xen_xl.c | 176 +++++++++++++++++-- .../test-disk-positional-parms-full.cfg | 26 +++ .../test-disk-positional-parms-full.xml | 54 ++++++ .../test-disk-positional-parms-partial.cfg | 26 +++ .../test-disk-positional-parms-partial.xml | 54 ++++++ .../test-fullvirt-direct-kernel-boot.cfg | 2 +- tests/xlconfigdata/test-fullvirt-multiusb.cfg | 2 +- tests/xlconfigdata/test-new-disk.cfg | 2 +- tests/xlconfigdata/test-paravirt-cmdline.cfg | 2 +- tests/xlconfigdata/test-paravirt-maxvcpus.cfg | 2 +- tests/xlconfigdata/test-rbd-multihost-noauth.cfg | 26 +++ tests/xlconfigdata/test-rbd-multihost-noauth.xml | 51 ++++++ tests/xlconfigdata/test-spice-features.cfg | 2 +- tests/xlconfigdata/test-spice.cfg | 2 +- tests/xlconfigdata/test-vif-rate.cfg | 2 +- tests/xlconfigtest.c | 37 ++-- 17 files changed, 618 insertions(+), 40 deletions(-) create mode 100644 tests/xlconfigdata/test-disk-positional-parms-full.cfg create mode 100644 tests/xlconfigdata/test-disk-positional-parms-full.xml create mode 100644 tests/xlconfigdata/test-disk-positional-parms-partial.cfg create mode 100644 tests/xlconfigdata/test-disk-positional-parms-partial.xml create mode 100644 tests/xlconfigdata/test-rbd-multihost-noauth.cfg create mode 100644 tests/xlconfigdata/test-rbd-multihost-noauth.xml -- 2.1.4
While at it, improve a few comments. No functional change. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- tests/xlconfigtest.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 4b2f28f..aa53ed8 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -1,5 +1,5 @@ /* - * xlconfigtest.c: Test backend for xl_internal config file handling + * xlconfigtest.c: Test xl.cfg(5) <-> domXML config conversions * * Copyright (C) 2007, 2010-2011, 2014 Red Hat, Inc. * Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany. @@ -42,20 +42,22 @@ static virCapsPtr caps; static virDomainXMLOptionPtr xmlopt; + /* - * parses the xml, creates a domain def and compare with equivalent xm config + * Parses domXML to virDomainDef object, which is then converted to xl.cfg(5) + * config and compared with expected config. */ static int -testCompareParseXML(const char *xmcfg, const char *xml) +testCompareParseXML(const char *xlcfg, const char *xml) { - char *gotxmcfgData = NULL; + char *gotxlcfgData = NULL; virConfPtr conf = NULL; virConnectPtr conn = NULL; int wrote = 4096; int ret = -1; virDomainDefPtr def = NULL; - if (VIR_ALLOC_N(gotxmcfgData, wrote) < 0) + if (VIR_ALLOC_N(gotxlcfgData, wrote) < 0) goto fail; conn = virGetConnect(); @@ -73,17 +75,17 @@ testCompareParseXML(const char *xmcfg, const char *xml) if (!(conf = xenFormatXL(def, conn))) goto fail; - if (virConfWriteMem(gotxmcfgData, &wrote, conf) < 0) + if (virConfWriteMem(gotxlcfgData, &wrote, conf) < 0) goto fail; - gotxmcfgData[wrote] = '\0'; + gotxlcfgData[wrote] = '\0'; - if (virtTestCompareToFile(gotxmcfgData, xmcfg) < 0) + if (virtTestCompareToFile(gotxlcfgData, xlcfg) < 0) goto fail; ret = 0; fail: - VIR_FREE(gotxmcfgData); + VIR_FREE(gotxlcfgData); if (conf) virConfFree(conf); virDomainDefFree(def); @@ -91,13 +93,15 @@ testCompareParseXML(const char *xmcfg, const char *xml) return ret; } + /* - * parses the xl config, develops domain def and compares with equivalent xm config + * Parses xl.cfg(5) config to virDomainDef object, which is then converted to + * domXML and compared to expected XML. */ static int -testCompareFormatXML(const char *xmcfg, const char *xml) +testCompareFormatXML(const char *xlcfg, const char *xml) { - char *xmcfgData = NULL; + char *xlcfgData = NULL; char *gotxml = NULL; virConfPtr conf = NULL; int ret = -1; @@ -107,10 +111,10 @@ testCompareFormatXML(const char *xmcfg, const char *xml) conn = virGetConnect(); if (!conn) goto fail; - if (virtTestLoadFile(xmcfg, &xmcfgData) < 0) + if (virtTestLoadFile(xlcfg, &xlcfgData) < 0) goto fail; - if (!(conf = virConfReadMem(xmcfgData, strlen(xmcfgData), 0))) + if (!(conf = virConfReadMem(xlcfgData, strlen(xlcfgData), 0))) goto fail; if (!(def = xenParseXL(conf, caps, xmlopt))) @@ -128,7 +132,7 @@ testCompareFormatXML(const char *xmcfg, const char *xml) fail: if (conf) virConfFree(conf); - VIR_FREE(xmcfgData); + VIR_FREE(xlcfgData); VIR_FREE(gotxml); virDomainDefFree(def); virObjectUnref(conn); -- 2.1.4
On Wed, Feb 17, 2016 at 05:33:42PM -0700, Jim Fehlig wrote:
While at it, improve a few comments. No functional change.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- tests/xlconfigtest.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-)
ACK, cosmetic change. Jan
The most formal form of xl disk configuration uses key=value syntax to define each configuration item, e.g. format=raw, vdev=xvda, access=rw, backendtype=phy, target=disksrc Change the xl disk formatter to produce this syntax, which allows target= to contain meta info needed to setup a network-based disksrc (e.g. rbd, nbd, iscsi). For details on xl disk config format, see $xen-src/docs/misc/xl-disk-configuration.txt Update the disk config in the tests to use the formal syntax. But add tests to ensure disks specified with the positional parameter syntax are correctly converted to <disk> XML. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/xenconfig/xen_xl.c | 27 ++++++----- .../test-disk-positional-parms-full.cfg | 26 +++++++++++ .../test-disk-positional-parms-full.xml | 54 ++++++++++++++++++++++ .../test-disk-positional-parms-partial.cfg | 26 +++++++++++ .../test-disk-positional-parms-partial.xml | 54 ++++++++++++++++++++++ .../test-fullvirt-direct-kernel-boot.cfg | 2 +- tests/xlconfigdata/test-fullvirt-multiusb.cfg | 2 +- tests/xlconfigdata/test-new-disk.cfg | 2 +- tests/xlconfigdata/test-paravirt-cmdline.cfg | 2 +- tests/xlconfigdata/test-paravirt-maxvcpus.cfg | 2 +- tests/xlconfigdata/test-spice-features.cfg | 2 +- tests/xlconfigdata/test-spice.cfg | 2 +- tests/xlconfigdata/test-vif-rate.cfg | 2 +- tests/xlconfigtest.c | 2 + 14 files changed, 186 insertions(+), 19 deletions(-) diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index be194e3..f3e8b55 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -587,9 +587,8 @@ xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk) int format = virDomainDiskGetFormat(disk); const char *driver = virDomainDiskGetDriver(disk); - /* target */ - virBufferAsprintf(&buf, "%s,", src); /* format */ + virBufferAddLit(&buf, "format="); switch (format) { case VIR_STORAGE_FILE_RAW: virBufferAddLit(&buf, "raw,"); @@ -609,31 +608,37 @@ xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk) } /* device */ - virBufferAdd(&buf, disk->dst, -1); - - virBufferAddLit(&buf, ","); + virBufferAsprintf(&buf, "vdev=%s,", disk->dst); + /* access */ + virBufferAddLit(&buf, "access="); if (disk->src->readonly) - virBufferAddLit(&buf, "r,"); + virBufferAddLit(&buf, "ro,"); else if (disk->src->shared) virBufferAddLit(&buf, "!,"); else - virBufferAddLit(&buf, "w,"); + virBufferAddLit(&buf, "rw,"); if (disk->transient) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("transient disks not supported yet")); goto cleanup; } + /* backendtype */ + virBufferAddLit(&buf, "backendtype="); if (STREQ_NULLABLE(driver, "qemu")) - virBufferAddLit(&buf, "backendtype=qdisk"); + virBufferAddLit(&buf, "qdisk,"); else if (STREQ_NULLABLE(driver, "tap")) - virBufferAddLit(&buf, "backendtype=tap"); + virBufferAddLit(&buf, "tap,"); else if (STREQ_NULLABLE(driver, "phy")) - virBufferAddLit(&buf, "backendtype=phy"); + virBufferAddLit(&buf, "phy,"); + /* devtype */ if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) - virBufferAddLit(&buf, ",devtype=cdrom"); + virBufferAddLit(&buf, "devtype=cdrom,"); + + /* target */ + virBufferAsprintf(&buf, "target=%s", src); if (virBufferCheckError(&buf) < 0) goto cleanup; diff --git a/tests/xlconfigdata/test-disk-positional-parms-full.cfg b/tests/xlconfigdata/test-disk-positional-parms-full.cfg new file mode 100644 index 0000000..026e451 --- /dev/null +++ b/tests/xlconfigdata/test-disk-positional-parms-full.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 = [ "/dev/HostVG/XenGuest2,raw,hda,rw,backendtype=phy", "/var/lib/libvirt/images/XenGuest2-home,qcow2,hdb,rw", "/root/boot.iso,raw,hdc,ro,devtype=cdrom" ] diff --git a/tests/xlconfigdata/test-disk-positional-parms-full.xml b/tests/xlconfigdata/test-disk-positional-parms-full.xml new file mode 100644 index 0000000..49f6dbe --- /dev/null +++ b/tests/xlconfigdata/test-disk-positional-parms-full.xml @@ -0,0 +1,54 @@ +<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='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/XenGuest2-home'/> + <target dev='hdb' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/root/boot.iso'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </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/xlconfigdata/test-disk-positional-parms-partial.cfg b/tests/xlconfigdata/test-disk-positional-parms-partial.cfg new file mode 100644 index 0000000..0591037 --- /dev/null +++ b/tests/xlconfigdata/test-disk-positional-parms-partial.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 = [ "/dev/HostVG/XenGuest2,,hda,,backendtype=phy", "/var/lib/libvirt/images/XenGuest2-home,,hdb,,", "/root/boot.iso,,hdc,,devtype=cdrom" ] diff --git a/tests/xlconfigdata/test-disk-positional-parms-partial.xml b/tests/xlconfigdata/test-disk-positional-parms-partial.xml new file mode 100644 index 0000000..3268295 --- /dev/null +++ b/tests/xlconfigdata/test-disk-positional-parms-partial.xml @@ -0,0 +1,54 @@ +<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='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/XenGuest2-home'/> + <target dev='hdb' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/root/boot.iso'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </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/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg index 32b08e1..9ebbc89 100644 --- a/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg +++ b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg @@ -27,4 +27,4 @@ kernel = "/tmp/vmlinuz" ramdisk = "/tmp/initrd" cmdline = "ignore_loglvl" boot = "d" -disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,backendtype=phy", "/root/boot.iso,raw,hdc,r,backendtype=qdisk,devtype=cdrom" ] +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2", "format=raw,vdev=hdc,access=ro,backendtype=qdisk,devtype=cdrom,target=/root/boot.iso" ] diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.cfg b/tests/xlconfigdata/test-fullvirt-multiusb.cfg index d0482a8..097de88 100755 --- a/tests/xlconfigdata/test-fullvirt-multiusb.cfg +++ b/tests/xlconfigdata/test-fullvirt-multiusb.cfg @@ -24,6 +24,6 @@ parallel = "none" serial = "none" builder = "hvm" boot = "d" -disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,backendtype=phy", "/root/boot.iso,raw,hdc,r,backendtype=qdisk,devtype=cdrom" ] +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2", "format=raw,vdev=hdc,access=ro,backendtype=qdisk,devtype=cdrom,target=/root/boot.iso" ] usb = 1 usbdevice = [ "mouse", "tablet" ] diff --git a/tests/xlconfigdata/test-new-disk.cfg b/tests/xlconfigdata/test-new-disk.cfg index 9b9fb36..4a5b66e 100644 --- a/tests/xlconfigdata/test-new-disk.cfg +++ b/tests/xlconfigdata/test-new-disk.cfg @@ -23,4 +23,4 @@ parallel = "none" serial = "none" builder = "hvm" boot = "d" -disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,backendtype=phy", "/var/lib/libvirt/images/XenGuest2-home,qcow2,hdb,w,backendtype=qdisk", "/root/boot.iso,raw,hdc,r,backendtype=qdisk,devtype=cdrom" ] +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2", "format=qcow2,vdev=hdb,access=rw,backendtype=qdisk,target=/var/lib/libvirt/images/XenGuest2-home", "format=raw,vdev=hdc,access=ro,backendtype=qdisk,devtype=cdrom,target=/root/boot.iso" ] diff --git a/tests/xlconfigdata/test-paravirt-cmdline.cfg b/tests/xlconfigdata/test-paravirt-cmdline.cfg index c512a05..e82d900 100644 --- a/tests/xlconfigdata/test-paravirt-cmdline.cfg +++ b/tests/xlconfigdata/test-paravirt-cmdline.cfg @@ -11,4 +11,4 @@ vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge" ] kernel = "/tmp/vmlinuz" ramdisk = "/tmp/initrd" cmdline = "root=/dev/xvda1 console=hvc0" -disk = [ "/dev/HostVG/XenGuest2,raw,xvda,w,backendtype=qdisk" ] +disk = [ "format=raw,vdev=xvda,access=rw,backendtype=qdisk,target=/dev/HostVG/XenGuest2" ] diff --git a/tests/xlconfigdata/test-paravirt-maxvcpus.cfg b/tests/xlconfigdata/test-paravirt-maxvcpus.cfg index d506b75..8316774 100644 --- a/tests/xlconfigdata/test-paravirt-maxvcpus.cfg +++ b/tests/xlconfigdata/test-paravirt-maxvcpus.cfg @@ -10,4 +10,4 @@ on_reboot = "restart" on_crash = "preserve" vif = [ "mac=5a:36:0e:be:00:09" ] bootloader = "/usr/bin/pygrub" -disk = [ "/var/lib/xen/images/debian/disk.qcow2,qcow2,xvda,w,backendtype=qdisk" ] +disk = [ "format=qcow2,vdev=xvda,access=rw,backendtype=qdisk,target=/var/lib/xen/images/debian/disk.qcow2" ] diff --git a/tests/xlconfigdata/test-spice-features.cfg b/tests/xlconfigdata/test-spice-features.cfg index 152cb27..8c1ca18 100644 --- a/tests/xlconfigdata/test-spice-features.cfg +++ b/tests/xlconfigdata/test-spice-features.cfg @@ -19,7 +19,7 @@ parallel = "none" serial = "none" builder = "hvm" boot = "d" -disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,backendtype=phy", "/root/boot.iso,raw,hdc,r,backendtype=qdisk,devtype=cdrom" ] +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2", "format=raw,vdev=hdc,access=ro,backendtype=qdisk,devtype=cdrom,target=/root/boot.iso" ] sdl = 0 vnc = 0 spice = 1 diff --git a/tests/xlconfigdata/test-spice.cfg b/tests/xlconfigdata/test-spice.cfg index 1a96114..84b3ae6 100644 --- a/tests/xlconfigdata/test-spice.cfg +++ b/tests/xlconfigdata/test-spice.cfg @@ -19,7 +19,7 @@ parallel = "none" serial = "none" builder = "hvm" boot = "d" -disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,backendtype=phy", "/root/boot.iso,raw,hdc,r,backendtype=qdisk,devtype=cdrom" ] +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2", "format=raw,vdev=hdc,access=ro,backendtype=qdisk,devtype=cdrom,target=/root/boot.iso" ] sdl = 0 vnc = 0 spice = 1 diff --git a/tests/xlconfigdata/test-vif-rate.cfg b/tests/xlconfigdata/test-vif-rate.cfg index c5484dc..44bfa3c 100644 --- a/tests/xlconfigdata/test-vif-rate.cfg +++ b/tests/xlconfigdata/test-vif-rate.cfg @@ -23,4 +23,4 @@ parallel = "none" serial = "none" builder = "hvm" boot = "d" -disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,backendtype=phy", "/var/lib/libvirt/images/XenGuest2-home,qcow2,hdb,w,backendtype=qdisk", "/root/boot.iso,raw,hdc,r,backendtype=qdisk,devtype=cdrom" ] +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2", "format=qcow2,vdev=hdb,access=rw,backendtype=qdisk,target=/var/lib/libvirt/images/XenGuest2-home", "format=raw,vdev=hdc,access=ro,backendtype=qdisk,devtype=cdrom,target=/root/boot.iso" ] diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index aa53ed8..13b99f2 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -208,6 +208,8 @@ mymain(void) DO_TEST("paravirt-maxvcpus"); DO_TEST("new-disk"); + DO_TEST_FORMAT("disk-positional-parms-full"); + DO_TEST_FORMAT("disk-positional-parms-partial"); DO_TEST("spice"); DO_TEST("spice-features"); DO_TEST("vif-rate"); -- 2.1.4
On Wed, Feb 17, 2016 at 05:33:43PM -0700, Jim Fehlig wrote:
The most formal form of xl disk configuration uses key=value syntax to define each configuration item, e.g.
format=raw, vdev=xvda, access=rw, backendtype=phy, target=disksrc
Change the xl disk formatter to produce this syntax, which allows target= to contain meta info needed to setup a network-based disksrc (e.g. rbd, nbd, iscsi). For details on xl disk config format, see $xen-src/docs/misc/xl-disk-configuration.txt
Update the disk config in the tests to use the formal syntax. But add tests to ensure disks specified with the positional parameter syntax are correctly converted to <disk> XML.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/xenconfig/xen_xl.c | 27 ++++++----- .../test-disk-positional-parms-full.cfg | 26 +++++++++++ .../test-disk-positional-parms-full.xml | 54 ++++++++++++++++++++++ .../test-disk-positional-parms-partial.cfg | 26 +++++++++++ .../test-disk-positional-parms-partial.xml | 54 ++++++++++++++++++++++ .../test-fullvirt-direct-kernel-boot.cfg | 2 +- tests/xlconfigdata/test-fullvirt-multiusb.cfg | 2 +- tests/xlconfigdata/test-new-disk.cfg | 2 +- tests/xlconfigdata/test-paravirt-cmdline.cfg | 2 +- tests/xlconfigdata/test-paravirt-maxvcpus.cfg | 2 +- tests/xlconfigdata/test-spice-features.cfg | 2 +- tests/xlconfigdata/test-spice.cfg | 2 +- tests/xlconfigdata/test-vif-rate.cfg | 2 +- tests/xlconfigtest.c | 2 + 14 files changed, 186 insertions(+), 19 deletions(-)
ACK Jan
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(-) 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"); -- 2.1.4
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
xl/libxl already supports qemu's network-based block backends such as nbd and rbd. libvirt has supported configuring such <disk>s for long time too. This patch adds support for rbd disks in the libxl driver by generating a rbd device URL from the virDomainDiskDef object. The URL is passed to libxl via the pdev_path field of libxl_device_disk struct. libxl then passes the URL to qemu for cosumption by the rbd backend. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 191 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 48b77d2..5133299 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -46,6 +46,7 @@ #include "libxl_conf.h" #include "libxl_utils.h" #include "virstoragefile.h" +#include "base64.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -920,17 +921,206 @@ libxlDomainGetEmulatorType(const virDomainDef *def) return ret; } +static char * +libxlGetSecretString(virConnectPtr conn, + const char *scheme, + bool encoded, + virStorageAuthDefPtr authdef, + virSecretUsageType secretUsageType) +{ + size_t secret_size; + virSecretPtr sec = NULL; + char *secret = NULL; + char uuidStr[VIR_UUID_STRING_BUFLEN]; + + /* look up secret */ + switch (authdef->secretType) { + case VIR_STORAGE_SECRET_TYPE_UUID: + sec = virSecretLookupByUUID(conn, authdef->secret.uuid); + virUUIDFormat(authdef->secret.uuid, uuidStr); + break; + case VIR_STORAGE_SECRET_TYPE_USAGE: + sec = virSecretLookupByUsage(conn, secretUsageType, + authdef->secret.usage); + break; + } + + if (!sec) { + if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { + virReportError(VIR_ERR_NO_SECRET, + _("%s no secret matches uuid '%s'"), + scheme, uuidStr); + } else { + virReportError(VIR_ERR_NO_SECRET, + _("%s no secret matches usage value '%s'"), + scheme, authdef->secret.usage); + } + goto cleanup; + } + + secret = (char *)conn->secretDriver->secretGetValue(sec, &secret_size, 0, + VIR_SECRET_GET_VALUE_INTERNAL_CALL); + if (!secret) { + if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("could not get value of the secret for " + "username '%s' using uuid '%s'"), + authdef->username, uuidStr); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("could not get value of the secret for " + "username '%s' using usage value '%s'"), + authdef->username, authdef->secret.usage); + } + goto cleanup; + } + + if (encoded) { + char *base64 = NULL; + + base64_encode_alloc(secret, secret_size, &base64); + VIR_FREE(secret); + if (!base64) { + virReportOOMError(); + goto cleanup; + } + secret = base64; + } + + cleanup: + virObjectUnref(sec); + return secret; +} + +static char * +libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src, + const char *username, + const char *secret) +{ + 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); + + if (username) { + virBufferEscape(&buf, '\\', ":", ":id=%s", username); + virBufferEscape(&buf, '\\', ":", + ":key=%s:auth_supported=cephx\\;none", + secret); + } else { + 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 (src->configFile) + virBufferEscape(&buf, '\\', ":", ":conf=%s", src->configFile); + + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + ret = virBufferContentAndReset(&buf); + break; + } + + cleanup: + virBufferFreeAndReset(&buf); + return ret; +} + +static int +libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) +{ + virConnectPtr conn = NULL; + char *secret = NULL; + char *username = NULL; + int ret = -1; + + *srcstr = NULL; + if (src->auth && src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { + const char *protocol = virStorageNetProtocolTypeToString(src->protocol); + + username = src->auth->username; + if (!(conn = virConnectOpen("xen:///system"))) + goto cleanup; + + if (!(secret = libxlGetSecretString(conn, + protocol, + true, + src->auth, + VIR_SECRET_USAGE_TYPE_CEPH))) + goto cleanup; + } + + if (!(*srcstr = libxlMakeNetworkDiskSrcStr(src, username, secret))) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(secret); + virObjectUnref(conn); + return ret; +} int libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) { const char *driver; int format; + int actual_type = virStorageSourceGetActualType(l_disk->src); libxl_device_disk_init(x_disk); - if (VIR_STRDUP(x_disk->pdev_path, virDomainDiskGetSource(l_disk)) < 0) + if (actual_type == VIR_STORAGE_TYPE_NETWORK) { + if (libxlMakeNetworkDiskSrc(l_disk->src, &x_disk->pdev_path) < 0) + return -1; + } else { + if (VIR_STRDUP(x_disk->pdev_path, virDomainDiskGetSource(l_disk)) < 0) return -1; + } if (VIR_STRDUP(x_disk->vdev, l_disk->dst) < 0) return -1; -- 2.1.4
On Wed, Feb 17, 2016 at 05:33:45PM -0700, Jim Fehlig wrote:
xl/libxl already supports qemu's network-based block backends such as nbd and rbd. libvirt has supported configuring such <disk>s for long time too. This patch adds support for rbd disks in the libxl driver by generating a rbd device URL from the virDomainDiskDef object. The URL is passed to libxl via the pdev_path field of libxl_device_disk struct. libxl then passes the URL to qemu for cosumption by the rbd backend.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 191 insertions(+), 1 deletion(-)
ACK with the whitespace fix.
+ +static int +libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) +{ + virConnectPtr conn = NULL; + char *secret = NULL; + char *username = NULL; + int ret = -1; + + *srcstr = NULL; + if (src->auth && src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { + const char *protocol = virStorageNetProtocolTypeToString(src->protocol); + + username = src->auth->username; + if (!(conn = virConnectOpen("xen:///system"))) + goto cleanup; +
Opening a connection feels out of place in this function, but I see it's already done for NICs. It would be nice to reuse it as is done in the qemu driver.
+ if (!(secret = libxlGetSecretString(conn, + protocol, + true, + src->auth, + VIR_SECRET_USAGE_TYPE_CEPH))) + goto cleanup; + } + + if (!(*srcstr = libxlMakeNetworkDiskSrcStr(src, username, secret))) + goto cleanup;
The indentation looks off here. Jan
On 02/22/2016 07:35 AM, Ján Tomko wrote:
On Wed, Feb 17, 2016 at 05:33:45PM -0700, Jim Fehlig wrote:
xl/libxl already supports qemu's network-based block backends such as nbd and rbd. libvirt has supported configuring such <disk>s for long time too. This patch adds support for rbd disks in the libxl driver by generating a rbd device URL from the virDomainDiskDef object. The URL is passed to libxl via the pdev_path field of libxl_device_disk struct. libxl then passes the URL to qemu for cosumption by the rbd backend.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 191 insertions(+), 1 deletion(-)
ACK with the whitespace fix.
Hi Jan, Sorry I missed your comments before pushing this series. I'll address them in a follow-up. Thanks for your time reviewing these changes! Regards, Jim
Ján Tomko wrote:
On Wed, Feb 17, 2016 at 05:33:45PM -0700, Jim Fehlig wrote:
xl/libxl already supports qemu's network-based block backends such as nbd and rbd. libvirt has supported configuring such <disk>s for long time too. This patch adds support for rbd disks in the libxl driver by generating a rbd device URL from the virDomainDiskDef object. The URL is passed to libxl via the pdev_path field of libxl_device_disk struct. libxl then passes the URL to qemu for cosumption by the rbd backend.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 191 insertions(+), 1 deletion(-)
ACK with the whitespace fix.
I pushed a trivial fix for the whitespace.
+ +static int +libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) +{ + virConnectPtr conn = NULL; + char *secret = NULL; + char *username = NULL; + int ret = -1; + + *srcstr = NULL; + if (src->auth && src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { + const char *protocol = virStorageNetProtocolTypeToString(src->protocol); + + username = src->auth->username; + if (!(conn = virConnectOpen("xen:///system"))) + goto cleanup; +
Opening a connection feels out of place in this function, but I see it's already done for NICs.
It would be nice to reuse it as is done in the qemu driver.
Agreed. Still working on this one as it's requiring a bit of code turmoil. Regards, Jim
On 02/22/2016 07:35 AM, Ján Tomko wrote:
On Wed, Feb 17, 2016 at 05:33:45PM -0700, Jim Fehlig wrote:
xl/libxl already supports qemu's network-based block backends such as nbd and rbd. libvirt has supported configuring such <disk>s for long time too. This patch adds support for rbd disks in the libxl driver by generating a rbd device URL from the virDomainDiskDef object. The URL is passed to libxl via the pdev_path field of libxl_device_disk struct. libxl then passes the URL to qemu for cosumption by the rbd backend.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 191 insertions(+), 1 deletion(-)
ACK with the whitespace fix.
+ +static int +libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) +{ + virConnectPtr conn = NULL; + char *secret = NULL; + char *username = NULL; + int ret = -1; + + *srcstr = NULL; + if (src->auth && src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { + const char *protocol = virStorageNetProtocolTypeToString(src->protocol); + + username = src->auth->username; + if (!(conn = virConnectOpen("xen:///system"))) + goto cleanup; + Opening a connection feels out of place in this function, but I see it's already done for NICs.
It would be nice to reuse it as is done in the qemu driver.
Heh, this has turned out to be a PITA. Something like the attached patch works, but it still has the virConnectOpen in the libxlBuildDomainConfig call path. I would prefer to use the virConnect object associated with the request, instead of opening one in this code. I have some old, dusty patches (originally started by danpb) that provide unit tests for libxlBuildDomainConfig. E.g. domXML -> virDomainDef -> libxl_domain_config -> json representation -> compare with expected json. Code in this path attempting to open "xen:///" connections is likely to break many 'make check' setups :-). But fixing this is not easy given the current code structure. There is one place in particular that is rather troublesome - reboot. libxlDomainStart, and hence libxlBuildDomainConfig, are called when a reboot event is received from libxl (e.g. 'shutdown -r now' within a VM). As you might guess, there is no virConnect object associated with such request. The code needs reworked quite a bit to handle accessing a virConnect object while building a libxl_domain_config object. I'll work on this when dusting off the unit test patches, although it is not high in my queue. Maybe it would make a good libvirt GSoC project :-). Regards, Jim
On Wed, 2016-02-17 at 17:33 -0700, Jim Fehlig wrote:
xl/libxl already supports qemu's network-based block backends such as nbd and rbd. libvirt has supported configuring network disks for long time too. This series marries the two in the libxl driver and in the xl<->xml converter. Only rbd supported is added in this series. Support for other backends such as nbd and iscsi can be added as a follow-up improvement.
All looks good to me, so FWIW: Acked-by: Ian Campbell <ian.campbell@citrix.com>
On 02/18/2016 03:51 AM, Ian Campbell wrote:
On Wed, 2016-02-17 at 17:33 -0700, Jim Fehlig wrote:
xl/libxl already supports qemu's network-based block backends such as nbd and rbd. libvirt has supported configuring network disks for long time too. This series marries the two in the libxl driver and in the xl<->xml converter. Only rbd supported is added in this series. Support for other backends such as nbd and iscsi can be added as a follow-up improvement. All looks good to me, so FWIW: Acked-by: Ian Campbell <ian.campbell@citrix.com>
Thanks. I'm going to push this series. If we ever decide to extend libxl_device_disk in libxl, corresponding changes in libvirt can be done with LIBXL_HAVE_<whatever-disk-extension>. Regards, Jim
participants (3)
-
Ian Campbell -
Jim Fehlig -
Ján Tomko