Re: [Xen-devel] [PATCH] xl: Perform minimal validation of virtual disk file while parsing config file

[Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>]

On Thu, 2011-01-20 at 14:04 +0000, Gianni Tedesco wrote:
> On Wed, 2011-01-19 at 18:26 +0000, Kamala Narasimhan wrote:
> > Apologies.  I inadvertently neglected Gianni's suggestion to switch to
> > logging from fprintf.
> > 
> > Signed-off-by: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxxx>
> > 
> > Kamala
> > 
> > diff -r fe8a177ae9cb tools/libxl/libxl.c
> > --- a/tools/libxl/libxl.c       Wed Jan 19 15:29:04 2011 +0000
> > +++ b/tools/libxl/libxl.c       Wed Jan 19 13:23:16 2011 -0500
> > @@ -826,6 +826,41 @@ skip_autopass:
> > 
> >  
> > /******************************************************************************/
> > 
> > +static int validate_virtual_disk(libxl_ctx *ctx, char *file_name,
> > libxl_disk_phystype disk_type)
> > +{
> > +    struct stat stat_buf;
> > +
> > +    if ( file_name == NULL ) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk file name is 
> > NULL!\n");
> > +        return 0;
> > +    }
> 
> I prefer assert() for things caused by programmer error. But in this
> case we could just let the dereference below catch it...
> 
> > +    /* Return without further validation for empty cdrom drive.
> > +       Note: Post 4.1 we need to change the interface to handle empty
> > +       cdrom rather than go with the below assumption.
> > +     */
> 
> So this handles CD-ROM images too? See below...
> In which case libxl_cdrom_insert() needs the same addition?

Ah, my mistake, libxl_cdrom_insert() is implemented in terms of
libxl_device_disk_add() so only one check is necessary.

Gianni


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel