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

Osier Yang jyang at redhat.com
Tue Sep 20 03:47:02 UTC 2011


于 2011年09月20日 04:47, Oskari Saarenmaa 写道:
> 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.
> ---
>   docs/formatdomain.html.in                          |    7 ++--
>   docs/schemas/domaincommon.rng                      |    1 +
>   src/conf/domain_conf.c                             |    3 +-
>   src/conf/domain_conf.h                             |    1 +
>   src/qemu/qemu_capabilities.c                       |    3 ++
>   src/qemu/qemu_capabilities.h                       |    1 +
>   src/qemu/qemu_command.c                            |   15 ++++++++-
>   tests/qemuargv2xmltest.c                           |    1 +
>   tests/qemuhelptest.c                               |   19 ++++++-----
>   .../qemuxml2argv-disk-drive-cache-unsafe.args      |    5 +++
>   .../qemuxml2argv-disk-drive-cache-unsafe.xml       |   33 ++++++++++++++++++++
>   tests/qemuxml2argvtest.c                           |    3 ++
>   tools/virsh.pod                                    |    4 +-
>   13 files changed, 80 insertions(+), 16 deletions(-)
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-unsafe.args
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-unsafe.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 0a7abaf..8087327 100644
> --- a/docs/formatdomain.html.in
> +++ 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>
>             </li>
>             <li>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index d0da41c..be98be0 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -848,6 +848,7 @@
>           <value>writeback</value>
>           <value>writethrough</value>
>           <value>directsync</value>
> +<value>unsafe</value>
>         </choice>
>       </attribute>
>     </define>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7476447..21f4c6b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -164,7 +164,8 @@ VIR_ENUM_IMPL(virDomainDiskCache, VIR_DOMAIN_DISK_CACHE_LAST,
>                 "none",
>                 "writethrough",
>                 "writeback",
> -              "directsync")
> +              "directsync",
> +              "unsafe")
>
>   VIR_ENUM_IMPL(virDomainDiskErrorPolicy, VIR_DOMAIN_DISK_ERROR_POLICY_LAST,
>                 "default",
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 371f270..86b4c79 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -192,6 +192,7 @@ enum  virDomainDiskCache {
>       VIR_DOMAIN_DISK_CACHE_WRITETHRU,
>       VIR_DOMAIN_DISK_CACHE_WRITEBACK,
>       VIR_DOMAIN_DISK_CACHE_DIRECTSYNC,
> +    VIR_DOMAIN_DISK_CACHE_UNSAFE,
>
>       VIR_DOMAIN_DISK_CACHE_LAST
>   };
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 36f47a9..13f1aea 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -136,6 +136,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
>                 "pci-ohci",
>                 "usb-redir",
>                 "usb-hub",
> +              "cache-unsafe",
>       );
>
>   struct qemu_feature_flags {
> @@ -918,6 +919,8 @@ qemuCapsComputeCmdFlags(const char *help,
>               if (strstr(help, "directsync"))
>                   qemuCapsSet(flags, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC);
>           }
> +        if (strstr(help, "|unsafe"))
> +            qemuCapsSet(flags, QEMU_CAPS_DRIVE_CACHE_UNSAFE);

IMHO this is unsafe indeed, :-)

If there is a "|unsafe" not for "cache=", things are different, though
there is no another "|unsafe" currently.

>           if (strstr(help, "format="))
>               qemuCapsSet(flags, QEMU_CAPS_DRIVE_FORMAT);
>           if (strstr(help, "readonly="))
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 96b7a3b..97fda0c 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -110,6 +110,7 @@ enum qemuCapsFlags {
>       QEMU_CAPS_PCI_OHCI          = 71, /* -device pci-ohci */
>       QEMU_CAPS_USB_REDIR         = 72, /* -device usb-redir */
>       QEMU_CAPS_USB_HUB           = 73, /* -device usb-hub */
> +    QEMU_CAPS_DRIVE_CACHE_UNSAFE = 74, /* Is cache=unsafe supported? */
>
>       QEMU_CAPS_LAST,                   /* this must always be the last item */
>   };
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index e8b1157..372f9fb 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -66,14 +66,16 @@ VIR_ENUM_IMPL(qemuDiskCacheV1, VIR_DOMAIN_DISK_CACHE_LAST,
>                 "off",
>                 "off",  /* writethrough not supported, so for safety, disable */
>                 "on",   /* Old 'on' was equivalent to 'writeback' */
> -              "off"); /* directsync not supported, for safety, disable */
> +              "off",  /* directsync not supported, for safety, disable */
> +              "off"); /* unsafe not supported, for safety, disable */
>
>   VIR_ENUM_IMPL(qemuDiskCacheV2, VIR_DOMAIN_DISK_CACHE_LAST,
>                 "default",
>                 "none",
>                 "writethrough",
>                 "writeback",
> -              "directsync");
> +              "directsync",
> +              "unsafe");
>
>   VIR_ENUM_DECL(qemuVideo)
>
> @@ -1623,6 +1625,13 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
>                                     "supported by this QEMU"));
>                   goto error;
>               }
> +            else if (disk->cachemode == VIR_DOMAIN_DISK_CACHE_UNSAFE&&

Code style, libvirt prefers:

if {
foo();
} else if {
bar();
}

> +                !qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_UNSAFE)) {
> +                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                                _("disk cache mode 'unsafe' is not "
> +                                  "supported by this QEMU"));
> +                goto error;
> +            }
>           } else {
>               mode = qemuDiskCacheV1TypeToString(disk->cachemode);
>           }
> @@ -5536,6 +5545,8 @@ qemuParseCommandLineDisk(virCapsPtr caps,
>                   def->cachemode = VIR_DOMAIN_DISK_CACHE_WRITETHRU;
>               else if (STREQ(values[i], "directsync"))
>                   def->cachemode = VIR_DOMAIN_DISK_CACHE_DIRECTSYNC;
> +            else if (STREQ(values[i], "unsafe"))
> +                def->cachemode = VIR_DOMAIN_DISK_CACHE_UNSAFE;
>           } else if (STREQ(keywords[i], "werror") ||
>                      STREQ(keywords[i], "rerror")) {
>               if (STREQ(values[i], "stop"))
> diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
> index 91f15af..6a79630 100644
> --- a/tests/qemuargv2xmltest.c
> +++ b/tests/qemuargv2xmltest.c
> @@ -169,6 +169,7 @@ mymain(void)
>       DO_TEST("disk-drive-cache-v2-wb");
>       DO_TEST("disk-drive-cache-v2-none");
>       DO_TEST("disk-drive-cache-directsync");
> +    DO_TEST("disk-drive-cache-unsafe");
>       DO_TEST("disk-drive-network-nbd");
>       DO_TEST("disk-drive-network-rbd");
>       DO_TEST("disk-drive-network-sheepdog");
> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
> index ffd30e2..4d757e5 100644
> --- a/tests/qemuhelptest.c
> +++ 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);
>
>           if (getenv("VIR_TEST_DEBUG"))
>               printMismatchedFlags(flags, info->flags);
> @@ -87,22 +87,22 @@ static int testHelpStrParsing(const void *data)
>       }
>
>       if (version != info->version) {
> -        fprintf(stderr, "Parsed versions do not match: got %u, expected %u\n",
> -                version, info->version);
> +        fprintf(stderr, "%s: parsed versions do not match: got %u, expected %u\n",
> +                info->name, version, info->version);
>           goto cleanup;
>       }
>
>       if (is_kvm != info->is_kvm) {
>           fprintf(stderr,
> -                "Parsed is_kvm flag does not match: got %u, expected %u\n",
> -                is_kvm, info->is_kvm);
> +                "%s: parsed is_kvm flag does not match: got %u, expected %u\n",
> +                info->name, is_kvm, info->is_kvm);
>           goto cleanup;
>       }
>
>       if (kvm_version != info->kvm_version) {
>           fprintf(stderr,
> -                "Parsed KVM versions do not match: got %u, expected %u\n",
> -                kvm_version, info->kvm_version);
> +                "%s: parsed KVM versions do not match: got %u, expected %u\n",
> +                info->name, kvm_version, info->kvm_version);
>           goto cleanup;
>       }
>
> @@ -164,6 +164,7 @@ mymain(void)
>               QEMU_CAPS_MIGRATE_QEMU_TCP,
>               QEMU_CAPS_MIGRATE_QEMU_EXEC,
>               QEMU_CAPS_DRIVE_CACHE_V2,
> +            QEMU_CAPS_DRIVE_CACHE_UNSAFE,
>               QEMU_CAPS_KVM,
>               QEMU_CAPS_DRIVE_FORMAT,
>               QEMU_CAPS_DRIVE_SERIAL,
> @@ -399,6 +400,7 @@ mymain(void)
>               QEMU_CAPS_MIGRATE_QEMU_TCP,
>               QEMU_CAPS_MIGRATE_QEMU_EXEC,
>               QEMU_CAPS_DRIVE_CACHE_V2,
> +            QEMU_CAPS_DRIVE_CACHE_UNSAFE,
>               QEMU_CAPS_KVM,
>               QEMU_CAPS_DRIVE_FORMAT,
>               QEMU_CAPS_DRIVE_SERIAL,
> @@ -450,6 +452,7 @@ mymain(void)
>               QEMU_CAPS_MIGRATE_QEMU_TCP,
>               QEMU_CAPS_MIGRATE_QEMU_EXEC,
>               QEMU_CAPS_DRIVE_CACHE_V2,
> +            QEMU_CAPS_DRIVE_CACHE_UNSAFE,
>               QEMU_CAPS_KVM,
>               QEMU_CAPS_DRIVE_FORMAT,
>               QEMU_CAPS_DRIVE_SERIAL,
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-unsafe.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-unsafe.args
> new file mode 100644
> index 0000000..f8ddcd8
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-unsafe.args
> @@ -0,0 +1,5 @@
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
> +pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \
> +-no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,\
> +format=qcow2,cache=unsafe -drive file=/dev/HostVG/QEMUGuest2,if=ide,\
> +media=cdrom,bus=1,unit=0,format=raw -net none -serial none -parallel none -usb
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-unsafe.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-unsafe.xml
> new file mode 100644
> index 0000000..37185f6
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-cache-unsafe.xml
> @@ -0,0 +1,33 @@
> +<domain type='qemu'>
> +<name>QEMUGuest1</name>
> +<uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +<memory>219136</memory>
> +<currentMemory>219136</currentMemory>
> +<vcpu>1</vcpu>
> +<os>
> +<type arch='i686' machine='pc'>hvm</type>
> +<boot dev='hd'/>
> +</os>
> +<clock offset='utc'/>
> +<on_poweroff>destroy</on_poweroff>
> +<on_reboot>restart</on_reboot>
> +<on_crash>destroy</on_crash>
> +<devices>
> +<emulator>/usr/bin/qemu</emulator>
> +<disk type='block' device='disk'>
> +<driver name='qemu' type='qcow2' cache='unsafe'/>
> +<source dev='/dev/HostVG/QEMUGuest1'/>
> +<target dev='hda' bus='ide'/>
> +<address type='drive' controller='0' bus='0' unit='0'/>
> +</disk>
> +<disk type='block' device='cdrom'>
> +<driver name='qemu' type='raw'/>
> +<source dev='/dev/HostVG/QEMUGuest2'/>
> +<target dev='hdc' bus='ide'/>
> +<readonly/>
> +<address type='drive' controller='0' bus='1' unit='0'/>
> +</disk>
> +<controller type='ide' index='0'/>
> +<memballoon model='virtio'/>
> +</devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index fcb20bb..97b61d1 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -341,6 +341,9 @@ mymain(void)
>       DO_TEST("disk-drive-cache-directsync", false,
>               QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_CACHE_V2,
>               QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC, QEMU_CAPS_DRIVE_FORMAT);
> +    DO_TEST("disk-drive-cache-unsafe", false,
> +            QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_CACHE_V2,
> +            QEMU_CAPS_DRIVE_CACHE_UNSAFE, QEMU_CAPS_DRIVE_FORMAT);
>       DO_TEST("disk-drive-network-nbd", false,
>               QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT);
>       DO_TEST("disk-drive-network-rbd", false,
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 02726f3..73ed045 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -1173,8 +1173,8 @@ floppy device; consider using B<update-device>  for this usage instead.
>   I<mode>  can specify the two specific mode I<readonly>  or I<shareable>.
>   I<persistent>  indicates the changes will affect the next boot of the domain.
>   I<sourcetype>  can indicate the type of source (block|file)
> -I<cache>  can be one of "default", "none", "writethrough", "writeback", or
> -"directsync".
> +I<cache>  can be one of "default", "none", "writethrough", "writeback",
> +"directsync" or "unsafe".
>   I<serial>  is the serial of disk device. I<shareable>  indicates the disk device
>   is shareable between domains.
>   I<address>  is the address of disk device in the form of pci:domain.bus.slot.function,

Others looks file, ACK.

Osier




More information about the libvir-list mailing list