Re: [Xen-devel] [PATCH 4 of 4] libxl: Use libblktapctl.so

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

> -static char *get_blktap2_device(struct libxl_ctx *ctx, char *name, char 
> *type)
> +static char *make_blktap2_device(struct libxl_ctx *ctx,
> +                              const char *name, const char *type)
>  {
> -    char buf[1024];
> -    char *p;
> -    int devnum;
> -    FILE *f = fopen("/sys/class/blktap2/devices", "r");
> -
> -    
> -    while (!feof(f)) {
> -        if (fscanf(f, "%d %s", &devnum, buf) != 2)
> -            continue;
> -        p = strchr(buf, ':');
> -        if (p == NULL)
> -            continue;
> -        p++;
> -        if (!strcmp(p, name) && !strncmp(buf, type, 3)) {
> -            fclose(f);
> -            return libxl_sprintf(ctx, "/dev/xen/blktap-2/tapdev%d", devnum);
> -        }
> -    }
> -    fclose(f);
> -    return NULL;
> +    char *params, *devname;
> +    int err;
> +    params = libxl_sprintf(ctx, "%s:%s", type, name);
> +    devname = NULL;
> +    err = tap_ctl_create(params, &devname);
> +    free(params);
> +    return err ? NULL : devname;
>  }
> 

you shouldn't free pointers returned by libxl internal functions,
because libxl will take care of free'ing them.

 
>  int libxl_device_disk_add(struct libxl_ctx *ctx, uint32_t domid, 
> libxl_device_disk *disk)
> @@ -1328,38 +1310,16 @@
>          case PHYSTYPE_FILE:
>              /* let's pretend is tap:aio for the moment */
>              disk->phystype = PHYSTYPE_AIO;
> -        case PHYSTYPE_AIO: case PHYSTYPE_QCOW: case PHYSTYPE_QCOW2: case 
> PHYSTYPE_VHD:
> -            if (is_blktap2_supported()) {
> -                int rc, c, p[2], tot;
> -                char buf[1024], *dev;
> -                dev = get_blktap2_device(ctx, disk->physpath, 
> device_disk_string_of_phystype(disk->phystype));
> -                if (dev == NULL) {
> -                    rc= libxl_pipe(ctx, p);  if (rc==-1) return -1;
> -                    rc= libxl_fork(ctx);  if (rc==-1) return -1;
> -                    if (!rc) { /* child */
> -                        int null_r, null_w;
> -                        char *args[4];
> -                        args[0] = "tapdisk2";
> -                        args[1] = "-n";
> -                        args[2] = libxl_sprintf(ctx, "%s:%s", 
> device_disk_string_of_phystype(disk->phystype), disk->physpath);
> -                        args[3] = NULL;
> -
> -                        null_r = open("/dev/null", O_RDONLY);
> -                        null_w = open("/dev/null", O_WRONLY);
> -                        libxl_exec(null_r, p[1], null_w,
> -                                   libxl_abs_path(ctx, "tapdisk2",
> -                                                  libxl_sbindir_path()),
> -                                   args);
> -                        XL_LOG(ctx, XL_LOG_ERROR, "Error execing tapdisk2");
> -                    }
> -                    close(p[1]);
> -                    tot = 0;
> -                    while ((c = read(p[0], buf + tot, sizeof(buf) - tot)) > 
> 0)
> -                        tot = tot + c;
> -                    close(p[0]);
> -                    buf[tot - 1] = '\0';
> -                    dev = buf;
> -                }
> +        case PHYSTYPE_AIO: case PHYSTYPE_QCOW: case PHYSTYPE_QCOW2: case 
> PHYSTYPE_VHD: {
> +            const char *msg;
> +            if (!tap_ctl_check(&msg)) {
> +                const char *type = 
> device_disk_string_of_phystype(disk->phystype);
> +                char *dev;
> +                dev = get_blktap2_device(ctx, disk->physpath, type);
> +                if (!dev)
> +                    dev = make_blktap2_device(ctx, disk->physpath, type);
> +                if (!dev)
> +                    return -1;
>                  flexarray_set(back, boffset++, "tapdisk-params");
>                  flexarray_set(back, boffset++, libxl_sprintf(ctx, "%s:%s", 
> device_disk_string_of_phystype(disk->phystype), disk->physpath));
>                  flexarray_set(back, boffset++, "params");
> 

you are calling make_blktap2_device only if(!dev), are you sure it is
correct? get_blktap2_device returns a pointer on success...

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