Re: [libvirt] [PATCH 2/6] Move all the QEMU migration code to a new file

2011-02-11 Thread Daniel P. Berrange
On Wed, Feb 09, 2011 at 01:20:39PM -0700, Eric Blake wrote:
> On 02/09/2011 09:58 AM, Daniel P. Berrange wrote:
> > The introduction of the v3 migration protocol, along with
> > support for migration cookies, will significantly expand
> > the size of the migration code. Move it all to a separate
> > file to make it more manageable
> > 
> > The functions are not moved 100%. The API entry points
> > remain in the main QEMU driver, but once the public
> > virDomainPtr is resolved to the internal virDomainObjPtr,
> > all following code is moved.
> > 
> > This will allow the new v3 API entry points to call into the
> > same shared internal migration functions
> > 
> > * src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add
> >   qemuDomainFormatXML helper method
> > * src/qemu/qemu_driver.c: Remove all migration code
> > * src/qemu/qemu_migration.c, src/qemu/qemu_migration.h: Add
> >   all migration code.
> > ---
> >  po/POTFILES.in|1 +
> >  src/Makefile.am   |3 +-
> >  src/qemu/qemu_domain.c|   39 ++
> >  src/qemu/qemu_domain.h|4 +
> >  src/qemu/qemu_driver.c| 1297 
> > ++---
> >  src/qemu/qemu_migration.c | 1295 
> > 
> >  src/qemu/qemu_migration.h |   63 +++
> >  7 files changed, 1445 insertions(+), 1257 deletions(-)
> >  create mode 100644 src/qemu/qemu_migration.c
> >  create mode 100644 src/qemu/qemu_migration.h
> 
> You fixed my concerns from v1; however, you missed that commit ee3b030
> in the meantime has changed what needed migration.
> 
> > + * 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,
> > +   virStreamPtr st,
> > +   const char *dname,
> > +   const char *dom_xml)
> > +{
> 
> > +/* Parse the domain XML. */
> > +if (!(def = virDomainDefParseString(driver->caps, dom_xml,
> > +VIR_DOMAIN_XML_INACTIVE))) {
> > +qemuReportError(VIR_ERR_OPERATION_FAILED,
> > +"%s", _("failed to parse XML"));
> > +goto cleanup;
> 
> This needs to be:
> 
>  qemuReportError(VIR_ERR_OPERATION_FAILED,
> -"%s", _("failed to parse XML"));
> +"%s", _("failed to parse XML, libvirt version
> may be "
> +"different between source and
> destination host"));
> 
> ACK with that nit fixed.

Actually that change should not have been included in the first place.
The entire line should have been deleted as per:

http://www.redhat.com/archives/libvir-list/2011-January/msg01271.html

I'm deleting it in my series

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
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2/6] Move all the QEMU migration code to a new file

2011-02-09 Thread Eric Blake
On 02/09/2011 09:58 AM, Daniel P. Berrange wrote:
> The introduction of the v3 migration protocol, along with
> support for migration cookies, will significantly expand
> the size of the migration code. Move it all to a separate
> file to make it more manageable
> 
> The functions are not moved 100%. The API entry points
> remain in the main QEMU driver, but once the public
> virDomainPtr is resolved to the internal virDomainObjPtr,
> all following code is moved.
> 
> This will allow the new v3 API entry points to call into the
> same shared internal migration functions
> 
> * src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add
>   qemuDomainFormatXML helper method
> * src/qemu/qemu_driver.c: Remove all migration code
> * src/qemu/qemu_migration.c, src/qemu/qemu_migration.h: Add
>   all migration code.
> ---
>  po/POTFILES.in|1 +
>  src/Makefile.am   |3 +-
>  src/qemu/qemu_domain.c|   39 ++
>  src/qemu/qemu_domain.h|4 +
>  src/qemu/qemu_driver.c| 1297 
> ++---
>  src/qemu/qemu_migration.c | 1295 
>  src/qemu/qemu_migration.h |   63 +++
>  7 files changed, 1445 insertions(+), 1257 deletions(-)
>  create mode 100644 src/qemu/qemu_migration.c
>  create mode 100644 src/qemu/qemu_migration.h

You fixed my concerns from v1; however, you missed that commit ee3b030
in the meantime has changed what needed migration.

> + * 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,
> +   virStreamPtr st,
> +   const char *dname,
> +   const char *dom_xml)
> +{

> +/* Parse the domain XML. */
> +if (!(def = virDomainDefParseString(driver->caps, dom_xml,
> +VIR_DOMAIN_XML_INACTIVE))) {
> +qemuReportError(VIR_ERR_OPERATION_FAILED,
> +"%s", _("failed to parse XML"));
> +goto cleanup;

This needs to be:

 qemuReportError(VIR_ERR_OPERATION_FAILED,
-"%s", _("failed to parse XML"));
+"%s", _("failed to parse XML, libvirt version
may be "
+"different between source and
destination host"));

ACK with that nit fixed.

-- 
Eric Blake   [email protected]+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list