[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2] Add unsafe cache mode support for disk driver

On 09/21/2011 10:29 AM, Oskari Saarenmaa wrote:
QEMU 0.13 introduced cache=unsafe for -drive, this patch exposes
it in the libvirt layer.

   * Introduced a new QEMU capability flag ($prefix_CACHE_UNSAFE),
     as even if $prefix_CACHE_V2 is set, we can't known if unsafe
     is supported.

   * qemuhelptest prints test case name on failure.
  Updated patch based on Osier Yang's comments and rebased it to 3abadf82

I'm thinking that this is enough of a feature to delay until after we decide whether to do a rapid-turnaround 0.9.6 for the qemu workaround.

+++ b/docs/formatdomain.html.in
@@ -996,9 +996,10 @@
              The optional<code>cache</code>  attribute controls the
              cache mechanism, possible values are "default", "none",
-            "writethrough", "writeback", and "directsync". "directsync"
-            is like "writethrough", but it bypasses the host page
-            cache.
+            "writethrough", "writeback", "directsync" (like
+            "writethrough", but it bypasses the host page cache) and
+            "unsafe" (host may cache all disk io and sync requests from
+            guest are ignored).
              <span class="since">Since 0.6.0</span>

Tweak the since wording:

<span class="since">Since 0.6.0, "unsafe" since 0.9.x</span>

(where x is 6 or 7, depending on delay)

+++ b/src/qemu/qemu_capabilities.c
@@ -137,6 +137,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
+              "cache-unsafe", /* 75 */

  struct qemu_feature_flags {
@@ -918,6 +920,8 @@ qemuCapsComputeCmdFlags(const char *help,
              qemuCapsSet(flags, QEMU_CAPS_DRIVE_CACHE_V2);
              if (strstr(help, "directsync"))
                  qemuCapsSet(flags, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC);
+            if (strstr(help, "|unsafe"))
+                qemuCapsSet(flags, QEMU_CAPS_DRIVE_CACHE_UNSAFE);

I'd feel a bit better if we did strstr(help, "cache="), then strchr(str, '\n'), then validated that strstr(str, "|unsafe") falls before the strchr result, since "unsafe" is a string likely to appear elsewhere in future -help output.

+++ b/tests/qemuhelptest.c
@@ -77,8 +77,8 @@ static int testHelpStrParsing(const void *data)

      if (STRNEQ(got, expected)) {
-                "Computed flags do not match: got %s, expected %s\n",
-                got, expected);
+                "%s: computed flags do not match: got %s, expected %s\n",
+                info->name, got, expected);

This addition of info->name throughout failed tests output is useful as an independent patch, that we could push even now.

Overall, looking pretty good. Hopefully we'll settle out the 0.9.6 decision soon, at which point a v3 of this patch should be worth including.

Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]