On Wed, May 15, 2013 at 01:26:03AM +0800, Osier Yang wrote:
On 13/05/13 21:18, Daniel Veillard wrote:
On Mon, May 13, 2013 at 02:00:31PM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The QEMU command line syntax for RBD disks is
file=rbd:pool/image:opt1=val1:opt2=val2...
There is no way to escape the ':' if it appears in the pool or image name. Thus it must be explicitly forbidden if it occurs in the libvirt XML. People are known to be abusing the lack of escaping in current libvirt to pass arbitrary args to QEMU. Urgh, :-(
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.c | 6 ++++ ...qemuxml2argv-disk-drive-network-rbd-invalid.xml | 37 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++ 3 files changed, 48 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-invalid.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eddc263..c0cb250 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2383,6 +2383,12 @@ qemuBuildRBDString(virConnectPtr conn, char *secret = NULL; size_t secret_size; + if (strchr(disk->src, ':')) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("':' not allowed in RBD source volume name")); + return -1; + } + virBufferEscape(opt, ',', ",", "rbd:%s", disk->src); if (disk->auth.username) { virBufferEscape(opt, '\\', ":", ":id=%s", disk->auth.username); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-invalid.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-invalid.xml new file mode 100644 index 0000000..e8d3280 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-invalid.xml @@ -0,0 +1,37 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <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='poolname/imagename:rbd_cache=1:rbd_cache_size=67108864:rbd_cache_max_dirty=0'>
This breaks domainschematest, as the pool name uses:
<define name="genericName"> <data type="string"> <param name="pattern">[a-zA-Z0-9_\+\-]+</param> </data> </define>
Actually it doesn't use genericName - it just has <attribute name='name'/> which allows any text. The problem is that I rebased across the recent change commit 470d5c4654b512d101a8665754b13866925eeaa2 Author: Ján Tomko <jtomko@redhat.com> Date: Thu May 9 13:43:32 2013 +0200 tests: files named '.*-invalid.xml' should fail validation since I named my XML file -invalid.xml it accidently tripped up on this new change. I'm pushing a fix which just renames my new xml file Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|