[libvirt] [PATCHv3 00/10] outgoing migration via fd: rather than exec:
Still not quite complete (I need to hook up compression to benefit from fd:), but in much better shape (this series could be applied as-is, if it receives ACKs, and I can do the compression improvements separately). Changes in v3: add lots of intermediate patches, to make fd passing much easier. Eric Blake (10): qemu: use lighter-weight fd:n on incoming tunneled migration qemu: support migration to fd util: use SCM_RIGHTS in virFileOperation when needed qemu: allow simple domain save to use fd: protocol qemu: simplify domain save fd handling storage: simplify fd handling util: rename virFileOperation to virFileOpenAs util: adjust indentation in previous patch qemu, storage: improve type safety qemu: improve efficiency of dd during snapshots src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 105 ++++++++++++++------------- src/qemu/qemu_migration.c | 45 ++++-------- src/qemu/qemu_monitor.c | 22 ++++++ src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 24 ++++++- src/qemu/qemu_monitor_json.h | 6 ++- src/qemu/qemu_monitor_text.c | 27 ++++++- src/qemu/qemu_monitor_text.h | 6 ++- src/storage/storage_backend.c | 77 ++++++++++---------- src/util/util.c | 161 ++++++++++++++++++++++++++++------------- src/util/util.h | 15 ++-- 12 files changed, 310 insertions(+), 184 deletions(-) -- 1.7.4
Outgoing migration still uses a Unix socket and or exec netcat until the next patch. * src/qemu/qemu_migration.c (qemuMigrationPrepareTunnel): Replace Unix socket with simpler pipe. Suggested by Paolo Bonzini. --- change in v3: mark pipe fd as cloexec src/qemu/qemu_migration.c | 45 +++++++++++++-------------------------------- 1 files changed, 13 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 822cb18..d2280c6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -239,11 +239,10 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, { virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; - char *migrateFrom; virDomainEventPtr event = NULL; int ret = -1; int internalret; - char *unixfile = NULL; + int dataFD[2] = { -1, -1 }; virBitmapPtr qemuCaps = NULL; qemuDomainObjPrivatePtr priv = NULL; struct timeval now; @@ -289,12 +288,12 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, /* Domain starts inactive, even if the domain XML had an id field. */ vm->def->id = -1; - if (virAsprintf(&unixfile, "%s/qemu.tunnelmigrate.dest.%s", - driver->libDir, vm->def->name) < 0) { - virReportOOMError(); + if (pipe(dataFD) < 0 || + virSetCloseExec(dataFD[0]) < 0) { + virReportSystemError(errno, "%s", + _("cannot create pipe for tunnelled migration")); goto endjob; } - unlink(unixfile); /* check that this qemu version supports the interactive exec */ if (qemuCapsExtractVersionInfo(vm->def->emulator, vm->def->os.arch, @@ -304,25 +303,11 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, vm->def->emulator); goto endjob; } - if (qemuCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_UNIX)) - internalret = virAsprintf(&migrateFrom, "unix:%s", unixfile); - else if (qemuCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_EXEC)) - internalret = virAsprintf(&migrateFrom, "exec:nc -U -l %s", unixfile); - else { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Destination qemu is too old to support tunnelled migration")); - goto endjob; - } - if (internalret < 0) { - virReportOOMError(); - goto endjob; - } /* Start the QEMU daemon, with the same command-line arguments plus - * -incoming unix:/path/to/file or exec:nc -U /path/to/file + * -incoming stdin (which qemu_command might convert to exec:cat or fd:n) */ - internalret = qemuProcessStart(dconn, driver, vm, migrateFrom, true, - -1, NULL, VIR_VM_OP_MIGRATE_IN_START); - VIR_FREE(migrateFrom); + internalret = qemuProcessStart(dconn, driver, vm, "stdin", true, dataFD[1], + NULL, VIR_VM_OP_MIGRATE_IN_START); if (internalret < 0) { qemuDomainStartAudit(vm, "migrated", false); /* Note that we don't set an error here because qemuProcessStart @@ -335,9 +320,7 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, goto endjob; } - if (virFDStreamConnectUNIX(st, - unixfile, - false) < 0) { + if (virFDStreamOpen(st, dataFD[0]) < 0) { qemuDomainStartAudit(vm, "migrated", false); qemuProcessStop(driver, vm, 0); if (!vm->persistent) { @@ -345,9 +328,8 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, virDomainRemoveInactive(&driver->domains, vm); vm = NULL; } - virReportSystemError(errno, - _("cannot open unix socket '%s' for tunnelled migration"), - unixfile); + virReportSystemError(errno, "%s", + _("cannot pass pipe for tunnelled migration")); goto endjob; } @@ -378,9 +360,8 @@ endjob: cleanup: qemuCapsFree(qemuCaps); virDomainDefFree(def); - if (unixfile) - unlink(unixfile); - VIR_FREE(unixfile); + VIR_FORCE_CLOSE(dataFD[0]); + VIR_FORCE_CLOSE(dataFD[1]); if (vm) virDomainObjUnlock(vm); if (event) -- 1.7.4
* src/qemu/qemu_monitor.h (qemuMonitorMigrateToFd): New prototype. * src/qemu/qemu_monitor_text.h (qemuMonitorTextMigrateToFd): Likewise. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONMigrateToFd): Likewise. * src/qemu/qemu_monitor.c (qemuMonitorMigrateToFd): New function. * src/qemu/qemu_monitor_text.c (qemuMonitorTextMigrateToFd): Likewise. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONMigrateToFd): Likewise. --- no change in v3 src/qemu/qemu_monitor.c | 22 ++++++++++++++++++++++ src/qemu/qemu_monitor.h | 4 ++++ src/qemu/qemu_monitor_json.c | 21 ++++++++++++++++++++- src/qemu/qemu_monitor_json.h | 6 +++++- src/qemu/qemu_monitor_text.c | 24 ++++++++++++++++++++++-- src/qemu/qemu_monitor_text.h | 6 +++++- 6 files changed, 78 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index dfb1aad..312e797 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1384,6 +1384,28 @@ int qemuMonitorGetMigrationStatus(qemuMonitorPtr mon, } +int qemuMonitorMigrateToFd(qemuMonitorPtr mon, + unsigned int flags, + int fd) +{ + int ret; + VIR_DEBUG("mon=%p fd=%d flags=%u", + mon, fd, flags); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONMigrateToFd(mon, flags, fd); + else + ret = qemuMonitorTextMigrateToFd(mon, flags, fd); + return ret; +} + + int qemuMonitorMigrateToHost(qemuMonitorPtr mon, unsigned int flags, const char *hostname, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 0ea1330..1a64ac0 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -260,6 +260,10 @@ typedef enum { QEMU_MONITOR_MIGRATION_FLAGS_LAST } QEMU_MONITOR_MIGRATE; +int qemuMonitorMigrateToFd(qemuMonitorPtr mon, + unsigned int flags, + int fd); + int qemuMonitorMigrateToHost(qemuMonitorPtr mon, unsigned int flags, const char *hostname, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e6903a1..81201ff 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1,7 +1,7 @@ /* * qemu_monitor_json.c: interaction with QEMU monitor console * - * Copyright (C) 2006-2010 Red Hat, Inc. + * Copyright (C) 2006-2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1697,6 +1697,25 @@ static int qemuMonitorJSONMigrate(qemuMonitorPtr mon, } +int qemuMonitorJSONMigrateToFd(qemuMonitorPtr mon, + unsigned int flags, + int fd) +{ + int ret; + + if (qemuMonitorJSONSendFileHandle(mon, "migrate", fd) < 0) + return -1; + + ret = qemuMonitorJSONMigrate(mon, flags, "fd:migrate"); + + if (ret < 0) { + if (qemuMonitorJSONCloseFileHandle(mon, "migrate") < 0) + VIR_WARN0("failed to close migration handle"); + } + + return ret; +} + int qemuMonitorJSONMigrateToHost(qemuMonitorPtr mon, unsigned int flags, const char *hostname, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 4ae472a..c2b45f3 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -1,7 +1,7 @@ /* * qemu_monitor_json.h: interaction with QEMU monitor console * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2009, 2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -104,6 +104,10 @@ int qemuMonitorJSONGetMigrationStatus(qemuMonitorPtr mon, unsigned long long *remaining, unsigned long long *total); +int qemuMonitorJSONMigrateToFd(qemuMonitorPtr mon, + unsigned int flags, + int fd); + int qemuMonitorJSONMigrateToHost(qemuMonitorPtr mon, unsigned int flags, const char *hostname, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 0aed690..76e0ec0 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1,7 +1,7 @@ /* * qemu_monitor_text.c: interaction with QEMU monitor console * - * Copyright (C) 2006-2010 Red Hat, Inc. + * Copyright (C) 2006-2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1256,7 +1256,8 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon, * unknown command: migrate" */ if (strstr(info, "unknown command:")) { qemuReportError(VIR_ERR_NO_SUPPORT, - _("migration to '%s' not supported by this qemu: %s"), dest, info); + _("migration to '%s' not supported by this qemu: %s"), + dest, info); goto cleanup; } @@ -1271,6 +1272,25 @@ cleanup: return ret; } +int qemuMonitorTextMigrateToFd(qemuMonitorPtr mon, + unsigned int flags, + int fd) +{ + int ret; + + if (qemuMonitorTextSendFileHandle(mon, "migrate", fd) < 0) + return -1; + + ret = qemuMonitorTextMigrate(mon, flags, "fd:migrate"); + + if (ret < 0) { + if (qemuMonitorTextCloseFileHandle(mon, "migrate") < 0) + VIR_WARN0("failed to close migration handle"); + } + + return ret; +} + int qemuMonitorTextMigrateToHost(qemuMonitorPtr mon, unsigned int flags, const char *hostname, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index b29dbcc..f1b53fc 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -1,7 +1,7 @@ /* * qemu_monitor_text.h: interaction with QEMU monitor console * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2009, 2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -102,6 +102,10 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, unsigned long long *remaining, unsigned long long *total); +int qemuMonitorTextMigrateToFd(qemuMonitorPtr mon, + unsigned int flags, + int fd); + int qemuMonitorTextMigrateToHost(qemuMonitorPtr mon, unsigned int flags, const char *hostname, -- 1.7.4
I'm tired of maintaining two copies of migration uri generation. * src/qemu/qemu_monitor_text.h (qemuMonitorTextMigrate): Declare in place of individual monitor commands. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONMigrate): Likewise. * src/qemu/qemu_monitor_text.c (qemuMonitorTextMigrateToFd) (qemuMonitorTextMigrateToHost, qemuMonitorTextMigrateToCommand) (qemuMonitorTextMigrateToFile, qemuMonitorTextMigrateToUnix): Delete. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONMigrateToFd) (qemuMonitorJSONMigrateToHost, qemuMonitorJSONMigrateToCommand) (qemuMonitorJSONMigrateToFile, qemuMonitorJSONMigrateToUnix): Delete. * src/qemu/qemu_monitor.c (qemuMonitorMigrateToFd) (qemuMonitorMigrateToHost, qemuMonitorMigrateToCommand) (qemuMonitorMigrateToFile, qemuMonitorMigrateToUnix): Consolidate shared code. --- To be applied after patch 2/10; or I can rebase and apply this first and make patch 2/10 smaller (only has to touch qemu_monitor.[ch]). src/qemu/qemu_monitor.c | 106 +++++++++++++++++++++++++++---- src/qemu/qemu_monitor_json.c | 143 +----------------------------------------- src/qemu/qemu_monitor_json.h | 25 +------- src/qemu/qemu_monitor_text.c | 141 +---------------------------------------- src/qemu/qemu_monitor_text.h | 25 +------- 5 files changed, 105 insertions(+), 335 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 312e797..1e79523 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1398,10 +1398,19 @@ int qemuMonitorMigrateToFd(qemuMonitorPtr mon, return -1; } + if (qemuMonitorSendFileHandle(mon, "migrate", fd) < 0) + return -1; + if (mon->json) - ret = qemuMonitorJSONMigrateToFd(mon, flags, fd); + ret = qemuMonitorJSONMigrate(mon, flags, "fd:migrate"); else - ret = qemuMonitorTextMigrateToFd(mon, flags, fd); + ret = qemuMonitorTextMigrate(mon, flags, "fd:migrate"); + + if (ret < 0) { + if (qemuMonitorCloseFileHandle(mon, "migrate") < 0) + VIR_WARN0("failed to close migration handle"); + } + return ret; } @@ -1412,6 +1421,7 @@ int qemuMonitorMigrateToHost(qemuMonitorPtr mon, int port) { int ret; + char *uri = NULL; VIR_DEBUG("mon=%p hostname=%s port=%d flags=%u", mon, hostname, port, flags); @@ -1421,10 +1431,18 @@ int qemuMonitorMigrateToHost(qemuMonitorPtr mon, return -1; } + + if (virAsprintf(&uri, "tcp:%s:%d", hostname, port) < 0) { + virReportOOMError(); + return -1; + } + if (mon->json) - ret = qemuMonitorJSONMigrateToHost(mon, flags, hostname, port); + ret = qemuMonitorJSONMigrate(mon, flags, uri); else - ret = qemuMonitorTextMigrateToHost(mon, flags, hostname, port); + ret = qemuMonitorTextMigrate(mon, flags, uri); + + VIR_FREE(uri); return ret; } @@ -1433,7 +1451,9 @@ int qemuMonitorMigrateToCommand(qemuMonitorPtr mon, unsigned int flags, const char * const *argv) { - int ret; + char *argstr; + char *dest = NULL; + int ret = -1; VIR_DEBUG("mon=%p argv=%p flags=%u", mon, argv, flags); @@ -1443,10 +1463,25 @@ int qemuMonitorMigrateToCommand(qemuMonitorPtr mon, return -1; } + argstr = virArgvToString(argv); + if (!argstr) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&dest, "exec:%s", argstr) < 0) { + virReportOOMError(); + goto cleanup; + } + if (mon->json) - ret = qemuMonitorJSONMigrateToCommand(mon, flags, argv); + ret = qemuMonitorJSONMigrate(mon, flags, dest); else - ret = qemuMonitorTextMigrateToCommand(mon, flags, argv); + ret = qemuMonitorTextMigrate(mon, flags, dest); + +cleanup: + VIR_FREE(argstr); + VIR_FREE(dest); return ret; } @@ -1456,7 +1491,10 @@ int qemuMonitorMigrateToFile(qemuMonitorPtr mon, const char *target, unsigned long long offset) { - int ret; + char *argstr; + char *dest = NULL; + int ret = -1; + char *safe_target = NULL; VIR_DEBUG("mon=%p argv=%p target=%s offset=%llu flags=%u", mon, argv, target, offset, flags); @@ -1473,10 +1511,44 @@ int qemuMonitorMigrateToFile(qemuMonitorPtr mon, return -1; } + argstr = virArgvToString(argv); + if (!argstr) { + virReportOOMError(); + goto cleanup; + } + + /* Migrate to file */ + safe_target = qemuMonitorEscapeShell(target); + if (!safe_target) { + virReportOOMError(); + goto cleanup; + } + + /* 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:" VIR_WRAPPER_SHELL_PREFIX "%s | " + "{ dd bs=%llu seek=%llu if=/dev/null && " + "dd ibs=%llu obs=%llu; } 1<>%s" VIR_WRAPPER_SHELL_SUFFIX, + argstr, QEMU_MONITOR_MIGRATE_TO_FILE_BS, + offset / QEMU_MONITOR_MIGRATE_TO_FILE_BS, + QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE, + QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE, + safe_target) < 0) { + virReportOOMError(); + goto cleanup; + } + if (mon->json) - ret = qemuMonitorJSONMigrateToFile(mon, flags, argv, target, offset); + ret = qemuMonitorJSONMigrate(mon, flags, dest); else - ret = qemuMonitorTextMigrateToFile(mon, flags, argv, target, offset); + ret = qemuMonitorTextMigrate(mon, flags, dest); + +cleanup: + VIR_FREE(safe_target); + VIR_FREE(argstr); + VIR_FREE(dest); return ret; } @@ -1484,7 +1556,8 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, unsigned int flags, const char *unixfile) { - int ret; + char *dest = NULL; + int ret = -1; VIR_DEBUG("mon=%p, unixfile=%s flags=%u", mon, unixfile, flags); @@ -1494,10 +1567,17 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, return -1; } + if (virAsprintf(&dest, "unix:%s", unixfile) < 0) { + virReportOOMError(); + return -1; + } + if (mon->json) - ret = qemuMonitorJSONMigrateToUnix(mon, flags, unixfile); + ret = qemuMonitorJSONMigrate(mon, flags, dest); else - ret = qemuMonitorTextMigrateToUnix(mon, flags, unixfile); + ret = qemuMonitorTextMigrate(mon, flags, dest); + + VIR_FREE(dest); return ret; } diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5fb43b0..5cf35b9 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1669,9 +1669,9 @@ int qemuMonitorJSONGetMigrationStatus(qemuMonitorPtr mon, } -static int qemuMonitorJSONMigrate(qemuMonitorPtr mon, - unsigned int flags, - const char *uri) +int qemuMonitorJSONMigrate(qemuMonitorPtr mon, + unsigned int flags, + const char *uri) { int ret; virJSONValuePtr cmd = @@ -1696,143 +1696,6 @@ static int qemuMonitorJSONMigrate(qemuMonitorPtr mon, return ret; } - -int qemuMonitorJSONMigrateToFd(qemuMonitorPtr mon, - unsigned int flags, - int fd) -{ - int ret; - - if (qemuMonitorJSONSendFileHandle(mon, "migrate", fd) < 0) - return -1; - - ret = qemuMonitorJSONMigrate(mon, flags, "fd:migrate"); - - if (ret < 0) { - if (qemuMonitorJSONCloseFileHandle(mon, "migrate") < 0) - VIR_WARN0("failed to close migration handle"); - } - - return ret; -} - -int qemuMonitorJSONMigrateToHost(qemuMonitorPtr mon, - unsigned int flags, - const char *hostname, - int port) -{ - char *uri = NULL; - int ret; - - if (virAsprintf(&uri, "tcp:%s:%d", hostname, port) < 0) { - virReportOOMError(); - return -1; - } - - ret = qemuMonitorJSONMigrate(mon, flags, uri); - - VIR_FREE(uri); - - return ret; -} - - -int qemuMonitorJSONMigrateToCommand(qemuMonitorPtr mon, - unsigned int flags, - const char * const *argv) -{ - char *argstr; - char *dest = NULL; - int ret = -1; - - argstr = virArgvToString(argv); - if (!argstr) { - virReportOOMError(); - goto cleanup; - } - - if (virAsprintf(&dest, "exec:%s", argstr) < 0) { - virReportOOMError(); - goto cleanup; - } - - ret = qemuMonitorJSONMigrate(mon, flags, dest); - -cleanup: - VIR_FREE(argstr); - VIR_FREE(dest); - return ret; -} - -int qemuMonitorJSONMigrateToFile(qemuMonitorPtr mon, - unsigned int flags, - const char * const *argv, - const char *target, - unsigned long long offset) -{ - char *argstr; - char *dest = NULL; - int ret = -1; - char *safe_target = NULL; - - argstr = virArgvToString(argv); - if (!argstr) { - virReportOOMError(); - goto cleanup; - } - - /* Migrate to file */ - safe_target = qemuMonitorEscapeShell(target); - if (!safe_target) { - virReportOOMError(); - goto cleanup; - } - - /* 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:" VIR_WRAPPER_SHELL_PREFIX "%s | " - "{ dd bs=%llu seek=%llu if=/dev/null && " - "dd ibs=%llu obs=%llu; } 1<>%s" VIR_WRAPPER_SHELL_SUFFIX, - argstr, QEMU_MONITOR_MIGRATE_TO_FILE_BS, - offset / QEMU_MONITOR_MIGRATE_TO_FILE_BS, - QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE, - QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE, - safe_target) < 0) { - virReportOOMError(); - goto cleanup; - } - - ret = qemuMonitorJSONMigrate(mon, flags, dest); - -cleanup: - VIR_FREE(safe_target); - VIR_FREE(argstr); - VIR_FREE(dest); - return ret; -} - -int qemuMonitorJSONMigrateToUnix(qemuMonitorPtr mon, - unsigned int flags, - const char *unixfile) -{ - char *dest = NULL; - int ret = -1; - - if (virAsprintf(&dest, "unix:%s", unixfile) < 0) { - virReportOOMError(); - return -1; - } - - ret = qemuMonitorJSONMigrate(mon, flags, dest); - - VIR_FREE(dest); - - return ret; -} - - int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) { int ret; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c2b45f3..bca7070 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -104,28 +104,9 @@ int qemuMonitorJSONGetMigrationStatus(qemuMonitorPtr mon, unsigned long long *remaining, unsigned long long *total); -int qemuMonitorJSONMigrateToFd(qemuMonitorPtr mon, - unsigned int flags, - int fd); - -int qemuMonitorJSONMigrateToHost(qemuMonitorPtr mon, - unsigned int flags, - const char *hostname, - int port); - -int qemuMonitorJSONMigrateToCommand(qemuMonitorPtr mon, - unsigned int flags, - const char * const *argv); - -int qemuMonitorJSONMigrateToFile(qemuMonitorPtr mon, - unsigned int flags, - const char * const *argv, - const char *target, - unsigned long long offset); - -int qemuMonitorJSONMigrateToUnix(qemuMonitorPtr mon, - unsigned int flags, - const char *unixfile); +int qemuMonitorJSONMigrate(qemuMonitorPtr mon, + unsigned int flags, + const char *uri); int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 3c9f799..f6275b2 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1205,9 +1205,9 @@ cleanup: } -static int qemuMonitorTextMigrate(qemuMonitorPtr mon, - unsigned int flags, - const char *dest) +int qemuMonitorTextMigrate(qemuMonitorPtr mon, + unsigned int flags, + const char *dest) { char *cmd = NULL; char *info = NULL; @@ -1272,141 +1272,6 @@ cleanup: return ret; } -int qemuMonitorTextMigrateToFd(qemuMonitorPtr mon, - unsigned int flags, - int fd) -{ - int ret; - - if (qemuMonitorTextSendFileHandle(mon, "migrate", fd) < 0) - return -1; - - ret = qemuMonitorTextMigrate(mon, flags, "fd:migrate"); - - if (ret < 0) { - if (qemuMonitorTextCloseFileHandle(mon, "migrate") < 0) - VIR_WARN0("failed to close migration handle"); - } - - return ret; -} - -int qemuMonitorTextMigrateToHost(qemuMonitorPtr mon, - unsigned int flags, - const char *hostname, - int port) -{ - char *uri = NULL; - int ret; - - if (virAsprintf(&uri, "tcp:%s:%d", hostname, port) < 0) { - virReportOOMError(); - return -1; - } - - ret = qemuMonitorTextMigrate(mon, flags, uri); - - VIR_FREE(uri); - - return ret; -} - - -int qemuMonitorTextMigrateToCommand(qemuMonitorPtr mon, - unsigned int flags, - const char * const *argv) -{ - char *argstr; - char *dest = NULL; - int ret = -1; - - argstr = virArgvToString(argv); - if (!argstr) { - virReportOOMError(); - goto cleanup; - } - - if (virAsprintf(&dest, "exec:%s", argstr) < 0) { - virReportOOMError(); - goto cleanup; - } - - ret = qemuMonitorTextMigrate(mon, flags, dest); - -cleanup: - VIR_FREE(argstr); - VIR_FREE(dest); - return ret; -} - -int qemuMonitorTextMigrateToFile(qemuMonitorPtr mon, - unsigned int flags, - const char * const *argv, - const char *target, - unsigned long long offset) -{ - char *argstr; - char *dest = NULL; - int ret = -1; - char *safe_target = NULL; - - argstr = virArgvToString(argv); - if (!argstr) { - virReportOOMError(); - goto cleanup; - } - - /* Migrate to file */ - safe_target = qemuMonitorEscapeShell(target); - if (!safe_target) { - virReportOOMError(); - goto cleanup; - } - - /* 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:" VIR_WRAPPER_SHELL_PREFIX "%s | " - "{ dd bs=%llu seek=%llu if=/dev/null && " - "dd ibs=%llu obs=%llu; } 1<>%s" VIR_WRAPPER_SHELL_SUFFIX, - argstr, QEMU_MONITOR_MIGRATE_TO_FILE_BS, - offset / QEMU_MONITOR_MIGRATE_TO_FILE_BS, - QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE, - QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE, - safe_target) < 0) { - virReportOOMError(); - goto cleanup; - } - - ret = qemuMonitorTextMigrate(mon, flags, dest); - -cleanup: - VIR_FREE(safe_target); - VIR_FREE(argstr); - VIR_FREE(dest); - return ret; -} - -int qemuMonitorTextMigrateToUnix(qemuMonitorPtr mon, - unsigned int flags, - const char *unixfile) -{ - char *dest = NULL; - int ret = -1; - - if (virAsprintf(&dest, "unix:%s", unixfile) < 0) { - virReportOOMError(); - return -1; - } - - ret = qemuMonitorTextMigrate(mon, flags, dest); - - VIR_FREE(dest); - - return ret; -} - int qemuMonitorTextMigrateCancel(qemuMonitorPtr mon) { char *info = NULL; diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index f1b53fc..c2a85b7 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -102,28 +102,9 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, unsigned long long *remaining, unsigned long long *total); -int qemuMonitorTextMigrateToFd(qemuMonitorPtr mon, - unsigned int flags, - int fd); - -int qemuMonitorTextMigrateToHost(qemuMonitorPtr mon, - unsigned int flags, - const char *hostname, - int port); - -int qemuMonitorTextMigrateToCommand(qemuMonitorPtr mon, - unsigned int flags, - const char * const *argv); - -int qemuMonitorTextMigrateToFile(qemuMonitorPtr mon, - unsigned int flags, - const char * const *argv, - const char *target, - unsigned long long offset); - -int qemuMonitorTextMigrateToUnix(qemuMonitorPtr mon, - unsigned int flags, - const char *unixfile); +int qemuMonitorTextMigrate(qemuMonitorPtr mon, + unsigned int flags, + const char *uri); int qemuMonitorTextMigrateCancel(qemuMonitorPtr mon); -- 1.7.4
Currently, the hook function in virFileOperation is extremely limited: it must be async-signal-safe, and cannot modify any memory in the parent process. It is much handier to return a valid fd and operate on it in the parent than to deal with hook restrictions. * src/util/util.h (VIR_FILE_OP_RETURN_FD): New flag. * src/util/util.c (virFileOperationNoFork, virFileOperation): Honor new flag. --- v3: new patch; some of this gets simplified later, but doing it in stages makes it much easier to review, so I plan to keep it broken into multiple patches src/util/util.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++------ src/util/util.h | 4 +- 2 files changed, 126 insertions(+), 17 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index f41e117..6bf3219 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1392,14 +1392,15 @@ static int virFileOperationNoFork(const char *path, int openflags, mode_t mode, if ((hook) && ((ret = hook(fd, hookdata)) != 0)) { goto error; } + if (flags & VIR_FILE_OP_RETURN_FD) + return fd; if (VIR_CLOSE(fd) < 0) { ret = -errno; virReportSystemError(errno, _("failed to close new file '%s'"), path); - fd = -1; goto error; } - fd = -1; + error: VIR_FORCE_CLOSE(fd); return ret; @@ -1444,7 +1445,32 @@ error: return ret; } -/* return -errno on failure, or 0 on success */ +/** + * virFileOperation: + * @path: file to open or create + * @openflags: flags to pass to open + * @mode: mode to use on creation or when forcing permissions + * @uid: uid that should own file on creation + * @gid: gid that should own file + * @hook: callback to run once file is open; see below for restrictions + * @hookdata: opaque data for @hook + * @flags: bit-wise or of VIR_FILE_OP_* flags + * + * Open @path, and execute an optional callback @hook on the open file + * description. @hook must return 0 on success, or -errno on failure. + * If @flags includes VIR_FILE_OP_AS_UID, then open the file while the + * effective user id is @uid (by using a child process); this allows + * one to bypass root-squashing NFS issues. If @flags includes + * VIR_FILE_OP_FORCE_PERMS, then ensure that @path has those + * permissions before the callback, even if the file already existed + * with different permissions. If @flags includes + * VIR_FILE_OP_RETURN_FD, then use SCM_RIGHTS to guarantee that any + * @hook is run in the parent, and the return value (if non-negative) + * is the file descriptor, left open. Otherwise, @hook might be run + * in a child process, so it must be async-signal-safe (ie. no + * malloc() or anything else that depends on modifying locks held in + * the parent), no file descriptor remains open, and 0 is returned on + * success. Return -errno on failure. */ int virFileOperation(const char *path, int openflags, mode_t mode, uid_t uid, gid_t gid, virFileOperationHook hook, void *hookdata, @@ -1452,7 +1478,14 @@ int virFileOperation(const char *path, int openflags, mode_t mode, struct stat st; pid_t pid; int waitret, status, ret = 0; - int fd; + int fd = -1; + int pair[2] = { -1, -1 }; + struct msghdr msg; + struct cmsghdr *cmsg; + char buf[CMSG_SPACE(sizeof(fd))]; + char dummy = 0; + struct iovec iov; + int forkRet; if ((!(flags & VIR_FILE_OP_AS_UID)) || (getuid() != 0) @@ -1466,7 +1499,29 @@ int virFileOperation(const char *path, int openflags, mode_t mode, * following dance avoids problems caused by root-squashing * NFS servers. */ - int forkRet = virFork(&pid); + if (flags & VIR_FILE_OP_RETURN_FD) { + if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0) { + ret = -errno; + virReportSystemError(errno, + _("failed to create socket needed for '%s'"), + path); + return ret; + } + + memset(&msg, 0, sizeof(msg)); + iov.iov_base = &dummy; + iov.iov_len = 1; + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + msg.msg_control = buf; + msg.msg_controllen = sizeof(buf); + cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = CMSG_LEN(sizeof(fd)); + } + + forkRet = virFork(&pid); if (pid < 0) { ret = -errno; @@ -1474,6 +1529,31 @@ int virFileOperation(const char *path, int openflags, mode_t mode, } if (pid) { /* parent */ + if (flags & VIR_FILE_OP_RETURN_FD) { + VIR_FORCE_CLOSE(pair[1]); + + do { + ret = recvmsg(pair[0], &msg, 0); + } while (ret < 0 && errno == EINTR); + + if (ret < 0) { + ret = -errno; + VIR_FORCE_CLOSE(pair[0]); + while ((waitret = waitpid(pid, NULL, 0) == -1) + && (errno == EINTR)); + goto parenterror; + } + VIR_FORCE_CLOSE(pair[0]); + + /* See if fd was transferred. */ + cmsg = CMSG_FIRSTHDR(&msg); + if (cmsg && cmsg->cmsg_len == CMSG_LEN(sizeof(fd)) && + cmsg->cmsg_level == SOL_SOCKET && + cmsg->cmsg_type == SCM_RIGHTS) { + memcpy(&fd, CMSG_DATA(cmsg), sizeof(fd)); + } + } + /* wait for child to complete, and retrieve its exit code */ while ((waitret = waitpid(pid, &status, 0) == -1) && (errno == EINTR)); @@ -1485,12 +1565,20 @@ int virFileOperation(const char *path, int openflags, mode_t mode, goto parenterror; } ret = -WEXITSTATUS(status); - if (!WIFEXITED(status) || (ret == -EACCES)) { + if (!WIFEXITED(status) || (ret == -EACCES) || + ((flags & VIR_FILE_OP_RETURN_FD) && fd == -1)) { /* fall back to the simpler method, which works better in * some cases */ return virFileOperationNoFork(path, openflags, mode, uid, gid, hook, hookdata, flags); } + if (!ret && (flags & VIR_FILE_OP_RETURN_FD)) { + if ((hook) && ((ret = hook(fd, hookdata)) != 0)) { + VIR_FORCE_CLOSE(fd); + goto parenterror; + } + ret = fd; + } parenterror: return ret; } @@ -1500,6 +1588,7 @@ parenterror: if (forkRet < 0) { /* error encountered and logged in virFork() after the fork. */ + ret = -errno; goto childerror; } @@ -1549,15 +1638,33 @@ parenterror: path, mode); goto childerror; } - if ((hook) && ((ret = hook(fd, hookdata)) != 0)) { - goto childerror; - } - if (VIR_CLOSE(fd) < 0) { - ret = -errno; - virReportSystemError(errno, _("child failed to close new file '%s'"), - path); - goto childerror; + if (flags & VIR_FILE_OP_RETURN_FD) { + VIR_FORCE_CLOSE(pair[0]); + memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd)); + + do { + ret = sendmsg(pair[1], &msg, 0); + } while (ret < 0 && errno == EINTR); + + if (ret < 0) { + ret = -errno; + VIR_FORCE_CLOSE(pair[1]); + goto childerror; + } + ret = 0; + } else { + if ((hook) && ((ret = hook(fd, hookdata)) != 0)) + goto childerror; + if (VIR_CLOSE(fd) < 0) { + ret = -errno; + virReportSystemError(errno, + _("child failed to close new file '%s'"), + path); + goto childerror; + } } + + ret = 0; childerror: /* ret tracks -errno on failure, but exit value must be positive. * If the child exits with EACCES, then the parent tries again. */ @@ -1688,7 +1795,7 @@ int virFileOperation(const char *path ATTRIBUTE_UNUSED, virUtilError(VIR_ERR_INTERNAL_ERROR, "%s", _("virFileOperation is not implemented for WIN32")); - return -1; + return -ENOSYS; } int virDirCreate(const char *path ATTRIBUTE_UNUSED, @@ -1700,7 +1807,7 @@ int virDirCreate(const char *path ATTRIBUTE_UNUSED, virUtilError(VIR_ERR_INTERNAL_ERROR, "%s", _("virDirCreate is not implemented for WIN32")); - return -1; + return -ENOSYS; } #endif /* WIN32 */ diff --git a/src/util/util.h b/src/util/util.h index 5f6473c..3970d58 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -128,12 +128,14 @@ enum { VIR_FILE_OP_NONE = 0, VIR_FILE_OP_AS_UID = (1 << 0), VIR_FILE_OP_FORCE_PERMS = (1 << 1), + VIR_FILE_OP_RETURN_FD = (1 << 2), }; typedef int (*virFileOperationHook)(int fd, void *data); int virFileOperation(const char *path, int openflags, mode_t mode, uid_t uid, gid_t gid, virFileOperationHook hook, void *hookdata, - unsigned int flags) ATTRIBUTE_RETURN_CHECK; + unsigned int flags) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; enum { VIR_DIR_CREATE_NONE = 0, -- 1.7.4
Work in progress - this fails for root-squash NFS, and doesn't cover compression coupled with migration; both of which need to be provided by a final solution. * src/qemu/qemu_driver.c (qemudDomainSaveFlag): Use new function when there is no compression. --- changes in v3: honor qemuCaps src/qemu/qemu_driver.c | 26 +++++++++++++++++++++++--- 1 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c9095bb..6b28ce0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1791,6 +1791,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, int is_reg = 0; unsigned long long offset; virCgroupPtr cgroup = NULL; + virBitmapPtr qemuCaps = NULL; memset(&header, 0, sizeof(header)); memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)); @@ -1821,6 +1822,11 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, } } + if (qemuCapsExtractVersionInfo(vm->def->emulator, vm->def->os.arch, + NULL, + &qemuCaps) < 0) + goto endjob; + /* Get XML for the domain */ xml = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_SECURE); if (!xml) { @@ -1981,10 +1987,23 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, if (header.compressed == QEMUD_SAVE_FORMAT_RAW) { const char *args[] = { "cat", NULL }; + /* XXX gross - why don't we reuse the fd already opened earlier */ + int fd = -1; + + if (qemuCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) + fd = open(path, O_WRONLY); qemuDomainObjEnterMonitorWithDriver(driver, vm); - rc = qemuMonitorMigrateToFile(priv->mon, - QEMU_MONITOR_MIGRATE_BACKGROUND, - args, path, offset); + /* XXX needs to depend on QEMU_CAPS_MIGRATE_QEMU_FD */ + if (fd >= 0 && lseek(fd, offset, SEEK_SET) == offset) { + rc = qemuMonitorMigrateToFd(priv->mon, + QEMU_MONITOR_MIGRATE_BACKGROUND, + fd); + } else { + rc = qemuMonitorMigrateToFile(priv->mon, + QEMU_MONITOR_MIGRATE_BACKGROUND, + args, path, offset); + } + VIR_FORCE_CLOSE(fd); qemuDomainObjExitMonitorWithDriver(driver, vm); } else { const char *prog = qemudSaveCompressionTypeToString(header.compressed); @@ -2067,6 +2086,7 @@ endjob: } cleanup: + qemuCapsFree(qemuCaps); VIR_FREE(xml); if (ret != 0 && is_reg) unlink(path); -- 1.7.4
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Use new virFileOperation flag to open fd only once. --- v3: new patch src/qemu/qemu_driver.c | 74 ++++++++++++++++++++++++----------------------- 1 files changed, 38 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6b28ce0..42d2125 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1792,6 +1792,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, unsigned long long offset; virCgroupPtr cgroup = NULL; virBitmapPtr qemuCaps = NULL; + int fd = -1; memset(&header, 0, sizeof(header)); memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)); @@ -1870,45 +1871,32 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, header.xml_len += pad; } - /* Setup hook data needed by virFileOperation hook function */ - hdata.dom = dom; - hdata.path = path; - hdata.xml = xml; - hdata.header = &header; - - /* Write header to file, followed by XML */ + /* Obtain the file handle. */ /* First try creating the file as root */ if (!is_reg) { - int fd = open(path, O_WRONLY | O_TRUNC); + fd = open(path, O_WRONLY | O_TRUNC); if (fd < 0) { virReportSystemError(errno, _("unable to open %s"), path); goto endjob; } - if (qemudDomainSaveFileOpHook(fd, &hdata) < 0) { - VIR_FORCE_CLOSE(fd); - goto endjob; - } - if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, _("unable to close %s"), path); - goto endjob; - } } else { - if ((rc = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY, - S_IRUSR|S_IWUSR, - getuid(), getgid(), - qemudDomainSaveFileOpHook, &hdata, - 0)) < 0) { + if ((fd = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY, + S_IRUSR|S_IWUSR, + getuid(), getgid(), + NULL, NULL, + VIR_FILE_OP_RETURN_FD)) < 0) { /* If we failed as root, and the error was permission-denied (EACCES or EPERM), assume it's on a network-connected share where root access is restricted (eg, root-squashed NFS). If the qemu user (driver->user) is non-root, just set a flag to bypass security driver shenanigans, and retry the operation after doing setuid to qemu user */ - + rc = fd; if (((rc != -EACCES) && (rc != -EPERM)) || driver->user == getuid()) { - virReportSystemError(-rc, _("Failed to create domain save file '%s'"), + virReportSystemError(-rc, + _("Failed to create domain save file '%s'"), path); goto endjob; } @@ -1933,7 +1921,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, default: /* local file - log the error returned by virFileOperation */ virReportSystemError(-rc, - _("Failed to create domain save file '%s'"), + _("Failed to create domain save file '%s'"), path); goto endjob; break; @@ -1942,13 +1930,15 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, /* Retry creating the file as driver->user */ - if ((rc = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY, + if ((fd = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP, driver->user, driver->group, - qemudDomainSaveFileOpHook, &hdata, - VIR_FILE_OP_AS_UID)) < 0) { - virReportSystemError(-rc, _("Error from child process creating '%s'"), - path); + NULL, NULL, + (VIR_FILE_OP_AS_UID | + VIR_FILE_OP_RETURN_FD))) < 0) { + virReportSystemError(-fd, + _("Error from child process creating '%s'"), + path); goto endjob; } @@ -1960,6 +1950,18 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, } } + /* Write header to file, followed by XML */ + hdata.dom = dom; + hdata.path = path; + hdata.xml = xml; + hdata.header = &header; + + if (qemudDomainSaveFileOpHook(fd, &hdata) < 0) { + VIR_FORCE_CLOSE(fd); + goto endjob; + } + + /* Allow qemu to access file */ if (!is_reg && qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { @@ -1987,14 +1989,9 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, if (header.compressed == QEMUD_SAVE_FORMAT_RAW) { const char *args[] = { "cat", NULL }; - /* XXX gross - why don't we reuse the fd already opened earlier */ - int fd = -1; - if (qemuCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) - fd = open(path, O_WRONLY); qemuDomainObjEnterMonitorWithDriver(driver, vm); - /* XXX needs to depend on QEMU_CAPS_MIGRATE_QEMU_FD */ - if (fd >= 0 && lseek(fd, offset, SEEK_SET) == offset) { + if (qemuCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) { rc = qemuMonitorMigrateToFd(priv->mon, QEMU_MONITOR_MIGRATE_BACKGROUND, fd); @@ -2003,7 +2000,6 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, offset); } - VIR_FORCE_CLOSE(fd); qemuDomainObjExitMonitorWithDriver(driver, vm); } else { const char *prog = qemudSaveCompressionTypeToString(header.compressed); @@ -2022,6 +2018,11 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, if (rc < 0) goto endjob; + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, _("unable to close %s"), path); + goto endjob; + } + rc = qemuMigrationWaitForCompletion(driver, vm); if (rc < 0) @@ -2087,6 +2088,7 @@ endjob: cleanup: qemuCapsFree(qemuCaps); + VIR_FORCE_CLOSE(fd); VIR_FREE(xml); if (ret != 0 && is_reg) unlink(path); -- 1.7.4
* src/storage/storage_backend.c (virStorageBackendCreateRaw): Use new virFileOperation flag. --- v3: new patch; unrelated, but the only other client of virFileOperation src/storage/storage_backend.c | 47 +++++++++++++++++++++++++--------------- 1 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 2eede74..02f8b5b 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1,7 +1,7 @@ /* * storage_backend.c: internal storage driver backend contract * - * Copyright (C) 2007-2010 Red Hat, Inc. + * Copyright (C) 2007-2011 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -344,11 +344,15 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { int ret = -1; - int createstat; + int fd = -1; struct createRawFileOpHookData hdata = { vol, inputvol }; + uid_t uid; + gid_t gid; + + virCheckFlags(0, -1); if (vol->target.encryption != NULL) { virStorageReportError(VIR_ERR_NO_SUPPORT, @@ -357,24 +361,31 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } - uid_t uid = (vol->target.perms.uid == -1) ? getuid() : vol->target.perms.uid; - gid_t gid = (vol->target.perms.gid == -1) ? getgid() : vol->target.perms.gid; + uid = (vol->target.perms.uid == -1) ? getuid() : vol->target.perms.uid; + gid = (vol->target.perms.gid == -1) ? getgid() : vol->target.perms.gid; + flags = VIR_FILE_OP_FORCE_PERMS | VIR_FILE_OP_RETURN_FD; + if (pool->def->type == VIR_STORAGE_POOL_NETFS) + flags |= VIR_FILE_OP_AS_UID; - if ((createstat = virFileOperation(vol->target.path, - O_RDWR | O_CREAT | O_EXCL, - vol->target.perms.mode, uid, gid, - createRawFileOpHook, &hdata, - VIR_FILE_OP_FORCE_PERMS | - (pool->def->type == VIR_STORAGE_POOL_NETFS - ? VIR_FILE_OP_AS_UID : 0))) < 0) { - virReportSystemError(-createstat, - _("cannot create path '%s'"), - vol->target.path); - goto cleanup; + if ((fd = virFileOperation(vol->target.path, + O_RDWR | O_CREAT | O_EXCL, + vol->target.perms.mode, uid, gid, + NULL, NULL, flags)) < 0) { + virReportSystemError(-fd, + _("cannot create path '%s'"), + vol->target.path); + goto cleanup; + } + + if ((ret = createRawFileOpHook(fd, &hdata)) < 0) { + virReportSystemError(-fd, + _("cannot create path '%s'"), + vol->target.path); + ret = -1; } - ret = 0; cleanup: + VIR_FORCE_CLOSE(fd); return ret; } -- 1.7.4
On 03/04/2011 01:37 AM, Eric Blake wrote:
- unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags)
Uhm, you're still not using the flags, you're just using the argument as a place to write the flags for virFileOperation. Perhaps the "inner" flags variable should be renamed? Paolo
On 03/04/2011 01:15 AM, Paolo Bonzini wrote:
On 03/04/2011 01:37 AM, Eric Blake wrote:
- unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags)
Uhm, you're still not using the flags, you're just using the argument as a place to write the flags for virFileOperation. Perhaps the "inner" flags variable should be renamed?
Actually, I did use the flags - the virCheckFlags(0,-1) added at the top of the function is a macro that uses the incoming argument. But yes, repurposing the incoming argument is confusing, I'll rename the second use of flags to something else. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
This patch intentionally doesn't change indentation, in order to make it easier to review the real changes. * src/util/util.h (VIR_FILE_OP_RETURN_FD, virFileOperationHook): Delete. (virFileOperation): Rename... (virFileOpenAs): ...and reduce parameters. * src/util/util.c (virFileOperationNoFork, virFileOperation): Rename and simplify. * src/qemu/qemu_driver.c (qemudDomainSaveFlag): Adjust caller. * src/storage/storage_backend.c (virStorageBackendCreateRaw): Likewise. * src/libvirt_private.syms: Reflect rename. --- v3: new patch src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 20 +++----- src/storage/storage_backend.c | 12 ++-- src/util/util.c | 103 +++++++++++++---------------------------- src/util/util.h | 15 ++---- 5 files changed, 53 insertions(+), 99 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 56b5bdf..4cc5705 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -874,8 +874,8 @@ virFileIsExecutable; virFileLinkPointsTo; virFileMakePath; virFileMatchesNameSuffix; +virFileOpenAs; virFileOpenTty; -virFileOperation; virFilePid; virFileReadAll; virFileReadLimFD; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 42d2125..a222309 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1881,11 +1881,9 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, goto endjob; } } else { - if ((fd = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY, - S_IRUSR|S_IWUSR, - getuid(), getgid(), - NULL, NULL, - VIR_FILE_OP_RETURN_FD)) < 0) { + if ((fd = virFileOpenAs(path, O_CREAT|O_TRUNC|O_WRONLY, + S_IRUSR|S_IWUSR, + getuid(), getgid(), 0)) < 0) { /* If we failed as root, and the error was permission-denied (EACCES or EPERM), assume it's on a network-connected share where root access is restricted (eg, root-squashed NFS). If the @@ -1919,7 +1917,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, case 0: default: - /* local file - log the error returned by virFileOperation */ + /* local file - log the error returned by virFileOpenAs */ virReportSystemError(-rc, _("Failed to create domain save file '%s'"), path); @@ -1930,12 +1928,10 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, /* Retry creating the file as driver->user */ - if ((fd = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY, - S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP, - driver->user, driver->group, - NULL, NULL, - (VIR_FILE_OP_AS_UID | - VIR_FILE_OP_RETURN_FD))) < 0) { + if ((fd = virFileOpenAs(path, O_CREAT|O_TRUNC|O_WRONLY, + S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP, + driver->user, driver->group, + VIR_FILE_OPEN_AS_UID)) < 0) { virReportSystemError(-fd, _("Error from child process creating '%s'"), path); diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 02f8b5b..5e8d94a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -363,14 +363,14 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, uid = (vol->target.perms.uid == -1) ? getuid() : vol->target.perms.uid; gid = (vol->target.perms.gid == -1) ? getgid() : vol->target.perms.gid; - flags = VIR_FILE_OP_FORCE_PERMS | VIR_FILE_OP_RETURN_FD; + flags = VIR_FILE_OPEN_FORCE_PERMS; if (pool->def->type == VIR_STORAGE_POOL_NETFS) - flags |= VIR_FILE_OP_AS_UID; + flags |= VIR_FILE_OPEN_AS_UID; - if ((fd = virFileOperation(vol->target.path, - O_RDWR | O_CREAT | O_EXCL, - vol->target.perms.mode, uid, gid, - NULL, NULL, flags)) < 0) { + if ((fd = virFileOpenAs(vol->target.path, + O_RDWR | O_CREAT | O_EXCL, + vol->target.perms.mode, uid, gid, + flags)) < 0) { virReportSystemError(-fd, _("cannot create path '%s'"), vol->target.path); diff --git a/src/util/util.c b/src/util/util.c index 6bf3219..137cdb9 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1355,10 +1355,10 @@ virFileIsExecutable(const char *file) #ifndef WIN32 /* return -errno on failure, or 0 on success */ -static int virFileOperationNoFork(const char *path, int openflags, mode_t mode, - uid_t uid, gid_t gid, - virFileOperationHook hook, void *hookdata, - unsigned int flags) { +static int +virFileOpenAsNoFork(const char *path, int openflags, mode_t mode, + uid_t uid, gid_t gid, unsigned int flags) +{ int fd = -1; int ret = 0; struct stat st; @@ -1381,7 +1381,7 @@ static int virFileOperationNoFork(const char *path, int openflags, mode_t mode, path, (unsigned int) uid, (unsigned int) gid); goto error; } - if ((flags & VIR_FILE_OP_FORCE_PERMS) + if ((flags & VIR_FILE_OPEN_FORCE_PERMS) && (fchmod(fd, mode) < 0)) { ret = -errno; virReportSystemError(errno, @@ -1389,17 +1389,7 @@ static int virFileOperationNoFork(const char *path, int openflags, mode_t mode, path, mode); goto error; } - if ((hook) && ((ret = hook(fd, hookdata)) != 0)) { - goto error; - } - if (flags & VIR_FILE_OP_RETURN_FD) - return fd; - if (VIR_CLOSE(fd) < 0) { - ret = -errno; - virReportSystemError(errno, _("failed to close new file '%s'"), - path); - goto error; - } + return fd; error: VIR_FORCE_CLOSE(fd); @@ -1446,35 +1436,27 @@ error: } /** - * virFileOperation: + * virFileOpenAs: * @path: file to open or create * @openflags: flags to pass to open * @mode: mode to use on creation or when forcing permissions * @uid: uid that should own file on creation * @gid: gid that should own file - * @hook: callback to run once file is open; see below for restrictions - * @hookdata: opaque data for @hook - * @flags: bit-wise or of VIR_FILE_OP_* flags + * @flags: bit-wise or of VIR_FILE_OPEN_* flags * * Open @path, and execute an optional callback @hook on the open file * description. @hook must return 0 on success, or -errno on failure. - * If @flags includes VIR_FILE_OP_AS_UID, then open the file while the + * If @flags includes VIR_FILE_OPEN_AS_UID, then open the file while the * effective user id is @uid (by using a child process); this allows * one to bypass root-squashing NFS issues. If @flags includes - * VIR_FILE_OP_FORCE_PERMS, then ensure that @path has those + * VIR_FILE_OPEN_FORCE_PERMS, then ensure that @path has those * permissions before the callback, even if the file already existed - * with different permissions. If @flags includes - * VIR_FILE_OP_RETURN_FD, then use SCM_RIGHTS to guarantee that any - * @hook is run in the parent, and the return value (if non-negative) - * is the file descriptor, left open. Otherwise, @hook might be run - * in a child process, so it must be async-signal-safe (ie. no - * malloc() or anything else that depends on modifying locks held in - * the parent), no file descriptor remains open, and 0 is returned on - * success. Return -errno on failure. */ -int virFileOperation(const char *path, int openflags, mode_t mode, - uid_t uid, gid_t gid, - virFileOperationHook hook, void *hookdata, - unsigned int flags) { + * with different permissions. The return value (if non-negative) + * is the file descriptor, left open. Return -errno on failure. */ +int +virFileOpenAs(const char *path, int openflags, mode_t mode, + uid_t uid, gid_t gid, unsigned int flags) +{ struct stat st; pid_t pid; int waitret, status, ret = 0; @@ -1487,11 +1469,10 @@ int virFileOperation(const char *path, int openflags, mode_t mode, struct iovec iov; int forkRet; - if ((!(flags & VIR_FILE_OP_AS_UID)) + if ((!(flags & VIR_FILE_OPEN_AS_UID)) || (getuid() != 0) || ((uid == 0) && (gid == 0))) { - return virFileOperationNoFork(path, openflags, mode, uid, gid, - hook, hookdata, flags); + return virFileOpenAsNoFork(path, openflags, mode, uid, gid, flags); } /* parent is running as root, but caller requested that the @@ -1499,7 +1480,7 @@ int virFileOperation(const char *path, int openflags, mode_t mode, * following dance avoids problems caused by root-squashing * NFS servers. */ - if (flags & VIR_FILE_OP_RETURN_FD) { + { if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0) { ret = -errno; virReportSystemError(errno, @@ -1529,7 +1510,7 @@ int virFileOperation(const char *path, int openflags, mode_t mode, } if (pid) { /* parent */ - if (flags & VIR_FILE_OP_RETURN_FD) { + { VIR_FORCE_CLOSE(pair[1]); do { @@ -1565,20 +1546,13 @@ int virFileOperation(const char *path, int openflags, mode_t mode, goto parenterror; } ret = -WEXITSTATUS(status); - if (!WIFEXITED(status) || (ret == -EACCES) || - ((flags & VIR_FILE_OP_RETURN_FD) && fd == -1)) { + if (!WIFEXITED(status) || (ret == -EACCES) || fd == -1) { /* fall back to the simpler method, which works better in * some cases */ - return virFileOperationNoFork(path, openflags, mode, uid, gid, - hook, hookdata, flags); + return virFileOpenAsNoFork(path, openflags, mode, uid, gid, flags); } - if (!ret && (flags & VIR_FILE_OP_RETURN_FD)) { - if ((hook) && ((ret = hook(fd, hookdata)) != 0)) { - VIR_FORCE_CLOSE(fd); - goto parenterror; - } + if (!ret) ret = fd; - } parenterror: return ret; } @@ -1630,7 +1604,7 @@ parenterror: path, (unsigned int) uid, (unsigned int) gid); goto childerror; } - if ((flags & VIR_FILE_OP_FORCE_PERMS) + if ((flags & VIR_FILE_OPEN_FORCE_PERMS) && (fchmod(fd, mode) < 0)) { ret = -errno; virReportSystemError(errno, @@ -1638,7 +1612,7 @@ parenterror: path, mode); goto childerror; } - if (flags & VIR_FILE_OP_RETURN_FD) { + { VIR_FORCE_CLOSE(pair[0]); memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd)); @@ -1651,17 +1625,6 @@ parenterror: VIR_FORCE_CLOSE(pair[1]); goto childerror; } - ret = 0; - } else { - if ((hook) && ((ret = hook(fd, hookdata)) != 0)) - goto childerror; - if (VIR_CLOSE(fd) < 0) { - ret = -errno; - virReportSystemError(errno, - _("child failed to close new file '%s'"), - path); - goto childerror; - } } ret = 0; @@ -1783,17 +1746,15 @@ childerror: #else /* WIN32 */ /* return -errno on failure, or 0 on success */ -int virFileOperation(const char *path ATTRIBUTE_UNUSED, - int openflags ATTRIBUTE_UNUSED, - mode_t mode ATTRIBUTE_UNUSED, - uid_t uid ATTRIBUTE_UNUSED, - gid_t gid ATTRIBUTE_UNUSED, - virFileOperationHook hook ATTRIBUTE_UNUSED, - void *hookdata ATTRIBUTE_UNUSED, - unsigned int flags ATTRIBUTE_UNUSED) +int virFileOpenAs(const char *path ATTRIBUTE_UNUSED, + int openflags ATTRIBUTE_UNUSED, + mode_t mode ATTRIBUTE_UNUSED, + uid_t uid ATTRIBUTE_UNUSED, + gid_t gid ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { virUtilError(VIR_ERR_INTERNAL_ERROR, - "%s", _("virFileOperation is not implemented for WIN32")); + "%s", _("virFileOpenAs is not implemented for WIN32")); return -ENOSYS; } diff --git a/src/util/util.h b/src/util/util.h index 3970d58..cecd348 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -125,16 +125,13 @@ bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1); char *virFileSanitizePath(const char *path); enum { - VIR_FILE_OP_NONE = 0, - VIR_FILE_OP_AS_UID = (1 << 0), - VIR_FILE_OP_FORCE_PERMS = (1 << 1), - VIR_FILE_OP_RETURN_FD = (1 << 2), + VIR_FILE_OPEN_NONE = 0, + VIR_FILE_OPEN_AS_UID = (1 << 0), + VIR_FILE_OPEN_FORCE_PERMS = (1 << 1), }; -typedef int (*virFileOperationHook)(int fd, void *data); -int virFileOperation(const char *path, int openflags, mode_t mode, - uid_t uid, gid_t gid, - virFileOperationHook hook, void *hookdata, - unsigned int flags) +int virFileOpenAs(const char *path, int openflags, mode_t mode, + uid_t uid, gid_t gid, + unsigned int flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; enum { -- 1.7.4
On 03/04/2011 01:37 AM, Eric Blake wrote:
(virFileOperation): Rename... (virFileOpenAs): ...and reduce parameters.
As an "external observer" of libvirt development, I must admit virFileOperation always seemed an extremely heavyweight and clunky interface. Thanks for cleaning it up! Paolo
Direct access to an open file is so much simpler than passing everything through a pipe! * src/qemu/qemu_driver.c (qemudOpenAsUID) (qemudDomainSaveImageClose): Delete. (qemudDomainSaveImageOpen): Rename... (qemuDomainSaveImageOpen): ...and drop read_pid argument. Use virFileOpenAs instead of qemudOpenAsUID. (qemudDomainSaveImageStartVM, qemudDomainRestore) (qemudDomainObjRestore): Rename... (qemuDomainSaveImageStartVM, qemuDomainRestore) (qemDomainObjRestore): ...and simplify accordingly. (qemudDomainObjStart, qemuDriver): Update callers. ---
On 03/04/2011 01:37 AM, Eric Blake wrote:
(virFileOperation): Rename... (virFileOpenAs): ...and reduce parameters.
As an "external observer" of libvirt development, I must admit virFileOperation always seemed an extremely heavyweight and clunky interface. Thanks for cleaning it up!
Love the size of this next code simplification! Plus it makes restoration faster for root-squash NFS, by avoiding passing all I/O through a pipe. src/qemu/qemu_driver.c | 265 ++++++++--------------------------------------- 1 files changed, 45 insertions(+), 220 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5502113..a6d2f30 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3013,183 +3013,32 @@ cleanup: return ret; } -/* qemudOpenAsUID() - pipe/fork/setuid/open a file, and return the - pipe fd to caller, so that it can read from the file. Also return - the pid of the child process, so the caller can wait for it to exit - after it's finished reading (to avoid a zombie, if nothing - else). */ - -static int -qemudOpenAsUID(const char *path, uid_t uid, gid_t gid, pid_t *child_pid) -{ - int pipefd[2]; - int fd = -1; - - *child_pid = -1; - - if (pipe(pipefd) < 0) { - virReportSystemError(errno, - _("failed to create pipe to read '%s'"), - path); - pipefd[0] = pipefd[1] = -1; - goto parent_cleanup; - } - - int forkRet = virFork(child_pid); - - if (*child_pid < 0) { - virReportSystemError(errno, - _("failed to fork child to read '%s'"), - path); - goto parent_cleanup; - } - - if (*child_pid > 0) { - - /* parent */ - - /* parent doesn't need the write side of the pipe */ - VIR_FORCE_CLOSE(pipefd[1]); - - if (forkRet < 0) { - virReportSystemError(errno, - _("failed in parent after forking child to read '%s'"), - path); - goto parent_cleanup; - } - /* caller gets the read side of the pipe */ - fd = pipefd[0]; - pipefd[0] = -1; -parent_cleanup: - VIR_FORCE_CLOSE(pipefd[0]); - VIR_FORCE_CLOSE(pipefd[1]); - if ((fd < 0) && (*child_pid > 0)) { - /* a child process was started and subsequently an error - occurred in the parent, so we need to wait for it to - exit, but its status is inconsequential. */ - while ((waitpid(*child_pid, NULL, 0) == -1) - && (errno == EINTR)) { - /* empty */ - } - *child_pid = -1; - } - return fd; - } - - /* child */ - - /* setuid to the qemu user, then open the file, read it, - and stuff it into the pipe for the parent process to - read */ - int exit_code; - char *buf = NULL; - size_t bufsize = 1024 * 1024; - int bytesread; - - /* child doesn't need the read side of the pipe */ - VIR_FORCE_CLOSE(pipefd[0]); - - if (forkRet < 0) { - exit_code = errno; - virReportSystemError(errno, - _("failed in child after forking to read '%s'"), - path); - goto child_cleanup; - } - - if (virSetUIDGID(uid, gid) < 0) { - exit_code = errno; - goto child_cleanup; - } - - if ((fd = open(path, O_RDONLY)) < 0) { - exit_code = errno; - virReportSystemError(errno, - _("cannot open '%s' as uid %d"), - path, uid); - goto child_cleanup; - } - - if (VIR_ALLOC_N(buf, bufsize) < 0) { - exit_code = ENOMEM; - virReportOOMError(); - goto child_cleanup; - } - - /* read from fd and write to pipefd[1] until EOF */ - do { - if ((bytesread = saferead(fd, buf, bufsize)) < 0) { - exit_code = errno; - virReportSystemError(errno, - _("child failed reading from '%s'"), - path); - goto child_cleanup; - } - if (safewrite(pipefd[1], buf, bytesread) != bytesread) { - exit_code = errno; - virReportSystemError(errno, "%s", - _("child failed writing to pipe")); - goto child_cleanup; - } - } while (bytesread > 0); - exit_code = 0; - -child_cleanup: - VIR_FREE(buf); - VIR_FORCE_CLOSE(fd); - VIR_FORCE_CLOSE(pipefd[1]); - _exit(exit_code); -} - -static int qemudDomainSaveImageClose(int fd, pid_t read_pid, int *status) -{ - int ret = 0; - - if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, "%s", - _("cannot close file")); - } - - if (read_pid != -1) { - /* reap the process that read the file */ - while ((ret = waitpid(read_pid, status, 0)) == -1 - && errno == EINTR) { - /* empty */ - } - } else if (status) { - *status = 0; - } - - return ret; -} - -static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) -qemudDomainSaveImageOpen(struct qemud_driver *driver, - const char *path, - virDomainDefPtr *ret_def, - struct qemud_save_header *ret_header, - pid_t *ret_read_pid) +static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) +qemuDomainSaveImageOpen(struct qemud_driver *driver, + const char *path, + virDomainDefPtr *ret_def, + struct qemud_save_header *ret_header) { int fd; - pid_t read_pid = -1; struct qemud_save_header header; char *xml = NULL; virDomainDefPtr def = NULL; - if ((fd = open(path, O_RDONLY)) < 0) { - if ((driver->user == 0) || (getuid() != 0)) { + if ((fd = virFileOpenAs(path, O_RDONLY, 0, getuid(), getgid(), 0)) < 0) { + if ((fd != -EACCES && fd != -EPERM) || + driver->user == getuid()) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot read domain image")); goto error; } /* Opening as root failed, but qemu runs as a different user - that might have better luck. Create a pipe, then fork a - child process to run as the qemu user, which will hopefully - have the necessary authority to read the file. */ - if ((fd = qemudOpenAsUID(path, - driver->user, driver->group, &read_pid)) < 0) { - /* error already reported */ + * that might have better luck. */ + if ((fd = virFileOpenAs(path, O_RDONLY, 0, + driver->user, driver->group, + VIR_FILE_OPEN_AS_UID)) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("cannot read domain image")); goto error; } } @@ -3239,34 +3088,30 @@ qemudDomainSaveImageOpen(struct qemud_driver *driver, *ret_def = def; *ret_header = header; - *ret_read_pid = read_pid; return fd; error: virDomainDefFree(def); VIR_FREE(xml); - qemudDomainSaveImageClose(fd, read_pid, NULL); + VIR_FORCE_CLOSE(fd); return -1; } -static int ATTRIBUTE_NONNULL(6) -qemudDomainSaveImageStartVM(virConnectPtr conn, - struct qemud_driver *driver, - virDomainObjPtr vm, - int *fd, - pid_t *read_pid, - const struct qemud_save_header *header, - const char *path) +static int ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6) +qemuDomainSaveImageStartVM(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + int *fd, + const struct qemud_save_header *header, + const char *path) { int ret = -1; virDomainEventPtr event; int intermediatefd = -1; pid_t intermediate_pid = -1; int childstat; - int wait_ret; - int status; if (header->version == 2) { const char *intermediate_argv[3] = { NULL, "-dc", NULL }; @@ -3299,8 +3144,9 @@ qemudDomainSaveImageStartVM(virConnectPtr conn, if (intermediate_pid != -1) { if (ret < 0) { - /* if there was an error setting up qemu, the intermediate process will - * wait forever to write to stdout, so we must manually kill it. + /* if there was an error setting up qemu, the intermediate + * process will wait forever to write to stdout, so we + * must manually kill it. */ VIR_FORCE_CLOSE(intermediatefd); VIR_FORCE_CLOSE(*fd); @@ -3315,30 +3161,10 @@ qemudDomainSaveImageStartVM(virConnectPtr conn, } VIR_FORCE_CLOSE(intermediatefd); - wait_ret = qemudDomainSaveImageClose(*fd, *read_pid, &status); - *fd = -1; - if (*read_pid != -1) { - if (wait_ret == -1) { - virReportSystemError(errno, - _("failed to wait for process reading '%s'"), - path); - ret = -1; - } else if (!WIFEXITED(status)) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - _("child process exited abnormally reading '%s'"), - path); - ret = -1; - } else { - int exit_status = WEXITSTATUS(status); - if (exit_status != 0) { - virReportSystemError(exit_status, - _("child process returned error reading '%s'"), - path); - ret = -1; - } - } + if (VIR_CLOSE(*fd) < 0) { + virReportSystemError(errno, _("cannot close file: %s"), path); + ret = -1; } - *read_pid = -1; if (ret < 0) { qemuDomainStartAudit(vm, "restored", false); @@ -3377,19 +3203,20 @@ out: return ret; } -static int qemudDomainRestore(virConnectPtr conn, - const char *path) { +static int +qemuDomainRestore(virConnectPtr conn, + const char *path) +{ struct qemud_driver *driver = conn->privateData; virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; int fd = -1; - pid_t read_pid = -1; int ret = -1; struct qemud_save_header header; qemuDriverLock(driver); - fd = qemudDomainSaveImageOpen(driver, path, &def, &header, &read_pid); + fd = qemuDomainSaveImageOpen(driver, path, &def, &header); if (fd < 0) goto cleanup; @@ -3407,8 +3234,7 @@ static int qemudDomainRestore(virConnectPtr conn, if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) goto cleanup; - ret = qemudDomainSaveImageStartVM(conn, driver, vm, &fd, - &read_pid, &header, path); + ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path); if (qemuDomainObjEndJob(vm) == 0) vm = NULL; @@ -3419,25 +3245,25 @@ static int qemudDomainRestore(virConnectPtr conn, cleanup: virDomainDefFree(def); - qemudDomainSaveImageClose(fd, read_pid, NULL); + VIR_FORCE_CLOSE(fd); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; } -static int qemudDomainObjRestore(virConnectPtr conn, - struct qemud_driver *driver, - virDomainObjPtr vm, - const char *path) +static int +qemuDomainObjRestore(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + const char *path) { virDomainDefPtr def = NULL; int fd = -1; - pid_t read_pid = -1; int ret = -1; struct qemud_save_header header; - fd = qemudDomainSaveImageOpen(driver, path, &def, &header, &read_pid); + fd = qemuDomainSaveImageOpen(driver, path, &def, &header); if (fd < 0) goto cleanup; @@ -3458,12 +3284,11 @@ static int qemudDomainObjRestore(virConnectPtr conn, virDomainObjAssignDef(vm, def, true); def = NULL; - ret = qemudDomainSaveImageStartVM(conn, driver, vm, &fd, - &read_pid, &header, path); + ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path); cleanup: virDomainDefFree(def); - qemudDomainSaveImageClose(fd, read_pid, NULL); + VIR_FORCE_CLOSE(fd); return ret; } @@ -3674,7 +3499,7 @@ static int qemudDomainObjStart(virConnectPtr conn, */ managed_save = qemuDomainManagedSavePath(driver, vm); if ((managed_save) && (virFileExists(managed_save))) { - ret = qemudDomainObjRestore(conn, driver, vm, managed_save); + ret = qemuDomainObjRestore(conn, driver, vm, managed_save); if (unlink(managed_save) < 0) { VIR_WARN("Failed to remove the managed state %s", managed_save); @@ -6814,7 +6639,7 @@ static virDriver qemuDriver = { qemudDomainSetMemory, /* domainSetMemory */ qemudDomainGetInfo, /* domainGetInfo */ qemudDomainSave, /* domainSave */ - qemudDomainRestore, /* domainRestore */ + qemuDomainRestore, /* domainRestore */ qemudDomainCoreDump, /* domainCoreDump */ qemudDomainSetVcpus, /* domainSetVcpus */ qemudDomainSetVcpusFlags, /* domainSetVcpusFlags */ -- 1.7.4
Separating the indenation from the real patch made review easier. * src/util/util.c (virFileOpenAs): Whitespace changes. --- v3: new patch. 'git diff -b' first, then real patch: diff --git a/src/util/util.c b/src/util/util.c index 137cdb9..8d00ded 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1403,8 +1403,7 @@ static int virDirCreateNoFork(const char *path, mode_t mode, uid_t uid, gid_t gi struct stat st; if ((mkdir(path, mode) < 0) - && !((errno == EEXIST) && (flags & VIR_DIR_CREATE_ALLOW_EXIST))) - { + && !((errno == EEXIST) && (flags & VIR_DIR_CREATE_ALLOW_EXIST))) { ret = -errno; virReportSystemError(errno, _("failed to create directory '%s'"), path); @@ -1480,7 +1479,6 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, * following dance avoids problems caused by root-squashing * NFS servers. */ - { if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0) { ret = -errno; virReportSystemError(errno, @@ -1500,7 +1498,6 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, cmsg->cmsg_level = SOL_SOCKET; cmsg->cmsg_type = SCM_RIGHTS; cmsg->cmsg_len = CMSG_LEN(sizeof(fd)); - } forkRet = virFork(&pid); @@ -1510,7 +1507,6 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, } if (pid) { /* parent */ - { VIR_FORCE_CLOSE(pair[1]); do { @@ -1533,7 +1529,6 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, cmsg->cmsg_type == SCM_RIGHTS) { memcpy(&fd, CMSG_DATA(cmsg), sizeof(fd)); } - } /* wait for child to complete, and retrieve its exit code */ while ((waitret = waitpid(pid, &status, 0) == -1) @@ -1612,7 +1607,6 @@ parenterror: path, mode); goto childerror; } - { VIR_FORCE_CLOSE(pair[0]); memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd)); @@ -1625,7 +1619,6 @@ parenterror: VIR_FORCE_CLOSE(pair[1]); goto childerror; } - } ret = 0; childerror: src/util/util.c | 101 +++++++++++++++++++++++++----------------------------- 1 files changed, 47 insertions(+), 54 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 137cdb9..8d00ded 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1403,8 +1403,7 @@ static int virDirCreateNoFork(const char *path, mode_t mode, uid_t uid, gid_t gi struct stat st; if ((mkdir(path, mode) < 0) - && !((errno == EEXIST) && (flags & VIR_DIR_CREATE_ALLOW_EXIST))) - { + && !((errno == EEXIST) && (flags & VIR_DIR_CREATE_ALLOW_EXIST))) { ret = -errno; virReportSystemError(errno, _("failed to create directory '%s'"), path); @@ -1480,28 +1479,26 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, * following dance avoids problems caused by root-squashing * NFS servers. */ - { - if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0) { - ret = -errno; - virReportSystemError(errno, - _("failed to create socket needed for '%s'"), - path); - return ret; - } - - memset(&msg, 0, sizeof(msg)); - iov.iov_base = &dummy; - iov.iov_len = 1; - msg.msg_iov = &iov; - msg.msg_iovlen = 1; - msg.msg_control = buf; - msg.msg_controllen = sizeof(buf); - cmsg = CMSG_FIRSTHDR(&msg); - cmsg->cmsg_level = SOL_SOCKET; - cmsg->cmsg_type = SCM_RIGHTS; - cmsg->cmsg_len = CMSG_LEN(sizeof(fd)); + if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0) { + ret = -errno; + virReportSystemError(errno, + _("failed to create socket needed for '%s'"), + path); + return ret; } + memset(&msg, 0, sizeof(msg)); + iov.iov_base = &dummy; + iov.iov_len = 1; + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + msg.msg_control = buf; + msg.msg_controllen = sizeof(buf); + cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = CMSG_LEN(sizeof(fd)); + forkRet = virFork(&pid); if (pid < 0) { @@ -1510,29 +1507,27 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, } if (pid) { /* parent */ - { - VIR_FORCE_CLOSE(pair[1]); - - do { - ret = recvmsg(pair[0], &msg, 0); - } while (ret < 0 && errno == EINTR); - - if (ret < 0) { - ret = -errno; - VIR_FORCE_CLOSE(pair[0]); - while ((waitret = waitpid(pid, NULL, 0) == -1) - && (errno == EINTR)); - goto parenterror; - } + VIR_FORCE_CLOSE(pair[1]); + + do { + ret = recvmsg(pair[0], &msg, 0); + } while (ret < 0 && errno == EINTR); + + if (ret < 0) { + ret = -errno; VIR_FORCE_CLOSE(pair[0]); + while ((waitret = waitpid(pid, NULL, 0) == -1) + && (errno == EINTR)); + goto parenterror; + } + VIR_FORCE_CLOSE(pair[0]); - /* See if fd was transferred. */ - cmsg = CMSG_FIRSTHDR(&msg); - if (cmsg && cmsg->cmsg_len == CMSG_LEN(sizeof(fd)) && - cmsg->cmsg_level == SOL_SOCKET && - cmsg->cmsg_type == SCM_RIGHTS) { - memcpy(&fd, CMSG_DATA(cmsg), sizeof(fd)); - } + /* See if fd was transferred. */ + cmsg = CMSG_FIRSTHDR(&msg); + if (cmsg && cmsg->cmsg_len == CMSG_LEN(sizeof(fd)) && + cmsg->cmsg_level == SOL_SOCKET && + cmsg->cmsg_type == SCM_RIGHTS) { + memcpy(&fd, CMSG_DATA(cmsg), sizeof(fd)); } /* wait for child to complete, and retrieve its exit code */ @@ -1612,19 +1607,17 @@ parenterror: path, mode); goto childerror; } - { - VIR_FORCE_CLOSE(pair[0]); - memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd)); + VIR_FORCE_CLOSE(pair[0]); + memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd)); - do { - ret = sendmsg(pair[1], &msg, 0); - } while (ret < 0 && errno == EINTR); + do { + ret = sendmsg(pair[1], &msg, 0); + } while (ret < 0 && errno == EINTR); - if (ret < 0) { - ret = -errno; - VIR_FORCE_CLOSE(pair[1]); - goto childerror; - } + if (ret < 0) { + ret = -errno; + VIR_FORCE_CLOSE(pair[1]); + goto childerror; } ret = 0; -- 1.7.4
* src/storage/storage_backend.c (createRawFileOpHook): Change signature. (struct createRawFileOpHookData): Delete unused struct. (virStorageBackendCreateRaw): Adjust caller. * src/qemu/qemu_driver.c (struct fileOpHookData): Delete unused struct. (qemudDomainSaveFileOpHook): Rename... (qemuDomainSaveFileOpHook): ...and change signature. (qemudDomainSaveFlag): Adjust caller. --- v3: new patch src/qemu/qemu_driver.c | 29 +++++++++-------------------- src/storage/storage_backend.c | 32 ++++++++++++-------------------- 2 files changed, 21 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a222309..6eee7d6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1742,30 +1742,25 @@ struct qemud_save_header { int unused[15]; }; -struct fileOpHookData { - virDomainPtr dom; - const char *path; - char *xml; - struct qemud_save_header *header; -}; - /* return -errno on failure, or 0 on success */ -static int qemudDomainSaveFileOpHook(int fd, void *data) { - struct fileOpHookData *hdata = data; +static int +qemuDomainSaveFileOpHook(int fd, const char *path, char *xml, + struct qemud_save_header *header) +{ int ret = 0; - if (safewrite(fd, hdata->header, sizeof(*hdata->header)) != sizeof(*hdata->header)) { + if (safewrite(fd, header, sizeof(*header)) != sizeof(*header)) { ret = -errno; qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to write header to domain save file '%s'"), - hdata->path); + path); goto endjob; } - if (safewrite(fd, hdata->xml, hdata->header->xml_len) != hdata->header->xml_len) { + if (safewrite(fd, xml, header->xml_len) != header->xml_len) { ret = -errno; qemuReportError(VIR_ERR_OPERATION_FAILED, - _("failed to write xml to '%s'"), hdata->path); + _("failed to write xml to '%s'"), path); goto endjob; } endjob: @@ -1781,7 +1776,6 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, { char *xml = NULL; struct qemud_save_header header; - struct fileOpHookData hdata; int bypassSecurityDriver = 0; int ret = -1; int rc; @@ -1947,12 +1941,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, } /* Write header to file, followed by XML */ - hdata.dom = dom; - hdata.path = path; - hdata.xml = xml; - hdata.header = &header; - - if (qemudDomainSaveFileOpHook(fd, &hdata) < 0) { + if (qemuDomainSaveFileOpHook(fd, path, xml, &header) < 0) { VIR_FORCE_CLOSE(fd); goto endjob; } diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 5e8d94a..0876cd4 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -266,31 +266,25 @@ cleanup: return ret; } -struct createRawFileOpHookData { - virStorageVolDefPtr vol; - virStorageVolDefPtr inputvol; -}; - -static int createRawFileOpHook(int fd, void *data) { - struct createRawFileOpHookData *hdata = data; +static int createRawFileOpHook(int fd, virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol) { int ret = 0; unsigned long long remain; /* Seek to the final size, so the capacity is available upfront * for progress reporting */ - if (ftruncate(fd, hdata->vol->capacity) < 0) { + if (ftruncate(fd, vol->capacity) < 0) { ret = -errno; virReportSystemError(errno, _("cannot extend file '%s'"), - hdata->vol->target.path); + vol->target.path); goto cleanup; } - remain = hdata->vol->allocation; + remain = vol->allocation; - if (hdata->inputvol) { - ret = virStorageBackendCopyToFD(hdata->vol, hdata->inputvol, - fd, &remain, 1); + if (inputvol) { + ret = virStorageBackendCopyToFD(vol, inputvol, fd, &remain, 1); if (ret < 0) { goto cleanup; } @@ -308,11 +302,10 @@ static int createRawFileOpHook(int fd, void *data) { if (bytes > remain) bytes = remain; - if (safezero(fd, 0, hdata->vol->allocation - remain, - bytes) != 0) { + if (safezero(fd, 0, vol->allocation - remain, bytes) != 0) { ret = -errno; virReportSystemError(errno, _("cannot fill file '%s'"), - hdata->vol->target.path); + vol->target.path); goto cleanup; } remain -= bytes; @@ -321,7 +314,7 @@ static int createRawFileOpHook(int fd, void *data) { if (safezero(fd, 0, 0, remain) != 0) { ret = -errno; virReportSystemError(errno, _("cannot fill file '%s'"), - hdata->vol->target.path); + vol->target.path); goto cleanup; } } @@ -331,7 +324,7 @@ static int createRawFileOpHook(int fd, void *data) { if (fsync(fd) < 0) { ret = -errno; virReportSystemError(errno, _("cannot sync data to file '%s'"), - hdata->vol->target.path); + vol->target.path); goto cleanup; } @@ -348,7 +341,6 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, { int ret = -1; int fd = -1; - struct createRawFileOpHookData hdata = { vol, inputvol }; uid_t uid; gid_t gid; @@ -377,7 +369,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } - if ((ret = createRawFileOpHook(fd, &hdata)) < 0) { + if ((ret = createRawFileOpHook(fd, vol, inputvol)) < 0) { virReportSystemError(-fd, _("cannot create path '%s'"), vol->target.path); -- 1.7.4
On 03/04/2011 01:37 AM, Eric Blake wrote:
* src/storage/storage_backend.c (createRawFileOpHook): Change signature. (struct createRawFileOpHookData): Delete unused struct. (virStorageBackendCreateRaw): Adjust caller. * src/qemu/qemu_driver.c (struct fileOpHookData): Delete unused struct. (qemudDomainSaveFileOpHook): Rename... (qemuDomainSaveFileOpHook): ...and change signature. (qemudDomainSaveFlag): Adjust caller.
Might as well rename the functions to e.g. initializeRawFile and qemuDomainWriteSaveHeader? They are not hooks anymore. Paolo
POSIX states about dd: If the bs=expr operand is specified and no conversions other than sync, noerror, or notrunc are requested, the data returned from each input block shall be written as a separate output block; if the read returns less than a full block and the sync conversion is not specified, the resulting output block shall be the same size as the input block. If the bs=expr operand is not specified, or a conversion other than sync, noerror, or notrunc is requested, the input shall be processed and collected into full-sized output blocks until the end of the input is reached. Since we aren't using conv=sync, there is no zero-padding, but our use of bs= means that a short read results in a short write. If instead we use ibs= and obs=, then short reads are collected and dd only has to do a single write, which can make dd more efficient. * src/qemu/qemu_monitor_text.c (qemuMonitorTextMigrateToFile): Avoid 'dd bs=', since it can cause short writes. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONMigrateToFile): Likewise. --- no changes in v3: I've had this one sitting around for a while; technically, this is less important now that we're avoiding exec: with newer qemu, but it is still useful for qemu 0.11.0. src/qemu/qemu_monitor_json.c | 3 ++- src/qemu/qemu_monitor_text.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 81201ff..5fb43b0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1794,10 +1794,11 @@ int qemuMonitorJSONMigrateToFile(qemuMonitorPtr mon, * <> redirection to avoid truncating a regular file. */ if (virAsprintf(&dest, "exec:" VIR_WRAPPER_SHELL_PREFIX "%s | " "{ dd bs=%llu seek=%llu if=/dev/null && " - "dd bs=%llu; } 1<>%s" VIR_WRAPPER_SHELL_SUFFIX, + "dd ibs=%llu obs=%llu; } 1<>%s" VIR_WRAPPER_SHELL_SUFFIX, argstr, QEMU_MONITOR_MIGRATE_TO_FILE_BS, offset / QEMU_MONITOR_MIGRATE_TO_FILE_BS, QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE, + QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE, safe_target) < 0) { virReportOOMError(); goto cleanup; diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 76e0ec0..3c9f799 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1369,10 +1369,11 @@ int qemuMonitorTextMigrateToFile(qemuMonitorPtr mon, * <> redirection to avoid truncating a regular file. */ if (virAsprintf(&dest, "exec:" VIR_WRAPPER_SHELL_PREFIX "%s | " "{ dd bs=%llu seek=%llu if=/dev/null && " - "dd bs=%llu; } 1<>%s" VIR_WRAPPER_SHELL_SUFFIX, + "dd ibs=%llu obs=%llu; } 1<>%s" VIR_WRAPPER_SHELL_SUFFIX, argstr, QEMU_MONITOR_MIGRATE_TO_FILE_BS, offset / QEMU_MONITOR_MIGRATE_TO_FILE_BS, QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE, + QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE, safe_target) < 0) { virReportOOMError(); goto cleanup; -- 1.7.4
participants (2)
-
Eric Blake -
Paolo Bonzini