Subject typo - "issugin"
On Thu, 2009-09-24 at 16:00 +0100, Daniel P. Berrange wrote:
> * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h: Add new
> qemuMonitorMigrateToCommand() API
> * src/qemu/qemu_driver.c: Switch over to using the
> qemuMonitorMigrateToCommand() API for core dumps and save
> to file APIs
> ---
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_driver.c | 119
> ++++++++----------------------------------
> src/qemu/qemu_monitor_text.c | 63 +++++++++++++++++++++--
> src/qemu/qemu_monitor_text.h | 4 ++
> 4 files changed, 86 insertions(+), 101 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a6668f3..f8598c7 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -453,6 +453,7 @@ virGetGroupID;
> virFileFindMountPoint;
> virFileWaitForDevices;
> virFileMatchesNameSuffix;
> +virArgvToString;
>
>
> # usb.h
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f234639..da08af9 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3170,11 +3170,6 @@ static char *qemudEscapeMonitorArg(const char *in)
> return qemudEscape(in, 0);
> }
>
> -static char *qemudEscapeShellArg(const char *in)
> -{
> - return qemudEscape(in, 1);
> -}
> -
> #define QEMUD_SAVE_MAGIC "LibvirtQemudSave"
> #define QEMUD_SAVE_VERSION 2
>
> @@ -3217,15 +3212,11 @@ static int qemudDomainSave(virDomainPtr dom,
> {
> struct qemud_driver *driver = dom->conn->privateData;
> virDomainObjPtr vm = NULL;
> - char *command = NULL;
> - char *info = NULL;
> int fd = -1;
> - char *safe_path = NULL;
> char *xml = NULL;
> struct qemud_save_header header;
> int ret = -1;
> virDomainEventPtr event = NULL;
> - int internalret;
>
> memset(&header, 0, sizeof(header));
> memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic));
> @@ -3267,7 +3258,6 @@ static int qemudDomainSave(virDomainPtr dom,
> if (qemuMonitorStopCPUs(vm) < 0)
> goto cleanup;
> vm->state = VIR_DOMAIN_PAUSED;
> - VIR_FREE(info);
> }
>
> /* Get XML for the domain */
> @@ -3306,55 +3296,21 @@ static int qemudDomainSave(virDomainPtr dom,
> }
> fd = -1;
>
> - /* Migrate to file */
> - safe_path = qemudEscapeShellArg(path);
> - if (!safe_path) {
> - virReportOOMError(dom->conn);
> - goto cleanup;
> - }
> -
> - {
> + if (header.compressed == QEMUD_SAVE_FORMAT_RAW) {
> + const char *args[] = { "cat", NULL };
> + ret = qemuMonitorMigrateToCommand(vm, args, path);
> + } else {
> const char *prog =
> qemudSaveCompressionTypeToString(header.compressed);
> - const char *args;
> -
> - if (prog == NULL) {
> - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
> - _("Invalid compress format %d"),
> header.compressed);
> - goto cleanup;
> - }
> -
> - if (STREQ (prog, "raw")) {
> - prog = "cat";
> - args = "";
> - } else {
> - args = "-c";
> - }
Using the enum value to catch the raw case rather than the "raw" string
is a sensible change
> - internalret = virAsprintf(&command, "migrate \"exec:"
> - "%s %s >> '%s' 2>/dev/null\"", prog, args,
> - safe_path);
> - }
> -
> - if (internalret < 0) {
> - virReportOOMError(dom->conn);
> - goto cleanup;
> - }
> -
> - if (qemudMonitorCommand(vm, command, &info) < 0) {
> - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> - "%s", _("migrate operation failed"));
> - goto cleanup;
> + const char *args[] = {
> + prog,
> + "-c",
> + NULL
> + };
> + ret = qemuMonitorMigrateToCommand(vm, args, path);
> }
>
> - DEBUG ("%s: migrate reply: %s", vm->def->name, info);
> -
> - /* If the command isn't supported then qemu prints:
> - * unknown command: migrate" */
> - if (strstr(info, "unknown command:")) {
> - qemudReportError (dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
> - "%s",
> - _("'migrate' not supported by this qemu"));
> + if (ret < 0)
> goto cleanup;
> - }
>
> /* Shut it down */
> qemudShutdownVMDaemon(dom->conn, driver, vm);
> @@ -3366,15 +3322,11 @@ static int qemudDomainSave(virDomainPtr dom,
> vm);
> vm = NULL;
> }
> - ret = 0;
>
> cleanup:
> if (fd != -1)
> close(fd);
> VIR_FREE(xml);
> - VIR_FREE(safe_path);
> - VIR_FREE(command);
> - VIR_FREE(info);
> if (ret != 0)
> unlink(path);
> if (vm)
> @@ -3391,11 +3343,12 @@ static int qemudDomainCoreDump(virDomainPtr dom,
> int flags ATTRIBUTE_UNUSED) {
> struct qemud_driver *driver = dom->conn->privateData;
> virDomainObjPtr vm;
> - char *command = NULL;
> - char *info = NULL;
> - char *safe_path = NULL;
> int resume = 0, paused = 0;
> int ret = -1;
> + const char *args[] = {
> + "cat",
> + NULL,
> + };
>
> qemuDriverLock(driver);
> vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> @@ -3427,43 +3380,9 @@ static int qemudDomainCoreDump(virDomainPtr dom,
> paused = 1;
> }
>
> - /* Migrate to file */
> - safe_path = qemudEscapeShellArg(path);
> - if (!safe_path) {
> - virReportOOMError(dom->conn);
> - goto cleanup;
> - }
> - if (virAsprintf(&command, "migrate \"exec:"
> - "dd of='%s' 2>/dev/null"
> - "\"", safe_path) == -1) {
Would have like to see switch from dd to cat as a separate patch with an
explanation in the changelog
> - virReportOOMError(dom->conn);
> - command = NULL;
> - goto cleanup;
> - }
> -
> - if (qemudMonitorCommand(vm, command, &info) < 0) {
> - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> - "%s", _("migrate operation failed"));
> - goto cleanup;
> - }
> -
> - DEBUG ("%s: migrate reply: %s", vm->def->name, info);
> -
> - /* If the command isn't supported then qemu prints:
> - * unknown command: migrate" */
> - if (strstr(info, "unknown command:")) {
> - qemudReportError (dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
> - "%s",
> - _("'migrate' not supported by this qemu"));
> - goto cleanup;
> - }
> -
> + ret = qemuMonitorMigrateToCommand(vm, args, path);
> paused = 1;
> - ret = 0;
> cleanup:
> - VIR_FREE(safe_path);
> - VIR_FREE(command);
> - VIR_FREE(info);
>
> /* Since the monitor is always attached to a pty for libvirt, it
> will support synchronous operations so we always get here after
> @@ -6370,6 +6289,11 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
> goto cleanup;
> }
>
> + /* XXX this really should have been a properly well-formed
> + * URI, but we can't add in tcp:// now without breaking
> + * compatability with old targets. We at least make the
> + * new targets accept both syntaxes though.
> + */
> /* Caller frees */
> internalret = virAsprintf(uri_out, "tcp:%s:%d", hostname, this_port);
> VIR_FREE(hostname);
> @@ -6536,6 +6460,7 @@ qemudDomainMigratePerform (virDomainPtr dom,
>
> /* Issue the migrate command. */
> if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://")) {
> + /* HACK: source host generates bogus URIs, so fix them up */
> char *tmpuri;
> if (virAsprintf(&tmpuri, "tcp://%s", uri + strlen("tcp:")) < 0) {
> virReportOOMError(dom->conn);
Two unrelated comments - looks like it's more related to the URI changes
you made in the previous patch
> diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
> index 4f8d72e..c154019 100644
> --- a/src/qemu/qemu_monitor_text.c
> +++ b/src/qemu/qemu_monitor_text.c
> @@ -118,6 +118,10 @@ static char *qemudEscapeMonitorArg(const char *in)
> return qemudEscape(in, 0);
> }
>
> +static char *qemudEscapeShellArg(const char *in)
> +{
> + return qemudEscape(in, 1);
> +}
>
> /* Throw away any data available on the monitor
> * This is done before executing a command, in order
> @@ -1096,12 +1100,18 @@ static int qemuMonitorMigrate(const virDomainObjPtr
> vm,
> char *cmd = NULL;
> char *info = NULL;
> int ret = -1;
> + char *safedest = qemudEscapeMonitorArg(dest);
>
> - if (virAsprintf(&cmd, "migrate %s", cmd) < 0) {
> + if (!safedest) {
> virReportOOMError(NULL);
> return -1;
> }
>
> + if (virAsprintf(&cmd, "migrate \"%s\"", safedest) < 0) {
> + virReportOOMError(NULL);
> + goto cleanup;
> + }
Little bit worried that the introduction of escaping will break other
usages, but it should be fine
> +
> if (qemudMonitorCommand(vm, cmd, &info) < 0) {
> qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> _("unable to start migration to %s"), dest);
> @@ -1112,8 +1122,15 @@ static int qemuMonitorMigrate(const virDomainObjPtr vm,
>
> /* Now check for "fail" in the output string */
> if (strstr(info, "fail") != NULL) {
> - qemudReportError (NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> - _("migration to '%s' failed: %s"), dest, info);
> + qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> + _("migration to '%s' failed: %s"), dest, info);
Unrelated
> + goto cleanup;
> + }
> + /* If the command isn't supported then qemu prints:
> + * unknown command: migrate" */
> + if (strstr(info, "unknown command:")) {
> + qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_SUPPORT,
> + _("migration to '%s' not supported by this qemu:
> %s"), dest, info);
> goto cleanup;
> }
>
> @@ -1121,6 +1138,7 @@ static int qemuMonitorMigrate(const virDomainObjPtr vm,
> ret = 0;
>
> cleanup:
> + VIR_FREE(safedest);
> VIR_FREE(info);
> VIR_FREE(cmd);
> return ret;
> @@ -1130,7 +1148,7 @@ int qemuMonitorMigrateToHost(const virDomainObjPtr vm,
> const char *hostname,
> int port)
> {
> - char *uri;
> + char *uri = NULL;
Unrelated
ACK
Cheers,
Mark.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
