9:28 a.m.
Revised patch against libvirt 0.7.6 to support qemu's aio option.
qemu allows the user to choose what io storage api should be used, either the
default (threads) or native (linux aio) which in the latter case can result in
better performance.
This patch exposes this functionality through libvirt.
Thanks a lot to Eric Blake and Matthias Bolte for their comments.
---
docs/formatdomain.html.in | 4 +++-
docs/schemas/domain.rng | 13 +++++++++++--
src/conf/domain_conf.c | 23 +++++++++++++++++++++++
src/conf/domain_conf.h | 10 ++++++++++
src/qemu/qemu_conf.c | 14 ++++++++++++++
src/qemu/qemu_conf.h | 2 ++
6 files changed, 63 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ce49f7d..ec85ef3 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -478,7 +478,9 @@
attribute is the primary backend driver name, while the optional
<code>type</code>
attribute provides the sub-type. The optional <code>cache</code>
attribute
controls the cache mechanism, possible values are "default",
"none",
- "writethrough" and "writeback". <span
class="since">Since 0.1.8</span>
+ "writethrough" and "writeback". Specific to KVM guests is the
optional <code>aio</code>
+ attribute which controls what storage api is used for io operations, its
possible
+ values are "threads" and "native". <span
class="since">Since 0.1.8; "aio" attribute since
0.7.7</span>
</dd>
<dt><code>encryption</code></dt>
<dd>If present, specifies how the volume is encrypted. See
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index bb6d00d..5d2dafe 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -499,8 +499,9 @@
<ref name="driverCache"/>
</group>
</choice>
- <empty/>
- </element>
+ <optional>
+ <ref name="driverAIO"/>
+ </optional>
</define>
<define name="driverFormat">
<attribute name="name">
@@ -521,6 +522,14 @@
</choice>
</attribute>
</define>
+ <define name="driverAIO">
+ <attribute name="aio">
+ <choice>
+ <value>threads</value>
+ <value>native</value>
+ </choice>
+ </attribute>
+ </define>
<define name="controller">
<element name="controller">
<optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 766993c..8bee7b8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -120,6 +120,11 @@ VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST,
"writethrough",
"writeback")
+VIR_ENUM_IMPL(virDomainDiskAIO, VIR_DOMAIN_DISK_AIO_LAST,
+ "default",
+ "native",
+ "threads")
+
VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST,
"ide",
"fdc",
@@ -1228,6 +1233,7 @@ virDomainDiskDefParseXML(virConnectPtr conn,
char *target = NULL;
char *bus = NULL;
char *cachetag = NULL;
+ char *aiotag = NULL;
char *devaddr = NULL;
virStorageEncryptionPtr encryption = NULL;
char *serial = NULL;
@@ -1293,6 +1299,7 @@ virDomainDiskDefParseXML(virConnectPtr conn,
driverName = virXMLPropString(cur, "name");
driverType = virXMLPropString(cur, "type");
cachetag = virXMLPropString(cur, "cache");
+ aiotag = virXMLPropString(cur, "aio");
} else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) {
def->readonly = 1;
} else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) {
@@ -1409,6 +1416,13 @@ virDomainDiskDefParseXML(virConnectPtr conn,
goto error;
}
+ if (aiotag &&
+ (def->aiomode = virDomainDiskAIOTypeFromString(aiotag)) < 0) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("unknown disk aio mode '%s'"), aiotag);
+ goto error;
+ }
+
if (devaddr) {
if (sscanf(devaddr, "%x:%x:%x",
&def->info.addr.pci.domain,
@@ -1450,6 +1464,7 @@ cleanup:
VIR_FREE(driverType);
VIR_FREE(driverName);
VIR_FREE(cachetag);
+ VIR_FREE(aiotag);
VIR_FREE(devaddr);
VIR_FREE(serial);
virStorageEncryptionFree(encryption);
@@ -4565,6 +4580,7 @@ virDomainDiskDefFormat(virConnectPtr conn,
const char *device = virDomainDiskDeviceTypeToString(def->device);
const char *bus = virDomainDiskBusTypeToString(def->bus);
const char *cachemode = virDomainDiskCacheTypeToString(def->cachemode);
+ const char *aiomode = virDomainDiskAIOTypeToString(def->aiomode);
if (!type) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
@@ -4586,6 +4602,11 @@ virDomainDiskDefFormat(virConnectPtr conn,
_("unexpected disk cache mode %d"),
def->cachemode);
return -1;
}
+ if (!aiomode) {
+ virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("unexpected disk aio mode %d"),
def->aiomode);
+ return -1;
+ }
virBufferVSprintf(buf,
" <disk type='%s'
device='%s'>\n",
@@ -4597,6 +4618,8 @@ virDomainDiskDefFormat(virConnectPtr conn,
virBufferVSprintf(buf, " type='%s'", def->driverType);
if (def->cachemode)
virBufferVSprintf(buf, " cache='%s'", cachemode);
+ if (def->aiomode)
+ virBufferVSprintf(buf, " aio='%s'", aiomode);
virBufferVSprintf(buf, "/>\n");
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0b79e88..07805a6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -141,6 +141,14 @@ enum virDomainDiskCache {
VIR_DOMAIN_DISK_CACHE_LAST
};
+enum virDomainDiskAIO {
+ VIR_DOMAIN_DISK_AIO_DEFAULT,
+ VIR_DOMAIN_DISK_AIO_NATIVE,
+ VIR_DOMAIN_DISK_AIO_THREADS,
+
+ VIR_DOMAIN_DISK_AIO_LAST
+};
+
/* Stores the virtual disk configuration */
typedef struct _virDomainDiskDef virDomainDiskDef;
typedef virDomainDiskDef *virDomainDiskDefPtr;
@@ -154,6 +162,7 @@ struct _virDomainDiskDef {
char *driverType;
char *serial;
int cachemode;
+ int aiomode;
unsigned int readonly : 1;
unsigned int shared : 1;
virDomainDeviceInfo info;
@@ -897,6 +906,7 @@ VIR_ENUM_DECL(virDomainDisk)
VIR_ENUM_DECL(virDomainDiskDevice)
VIR_ENUM_DECL(virDomainDiskBus)
VIR_ENUM_DECL(virDomainDiskCache)
+VIR_ENUM_DECL(virDomainDiskAIO)
VIR_ENUM_DECL(virDomainController)
VIR_ENUM_DECL(virDomainFS)
VIR_ENUM_DECL(virDomainNet)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 3d83a8f..fd3a670 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -83,6 +83,12 @@ VIR_ENUM_IMPL(qemuDiskCacheV2, VIR_DOMAIN_DISK_CACHE_LAST,
"writethrough",
"writeback");
+VIR_ENUM_DECL(qemuDiskAIO)
+VIR_ENUM_IMPL(qemuDiskAIO, VIR_DOMAIN_DISK_AIO_LAST,
+ "default",
+ "native",
+ "threads");
+
VIR_ENUM_DECL(qemuVideo)
VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
@@ -1137,6 +1143,8 @@ static unsigned int qemudComputeCmdFlags(const char *help,
flags |= QEMUD_CMD_FLAG_DRIVE_CACHE_V2;
if (strstr(help, "format="))
flags |= QEMUD_CMD_FLAG_DRIVE_FORMAT;
+ if (strstr(help, "aio=threads|native"))
+ flags |= QEMUD_CMD_FLAG_DRIVE_AIO;
}
if (strstr(help, "-vga") && !strstr(help, "-std-vga"))
flags |= QEMUD_CMD_FLAG_VGA;
@@ -2340,6 +2348,12 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
virBufferAddLit(&opt, ",cache=off");
}
+ if (disk->aiomode && (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_AIO)) {
+ const char * mode = qemuDiskAIOTypeToString(disk->aiomode);
+
+ virBufferVSprintf(&opt, ",aio=%s", mode);
+ }
+
if (virBufferError(&opt)) {
virReportOOMError(NULL);
goto error;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 101f187..780ae6e 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -82,6 +82,8 @@ enum qemud_cmd_flags {
QEMUD_CMD_FLAG_SDL = (1 << 27), /* Is the new -sdl arg available */
QEMUD_CMD_FLAG_SMP_TOPOLOGY = (1 << 28), /* Is sockets=s,cores=c,threads=t
available for -smp? */
QEMUD_CMD_FLAG_NETDEV = (1 << 29), /* The -netdev flag &
netdev_add/remove monitor commands */
+
+ QEMUD_CMD_FLAG_DRIVE_AIO = (1 << 30), /* Is -drive aio= avail */
};
/* Main driver state */
--
1.7.0.4
10:19 a.m.
On 04/21/2010 08:28 AM, Matthias Dahl wrote:
Revised patch against libvirt 0.7.6 to support qemu's aio option.
It's hard to tell from your comments whether you built the patch against
the latest git (0.8.0+).
+++ b/docs/formatdomain.html.in
@@ -478,7 +478,9 @@
attribute is the primary backend driver name, while the optional
<code>type</code>
attribute provides the sub-type. The optional <code>cache</code>
attribute
controls the cache mechanism, possible values are "default",
"none",
- "writethrough" and "writeback". <span
class="since">Since 0.1.8</span>
+ "writethrough" and "writeback". Specific to KVM guests is
the optional <code>aio</code>
+ attribute which controls what storage api is used for io operations, its
possible
+ values are "threads" and "native". <span
class="since">Since 0.1.8; "aio" attribute since 0.7.7</span>
Which means you would want to s/0.7.7/0.8.1/.
</define>
+ <define name="driverAIO">
+ <attribute name="aio">
+ <choice>
+ <value>threads</value>
+ <value>native</value>
+ </choice>
+ </attribute>
+ </define>
<define name="controller">
<element name="controller">
<optional>
...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 766993c..8bee7b8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -120,6 +120,11 @@ VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST,
"writethrough",
"writeback")
+VIR_ENUM_IMPL(virDomainDiskAIO, VIR_DOMAIN_DISK_AIO_LAST,
+ "default",
+ "native",
+ "threads")
Why the use of "default" in the enum, but not in the XML? Is it a third
possibility, or is it an alias for 'threads', in which case 'threads'
should be listed first?
But you are getting closer :)
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
8:39 p.m.
On 04/21/2010 09:19 AM, Eric Blake wrote:
On 04/21/2010 08:28 AM, Matthias Dahl wrote:
> Revised patch against libvirt 0.7.6 to support qemu's aio option.
It's hard to tell from your comments whether you built the patch against
the latest git (0.8.0+).
This is now https://bugzilla.redhat.com/show_bug.cgi?id=591703, and now
that 0.8.1 is out the door, I'm going to take another look at it in the
near future.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:26 p.m.
New subject: [libvirt] [PATCH] Support for qemu aio drive option
From: Matthias Dahl <mdvirt(a)designassembly.de>
qemu allows the user to choose what io storage api should be used, either the
default (threads) or native (linux aio) which in the latter case can result in
better performance.
Thanks a lot to Eric Blake and Matthias Bolte for their comments.
Red Hat Bugzilla #591703
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I've rebased Matthias' patch against the latest git, and discovered
a bug in qemu_conf in the process (separate patch sent for that).
I also discovered that formatdomain.html.in is incomplete; it lacks
mention of at least the recent error_policy option. But I ran out
of time to make a separate patch to bring it all up to date tonight.
docs/formatdomain.html.in | 5 ++++-
docs/schemas/domain.rng | 11 +++++++++++
src/conf/domain_conf.c | 23 +++++++++++++++++++++++
src/conf/domain_conf.h | 10 ++++++++++
src/qemu/qemu_conf.c | 15 ++++++++++++++-
src/qemu/qemu_conf.h | 1 +
tests/qemuhelptest.c | 6 ++++--
7 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a23663a..5c178d8 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -520,7 +520,10 @@
attribute is the primary backend driver name, while the optional
<code>type</code>
attribute provides the sub-type. The optional <code>cache</code>
attribute
controls the cache mechanism, possible values are "default",
"none",
- "writethrough" and "writeback". <span
class="since">Since 0.1.8</span>
+ "writethrough" and "writeback". Specific to KVM guests is the
optional <code>aio</code>
+ attribute which controls what storage api is used for io operations, its
possible
+ values are "threads" and "native". <span
class="since">Since
+ 0.1.8; <code>aio</code> attribute since 0.8.2</span>
</dd>
<dt><code>encryption</code></dt>
<dd>If present, specifies how the volume is encrypted. See
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 56b6705..acbbf9f 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -602,6 +602,9 @@
<optional>
<ref name="driverErrorPolicy"/>
</optional>
+ <optional>
+ <ref name="driverAIO"/>
+ </optional>
<empty/>
</element>
</define>
@@ -633,6 +636,14 @@
</choice>
</attribute>
</define>
+ <define name="driverAIO">
+ <attribute name="aio">
+ <choice>
+ <value>threads</value>
+ <value>native</value>
+ </choice>
+ </attribute>
+ </define>
<define name="controller">
<element name="controller">
<choice>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3e45f79..118585a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -132,6 +132,11 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy,
VIR_DOMAIN_DISK_ERROR_POLICY_LAST,
"ignore",
"enospace")
+VIR_ENUM_IMPL(virDomainDiskAIO, VIR_DOMAIN_DISK_AIO_LAST,
+ "default",
+ "native",
+ "threads")
+
VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST,
"ide",
"fdc",
@@ -1378,6 +1383,7 @@ virDomainDiskDefParseXML(xmlNodePtr node,
char *bus = NULL;
char *cachetag = NULL;
char *error_policy = NULL;
+ char *aiotag = NULL;
char *devaddr = NULL;
virStorageEncryptionPtr encryption = NULL;
char *serial = NULL;
@@ -1444,6 +1450,7 @@ virDomainDiskDefParseXML(xmlNodePtr node,
driverType = virXMLPropString(cur, "type");
cachetag = virXMLPropString(cur, "cache");
error_policy = virXMLPropString(cur, "error_policy");
+ aiotag = virXMLPropString(cur, "aio");
} else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) {
def->readonly = 1;
} else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) {
@@ -1567,6 +1574,13 @@ virDomainDiskDefParseXML(xmlNodePtr node,
goto error;
}
+ if (aiotag &&
+ (def->aiomode = virDomainDiskAIOTypeFromString(aiotag)) < 0) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown disk aio mode '%s'"), aiotag);
+ goto error;
+ }
+
if (devaddr) {
if (virDomainParseLegacyDeviceAddress(devaddr,
&def->info.addr.pci) < 0) {
@@ -1608,6 +1622,7 @@ cleanup:
VIR_FREE(driverName);
VIR_FREE(cachetag);
VIR_FREE(error_policy);
+ VIR_FREE(aiotag);
VIR_FREE(devaddr);
VIR_FREE(serial);
virStorageEncryptionFree(encryption);
@@ -4887,6 +4902,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
const char *bus = virDomainDiskBusTypeToString(def->bus);
const char *cachemode = virDomainDiskCacheTypeToString(def->cachemode);
const char *error_policy =
virDomainDiskErrorPolicyTypeToString(def->error_policy);
+ const char *aiomode = virDomainDiskAIOTypeToString(def->aiomode);
if (!type) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4908,6 +4924,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
_("unexpected disk cache mode %d"),
def->cachemode);
return -1;
}
+ if (!aiomode) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected disk aio mode %d"),
def->aiomode);
+ return -1;
+ }
virBufferVSprintf(buf,
" <disk type='%s'
device='%s'>\n",
@@ -4923,6 +4944,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
virBufferVSprintf(buf, " cache='%s'", cachemode);
if (def->error_policy)
virBufferVSprintf(buf, " error_policy='%s'",
error_policy);
+ if (def->aiomode)
+ virBufferVSprintf(buf, " aio='%s'", aiomode);
virBufferVSprintf(buf, "/>\n");
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index fadc8bd..1eec1d2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -161,6 +161,14 @@ enum virDomainDiskErrorPolicy {
VIR_DOMAIN_DISK_ERROR_POLICY_LAST
};
+enum virDomainDiskAIO {
+ VIR_DOMAIN_DISK_AIO_DEFAULT,
+ VIR_DOMAIN_DISK_AIO_NATIVE,
+ VIR_DOMAIN_DISK_AIO_THREADS,
+
+ VIR_DOMAIN_DISK_AIO_LAST
+};
+
/* Stores the virtual disk configuration */
typedef struct _virDomainDiskDef virDomainDiskDef;
typedef virDomainDiskDef *virDomainDiskDefPtr;
@@ -175,6 +183,7 @@ struct _virDomainDiskDef {
char *serial;
int cachemode;
int error_policy;
+ int aiomode;
unsigned int readonly : 1;
unsigned int shared : 1;
virDomainDeviceInfo info;
@@ -1069,6 +1078,7 @@ VIR_ENUM_DECL(virDomainDiskDevice)
VIR_ENUM_DECL(virDomainDiskBus)
VIR_ENUM_DECL(virDomainDiskCache)
VIR_ENUM_DECL(virDomainDiskErrorPolicy)
+VIR_ENUM_DECL(virDomainDiskAIO)
VIR_ENUM_DECL(virDomainController)
VIR_ENUM_DECL(virDomainFS)
VIR_ENUM_DECL(virDomainNet)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 5fa8c0a..87998c1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1,7 +1,7 @@
/*
* qemu_conf.c: QEMU configuration management
*
- * Copyright (C) 2006, 2007, 2008, 2009, 2010 Red Hat, Inc.
+ * Copyright (C) 2006-2010 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -85,6 +85,12 @@ VIR_ENUM_IMPL(qemuDiskCacheV2, VIR_DOMAIN_DISK_CACHE_LAST,
"writethrough",
"writeback");
+VIR_ENUM_DECL(qemuDiskAIO)
+VIR_ENUM_IMPL(qemuDiskAIO, VIR_DOMAIN_DISK_AIO_LAST,
+ "default",
+ "native",
+ "threads");
+
VIR_ENUM_DECL(qemuVideo)
VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
@@ -1150,6 +1156,8 @@ static unsigned long long qemudComputeCmdFlags(const char *help,
flags |= QEMUD_CMD_FLAG_DRIVE_CACHE_V2;
if (strstr(help, "format="))
flags |= QEMUD_CMD_FLAG_DRIVE_FORMAT;
+ if (strstr(help, "aio=threads|native"))
+ flags |= QEMUD_CMD_FLAG_DRIVE_AIO;
}
if (strstr(help, "-vga") && !strstr(help, "-std-vga"))
flags |= QEMUD_CMD_FLAG_VGA;
@@ -2518,6 +2526,11 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
}
}
+ if (disk->aiomode && (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_AIO)) {
+ virBufferVSprintf(&opt, ",aio=%s",
+ qemuDiskAIOTypeToString(disk->aiomode));
+ }
+
if (virBufferError(&opt)) {
virReportOOMError();
goto error;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a101e47..44043b4 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -88,6 +88,7 @@ enum qemud_cmd_flags {
QEMUD_CMD_FLAG_NO_HPET = (1LL << 33), /* -no-hpet flag is supported */
QEMUD_CMD_FLAG_NO_KVM_PIT = (1LL << 34), /* -no-kvm-pit-reinjection
supported */
QEMUD_CMD_FLAG_TDF = (1LL << 35), /* -tdf flag (user-mode pit
catchup) */
+ QEMUD_CMD_FLAG_DRIVE_AIO = (1LL << 36), /* -drive aio= supported */
};
/* Main driver state */
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 517a8fe..a00c743 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -244,7 +244,8 @@ mymain(int argc, char **argv)
QEMUD_CMD_FLAG_DEVICE |
QEMUD_CMD_FLAG_SMP_TOPOLOGY |
QEMUD_CMD_FLAG_RTC |
- QEMUD_CMD_FLAG_NO_HPET,
+ QEMUD_CMD_FLAG_NO_HPET |
+ QEMUD_CMD_FLAG_DRIVE_AIO,
12001, 0, 0);
DO_TEST("qemu-kvm-0.12.3",
QEMUD_CMD_FLAG_VNC_COLON |
@@ -274,7 +275,8 @@ mymain(int argc, char **argv)
QEMUD_CMD_FLAG_VNET_HOST |
QEMUD_CMD_FLAG_NO_HPET |
QEMUD_CMD_FLAG_NO_KVM_PIT |
- QEMUD_CMD_FLAG_TDF,
+ QEMUD_CMD_FLAG_TDF |
+ QEMUD_CMD_FLAG_DRIVE_AIO,
12003, 1, 0);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
--
1.7.0.1
5:43 p.m.
New subject: [libvirt] [PATCHv4 0/4] qemu aio support
In looking for other examples of XML additions, I noticed
commit 8bf6799 was the last of a four-patch series for adding
timer support. I'm reposting this into a split series,
but still leaving attribution to Matthias, since the text
of the result is still very close to his original submission.
[PATCHv4 1/4] qemu aio: add XML parsing
[PATCHv4 2/4] qemu aio: add aio attribute to schema
[PATCHv4 3/4] qemu aio: parse aio support from qemu -help
[PATCHv4 4/4] qemu aio: enable support
5:43 p.m.
New subject: [libvirt] [PATCHv4 1/4] qemu aio: add XML parsing
From: Matthias Dahl <mdvirt(a)designassembly.de>
Allows aio={threads|native} as an optional attribute to <driver>.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
Changes from v3:
export the private symbol
break a single patch into a 4-patch series, along the lines
of what was done in commit 8bf6799 when adding timer support
src/conf/domain_conf.c | 23 +++++++++++++++++++++++
src/conf/domain_conf.h | 10 ++++++++++
src/libvirt_private.syms | 3 +++
3 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3e45f79..118585a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -132,6 +132,11 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy,
VIR_DOMAIN_DISK_ERROR_POLICY_LAST,
"ignore",
"enospace")
+VIR_ENUM_IMPL(virDomainDiskAIO, VIR_DOMAIN_DISK_AIO_LAST,
+ "default",
+ "native",
+ "threads")
+
VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST,
"ide",
"fdc",
@@ -1378,6 +1383,7 @@ virDomainDiskDefParseXML(xmlNodePtr node,
char *bus = NULL;
char *cachetag = NULL;
char *error_policy = NULL;
+ char *aiotag = NULL;
char *devaddr = NULL;
virStorageEncryptionPtr encryption = NULL;
char *serial = NULL;
@@ -1444,6 +1450,7 @@ virDomainDiskDefParseXML(xmlNodePtr node,
driverType = virXMLPropString(cur, "type");
cachetag = virXMLPropString(cur, "cache");
error_policy = virXMLPropString(cur, "error_policy");
+ aiotag = virXMLPropString(cur, "aio");
} else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) {
def->readonly = 1;
} else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) {
@@ -1567,6 +1574,13 @@ virDomainDiskDefParseXML(xmlNodePtr node,
goto error;
}
+ if (aiotag &&
+ (def->aiomode = virDomainDiskAIOTypeFromString(aiotag)) < 0) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown disk aio mode '%s'"), aiotag);
+ goto error;
+ }
+
if (devaddr) {
if (virDomainParseLegacyDeviceAddress(devaddr,
&def->info.addr.pci) < 0) {
@@ -1608,6 +1622,7 @@ cleanup:
VIR_FREE(driverName);
VIR_FREE(cachetag);
VIR_FREE(error_policy);
+ VIR_FREE(aiotag);
VIR_FREE(devaddr);
VIR_FREE(serial);
virStorageEncryptionFree(encryption);
@@ -4887,6 +4902,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
const char *bus = virDomainDiskBusTypeToString(def->bus);
const char *cachemode = virDomainDiskCacheTypeToString(def->cachemode);
const char *error_policy =
virDomainDiskErrorPolicyTypeToString(def->error_policy);
+ const char *aiomode = virDomainDiskAIOTypeToString(def->aiomode);
if (!type) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4908,6 +4924,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
_("unexpected disk cache mode %d"),
def->cachemode);
return -1;
}
+ if (!aiomode) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected disk aio mode %d"),
def->aiomode);
+ return -1;
+ }
virBufferVSprintf(buf,
" <disk type='%s'
device='%s'>\n",
@@ -4923,6 +4944,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
virBufferVSprintf(buf, " cache='%s'", cachemode);
if (def->error_policy)
virBufferVSprintf(buf, " error_policy='%s'",
error_policy);
+ if (def->aiomode)
+ virBufferVSprintf(buf, " aio='%s'", aiomode);
virBufferVSprintf(buf, "/>\n");
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index fadc8bd..1eec1d2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -161,6 +161,14 @@ enum virDomainDiskErrorPolicy {
VIR_DOMAIN_DISK_ERROR_POLICY_LAST
};
+enum virDomainDiskAIO {
+ VIR_DOMAIN_DISK_AIO_DEFAULT,
+ VIR_DOMAIN_DISK_AIO_NATIVE,
+ VIR_DOMAIN_DISK_AIO_THREADS,
+
+ VIR_DOMAIN_DISK_AIO_LAST
+};
+
/* Stores the virtual disk configuration */
typedef struct _virDomainDiskDef virDomainDiskDef;
typedef virDomainDiskDef *virDomainDiskDefPtr;
@@ -175,6 +183,7 @@ struct _virDomainDiskDef {
char *serial;
int cachemode;
int error_policy;
+ int aiomode;
unsigned int readonly : 1;
unsigned int shared : 1;
virDomainDeviceInfo info;
@@ -1069,6 +1078,7 @@ VIR_ENUM_DECL(virDomainDiskDevice)
VIR_ENUM_DECL(virDomainDiskBus)
VIR_ENUM_DECL(virDomainDiskCache)
VIR_ENUM_DECL(virDomainDiskErrorPolicy)
+VIR_ENUM_DECL(virDomainDiskAIO)
VIR_ENUM_DECL(virDomainController)
VIR_ENUM_DECL(virDomainFS)
VIR_ENUM_DECL(virDomainNet)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 09f3da1..f1555d8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -198,6 +198,9 @@ virDomainDeviceInfoIterate;
virDomainClockOffsetTypeToString;
virDomainClockOffsetTypeFromString;
virDomainDiskErrorPolicyTypeToString;
+virDomainDiskErrorPolicyTypeFromString;
+virDomainDiskAIOToString;
+virDomainDiskAIOFromString;
virDomainTimerNameTypeToString;
virDomainTimerNameTypeFromString;
virDomainTimerTrackTypeToString;
--
1.7.0.1
5:43 p.m.
New subject: [libvirt] [PATCHv4 2/4] qemu aio: add aio attribute to schema
From: Matthias Dahl <mdvirt(a)designassembly.de>
aio is an optional attribute of <driver> for a given disk.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/formatdomain.html.in | 5 ++++-
docs/schemas/domain.rng | 11 +++++++++++
2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index be0446d..6d6641b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -586,7 +586,10 @@
attribute is the primary backend driver name, while the optional
<code>type</code>
attribute provides the sub-type. The optional <code>cache</code>
attribute
controls the cache mechanism, possible values are "default",
"none",
- "writethrough" and "writeback". <span
class="since">Since 0.1.8</span>
+ "writethrough" and "writeback". Specific to KVM guests is the
optional <code>aio</code>
+ attribute which controls what storage api is used for io operations, its
possible
+ values are "threads" and "native". <span
class="since">Since
+ 0.1.8; <code>aio</code> attribute since 0.8.2</span>
</dd>
<dt><code>encryption</code></dt>
<dd>If present, specifies how the volume is encrypted. See
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 56b6705..acbbf9f 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -602,6 +602,9 @@
<optional>
<ref name="driverErrorPolicy"/>
</optional>
+ <optional>
+ <ref name="driverAIO"/>
+ </optional>
<empty/>
</element>
</define>
@@ -633,6 +636,14 @@
</choice>
</attribute>
</define>
+ <define name="driverAIO">
+ <attribute name="aio">
+ <choice>
+ <value>threads</value>
+ <value>native</value>
+ </choice>
+ </attribute>
+ </define>
<define name="controller">
<element name="controller">
<choice>
--
1.7.0.1
5:43 p.m.
New subject: [libvirt] [PATCHv4 3/4] qemu aio: parse aio support from qemu -help
From: Matthias Dahl <mdvirt(a)designassembly.de>
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I'm still not sure why this code includes "default" in its
list of enum options, as the XML does not allow it. But since
that was copying from other elements, like qemuDiskCacheV2,
which also have a "default" listing not present in XML...
src/qemu/qemu_conf.c | 10 +++++++++-
src/qemu/qemu_conf.h | 1 +
tests/qemuhelptest.c | 6 ++++--
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 5fa8c0a..9f84988 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1,7 +1,7 @@
/*
* qemu_conf.c: QEMU configuration management
*
- * Copyright (C) 2006, 2007, 2008, 2009, 2010 Red Hat, Inc.
+ * Copyright (C) 2006-2010 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -85,6 +85,12 @@ VIR_ENUM_IMPL(qemuDiskCacheV2, VIR_DOMAIN_DISK_CACHE_LAST,
"writethrough",
"writeback");
+VIR_ENUM_DECL(qemuDiskAIO)
+VIR_ENUM_IMPL(qemuDiskAIO, VIR_DOMAIN_DISK_AIO_LAST,
+ "default",
+ "native",
+ "threads");
+
VIR_ENUM_DECL(qemuVideo)
VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
@@ -1150,6 +1156,8 @@ static unsigned long long qemudComputeCmdFlags(const char *help,
flags |= QEMUD_CMD_FLAG_DRIVE_CACHE_V2;
if (strstr(help, "format="))
flags |= QEMUD_CMD_FLAG_DRIVE_FORMAT;
+ if (strstr(help, "aio=threads|native"))
+ flags |= QEMUD_CMD_FLAG_DRIVE_AIO;
}
if (strstr(help, "-vga") && !strstr(help, "-std-vga"))
flags |= QEMUD_CMD_FLAG_VGA;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a101e47..44043b4 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -88,6 +88,7 @@ enum qemud_cmd_flags {
QEMUD_CMD_FLAG_NO_HPET = (1LL << 33), /* -no-hpet flag is supported */
QEMUD_CMD_FLAG_NO_KVM_PIT = (1LL << 34), /* -no-kvm-pit-reinjection
supported */
QEMUD_CMD_FLAG_TDF = (1LL << 35), /* -tdf flag (user-mode pit
catchup) */
+ QEMUD_CMD_FLAG_DRIVE_AIO = (1LL << 36), /* -drive aio= supported */
};
/* Main driver state */
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 517a8fe..a00c743 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -244,7 +244,8 @@ mymain(int argc, char **argv)
QEMUD_CMD_FLAG_DEVICE |
QEMUD_CMD_FLAG_SMP_TOPOLOGY |
QEMUD_CMD_FLAG_RTC |
- QEMUD_CMD_FLAG_NO_HPET,
+ QEMUD_CMD_FLAG_NO_HPET |
+ QEMUD_CMD_FLAG_DRIVE_AIO,
12001, 0, 0);
DO_TEST("qemu-kvm-0.12.3",
QEMUD_CMD_FLAG_VNC_COLON |
@@ -274,7 +275,8 @@ mymain(int argc, char **argv)
QEMUD_CMD_FLAG_VNET_HOST |
QEMUD_CMD_FLAG_NO_HPET |
QEMUD_CMD_FLAG_NO_KVM_PIT |
- QEMUD_CMD_FLAG_TDF,
+ QEMUD_CMD_FLAG_TDF |
+ QEMUD_CMD_FLAG_DRIVE_AIO,
12003, 1, 0);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
--
1.7.0.1
5:43 p.m.
New subject: [libvirt] [PATCHv4 4/4] qemu aio: enable support
From: Matthias Dahl <mdvirt(a)designassembly.de>
qemu allows the user to choose what io storage api should be used, either the
default (threads) or native (linux aio) which in the latter case can result in
better performance.
Thanks a lot to Eric Blake and Matthias Bolte for their comments.
Red Hat Bugzilla #591703
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
This is so simple, maybe it's worth squashing into 3/4?
src/qemu/qemu_conf.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 9f84988..87998c1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -2526,6 +2526,11 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
}
}
+ if (disk->aiomode && (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_AIO)) {
+ virBufferVSprintf(&opt, ",aio=%s",
+ qemuDiskAIOTypeToString(disk->aiomode));
+ }
+
if (virBufferError(&opt)) {
virReportOOMError();
goto error;
--
1.7.0.1
4:07 p.m.
New subject: [libvirt] [PATCH] Support for qemu aio drive option
On 05/12/2010 11:26 PM, Eric Blake wrote:
From: Matthias Dahl <mdvirt(a)designassembly.de>
qemu allows the user to choose what io storage api should be used, either the
default (threads) or native (linux aio) which in the latter case can result in
better performance.
Thanks a lot to Eric Blake and Matthias Bolte for their comments.
(sorry, I reviewed the one big patch instead of the split up patches. It seems
to be the same except for the addition of the symbols to libvirt_private.syms)
The implementation looks perfectly reasonable. I'm just concerned that the
concept of what we are doing is too qemu specific, though. Basically, I think
what we are trying to model here is the concept of an I/O backend implementation,
correct? Should we maybe change this to be "<iobackend
type='%s'/>", and then
have available enums like:
aiothreads
aionative
...
That way, for other hypervisors that do something different (like VirtualBox,
which just has AIO on/off), we can have additional enums to describe their
behavior. Even further, if a given hypervisor wanted to do something like
"Direct I/O" for the I/O backend (as an example), we could also use this
element
to specify that. What do you think?
--
Chris Lalancette
4:31 p.m.
New subject: [libvirt] [PATCH] Support for qemu aio drive option
[adding Matthias on cc]
On 05/17/2010 03:07 PM, Chris Lalancette wrote:
On 05/12/2010 11:26 PM, Eric Blake wrote:
> From: Matthias Dahl <mdvirt(a)designassembly.de>
>
> qemu allows the user to choose what io storage api should be used, either the
> default (threads) or native (linux aio) which in the latter case can result in
> better performance.
>
The implementation looks perfectly reasonable. I'm just concerned that the
concept of what we are doing is too qemu specific, though. Basically, I think
what we are trying to model here is the concept of an I/O backend implementation,
correct? Should we maybe change this to be "<iobackend
type='%s'/>", and then
have available enums like:
aiothreads
aionative
...
That way, for other hypervisors that do something different (like VirtualBox,
which just has AIO on/off), we can have additional enums to describe their
behavior. Even further, if a given hypervisor wanted to do something like
"Direct I/O" for the I/O backend (as an example), we could also use this
element
to specify that. What do you think?
Indeed, having a more-generic attribute "type", where only a subset of
known enum values are appropriate per a given hypervisor, makes sense to
me. After all, that's how we treated the <clock> addition in 0.8.0; not
all HV's support all the <timer name="foo"> timers. That is a bigger
rewrite, though, and while we could still complete it in time for 0.8.2,
it becomes much tougher to get it complete this week. If we really want
it now, then I can shepherd those changes through, but I need feedback
from other list members first on whether the proposed <iobackend> makes
more sense.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5:43 p.m.
New subject: [libvirt] [PATCH] Support for qemu aio drive option
On 05/17/2010 03:31 PM, Eric Blake wrote:
[adding Matthias on cc]
Reviving an old thread, as it is something I plan on working in the near
future.
On 05/17/2010 03:07 PM, Chris Lalancette wrote:
> On 05/12/2010 11:26 PM, Eric Blake wrote:
>> From: Matthias Dahl <mdvirt(a)designassembly.de>
>>
>> qemu allows the user to choose what io storage api should be used, either the
>> default (threads) or native (linux aio) which in the latter case can result in
>> better performance.
>>
>
> The implementation looks perfectly reasonable. I'm just concerned that the
> concept of what we are doing is too qemu specific, though. Basically, I think
> what we are trying to model here is the concept of an I/O backend implementation,
> correct? Should we maybe change this to be "<iobackend
type='%s'/>", and then
> have available enums like:
>
> aiothreads
> aionative
> ...
>
> That way, for other hypervisors that do something different (like VirtualBox,
> which just has AIO on/off), we can have additional enums to describe their
> behavior. Even further, if a given hypervisor wanted to do something like
> "Direct I/O" for the I/O backend (as an example), we could also use this
element
> to specify that. What do you think?
Indeed, having a more-generic attribute "type", where only a subset of
known enum values are appropriate per a given hypervisor, makes sense to
me.
I'm thinking that the following XML changes make the most sense for this
(note that the choice of aio mode is per-disk), where the change is the
addition of the new <iobackend> element:
<domain ...>
<devices>
<disk type='block' device='disk'>
<driver name='qemu' type='qcow2' cache='none'/>
<source dev='...'/>
<target dev='hda' bus='ide'/>
<iobackend type='aiothreads'/>
</disk>
</devices>
</domain>
If <iobackend> is missing, it is the same as type='default'; type can be
'default' (whatever qemu prefers, which may differ over qemu versions),
'aiothreads' (use ,aio=threads in qemu -drive argument), or 'aionative'
(use ,aio=native in qemu -drive argument); where we can add other types
later if needed.
Does this deserve a new <iobackend> element, or should it merely be a
new optional attribute of the existing <driver> element, as in:
<driver name='qemu' type='qcow2' cache='none'
iobackend='aiothreads'/>
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:12 a.m.
New subject: [libvirt] [PATCH] Support for qemu aio drive option
On 01/03/11 - 04:43:06PM, Eric Blake wrote:
On 05/17/2010 03:31 PM, Eric Blake wrote:
> [adding Matthias on cc]
Reviving an old thread, as it is something I plan on working in the near
future.
>
> On 05/17/2010 03:07 PM, Chris Lalancette wrote:
>> On 05/12/2010 11:26 PM, Eric Blake wrote:
>>> From: Matthias Dahl <mdvirt(a)designassembly.de>
>>>
>>> qemu allows the user to choose what io storage api should be used, either
the
>>> default (threads) or native (linux aio) which in the latter case can result
in
>>> better performance.
>>>
>>
>> The implementation looks perfectly reasonable. I'm just concerned that the
>> concept of what we are doing is too qemu specific, though. Basically, I think
>> what we are trying to model here is the concept of an I/O backend
implementation,
>> correct? Should we maybe change this to be "<iobackend
type='%s'/>", and then
>> have available enums like:
>>
>> aiothreads
>> aionative
>> ...
>>
>> That way, for other hypervisors that do something different (like VirtualBox,
>> which just has AIO on/off), we can have additional enums to describe their
>> behavior. Even further, if a given hypervisor wanted to do something like
>> "Direct I/O" for the I/O backend (as an example), we could also use
this element
>> to specify that. What do you think?
>
> Indeed, having a more-generic attribute "type", where only a subset of
> known enum values are appropriate per a given hypervisor, makes sense to
> me.
I'm thinking that the following XML changes make the most sense for this
(note that the choice of aio mode is per-disk), where the change is the
addition of the new <iobackend> element:
<domain ...>
<devices>
<disk type='block' device='disk'>
<driver name='qemu' type='qcow2' cache='none'/>
<source dev='...'/>
<target dev='hda' bus='ide'/>
<iobackend type='aiothreads'/>
</disk>
</devices>
</domain>
If <iobackend> is missing, it is the same as type='default'; type can be
'default' (whatever qemu prefers, which may differ over qemu versions),
'aiothreads' (use ,aio=threads in qemu -drive argument), or 'aionative'
(use ,aio=native in qemu -drive argument); where we can add other types
later if needed.
Does this deserve a new <iobackend> element, or should it merely be a
new optional attribute of the existing <driver> element, as in:
<driver name='qemu' type='qcow2' cache='none'
iobackend='aiothreads'/>
I slightly prefer making it a new attribute of <driver>. The only reason I
can see to make it a new <iobackend> element is if we think it will get
significantly more complex in the future (necessitating new XML elements). Do
we think that?
--
Chris Lalancette
10:33 a.m.
New subject: [libvirt] [PATCH] Support for qemu aio drive option
On Tue, Jan 04, 2011 at 08:12:21AM -0500, Chris Lalancette wrote:
On 01/03/11 - 04:43:06PM, Eric Blake wrote:
> On 05/17/2010 03:31 PM, Eric Blake wrote:
> > [adding Matthias on cc]
>
> Reviving an old thread, as it is something I plan on working in the near
> future.
>
> >
> > On 05/17/2010 03:07 PM, Chris Lalancette wrote:
> >> On 05/12/2010 11:26 PM, Eric Blake wrote:
> >>> From: Matthias Dahl <mdvirt(a)designassembly.de>
> >>>
> >>> qemu allows the user to choose what io storage api should be used,
either the
> >>> default (threads) or native (linux aio) which in the latter case can
result in
> >>> better performance.
> >>>
> >>
> >> The implementation looks perfectly reasonable. I'm just concerned that
the
> >> concept of what we are doing is too qemu specific, though. Basically, I
think
> >> what we are trying to model here is the concept of an I/O backend
implementation,
> >> correct? Should we maybe change this to be "<iobackend
type='%s'/>", and then
> >> have available enums like:
> >>
> >> aiothreads
> >> aionative
> >> ...
> >>
> >> That way, for other hypervisors that do something different (like
VirtualBox,
> >> which just has AIO on/off), we can have additional enums to describe their
> >> behavior. Even further, if a given hypervisor wanted to do something like
> >> "Direct I/O" for the I/O backend (as an example), we could also
use this element
> >> to specify that. What do you think?
> >
> > Indeed, having a more-generic attribute "type", where only a subset
of
> > known enum values are appropriate per a given hypervisor, makes sense to
> > me.
>
> I'm thinking that the following XML changes make the most sense for this
> (note that the choice of aio mode is per-disk), where the change is the
> addition of the new <iobackend> element:
>
> <domain ...>
> <devices>
> <disk type='block' device='disk'>
> <driver name='qemu' type='qcow2' cache='none'/>
> <source dev='...'/>
> <target dev='hda' bus='ide'/>
> <iobackend type='aiothreads'/>
> </disk>
> </devices>
> </domain>
>
> If <iobackend> is missing, it is the same as type='default'; type can
be
> 'default' (whatever qemu prefers, which may differ over qemu versions),
> 'aiothreads' (use ,aio=threads in qemu -drive argument), or
'aionative'
> (use ,aio=native in qemu -drive argument); where we can add other types
> later if needed.
>
> Does this deserve a new <iobackend> element, or should it merely be a
> new optional attribute of the existing <driver> element, as in:
>
> <driver name='qemu' type='qcow2' cache='none'
iobackend='aiothreads'/>
I slightly prefer making it a new attribute of <driver>. The only reason I
can see to make it a new <iobackend> element is if we think it will get
significantly more complex in the future (necessitating new XML elements). Do
we think that?
Yep, the whole '<driver>' element is really specifying an I/O backend,
so I don't think it makes sense to add another <iobackend> element,
nor is it particularly great as an attribute name. It should be an
attribute on <driver> though
Daniel
10:40 a.m.
New subject: [libvirt] [PATCH] Support for qemu aio drive option
On 01/04/2011 09:33 AM, Daniel P. Berrange wrote:
>> Does this deserve a new <iobackend> element, or should
it merely be a
>> new optional attribute of the existing <driver> element, as in:
>>
>> <driver name='qemu' type='qcow2' cache='none'
iobackend='aiothreads'/>
>
> I slightly prefer making it a new attribute of <driver>. The only reason I
> can see to make it a new <iobackend> element is if we think it will get
> significantly more complex in the future (necessitating new XML elements). Do
> we think that?
Yep, the whole '<driver>' element is really specifying an I/O backend,
so I don't think it makes sense to add another <iobackend> element,
nor is it particularly great as an attribute name. It should be an
attribute on <driver> though
Any better ideas for an attribute name? The complaint was that
Matthias' original proposal of naming the attribute 'aio' (as in <driver
aio='threads'/>) felt a bit too specific to qemu's implementation; yet
iobackend='aiothreads' is long. Maybe just io?
<driver io='aiothreads'/>
where io maps to qemu's ,aio=threads designation, but leaves us
flexibility of adding other attribute values unrelated to the name aio?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5101
days inactive
5359
days old
14 comments
4 participants
participants (4)
-
Chris Lalancette -
Daniel P. Berrange -
Eric Blake -
Matthias Dahl