From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jim Fehlig Subject: [RFC] libxl: relax readonly check introduced by XSA-142 fix Date: Wed, 11 Nov 2015 10:15:10 -0700 Message-ID: <5643779E.1010107@suse.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------070507040103070809000609" Return-path: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "xen-devel@lists.xen.org" Cc: Ian Jackson , Wei Liu , Ian Campbell , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --------------070507040103070809000609 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Hi All, Apologies for only noticing the fix for XSA-142 as it starting flowing to our various downstreams. The current fix seems like quite a big hammer IMO. qemu doesn't support readonly IDE/SATA disks # /usr/lib/xen/bin/qemu-system-i386 -drive file=/tmp/disk.raw,if=ide,media=disk,format=raw,readonly=on qemu-system-i386: Can't use a read-only drive But it does support readonly SCSI disks # /usr/lib/xen/bin/qemu-system-i386 -drive file=/tmp/disk.raw,if=scsi,media=disk,format=raw,readonly=on [ok] Inside a guest using such a disk, the SCSI kernel driver sees write protect on [ 7.339232] sd 2:0:1:0: [sdb] Write Protect is on Also, PV drivers support readonly, but the patch rejects such configuration even when PV drivers (vdev=xvd*) have been explicitly specified and creation of an emulated twin is skipped. The attached follow-up loosens the restriction to reject readonly when creating and emulated IDE disk, but allows it when the backend is known to support readonly. Regards, Jim --------------070507040103070809000609 Content-Type: text/x-diff; name="relax-xsa142-readonly-check.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="relax-xsa142-readonly-check.patch" >>From 3d58f93b58ab544b9ee168b7bbaf59de1c5532ce Mon Sep 17 00:00:00 2001 From: Jim Fehlig Date: Tue, 10 Nov 2015 11:33:44 -0700 Subject: [PATCH] libxl: relax readonly check introduced by XSA-142 fix The fix for XSA-142 is quite a big hammer, rejecting readonly disk configuration even when the requested backend is known to support readonly. While it is true that qemu doesn't support readonly for emulated IDE disks $ /usr/lib/xen/bin/qemu-system-i386 -drive file=/tmp/disk.raw,if=ide,media=disk,format=raw,readonly=on qemu-system-i386: Can't use a read-only drive It does support readonly SCSI disks $ /usr/lib/xen/bin/qemu-system-i386 -drive file=/tmp/disk.raw,if=scsi,media=disk,format=raw,readonly=on [ok] Inside a guest using such a disk, the SCSI kernel driver sees write protect on [ 7.339232] sd 2:0:1:0: [sdb] Write Protect is on Also, PV drivers support readonly, but the patch rejects such configuration even when PV drivers (vdev=xvd*) have been explicitly specified and creation of an emulated twin is skiped. This follow-up patch loosens the restriction to reject readonly when creating and emulated IDE disk, but allows it when the backend is known to support readonly. Signed-off-by: Jim Fehlig --- tools/libxl/libxl_dm.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 9c9eaa3..ccfc827 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -1152,12 +1152,6 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, (gc, "file=%s,if=ide,index=%d,readonly=%s,media=cdrom,format=%s,cache=writeback,id=ide-%i", disks[i].pdev_path, disk, disks[i].readwrite ? "off" : "on", format, dev_number); } else { - if (!disks[i].readwrite) { - LOG(ERROR, - "qemu-xen doesn't support read-only disk drivers"); - return ERROR_INVAL; - } - if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) { LOG(WARN, "cannot support"" empty disk format for %s", disks[i].vdev); @@ -1185,29 +1179,34 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, * For other disks we translate devices 0..3 into * hd[a-d] and ignore the rest. */ - if (strncmp(disks[i].vdev, "sd", 2) == 0) + if (strncmp(disks[i].vdev, "sd", 2) == 0) { drive = libxl__sprintf - (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback", - pdev_path, disk, format); - else if (strncmp(disks[i].vdev, "xvd", 3) == 0) + (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,readonly=%s,cache=writeback", + pdev_path, disk, format, disks[i].readwrite ? "off" : "on"); + } else if (strncmp(disks[i].vdev, "xvd", 3) == 0) { /* * Do not add any emulated disk when PV disk are * explicitly asked for. */ continue; - else if (disk < 6 && b_info->u.hvm.hdtype == LIBXL_HDTYPE_AHCI) { + } else if (disk < 6 && b_info->u.hvm.hdtype == LIBXL_HDTYPE_AHCI) { flexarray_vappend(dm_args, "-drive", - GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,cache=writeback", - pdev_path, disk, format), + GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,readonly=%s,cache=writeback", + pdev_path, disk, format, disks[i].readwrite ? "off" : "on"), "-device", GCSPRINTF("ide-hd,bus=ahci0.%d,unit=0,drive=ahcidisk-%d", disk, disk), NULL); continue; - } else if (disk < 4) + } else if (disk < 4) { + if (!disks[i].readwrite) { + LOG(ERROR, "qemu-xen doesn't support read-only IDE disk drivers"); + return ERROR_INVAL; + } drive = libxl__sprintf (gc, "file=%s,if=ide,index=%d,media=disk,format=%s,cache=writeback", pdev_path, disk, format); - else + } else { continue; /* Do not emulate this disk */ + } } flexarray_append(dm_args, "-drive"); -- 1.8.0.1 --------------070507040103070809000609 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --------------070507040103070809000609--