WARNING - OLD ARCHIVES

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/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it i

To: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxx>
Subject: Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date: Thu, 10 Feb 2011 11:50:42 +0000
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Thu, 10 Feb 2011 03:49:06 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4D52DB1E.6080101@xxxxxxxxx>
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: <4D5060EB.3060109@xxxxxxxxx> <4D52DB1E.6080101@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)
On Wed, 9 Feb 2011, Kamala Narasimhan wrote:
> Description:
> 
> This patch refactors xl disk specific interface to separate disk format and 
> backend types, rename some variables, changes code that is impacted by this 
> interface change.
> 
> Signed-off-by: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxxx>
> 
> Kamala
> 
> PS:  This patch incorporates all the comments made so far that are specific 
> to this patch.

Better, we are almost there.
The only problems I have found are in the changes to
libxl_device_disk_local_attach:


> @@ -1052,36 +1051,44 @@ char * libxl_device_disk_local_attach(li
>      libxl__gc gc = LIBXL_INIT_GC(ctx);
>      const char *dev = NULL;
>      char *ret = NULL;
> -    int phystype = disk->phystype;
> -    switch (phystype) {
> -        case PHYSTYPE_PHY: {
> -            fprintf(stderr, "attaching PHY disk %s to domain 0\n", 
> disk->physpath);
> -            dev = disk->physpath;
> +
> +    switch (disk->backend) {
> +        case DISK_BACKEND_PHY: {
> +            fprintf(stderr, "attaching PHY disk %s to domain 0\n", 
> disk->pdev_path);
> +            dev = disk->pdev_path;
>              break;
>          }
> -        case PHYSTYPE_FILE:
> -            /* let's pretend is tap:aio for the moment */
> -            phystype = PHYSTYPE_AIO;
> -        case PHYSTYPE_AIO:
> -            if (!libxl__blktap_enabled(&gc)) {
> -                dev = disk->physpath;
> +        case DISK_BACKEND_TAP: {
> +            if (disk->format == DISK_FORMAT_VHD)
> +            {
> +                if (libxl__blktap_enabled(&gc))
> +                    dev = libxl__blktap_devpath(&gc, disk->pdev_path, 
> disk->format);
> +                else
> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to 
> open a vhd disk\n");
> +                break;
> +            } else if (disk->format == DISK_FORMAT_QCOW ||
> +                       disk->format == DISK_FORMAT_QCOW2) {
> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a 
> qcow or qcow2 disk image\n");
> +                break;
> +            } else {
> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend 
> "
> +                    "type: %d\n", disk->backend);
>                  break;
>              }
> -        case PHYSTYPE_VHD:
> -            if (libxl__blktap_enabled(&gc))
> -                dev = libxl__blktap_devpath(&gc, disk->physpath, phystype);
> -            else
> -                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to 
> open a vhd disk\n");
> +        }
> +        case DISK_BACKEND_QDISK: {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qdisk 
> "
> +                "image\n");
>              break;
> -        case PHYSTYPE_QCOW:
> -        case PHYSTYPE_QCOW2:
> -            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow 
> or qcow2 disk image\n");
> +        }
> +        case DISK_BACKEND_UNKNOWN:
> +        default: {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
> +                "type: %d\n", disk->backend);
>              break;
> +        }
> +    }
>  
> -        default:
> -            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical 
> type: %d\n", phystype);
> -            break;
> -    }
>      if (dev != NULL)
>          ret = strdup(dev);
>      libxl__free_all(&gc);
> 

The simple raw file case is not supported anywhere, besides it is
possible to attach a qdisk if it is in raw format.
It should be more or less like this:


phy && raw -> OK
phy && anything but raw -> KO
qdisk && raw -> OK
qdisk && anything but raw -> KO
tap && (qcow || qcow2) -> KO
tap && blktap2 enabled && (vhd || raw) -> OK
tap && blktap2 disabled && vhd -> KO
tap && blktap2 disabled && raw -> OK (same as qdisk)


> diff -r 1967c7c290eb tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c        Wed Feb 09 12:03:09 2011 +0000
> +++ b/tools/libxl/xl_cmdimpl.c        Wed Feb 09 10:57:42 2011 -0500
> @@ -361,9 +361,9 @@ static void printf_info(int domid,
>          printf("\t\t(tap\n");
>          printf("\t\t\t(backend_domid %d)\n", 
> d_config->disks[i].backend_domid);
>          printf("\t\t\t(domid %d)\n", d_config->disks[i].domid);
> -        printf("\t\t\t(physpath %s)\n", d_config->disks[i].physpath);
> -        printf("\t\t\t(phystype %d)\n", d_config->disks[i].phystype);
> -        printf("\t\t\t(virtpath %s)\n", d_config->disks[i].virtpath);
> +        printf("\t\t\t(physpath %s)\n", d_config->disks[i].pdev_path);
> +        printf("\t\t\t(phystype %d)\n", d_config->disks[i].backend);
> +        printf("\t\t\t(virtpath %s)\n", d_config->disks[i].vdev);
> 

we should print pdev_path, backend and vdev here

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