From patchwork Fri Apr 1 14:31:35 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Jackson X-Patchwork-Id: 8725301 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5B10F9F3D1 for ; Fri, 1 Apr 2016 14:34:32 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 335D0203A4 for ; Fri, 1 Apr 2016 14:34:31 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id EA09320279 for ; Fri, 1 Apr 2016 14:34:29 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1am06x-0000XA-Ed; Fri, 01 Apr 2016 14:31:43 +0000 Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1am06v-0000X3-Ha for xen-devel@lists.xen.org; Fri, 01 Apr 2016 14:31:41 +0000 Received: from [193.109.254.147] by server-6.bemta-14.messagelabs.com id B3/8B-03497-C468EF65; Fri, 01 Apr 2016 14:31:40 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrNIsWRWlGSWpSXmKPExsXitHRDpK5P278 wg2u/uSyWfFzM4sDocXT3b6YAxijWzLyk/IoE1ozrP3YyFzwOrzj25R57A+N0yy5GTg4JAX+J e0tXsYPYbAK6EpemTWQGsXkFBCVOznzCAmIzC+hILNj9iQ3ClpfY/nYOVI29xPzGzWBxFgEVi V/zNzOC2CJA9Vf3vmDtYuQCql/AKHHq9WSwImEBX4mju3YwgdicAp4Sxxf1s4MUCQm0MUosW/ yEFeIiY4nV9z8CbeYASqhJzF0fDxHmlrh9eirzBEb+WUjum4XkvllI7lvAyLyKUb04tagstUj XWC+pKDM9oyQ3MTNH19DQRC83tbg4MT01JzGpWC85P3cTIzAIGYBgB+PdPudDjJIcTEqivCIN /8KE+JLyUyozEosz4otKc1KLDzHqcXAInNtz9xijQN/nCYuYpFjy8vNSlSR4xVuBqgWLUtNTK 9Iyc4AxA9MgwcGjJMIrB5LmLS5IzC3OTIdInWJUlBLnfdkClBAASWSU5sG1weL0EqOslDAvI9 BpQjwFqUW5mSWo8q8YxTkYlYR5RUHG82TmlcBNfwW0mAlocYc02OKSRISUVAPj/u/P1oXtvCJ aIas35bfeTS+u/n2icdKvepnPBjEk+ycm7E9OKf3zrXZW0be0uJq6zsTlnGImvmu5uhqKt8Z+ OPdx79LoUPOe+4Wbwt+p8r6cNdHz72qV0pf3V+j3pJf8uzr98bt3b1zOmjyRX9e+rOvQu2UHI o8fknK/XHf/wQxfRpYbE8/tVmIpzkg01GIuKk4EAGC7CJjOAgAA X-Env-Sender: prvs=8927ccde4=Ian.Jackson@citrix.com X-Msg-Ref: server-12.tower-27.messagelabs.com!1459521098!35000913!1 X-Originating-IP: [66.165.176.89] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni44OSA9PiAyMDMwMDc=\n, received_headers: No Received headers X-StarScan-Received: X-StarScan-Version: 8.28; banners=-,-,- X-VirusChecked: Checked Received: (qmail 19629 invoked from network); 1 Apr 2016 14:31:39 -0000 Received: from smtp.citrix.com (HELO SMTP.CITRIX.COM) (66.165.176.89) by server-12.tower-27.messagelabs.com with RC4-SHA encrypted SMTP; 1 Apr 2016 14:31:39 -0000 X-IronPort-AV: E=Sophos;i="5.24,427,1454976000"; d="scan'208";a="344112252" From: Ian Jackson MIME-Version: 1.0 Message-ID: <22270.34375.133949.13098@mariner.uk.xensource.com> Date: Fri, 1 Apr 2016 15:31:35 +0100 To: George Dunlap In-Reply-To: <1458840160-26733-6-git-send-email-george.dunlap@citrix.com> References: <1458840160-26733-1-git-send-email-george.dunlap@citrix.com> <1458840160-26733-6-git-send-email-george.dunlap@citrix.com> X-Mailer: VM 8.1.0 under 23.4.1 (i486-pc-linux-gnu) X-DLP: MIA2 Cc: Anthony Perard , Stefano Stabellini , Wei Liu , xen-devel@lists.xen.org Subject: Re: [Xen-devel] [PATCH v3 5/9] libxl: Rearrange qemu upstream disk argument code X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP George Dunlap writes ("[PATCH v3 5/9] libxl: Rearrange qemu upstream disk argument code"): > Reorganize the qemuu disk argument code to make a clean separation > between finding a file to use, and constructing the parameters: This didn't apply to staging, since colo went in. I have tried to rebase it and the result compiles. Can you check it's right please ? Thanks, Ian. From 6ab86b63462c8e6dc243c796f5ea10240aadc5de Mon Sep 17 00:00:00 2001 From: George Dunlap Date: Thu, 24 Mar 2016 17:18:33 +0000 Subject: [PATCH] libxl: Rearrange qemu upstream disk argument code Reorganize the qemuu disk argument code to make a clean separation between finding a file to use, and constructing the parameters: * Rename pdev_path to target_path * Only use qemu_disk_format_string() in circumstances where qemu may be interpreting the disk (i.e., backend==QDISK). In all other cases, it should use RAW. * Share as much as possible between the is_cdrom path and the normal path. This is mainly prep for sharing the local path finder with the bootloader; but it does allow cdroms to use any backend that a normal disk can use. Previously this was limited to RAW files or things that qemu could handle directly; as of this changeset, it now includes tap disks; and in future changesets it will include backends with custom block scripts. NB that this retains an existing bug, that disks with custom block scripts or non-dom0 backends will have the bogus pdev_path passed in to qemu, most likely resulting in qemu exiting with an error. This will be fixed in follow-up patches. Signed-off-by: George Dunlap Signed-off-by: Ian Jackson Reviewed-by: George Dunlap Tested-by: George Dunlap --- Changes since v1: - Move qemuu disk argument refactoring into a separate patch CC: Ian Jackson CC: Wei Liu CC: Stefano Stabellini CC: Anthony Perard --- tools/libxl/libxl_dm.c | 76 ++++++++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 3522fbf..c11e9b8 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -761,7 +761,7 @@ enum { LIBXL__COLO_SECONDARY, }; -static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *pdev_path, +static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *target_path, int unit, const char *format, const libxl_device_disk *disk, int colo_mode) @@ -775,14 +775,14 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *pdev_path, case LIBXL__COLO_NONE: drive = libxl__sprintf (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback", - pdev_path, unit, format); + target_path, unit, format); break; case LIBXL__COLO_PRIMARY: /* * primary: * -dirve if=scsi,bus=0,unit=x,cache=writeback,driver=quorum,\ * id=exportname,\ - * children.0.file.filename=pdev_path,\ + * children.0.file.filename=target_path,\ * children.0.driver=format,\ * read-pattern=fifo,\ * vote-threshold=1 @@ -794,7 +794,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *pdev_path, "children.0.driver=%s," "read-pattern=fifo," "vote-threshold=1", - unit, exportname, pdev_path, format); + unit, exportname, target_path, format); break; case LIBXL__COLO_SECONDARY: /* @@ -823,7 +823,7 @@ static char *qemu_disk_scsi_drive_string(libxl__gc *gc, const char *pdev_path, return drive; } -static char *qemu_disk_ide_drive_string(libxl__gc *gc, const char *pdev_path, +static char *qemu_disk_ide_drive_string(libxl__gc *gc, const char *target_path, int unit, const char *format, const libxl_device_disk *disk, int colo_mode) @@ -837,14 +837,14 @@ static char *qemu_disk_ide_drive_string(libxl__gc *gc, const char *pdev_path, case LIBXL__COLO_NONE: drive = GCSPRINTF ("file=%s,if=ide,index=%d,media=disk,format=%s,cache=writeback", - pdev_path, unit, format); + target_path, unit, format); break; case LIBXL__COLO_PRIMARY: /* * primary: * -dirve if=ide,index=x,media=disk,cache=writeback,driver=quorum,\ * id=exportname,\ - * children.0.file.filename=pdev_path,\ + * children.0.file.filename=target_path,\ * children.0.driver=format,\ * read-pattern=fifo,\ * vote-threshold=1 @@ -856,7 +856,7 @@ static char *qemu_disk_ide_drive_string(libxl__gc *gc, const char *pdev_path, "children.0.driver=%s," "read-pattern=fifo," "vote-threshold=1", - unit, exportname, pdev_path, format); + unit, exportname, target_path, format); break; case LIBXL__COLO_SECONDARY: /* @@ -1305,9 +1305,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, int disk, part; int dev_number = libxl__device_disk_dev_number(disks[i].vdev, &disk, &part); - const char *format = qemu_disk_format_string(disks[i].format); + const char *format; char *drive; - const char *pdev_path; + const char *target_path; int colo_mode; if (dev_number == -1) { @@ -1316,22 +1316,22 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, continue; } - if (disks[i].is_cdrom) { - if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) - drive = libxl__sprintf - (gc, "if=ide,index=%d,readonly=%s,media=cdrom,cache=writeback,id=ide-%i", - disk, disks[i].readwrite ? "off" : "on", dev_number); - else - drive = libxl__sprintf - (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].format == LIBXL_DISK_FORMAT_EMPTY) { + /* + * If qemu isn't doing the interpreting, the parameter is + * always raw + */ + if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK) + format = qemu_disk_format_string(disks[i].format); + else + format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW); + + if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) { + if (!disks[i].is_cdrom) { LOG(WARN, "cannot support"" empty disk format for %s", disks[i].vdev); continue; } - + } else { if (format == NULL) { LOG(WARN, "unable to determine"" disk image format %s", @@ -1339,14 +1339,22 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, continue; } - if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) { - format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW); - pdev_path = libxl__blktap_devpath(gc, disks[i].pdev_path, - disks[i].format); - } else { - pdev_path = disks[i].pdev_path; - } + if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) + target_path = libxl__blktap_devpath(gc, disks[i].pdev_path, + disks[i].format); + else + target_path = disks[i].pdev_path; + } + if (disks[i].is_cdrom) { + drive = libxl__sprintf(gc, + "if=ide,index=%d,readonly=%s,media=cdrom,cache=writeback,id=ide-%i", + disk, disks[i].readwrite ? "off" : "on", dev_number); + + if (target_path) + drive = libxl__sprintf(gc, "%s,file=%s,format=%s", + drive, target_path, format); + } else { /* * Explicit sd disks are passed through as is. * @@ -1367,12 +1375,12 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, if (colo_mode == LIBXL__COLO_SECONDARY) { drive = libxl__sprintf (gc, "if=none,driver=%s,file=%s,id=%s", - format, pdev_path, disks[i].colo_export); + format, target_path, disks[i].colo_export); flexarray_append(dm_args, "-drive"); flexarray_append(dm_args, drive); } - drive = qemu_disk_scsi_drive_string(gc, pdev_path, disk, + drive = qemu_disk_scsi_drive_string(gc, target_path, disk, format, &disks[i], colo_mode); @@ -1389,7 +1397,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, } flexarray_vappend(dm_args, "-drive", GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,cache=writeback", - pdev_path, disk, format), + target_path, disk, format), "-device", GCSPRINTF("ide-hd,bus=ahci0.%d,unit=0,drive=ahcidisk-%d", disk, disk), NULL); continue; @@ -1401,12 +1409,12 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, if (colo_mode == LIBXL__COLO_SECONDARY) { drive = libxl__sprintf (gc, "if=none,driver=%s,file=%s,id=%s", - format, pdev_path, disks[i].colo_export); + format, target_path, disks[i].colo_export); flexarray_append(dm_args, "-drive"); flexarray_append(dm_args, drive); } - drive = qemu_disk_ide_drive_string(gc, pdev_path, disk, + drive = qemu_disk_ide_drive_string(gc, target_path, disk, format, &disks[i], colo_mode);