4:27 p.m.
* src/qemu/qemu_capabilities.h (QEMUD_CMD_FLAG_MIGRATE_QEMU_FD):
New enum value.
* src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags): Populate
flags according to qemu version.
* tests/qemuhelptest.c (mymain): Adjust test.
---
I feel a bit dirty relying on just a version test for whether
-incoming fd:n is supported, but it's no different than the
existing code for -incoming unix:path which was added about the
same time. Unfortunately, 'qemu -help' doesn't list which
particular protocols it supports for -incoming.
src/qemu/qemu_capabilities.c | 6 +++++-
src/qemu/qemu_capabilities.h | 1 +
tests/qemuhelptest.c | 12 ++++++++----
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 913fbf7..3d10b42 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -943,6 +943,8 @@ qemuCapsComputeCmdFlags(const char *help,
* Handling of -incoming arg with varying features
* -incoming tcp (kvm >= 79, qemu >= 0.10.0)
* -incoming exec (kvm >= 80, qemu >= 0.10.0)
+ * -incoming unix (qemu >= 0.12.0)
+ * -incoming fd (qemu >= 0.12.0)
* -incoming stdio (all earlier kvm)
*
* NB, there was a pre-kvm-79 'tcp' support, but it
@@ -952,8 +954,10 @@ qemuCapsComputeCmdFlags(const char *help,
if (version >= 10000) {
flags |= QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP;
flags |= QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC;
- if (version >= 12000)
+ if (version >= 12000) {
flags |= QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX;
+ flags |= QEMUD_CMD_FLAG_MIGRATE_QEMU_FD;
+ }
} else if (kvm_version >= 79) {
flags |= QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP;
if (kvm_version >= 80)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 83afd9b..ee648f0 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -82,6 +82,7 @@ enum qemuCapsFlags {
QEMUD_CMD_FLAG_VGA_QXL = (1LL << 45), /* The 'qxl' arg for
'-vga' */
QEMUD_CMD_FLAG_SPICE = (1LL << 46), /* Is -spice avail */
QEMUD_CMD_FLAG_VGA_NONE = (1LL << 47), /* The 'none' arg for
'-vga' */
+ QEMUD_CMD_FLAG_MIGRATE_QEMU_FD = (1LL << 48), /* -incoming fd:n */
};
virCapsPtr qemuCapsInit(virCapsPtr old_caps);
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 553abbc..18a71fa 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -286,7 +286,8 @@ mymain(int argc, char **argv)
QEMUD_CMD_FLAG_BOOT_MENU |
QEMUD_CMD_FLAG_NAME_PROCESS |
QEMUD_CMD_FLAG_SMBIOS_TYPE |
- QEMUD_CMD_FLAG_VGA_NONE,
+ QEMUD_CMD_FLAG_VGA_NONE |
+ QEMUD_CMD_FLAG_MIGRATE_QEMU_FD,
12001, 0, 0);
DO_TEST("qemu-kvm-0.12.1.2-rhel60",
QEMUD_CMD_FLAG_VNC_COLON |
@@ -324,7 +325,8 @@ mymain(int argc, char **argv)
QEMUD_CMD_FLAG_SMBIOS_TYPE |
QEMUD_CMD_FLAG_VGA_QXL |
QEMUD_CMD_FLAG_SPICE |
- QEMUD_CMD_FLAG_VGA_NONE,
+ QEMUD_CMD_FLAG_VGA_NONE |
+ QEMUD_CMD_FLAG_MIGRATE_QEMU_FD,
12001, 1, 0);
DO_TEST("qemu-kvm-0.12.3",
QEMUD_CMD_FLAG_VNC_COLON |
@@ -360,7 +362,8 @@ mymain(int argc, char **argv)
QEMUD_CMD_FLAG_NESTING |
QEMUD_CMD_FLAG_NAME_PROCESS |
QEMUD_CMD_FLAG_SMBIOS_TYPE |
- QEMUD_CMD_FLAG_VGA_NONE,
+ QEMUD_CMD_FLAG_VGA_NONE |
+ QEMUD_CMD_FLAG_MIGRATE_QEMU_FD,
12003, 1, 0);
DO_TEST("qemu-kvm-0.13.0",
QEMUD_CMD_FLAG_VNC_COLON |
@@ -403,7 +406,8 @@ mymain(int argc, char **argv)
QEMUD_CMD_FLAG_NAME_PROCESS |
QEMUD_CMD_FLAG_SMBIOS_TYPE |
QEMUD_CMD_FLAG_SPICE |
- QEMUD_CMD_FLAG_VGA_NONE,
+ QEMUD_CMD_FLAG_VGA_NONE |
+ QEMUD_CMD_FLAG_MIGRATE_QEMU_FD,
13000, 1, 0);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
--
1.7.3.3
4:27 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: use -incoming fd:n to avoid qemu holding fd indefinitely
https://bugzilla.redhat.com/show_bug.cgi?id=620363
When using -incoming stdio or -incoming exec:cat, 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.
It would also be possible to fix this bug for qemu < 0.12.0, by
creating a dummy 'cat' process that reads from real fd and writes to a
pipe, and pass the other end of the pipe into -incoming exec:cat or
-incoming stdio, at the expense of even more I/O. I don't know if
that is worth the patch, though.
* 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 bde3904..3187858 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2452,6 +2452,7 @@ qemuBuildCommandLine(virConnectPtr conn,
bool monitor_json,
unsigned long long qemuCmdFlags,
const char *migrateFrom,
+ int migrateFd,
virDomainSnapshotObjPtr current_snapshot,
enum virVMOperationType vmop)
{
@@ -2479,33 +2480,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;
/*
@@ -3880,8 +3854,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);
+ virCommandTransferFD(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);
+ virCommandTransferFD(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 4c42a10..bdab911 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 924446f..0511266 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2803,7 +2803,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;
@@ -2853,9 +2853,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);
@@ -6146,7 +6143,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 6760f67..eb0a401 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]);
@@ -113,7 +114,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) {
@@ -161,6 +163,7 @@ struct testInfo {
const char *name;
unsigned long long extraFlags;
const char *migrateFrom;
+ int migrateFd;
bool expectError;
};
@@ -173,7 +176,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);
}
@@ -218,10 +222,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) \
@@ -229,7 +233,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
@@ -423,10 +427,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.3
4:47 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: use -incoming fd:n to avoid qemu holding fd indefinitely
On 12/22/2010 03:27 PM, Eric Blake wrote:
+ } else if (STREQ(migrateFrom, "stdio")) {
+ if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_FD) {
+ virCommandAddArgFormat(cmd, "fd:%d", migrateFd);
+ virCommandTransferFD(cmd, migrateFd);
This needs to be preserve rather than transfer, since the caller in
qemu_driver.c expects to be able to still manage and close migrateFd.
I'm squashing this in:
diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
index 3187858..31c23c0 100644
--- i/src/qemu/qemu_command.c
+++ w/src/qemu/qemu_command.c
@@ -3870,7 +3870,7 @@ qemuBuildCommandLine(virConnectPtr conn,
} else if (STREQ(migrateFrom, "stdio")) {
if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_FD) {
virCommandAddArgFormat(cmd, "fd:%d", migrateFd);
- virCommandTransferFD(cmd, migrateFd);
+ virCommandPreserveFD(cmd, migrateFd);
} else if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) {
virCommandAddArg(cmd, "exec:cat");
virCommandSetInputFD(cmd, migrateFd);
@@ -3899,7 +3899,7 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
}
virCommandAddArg(cmd, migrateFrom);
- virCommandTransferFD(cmd, migrateFd);
+ virCommandPreserveFD(cmd, migrateFd);
} else {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("unknown migration protocol"));
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
6:49 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: use -incoming fd:n to avoid qemu holding fd indefinitely
On 12/22/2010 11:27 PM, Eric Blake wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=620363
When using -incoming stdio or -incoming exec:cat, 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.
It would also be possible to fix this bug for qemu< 0.12.0, by
creating a dummy 'cat' process that reads from real fd and writes to a
pipe, and pass the other end of the pipe into -incoming exec:cat or
-incoming stdio, at the expense of even more I/O. I don't know if
that is worth the patch, though.
Since you are at it, you may also want to replace the usage of netcat
and Unix sockets with "-incoming fd:" and a one-way pipe.
Paolo
10:34 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: use -incoming fd:n to avoid qemu holding fd indefinitely
On 12/23/2010 05:49 AM, Paolo Bonzini wrote:
On 12/22/2010 11:27 PM, Eric Blake wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=620363
>
> When using -incoming stdio or -incoming exec:cat, 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.
>
> It would also be possible to fix this bug for qemu< 0.12.0, by
> creating a dummy 'cat' process that reads from real fd and writes to a
> pipe, and pass the other end of the pipe into -incoming exec:cat or
> -incoming stdio, at the expense of even more I/O. I don't know if
> that is worth the patch, though.
Since you are at it, you may also want to replace the usage of netcat
and Unix sockets with "-incoming fd:" and a one-way pipe.
Not sure it's worth it. Is unix:path any less efficient than creating a
one-way pipe for use by fd:n? Also, we can't replace netcat; the use of
'-incoming "exec:nc -lU path"' is only used in libvirt that predates
'-incoming unix:path', which also happens to predate '-incoming fd:n'.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:51 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: use -incoming fd:n to avoid qemu holding fd indefinitely
On 12/23/2010 09:34 AM, Eric Blake wrote:
> Since you are at it, you may also want to replace the usage of
netcat
> and Unix sockets with "-incoming fd:" and a one-way pipe.
Not sure it's worth it. Is unix:path any less efficient than creating a
one-way pipe for use by fd:n? Also, we can't replace netcat; the use of
'-incoming "exec:nc -lU path"' is only used in libvirt that predates
'-incoming unix:path', which also happens to predate '-incoming fd:n'.
Thinking about it more:
In the outgoing direction, we still have to use a unix socket (outgoing
migration is started via a monitor command, and I don't know how to pass
a new fd into qemu for using with an fd:n outgoing migration via the
monitor - it seems like fd is only useful on the command line).
But you are right that in the incoming direction, we could use
virFDStreamOpen rather than virFDStreamConnectUNIX, since a one-way pipe
is a lot less code to set up than a unix socket; and this could fall
back to '-incoming stdio' on older qemu with no loss over the current
fallback of '-incoming "exec:nc -lU path"'. I'll play with a patch
to
see how it looks.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:06 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: use -incoming fd:n to avoid qemu holding fd indefinitely
On 12/23/2010 05:51 PM, Eric Blake wrote:
>> Since you are at it, you may also want to replace the usage
of netcat
>> >> and Unix sockets with "-incoming fd:" and a one-way pipe.
> >
> > Not sure it's worth it. Is unix:path any less efficient than creating a
> > one-way pipe for use by fd:n? Also, we can't replace netcat; the use of
> > '-incoming "exec:nc -lU path"' is only used in libvirt that
predates
> > '-incoming unix:path', which also happens to predate '-incoming
fd:n'.
Thinking about it more:
In the outgoing direction, we still have to use a unix socket (outgoing
migration is started via a monitor command, and I don't know how to pass
a new fd into qemu for using with an fd:n outgoing migration via the
monitor - it seems like fd is only useful on the command line).
File descriptor in the outgoing direction is supported using SCM_RIGHTS,
but you're right that when qemu doesn't support the fd: migration
protocol you would still have to use a fallback (it could be mkfifo +
cat, instead of netcat, but it would be a substantial amount of work for
a limited gain in portability).
Paolo
7:39 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: use -incoming fd:n to avoid qemu holding fd indefinitely
On 12/23/2010 05:51 PM, Eric Blake wrote:
In the outgoing direction, we still have to use a unix socket
(outgoing
migration is started via a monitor command, and I don't know how to pass
a new fd into qemu for using with an fd:n outgoing migration via the
monitor - it seems like fd is only useful on the command line).
See qemuMonitorCommandWithFd and qemuMonitorJSONCommandWithFd.
Paolo
11:16 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: use -incoming fd:n to avoid qemu holding fd indefinitely
On 01/03/2011 06:39 AM, Paolo Bonzini wrote:
On 12/23/2010 05:51 PM, Eric Blake wrote:
> In the outgoing direction, we still have to use a unix socket (outgoing
> migration is started via a monitor command, and I don't know how to pass
> a new fd into qemu for using with an fd:n outgoing migration via the
> monitor - it seems like fd is only useful on the command line).
See qemuMonitorCommandWithFd and qemuMonitorJSONCommandWithFd.
I'm looking at this problem again, now that we know that qemu exec:
migration is inherently racy (see
https://bugzilla.redhat.com/show_bug.cgi?id=678524). I've managed to
work out the code to send an fd over to qemu, but how do I formulate the
fd:n migration command to use that fd? That is, if I call
qemuMonitorCommandWithFd(,20), that copies fd 20 from libvirt's point of
view into qemu, but using the command 'migrate fd:20' is wrong because
qemu's copy is not necessarily fd 20.
Is there a monitor command that lets libvirt pass in an fd and get a
return value of what qemu sees for that newly copied fd, with no other
operation? If so, then I can use that to do a two-step process - first
pass the fd, then pass the migrate fd:n command. And what if the first
step succeeds but the second fails? Is there a way to request that qemu
close an arbitrary fd that was passed to it earlier on? Otherwise,
libvirt has just leaked an fd to qemu, and we have a repeat of the
earlier bug report where if the fd is a lvm partition, then the leaked
reference prevents one from closing out the logical volume.
I don't want to have to resort to brute-force searching through
/proc/<qemu-pid>/fd to find which fd in qemu matches the one that
libvirt sent over.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
1:47 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: use -incoming fd:n to avoid qemu holding fd indefinitely
On 03/02/2011 06:16 AM, Eric Blake wrote:
On 01/03/2011 06:39 AM, Paolo Bonzini wrote:
> On 12/23/2010 05:51 PM, Eric Blake wrote:
>> In the outgoing direction, we still have to use a unix socket (outgoing
>> migration is started via a monitor command, and I don't know how to pass
>> a new fd into qemu for using with an fd:n outgoing migration via the
>> monitor - it seems like fd is only useful on the command line).
>
> See qemuMonitorCommandWithFd and qemuMonitorJSONCommandWithFd.
I'm looking at this problem again, now that we know that qemu exec:
migration is inherently racy (see
https://bugzilla.redhat.com/show_bug.cgi?id=678524). I've managed to
work out the code to send an fd over to qemu, but how do I formulate the
fd:n migration command to use that fd? That is, if I call
qemuMonitorCommandWithFd(,20), that copies fd 20 from libvirt's point of
view into qemu, but using the command 'migrate fd:20' is wrong because
qemu's copy is not necessarily fd 20.
This doesn't matter. Libvirt passes its own fd 20 to QEMU "by name" via
SCM_RIGHTS and the getfd command (which associates a name to the file
descriptor being sent at the same time). What is then passed to
"migrate fd:", is the symbolic name.
Actually, you do not need to call qemuMonitorCommandWithFd. What you
need to do with it is already wrapped by qemuMonitorSendFileHandle. So
you will do something like qemuMonitorSendFileHandle(mon, "migr-1", 20)
and then use "fd:migr-1" on the migrate command.
Is there a monitor command that lets libvirt pass in an fd and get a
return value of what qemu sees for that newly copied fd, with no other
operation? If so, then I can use that to do a two-step process - first
pass the fd, then pass the migrate fd:n command. And what if the first
step succeeds but the second fails?
I believe the fd will be closed even if it fails, but there is also
qemuMonitorCloseFileHandle that you can use.
Paolo
6:14 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: use -incoming fd:n to avoid qemu holding fd indefinitely
On Tue, Mar 01, 2011 at 10:16:52PM -0700, Eric Blake wrote:
On 01/03/2011 06:39 AM, Paolo Bonzini wrote:
> On 12/23/2010 05:51 PM, Eric Blake wrote:
>> In the outgoing direction, we still have to use a unix socket (outgoing
>> migration is started via a monitor command, and I don't know how to pass
>> a new fd into qemu for using with an fd:n outgoing migration via the
>> monitor - it seems like fd is only useful on the command line).
>
> See qemuMonitorCommandWithFd and qemuMonitorJSONCommandWithFd.
I'm looking at this problem again, now that we know that qemu exec:
migration is inherently racy (see
https://bugzilla.redhat.com/show_bug.cgi?id=678524). I've managed to
work out the code to send an fd over to qemu, but how do I formulate the
fd:n migration command to use that fd? That is, if I call
qemuMonitorCommandWithFd(,20), that copies fd 20 from libvirt's point of
view into qemu, but using the command 'migrate fd:20' is wrong because
qemu's copy is not necessarily fd 20.
As Paolo mentions, file descriptors are passed by name in the monitor
args, instead of by number as they are on the command line. So if doing
a NIC on the command line libvirt would do
-netdev tap,fd=20
The corresponding monitor code would be
getfd foo (with SCM_RIGHTS for fd=20)
netdev_add tap,fd=foo
closefd foo (only if we failed netdev_add stage)
So libvirt never needs to know what FD number QEMU sees.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
10:53 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: use -incoming fd:n to avoid qemu holding fd indefinitely
On 12/22/2010 03:27 PM, Eric Blake wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=620363
When using -incoming stdio or -incoming exec:cat, 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.
Ping. Do we want this patch included into 0.8.7? It's technically a new
feature, but it was proposed before we entered feature freeze.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:29 p.m.
New subject: [libvirt] [PATCH 3/2] qemu: use lighter-weight fd:n on incoming tunneled migration
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.
---
So far only compile tested. After sending this mail, I'm going
to reboot into compatible systems to actually test it, so here's
hoping I don't have to send a v2 from something I overlooked.
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 490dc95..3ef2c22 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7982,11 +7982,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;
@@ -8052,39 +8051,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
@@ -8097,9 +8084,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) {
@@ -8107,9 +8092,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;
}
@@ -8139,9 +8123,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.3
9:52 a.m.
New subject: [libvirt] [PATCH 3/2] qemu: use lighter-weight fd:n on incoming tunneled migration
On 12/23/2010 11:29 AM, 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.
Ping - patch 1/3 is upstream in 0.8.7, but 2/3 and 3/3 are still
awaiting review.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
1:25 p.m.
2010/12/22 Eric Blake <eblake(a)redhat.com>:
* src/qemu/qemu_capabilities.h (QEMUD_CMD_FLAG_MIGRATE_QEMU_FD):
New enum value.
* src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags): Populate
flags according to qemu version.
* tests/qemuhelptest.c (mymain): Adjust test.
---
I feel a bit dirty relying on just a version test for whether
-incoming fd:n is supported, but it's no different than the
existing code for -incoming unix:path which was added about the
same time. Unfortunately, 'qemu -help' doesn't list which
particular protocols it supports for -incoming.
src/qemu/qemu_capabilities.c | 6 +++++-
src/qemu/qemu_capabilities.h | 1 +
tests/qemuhelptest.c | 12 ++++++++----
3 files changed, 14 insertions(+), 5 deletions(-)
ACK.
Matthias
5:46 p.m.
On 12/23/2010 12:25 PM, Matthias Bolte wrote:
2010/12/22 Eric Blake <eblake(a)redhat.com>:
> * src/qemu/qemu_capabilities.h (QEMUD_CMD_FLAG_MIGRATE_QEMU_FD):
> New enum value.
> * src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags): Populate
> flags according to qemu version.
> * tests/qemuhelptest.c (mymain): Adjust test.
> ---
>
> I feel a bit dirty relying on just a version test for whether
> -incoming fd:n is supported, but it's no different than the
> existing code for -incoming unix:path which was added about the
> same time. Unfortunately, 'qemu -help' doesn't list which
> particular protocols it supports for -incoming.
>
> src/qemu/qemu_capabilities.c | 6 +++++-
> src/qemu/qemu_capabilities.h | 1 +
> tests/qemuhelptest.c | 12 ++++++++----
> 3 files changed, 14 insertions(+), 5 deletions(-)
>
ACK.
Thanks; pushed. I've now done actual migration testing with patches 2
and 3 in place, and was pleased that it worked without having to make
any more modifications, so feel free to review those as well.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
4827
days inactive
4897
days old
15 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Matthias Bolte -
Paolo Bonzini