From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= Subject: Re: [PATCH v6] libxl: allow 'phy' backend to use empty files Date: Fri, 19 Feb 2016 18:41:50 +0100 Message-ID: <56C753DE.7030409@citrix.com> References: <22212.25727.162858.164530@mariner.uk.xensource.com> <1455729638-41937-1-git-send-email-roger.pau@citrix.com> <22215.20813.59866.409199@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aWp40-00081B-BU for xen-devel@lists.xenproject.org; Fri, 19 Feb 2016 17:41:56 +0000 In-Reply-To: <22215.20813.59866.409199@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: xen-devel@lists.xenproject.org, Wei Liu , Ian Campbell , Alex Braunegg List-Id: xen-devel@lists.xenproject.org El 19/2/16 a les 18:30, Ian Jackson ha escrit: > Roger Pau Monne writes ("[PATCH v6] libxl: allow 'phy' backend to use emp= ty files"): >> This was introduced by 97ee1f (~5 years ago), but was probably never >> surfaced because most people used regular files as CDROM images, so the = PHY >> backend was actually never selected. A year ago this was changed, and now >> regular RAW files are also handled by the PHY backend, which has made th= is >> bug surface. >> >> Fix it by allowing empty disks to use the PHY backend, skipping the stat >> tests. >> >> Signed-off-by: Roger Pau Monn=E9 >> Reported-by: Alex Braunegg > = > Thanks, and thanks to Alex for the testing, but: > = >> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c >> index 8bb5e93..2e08108 100644 >> --- a/tools/libxl/libxl_device.c >> +++ b/tools/libxl/libxl_device.c >> @@ -196,6 +196,14 @@ static int disk_try_backend(disk_try_backend_args *= a, >> goto bad_format; >> } >> = >> + if (a->disk->format =3D=3D LIBXL_DISK_FORMAT_EMPTY) { >> + assert(a->disk->pdev_path =3D=3D NULL || >> + !strcmp(a->disk->pdev_path, "")); > = > I agree that these things ought to be true but I don't see what code > in libxl ensures that they definitely are. > = > I think this check ought to be moved to libxl__device_disk_set_backend, > or perhaps even earlier, and should generate an ERROR_INVAL rather > than an assertion failure. Thanks, libxl can return EINVAL without problems from libxl__device_disk_set_backend, so I think it's a fine place to put this check. libxl__device_disk_setdefault should also be a good place, but I feel _backend is where it makes more sense. v7 is on the way. Roger.