Jim Meyering wrote:
Hi John,
I tried to apply that, but failed miserably,
since all of the following was recently redone to
use virBufferVSprintf rather than snprintf.
Yea I suspected the code was likely seeing some
motion. Thanks for bringing it forward.
And it's
a good thing, because with the changes below, there was
potential buffer overrun: nc could end up larger than PATH_MAX.
You are correct. I mistook the return value of
snprintf() in the case of truncation.
One remaining bit that I haven't done:
add tests for this, exercising e.g.,
- cachemode
- !cachemode && (disk->shared && !disk->readonly)
- !cachemode && (!disk->shared || disk->readonly)
That logic should be correct as it exists in the
patch, namely we'll generate a "cache=*" flag in
the case cachemode was specified explicitly in
the xml definition or for the preexisting case of
(shared && !readonly). Here if both happen to be
true the explicit xml cache tag takes precedence.
But with the existing code we need to remove the
clause of:
@@ -1007,18 +1025,34 @@ int qemudBuildCommandLine(virConnectPtr
if (bootable &&
disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)
virBufferAddLit(&opt, ",boot=on");
- if (disk->shared && !disk->readonly)
- virBufferAddLit(&opt, ",cache=off");
if (disk->driverType)
virBufferVSprintf(&opt, ",fmt=%s", disk->driverType);
As this results in generation of a "cache=*"
twice on the qemu cmdline, and possibly of
different dispositions.
With this change I think it is good to go.
The attached patch incorporates the above
modification.
-john
--
[email protected]
domain_conf.c | 34 ++++++++++++++++++++++++++--------
domain_conf.h | 16 ++++++++++++++++
qemu_conf.c | 43 ++++++++++++++++++++++++++++++++++++++-----
qemu_conf.h | 3 ++-
4 files changed, 82 insertions(+), 14 deletions(-)
=================================================================
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -81,6 +81,21 @@ enum virDomainDiskBus {
VIR_DOMAIN_DISK_BUS_LAST
};
+/* summary of all possible host open file cache modes
+ */
+typedef enum {
+ VIR_DOMAIN_DISK_CACHE_UNSPECIFIED, /* reserved */
+ VIR_DOMAIN_DISK_CACHE_OFF,
+ VIR_DOMAIN_DISK_CACHE_ON,
+ VIR_DOMAIN_DISK_CACHE_DISABLE,
+ VIR_DOMAIN_DISK_CACHE_WRITEBACK,
+ VIR_DOMAIN_DISK_CACHE_WRITETHRU,
+
+ VIR_DOMAIN_DISK_CACHE_LAST
+ } virDomainDiskCache;
+
+VIR_ENUM_DECL(virDomainDiskCache)
+
/* Stores the virtual disk configuration */
typedef struct _virDomainDiskDef virDomainDiskDef;
typedef virDomainDiskDef *virDomainDiskDefPtr;
@@ -92,6 +107,7 @@ struct _virDomainDiskDef {
char *dst;
char *driverName;
char *driverType;
+ int cachemode;
unsigned int readonly : 1;
unsigned int shared : 1;
int slotnum; /* pci slot number for unattach */
=================================================================
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -1,7 +1,7 @@
/*
* config.c: VM configuration management
*
- * Copyright (C) 2006, 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -399,7 +399,9 @@ int qemudExtractVersionInfo(const char *
if (strstr(help, "-domid"))
flags |= QEMUD_CMD_FLAG_DOMID;
if (strstr(help, "-drive"))
- flags |= QEMUD_CMD_FLAG_DRIVE;
+ flags |= QEMUD_CMD_FLAG_DRIVE |
+ (strstr(help, "cache=writethrough|writeback|none") ?
+ QEMUD_CMD_FLAG_DRIVE_CACHEMODE : 0);
if (strstr(help, "boot=on"))
flags |= QEMUD_CMD_FLAG_DRIVE_BOOT;
if (version >= 9000)
@@ -591,6 +593,22 @@ qemudNetworkIfaceConnect(virConnectPtr c
return NULL;
}
+VIR_ENUM_DECL(qemu_cache_map)
+
+/* map from internal cache mode to qemu cache arg text
+ *
+ * Note: currently this replicates virDomainDiskCache, but will need to
+ * error flag potential new entries in virDomainDiskCache which are
+ * not supported by qemu (raising exceptions as appropriate).
+ */
+VIR_ENUM_IMPL(qemu_cache_map, VIR_DOMAIN_DISK_CACHE_LAST,
+ "", /* reserved -- mode not specified */
+ "off", /* deprecated; use "none" */
+ "on", /* obsolete; use "writethrough" */
+ "none",
+ "writeback",
+ "writethrough")
+
static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev,
char *buf,
int buflen)
@@ -1007,11 +1025,27 @@ int qemudBuildCommandLine(virConnectPtr
if (bootable &&
disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)
virBufferAddLit(&opt, ",boot=on");
- if (disk->shared && !disk->readonly)
- virBufferAddLit(&opt, ",cache=off");
if (disk->driverType)
virBufferVSprintf(&opt, ",fmt=%s", disk->driverType);
+ unsigned int qf;
+ int cachemode = disk->cachemode;
+ if (cachemode) {
+ if (qemudExtractVersionInfo(vm->def->emulator, NULL, &qf) < 0)
+ ; /* error reported */
+ else if (!(qf & QEMUD_CMD_FLAG_DRIVE_CACHEMODE)
+ && cachemode == VIR_DOMAIN_DISK_CACHE_WRITETHRU)
+ cachemode = VIR_DOMAIN_DISK_CACHE_ON;
+ else if (qf & QEMUD_CMD_FLAG_DRIVE_CACHEMODE
+ && cachemode == VIR_DOMAIN_DISK_CACHE_ON)
+ cachemode = VIR_DOMAIN_DISK_CACHE_WRITETHRU;
+ }
+ if (cachemode || (disk->shared && !disk->readonly))
+ virBufferVSprintf(&opt, ",cache=%s",
+ (cachemode
+ ? qemu_cache_mapTypeToString(cachemode)
+ : "off"));
+
if (virBufferError(&opt)) {
virReportOOMError(conn);
goto error;
@@ -1577,4 +1611,3 @@ cleanup:
VIR_FREE(xml);
return ret;
}
-
=================================================================
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -1,7 +1,7 @@
/*
* config.h: VM configuration management
*
- * Copyright (C) 2006, 2007 Red Hat, Inc.
+ * Copyright (C) 2006, 2007, 2009 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -53,6 +53,7 @@ enum qemud_cmd_flags {
* since it had a design bug blocking the entire monitor console */
QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP = (1 << 10), /* New migration syntax after merge to QEMU with TCP transport */
QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC = (1 << 11), /* New migration syntax after merge to QEMU with EXEC transport */
+ QEMUD_CMD_FLAG_DRIVE_CACHEMODE = (1 << 12), /* Is the -cache option available */
};
/* Main driver state */
=================================================================
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -1,7 +1,7 @@
/*
* domain_conf.c: domain XML processing
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -551,6 +551,17 @@ int virDomainDiskCompare(virDomainDiskDe
#ifndef PROXY
+
+/* map from xml cache tag to internal cache mode
+ */
+VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST,
+ "", /* reserved -- mode not specified */
+ "off",
+ "on",
+ "none",
+ "writeback",
+ "writethrough");
+
/* Parse the XML definition for a disk
* @param node XML nodeset to parse for disk definition
*/
@@ -567,6 +578,8 @@ virDomainDiskDefParseXML(virConnectPtr c
char *source = NULL;
char *target = NULL;
char *bus = NULL;
+ char *cachetag = NULL;
+ int cm;
if (VIR_ALLOC(def) < 0) {
virReportOOMError(conn);
@@ -616,6 +629,12 @@ virDomainDiskDefParseXML(virConnectPtr c
(xmlStrEqual(cur->name, BAD_CAST "driver"))) {
driverName = virXMLPropString(cur, "name");
driverType = virXMLPropString(cur, "type");
+ cachetag = virXMLPropString(cur, "cache");
+ if (cachetag) {
+ if ((cm = virDomainDiskCacheTypeFromString(cachetag)) != -1)
+ def->cachemode = cm;
+ VIR_FREE(cachetag);
+ }
} else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) {
def->readonly = 1;
} else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) {
@@ -2770,14 +2789,13 @@ virDomainDiskDefFormat(virConnectPtr con
type, device);
if (def->driverName) {
+ virBufferVSprintf(buf, " <driver name='%s'", def->driverName);
if (def->driverType)
- virBufferVSprintf(buf,
- " <driver name='%s' type='%s'/>\n",
- def->driverName, def->driverType);
- else
- virBufferVSprintf(buf,
- " <driver name='%s'/>\n",
- def->driverName);
+ virBufferVSprintf(buf, " type='%s'", def->driverType);
+ if (def->cachemode)
+ virBufferVSprintf(buf, " cache='%s'",
+ virDomainDiskCacheTypeToString(def->cachemode));
+ virBufferVSprintf(buf, "/>\n");
}
if (def->src) {
--
Libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
