Re: [PATCH][ioemu] strip tap subtype prefix from image name (restyled)

["Christoph Egger" <Christoph.Egger@amd.com>]

On Wednesday 30 January 2008 15:14:24 Pat Campbell wrote:
> More readable version
>
> Currently I am not able to mount or boot from an HVM CDROM when it is
> configured for 'tap:aio' instead of 'file'.
>
> disk=[ 'tap:aio:/var/lib/xen/images/sles10-sp2-fv/disk0,hda,w', '
>        tap:aio:/home/iso/sles/SLES10.iso,hdc:cdrom,r', ]
>
> With the attached patch I am able to boot from the CDROM and or mount it.
>
> Patch changes xenstore.c:xenstore_process_event() to strip the tap subtype
> prefix from the image name.
>
> Please apply to xen-unstable tip.
>
> Signed-off-by: Pat Campbell <plc@novell.com>

That looks much better.
See inline for comments.

> xen-blktab-subtype-strip.patch
> diff -r 1c826ea72a80 tools/ioemu/xenstore.c 
> --- a/tools/ioemu/xenstore.c    Wed Jan 23 15:42:52 2008 +0000
> +++ b/tools/ioemu/xenstore.c    Wed Jan 30 07:06:12 2008 -0700
> @@ -418,7 +418,7 @@ void xenstore_record_dm_state(char *stat
>  
> void xenstore_process_event(void *opaque)
>  {
> -    char **vec, *image = NULL;
> +    char **vec, *offset, *bpath = NULL, *buf = NULL, *drv = NULL, *image = 
NULL;
>      unsigned int len, num, hd_index;
>  
>      vec = xs_read_watch(xsh, &num);
> @@ -440,8 +440,28 @@ void xenstore_process_event(void *opaque
>          goto out;
>      hd_index = vec[XS_WATCH_TOKEN][2] - 'a';
>      image = xs_read(xsh, XBT_NULL, vec[XS_WATCH_PATH], &len);
> -    if (image == NULL || !strcmp(image, bs_table[hd_index]->filename))
> -        goto out;  /* gone or identical */
> +    if (image == NULL)
> +        goto out;  /* gone */
> +
> +    /* Strip off blktap sub-type prefix */
> +    bpath = strdup(vec[XS_WATCH_PATH]); 
> +    if (bpath)

I think, you mean if (!bpath)  here.

> +        goto out;
> +    if ((offset = strrchr(bpath, '/')) != NULL) 
> +        *offset = '\0';
> +    if (pasprintf(&buf, "%s/type", bpath) == -1) 
> +        goto out;
> +    drv = xs_read(xsh, XBT_NULL, buf, &len);
> +    if (drv) {

I think, you mean  "if (!drv) goto out;" here.

> +        if (!strcmp(drv, "tap")) {
> +            offset = strchr(image, ':'); 
> +            if (offset) 
> +                memmove(image, offset+1, strlen(offset+1)+1 );
> +        }
> +    }
> +
> +    if (!strcmp(image, bs_table[hd_index]->filename))
> +        goto out;  /* identical */
>  
>      do_eject(0, vec[XS_WATCH_TOKEN]);
>      bs_table[hd_index]->filename[0] = 0;
> @@ -456,6 +476,9 @@ void xenstore_process_event(void *opaque
>      }
>  
>   out:
> +    free(drv);
> +    free(buf);
> +    free(bpath);
>      free(image);
>      free(vec);

That doesn't work. If strdup() fails, then drv and buf are NULL here.
Analagous counts for the other failures.
You need to check for != NULL before calling free().

>  }


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy