[PATCH] libxl: basic support for virtio disk

[PATCH] libxl: basic support for virtio disk

[Wei Liu]

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;
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk
backend type: %d\n", disk->backend);
             rc = ERROR_INVAL;
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;
+    }
     return -1;
 }

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 76479fe..407e8c5 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -419,6 +419,10 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
                     drive = libxl__sprintf
                         (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s",
                          disks[i].pdev_path, disk, format);
+                else if (strncmp(disks[i].vdev, "vd", 2) == 0)
+                    drive = libxl__sprintf
+                        (gc, "file=%s,if=virtio,index=%d,media=disk,format=%s",
+                         disks[i].pdev_path, disk, format);
                 else if (disk < 4)
                     drive = libxl__sprintf
                         (gc, "file=%s,if=ide,index=%d,media=disk,format=%s",
@@ -976,6 +980,7 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,

             case LIBXL_DISK_BACKEND_PHY:
             case LIBXL_DISK_BACKEND_UNKNOWN:
+            case LIBXL_DISK_BACKEND_NONE:
                 break;
             }
         }

-- 
Best regards
Wei Liu
Twitter: @iliuw
Site: http://liuw.name

Re: [PATCH] libxl: basic support for virtio disk

[Stefano Stabellini]

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).

Re: [PATCH] libxl: basic support for virtio disk

[Wei Liu]

On Fri, May 27, 2011 at 8:26 PM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
>
> I think you should print an error here, because we should never reach
> this point.
>

You mean we should never setup Xenstore entries for a virtio disk?

> 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.
>

I notice those functions. But I haven't decide what to add to them.
But if we are not storing any information in Xenstore for virtio disk,
the modification to these functions should be minimal.

> 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).
>
>



-- 
Best regards
Wei Liu
Twitter: @iliuw
Site: http://liuw.name

Re: [PATCH] libxl: basic support for virtio disk

[Stefano Stabellini]

On Fri, 27 May 2011, Wei Liu wrote:
> On Fri, May 27, 2011 at 8:26 PM, Stefano Stabellini
> <stefano.stabellini@eu.citrix.com> wrote:
> >
> > I think you should print an error here, because we should never reach
> > this point.
> >
> 
> You mean we should never setup Xenstore entries for a virtio disk?

I meant that the change is for a code path that we are never going to
reach with a virtio disk because at the beginning of
libxl_device_disk_add libxl__device_disk_dev_number will return -2.

But now that I think about it, it is probably a good idea to add the
LIBXL_DISK_BACKEND_NONE case anyway because we must think of the most
general use case.


> > 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.
> >
> 
> I notice those functions. But I haven't decide what to add to them.
> But if we are not storing any information in Xenstore for virtio disk,
> the modification to these functions should be minimal.

right

Re: [PATCH] libxl: basic support for virtio disk

[Wei Liu]

I hope that I catch your ideas correctly.

------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");
         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;
+    }
     return -1;
 }

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 76479fe..407e8c5 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -419,6 +419,10 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
                     drive = libxl__sprintf
                         (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s",
                          disks[i].pdev_path, disk, format);
+                else if (strncmp(disks[i].vdev, "vd", 2) == 0)
+                    drive = libxl__sprintf
+                        (gc, "file=%s,if=virtio,index=%d,media=disk,format=%s",
+                         disks[i].pdev_path, disk, format);
                 else if (disk < 4)
                     drive = libxl__sprintf
                         (gc, "file=%s,if=ide,index=%d,media=disk,format=%s",
@@ -976,6 +980,7 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,

             case LIBXL_DISK_BACKEND_PHY:
             case LIBXL_DISK_BACKEND_UNKNOWN:
+            case LIBXL_DISK_BACKEND_NONE:
                 break;
             }
         }
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 68b5a9a..7785c24 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -300,6 +300,8 @@ int libxl_string_to_backend(libxl_ctx *ctx, char
*s, libxl_disk_backend *backend
         } else if (!strcmp(p, "qcow2")) {
             *backend = LIBXL_DISK_BACKEND_QDISK;
         }
+    } else if (!strcmp(s, "none")) {
+        *backend = LIBXL_DISK_BACKEND_NONE;
     }
 out:
     return rc;


-- 
Best regards
Wei Liu
Twitter: @iliuw
Site: http://liuw.name

Re: [PATCH] libxl: basic support for virtio disk

[Stefano Stabellini]

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.

Re: [PATCH] libxl: basic support for virtio disk

[Wei Liu]

Revised patch.

Add code in libxl__device_disk_string_of_backend.

Upper limit of virtio disk follows scsi.

----------8<-------------

commit eef16001490d16e9c1ea5fe84cf3f1f59e270752
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.

    Upper limit of virtio disk is the same as scsi. 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..aa351e2 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,9 @@ 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");
+            break;
         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..2bce7d7 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -136,6 +136,7 @@ char
*libxl__device_disk_string_of_backend(libxl_disk_backend backend)
         case LIBXL_DISK_BACKEND_QDISK: return "qdisk";
         case LIBXL_DISK_BACKEND_TAP: return "phy";
         case LIBXL_DISK_BACKEND_PHY: return "phy";
+        case LIBXL_DISK_BACKEND_NONE: return "none";
         default: return NULL;
     }
 }
@@ -239,6 +240,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;
+    }
     return -1;
 }

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 76479fe..407e8c5 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -419,6 +419,10 @@ static char **
libxl__build_device_model_args_new(libxl__gc *gc,
                     drive = libxl__sprintf
                         (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s",
                          disks[i].pdev_path, disk, format);
+                else if (strncmp(disks[i].vdev, "vd", 2) == 0)
+                    drive = libxl__sprintf
+                        (gc, "file=%s,if=virtio,index=%d,media=disk,format=%s",
+                         disks[i].pdev_path, disk, format);
                 else if (disk < 4)
                     drive = libxl__sprintf
                         (gc, "file=%s,if=ide,index=%d,media=disk,format=%s",
@@ -976,6 +980,7 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,

             case LIBXL_DISK_BACKEND_PHY:
             case LIBXL_DISK_BACKEND_UNKNOWN:
+            case LIBXL_DISK_BACKEND_NONE:
                 break;
             }
         }
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 68b5a9a..7785c24 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -300,6 +300,8 @@ int libxl_string_to_backend(libxl_ctx *ctx, char
*s, libxl_disk_backend *backend
         } else if (!strcmp(p, "qcow2")) {
             *backend = LIBXL_DISK_BACKEND_QDISK;
         }
+    } else if (!strcmp(s, "none")) {
+        *backend = LIBXL_DISK_BACKEND_NONE;
     }
 out:
     return rc;

Re: [PATCH] libxl: basic support for virtio disk

[Ian Jackson]

Wei Liu writes ("[Xen-devel] Re: [PATCH] libxl: basic support for virtio disk"):
> Revised patch.
> 
> Add code in libxl__device_disk_string_of_backend.
> 
> Upper limit of virtio disk follows scsi.

I'm not sure what you mean here.  Do you mean that Linux only supports
as many virtio disks as it supports scsi disks ?  Is this a
fundamental limitation of the virtio protocol ?

> +                else if (strncmp(disks[i].vdev, "vd", 2) == 0)
> +                    drive = libxl__sprintf
> +                        (gc, "file=%s,if=virtio,index=%d,media=disk,format=%s",
> +                         disks[i].pdev_path, disk, format);

Maybe I'm missing something but this seems not to use the partition
number at all ?

The existing code seems rather broken TBH.

Ian.

Re: Re: [PATCH] libxl: basic support for virtio disk

[Wei Liu]

On Wed, Jun 1, 2011 at 9:59 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> Wei Liu writes ("[Xen-devel] Re: [PATCH] libxl: basic support for virtio disk"):
>> Revised patch.
>>
>> Add code in libxl__device_disk_string_of_backend.
>>
>> Upper limit of virtio disk follows scsi.
>
> I'm not sure what you mean here.  Do you mean that Linux only supports
> as many virtio disks as it supports scsi disks ?  Is this a
> fundamental limitation of the virtio protocol ?
>

I asked about the limitation in qemu-devel, the answer is

"virtio-blk as used by KVM is exposed as a virtio PCI adapter.  There
is a 1:1 mapping between virtio-blk, PCI adapters, and block devices
being presented by QEMU:

1 virtio-blk device in guest == 1 virtio-pci adapter in guest == 1
block device in QEMU

The maximum number is really limited by the PCI bus, not virtio.  In
terms of coding, you should try not to impose a hard limit at all."

Thus Stefano suggest I use the same limitation as SCSI disk.

>> +                else if (strncmp(disks[i].vdev, "vd", 2) == 0)
>> +                    drive = libxl__sprintf
>> +                        (gc, "file=%s,if=virtio,index=%d,media=disk,format=%s",
>> +                         disks[i].pdev_path, disk, format);
>
> Maybe I'm missing something but this seems not to use the partition
> number at all ?
>

Also answered in qemu-devel

"Partitions are not at the virtio-blk level.  The guest operating
system will see the virtio-blk disk and scan its partition table to
determine which partitions are available.  The limit then depends on
the partitioning scheme that you use (legacy boot record, GPT, etc)."

So I'm not using the partition number here.

In fact, we should not support vda1, vda2, right?

The discussion thread is at

http://marc.info/?l=qemu-devel&m=130689044627041&w=2

> The existing code seems rather broken TBH.
>

Hmm... I'm not catching up with libxl. Should be more careful next time...

> Ian.
>

Wei.

Re: Re: [PATCH] libxl: basic support for virtio disk

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 2678 bytes --]

On Wed, 1 Jun 2011, Wei Liu wrote:
> On Wed, Jun 1, 2011 at 9:59 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> > Wei Liu writes ("[Xen-devel] Re: [PATCH] libxl: basic support for virtio disk"):
> >> Revised patch.
> >>
> >> Add code in libxl__device_disk_string_of_backend.
> >>
> >> Upper limit of virtio disk follows scsi.
> >
> > I'm not sure what you mean here.  Do you mean that Linux only supports
> > as many virtio disks as it supports scsi disks ?  Is this a
> > fundamental limitation of the virtio protocol ?
> >
> 
> I asked about the limitation in qemu-devel, the answer is
> 
> "virtio-blk as used by KVM is exposed as a virtio PCI adapter.  There
> is a 1:1 mapping between virtio-blk, PCI adapters, and block devices
> being presented by QEMU:
> 
> 1 virtio-blk device in guest == 1 virtio-pci adapter in guest == 1
> block device in QEMU
> 
> The maximum number is really limited by the PCI bus, not virtio.  In
> terms of coding, you should try not to impose a hard limit at all."
> 
> Thus Stefano suggest I use the same limitation as SCSI disk.

The limitation on the number of partitions is meaningless for virtio and
the limitation on the number of disks in arbitrary, as it is arbitrary
the current limitation on the number of scsi disks.



> >> +                else if (strncmp(disks[i].vdev, "vd", 2) == 0)
> >> +                    drive = libxl__sprintf
> >> +                        (gc, "file=%s,if=virtio,index=%d,media=disk,format=%s",
> >> +                         disks[i].pdev_path, disk, format);
> >
> > Maybe I'm missing something but this seems not to use the partition
> > number at all ?
> >
> 
> Also answered in qemu-devel
> 
> "Partitions are not at the virtio-blk level.  The guest operating
> system will see the virtio-blk disk and scan its partition table to
> determine which partitions are available.  The limit then depends on
> the partitioning scheme that you use (legacy boot record, GPT, etc)."
> 
> So I'm not using the partition number here.
> 
> In fact, we should not support vda1, vda2, right?
> 
> The discussion thread is at
> 
> http://marc.info/?l=qemu-devel&m=130689044627041&w=2
> 

Device names like xvda1, xvda2, etc, are currently supported only with
PV guests.
There is no way to specify a single partition with HVM guests.
Considering that this patch is only meant for HVM guest, I think it is
correct to leave out the partition number.


> > The existing code seems rather broken TBH.
> >
> 
> Hmm... I'm not catching up with libxl. Should be more careful next time...
 
Why should this code be broken?
It seems all right to me. Does it break something?

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: Re: [PATCH] libxl: basic support for virtio disk

[Ian Jackson]

Stefano Stabellini writes ("Re: [Xen-devel] Re: [PATCH] libxl: basic support for virtio disk"):
> The limitation on the number of partitions is meaningless for virtio and
> the limitation on the number of disks in arbitrary, as it is arbitrary
> the current limitation on the number of scsi disks.

So the code in libxl should accept any disk number that it can
correctly format into an option to qemu, and allow qemu to deal with
any resulting problems.

> Device names like xvda1, xvda2, etc, are currently supported only with
> PV guests.

Currently the partition number is ignored.  Nonzero partitions should
instead then be rejected.

> There is no way to specify a single partition with HVM guests.
> Considering that this patch is only meant for HVM guest, I think it is
> correct to leave out the partition number.

It should be checked, not ignored.

Ian.

Re: Re: [PATCH] libxl: basic support for virtio disk

[Stefano Stabellini]

On Thu, 2 Jun 2011, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] Re: [PATCH] libxl: basic support for virtio disk"):
> > The limitation on the number of partitions is meaningless for virtio and
> > the limitation on the number of disks in arbitrary, as it is arbitrary
> > the current limitation on the number of scsi disks.
> 
> So the code in libxl should accept any disk number that it can
> correctly format into an option to qemu, and allow qemu to deal with
> any resulting problems.

it is going to be probably INT_MAX then


> > Device names like xvda1, xvda2, etc, are currently supported only with
> > PV guests.
> 
> Currently the partition number is ignored.  Nonzero partitions should
> instead then be rejected.
> 

good point

Re: Re: [PATCH] libxl: basic support for virtio disk

[Wei Liu]

On Fri, Jun 3, 2011 at 12:03 AM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] Re: [PATCH] libxl: basic support for virtio disk"):
>> The limitation on the number of partitions is meaningless for virtio and
>> the limitation on the number of disks in arbitrary, as it is arbitrary
>> the current limitation on the number of scsi disks.
>
> So the code in libxl should accept any disk number that it can
> correctly format into an option to qemu, and allow qemu to deal with
> any resulting problems.
>
>> Device names like xvda1, xvda2, etc, are currently supported only with
>> PV guests.
>
> Currently the partition number is ignored.  Nonzero partitions should
> instead then be rejected.
>
>> There is no way to specify a single partition with HVM guests.
>> Considering that this patch is only meant for HVM guest, I think it is
>> correct to leave out the partition number.
>
> It should be checked, not ignored.
>
> Ian.
>

OK. Looks like the disk parsing code in libxl is going through a re-factoring.

I will get back to this patch when your re-factoring is done.

Wei.