This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] [PATCH] xl: Perform minimal validation of virtual disk f

To: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xl: Perform minimal validation of virtual disk file while parsing config file
From: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxx>
Date: Thu, 20 Jan 2011 10:08:34 -0500
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Delivery-date: Thu, 20 Jan 2011 07:09:30 -0800
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=4cyoBc344wYcQnBN8PQ0o/IfL4E59xDtA6MnslfjMbA=; b=Ml+chPWaD/xsQbMCM04hg355OmwrlwA7y2syD309MPlyJ6OfbXvTfNr/LdAEGzzT/x s3RC/1oIqWyQeG2N6YaXhHhcOHTjYv9VPppE++KMJjvMflrr2/IRMI44TGCSK8PyrluO 3+Gnt1+BGQMkVDqxthRbmoDkiy9V5l2CdNHJs=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=b5k9opGhOHcv+Bt5TL9XxwKeYWrHCVpsZ6IA53PK2pFLL3iTfDSQdIS1QkatusbniT 9fcAT5z2zvi6/OWSHDcIG4qE66oRSflbaUbxvXKSdb8+2MHBW6OiCr5oQ1vwCumilwWY t6+k5lhbcyEZdM69Fa0e3WgfQvOnVYPYZHJbw=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1295532296.12018.337.camel@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <AANLkTimSUym0u+SNm0AvNp3AZQQFspetAaXmNShkPJd4@xxxxxxxxxxxxxx> <1294995912.8240.86.camel@xxxxxxxxxxxxxxxxxxxxxx> <AANLkTikfUHHc+-gVkgnRJc722wObLF3TumpK5WSfJVAE@xxxxxxxxxxxxxx> <1295024348.12018.222.camel@xxxxxxxxxxxxxxxxxxxxxx> <AANLkTikf3TVJwE_N-OyuS-UhyA8+cgzAG__9hz3AETeM@xxxxxxxxxxxxxx> <AANLkTinJ=PYsC6vbPvU8g2T8NmyohLa=4rd9zfhTMCCO@xxxxxxxxxxxxxx> <AANLkTin1AGxH26158mn37_Oar1PgSSJoJOnGHs+XnxsV@xxxxxxxxxxxxxx> <1295532296.12018.337.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
> So this handles CD-ROM images too? See below...

I didn't think CD-ROM image would be of type PHYSTYPE_PHY.

>> +    if ( (strncmp(file_name, "", sizeof("")) == 0) && (disk_type == 
>> +        return 1;
> Hrm, strlen(file_name) == 0 ? :)

Of course :) I had a bug in that code earlier with a space between
quotes (" ") and I simply pulled out the space and replaced a bug with
stupidity :)

> In which case libxl_cdrom_insert() needs the same addition?
I think you answered this in a follow up email.

> I also wonder about the motivation of the patch. To be honest, usually,
> the best way to validate things like this is to go ahead and try to use
> them and bail with an error at that time. Where do things bail out for
> you and what problems does that cause? I also think that these checks
> are maximal as well as minimal in that if we checked much further we
> would be re-implementing code?

We fail in a very non obvious way when an invalid image path or zero
sized image is passed.  Not only do we perform a whole load of
initialization but also end up wasting time troubleshooting otherwise
trivial issues.  We shouldn't go all the way to spawn qemu and get to
its block device initialization code and then fail in a way that is
not obvious!  Although, once we establish a good communication between
upstream qemu and xl, this validation should go into upstream qemu as
they too would need to perform this kind of validation when run
independently (i.e. without an accelerator in their terminology).


Xen-devel mailing list

<Prev in Thread] Current Thread [Next in Thread>