12:54 a.m.
This introduces new attribute writeout with only supported
value as immediate. This will be an optional
attribute with no defaults. This helps specify whether
to skip the host page cache.
When writeout is specified, meaning when writeout=immediate
a writeback is explicitly initiated for the dirty pages in
the host page cache as part of the guest file write operation.
Usage:
<filesystem type='mount' accessmode='passthrough'
writeout='immediate'>
<source dir='/export/to/guest'/>
<target dir='mount_tag'/>
</filesystem>
Currently this only works with type='mount' for the QEMU/KVM driver.
Signed-off-by: Deepak C Shetty <deepakcs(a)linux.vnet.ibm.com>
---
docs/formatdomain.html.in | 8 +++++++-
docs/schemas/domaincommon.rng | 5 +++++
src/conf/domain_conf.c | 29 +++++++++++++++++++++++++++--
src/conf/domain_conf.h | 10 ++++++++++
src/qemu/qemu_command.c | 9 +++++++++
5 files changed, 58 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c57b7b3..803db8e 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1303,7 +1303,7 @@
<source name='my-vm-template'/>
<target dir='/'/>
</filesystem>
- <filesystem type='mount' accessmode='passthrough'>
+ <filesystem type='mount' accessmode='passthrough'
writeout='immediate'>
<driver type='path'/>
<source dir='/export/to/guest'/>
<target dir='/import/from/host'/>
@@ -1379,6 +1379,12 @@
</dd>
</dl>
+ The filesystem block has an optional attribute
<code>writeout='immediate'</code>
+ to help specify whether to skip the host page cache. When writeout is specified,
meaning when
+ writeout=immediate a writeback is explicitly initiated for the dirty pages in
+ the host page cache as part of the guest file write operation. When this attribute
is not
+ specified, there are no defaults, meaning explicit writeback won't be
initiated.
+
</dd>
<dt><code>source</code></dt>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 27ec1da..2298994 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1106,6 +1106,11 @@
<value>squash</value>
</choice>
</attribute>
+ <attribute name="writeout">
+ <choice>
+ <value>immediate</value>
+ </choice>
+ </attribute>
</optional>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2c91f82..80b8eab 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -256,6 +256,9 @@ VIR_ENUM_IMPL(virDomainFSAccessMode, VIR_DOMAIN_FS_ACCESSMODE_LAST,
"mapped",
"squash")
+VIR_ENUM_IMPL(virDomainFSWriteout, VIR_DOMAIN_FS_WRITEOUT_LAST,
+ "default",
+ "immediate")
VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
"user",
@@ -3258,6 +3261,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
char *source = NULL;
char *target = NULL;
char *accessmode = NULL;
+ char *writeout = NULL;
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
@@ -3286,6 +3290,17 @@ virDomainFSDefParseXML(xmlNodePtr node,
def->accessmode = VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH;
}
+ writeout = virXMLPropString(node, "writeout");
+ if (writeout) {
+ if ((def->writeout = virDomainFSWriteoutTypeFromString(writeout)) < 0) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown filesystem writeout '%s'"),
writeout);
+ goto error;
+ }
+ } else {
+ def->writeout = VIR_DOMAIN_FS_WRITEOUT_DEFAULT;
+ }
+
cur = node->children;
while (cur != NULL) {
if (cur->type == XML_ELEMENT_NODE) {
@@ -3348,6 +3363,7 @@ cleanup:
VIR_FREE(target);
VIR_FREE(source);
VIR_FREE(accessmode);
+ VIR_FREE(writeout);
return def;
@@ -10006,6 +10022,7 @@ virDomainFSDefFormat(virBufferPtr buf,
const char *type = virDomainFSTypeToString(def->type);
const char *accessmode = virDomainFSAccessModeTypeToString(def->accessmode);
const char *fsdriver = virDomainFSDriverTypeTypeToString(def->fsdriver);
+ const char *writeout = virDomainFSWriteoutTypeToString(def->writeout);
if (!type) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
@@ -10022,12 +10039,20 @@ virDomainFSDefFormat(virBufferPtr buf,
if (def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH ||
def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) {
virBufferAsprintf(buf,
- " <filesystem type='%s'
accessmode='%s'>\n",
+ " <filesystem type='%s'
accessmode='%s'",
type, accessmode);
} else {
virBufferAsprintf(buf,
- " <filesystem type='%s'>\n", type);
+ " <filesystem type='%s'", type);
}
+
+ /* Dont generate anything if writeout is set to default */
+ if (def->writeout) {
+ virBufferAsprintf(buf, " writeout='%s'", writeout);
+ }
+
+ /* close the filesystem element */
+ virBufferAddLit(buf,">\n");
if (def->fsdriver) {
virBufferAsprintf(buf, " <driver type='%s'/>\n",
fsdriver);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3229a6f..bf4d79b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -450,12 +450,21 @@ enum virDomainFSAccessMode {
VIR_DOMAIN_FS_ACCESSMODE_LAST
};
+/* Filesystem Writeout */
+enum virDomainFSWriteout {
+ VIR_DOMAIN_FS_WRITEOUT_DEFAULT = 0,
+ VIR_DOMAIN_FS_WRITEOUT_IMMEDIATE,
+
+ VIR_DOMAIN_FS_WRITEOUT_LAST
+};
+
typedef struct _virDomainFSDef virDomainFSDef;
typedef virDomainFSDef *virDomainFSDefPtr;
struct _virDomainFSDef {
int type;
int fsdriver;
int accessmode;
+ int writeout;
char *src;
char *dst;
unsigned int readonly : 1;
@@ -1973,6 +1982,7 @@ VIR_ENUM_DECL(virDomainControllerModelUSB)
VIR_ENUM_DECL(virDomainFS)
VIR_ENUM_DECL(virDomainFSDriverType)
VIR_ENUM_DECL(virDomainFSAccessMode)
+VIR_ENUM_DECL(virDomainFSWriteout)
VIR_ENUM_DECL(virDomainNet)
VIR_ENUM_DECL(virDomainNetBackend)
VIR_ENUM_DECL(virDomainNetVirtioTxMode)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1f70eb1..ccfd092 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -108,6 +108,10 @@ VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
"local",
"handle");
+VIR_ENUM_DECL(qemuDomainFSWriteout)
+VIR_ENUM_IMPL(qemuDomainFSWriteout, VIR_DOMAIN_FS_WRITEOUT_LAST,
+ "default",
+ "immediate");
static void
uname_normalize (struct utsname *ut)
@@ -1990,6 +1994,7 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
{
virBuffer opt = VIR_BUFFER_INITIALIZER;
const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver);
+ const char *writeout = qemuDomainFSWriteoutTypeToString(fs->writeout);
if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -2014,6 +2019,10 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
virBufferAddLit(&opt, ",security_model=none");
}
}
+
+ if (fs->writeout != VIR_DOMAIN_FS_WRITEOUT_DEFAULT) {
+ virBufferAsprintf(&opt, ",writeout=%s", writeout);
+ }
virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX,
fs->info.alias);
virBufferAsprintf(&opt, ",path=%s", fs->src);
1:25 a.m.
On 2011年12月22日 14:54, Deepak C Shetty wrote:
This introduces new attribute writeout with only supported
value as immediate. This will be an optional
attribute with no defaults. This helps specify whether
to skip the host page cache.
When writeout is specified, meaning when writeout=immediate
a writeback is explicitly initiated for the dirty pages in
the host page cache as part of the guest file write operation.
Usage:
<filesystem type='mount' accessmode='passthrough'
writeout='immediate'>
<source dir='/export/to/guest'/>
<target dir='mount_tag'/>
</filesystem>
Currently this only works with type='mount' for the QEMU/KVM driver.
Signed-off-by: Deepak C Shetty<deepakcs(a)linux.vnet.ibm.com>
---
docs/formatdomain.html.in | 8 +++++++-
docs/schemas/domaincommon.rng | 5 +++++
src/conf/domain_conf.c | 29 +++++++++++++++++++++++++++--
src/conf/domain_conf.h | 10 ++++++++++
src/qemu/qemu_command.c | 9 +++++++++
5 files changed, 58 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c57b7b3..803db8e 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1303,7 +1303,7 @@
<source name='my-vm-template'/>
<target dir='/'/>
</filesystem>
-<filesystem type='mount' accessmode='passthrough'>
+<filesystem type='mount' accessmode='passthrough'
writeout='immediate'>
<driver type='path'/>
<source dir='/export/to/guest'/>
<target dir='/import/from/host'/>
@@ -1379,6 +1379,12 @@
</dd>
</dl>
+ The filesystem block has an optional
attribute<code>writeout='immediate'</code>
<code>write</code>, and it's better to clarify it only supports
"immediate" currently.
+ to help specify whether to skip the host page cache. When
writeout is specified, meaning when
+ writeout=immediate a writeback is explicitly initiated for the dirty pages in
+ the host page cache as part of the guest file write operation. When this attribute
is not
+ specified, there are no defaults, meaning explicit writeback won't be
initiated.
+
</dd>
<dt><code>source</code></dt>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 27ec1da..2298994 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1106,6 +1106,11 @@
<value>squash</value>
</choice>
</attribute>
+<attribute name="writeout">
+<choice>
+<value>immediate</value>
+</choice>
+</attribute>
</optional>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2c91f82..80b8eab 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -256,6 +256,9 @@ VIR_ENUM_IMPL(virDomainFSAccessMode, VIR_DOMAIN_FS_ACCESSMODE_LAST,
"mapped",
"squash")
+VIR_ENUM_IMPL(virDomainFSWriteout, VIR_DOMAIN_FS_WRITEOUT_LAST,
+ "default",
+ "immediate")
VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
"user",
@@ -3258,6 +3261,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
char *source = NULL;
char *target = NULL;
char *accessmode = NULL;
+ char *writeout = NULL;
if (VIR_ALLOC(def)< 0) {
virReportOOMError();
@@ -3286,6 +3290,17 @@ virDomainFSDefParseXML(xmlNodePtr node,
def->accessmode = VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH;
}
+ writeout = virXMLPropString(node, "writeout");
+ if (writeout) {
+ if ((def->writeout = virDomainFSWriteoutTypeFromString(writeout))< 0) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
IMHO VIR_ERR_CONFIG_UNSUPPORTED is more proper here.
+ _("unknown filesystem writeout
'%s'"), writeout);
+ goto error;
+ }
+ } else {
+ def->writeout = VIR_DOMAIN_FS_WRITEOUT_DEFAULT;
+ }
+
cur = node->children;
while (cur != NULL) {
if (cur->type == XML_ELEMENT_NODE) {
@@ -3348,6 +3363,7 @@ cleanup:
VIR_FREE(target);
VIR_FREE(source);
VIR_FREE(accessmode);
+ VIR_FREE(writeout);
return def;
@@ -10006,6 +10022,7 @@ virDomainFSDefFormat(virBufferPtr buf,
const char *type = virDomainFSTypeToString(def->type);
const char *accessmode = virDomainFSAccessModeTypeToString(def->accessmode);
const char *fsdriver = virDomainFSDriverTypeTypeToString(def->fsdriver);
+ const char *writeout = virDomainFSWriteoutTypeToString(def->writeout);
if (!type) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
@@ -10022,12 +10039,20 @@ virDomainFSDefFormat(virBufferPtr buf,
if (def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH ||
def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) {
virBufferAsprintf(buf,
- "<filesystem type='%s'
accessmode='%s'>\n",
+ "<filesystem type='%s'
accessmode='%s'",
type, accessmode);
} else {
virBufferAsprintf(buf,
- "<filesystem type='%s'>\n", type);
+ "<filesystem type='%s'", type);
}
+
+ /* Dont generate anything if writeout is set to default */
Dont -> Don't
+ if (def->writeout) {
+ virBufferAsprintf(buf, " writeout='%s'", writeout);
+ }
+
+ /* close the filesystem element */
+ virBufferAddLit(buf,">\n");
if (def->fsdriver) {
virBufferAsprintf(buf, "<driver type='%s'/>\n",
fsdriver);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3229a6f..bf4d79b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -450,12 +450,21 @@ enum virDomainFSAccessMode {
VIR_DOMAIN_FS_ACCESSMODE_LAST
};
+/* Filesystem Writeout */
+enum virDomainFSWriteout {
+ VIR_DOMAIN_FS_WRITEOUT_DEFAULT = 0,
+ VIR_DOMAIN_FS_WRITEOUT_IMMEDIATE,
+
+ VIR_DOMAIN_FS_WRITEOUT_LAST
+};
+
typedef struct _virDomainFSDef virDomainFSDef;
typedef virDomainFSDef *virDomainFSDefPtr;
struct _virDomainFSDef {
int type;
int fsdriver;
int accessmode;
+ int writeout;
char *src;
char *dst;
unsigned int readonly : 1;
@@ -1973,6 +1982,7 @@ VIR_ENUM_DECL(virDomainControllerModelUSB)
VIR_ENUM_DECL(virDomainFS)
VIR_ENUM_DECL(virDomainFSDriverType)
VIR_ENUM_DECL(virDomainFSAccessMode)
+VIR_ENUM_DECL(virDomainFSWriteout)
VIR_ENUM_DECL(virDomainNet)
VIR_ENUM_DECL(virDomainNetBackend)
VIR_ENUM_DECL(virDomainNetVirtioTxMode)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1f70eb1..ccfd092 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -108,6 +108,10 @@ VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
"local",
"handle");
+VIR_ENUM_DECL(qemuDomainFSWriteout)
+VIR_ENUM_IMPL(qemuDomainFSWriteout, VIR_DOMAIN_FS_WRITEOUT_LAST,
+ "default",
+ "immediate");
static void
uname_normalize (struct utsname *ut)
@@ -1990,6 +1994,7 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
{
virBuffer opt = VIR_BUFFER_INITIALIZER;
const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver);
+ const char *writeout = qemuDomainFSWriteoutTypeToString(fs->writeout);
if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -2014,6 +2019,10 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
virBufferAddLit(&opt, ",security_model=none");
}
}
+
+ if (fs->writeout != VIR_DOMAIN_FS_WRITEOUT_DEFAULT) {
Since the "writeout" option is introduced far later than the the
"-fsdev". i.e. "writeout" can be not supported by qemu even if
"-fsdev" is supported. So we might want to detect the capability
first and report error earlier than starting the qemu process.
Like QEMU_CAPS_FSDEV_READONLY in commit a1a83c5.
Others looks good, (I just planned to add this support, you step
one more than me. :-)
Regards,
Osier
2:01 a.m.
On 12/22/2011 12:55 PM, Osier Yang wrote:
On 2011年12月22日 14:54, Deepak C Shetty wrote:
> This introduces new attribute writeout with only supported
> value as immediate. This will be an optional
> attribute with no defaults. This helps specify whether
> to skip the host page cache.
>
> When writeout is specified, meaning when writeout=immediate
> a writeback is explicitly initiated for the dirty pages in
> the host page cache as part of the guest file write operation.
>
> Usage:
> <filesystem type='mount' accessmode='passthrough'
writeout='immediate'>
> <source dir='/export/to/guest'/>
> <target dir='mount_tag'/>
> </filesystem>
>
> Currently this only works with type='mount' for the QEMU/KVM driver.
>
> Signed-off-by: Deepak C Shetty<deepakcs(a)linux.vnet.ibm.com>
> ---
>
> docs/formatdomain.html.in | 8 +++++++-
> docs/schemas/domaincommon.rng | 5 +++++
> src/conf/domain_conf.c | 29 +++++++++++++++++++++++++++--
> src/conf/domain_conf.h | 10 ++++++++++
> src/qemu/qemu_command.c | 9 +++++++++
> 5 files changed, 58 insertions(+), 3 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index c57b7b3..803db8e 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1303,7 +1303,7 @@
> <source name='my-vm-template'/>
> <target dir='/'/>
> </filesystem>
> -<filesystem type='mount' accessmode='passthrough'>
> +<filesystem type='mount' accessmode='passthrough'
> writeout='immediate'>
> <driver type='path'/>
> <source dir='/export/to/guest'/>
> <target dir='/import/from/host'/>
> @@ -1379,6 +1379,12 @@
> </dd>
> </dl>
>
> + The filesystem block has an optional
> attribute<code>writeout='immediate'</code>
<code>write</code>, and it's better to clarify it only supports
"immediate" currently.
So you are proposing to change the attribute name to write instead of
writeout ?
I wanted to keep it as writeout, since it matches with the qemu option
also, will
be easier to locate/debug, no ?
Will make the changes in v2 and submit.
> + to help specify whether to skip the host page cache. When
> writeout is specified, meaning when
> + writeout=immediate a writeback is explicitly initiated for the
> dirty pages in
> + the host page cache as part of the guest file write operation.
> When this attribute is not
> + specified, there are no defaults, meaning explicit writeback
> won't be initiated.
> +
> </dd>
>
> <dt><code>source</code></dt>
> diff --git a/docs/schemas/domaincommon.rng
> b/docs/schemas/domaincommon.rng
> index 27ec1da..2298994 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1106,6 +1106,11 @@
> <value>squash</value>
> </choice>
> </attribute>
> +<attribute name="writeout">
> +<choice>
> +<value>immediate</value>
> +</choice>
> +</attribute>
> </optional>
> </element>
> </define>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 2c91f82..80b8eab 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -256,6 +256,9 @@ VIR_ENUM_IMPL(virDomainFSAccessMode,
> VIR_DOMAIN_FS_ACCESSMODE_LAST,
> "mapped",
> "squash")
>
> +VIR_ENUM_IMPL(virDomainFSWriteout, VIR_DOMAIN_FS_WRITEOUT_LAST,
> + "default",
> + "immediate")
>
> VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
> "user",
> @@ -3258,6 +3261,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
> char *source = NULL;
> char *target = NULL;
> char *accessmode = NULL;
> + char *writeout = NULL;
>
> if (VIR_ALLOC(def)< 0) {
> virReportOOMError();
> @@ -3286,6 +3290,17 @@ virDomainFSDefParseXML(xmlNodePtr node,
> def->accessmode = VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH;
> }
>
> + writeout = virXMLPropString(node, "writeout");
> + if (writeout) {
> + if ((def->writeout =
> virDomainFSWriteoutTypeFromString(writeout))< 0) {
> + virDomainReportError(VIR_ERR_INTERNAL_ERROR,
IMHO VIR_ERR_CONFIG_UNSUPPORTED is more proper here.
IIUC, i should return VIR_ERR_CONFIG_UNSUPPORTED if the xml has 'writeout'
but QEMU_CAPS does not support writeout, correct ?
Will make the changes in v2 and submit.
> + _("unknown filesystem writeout
> '%s'"), writeout);
> + goto error;
> + }
> + } else {
> + def->writeout = VIR_DOMAIN_FS_WRITEOUT_DEFAULT;
> + }
> +
> cur = node->children;
> while (cur != NULL) {
> if (cur->type == XML_ELEMENT_NODE) {
> @@ -3348,6 +3363,7 @@ cleanup:
> VIR_FREE(target);
> VIR_FREE(source);
> VIR_FREE(accessmode);
> + VIR_FREE(writeout);
>
> return def;
>
> @@ -10006,6 +10022,7 @@ virDomainFSDefFormat(virBufferPtr buf,
> const char *type = virDomainFSTypeToString(def->type);
> const char *accessmode =
> virDomainFSAccessModeTypeToString(def->accessmode);
> const char *fsdriver =
> virDomainFSDriverTypeTypeToString(def->fsdriver);
> + const char *writeout =
> virDomainFSWriteoutTypeToString(def->writeout);
>
> if (!type) {
> virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -10022,12 +10039,20 @@ virDomainFSDefFormat(virBufferPtr buf,
> if (def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH ||
> def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) {
> virBufferAsprintf(buf,
> - "<filesystem type='%s'
accessmode='%s'>\n",
> + "<filesystem type='%s'
accessmode='%s'",
> type, accessmode);
> } else {
> virBufferAsprintf(buf,
> - "<filesystem type='%s'>\n",
type);
> + "<filesystem type='%s'", type);
> }
> +
> + /* Dont generate anything if writeout is set to default */
Dont -> Don't
thanks :)
Will make the changes in v2 and submit.
> + if (def->writeout) {
> + virBufferAsprintf(buf, " writeout='%s'", writeout);
> + }
> +
> + /* close the filesystem element */
> + virBufferAddLit(buf,">\n");
>
> if (def->fsdriver) {
> virBufferAsprintf(buf, "<driver type='%s'/>\n",
fsdriver);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 3229a6f..bf4d79b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -450,12 +450,21 @@ enum virDomainFSAccessMode {
> VIR_DOMAIN_FS_ACCESSMODE_LAST
> };
>
> +/* Filesystem Writeout */
> +enum virDomainFSWriteout {
> + VIR_DOMAIN_FS_WRITEOUT_DEFAULT = 0,
> + VIR_DOMAIN_FS_WRITEOUT_IMMEDIATE,
> +
> + VIR_DOMAIN_FS_WRITEOUT_LAST
> +};
> +
> typedef struct _virDomainFSDef virDomainFSDef;
> typedef virDomainFSDef *virDomainFSDefPtr;
> struct _virDomainFSDef {
> int type;
> int fsdriver;
> int accessmode;
> + int writeout;
> char *src;
> char *dst;
> unsigned int readonly : 1;
> @@ -1973,6 +1982,7 @@ VIR_ENUM_DECL(virDomainControllerModelUSB)
> VIR_ENUM_DECL(virDomainFS)
> VIR_ENUM_DECL(virDomainFSDriverType)
> VIR_ENUM_DECL(virDomainFSAccessMode)
> +VIR_ENUM_DECL(virDomainFSWriteout)
> VIR_ENUM_DECL(virDomainNet)
> VIR_ENUM_DECL(virDomainNetBackend)
> VIR_ENUM_DECL(virDomainNetVirtioTxMode)
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1f70eb1..ccfd092 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -108,6 +108,10 @@ VIR_ENUM_IMPL(qemuDomainFSDriver,
> VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
> "local",
> "handle");
>
> +VIR_ENUM_DECL(qemuDomainFSWriteout)
> +VIR_ENUM_IMPL(qemuDomainFSWriteout, VIR_DOMAIN_FS_WRITEOUT_LAST,
> + "default",
> + "immediate");
>
> static void
> uname_normalize (struct utsname *ut)
> @@ -1990,6 +1994,7 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
> {
> virBuffer opt = VIR_BUFFER_INITIALIZER;
> const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver);
> + const char *writeout =
> qemuDomainFSWriteoutTypeToString(fs->writeout);
>
> if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) {
> qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -2014,6 +2019,10 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
> virBufferAddLit(&opt, ",security_model=none");
> }
> }
> +
> + if (fs->writeout != VIR_DOMAIN_FS_WRITEOUT_DEFAULT) {
Since the "writeout" option is introduced far later than the the
"-fsdev". i.e. "writeout" can be not supported by qemu even if
"-fsdev" is supported. So we might want to detect the capability
first and report error earlier than starting the qemu process.
Like QEMU_CAPS_FSDEV_READONLY in commit a1a83c5.
So basically this would help in maintain correct error reporting when
xml has
writeout but user has older qemu which does not support writeout yet,
correct ?
Others looks good, (I just planned to add this support, you step
one more than me. :-)
'guess what, we both probably are stepping on each other, I had earlier
planned
to do readonly fsdev support, saw ur post and moved to writeout support.
Regards,
Osier
2:36 a.m.
On 2011年12月22日 16:01, Deepak C Shetty wrote:
On 12/22/2011 12:55 PM, Osier Yang wrote:
> On 2011年12月22日 14:54, Deepak C Shetty wrote:
>> This introduces new attribute writeout with only supported
>> value as immediate. This will be an optional
>> attribute with no defaults. This helps specify whether
>> to skip the host page cache.
>>
>> When writeout is specified, meaning when writeout=immediate
>> a writeback is explicitly initiated for the dirty pages in
>> the host page cache as part of the guest file write operation.
>>
>> Usage:
>> <filesystem type='mount' accessmode='passthrough'
writeout='immediate'>
>> <source dir='/export/to/guest'/>
>> <target dir='mount_tag'/>
>> </filesystem>
>>
>> Currently this only works with type='mount' for the QEMU/KVM driver.
>>
>> Signed-off-by: Deepak C Shetty<deepakcs(a)linux.vnet.ibm.com>
>> ---
>>
>> docs/formatdomain.html.in | 8 +++++++-
>> docs/schemas/domaincommon.rng | 5 +++++
>> src/conf/domain_conf.c | 29 +++++++++++++++++++++++++++--
>> src/conf/domain_conf.h | 10 ++++++++++
>> src/qemu/qemu_command.c | 9 +++++++++
>> 5 files changed, 58 insertions(+), 3 deletions(-)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index c57b7b3..803db8e 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -1303,7 +1303,7 @@
>> <source name='my-vm-template'/>
>> <target dir='/'/>
>> </filesystem>
>> -<filesystem type='mount'
accessmode='passthrough'>
>> +<filesystem type='mount' accessmode='passthrough'
>> writeout='immediate'>
>> <driver type='path'/>
>> <source dir='/export/to/guest'/>
>> <target dir='/import/from/host'/>
>> @@ -1379,6 +1379,12 @@
>> </dd>
>> </dl>
>>
>> + The filesystem block has an optional
>> attribute<code>writeout='immediate'</code>
>
> <code>write</code>, and it's better to clarify it only supports
> "immediate" currently.
So you are proposing to change the attribute name to write instead of
writeout ?
Sorry, I meant <code>writeout</code>, it shouldn't mix attribute
and its value together when saying "an optional attribute".
I wanted to keep it as writeout, since it matches with the qemu
option
also, will
be easier to locate/debug, no ?
Will make the changes in v2 and submit.
>
>> + to help specify whether to skip the host page cache. When writeout
>> is specified, meaning when
>> + writeout=immediate a writeback is explicitly initiated for the
>> dirty pages in
>> + the host page cache as part of the guest file write operation. When
>> this attribute is not
>> + specified, there are no defaults, meaning explicit writeback won't
>> be initiated.
>> +
>> </dd>
>>
>> <dt><code>source</code></dt>
>> diff --git a/docs/schemas/domaincommon.rng
>> b/docs/schemas/domaincommon.rng
>> index 27ec1da..2298994 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -1106,6 +1106,11 @@
>> <value>squash</value>
>> </choice>
>> </attribute>
>> +<attribute name="writeout">
>> +<choice>
>> +<value>immediate</value>
>> +</choice>
>> +</attribute>
>> </optional>
>> </element>
>> </define>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 2c91f82..80b8eab 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -256,6 +256,9 @@ VIR_ENUM_IMPL(virDomainFSAccessMode,
>> VIR_DOMAIN_FS_ACCESSMODE_LAST,
>> "mapped",
>> "squash")
>>
>> +VIR_ENUM_IMPL(virDomainFSWriteout, VIR_DOMAIN_FS_WRITEOUT_LAST,
>> + "default",
>> + "immediate")
>>
>> VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
>> "user",
>> @@ -3258,6 +3261,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
>> char *source = NULL;
>> char *target = NULL;
>> char *accessmode = NULL;
>> + char *writeout = NULL;
>>
>> if (VIR_ALLOC(def)< 0) {
>> virReportOOMError();
>> @@ -3286,6 +3290,17 @@ virDomainFSDefParseXML(xmlNodePtr node,
>> def->accessmode = VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH;
>> }
>>
>> + writeout = virXMLPropString(node, "writeout");
>> + if (writeout) {
>> + if ((def->writeout = virDomainFSWriteoutTypeFromString(writeout))<
>> 0) {
>> + virDomainReportError(VIR_ERR_INTERNAL_ERROR,
>
> IMHO VIR_ERR_CONFIG_UNSUPPORTED is more proper here.
IIUC, i should return VIR_ERR_CONFIG_UNSUPPORTED if the xml has 'writeout'
but QEMU_CAPS does not support writeout, correct ?
Yes
Will make the changes in v2 and submit.
>
>> + _("unknown filesystem writeout '%s'"), writeout);
>> + goto error;
>> + }
>> + } else {
>> + def->writeout = VIR_DOMAIN_FS_WRITEOUT_DEFAULT;
>> + }
>> +
>> cur = node->children;
>> while (cur != NULL) {
>> if (cur->type == XML_ELEMENT_NODE) {
>> @@ -3348,6 +3363,7 @@ cleanup:
>> VIR_FREE(target);
>> VIR_FREE(source);
>> VIR_FREE(accessmode);
>> + VIR_FREE(writeout);
>>
>> return def;
>>
>> @@ -10006,6 +10022,7 @@ virDomainFSDefFormat(virBufferPtr buf,
>> const char *type = virDomainFSTypeToString(def->type);
>> const char *accessmode =
>> virDomainFSAccessModeTypeToString(def->accessmode);
>> const char *fsdriver = virDomainFSDriverTypeTypeToString(def->fsdriver);
>> + const char *writeout = virDomainFSWriteoutTypeToString(def->writeout);
>>
>> if (!type) {
>> virDomainReportError(VIR_ERR_INTERNAL_ERROR,
>> @@ -10022,12 +10039,20 @@ virDomainFSDefFormat(virBufferPtr buf,
>> if (def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH ||
>> def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) {
>> virBufferAsprintf(buf,
>> - "<filesystem type='%s' accessmode='%s'>\n",
>> + "<filesystem type='%s' accessmode='%s'",
>> type, accessmode);
>> } else {
>> virBufferAsprintf(buf,
>> - "<filesystem type='%s'>\n", type);
>> + "<filesystem type='%s'", type);
>> }
>> +
>> + /* Dont generate anything if writeout is set to default */
>
> Dont -> Don't
thanks :)
Will make the changes in v2 and submit.
>
>> + if (def->writeout) {
>> + virBufferAsprintf(buf, " writeout='%s'", writeout);
>> + }
>> +
>> + /* close the filesystem element */
>> + virBufferAddLit(buf,">\n");
>>
>> if (def->fsdriver) {
>> virBufferAsprintf(buf, "<driver type='%s'/>\n",
fsdriver);
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 3229a6f..bf4d79b 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -450,12 +450,21 @@ enum virDomainFSAccessMode {
>> VIR_DOMAIN_FS_ACCESSMODE_LAST
>> };
>>
>> +/* Filesystem Writeout */
>> +enum virDomainFSWriteout {
>> + VIR_DOMAIN_FS_WRITEOUT_DEFAULT = 0,
>> + VIR_DOMAIN_FS_WRITEOUT_IMMEDIATE,
>> +
>> + VIR_DOMAIN_FS_WRITEOUT_LAST
>> +};
>> +
>> typedef struct _virDomainFSDef virDomainFSDef;
>> typedef virDomainFSDef *virDomainFSDefPtr;
>> struct _virDomainFSDef {
>> int type;
>> int fsdriver;
>> int accessmode;
>> + int writeout;
>> char *src;
>> char *dst;
>> unsigned int readonly : 1;
>> @@ -1973,6 +1982,7 @@ VIR_ENUM_DECL(virDomainControllerModelUSB)
>> VIR_ENUM_DECL(virDomainFS)
>> VIR_ENUM_DECL(virDomainFSDriverType)
>> VIR_ENUM_DECL(virDomainFSAccessMode)
>> +VIR_ENUM_DECL(virDomainFSWriteout)
>> VIR_ENUM_DECL(virDomainNet)
>> VIR_ENUM_DECL(virDomainNetBackend)
>> VIR_ENUM_DECL(virDomainNetVirtioTxMode)
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 1f70eb1..ccfd092 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -108,6 +108,10 @@ VIR_ENUM_IMPL(qemuDomainFSDriver,
>> VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
>> "local",
>> "handle");
>>
>> +VIR_ENUM_DECL(qemuDomainFSWriteout)
>> +VIR_ENUM_IMPL(qemuDomainFSWriteout, VIR_DOMAIN_FS_WRITEOUT_LAST,
>> + "default",
>> + "immediate");
>>
>> static void
>> uname_normalize (struct utsname *ut)
>> @@ -1990,6 +1994,7 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
>> {
>> virBuffer opt = VIR_BUFFER_INITIALIZER;
>> const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver);
>> + const char *writeout = qemuDomainFSWriteoutTypeToString(fs->writeout);
>>
>> if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) {
>> qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> @@ -2014,6 +2019,10 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
>> virBufferAddLit(&opt, ",security_model=none");
>> }
>> }
>> +
>> + if (fs->writeout != VIR_DOMAIN_FS_WRITEOUT_DEFAULT) {
>
> Since the "writeout" option is introduced far later than the the
> "-fsdev". i.e. "writeout" can be not supported by qemu even if
> "-fsdev" is supported. So we might want to detect the capability
> first and report error earlier than starting the qemu process.
> Like QEMU_CAPS_FSDEV_READONLY in commit a1a83c5.
>
So basically this would help in maintain correct error reporting when
xml has
writeout but user has older qemu which does not support writeout yet,
correct ?
Yes.
> Others looks good, (I just planned to add this support, you step
> one more than me. :-)
>
'guess what, we both probably are stepping on each other, I had earlier
planned
to do readonly fsdev support, saw ur post and moved to writeout support.
lol. :-)
> Regards,
> Osier
>
>
8:17 a.m.
On 12/22/2011 01:01 AM, Deepak C Shetty wrote:
>> + The filesystem block has an optional
>> attribute<code>writeout='immediate'</code>
>
> <code>write</code>, and it's better to clarify it only supports
> "immediate" currently.
So you are proposing to change the attribute name to write instead of
writeout ?
I wanted to keep it as writeout, since it matches with the qemu option
also, will
be easier to locate/debug, no ?
No. Just because qemu names a feature by a particular name does not
mean the libvirt XML must reflect that same name. Libvirt is striving
for a hypervisor-agnostic description of what is happening (in this
case, what is happening is a change to the 'write' policy), not a blind
copy of the qemu command line option.
Perhaps 'write_policy=' makes more sense than 'write=', but it's
longer.
More importantly, I think you're attaching the attribute to the wrong
XML element. This belongs in the <driver> sub-element of <disk>,
alongside the <driver
cache='none|writeback|writethrough|directsync|unsafe'> attribute. It is
not a characteristic of the <disk> as seen by the guest, so much as it
is an attribute of the <driver> used to control how the changes
requested by the guest are reflected back to the host.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4722
days inactive
4722
days old
5 comments
3 participants
participants (3)
-
Deepak C Shetty -
Eric Blake -
Osier Yang