Re: [libvirt] [PATCH 08/19] qemu: Consolidate qemuMigrationPrepare{Direct, Tunnel}

2011-07-13 Thread Jiri Denemark
On Mon, Jul 11, 2011 at 21:28:30 +0100, Daniel P. Berrange wrote:
> On Fri, Jul 08, 2011 at 01:34:13AM +0200, Jiri Denemark wrote:
> > Most of the code in these two functions is supposed to be identical but
> > currently it isn't (which is natural since the code is duplicated).
> > Let's move common parts of these functions into qemuMigrationPrepareAny.
> 
> Were there any actual bugs fixed in this consolidation, or were
> the differences in the code just cosmetic ? If so it'd be good to
> mention the bugs fixed in the commit message

Yes, I added the following to the commit message:

This also fixes qemuMigrationPrepareTunnel which didn't store received
lockState in the domain object.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 08/19] qemu: Consolidate qemuMigrationPrepare{Direct, Tunnel}

2011-07-11 Thread Daniel P. Berrange
On Fri, Jul 08, 2011 at 01:34:13AM +0200, Jiri Denemark wrote:
> Most of the code in these two functions is supposed to be identical but
> currently it isn't (which is natural since the code is duplicated).
> Let's move common parts of these functions into qemuMigrationPrepareAny.

Were there any actual bugs fixed in this consolidation, or were
the differences in the code just cosmetic ? If so it'd be good to
mention the bugs fixed in the commit message

> ---
>  src/qemu/qemu_migration.c |  255 +++-
>  1 files changed, 87 insertions(+), 168 deletions(-)

ACK

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 08/19] qemu: Consolidate qemuMigrationPrepare{Direct, Tunnel}

2011-07-07 Thread Jiri Denemark
Most of the code in these two functions is supposed to be identical but
currently it isn't (which is natural since the code is duplicated).
Let's move common parts of these functions into qemuMigrationPrepareAny.
---
 src/qemu/qemu_migration.c |  255 +++-
 1 files changed, 87 insertions(+), 168 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 4c516b0..e595596 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1057,40 +1057,33 @@ cleanup:
 
 
 /* Prepare is the first step, and it runs on the destination host.
- *
- * This version starts an empty VM listening on a localhost TCP port, and
- * sets up the corresponding virStream to handle the incoming data.
  */
-int
-qemuMigrationPrepareTunnel(struct qemud_driver *driver,
-   virConnectPtr dconn,
-   const char *cookiein,
-   int cookieinlen,
-   char **cookieout,
-   int *cookieoutlen,
-   virStreamPtr st,
-   const char *dname,
-   const char *dom_xml)
+
+static int
+qemuMigrationPrepareAny(struct qemud_driver *driver,
+virConnectPtr dconn,
+const char *cookiein,
+int cookieinlen,
+char **cookieout,
+int *cookieoutlen,
+const char *dname,
+const char *dom_xml,
+const char *migrateFrom,
+virStreamPtr st)
 {
 virDomainDefPtr def = NULL;
 virDomainObjPtr vm = NULL;
 virDomainEventPtr event = NULL;
 int ret = -1;
-int internalret;
 int dataFD[2] = { -1, -1 };
 qemuDomainObjPrivatePtr priv = NULL;
 unsigned long long now;
 qemuMigrationCookiePtr mig = NULL;
-
-VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, "
-  "cookieout=%p, cookieoutlen=%p, st=%p, dname=%s, dom_xml=%s",
-  driver, dconn, NULLSTR(cookiein), cookieinlen,
-  cookieout, cookieoutlen, st, NULLSTR(dname), dom_xml);
+bool tunnel = !!st;
 
 if (virTimeMs(&now) < 0)
 return -1;
 
-/* Parse the domain XML. */
 if (!(def = virDomainDefParseString(driver->caps, dom_xml,
 VIR_DOMAIN_XML_INACTIVE)))
 goto cleanup;
@@ -1129,50 +1122,45 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver,
 /* Domain starts inactive, even if the domain XML had an id field. */
 vm->def->id = -1;
 
-if (pipe(dataFD) < 0 ||
-virSetCloseExec(dataFD[1]) < 0) {
+if (tunnel &&
+(pipe(dataFD) < 0 || virSetCloseExec(dataFD[1]) < 0)) {
 virReportSystemError(errno, "%s",
  _("cannot create pipe for tunnelled migration"));
 goto endjob;
 }
 
 /* Start the QEMU daemon, with the same command-line arguments plus
- * -incoming stdio (which qemu_command might convert to exec:cat or fd:n)
+ * -incoming $migrateFrom
  */
-internalret = qemuProcessStart(dconn, driver, vm, "stdio", true,
-   true, dataFD[0], NULL,
-   VIR_VM_OP_MIGRATE_IN_START);
-if (internalret < 0) {
+if (qemuProcessStart(dconn, driver, vm, migrateFrom, true,
+ true, dataFD[0], NULL,
+ VIR_VM_OP_MIGRATE_IN_START) < 0) {
 qemuAuditDomainStart(vm, "migrated", false);
 /* Note that we don't set an error here because qemuProcessStart
  * should have already done that.
  */
-if (!vm->persistent) {
-virDomainRemoveInactive(&driver->domains, vm);
-vm = NULL;
-}
 goto endjob;
 }
 
-if (virFDStreamOpen(st, dataFD[1]) < 0) {
-qemuAuditDomainStart(vm, "migrated", false);
-qemuProcessStop(driver, vm, 0, VIR_DOMAIN_SHUTOFF_FAILED);
-if (!vm->persistent) {
-if (qemuDomainObjEndAsyncJob(driver, vm) > 0)
-virDomainRemoveInactive(&driver->domains, vm);
-vm = NULL;
+if (tunnel) {
+if (virFDStreamOpen(st, dataFD[1]) < 0) {
+virReportSystemError(errno, "%s",
+ _("cannot pass pipe for tunnelled 
migration"));
+qemuAuditDomainStart(vm, "migrated", false);
+qemuProcessStop(driver, vm, 0, VIR_DOMAIN_SHUTOFF_FAILED);
+goto endjob;
 }
-virReportSystemError(errno, "%s",
- _("cannot pass pipe for tunnelled migration"));
-goto endjob;
+dataFD[1] = -1; /* 'st' owns the FD now & will close it */
 }
-dataFD[1] = -1; /* 'st' owns the FD now & will close it */
-
-qemuAuditDomainStart(vm, "migrated", true);
 
-event =