[libvirt] [PATCH 0/7] patch queue cleanup
I've been building up several (unrelated) patches that haven't been reviewed yet; here they are in no particular order except that 6 depends on 5, and that 4 depends on 3. Eric Blake (6): datatypes: avoid redundant __FUNCTION__ maint: improve sc_prohibit_strncmp syntax check build: let xgettext see strings in libvirt-guests qemu: use -incoming fd:n to avoid qemu holding fd indefinitely qemu: use lighter-weight fd:n on incoming tunneled migration qemu: improve efficiency of dd during snapshots Laurent Léonard (1): libvirt-guests: remove bashisms .gnulib | 2 +- .x-sc_prohibit_strcmp | 1 - .x-sc_prohibit_strncmp | 1 - Makefile.am | 2 - cfg.mk | 7 +- po/POTFILES.in | 1 + src/datatypes.c | 129 +++++++++++++++----- src/qemu/qemu_command.c | 82 ++++++++----- src/qemu/qemu_command.h | 1 + src/qemu/qemu_driver.c | 56 +++------ src/qemu/qemu_monitor_json.c | 5 +- src/qemu/qemu_monitor_text.c | 5 +- .../qemuxml2argv-restore-v2-fd.args | 1 + .../qemuxml2argv-restore-v2-fd.xml | 25 ++++ tests/qemuxml2argvtest.c | 30 +++-- tools/Makefile.am | 13 ++- ...bvirt-guests.init.in => libvirt-guests.init.sh} | 72 +++++++----- 17 files changed, 279 insertions(+), 154 deletions(-) delete mode 100644 .x-sc_prohibit_strcmp delete mode 100644 .x-sc_prohibit_strncmp create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml rename tools/{libvirt-guests.init.in => libvirt-guests.init.sh} (76%) -- 1.7.3.4
virLibConnError already includes __FUNCTION__ in its output, so we were redundant. Furthermore, clang warns that __FUNCTION__ is not a string literal (at least __FUNCTION__ will never contain %, so it was not a security risk). * src/datatypes.c: Replace __FUNCTION__ with a descriptive string. --- Wow, I've been sitting on this one for a while (commit date of May 2010). src/datatypes.c | 129 +++++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 98 insertions(+), 31 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index 9817538..855bd95 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -1,7 +1,7 @@ /* * datatypes.h: management of structs for public data types * - * Copyright (C) 2006-2010 Red Hat, Inc. + * Copyright (C) 2006-2011 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -310,7 +310,7 @@ virUnrefConnect(virConnectPtr conn) { int refs; if ((!VIR_IS_CONNECT(conn))) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection")); return(-1); } virMutexLock(&conn->lock); @@ -344,8 +344,16 @@ virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) { virDomainPtr ret = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection")); + return(NULL); + } + if (name == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing name")); + return(NULL); + } + if (uuid == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing uuid")); return(NULL); } virMutexLock(&conn->lock); @@ -455,7 +463,7 @@ virUnrefDomain(virDomainPtr domain) { int refs; if (!VIR_IS_CONNECTED_DOMAIN(domain)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("bad domain or no connection")); return(-1); } virMutexLock(&domain->conn->lock); @@ -490,8 +498,16 @@ virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) { virNetworkPtr ret = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection")); + return(NULL); + } + if (name == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing name")); + return(NULL); + } + if (uuid == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing uuid")); return(NULL); } virMutexLock(&conn->lock); @@ -593,7 +609,8 @@ virUnrefNetwork(virNetworkPtr network) { int refs; if (!VIR_IS_CONNECTED_NETWORK(network)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, + _("bad network or no connection")); return(-1); } virMutexLock(&network->conn->lock); @@ -629,8 +646,12 @@ virInterfacePtr virGetInterface(virConnectPtr conn, const char *name, const char *mac) { virInterfacePtr ret = NULL; - if ((!VIR_IS_CONNECT(conn)) || (name == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection")); + return(NULL); + } + if (name == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing name")); return(NULL); } @@ -775,7 +796,8 @@ virUnrefInterface(virInterfacePtr iface) { int refs; if (!VIR_IS_CONNECTED_INTERFACE(iface)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, + _("bad interface or no connection")); return(-1); } virMutexLock(&iface->conn->lock); @@ -807,12 +829,21 @@ virUnrefInterface(virInterfacePtr iface) { * Returns a pointer to the network, or NULL in case of failure */ virStoragePoolPtr -virGetStoragePool(virConnectPtr conn, const char *name, const unsigned char *uuid) { +virGetStoragePool(virConnectPtr conn, const char *name, + const unsigned char *uuid) { virStoragePoolPtr ret = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection")); + return(NULL); + } + if (name == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing name")); + return(NULL); + } + if (uuid == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing uuid")); return(NULL); } virMutexLock(&conn->lock); @@ -915,7 +946,8 @@ virUnrefStoragePool(virStoragePoolPtr pool) { int refs; if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, + _("bad storage pool or no connection")); return(-1); } virMutexLock(&pool->conn->lock); @@ -948,11 +980,20 @@ virUnrefStoragePool(virStoragePoolPtr pool) { * Returns a pointer to the storage vol, or NULL in case of failure */ virStorageVolPtr -virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const char *key) { +virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, + const char *key) { virStorageVolPtr ret = NULL; - if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (key == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection")); + return(NULL); + } + if (name == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing name")); + return(NULL); + } + if (key == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing key")); return(NULL); } virMutexLock(&conn->lock); @@ -1063,7 +1104,8 @@ virUnrefStorageVol(virStorageVolPtr vol) { int refs; if (!VIR_IS_CONNECTED_STORAGE_VOL(vol)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, + _("bad storage volume or no connection")); return(-1); } virMutexLock(&vol->conn->lock); @@ -1098,8 +1140,12 @@ virGetNodeDevice(virConnectPtr conn, const char *name) { virNodeDevicePtr ret = NULL; - if ((!VIR_IS_CONNECT(conn)) || (name == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection")); + return(NULL); + } + if (name == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing name")); return(NULL); } virMutexLock(&conn->lock); @@ -1230,9 +1276,17 @@ virGetSecret(virConnectPtr conn, const unsigned char *uuid, virSecretPtr ret = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (!VIR_IS_CONNECT(conn) || uuid == NULL || usageID == NULL) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return NULL; + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection")); + return(NULL); + } + if (uuid == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing uuid")); + return(NULL); + } + if (usageID == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing usageID")); + return(NULL); } virMutexLock(&conn->lock); @@ -1330,7 +1384,7 @@ virUnrefSecret(virSecretPtr secret) { int refs; if (!VIR_IS_CONNECTED_SECRET(secret)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("bad secret or no connection")); return -1; } virMutexLock(&secret->conn->lock); @@ -1424,8 +1478,16 @@ virGetNWFilter(virConnectPtr conn, const char *name, const unsigned char *uuid) virNWFilterPtr ret = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection")); + return(NULL); + } + if (name == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing name")); + return(NULL); + } + if (uuid == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing uuid")); return(NULL); } virMutexLock(&conn->lock); @@ -1528,7 +1590,8 @@ virUnrefNWFilter(virNWFilterPtr pool) { int refs; if (!VIR_IS_CONNECTED_NWFILTER(pool)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, + _("bad nwfilter or no connection")); return -1; } virMutexLock(&pool->conn->lock); @@ -1551,8 +1614,12 @@ virGetDomainSnapshot(virDomainPtr domain, const char *name) { virDomainSnapshotPtr ret = NULL; - if ((!VIR_IS_DOMAIN(domain)) || (name == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!VIR_IS_DOMAIN(domain)) { + virLibConnError(VIR_ERR_INVALID_ARG, _("bad domain")); + return(NULL); + } + if (name == NULL) { + virLibConnError(VIR_ERR_INVALID_ARG, _("missing name")); return(NULL); } virMutexLock(&domain->conn->lock); @@ -1632,7 +1699,7 @@ virUnrefDomainSnapshot(virDomainSnapshotPtr snapshot) int refs; if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virLibConnError(VIR_ERR_INVALID_ARG, _("not a snapshot")); return(-1); } -- 1.7.3.4
2011/1/14 Eric Blake <eblake@redhat.com>:
virLibConnError already includes __FUNCTION__ in its output, so we were redundant. Furthermore, clang warns that __FUNCTION__ is not a string literal (at least __FUNCTION__ will never contain %, so it was not a security risk).
@@ -344,8 +344,16 @@ virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) { virDomainPtr ret = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN];
- if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection")); + return(NULL); + }
Small cosmetic nit: "return isn't a function". I know you just keep in line with the existing style in this file :) ACK. Matthias
On 01/14/2011 02:45 PM, Matthias Bolte wrote:
2011/1/14 Eric Blake <eblake@redhat.com>:
virLibConnError already includes __FUNCTION__ in its output, so we were redundant. Furthermore, clang warns that __FUNCTION__ is not a string literal (at least __FUNCTION__ will never contain %, so it was not a security risk).
@@ -344,8 +344,16 @@ virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) { virDomainPtr ret = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN];
- if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) { - virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_ARG, _("no connection")); + return(NULL); + }
Small cosmetic nit: "return isn't a function". I know you just keep in line with the existing style in this file :)
ACK.
I removed the spurious () on any return in a hunk within my original patch, then pushed (there are still bogus return() in the rest of the file, but that can be for another day). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* .gnulib: Update, for sc_prohibit_strcmp fix. * cfg.mk: Adjust copyright; the only FSF portions come from when this file was copied from coreutils. (sc_prohibit_strncmp): Copy bug-fixes from sc_prohibit_strcmp. * .x-sc_prohibit_strcmp: Delete, now that rule is smarter. * .x-sc_prohibit_strncmp: Likewise. * Makefile.am (syntax_check_exceptions): Track deletion. --- The current list of gnulib differences: * .gnulib 4f2c339...9779055 (40):
filemode: Make function declarations usable in C++ mode. save-cwd: no longer include "xgetcwd.h" ftoastr: split into 3 modules ftoastr, dtoastr, ldtoastr save-cwd: remove #if-!HAVE_FCHDIR'd code; use the fchdir module openat, save-cwd: avoid xmalloc openat: Increase OPENAT_BUFFER_SIZE from 512 to at least 1024 doc: Update users.txt. missing @item document configmake in the manual instead of the source Update to Unicode 6.0.0. Update to Unicode 5.2.0. New Unicode character properties, from Unicode 5.2.0. New module 'unictype/property-changes-when-casemapped'. New module 'unictype/property-changes-when-casefolded'. New module 'unictype/property-changes-when-titlecased'. New module 'unictype/property-changes-when-uppercased'. New module 'unictype/property-changes-when-lowercased'. New module 'unictype/property-case-ignorable'. New module 'unictype/property-cased'. Update to Unicode 5.2.0. useless-if-before-free: fix typo in --help and make the internal, uniwidth/width: Fix width of U+1D173..U+1D17A. uninorm tests: Preserve copyright of Unicode data file. gen-uni-tables: Oops, fix last commit. gen-uni-tables: Prepare for Unicode 5.2.0. unilbrk: Clarify gen-uni-tables.c code. strtod: Restore errno when successfully parsing Infinity or NaN. remove test: Avoid failure on HP-UX 11. mkdir, mkdirat tests: Avoid failure on HP-UX 11.11. ignore-value: clarify some comments ignore-value: support aggregate types maint.mk: improve sc_prohibit_strcmp regex maint: fix ChangeLog order signal: work around Haiku issue with SIGBUS maint.mk: add pre-release check to ensure submodule commits are public ignore-value: make ignore_value more generic; deprecate ignore_ptr doc: regenerate INSTALL Merge remote branch 'origin/coreutils-8.9' Merge branch 'coreutils-8.9' avoid an unnecessary sub-shell
.gnulib | 2 +- .x-sc_prohibit_strcmp | 1 - .x-sc_prohibit_strncmp | 1 - Makefile.am | 2 -- cfg.mk | 7 ++++--- 5 files changed, 5 insertions(+), 8 deletions(-) delete mode 100644 .x-sc_prohibit_strcmp delete mode 100644 .x-sc_prohibit_strncmp diff --git a/.gnulib b/.gnulib index 4f2c339..9779055 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 4f2c339efdaf1fcca9eed7b0700083b5e24942d4 +Subproject commit 9779055889c2715b593930e39ead552759b5ddc2 diff --git a/.x-sc_prohibit_strcmp b/.x-sc_prohibit_strcmp deleted file mode 100644 index b7c456e..0000000 --- a/.x-sc_prohibit_strcmp +++ /dev/null @@ -1 +0,0 @@ -^gnulib/ diff --git a/.x-sc_prohibit_strncmp b/.x-sc_prohibit_strncmp deleted file mode 100644 index 8be2055..0000000 --- a/.x-sc_prohibit_strncmp +++ /dev/null @@ -1 +0,0 @@ -^src/internal\.h$ diff --git a/Makefile.am b/Makefile.am index c525e65..36463f5 100644 --- a/Makefile.am +++ b/Makefile.am @@ -33,8 +33,6 @@ syntax_check_exceptions = \ .x-sc_prohibit_nonreentrant \ .x-sc_prohibit_readlink \ .x-sc_prohibit_sprintf \ - .x-sc_prohibit_strcmp \ - .x-sc_prohibit_strncmp \ .x-sc_prohibit_strncpy \ .x-sc_prohibit_test_minus_ao \ .x-sc_prohibit_VIR_ERR_NO_MEMORY \ diff --git a/cfg.mk b/cfg.mk index d4c791a..d4c593f 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1,5 +1,6 @@ # Customize Makefile.maint. -*- makefile -*- -# Copyright (C) 2003-2010 Free Software Foundation, Inc. +# Copyright (C) 2008-2011 Red Hat, Inc. +# Copyright (C) 2003-2008 Free Software Foundation, Inc. # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -250,9 +251,9 @@ sc_prohibit_close: # Similar to the gnulib maint.mk rule for sc_prohibit_strcmp # Use STREQLEN or STRPREFIX rather than comparing strncmp == 0, or != 0. sc_prohibit_strncmp: - @grep -nE '! *str''ncmp *\(|\<str''ncmp *\([^)]+\) *==' \ + @grep -nE '! *str''ncmp *\(|\<str''ncmp *\(.+\) *[!=]=' \ $$($(VC_LIST_EXCEPT)) \ - | grep -vE ':# *define STREQ\(' && \ + | grep -vE ':# *define STR(N?EQLEN|PREFIX)\(' && \ { echo '$(ME): use STREQLEN or STRPREFIX instead of str''ncmp' \ 1>&2; exit 1; } || : -- 1.7.3.4
2011/1/14 Eric Blake <eblake@redhat.com>:
* .gnulib: Update, for sc_prohibit_strcmp fix. * cfg.mk: Adjust copyright; the only FSF portions come from when this file was copied from coreutils. (sc_prohibit_strncmp): Copy bug-fixes from sc_prohibit_strcmp. * .x-sc_prohibit_strcmp: Delete, now that rule is smarter. * .x-sc_prohibit_strncmp: Likewise. * Makefile.am (syntax_check_exceptions): Track deletion. ---
ACK. Matthias
On 01/14/2011 03:01 PM, Matthias Bolte wrote:
2011/1/14 Eric Blake <eblake@redhat.com>:
* .gnulib: Update, for sc_prohibit_strcmp fix. * cfg.mk: Adjust copyright; the only FSF portions come from when this file was copied from coreutils. (sc_prohibit_strncmp): Copy bug-fixes from sc_prohibit_strcmp. * .x-sc_prohibit_strcmp: Delete, now that rule is smarter. * .x-sc_prohibit_strncmp: Likewise. * Makefile.am (syntax_check_exceptions): Track deletion. ---
ACK.
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
From: Laurent Léonard <laurent@open-minds.org> * tools/libvirt-guests.init.sh: Use only POSIX shell features, which includes using gettext.sh for translation rather than $"". * tools/Makefile.am (libvirt-guests.init): Supply a few more substitutions. * po/POTFILES.in: Mark that libvirt-guests.init needs translation. Signed-off-by: Eric Blake <eblake@redhat.com> --- po/POTFILES.in | 1 + tools/Makefile.am | 9 +++-- tools/libvirt-guests.init.in | 72 ++++++++++++++++++++++++----------------- 3 files changed, 49 insertions(+), 33 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 3521ba6..5babd90 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -118,4 +118,5 @@ src/xen/xs_internal.c src/xenapi/xenapi_driver.c src/xenapi/xenapi_utils.c tools/console.c +tools/libvirt-guests.init.in tools/virsh.c diff --git a/tools/Makefile.am b/tools/Makefile.am index 271c11b..87cf9bd 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -146,9 +146,12 @@ BUILT_SOURCES += libvirt-guests.init libvirt-guests.init: libvirt-guests.init.in $(top_builddir)/config.status $(AM_V_GEN)sed \ - -e s!\@localstatedir\@!@localstatedir@!g \ - -e s!\@sbindir\@!@sbindir@!g \ - -e s!\@sysconfdir\@!@sysconfdir@!g \ + -e 's!\@PACKAGE\@!$(PACKAGE)!g' \ + -e 's!\@bindir\@!$(bindir)!g' \ + -e 's!\@localedir\@!$(localedir)!g' \ + -e 's!\@localstatedir\@!$(localstatedir)!g' \ + -e 's!\@sbindir\@!$(sbindir)!g' \ + -e 's!\@sysconfdir\@!$(sysconfdir)!g' \ < $< > $@-t && \ chmod a+x $@-t && \ mv $@-t $@ diff --git a/tools/libvirt-guests.init.in b/tools/libvirt-guests.init.in index e28938d..8823d06 100644 --- a/tools/libvirt-guests.init.in +++ b/tools/libvirt-guests.init.in @@ -24,20 +24,27 @@ # See http://libvirt.org # -sysconfdir=@sysconfdir@ -localstatedir=@localstatedir@ -libvirtd=@sbindir@/libvirtd +sysconfdir="@sysconfdir@" +localstatedir="@localstatedir@" +libvirtd="@sbindir@"/libvirtd # Source function library. test ! -r "$sysconfdir"/rc.d/init.d/functions || - . "$sysconfdir"/rc.d/init.d/functions + . "$sysconfdir"/rc.d/init.d/functions + +# Source gettext library. +# Make sure this file is recognized as having translations: _("dummy") +. "@bindir@"/gettext.sh + +export TEXTDOMAIN="@PACKAGE@" TEXTDOMAINDIR="@localedir@" URIS=default ON_BOOT=start ON_SHUTDOWN=suspend SHUTDOWN_TIMEOUT=0 -test -f "$sysconfdir"/sysconfig/libvirt-guests && . "$sysconfdir"/sysconfig/libvirt-guests +test -f "$sysconfdir"/sysconfig/libvirt-guests && + . "$sysconfdir"/sysconfig/libvirt-guests LISTFILE="$localstatedir"/lib/libvirt/libvirt-guests VAR_SUBSYS_LIBVIRT_GUESTS="$localstatedir"/lock/subsys/libvirt-guests @@ -129,7 +136,8 @@ start() { [ -f "$LISTFILE" ] || { started; return 0; } if [ "x$ON_BOOT" != xstart ]; then - echo $"libvirt-guests is configured not to start any guests on boot" + gettext "libvirt-guests is configured not to start any guests on boot" + echo rm -f "$LISTFILE" started return 0 @@ -144,20 +152,20 @@ start() { fi done if ! $configured; then - echo $"Ignoring guests on $uri URI" + eval_gettext "Ignoring guests on \$uri URI"; echo continue fi - echo $"Resuming guests on $uri URI..." + eval_gettext "Resuming guests on \$uri URI..."; echo for guest in $list; do name=$(guest_name $uri $guest) - echo -n $"Resuming guest $name: " + eval_gettext "Resuming guest \$name: " if guest_is_on $uri $guest; then if $guest_running; then - echo $"already active" + gettext "already active"; echo else retval run_virsh $uri start "$name" >/dev/null && \ - echo $"done" + gettext "done"; echo fi fi done @@ -173,8 +181,8 @@ suspend_guest() guest=$2 name=$(guest_name $uri $guest) - label=$"Suspending $name: " - echo -n "$label" + label=$(eval_gettext "Suspending \$name: ") + printf %s "$label" run_virsh $uri managedsave $guest >/dev/null & virsh_pid=$! while true; do @@ -188,7 +196,7 @@ suspend_guest() printf '\r%s%-12s ' "$label" "..." fi done - retval wait $virsh_pid && printf '\r%s%-12s\n' "$label" $"done" + retval wait $virsh_pid && printf '\r%s%-12s\n' "$label" "$(gettext "done")" } shutdown_guest() @@ -197,13 +205,13 @@ shutdown_guest() guest=$2 name=$(guest_name $uri $guest) - label=$"Shutting down $name: " - echo -n "$label" + label=$(eval_gettext "Shutting down \$name: ") + printf %s "$label" retval run_virsh $uri shutdown $guest >/dev/null || return timeout=$SHUTDOWN_TIMEOUT while [ $timeout -gt 0 ]; do sleep 1 - timeout=$[timeout - 1] + timeout=$((timeout - 1)) guest_is_on $uri $guest || return $guest_running || break printf '\r%s%-12d ' "$label" $timeout @@ -211,9 +219,10 @@ shutdown_guest() if guest_is_on $uri $guest; then if $guest_running; then - printf '\r%s%-12s\n' "$label" $"failed to shutdown in time" + printf '\r%s%-12s\n' "$label" \ + "$(gettext "failed to shutdown in time")" else - printf '\r%s%-12s\n' "$label" $"done" + printf '\r%s%-12s\n' "$label" "$(gettext "done")" fi fi } @@ -226,7 +235,8 @@ stop() { if [ "x$ON_SHUTDOWN" = xshutdown ]; then suspending=false if [ $SHUTDOWN_TIMEOUT -le 0 ]; then - echo $"Shutdown action requested but SHUTDOWN_TIMEOUT was not set" + gettext "Shutdown action requested but SHUTDOWN_TIMEOUT was not set" + echo RETVAL=6 return fi @@ -234,10 +244,10 @@ stop() { : >"$LISTFILE" for uri in $URIS; do - echo -n $"Running guests on $uri URI: " + eval_gettext "Running guests on \$uri URI: " if [ "x$uri" = xdefault ] && [ ! -x "$libvirtd" ]; then - echo $"libvirtd not installed; skipping this URI." + gettext "libvirtd not installed; skipping this URI."; echo continue fi @@ -246,11 +256,11 @@ stop() { empty=true for uuid in $list; do $empty || printf ", " - echo -n $(guest_name $uri $uuid) + printf %s "$(guest_name $uri $uuid)" empty=false done if $empty; then - echo $"no running guests." + gettext "no running guests."; echo else echo echo $uri $list >>"$LISTFILE" @@ -260,9 +270,9 @@ stop() { while read uri list; do if $suspending; then - echo $"Suspending guests on $uri URI..." + eval_gettext "Suspending guests on \$uri URI..."; echo else - echo $"Shutting down guests on $uri URI..." + eval_gettext "Shutting down guests on \$uri URI..."; echo fi for guest in $list; do @@ -290,13 +300,13 @@ gueststatus() { # since there is no external daemon process matching this init script. rh_status() { if [ -f "$LISTFILE" ]; then - echo $"stopped, with saved guests" + gettext "stopped, with saved guests"; echo RETVAL=3 else if [ -f "$VAR_SUBSYS_LIBVIRT_GUESTS" ]; then - echo $"started" + gettext "started"; echo else - echo $"stopped, with no saved guests" + gettext "stopped, with no saved guests"; echo fi RETVAL=0 fi @@ -305,7 +315,9 @@ rh_status() { # usage [val] # Display usage string, then exit with VAL (defaults to 2). usage() { - echo $"Usage: $0 {start|stop|status|restart|condrestart|try-restart|reload|force-reload|gueststatus|shutdown}" + program_name=$0 + eval_gettext "Usage: \$program_name {start|stop|status|restart|"\ +"condrestart|try-restart|reload|force-reload|gueststatus|shutdown}"; echo exit ${1-2} } -- 1.7.3.4
2011/1/14 Eric Blake <eblake@redhat.com>:
From: Laurent Léonard <laurent@open-minds.org>
* tools/libvirt-guests.init.sh: Use only POSIX shell features, which includes using gettext.sh for translation rather than $"". * tools/Makefile.am (libvirt-guests.init): Supply a few more substitutions. * po/POTFILES.in: Mark that libvirt-guests.init needs translation.
Signed-off-by: Eric Blake <eblake@redhat.com> --- po/POTFILES.in | 1 + tools/Makefile.am | 9 +++-- tools/libvirt-guests.init.in | 72 ++++++++++++++++++++++++----------------- 3 files changed, 49 insertions(+), 33 deletions(-)
Looks correct, ACK. Matthias
* tools/libvirt-guests.init.in: Rename... * tools/libvirt-guests.init.sh: ...so that xgettext's language detection via suffix will work. * po/POTFILES.in: Update all references. * tools/Makefile.am (EXTRA_DIST, libvirt-guests.init): Likewise. --- po/POTFILES.in | 2 +- tools/Makefile.am | 4 ++-- ...bvirt-guests.init.in => libvirt-guests.init.sh} | 0 3 files changed, 3 insertions(+), 3 deletions(-) rename tools/{libvirt-guests.init.in => libvirt-guests.init.sh} (100%) diff --git a/po/POTFILES.in b/po/POTFILES.in index 5babd90..5f2ed75 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -118,5 +118,5 @@ src/xen/xs_internal.c src/xenapi/xenapi_driver.c src/xenapi/xenapi_utils.c tools/console.c -tools/libvirt-guests.init.in +tools/libvirt-guests.init.sh tools/virsh.c diff --git a/tools/Makefile.am b/tools/Makefile.am index 87cf9bd..f008078 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -13,7 +13,7 @@ EXTRA_DIST = \ virt-xml-validate.in \ virt-pki-validate.in \ virsh.pod \ - libvirt-guests.init.in \ + libvirt-guests.init.sh \ libvirt-guests.sysconf bin_SCRIPTS = virt-xml-validate virt-pki-validate @@ -144,7 +144,7 @@ uninstall-init: BUILT_SOURCES += libvirt-guests.init -libvirt-guests.init: libvirt-guests.init.in $(top_builddir)/config.status +libvirt-guests.init: libvirt-guests.init.sh $(top_builddir)/config.status $(AM_V_GEN)sed \ -e 's!\@PACKAGE\@!$(PACKAGE)!g' \ -e 's!\@bindir\@!$(bindir)!g' \ diff --git a/tools/libvirt-guests.init.in b/tools/libvirt-guests.init.sh similarity index 100% rename from tools/libvirt-guests.init.in rename to tools/libvirt-guests.init.sh -- 1.7.3.4
2011/1/14 Eric Blake <eblake@redhat.com>:
* tools/libvirt-guests.init.in: Rename... * tools/libvirt-guests.init.sh: ...so that xgettext's language detection via suffix will work. * po/POTFILES.in: Update all references. * tools/Makefile.am (EXTRA_DIST, libvirt-guests.init): Likewise. ---
ACK. Matthias
On 01/15/2011 12:49 PM, Matthias Bolte wrote:
2011/1/14 Eric Blake <eblake@redhat.com>:
* tools/libvirt-guests.init.in: Rename... * tools/libvirt-guests.init.sh: ...so that xgettext's language detection via suffix will work. * po/POTFILES.in: Update all references. * tools/Makefile.am (EXTRA_DIST, libvirt-guests.init): Likewise. ---
ACK.
Thanks; I've pushed 3 and 4 now. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
https://bugzilla.redhat.com/show_bug.cgi?id=620363 When using -incoming stdio or -incoming exec:, qemu keeps the stdin fd open long after the migration is complete. Not to mention that exec:cat is horribly inefficient, by doubling the I/O and going through a popen interface in qemu. The new -incoming fd: of qemu 0.12.0 closes the fd after using it, and allows us to bypass an intermediary cat process for less I/O. * src/qemu/qemu_command.h (qemuBuildCommandLine): Add parameter. * src/qemu/qemu_command.c (qemuBuildCommandLine): Support migration via fd: when possible. Consolidate migration handling into one spot, now that it is more complex. * src/qemu/qemu_driver.c (qemudStartVMDaemon): Update caller. * tests/qemuxml2argvtest.c (mymain): Likewise. * tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args: New file. * tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml: Likewise. --- src/qemu/qemu_command.c | 82 +++++++++++++------- src/qemu/qemu_command.h | 1 + src/qemu/qemu_driver.c | 7 +- .../qemuxml2argv-restore-v2-fd.args | 1 + .../qemuxml2argv-restore-v2-fd.xml | 25 ++++++ tests/qemuxml2argvtest.c | 30 +++++-- 6 files changed, 103 insertions(+), 43 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 205c303..ea012ed 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2456,6 +2456,7 @@ qemuBuildCommandLine(virConnectPtr conn, bool monitor_json, unsigned long long qemuCmdFlags, const char *migrateFrom, + int migrateFd, virDomainSnapshotObjPtr current_snapshot, enum virVMOperationType vmop) { @@ -2483,33 +2484,6 @@ qemuBuildCommandLine(virConnectPtr conn, virUUIDFormat(def->uuid, uuid); - /* Migration is very annoying due to wildly varying syntax & capabilities - * over time of KVM / QEMU codebases - */ - if (migrateFrom) { - if (STRPREFIX(migrateFrom, "tcp")) { - if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP)) { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("TCP migration is not supported with this QEMU binary")); - return NULL; - } - } else if (STREQ(migrateFrom, "stdio")) { - if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) { - migrateFrom = "exec:cat"; - } else if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO)) { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("STDIO migration is not supported with this QEMU binary")); - return NULL; - } - } else if (STRPREFIX(migrateFrom, "exec")) { - if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC)) { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("STDIO migration is not supported with this QEMU binary")); - return NULL; - } - } - } - emulator = def->emulator; /* @@ -3897,8 +3871,58 @@ qemuBuildCommandLine(virConnectPtr conn, } } - if (migrateFrom) - virCommandAddArgList(cmd, "-incoming", migrateFrom, NULL); + /* Migration is very annoying due to wildly varying syntax & + * capabilities over time of KVM / QEMU codebases. + */ + if (migrateFrom) { + virCommandAddArg(cmd, "-incoming"); + if (STRPREFIX(migrateFrom, "tcp")) { + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("TCP migration is not supported with " + "this QEMU binary")); + goto error; + } + virCommandAddArg(cmd, migrateFrom); + } else if (STREQ(migrateFrom, "stdio")) { + if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_FD) { + virCommandAddArgFormat(cmd, "fd:%d", migrateFd); + virCommandPreserveFD(cmd, migrateFd); + } else if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) { + virCommandAddArg(cmd, "exec:cat"); + virCommandSetInputFD(cmd, migrateFd); + } else if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO) { + virCommandAddArg(cmd, migrateFrom); + virCommandSetInputFD(cmd, migrateFd); + } else { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("STDIO migration is not supported " + "with this QEMU binary")); + goto error; + } + } else if (STRPREFIX(migrateFrom, "exec")) { + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("EXEC migration is not supported " + "with this QEMU binary")); + goto error; + } + virCommandAddArg(cmd, migrateFrom); + } else if (STRPREFIX(migrateFrom, "fd")) { + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_FD)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("FD migration is not supported " + "with this QEMU binary")); + goto error; + } + virCommandAddArg(cmd, migrateFrom); + virCommandPreserveFD(cmd, migrateFd); + } else { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unknown migration protocol")); + goto error; + } + } /* QEMU changed its default behavior to not include the virtio balloon * device. Explicitly request it to ensure it will be present. diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index b3adc22..7646705 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -44,6 +44,7 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn, bool monitor_json, unsigned long long qemuCmdFlags, const char *migrateFrom, + int migrateFd, virDomainSnapshotObjPtr current_snapshot, enum virVMOperationType vmop) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0eeacaf..a82392e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2777,7 +2777,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, vm->def->id = driver->nextvmid++; if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig, priv->monJSON != 0, qemuCmdFlags, - migrateFrom, + migrateFrom, stdin_fd, vm->current_snapshot, vmop))) goto cleanup; @@ -2827,9 +2827,6 @@ static int qemudStartVMDaemon(virConnectPtr conn, VIR_WARN("Executing %s", vm->def->emulator); virCommandSetPreExecHook(cmd, qemudSecurityHook, &hookData); - if (stdin_fd != -1) - virCommandSetInputFD(cmd, stdin_fd); - virCommandSetOutputFD(cmd, &logfile); virCommandSetErrorFD(cmd, &logfile); virCommandNonblockingFDs(cmd); @@ -6089,7 +6086,7 @@ static char *qemuDomainXMLToNative(virConnectPtr conn, if (!(cmd = qemuBuildCommandLine(conn, driver, def, &monConfig, false, qemuCmdFlags, - NULL, NULL, VIR_VM_OP_NO_OP))) + NULL, -1, NULL, VIR_VM_OP_NO_OP))) goto cleanup; ret = virCommandToString(cmd); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args b/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args new file mode 100644 index 0000000..5464d37 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args @@ -0,0 +1 @@ +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 -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -incoming fd:7 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml b/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml new file mode 100644 index 0000000..ed91e37 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml @@ -0,0 +1,25 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</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'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d7951df..e926db0 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -31,6 +31,7 @@ static int testCompareXMLToArgvFiles(const char *xml, const char *cmdline, unsigned long long extraFlags, const char *migrateFrom, + int migrateFd, bool expectError) { char argvData[MAX_FILE]; char *expectargv = &(argvData[0]); @@ -112,7 +113,8 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, false, flags, - migrateFrom, NULL, VIR_VM_OP_CREATE))) + migrateFrom, migrateFd, NULL, + VIR_VM_OP_CREATE))) goto fail; if (!!virGetLastError() != expectError) { @@ -160,6 +162,7 @@ struct testInfo { const char *name; unsigned long long extraFlags; const char *migrateFrom; + int migrateFd; bool expectError; }; @@ -172,7 +175,8 @@ static int testCompareXMLToArgvHelper(const void *data) { snprintf(args, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.args", abs_srcdir, info->name); return testCompareXMLToArgvFiles(xml, args, info->extraFlags, - info->migrateFrom, info->expectError); + info->migrateFrom, info->migrateFd, + info->expectError); } @@ -217,10 +221,10 @@ mymain(int argc, char **argv) if (cpuMapOverride(map) < 0) return EXIT_FAILURE; -# define DO_TEST_FULL(name, extraFlags, migrateFrom, expectError) \ +# define DO_TEST_FULL(name, extraFlags, migrateFrom, migrateFd, expectError) \ do { \ const struct testInfo info = { \ - name, extraFlags, migrateFrom, expectError \ + name, extraFlags, migrateFrom, migrateFd, expectError \ }; \ if (virtTestRun("QEMU XML-2-ARGV " name, \ 1, testCompareXMLToArgvHelper, &info) < 0) \ @@ -228,7 +232,7 @@ mymain(int argc, char **argv) } while (0) # define DO_TEST(name, extraFlags, expectError) \ - DO_TEST_FULL(name, extraFlags, NULL, expectError) + DO_TEST_FULL(name, extraFlags, NULL, -1, expectError) /* Unset or set all envvars here that are copied in qemudBuildCommandLine * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected @@ -422,10 +426,18 @@ mymain(int argc, char **argv) DO_TEST("hostdev-pci-address-device", QEMUD_CMD_FLAG_PCIDEVICE | QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_NODEFCONFIG, false); - DO_TEST_FULL("restore-v1", QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO, "stdio", false); - DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "stdio", false); - DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat", false); - DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000", false); + DO_TEST_FULL("restore-v1", QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO, "stdio", 7, + false); + DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "stdio", 7, + false); + DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat", 7, + false); + DO_TEST_FULL("restore-v2-fd", QEMUD_CMD_FLAG_MIGRATE_QEMU_FD, "stdio", 7, + false); + DO_TEST_FULL("restore-v2-fd", QEMUD_CMD_FLAG_MIGRATE_QEMU_FD, "fd:7", 7, + false); + DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, + "tcp:10.0.0.1:5000", -1, false); DO_TEST("qemu-ns", 0, false); -- 1.7.3.4
2011/1/14 Eric Blake <eblake@redhat.com>:
https://bugzilla.redhat.com/show_bug.cgi?id=620363
When using -incoming stdio or -incoming exec:, qemu keeps the stdin fd open long after the migration is complete. Not to mention that exec:cat is horribly inefficient, by doubling the I/O and going through a popen interface in qemu.
The new -incoming fd: of qemu 0.12.0 closes the fd after using it, and allows us to bypass an intermediary cat process for less I/O.
* src/qemu/qemu_command.h (qemuBuildCommandLine): Add parameter. * src/qemu/qemu_command.c (qemuBuildCommandLine): Support migration via fd: when possible. Consolidate migration handling into one spot, now that it is more complex. * src/qemu/qemu_driver.c (qemudStartVMDaemon): Update caller. * tests/qemuxml2argvtest.c (mymain): Likewise. * tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args: New file. * tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml: Likewise. ---
ACK. Matthias
On 01/15/2011 03:27 PM, Matthias Bolte wrote:
2011/1/14 Eric Blake <eblake@redhat.com>:
https://bugzilla.redhat.com/show_bug.cgi?id=620363
When using -incoming stdio or -incoming exec:, qemu keeps the stdin fd open long after the migration is complete. Not to mention that exec:cat is horribly inefficient, by doubling the I/O and going through a popen interface in qemu.
The new -incoming fd: of qemu 0.12.0 closes the fd after using it, and allows us to bypass an intermediary cat process for less I/O.
* src/qemu/qemu_command.h (qemuBuildCommandLine): Add parameter. * src/qemu/qemu_command.c (qemuBuildCommandLine): Support migration via fd: when possible. Consolidate migration handling into one spot, now that it is more complex. * src/qemu/qemu_driver.c (qemudStartVMDaemon): Update caller. * tests/qemuxml2argvtest.c (mymain): Likewise. * tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args: New file. * tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml: Likewise. ---
ACK.
Thanks; I've pushed this now. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Outgoing migration still has to use a Unix socket and or exec netcat, since there is no way to pass a migration fd into qemu via monitor commands, but incoming migration need not suffer from the complexity. * src/qemu/qemu_driver.c (qemudDomainMigratePrepareTunnel): Replace Unix socket with simpler pipe. Suggested by Paolo Bonzini. --- I've since been corrected; qemu_monitor.c has qemuMonitorIOWriteWithFD that looks like it can be used to avoid exec: on outgoing migrations by instead passing an open fd over a unix socket, although I have not tested that yet. However, this patch does not need to be held up by waiting for the followup patch that optimizes outgoing migrations. src/qemu/qemu_driver.c | 49 +++++++++++++++-------------------------------- 1 files changed, 16 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a82392e..8cb05b7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7925,11 +7925,10 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, struct qemud_driver *driver = dconn->privateData; virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; - char *migrateFrom; virDomainEventPtr event = NULL; int ret = -1; int internalret; - char *unixfile = NULL; + int dataFD[2] = { -1, -1 }; unsigned long long qemuCmdFlags; qemuDomainObjPrivatePtr priv = NULL; struct timeval now; @@ -7995,39 +7994,27 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, /* 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) { + 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, NULL, &qemuCmdFlags) < 0) { + if (qemuCapsExtractVersionInfo(vm->def->emulator, NULL, + &qemuCmdFlags) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot determine QEMU argv syntax %s"), vm->def->emulator); goto endjob; } - if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX) - internalret = virAsprintf(&migrateFrom, "unix:%s", unixfile); - else if (qemuCmdFlags & QEMUD_CMD_FLAG_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 = qemudStartVMDaemon(dconn, driver, vm, migrateFrom, true, - -1, NULL, VIR_VM_OP_MIGRATE_IN_START); - VIR_FREE(migrateFrom); + internalret = qemudStartVMDaemon(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 qemudStartVMDaemon @@ -8040,9 +8027,7 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, goto endjob; } - if (virFDStreamConnectUNIX(st, - unixfile, - false) < 0) { + if (virFDStreamOpen(st, dataFD[0]) < 0) { qemuDomainStartAudit(vm, "migrated", false); qemudShutdownVMDaemon(driver, vm, 0); if (!vm->persistent) { @@ -8050,9 +8035,8 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, 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; } @@ -8082,9 +8066,8 @@ endjob: cleanup: 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.3.4
On 01/14/2011 09:48 PM, Eric Blake wrote:
Outgoing migration still has to use a Unix socket and or exec netcat, since there is no way to pass a migration fd into qemu via monitor commands, but incoming migration need not suffer from the complexity.
* src/qemu/qemu_driver.c (qemudDomainMigratePrepareTunnel): Replace Unix socket with simpler pipe. Suggested by Paolo Bonzini. ---
I've since been corrected; qemu_monitor.c has qemuMonitorIOWriteWithFD that looks like it can be used to avoid exec: on outgoing migrations by instead passing an open fd over a unix socket, although I have not tested that yet. However, this patch does not need to be held up by waiting for the followup patch that optimizes outgoing migrations.
src/qemu/qemu_driver.c | 49 +++++++++++++++-------------------------------- 1 files changed, 16 insertions(+), 33 deletions(-)
Agreed, ACK. 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. --- src/qemu/qemu_monitor_json.c | 5 +++-- src/qemu/qemu_monitor_text.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7387089..44f4fbc 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 @@ -1751,10 +1751,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 291d958..bae71db 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 @@ -1349,10 +1349,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.3.4
participants (3)
-
Eric Blake -
Matthias Bolte -
Paolo Bonzini