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

Re: [libvirt] [PATCHv3] qemu: reduce file padding requirements



On 06/10/2010 06:31 AM, Eric Blake wrote:
Followup to https://bugzilla.redhat.com/show_bug.cgi?id=599091,
commit 20206a4b, to reduce disk waste in padding.

* src/qemu/qemu_monitor.h (QEMU_MONITOR_MIGRATE_TO_FILE_BS): Drop
back to 512.
(QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE): New macro.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Update comment.
* src/qemu/qemu_monitor_text.c (qemuMonitorTextMigrateToFile): Use
two invocations of dd to output non-aligned large blocks.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONMigrateToFile):
Likewise.

-    if (virAsprintf(&dest, "exec:%s | dd of=%s bs=%llu seek=%llu",
-                    argstr, safe_target,
-                    QEMU_MONITOR_MIGRATE_TO_FILE_BS,
-                    offset / QEMU_MONITOR_MIGRATE_TO_FILE_BS)<  0) {
+    /* Two dd processes, sharing the same stdout, are necessary to
+     * allow starting at an alignment of 512, but without wasting
+     * padding to get to the larger alignment useful for speed.  Use
+     *<>  redirection to avoid truncating a regular file.  */
+    if (virAsprintf(&dest, "exec:%s | { dd bs=%llu seek=%llu if=/dev/null&&  "
+                    "dd bs=%llu; } 1<>%s",
+                    argstr, QEMU_MONITOR_MIGRATE_TO_FILE_BS,
+                    offset / QEMU_MONITOR_MIGRATE_TO_FILE_BS,
+                    QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE,
+                    safe_target)<  0) {

On IRC, Matthias pointed out that stock dash 0.5.5 has a bug where using the '1<>file' redirection operator mistakenly truncates file. I hadn't seen it in my testing, because I was on a system where /bin/sh is bash, and bash is immune, as is zsh 4.3.10 and Solaris /bin/sh.

However, when /bin/sh is a buggy dash, then this mistaken truncation clobbers the header, breaking managed save of qemu guests.

The redirection bug has since been fixed in dash 0.5.6, and that fix has been backported to at least Ubuntu's dash 0.5.5 build. However, it raises the question - should libvirt be testing for this shell bug for the benefit of people hand-building new libvirt on systems with buggy /bin/sh (such as Ubuntu 10.04), and if so, should I work on a patch to provide a workaround?

--
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]