On Mon, Sep 28, 2009 at 02:22:59PM +0100, Mark McLoughlin wrote:
> On Thu, 2009-09-24 at 16:00 +0100, Daniel P. Berrange wrote:
> > @@ -6537,34 +6535,29 @@ qemudDomainMigratePerform (virDomainPtr dom,
> > goto cleanup;
> >
> > /* Issue the migrate command. */
> > - safe_uri = qemudEscapeMonitorArg (uri);
> > - if (!safe_uri) {
> > - virReportOOMError (dom->conn);
> > - goto cleanup;
> > + if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://")) {
> > + char *tmpuri;
> > + if (virAsprintf(&tmpuri, "tcp://%s", uri + strlen("tcp:")) < 0) {
> > + virReportOOMError(dom->conn);
> > + goto cleanup;
> > + }
> > + uribits = xmlParseURI(tmpuri);
> > + VIR_FREE(tmpuri);
> > + } else {
> > + uribits = xmlParseURI(uri);
>
> This is all new stuff and there's no explanation in the ChangeLog; not
> sure why you're so keen to split up the URI only to re-construct it
> again
We're calling this parameter a URI but doing zero validation that it
is actually correctly formated as a URI, and just passing it straight
down to QEMU. The application caller could have appended all sorts
of junk onto the URI. In fact the QEMU Prepare() method wasn't even
generating valid URIs, which rather justifies why we should have been
doing validation in the first place. So this code fixes the broken
tcp:hostname:port URIs to actually be tcp://hostname:port
Unfortunately we cna't fix the original Prepare() method since that
would break compatability with older libvirt. The whole thing is a
giant mess because it was directly exposing the QEMU monitor syntax
for TCP migration :-(
> > diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
> > index 0b746b9..4f8d72e 100644
> > --- a/src/qemu/qemu_monitor_text.c
> > +++ b/src/qemu/qemu_monitor_text.c
> > @@ -1088,3 +1088,59 @@ cleanup:
> > VIR_FREE(reply);
> > return ret;
> > }
> > +
> > +
> > +static int qemuMonitorMigrate(const virDomainObjPtr vm,
> > + const char *dest)
> > +{
> > + char *cmd = NULL;
> > + char *info = NULL;
> > + int ret = -1;
> > +
> > + if (virAsprintf(&cmd, "migrate %s", cmd) < 0) {
>
> Should be passing dest here
>
> Fixed in the next patch, but would be good to get it right for
> bisectability
Ahh I knew there was one place where I screwed up / missed a rebase.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list