Re: [PATCH 4/8] libxl: Move check for local access to a funciton

[Ian Jackson <Ian.Jackson@eu.citrix.com>]

George Dunlap writes ("[PATCH 4/8] libxl: Move check for local access to a funciton"):
> +static char * libxl__device_disk_find_local_path(libxl__gc *gc, 
> +                                                 const libxl_device_disk *disk) {
...
> +    if (disk->format == LIBXL_DISK_FORMAT_RAW
> +        && disk->script == NULL
> +        && disk->pdev_path) {

libxl__device_disk_local_initiate_attach asserts pdev_path.  Do you
foresee it maybe being NULL and if so how ?

> -    rc = libxl__device_disk_setdefault(gc, disk);
> -    if (rc) goto out;
> +    if(dls->diskpath)
         ^
Missing space.

> +        LOG(DEBUG, "Strange, dls->diskpath already set: %s", dls->diskpath);
>  
> -    /* If this is in a driver domain, or it's not a raw format, or it involves
> -     * running a script, we have to do a local attach. */
> -    if (disk->backend_domname != NULL
> -        || disk->format != LIBXL_DISK_FORMAT_RAW
> -        || disk->script != NULL) {
> +    LOG(DEBUG, "Trying to find local path");
> +
> +    if ((dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk))) {

We normally seem to avoid this kind of   if ((x = ...))  construction
in libxl.

But apart from that and my previous stylistic comments, this patch
looks OK.  I didn't see any other unexpected functional change.

ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel