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

[Eric Blake]

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 @@
>             <li>
>               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,
>                 "usb-redir",
>                 "usb-hub",
>                 "no-shutdown",
> +
> +              "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)) {
>           fprintf(stderr,
> -                "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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org