[libvirt] [PATCH] qemu: pass the information when disks are read-only
The problem is that even when the user indicates that a drive has to be opened as read-only (for example a cd-rom) qemu didn't get the information and tried to open read-write. This could get really nasty in case SELinux is monitoring that no read-write access is attempted: https://bugzilla.redhat.com/show_bug.cgi?id=536760 Patch is relatively simple and based on teh assumption that if the -device qemu options are allowed then qemu also knows about the readonly option (this was added to the help string quite later so we should not rely on that in this case). * src/qemu/qemu_conf.c: add the ",readonly=on" for read-only disks and also parse it back in qemuParseCommandLineDisk() * tests/qemuxml2argvtest.c tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml: add a specific regression test diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 40ca221..fb23c52 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2398,6 +2398,9 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, if (bootable && disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) virBufferAddLit(&opt, ",boot=on"); + if (disk->readonly && + qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) + virBufferAddLit(&opt, ",readonly=on"); if (disk->driverType && disk->type != VIR_DOMAIN_DISK_TYPE_DIR && qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT) @@ -4694,6 +4697,9 @@ qemuParseCommandLineDisk(const char *val, _("cannot parse drive unit '%s'"), val); goto cleanup; } + } else if (STREQ(keywords[i], "readonly")) { + if ((values[i] == NULL) || STREQ(values[i], "on")) + def->readonly = 1; } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args new file mode 100644 index 0000000..fbc3226 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=/dev/sr0,if=none,media=cdrom,id=drive-ide0-1-0,readonly=on -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml new file mode 100644 index 0000000..39c3a1c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>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'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <disk type='block' device='cdrom'> + <source dev='/dev/sr0'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' unit='0'/> + </disk> + <controller type='ide' index='0'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4ca946a..e3762c9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -249,6 +249,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-fat", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); + DO_TEST("disk-drive-readonly-disk", QEMUD_CMD_FLAG_DRIVE | + QEMUD_CMD_FLAG_DEVICE); DO_TEST("disk-drive-fmt-qcow", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-shared", QEMUD_CMD_FLAG_DRIVE | -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Thu, Mar 11, 2010 at 06:14:01PM +0100, Daniel Veillard wrote:
The problem is that even when the user indicates that a drive has to be opened as read-only (for example a cd-rom) qemu didn't get the information and tried to open read-write. This could get really nasty in case SELinux is monitoring that no read-write access is attempted:
https://bugzilla.redhat.com/show_bug.cgi?id=536760
Patch is relatively simple and based on teh assumption that if the -device qemu options are allowed then qemu also knows about the readonly option (this was added to the help string quite later so we should not rely on that in this case).
* src/qemu/qemu_conf.c: add the ",readonly=on" for read-only disks and also parse it back in qemuParseCommandLineDisk() * tests/qemuxml2argvtest.c tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml: add a specific regression test
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 40ca221..fb23c52 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2398,6 +2398,9 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, if (bootable && disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) virBufferAddLit(&opt, ",boot=on"); + if (disk->readonly && + qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) + virBufferAddLit(&opt, ",readonly=on"); if (disk->driverType && disk->type != VIR_DOMAIN_DISK_TYPE_DIR && qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT) @@ -4694,6 +4697,9 @@ qemuParseCommandLineDisk(const char *val, _("cannot parse drive unit '%s'"), val); goto cleanup; } + } else if (STREQ(keywords[i], "readonly")) { + if ((values[i] == NULL) || STREQ(values[i], "on")) + def->readonly = 1; } }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args new file mode 100644 index 0000000..fbc3226 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=/dev/sr0,if=none,media=cdrom,id=drive-ide0-1-0,readonly=on -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml new file mode 100644 index 0000000..39c3a1c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>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'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <disk type='block' device='cdrom'> + <source dev='/dev/sr0'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' unit='0'/> + </disk> + <controller type='ide' index='0'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4ca946a..e3762c9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -249,6 +249,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-fat", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); + DO_TEST("disk-drive-readonly-disk", QEMUD_CMD_FLAG_DRIVE | + QEMUD_CMD_FLAG_DEVICE); DO_TEST("disk-drive-fmt-qcow", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-shared", QEMUD_CMD_FLAG_DRIVE |
ACK, good to finally have this done Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Mon, Mar 15, 2010 at 03:41:34PM +0000, Daniel P. Berrange wrote:
On Thu, Mar 11, 2010 at 06:14:01PM +0100, Daniel Veillard wrote:
The problem is that even when the user indicates that a drive has to be opened as read-only (for example a cd-rom) qemu didn't get the information and tried to open read-write. This could get really nasty in case SELinux is monitoring that no read-write access is attempted:
https://bugzilla.redhat.com/show_bug.cgi?id=536760
Patch is relatively simple and based on teh assumption that if the -device qemu options are allowed then qemu also knows about the readonly option (this was added to the help string quite later so we should not rely on that in this case).
* src/qemu/qemu_conf.c: add the ",readonly=on" for read-only disks and also parse it back in qemuParseCommandLineDisk() * tests/qemuxml2argvtest.c tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml: add a specific regression test
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 40ca221..fb23c52 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2398,6 +2398,9 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, if (bootable && disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) virBufferAddLit(&opt, ",boot=on"); + if (disk->readonly && + qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) + virBufferAddLit(&opt, ",readonly=on"); if (disk->driverType && disk->type != VIR_DOMAIN_DISK_TYPE_DIR && qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT) @@ -4694,6 +4697,9 @@ qemuParseCommandLineDisk(const char *val, _("cannot parse drive unit '%s'"), val); goto cleanup; } + } else if (STREQ(keywords[i], "readonly")) { + if ((values[i] == NULL) || STREQ(values[i], "on")) + def->readonly = 1; } }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args new file mode 100644 index 0000000..fbc3226 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=/dev/sr0,if=none,media=cdrom,id=drive-ide0-1-0,readonly=on -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml new file mode 100644 index 0000000..39c3a1c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-disk.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>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'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <disk type='block' device='cdrom'> + <source dev='/dev/sr0'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' unit='0'/> + </disk> + <controller type='ide' index='0'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4ca946a..e3762c9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -249,6 +249,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-fat", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); + DO_TEST("disk-drive-readonly-disk", QEMUD_CMD_FLAG_DRIVE | + QEMUD_CMD_FLAG_DEVICE); DO_TEST("disk-drive-fmt-qcow", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-shared", QEMUD_CMD_FLAG_DRIVE |
ACK, good to finally have this done
Okay, pushed, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel P. Berrange -
Daniel Veillard