[libvirt] [PATCH 1/2] qemu: add -incoming fd:n capability checking
* 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
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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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
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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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
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 :|
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@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. --- 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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2010/12/22 Eric Blake <eblake@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
On 12/23/2010 12:25 PM, Matthias Bolte wrote:
2010/12/22 Eric Blake <eblake@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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Matthias Bolte -
Paolo Bonzini