Wei Liu
2011-Jul-28 02:08 UTC
[Xen-devel] [RFC PATCH] libxl: basic virtio disk / nic configuration support.
This is a prototype patch for libxl to write virtio disk and nic
configurations in xenstore.
I have following questions on this patch:
1. VBD encoding for virtio disk.
In fact, virtio disk driver doesn''t care about this encoding, so
arbitrary number is OK. I also discover a strange bug in xenstore -- if
I use (2<<28) | (disk<<4) | partition (reserved encoding), it will
trigger a xenstore parsing bug which affects existing protocol (VIF, VFB
etc.), causing them fail to initialize frontend and backend.
2. Configuration syntax and implementation for virtio disk and nic.
Currently some hacks are needed to distinguish virtio devices. I hope we
can get a cleaner implementation.
3. Hot plug / unplug support
This may require QMP support. Also proper backend implementation is
needed. Not an easy job. So I left this part as "not implemented".
Your comments are welcomed.
Wei.
---------8<-------------------------------------------------------
commit 34ff77a504f89b810b8145145b7f1884f05481dc
Author: Wei Liu <liuw@liuw.name>
Date: Mon Jul 18 11:34:50 2011 +0800
libxl: basic virtio disk / nic configuration support.
* virtio disk configuration syntax support.
Use "vd*" in vm config file to enable virtio disk for hvm.
See docs/misc/vbd-interface.txt for more information.
Pratically Linux virtio implementation imposes a limit of 15
partitions since it encodes the partition number in 4 bits. So
the limit for virtio disk is 15.
* add new xenstore fe/be protocol for virtio disk and nic.
The "protocol" is used when constructing xenstore path. It is like
/local/domain/$DOMID/device/$PROTOCOL. Two new protocols are
introduced -- virtio-blk and virtio-net.
After setting up the correct paths, xenpv qemu and Linux kernel
can read from / write to related entries.
diff --git a/docs/misc/vbd-interface.txt b/docs/misc/vbd-interface.txt
index 3952e73..3073ea2 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
@@ -54,6 +54,7 @@ The config file syntaxes are, for example
d536p37 xvdtq37 Xen virtual disk 536 partition 37
sdb3 SCSI disk 1 partition 3
hdc2 IDE disk 2 partition 2
+ vda Virtio disk 0 (whole disk)
The d*p* syntax is not supported by xm/xend.
diff --git a/docs/misc/xl-disk-configuration.txt
b/docs/misc/xl-disk-configuration.txt
index 91e0cc7..fbc6d25 100644
--- a/docs/misc/xl-disk-configuration.txt
+++ b/docs/misc/xl-disk-configuration.txt
@@ -98,7 +98,7 @@ vdev
Description: Virtual device as seen by the guest (also
referred to as guest drive designation in some
specifications). See docs/misc/vbd-interface.txt.
-Supported values: hd[x], xvd[x], sd[x] etc. Please refer to the
+Supported values: hd[x], xvd[x], sd[x], vd[x] etc. Please refer to the
above specification for further details.
Deprecated values: None
Default Value: None, this parameter is mandatory.
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 83d604c..7221685 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -944,6 +944,15 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *dis
device.domid = domid;
device.kind = DEVICE_VBD;
+ if (!memcmp(disk->vdev, "vd", 2)) {
+ LIBXL__LOG(ctx, LIBXL__LOG_INFO, "Using QEMU virtio backend
for"
+ " virtual disk %s", disk->vdev);
+ /* XXX liuw: We have no place for virtio disk in config
+ * syntax. If we detect virtio disk, set up proper backend here. */
+ disk->backend = LIBXL_DISK_BACKEND_VIRTIO;
+ device.kind = DEVICE_VIRTIO_BLK;
+ }
+
switch (disk->backend) {
case LIBXL_DISK_BACKEND_PHY:
dev = disk->pdev_path;
@@ -976,6 +985,12 @@ 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_VIRTIO:
+ flexarray_append(back, "params");
+ flexarray_append(back, libxl__sprintf(&gc, "%s:%s",
+ libxl__device_disk_string_of_format(disk->format),
disk->pdev_path));
+ device.backend_kind = DEVICE_VIRTIO_BLK;
+ break;
default:
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend
type: %d\n", disk->backend);
rc = ERROR_INVAL;
@@ -1043,6 +1058,11 @@ 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_VIRTIO:
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to delete a virtio
disk"
+ " backend, not implemented yet\n");
+ rc = ERROR_INVAL;
+ goto out_free;
default:
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend
type: %d\n",
disk->backend);
@@ -1106,6 +1126,10 @@ 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_VIRTIO:
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to attach a virtio
disk backend,"
+ " not implemented yet\n");
+ break;
default:
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend
"
"type: %d", disk->backend);
@@ -1201,6 +1225,12 @@ int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid,
libxl_device_nic *nic)
device.domid = domid;
device.kind = DEVICE_VIF;
+ if (!memcmp(nic->model, "virtio", 6)) {
+ /* XXX liuw: hack for virtio nic for pure pv */
+ device.kind = DEVICE_VIRTIO_NET;
+ device.backend_kind = DEVICE_VIRTIO_NET;
+ }
+
flexarray_append(back, "frontend-id");
flexarray_append(back, libxl__sprintf(&gc, "%d", domid));
flexarray_append(back, "online");
@@ -1257,6 +1287,13 @@ int libxl_device_nic_del(libxl_ctx *ctx, uint32_t domid,
libxl__device device;
int rc;
+ if (!memcpy(nic->model, "virtio", 6)) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to delete a virtio nic
backend,"
+ " not implemented yet\n");
+ rc = ERROR_INVAL;
+ goto out_free;
+ }
+
device.backend_devid = nic->devid;
device.backend_domid = nic->backend_domid;
device.backend_kind = DEVICE_VIF;
@@ -1265,6 +1302,8 @@ int libxl_device_nic_del(libxl_ctx *ctx, uint32_t domid,
device.kind = DEVICE_VIF;
rc = libxl__device_del(&gc, &device, wait);
+
+out_free:
libxl__free_all(&gc);
return rc;
}
diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index 183d2cd..5027b4a 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -54,11 +54,13 @@ libxl_disk_backend = Enumeration("disk_backend", [
(1, "PHY"),
(2, "TAP"),
(3, "QDISK"),
+ (4, "VIRTIO"),
])
libxl_nic_type = Enumeration("nic_type", [
(1, "IOEMU"),
(2, "VIF"),
+ (3, "VIRTIO"),
])
libxl_action_on_shutdown = Enumeration("action_on_shutdown", [
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index b644ce3..0ccc3f8 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -36,6 +36,8 @@ static const char *string_of_kinds[] = {
[DEVICE_VFB] = "vfb",
[DEVICE_VKBD] = "vkbd",
[DEVICE_CONSOLE] = "console",
+ [DEVICE_VIRTIO_BLK] = "virtio-blk",
+ [DEVICE_VIRTIO_NET] = "virtio-net",
};
char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device)
@@ -162,11 +164,14 @@ static int disk_try_backend(disk_try_backend_args *a,
case LIBXL_DISK_BACKEND_QDISK:
return backend;
+ case LIBXL_DISK_BACKEND_VIRTIO:
+ return backend;
+
default:
LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend "
" %d unknown", a->disk->vdev, backend);
return 0;
-
+
}
abort(); /* notreached */
@@ -177,7 +182,7 @@ static int disk_try_backend(disk_try_backend_args *a,
libxl_disk_backend_to_string(backend),
libxl_disk_format_to_string(a->disk->format));
return 0;
-}
+}
int libxl__device_disk_set_backend(libxl__gc *gc, libxl_device_disk *disk) {
libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -254,6 +259,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_VIRTIO: return "virtio";
default: return NULL;
}
}
@@ -358,6 +364,15 @@ int libxl__device_disk_dev_number(const char *virtpath, int
*pdisk,
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;
+ if (partition > 0) return -1; /* Only support whole disk */
+ /* XXX liuw: VBD encoding is to be discussed */
+ return (8 << 8) | (disk << 4) | partition;
+ }
return -1;
}
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index e8a7664..603e23e 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -466,6 +466,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",
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3175368..9c94dfe 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -102,9 +102,11 @@ typedef enum {
DEVICE_VFB,
DEVICE_VKBD,
DEVICE_CONSOLE,
+ DEVICE_VIRTIO_BLK,
+ DEVICE_VIRTIO_NET,
} libxl__device_kinds;
-#define is_valid_device_kind(kind) (((kind) >= DEVICE_VIF) &&
((kind) <= DEVICE_CONSOLE))
+#define is_valid_device_kind(kind) (((kind) >= DEVICE_VIF) &&
((kind) <= DEVICE_VIRTIO_NET))
typedef struct {
uint32_t backend_devid;
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 731b27e..8cdcb89 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, "virtio")) {
+ *backend = LIBXL_DISK_BACKEND_VIRTIO;
}
out:
return rc;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jul-28 13:05 UTC
[Xen-devel] Re: [RFC PATCH] libxl: basic virtio disk / nic configuration support.
On Thu, 28 Jul 2011, Wei Liu wrote:> This is a prototype patch for libxl to write virtio disk and nic > configurations in xenstore. > > I have following questions on this patch: > > 1. VBD encoding for virtio disk. > In fact, virtio disk driver doesn''t care about this encoding, so > arbitrary number is OK. I also discover a strange bug in xenstore -- if > I use (2<<28) | (disk<<4) | partition (reserved encoding), it will > trigger a xenstore parsing bug which affects existing protocol (VIF, VFB > etc.), causing them fail to initialize frontend and backend.Apart from the strange bug, I think that the encoding is reasonable.> 2. Configuration syntax and implementation for virtio disk and nic. > Currently some hacks are needed to distinguish virtio devices. I hope we > can get a cleaner implementation.I think you need to add a new field in libxl_device_disk to specify the protocol, something like libxl_disk_protocol, that can be: LIBXL_DISK_PROTOCOL_XENVBD LIBXL_DISK_PROTOCOL_VIRTIO then parse_disk_config can be modified to distinguish xen disks from virtio disks and set the field accordingly. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei Liu
2011-Jul-29 06:35 UTC
[Xen-devel] Re: [RFC PATCH] libxl: basic virtio disk / nic configuration support.
On Thu, Jul 28, 2011 at 02:05:08PM +0100, Stefano Stabellini wrote:> On Thu, 28 Jul 2011, Wei Liu wrote: > > This is a prototype patch for libxl to write virtio disk and nic > > configurations in xenstore. > > > > I have following questions on this patch: > > > > 1. VBD encoding for virtio disk. > > In fact, virtio disk driver doesn''t care about this encoding, so > > arbitrary number is OK. I also discover a strange bug in xenstore -- if > > I use (2<<28) | (disk<<4) | partition (reserved encoding), it will > > trigger a xenstore parsing bug which affects existing protocol (VIF, VFB > > etc.), causing them fail to initialize frontend and backend. > > Apart from the strange bug, I think that the encoding is reasonable. >A second thought comes to me. Since virtio frontend does not care about this encoding, can we just use 0,1,...,n for it? Just like VIF case.> > > 2. Configuration syntax and implementation for virtio disk and nic. > > Currently some hacks are needed to distinguish virtio devices. I hope we > > can get a cleaner implementation. > > I think you need to add a new field in libxl_device_disk to specify the > protocol, something like libxl_disk_protocol, that can be: > > LIBXL_DISK_PROTOCOL_XENVBD > LIBXL_DISK_PROTOCOL_VIRTIO > > then parse_disk_config can be modified to distinguish xen disks from > virtio disks and set the field accordingly.Good idea. Working on this. Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei Liu
2011-Jul-30 02:40 UTC
[Xen-devel] Re: [RFC PATCH] libxl: basic virtio disk / nic configuration support.
On Thu, Jul 28, 2011 at 02:05:08PM +0100, Stefano Stabellini wrote:> > I think you need to add a new field in libxl_device_disk to specify the > protocol, something like libxl_disk_protocol, that can be: > > LIBXL_DISK_PROTOCOL_XENVBD > LIBXL_DISK_PROTOCOL_VIRTIO > > then parse_disk_config can be modified to distinguish xen disks from > virtio disks and set the field accordingly.Hmm... wait... Adding this field is easy. But I don''t know if I fully understand your idea. I am just about to modify the parser. But looking back your replay, you are not suggesting adding configuration syntax support in config file (sort of `disk=["...,protocol=virito"]`). So essentially the new patch will be of no difference to the original one. But one advantage is that your plan seems cleaner (not exposing hacks to other functions). Do I get your point? Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Aug-01 08:51 UTC
[Xen-devel] Re: [RFC PATCH] libxl: basic virtio disk / nic configuration support.
On Sat, 30 Jul 2011, Wei Liu wrote:> On Thu, Jul 28, 2011 at 02:05:08PM +0100, Stefano Stabellini wrote: > > > > I think you need to add a new field in libxl_device_disk to specify the > > protocol, something like libxl_disk_protocol, that can be: > > > > LIBXL_DISK_PROTOCOL_XENVBD > > LIBXL_DISK_PROTOCOL_VIRTIO > > > > then parse_disk_config can be modified to distinguish xen disks from > > virtio disks and set the field accordingly. > > Hmm... wait... > > Adding this field is easy. But I don''t know if I fully understand your > idea. > > I am just about to modify the parser. But looking back your replay, > you are not suggesting adding configuration syntax support in config > file (sort of `disk=["...,protocol=virito"]`). > > So essentially the new patch will be of no difference to the original > one. But one advantage is that your plan seems cleaner (not exposing > hacks to other functions). > > Do I get your point?Yes, that was my main point. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel