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: Fri, 27 May 2011 13:26:35 +0100
Message-ID: <alpine.DEB.2.00.1105271315270.12963@kaball-desktop>
References: <BANLkTikbJFnVzqhnRy0=Vb0S=pgENDZ_qw@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: <BANLkTikbJFnVzqhnRy0=Vb0S=pgENDZ_qw@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>, Stabellini <Stefano.Stabellini@eu.citrix.com>Stefano
List-Id: xen-devel@lists.xenproject.org

On Fri, 27 May 2011, Wei Liu wrote:
> commit 5672b0151ad7904e771e45d934c2f5d8aa8eac73
> 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 backed by any backend, so 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..1b973c1 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -975,6 +975,9 @@ 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);
>      }
> 
>      device.backend_devid = devid;
> @@ -1028,6 +1031,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;

I think you should print an error here, because we should never reach
this point.

Also I think you need to add the LIBXL_DISK_BACKEND_NONE case to
libxl_device_disk_del, libxl_device_disk_local_attach and
libxl_string_to_backend.

The rest of the patch looks good even though disk hotplug is not handled
(but we need QMP support in libxl for that, I know that some patches are
being worked to add it as we speak).