From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH] xl: Perform minimal validation of virtual disk file while parsing config file Date: Wed, 26 Jan 2011 10:27:34 +0000 Message-ID: <1296037654.14780.6743.camel@zakaz.uk.xensource.com> References: <1294995912.8240.86.camel@zakaz.uk.xensource.com> <1295024348.12018.222.camel@qabil.uk.xensource.com> <19768.22912.878633.622270@mariner.uk.xensource.com> <19769.31094.274199.464586@mariner.uk.xensource.com> <1295616468.12018.352.camel@qabil.uk.xensource.com> <4D3A4203.2050706@gmail.com> <19775.4623.927492.247420@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <19775.4623.927492.247420@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Jackson Cc: Kamala Narasimhan , "xen-devel@lists.xensource.com" , Gianni Tedesco , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Tue, 2011-01-25 at 18:10 +0000, Ian Jackson wrote: > Kamala Narasimhan writes ("Re: [Xen-devel] [PATCH] xl: Perform minimal validation of virtual disk file while parsing config file"): > > Ian - Apologies for the delay. I think I have covered all comments so far. If there are more I will get to it ASAP. Please let me know. > > Thanks, I have applied your patch, with a minor tweak to make it work > with Stefano's PHYSTYPE_EMPTY patch. With this patch applied I get an error using an LVM device via file:// (which is currently necessary on upstream dom0 support in order to activate the qdisk support): # xl cr -c /etc/xen/debian-x86_32p-1 Parsing config file /etc/xen/debian-x86_32p-1 libxl: error: libxl.c:854:validate_virtual_disk Virtual disk /dev/VG/debian-x86_32-1 size is 0! My disk stanza is # grep ^disk /etc/xen/debian-x86_32p-1 disk = ['file:/dev/VG/debian-x86_32-1,xvda,w'] I think the issue is in this fragment: if ( disk_type == PHYSTYPE_PHY ) { if ( !(S_ISBLK(stat_buf.st_mode)) ) { LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a block device!\n", file_name); return ERROR_INVAL; } } else if ( stat_buf.st_size == 0 ) { LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s size is 0!\n", file_name); return ERROR_INVAL; } since stat(2) says that st_size is only valid for a regular file or symbolic link (the latter being irrelevant in this case since we are using stat() and not lstat()). Ian. 8<-------------------- # HG changeset patch # User Ian Campbell # Date 1296037362 0 # Node ID e4e69622dc95037eab6740f79ecf9c1d05bca529 # Parent 751232406cedb808149ece09480f7a7ec552483d libxl: only check size of regular files when validating a virtual disk st_size is only valid for regular files and not block devices. Signed-off-by: Ian Campbell diff -r 751232406ced -r e4e69622dc95 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Wed Jan 26 09:11:34 2011 +0000 +++ b/tools/libxl/libxl.c Wed Jan 26 10:22:42 2011 +0000 @@ -850,7 +850,7 @@ static int validate_virtual_disk(libxl_c file_name); return ERROR_INVAL; } - } else if ( stat_buf.st_size == 0 ) { + } else if ( S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0 ) { LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s size is 0!\n", file_name); return ERROR_INVAL; }