[libvirt] [PATCH 0/1] Disk error policy
The following patch adds support for setting the disk error policy to the domain XML and using it when setting up the qemu command line. It specifies the werror=somepolicy,rerror=somepolicy flags to tell qemu what to do in the case of an io error. This patch is my first foray into the setup of the qemu command line, so please review with heightened care. Dave David Allan (1): Add disk error policy to domain XML docs/schemas/domain.rng | 12 +++++++++++- src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/libvirt_private.syms | 2 +- src/qemu/qemu_conf.c | 12 +++++++++--- tests/qemuhelptest.c | 1 + tests/qemuxml2argvtest.c | 3 +++ 7 files changed, 50 insertions(+), 5 deletions(-)
--- docs/schemas/domain.rng | 12 +++++++++++- src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/libvirt_private.syms | 2 +- src/qemu/qemu_conf.c | 12 +++++++++--- tests/qemuhelptest.c | 1 + tests/qemuxml2argvtest.c | 3 +++ 7 files changed, 50 insertions(+), 5 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 5a8c82b..b276da7 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -521,7 +521,9 @@ <ref name="driverCache"/> </group> </choice> - <empty/> + <optional> + <ref name="driverErrorPolicy"/> + </optional> </element> </define> <define name="driverFormat"> @@ -543,6 +545,14 @@ </choice> </attribute> </define> + <define name="driverErrorPolicy"> + <attribute name="error_policy"> + <choice> + <value>stop</value> + <value>ignore</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 56c5239..6e90561 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -122,6 +122,11 @@ VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST, "writethrough", "writeback") +VIR_ENUM_IMPL(virDomainDiskErrorPolicy, VIR_DOMAIN_DISK_ERROR_POLICY_LAST, + "default", + "stop", + "ignore") + VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "ide", "fdc", @@ -1288,6 +1293,7 @@ virDomainDiskDefParseXML(xmlNodePtr node, char *target = NULL; char *bus = NULL; char *cachetag = NULL; + char *error_policy = NULL; char *devaddr = NULL; virStorageEncryptionPtr encryption = NULL; char *serial = NULL; @@ -1353,6 +1359,7 @@ virDomainDiskDefParseXML(xmlNodePtr node, driverName = virXMLPropString(cur, "name"); driverType = virXMLPropString(cur, "type"); cachetag = virXMLPropString(cur, "cache"); + error_policy = virXMLPropString(cur, "error_policy"); } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { def->readonly = 1; } else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) { @@ -1469,6 +1476,13 @@ virDomainDiskDefParseXML(xmlNodePtr node, goto error; } + if (error_policy && + (def->error_policy = virDomainDiskErrorPolicyTypeFromString(error_policy)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown disk error policy '%s'"), error_policy); + goto error; + } + if (devaddr) { if (sscanf(devaddr, "%x:%x:%x", &def->info.addr.pci.domain, @@ -1510,6 +1524,7 @@ cleanup: VIR_FREE(driverType); VIR_FREE(driverName); VIR_FREE(cachetag); + VIR_FREE(error_policy); VIR_FREE(devaddr); VIR_FREE(serial); virStorageEncryptionFree(encryption); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0540a77..b39ab38 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -150,6 +150,14 @@ enum virDomainDiskCache { VIR_DOMAIN_DISK_CACHE_LAST }; +enum virDomainDiskErrorPolicy { + VIR_DOMAIN_DISK_ERROR_POLICY_DEFAULT, + VIR_DOMAIN_DISK_ERROR_POLICY_STOP, + VIR_DOMAIN_DISK_ERROR_POLICY_IGNORE, + + VIR_DOMAIN_DISK_ERROR_POLICY_LAST +}; + /* Stores the virtual disk configuration */ typedef struct _virDomainDiskDef virDomainDiskDef; typedef virDomainDiskDef *virDomainDiskDefPtr; @@ -163,6 +171,7 @@ struct _virDomainDiskDef { char *driverType; char *serial; int cachemode; + int error_policy; unsigned int readonly : 1; unsigned int shared : 1; virDomainDeviceInfo info; @@ -935,6 +944,7 @@ VIR_ENUM_DECL(virDomainDisk) VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus) VIR_ENUM_DECL(virDomainDiskCache) +VIR_ENUM_DECL(virDomainDiskErrorPolicy) 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 c5ee23d..0cca3c8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -191,7 +191,7 @@ virDomainDefClearDeviceAliases; virDomainDeviceInfoIterate; virDomainClockOffsetTypeToString; virDomainClockOffsetTypeFromString; - +virDomainDiskErrorPolicyTypeToString; # domain_event.h virDomainEventCallbackListAdd; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f2d36f7..0b71ca9 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1215,10 +1215,8 @@ static unsigned long long qemudComputeCmdFlags(const char *help, /* Keep disabled till we're actually ready to turn on JSON mode * The plan is todo it in 0.13.0 QEMU, but lets wait & see... */ -#if 0 - if (version >= 13000) + if (version >= 12000) flags |= QEMUD_CMD_FLAG_MONITOR_JSON; -#endif return flags; } @@ -2448,6 +2446,14 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virBufferAddLit(&opt, ",cache=off"); } + if (qemuCmdFlags & QEMUD_CMD_FLAG_MONITOR_JSON) { + if (disk->error_policy) { + virBufferVSprintf(&opt, ",werror=%s,rerror=%s", + virDomainDiskErrorPolicyTypeToString(disk->error_policy), + virDomainDiskErrorPolicyTypeToString(disk->error_policy)); + } + } + if (virBufferError(&opt)) { virReportOOMError(); goto error; diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index dfdac75..b3ab209 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -227,6 +227,7 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_BALLOON | QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_SMP_TOPOLOGY | + QEMUD_CMD_FLAG_MONITOR_JSON | QEMUD_CMD_FLAG_RTC, 12001, 0, 0); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e3762c9..c98de19 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -261,6 +261,9 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-cache-v1-none", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_FORMAT); + DO_TEST("disk-drive-error-policy-stop", QEMUD_CMD_FLAG_DRIVE | + QEMUD_CMD_FLAG_MONITOR_JSON | + QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-cache-v2-wt", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_CACHE_V2 | QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-cache-v2-wb", QEMUD_CMD_FLAG_DRIVE | -- 1.7.0.1
On Wed, Mar 24, 2010 at 04:31:48PM -0400, David Allan wrote:
--- docs/schemas/domain.rng | 12 +++++++++++- src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/libvirt_private.syms | 2 +- src/qemu/qemu_conf.c | 12 +++++++++--- tests/qemuhelptest.c | 1 + tests/qemuxml2argvtest.c | 3 +++ 7 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 5a8c82b..b276da7 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -521,7 +521,9 @@ <ref name="driverCache"/> </group> </choice> - <empty/> + <optional> + <ref name="driverErrorPolicy"/> + </optional> </element> </define> <define name="driverFormat"> @@ -543,6 +545,14 @@ </choice> </attribute> </define> + <define name="driverErrorPolicy"> + <attribute name="error_policy"> + <choice> + <value>stop</value> + <value>ignore</value> + </choice> + </attribute> + </define> <define name="controller"> <element name="controller"> <choice>
okay, default being the attribute missing ... [...]
--- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1215,10 +1215,8 @@ static unsigned long long qemudComputeCmdFlags(const char *help,
/* Keep disabled till we're actually ready to turn on JSON mode * The plan is todo it in 0.13.0 QEMU, but lets wait & see... */ -#if 0 - if (version >= 13000) + if (version >= 12000) flags |= QEMUD_CMD_FLAG_MONITOR_JSON; -#endif
return flags; }
Hum, I assume it's a debugging left-over, please remove before commit [...]
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index dfdac75..b3ab209 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -227,6 +227,7 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_BALLOON | QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_SMP_TOPOLOGY | + QEMUD_CMD_FLAG_MONITOR_JSON | QEMUD_CMD_FLAG_RTC, 12001, 0, 0);
Hum again you're assuming JSON enabled for qemu 0.12 that sounds problematic, The patch looks fine except for this QEmu versioning for JSON support. That need to be fixed, because I don't think it's generally okay Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 03/25/2010 09:15 AM, Daniel Veillard wrote:
On Wed, Mar 24, 2010 at 04:31:48PM -0400, David Allan wrote:
--- docs/schemas/domain.rng | 12 +++++++++++- src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/libvirt_private.syms | 2 +- src/qemu/qemu_conf.c | 12 +++++++++--- tests/qemuhelptest.c | 1 + tests/qemuxml2argvtest.c | 3 +++ 7 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 5a8c82b..b276da7 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -521,7 +521,9 @@ <ref name="driverCache"/> </group> </choice> -<empty/> +<optional> +<ref name="driverErrorPolicy"/> +</optional> </element> </define> <define name="driverFormat"> @@ -543,6 +545,14 @@ </choice> </attribute> </define> +<define name="driverErrorPolicy"> +<attribute name="error_policy"> +<choice> +<value>stop</value> +<value>ignore</value> +</choice> +</attribute> +</define> <define name="controller"> <element name="controller"> <choice>
okay, default being the attribute missing ...
I actually left default out because it's, well, the default; I ought to have taken it out of the enum as well, but is there use in having it as an actual value?
[...]
--- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1215,10 +1215,8 @@ static unsigned long long qemudComputeCmdFlags(const char *help,
/* Keep disabled till we're actually ready to turn on JSON mode * The plan is todo it in 0.13.0 QEMU, but lets wait& see... */ -#if 0 - if (version>= 13000) + if (version>= 12000) flags |= QEMUD_CMD_FLAG_MONITOR_JSON; -#endif
return flags; }
Hum, I assume it's a debugging left-over, please remove before commit
No, it wasn't a debugging leftover; I misunderstood when JSON went into qemu. I'll put the #if 0 back, but is 13000 the correct version for the not-enabled code, or do we not know the correct version yet? Where is information available about what features are available in what versions of qemu?
[...]
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index dfdac75..b3ab209 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -227,6 +227,7 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_BALLOON | QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_SMP_TOPOLOGY | + QEMUD_CMD_FLAG_MONITOR_JSON | QEMUD_CMD_FLAG_RTC, 12001, 0, 0);
Hum again you're assuming JSON enabled for qemu 0.12 that sounds problematic,
The patch looks fine except for this QEmu versioning for JSON support. That need to be fixed, because I don't think it's generally okay
Daniel
On Wed, Mar 24, 2010 at 04:31:48PM -0400, David Allan wrote:
--- docs/schemas/domain.rng | 12 +++++++++++- src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 10 ++++++++++ src/libvirt_private.syms | 2 +- src/qemu/qemu_conf.c | 12 +++++++++--- tests/qemuhelptest.c | 1 + tests/qemuxml2argvtest.c | 3 +++ 7 files changed, 50 insertions(+), 5 deletions(-)
@@ -1288,6 +1293,7 @@ virDomainDiskDefParseXML(xmlNodePtr node, char *target = NULL; char *bus = NULL; char *cachetag = NULL; + char *error_policy = NULL; char *devaddr = NULL; virStorageEncryptionPtr encryption = NULL; char *serial = NULL; @@ -1353,6 +1359,7 @@ virDomainDiskDefParseXML(xmlNodePtr node, driverName = virXMLPropString(cur, "name"); driverType = virXMLPropString(cur, "type"); cachetag = virXMLPropString(cur, "cache"); + error_policy = virXMLPropString(cur, "error_policy"); } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { def->readonly = 1; } else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) { @@ -1469,6 +1476,13 @@ virDomainDiskDefParseXML(xmlNodePtr node, goto error; }
+ if (error_policy && + (def->error_policy = virDomainDiskErrorPolicyTypeFromString(error_policy)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown disk error policy '%s'"), error_policy); + goto error; + } + if (devaddr) { if (sscanf(devaddr, "%x:%x:%x", &def->info.addr.pci.domain,
Need to also add a chunk to the virDomainDiskDefFormat() method to print the attribute when generating XML
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f2d36f7..0b71ca9 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1215,10 +1215,8 @@ static unsigned long long qemudComputeCmdFlags(const char *help,
/* Keep disabled till we're actually ready to turn on JSON mode * The plan is todo it in 0.13.0 QEMU, but lets wait & see... */ -#if 0 - if (version >= 13000) + if (version >= 12000) flags |= QEMUD_CMD_FLAG_MONITOR_JSON; -#endif
return flags; }
I assume this i just from testing.
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index dfdac75..b3ab209 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -227,6 +227,7 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_BALLOON | QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_SMP_TOPOLOGY | + QEMUD_CMD_FLAG_MONITOR_JSON | QEMUD_CMD_FLAG_RTC, 12001, 0, 0);
Similarly this bit
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e3762c9..c98de19 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -261,6 +261,9 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-cache-v1-none", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_FORMAT); + DO_TEST("disk-drive-error-policy-stop", QEMUD_CMD_FLAG_DRIVE | + QEMUD_CMD_FLAG_MONITOR_JSON | + QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-cache-v2-wt", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_CACHE_V2 | QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-cache-v2-wb", QEMUD_CMD_FLAG_DRIVE |
I think you forgot to 'git add' the two data files for the new disk-drive-error-policy-stop test. If you also add this same datafile to the qemuxml2xmltest.c file you'll get coverage of XML output as well Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Dave Allan -
David Allan