* [PATCH V2 0/4] libxl: support qemu's network-based block backends
@ 2016-02-18 0:33 Jim Fehlig
2016-02-18 0:33 ` [PATCH V2 1/4] xenconfig: replace text 'xm' with 'xl' in xlconfigtest Jim Fehlig
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Jim Fehlig @ 2016-02-18 0:33 UTC (permalink / raw)
To: libvirt-list; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V2 1/4] xenconfig: replace text 'xm' with 'xl' in xlconfigtest
2016-02-18 0:33 [PATCH V2 0/4] libxl: support qemu's network-based block backends Jim Fehlig
@ 2016-02-18 0:33 ` Jim Fehlig
2016-02-22 14:15 ` [libvirt] " Ján Tomko
2016-02-18 0:33 ` [PATCH V2 2/4] xenconfig: produce key=value disk config syntax in xl formatter Jim Fehlig
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Jim Fehlig @ 2016-02-18 0:33 UTC (permalink / raw)
To: libvirt-list; +Cc: Jim Fehlig, xen-devel
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH V2 2/4] xenconfig: produce key=value disk config syntax in xl formatter
2016-02-18 0:33 [PATCH V2 0/4] libxl: support qemu's network-based block backends Jim Fehlig
2016-02-18 0:33 ` [PATCH V2 1/4] xenconfig: replace text 'xm' with 'xl' in xlconfigtest Jim Fehlig
@ 2016-02-18 0:33 ` Jim Fehlig
2016-02-22 14:16 ` [libvirt] " Ján Tomko
2016-02-18 0:33 ` [PATCH V2 3/4] xenconfig: support xl<->xml conversion of rbd disk devices Jim Fehlig
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Jim Fehlig @ 2016-02-18 0:33 UTC (permalink / raw)
To: libvirt-list; +Cc: Jim Fehlig, xen-devel
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH V2 3/4] xenconfig: support xl<->xml conversion of rbd disk devices
2016-02-18 0:33 [PATCH V2 0/4] libxl: support qemu's network-based block backends Jim Fehlig
2016-02-18 0:33 ` [PATCH V2 1/4] xenconfig: replace text 'xm' with 'xl' in xlconfigtest Jim Fehlig
2016-02-18 0:33 ` [PATCH V2 2/4] xenconfig: produce key=value disk config syntax in xl formatter Jim Fehlig
@ 2016-02-18 0:33 ` Jim Fehlig
2016-02-22 14:22 ` [libvirt] " Ján Tomko
2016-02-18 0:33 ` [PATCH V2 4/4] libxl: add support for rbd qdisk Jim Fehlig
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Jim Fehlig @ 2016-02-18 0:33 UTC (permalink / raw)
To: libvirt-list; +Cc: Jim Fehlig, xen-devel
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH V2 4/4] libxl: add support for rbd qdisk
2016-02-18 0:33 [PATCH V2 0/4] libxl: support qemu's network-based block backends Jim Fehlig
` (2 preceding siblings ...)
2016-02-18 0:33 ` [PATCH V2 3/4] xenconfig: support xl<->xml conversion of rbd disk devices Jim Fehlig
@ 2016-02-18 0:33 ` Jim Fehlig
2016-02-22 14:35 ` [libvirt] " Ján Tomko
2016-02-18 10:51 ` [PATCH V2 0/4] libxl: support qemu's network-based block backends Ian Campbell
[not found] ` <1455792671.6225.28.camel@citrix.com>
5 siblings, 1 reply; 14+ messages in thread
From: Jim Fehlig @ 2016-02-18 0:33 UTC (permalink / raw)
To: libvirt-list; +Cc: Jim Fehlig, xen-devel
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH V2 0/4] libxl: support qemu's network-based block backends
2016-02-18 0:33 [PATCH V2 0/4] libxl: support qemu's network-based block backends Jim Fehlig
` (3 preceding siblings ...)
2016-02-18 0:33 ` [PATCH V2 4/4] libxl: add support for rbd qdisk Jim Fehlig
@ 2016-02-18 10:51 ` Ian Campbell
[not found] ` <1455792671.6225.28.camel@citrix.com>
5 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2016-02-18 10:51 UTC (permalink / raw)
To: Jim Fehlig, libvirt-list; +Cc: xen-devel
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>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [libvirt] [PATCH V2 1/4] xenconfig: replace text 'xm' with 'xl' in xlconfigtest
2016-02-18 0:33 ` [PATCH V2 1/4] xenconfig: replace text 'xm' with 'xl' in xlconfigtest Jim Fehlig
@ 2016-02-22 14:15 ` Ján Tomko
0 siblings, 0 replies; 14+ messages in thread
From: Ján Tomko @ 2016-02-22 14:15 UTC (permalink / raw)
To: Jim Fehlig; +Cc: libvirt-list, xen-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [libvirt] [PATCH V2 2/4] xenconfig: produce key=value disk config syntax in xl formatter
2016-02-18 0:33 ` [PATCH V2 2/4] xenconfig: produce key=value disk config syntax in xl formatter Jim Fehlig
@ 2016-02-22 14:16 ` Ján Tomko
0 siblings, 0 replies; 14+ messages in thread
From: Ján Tomko @ 2016-02-22 14:16 UTC (permalink / raw)
To: Jim Fehlig; +Cc: libvirt-list, xen-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [libvirt] [PATCH V2 3/4] xenconfig: support xl<->xml conversion of rbd disk devices
2016-02-18 0:33 ` [PATCH V2 3/4] xenconfig: support xl<->xml conversion of rbd disk devices Jim Fehlig
@ 2016-02-22 14:22 ` Ján Tomko
0 siblings, 0 replies; 14+ messages in thread
From: Ján Tomko @ 2016-02-22 14:22 UTC (permalink / raw)
To: Jim Fehlig; +Cc: libvirt-list, xen-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [libvirt] [PATCH V2 4/4] libxl: add support for rbd qdisk
2016-02-18 0:33 ` [PATCH V2 4/4] libxl: add support for rbd qdisk Jim Fehlig
@ 2016-02-22 14:35 ` Ján Tomko
2016-02-23 3:34 ` Jim Fehlig
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Ján Tomko @ 2016-02-22 14:35 UTC (permalink / raw)
To: Jim Fehlig; +Cc: libvirt-list, xen-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 0/4] libxl: support qemu's network-based block backends
[not found] ` <1455792671.6225.28.camel@citrix.com>
@ 2016-02-22 19:08 ` Jim Fehlig
0 siblings, 0 replies; 14+ messages in thread
From: Jim Fehlig @ 2016-02-22 19:08 UTC (permalink / raw)
To: Ian Campbell, libvirt-list; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [libvirt] [PATCH V2 4/4] libxl: add support for rbd qdisk
2016-02-22 14:35 ` [libvirt] " Ján Tomko
@ 2016-02-23 3:34 ` Jim Fehlig
2016-02-23 23:46 ` Jim Fehlig
2016-02-24 4:11 ` Jim Fehlig
2 siblings, 0 replies; 14+ messages in thread
From: Jim Fehlig @ 2016-02-23 3:34 UTC (permalink / raw)
To: Ján Tomko; +Cc: libvirt-list, xen-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [libvirt] [PATCH V2 4/4] libxl: add support for rbd qdisk
2016-02-22 14:35 ` [libvirt] " Ján Tomko
2016-02-23 3:34 ` Jim Fehlig
@ 2016-02-23 23:46 ` Jim Fehlig
2016-02-24 4:11 ` Jim Fehlig
2 siblings, 0 replies; 14+ messages in thread
From: Jim Fehlig @ 2016-02-23 23:46 UTC (permalink / raw)
To: Ján Tomko; +Cc: libvirt-list, xen-devel
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [libvirt] [PATCH V2 4/4] libxl: add support for rbd qdisk
2016-02-22 14:35 ` [libvirt] " Ján Tomko
2016-02-23 3:34 ` Jim Fehlig
2016-02-23 23:46 ` Jim Fehlig
@ 2016-02-24 4:11 ` Jim Fehlig
2 siblings, 0 replies; 14+ messages in thread
From: Jim Fehlig @ 2016-02-24 4:11 UTC (permalink / raw)
To: Ján Tomko; +Cc: libvirt-list, xen-devel
[-- Attachment #1: Type: text/plain, Size: 2778 bytes --]
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
[-- Attachment #2: 0001-libxl-use-a-common-virConnect-object.patch --]
[-- Type: text/x-patch, Size: 15330 bytes --]
>From 869c76c029390ebb6e2b89b3bb0ccf9ac4610806 Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@suse.com>
Date: Tue, 23 Feb 2016 16:20:29 -0700
Subject: [PATCH] libxl: use a common virConnect object
Depending on the type of NIC or disk, libxlMakeNic and libxlMakeDisk
may need a virConnect object, e.g. to obtain a port from the network
driver or a secret from the secret driver. Instead of opening a
virConnect object in each of these functions, have the callers provide
the object.
This approach provides benefits over opening individual virConnect
objects, e.g. already fixing a virConnect unref bug in libxlMakeNic.
The downside is changing several functions to include a virConnectPtr
parameter.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
src/libxl/libxl_conf.c | 71 ++++++++++++++++++++++++++----------------------
src/libxl/libxl_conf.h | 7 +++--
src/libxl/libxl_driver.c | 43 ++++++++++++++++++-----------
3 files changed, 71 insertions(+), 50 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index a109bb5..5d5c85e 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1071,9 +1071,10 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src,
}
static int
-libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr)
+libxlMakeNetworkDiskSrc(virConnectPtr conn,
+ virStorageSourcePtr src,
+ char **srcstr)
{
- virConnectPtr conn = NULL;
char *secret = NULL;
char *username = NULL;
int ret = -1;
@@ -1083,9 +1084,6 @@ libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr)
const char *protocol = virStorageNetProtocolTypeToString(src->protocol);
username = src->auth->username;
- if (!(conn = virConnectOpen("xen:///system")))
- goto cleanup;
-
if (!(secret = libxlGetSecretString(conn,
protocol,
true,
@@ -1101,12 +1099,13 @@ libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr)
cleanup:
VIR_FREE(secret);
- virObjectUnref(conn);
return ret;
}
int
-libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
+libxlMakeDisk(virConnectPtr conn,
+ virDomainDiskDefPtr l_disk,
+ libxl_device_disk *x_disk)
{
const char *driver;
int format;
@@ -1115,7 +1114,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
libxl_device_disk_init(x_disk);
if (actual_type == VIR_STORAGE_TYPE_NETWORK) {
- if (libxlMakeNetworkDiskSrc(l_disk->src, &x_disk->pdev_path) < 0)
+ if (libxlMakeNetworkDiskSrc(conn, l_disk->src, &x_disk->pdev_path) < 0)
return -1;
} else {
if (VIR_STRDUP(x_disk->pdev_path, virDomainDiskGetSource(l_disk)) < 0)
@@ -1252,7 +1251,9 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
}
static int
-libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config)
+libxlMakeDiskList(virConnectPtr conn,
+ virDomainDefPtr def,
+ libxl_domain_config *d_config)
{
virDomainDiskDefPtr *l_disks = def->disks;
int ndisks = def->ndisks;
@@ -1263,7 +1264,7 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config)
return -1;
for (i = 0; i < ndisks; i++) {
- if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0)
+ if (libxlMakeDisk(conn, l_disks[i], &x_disks[i]) < 0)
goto error;
}
@@ -1280,7 +1281,8 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config)
}
int
-libxlMakeNic(virDomainDefPtr def,
+libxlMakeNic(virConnectPtr conn,
+ virDomainDefPtr def,
virDomainNetDefPtr l_nic,
libxl_device_nic *x_nic)
{
@@ -1340,16 +1342,10 @@ libxlMakeNic(virDomainDefPtr def,
bool fail = false;
char *brname = NULL;
virNetworkPtr network;
- virConnectPtr conn;
-
- if (!(conn = virConnectOpen("xen:///system")))
- return -1;
if (!(network =
- virNetworkLookupByName(conn, l_nic->data.network.name))) {
- virObjectUnref(conn);
+ virNetworkLookupByName(conn, l_nic->data.network.name)))
return -1;
- }
if (l_nic->nips > 0) {
x_nic->ip = virSocketAddrFormat(&l_nic->ips[0]->address);
@@ -1367,7 +1363,6 @@ libxlMakeNic(virDomainDefPtr def,
VIR_FREE(brname);
virObjectUnref(network);
- virObjectUnref(conn);
if (fail)
return -1;
break;
@@ -1442,7 +1437,9 @@ libxlMakeNic(virDomainDefPtr def,
}
static int
-libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config)
+libxlMakeNicList(virConnectPtr conn,
+ virDomainDefPtr def,
+ libxl_domain_config *d_config)
{
virDomainNetDefPtr *l_nics = def->nets;
size_t nnics = def->nnets;
@@ -1456,7 +1453,7 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config)
if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV)
continue;
- if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics]))
+ if (libxlMakeNic(conn, def, l_nics[i], &x_nics[nvnics]))
goto error;
/*
* The devid (at least right now) will not get initialized by
@@ -2118,28 +2115,34 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
libxl_ctx *ctx,
libxl_domain_config *d_config)
{
+ virConnectPtr conn = NULL;
+ int ret = -1;
+
libxl_domain_config_init(d_config);
- if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0)
+ if (!(conn = virConnectOpen("xen:///system")))
return -1;
+ if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0)
+ goto cleanup;
+
if (libxlMakeDomBuildInfo(def, ctx, d_config) < 0)
- return -1;
+ goto cleanup;
- if (libxlMakeDiskList(def, d_config) < 0)
- return -1;
+ if (libxlMakeDiskList(conn, def, d_config) < 0)
+ goto cleanup;
- if (libxlMakeNicList(def, d_config) < 0)
- return -1;
+ if (libxlMakeNicList(conn, def, d_config) < 0)
+ goto cleanup;
if (libxlMakeVfbList(graphicsports, def, d_config) < 0)
- return -1;
+ goto cleanup;
if (libxlMakeBuildInfoVfb(graphicsports, def, d_config) < 0)
- return -1;
+ goto cleanup;
if (libxlMakePCIList(def, d_config) < 0)
- return -1;
+ goto cleanup;
/*
* Now that any potential VFBs are defined, update the build info with
@@ -2147,13 +2150,17 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
* so but as it does not right now, better be explicit.
*/
if (libxlMakeVideo(def, d_config) < 0)
- return -1;
+ goto cleanup;
d_config->on_reboot = libxlActionFromVirLifecycle(def->onReboot);
d_config->on_poweroff = libxlActionFromVirLifecycle(def->onPoweroff);
d_config->on_crash = libxlActionFromVirLifecycleCrash(def->onCrash);
- return 0;
+ ret = 0;
+
+ cleanup:
+ virObjectUnref(conn);
+ return ret;
}
virDomainXMLOptionPtr
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 3c0eafb..94fb866 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -192,9 +192,12 @@ int
libxlDomainGetEmulatorType(const virDomainDef *def);
int
-libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev);
+libxlMakeDisk(virConnectPtr conn,
+ virDomainDiskDefPtr l_dev,
+ libxl_device_disk *x_dev);
int
-libxlMakeNic(virDomainDefPtr def,
+libxlMakeNic(virConnectPtr conn,
+ virDomainDefPtr def,
virDomainNetDefPtr l_nic,
libxl_device_nic *x_nic);
int
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 404016e..531e2d6 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2912,7 +2912,9 @@ libxlDomainUndefine(virDomainPtr dom)
}
static int
-libxlDomainChangeEjectableMedia(virDomainObjPtr vm, virDomainDiskDefPtr disk)
+libxlDomainChangeEjectableMedia(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk)
{
libxlDriverConfigPtr cfg = libxlDriverConfigGet(libxl_driver);
virDomainDiskDefPtr origdisk = NULL;
@@ -2942,7 +2944,7 @@ libxlDomainChangeEjectableMedia(virDomainObjPtr vm, virDomainDiskDefPtr disk)
return -1;
}
- if (libxlMakeDisk(disk, &x_disk) < 0)
+ if (libxlMakeDisk(conn, disk, &x_disk) < 0)
goto cleanup;
if ((ret = libxl_cdrom_insert(cfg->ctx, vm->def->id, &x_disk, NULL)) < 0) {
@@ -2966,7 +2968,9 @@ libxlDomainChangeEjectableMedia(virDomainObjPtr vm, virDomainDiskDefPtr disk)
}
static int
-libxlDomainAttachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev)
+libxlDomainAttachDeviceDiskLive(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
{
libxlDriverConfigPtr cfg = libxlDriverConfigGet(libxl_driver);
virDomainDiskDefPtr l_disk = dev->data.disk;
@@ -2975,7 +2979,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev)
switch (l_disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
- ret = libxlDomainChangeEjectableMedia(vm, l_disk);
+ ret = libxlDomainChangeEjectableMedia(conn, vm, l_disk);
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
if (l_disk->bus == VIR_DOMAIN_DISK_BUS_XEN) {
@@ -2994,7 +2998,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev)
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
goto cleanup;
- if (libxlMakeDisk(l_disk, &x_disk) < 0)
+ if (libxlMakeDisk(conn, l_disk, &x_disk) < 0)
goto cleanup;
if (virDomainLockDiskAttach(libxl_driver->lockManager,
@@ -3119,7 +3123,9 @@ libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver,
}
static int
-libxlDomainDetachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev)
+libxlDomainDetachDeviceDiskLive(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
{
libxlDriverConfigPtr cfg = libxlDriverConfigGet(libxl_driver);
virDomainDiskDefPtr l_disk = NULL;
@@ -3141,7 +3147,7 @@ libxlDomainDetachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev)
l_disk = vm->def->disks[idx];
- if (libxlMakeDisk(l_disk, &x_disk) < 0)
+ if (libxlMakeDisk(conn, l_disk, &x_disk) < 0)
goto cleanup;
if ((ret = libxl_device_disk_remove(cfg->ctx, vm->def->id,
@@ -3180,6 +3186,7 @@ libxlDomainDetachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev)
static int
libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
+ virConnectPtr conn,
virDomainObjPtr vm,
virDomainNetDefPtr net)
{
@@ -3221,7 +3228,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
}
libxl_device_nic_init(&nic);
- if (libxlMakeNic(vm->def, net, &nic) < 0)
+ if (libxlMakeNic(conn, vm->def, net, &nic) < 0)
goto cleanup;
if (libxl_device_nic_add(cfg->ctx, vm->def->id, &nic, 0)) {
@@ -3243,6 +3250,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
static int
libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver,
+ virConnectPtr conn,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
@@ -3250,13 +3258,13 @@ libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver,
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
- ret = libxlDomainAttachDeviceDiskLive(vm, dev);
+ ret = libxlDomainAttachDeviceDiskLive(conn, vm, dev);
if (!ret)
dev->data.disk = NULL;
break;
case VIR_DOMAIN_DEVICE_NET:
- ret = libxlDomainAttachNetDevice(driver, vm,
+ ret = libxlDomainAttachNetDevice(driver, conn, vm,
dev->data.net);
if (!ret)
dev->data.net = NULL;
@@ -3514,6 +3522,7 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver,
static int
libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
+ virConnectPtr conn,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
@@ -3521,7 +3530,7 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
- ret = libxlDomainDetachDeviceDiskLive(vm, dev);
+ ret = libxlDomainDetachDeviceDiskLive(conn, vm, dev);
break;
case VIR_DOMAIN_DEVICE_NET:
@@ -3595,7 +3604,9 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
}
static int
-libxlDomainUpdateDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev)
+libxlDomainUpdateDeviceLive(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
{
virDomainDiskDefPtr disk;
int ret = -1;
@@ -3605,7 +3616,7 @@ libxlDomainUpdateDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev)
disk = dev->data.disk;
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
- ret = libxlDomainChangeEjectableMedia(vm, disk);
+ ret = libxlDomainChangeEjectableMedia(conn, vm, disk);
if (ret == 0)
dev->data.disk = NULL;
break;
@@ -3733,7 +3744,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
VIR_DOMAIN_DEF_PARSE_INACTIVE)))
goto endjob;
- if (libxlDomainAttachDeviceLive(driver, vm, dev) < 0)
+ if (libxlDomainAttachDeviceLive(driver, dom->conn, vm, dev) < 0)
goto endjob;
/*
@@ -3841,7 +3852,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
VIR_DOMAIN_DEF_PARSE_INACTIVE)))
goto endjob;
- if (libxlDomainDetachDeviceLive(driver, vm, dev) < 0)
+ if (libxlDomainDetachDeviceLive(driver, dom->conn, vm, dev) < 0)
goto endjob;
/*
@@ -3948,7 +3959,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
VIR_DOMAIN_DEF_PARSE_INACTIVE)))
goto cleanup;
- if ((ret = libxlDomainUpdateDeviceLive(vm, dev)) < 0)
+ if ((ret = libxlDomainUpdateDeviceLive(dom->conn, vm, dev)) < 0)
goto cleanup;
/*
--
2.6.1
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-02-24 4:11 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-18 0:33 [PATCH V2 0/4] libxl: support qemu's network-based block backends Jim Fehlig
2016-02-18 0:33 ` [PATCH V2 1/4] xenconfig: replace text 'xm' with 'xl' in xlconfigtest Jim Fehlig
2016-02-22 14:15 ` [libvirt] " Ján Tomko
2016-02-18 0:33 ` [PATCH V2 2/4] xenconfig: produce key=value disk config syntax in xl formatter Jim Fehlig
2016-02-22 14:16 ` [libvirt] " Ján Tomko
2016-02-18 0:33 ` [PATCH V2 3/4] xenconfig: support xl<->xml conversion of rbd disk devices Jim Fehlig
2016-02-22 14:22 ` [libvirt] " Ján Tomko
2016-02-18 0:33 ` [PATCH V2 4/4] libxl: add support for rbd qdisk Jim Fehlig
2016-02-22 14:35 ` [libvirt] " Ján Tomko
2016-02-23 3:34 ` Jim Fehlig
2016-02-23 23:46 ` Jim Fehlig
2016-02-24 4:11 ` Jim Fehlig
2016-02-18 10:51 ` [PATCH V2 0/4] libxl: support qemu's network-based block backends Ian Campbell
[not found] ` <1455792671.6225.28.camel@citrix.com>
2016-02-22 19:08 ` Jim Fehlig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).