Daniel P. Berrange wrote:
If you loook at src/qemu_conf.c, you'll find a nice method called
qemudExtractVersionInfo, which runs 'qemu -help' and checks for
certain interesting command line arguments :-)
That problem does seem to be crying for some type
of structured interface to avoid subtle breakage
should someone modify the output of "--help".
I'm sure I'm preaching to the choir here.
So it now adapts for the cases of old syntax and
"writethrough" as well as new syntax and "on"
since qemu will otherwise balk at those cache
flag / version combinations.
One note about the enums - rather than adding old style CACHEON
CACHE_OFF options to the main enum in domain_conf, just create
a second enum in the qemu_conf.c file for recording the mapping
of virDomainDiskCache values to old style QEMU arguments values..
As we are adapting in both directions I left the
single enum representing the entire option list
to simplify things. Updated patch is attached.
-john
--
john.cooper@xxxxxxxxxx
domain_conf.c | 31 ++++++++++++++++++++++++-------
domain_conf.h | 16 ++++++++++++++++
qemu_conf.c | 40 +++++++++++++++++++++++++++++++++++-----
qemu_conf.h | 1 +
4 files changed, 76 insertions(+), 12 deletions(-)
=================================================================
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -80,6 +80,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;
@@ -91,6 +106,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
@@ -449,7 +449,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)
@@ -616,6 +618,20 @@ qemudNetworkIfaceConnect(virConnectPtr c
return NULL;
}
+/* 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", /* depreciated by "none" */
+ "on", /* obsolete by "writethrough" */
+ "none",
+ "writeback",
+ "writethrough")
+
static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev,
char *buf,
int buflen)
@@ -721,6 +737,7 @@ int qemudBuildCommandLine(virConnectPtr
const char *emulator;
char uuid[VIR_UUID_STRING_BUFLEN];
char domid[50];
+ unsigned int qf;
uname(&ut);
@@ -946,6 +963,7 @@ int qemudBuildCommandLine(virConnectPtr
virDomainDiskDefPtr disk = vm->def->disks[i];
int idx = virDiskNameToIndex(disk->dst);
const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
+ int nc, cachemode;
if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
@@ -980,15 +998,27 @@ int qemudBuildCommandLine(virConnectPtr
break;
}
- snprintf(opt, PATH_MAX, "file=%s,if=%s,%sindex=%d%s%s",
+ nc = snprintf(opt, PATH_MAX, "file=%s,if=%s,%sindex=%d%s",
disk->src ? disk->src : "", bus,
media ? media : "",
idx,
bootable &&
disk->device == VIR_DOMAIN_DISK_DEVICE_DISK
- ? ",boot=on" : "",
- disk->shared && ! disk->readonly
- ? ",cache=off" : "");
+ ? ",boot=on" : "");
+
+ if (cachemode = disk->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)
+ snprintf(opt + nc, PATH_MAX - nc, ",cache=%s",
+ cachemode ? qemu_cache_mapTypeToString(cachemode) : "off");
ADD_ARG_LIT("-drive");
ADD_ARG_LIT(opt);
=================================================================
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -47,6 +47,7 @@ enum qemud_cmd_flags {
QEMUD_CMD_FLAG_NAME = (1 << 5),
QEMUD_CMD_FLAG_UUID = (1 << 6),
QEMUD_CMD_FLAG_DOMID = (1 << 7), /* Xenner only */
+ QEMUD_CMD_FLAG_DRIVE_CACHEMODE = (1 << 8),
};
/* Main driver state */
=================================================================
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -528,6 +528,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
*/
@@ -543,6 +554,8 @@ virDomainDiskDefParseXML(virConnectPtr c
char *source = NULL;
char *target = NULL;
char *bus = NULL;
+ char *cachetag = NULL;
+ int cm;
if (VIR_ALLOC(def) < 0) {
virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL);
@@ -592,6 +605,11 @@ virDomainDiskDefParseXML(virConnectPtr c
(xmlStrEqual(cur->name, BAD_CAST "driver"))) {
driverName = virXMLPropString(cur, "name");
driverType = virXMLPropString(cur, "type");
+ if (cachetag = virXMLPropString(cur, "cache")) {
+ 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")) {
@@ -2620,14 +2638,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
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list