From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Subject: Re: [PATCH] libxl: basic support for virtio disk
Date: Tue, 31 May 2011 12:37:50 +0100
Message-ID: <alpine.DEB.2.00.1105311232300.12963@kaball-desktop>
References: <BANLkTikbJFnVzqhnRy0=Vb0S=pgENDZ_qw@mail.gmail.com>
	<alpine.DEB.2.00.1105271315270.12963@kaball-desktop>
	<BANLkTikM4AJz1jQi2v4vwM=EP7TEfUP-YQ@mail.gmail.com>
	<alpine.DEB.2.00.1105271559170.12963@kaball-desktop>
	<BANLkTikA_Gw8fgQ4K_=eEg=jkx1TDKMSwQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <BANLkTikA_Gw8fgQ4K_=eEg=jkx1TDKMSwQ@mail.gmail.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Wei Liu <liuw@liuw.name>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>
List-Id: xen-devel@lists.xenproject.org

On Mon, 30 May 2011, Wei Liu wrote:
> I hope that I catch your ideas correctly.
> 

yes, pretty close


> ------8<--------------
> 
> commit 0d173004c1f023121680bff0483f7cfe8f2c494e
> Author: Wei Liu <liuw@liuw.name>
> Date:   Fri May 27 10:22:05 2011 +0800
> 
>     libxl: basic virtio disk support.
> 
>     Use "vd*" in vm config file to enable virtio disk.
> 
>     Virtio disk is not storing any information in Xenstore, since it is not
>     backed by any backend. A new backend type NONE is added. More work is
>     needed to support hotplug virtio disk.
> 
>     Signed-off-by: Wei Liu <liuw@liuw.name>
> 
> diff --git a/docs/misc/vbd-interface.txt b/docs/misc/vbd-interface.txt
> index d97c458..83cbe39 100644
> --- a/docs/misc/vbd-interface.txt
> +++ b/docs/misc/vbd-interface.txt
> @@ -8,7 +8,7 @@ emulated IDE or SCSI disks.
>  The abstract interface involves specifying, for each block device:
> 
>   * Nominal disk type: Xen virtual disk (aka xvd*, the default); SCSI
> -   (sd*); IDE (hd*).
> +   (sd*); IDE (hd*); Virtio disk (vd*).
> 
>     For HVM guests, each whole-disk hd* and and sd* device is made
>     available _both_ via emulated IDE resp. SCSI controller, _and_ as a
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index ccf6518..0df9a18 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -975,6 +975,10 @@ int libxl_device_disk_add(libxl_ctx *ctx,
> uint32_t domid, libxl_device_disk *dis
>                 " virtual disk identifier %s", disk->vdev);
>          rc = ERROR_INVAL;
>          goto out_free;
> +    } else if (devid==-2) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_INFO, "Using QEMU virtio backend for"
> +                   " virtual disk %s", disk->vdev);
> +        goto out_free;
>      }
> 
>      device.backend_devid = devid;
> @@ -1028,6 +1032,9 @@ int libxl_device_disk_add(libxl_ctx *ctx,
> uint32_t domid, libxl_device_disk *dis
> 
> libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
>              device.backend_kind = DEVICE_QDISK;
>              break;
> +        case LIBXL_DISK_BACKEND_NONE:
> +	    /* Nothing to do, not a Xen VBD */
> +	    break;
>          default:
>              LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk
> backend type: %d\n", disk->backend);
>              rc = ERROR_INVAL;
> @@ -1095,6 +1102,10 @@ int libxl_device_disk_del(libxl_ctx *ctx, uint32_t domid,
>          case LIBXL_DISK_BACKEND_QDISK:
>              device.backend_kind = DEVICE_QDISK;
>              break;
> +        case LIBXL_DISK_BACKEND_NONE:
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to delete a
> none backend\n");
> +            rc = ERROR_INVAL;
> +            goto out_free;
>          default:
>              LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk
> backend type: %d\n",
>                         disk->backend);
> @@ -1167,6 +1178,8 @@ char * libxl_device_disk_local_attach(libxl_ctx
> *ctx, libxl_device_disk *disk)
>                  disk->pdev_path);
>              dev = disk->pdev_path;
>              break;
> +        case LIBXL_DISK_BACKEND_NONE:
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to attach a
> none backend\n");

I think you are missing a break; here.

>          case LIBXL_DISK_BACKEND_UNKNOWN:
>          default:
>              LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
> diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
> index a5be66f..6b27eae 100644
> --- a/tools/libxl/libxl.idl
> +++ b/tools/libxl/libxl.idl
> @@ -54,6 +54,7 @@ libxl_disk_backend = Enumeration("disk_backend", [
>      (1, "PHY"),
>      (2, "TAP"),
>      (3, "QDISK"),
> +    (4, "NONE"),
>      ])
> 
>  libxl_nic_type = Enumeration("nic_type", [
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 5d85822..25b783c 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -239,6 +239,13 @@ int libxl__device_disk_dev_number(char *virtpath,
> int *pdisk, int *ppartition)
>          if (ppartition) *ppartition = partition;
>          return (8 << 8) | (disk << 4) | partition;
>      }
> +    if (device_virtdisk_matches(virtpath, "vd",
> +                                &disk, 15,
> +                                &partition, 15)) {
> +        if (pdisk) *pdisk = disk;
> +        if (ppartition) *ppartition = partition;
> +        return -2;

is 15 really the limit for virtio disks and partitions?


> +    }
>      return -1;
>  }
> 

you missed libxl__device_disk_string_of_backend in this file.
The rest looks good to me.