[Qemu-devel] Live migration - exec: support to be reintroduced?

[Qemu-devel] Live migration - exec: support to be reintroduced?

[Charles Duffy]

Howdy.

KVM's live migration support used to support an exec: protoco, allowing 
  either a completely arbitrary transport or a mechanism for storing 
system state to a separate file for later resurrection, being used in 
the latter by libvirt's qemu driver. I notice that this support no 
longer exists in the current codebase.

Would a patch reimplementing this support be welcome?

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Anthony Liguori]

Charles Duffy wrote:
> Howdy.
>
> KVM's live migration support used to support an exec: protoco, 
> allowing  either a completely arbitrary transport or a mechanism for 
> storing system state to a separate file for later resurrection, being 
> used in the latter by libvirt's qemu driver. I notice that this 
> support no longer exists in the current codebase.
> Would a patch reimplementing this support be welcome?

Absolutely.  I wasn't aware that libvirt used the exec: protocol.  I'm 
somewhat surprised by that.  What did it use it for?

Regards,

Anthony Liguori


>
>

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Chris Lalancette]

Anthony Liguori wrote:
> Charles Duffy wrote:
>> Howdy.
>>
>> KVM's live migration support used to support an exec: protoco, 
>> allowing  either a completely arbitrary transport or a mechanism for 
>> storing system state to a separate file for later resurrection, being 
>> used in the latter by libvirt's qemu driver. I notice that this 
>> support no longer exists in the current codebase.
>> Would a patch reimplementing this support be welcome?
> 
> Absolutely.  I wasn't aware that libvirt used the exec: protocol.  I'm 
> somewhat surprised by that.  What did it use it for?

Hm, I didn't realize it used it either, but it uses it for doing a save operation:

static int qemudDomainSave(virDomainPtr dom,
                           const char *path) {
...
    if (asprintf (&command, "migrate \"exec:"
                  "dd of='%s' oflag=append conv=notrunc 2>/dev/null"
                  "\"", safe_path) == -1) {
        qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                         "%s", _("out of memory"));
        VIR_FREE(safe_path);
        return -1;
    }
    free(safe_path);

    if (qemudMonitorCommand(driver, vm, command, &info) < 0) {
...

I don't know if there is a better way to do that with the qemu monitor, to avoid
 using migrate altogether.  In any case, I'm sure other people would find exec:
useful as well.

-- 
Chris Lalancette

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Anthony Liguori]

Chris Lalancette wrote:
> Anthony Liguori wrote:
>   
>> Charles Duffy wrote:
>>     
>>> Howdy.
>>>
>>> KVM's live migration support used to support an exec: protoco, 
>>> allowing  either a completely arbitrary transport or a mechanism for 
>>> storing system state to a separate file for later resurrection, being 
>>> used in the latter by libvirt's qemu driver. I notice that this 
>>> support no longer exists in the current codebase.
>>> Would a patch reimplementing this support be welcome?
>>>       
>> Absolutely.  I wasn't aware that libvirt used the exec: protocol.  I'm 
>> somewhat surprised by that.  What did it use it for?
>>     
>
> Hm, I didn't realize it used it either, but it uses it for doing a save operation:
>
> static int qemudDomainSave(virDomainPtr dom,
>                            const char *path) {
> ...
>     if (asprintf (&command, "migrate \"exec:"
>                   "dd of='%s' oflag=append conv=notrunc 2>/dev/null"
>                   "\"", safe_path) == -1) {
>         qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
>                          "%s", _("out of memory"));
>         VIR_FREE(safe_path);
>         return -1;
>     }
>     free(safe_path);
>
>     if (qemudMonitorCommand(driver, vm, command, &info) < 0) {
> ...
>
> I don't know if there is a better way to do that with the qemu monitor, to avoid
>  using migrate altogether.  In any case, I'm sure other people would find exec:
> useful as well.
>   

Sure.  It would make sense for it to use file: instead of exec: though.

However, why is it not just using savevm?  Saving the CPU state in the 
absence of checkpointing storage doesn't make a lot of sense.

Regards,

Anthony Liguori

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Daniel P. Berrange]

On Wed, Nov 05, 2008 at 08:09:56AM -0600, Anthony Liguori wrote:
> Chris Lalancette wrote:
> >Anthony Liguori wrote:
> >  
> >>Charles Duffy wrote:
> >>    
> >>>Howdy.
> >>>
> >>>KVM's live migration support used to support an exec: protoco, 
> >>>allowing  either a completely arbitrary transport or a mechanism for 
> >>>storing system state to a separate file for later resurrection, being 
> >>>used in the latter by libvirt's qemu driver. I notice that this 
> >>>support no longer exists in the current codebase.
> >>>Would a patch reimplementing this support be welcome?
> >>>      
> >>Absolutely.  I wasn't aware that libvirt used the exec: protocol.  I'm 
> >>somewhat surprised by that.  What did it use it for?
> >>    
> >
> >Hm, I didn't realize it used it either, but it uses it for doing a save 
> >operation:
> >
> >static int qemudDomainSave(virDomainPtr dom,
> >                           const char *path) {
> >...
> >    if (asprintf (&command, "migrate \"exec:"
> >                  "dd of='%s' oflag=append conv=notrunc 2>/dev/null"
> >                  "\"", safe_path) == -1) {
> >        qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> >                         "%s", _("out of memory"));
> >        VIR_FREE(safe_path);
> >        return -1;
> >    }
> >    free(safe_path);
> >
> >    if (qemudMonitorCommand(driver, vm, command, &info) < 0) {
> >...
> >
> >I don't know if there is a better way to do that with the qemu monitor, to 
> >avoid
> > using migrate altogether.  In any case, I'm sure other people would find 
> > exec:
> >useful as well.
> >  
> 
> Sure.  It would make sense for it to use file: instead of exec: though.
> 
> However, why is it not just using savevm?  Saving the CPU state in the 
> absence of checkpointing storage doesn't make a lot of sense.

This isn't for snapshotting, so checkpointing of storage is unneccessary.
This is just straight save+restore, akin to hibernate-to-disk for a 
physical machine. 

The savevm command only works with qcow files, which is not guarenteed
to be available in this context.

We do intend to support real snapshotting of VMs at some point, for which
"savevm" could be useful

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

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Jamie Lokier]

Daniel P. Berrange wrote:
> > However, why is it not just using savevm?  Saving the CPU state in the 
> > absence of checkpointing storage doesn't make a lot of sense.
> 
> This isn't for snapshotting, so checkpointing of storage is unneccessary.
> This is just straight save+restore, akin to hibernate-to-disk for a 
> physical machine. 
> 
> The savevm command only works with qcow files, which is not guarenteed
> to be available in this context.
> 
> We do intend to support real snapshotting of VMs at some point, for which
> "savevm" could be useful

Yes, we had a thread a couple of months ago about why "savevm"
snapshotting in qcow2 is not always appropriate or useful.

-- Jamie

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Anthony Liguori]

Daniel P. Berrange wrote:
> This isn't for snapshotting, so checkpointing of storage is unneccessary.
> This is just straight save+restore, akin to hibernate-to-disk for a 
> physical machine. 
>   

This is a fundamentally broken concept.  You cannot just save the 
guest's memory state without saving the current state of the storage.

I know this exists in Xen, but no one can use it in a serious manner as 
it could very easily lead to data corruption.

Regards,

Anthony Liguori

[Qemu-devel] Re: Live migration - exec: support to be reintroduced?

[Charles Duffy]

Anthony Liguori wrote:
> Daniel P. Berrange wrote:
>> This isn't for snapshotting, so checkpointing of storage is unneccessary.
>> This is just straight save+restore, akin to hibernate-to-disk for a 
>> physical machine.   
> 
> This is a fundamentally broken concept.  You cannot just save the 
> guest's memory state without saving the current state of the storage.

You're requiring the user to manage storage checkpointing themselves in 
this case -- but whether they're using LVM snapshotting or replacing the 
qcow2 backend with an empty file backed by the old store, the user may 
choose to do that themselves rather than pushing the work off to qemu 
(and forcing everything to be stored in a single file); I use (and wish 
not to lose) this functionality myself.

[Qemu-devel] Re: Live migration - exec: support to be reintroduced?

[Charles Duffy]

Anthony Liguori wrote:
> Daniel P. Berrange wrote:
>> This isn't for snapshotting, so checkpointing of storage is unneccessary.
>> This is just straight save+restore, akin to hibernate-to-disk for a 
>> physical machine.   
> 
> This is a fundamentally broken concept.  You cannot just save the 
> guest's memory state without saving the current state of the storage.

Oh -- let me follow up on how this works in libvirt:

The "save" and "restore" commands shut down and start the VM when 
invoked; consequently, no storage changes are made while the VM is down. 
Restoring more than once off the same "save" is not an intended use case.

Re: [Qemu-devel] Re: Live migration - exec: support to be reintroduced?

[Anthony Liguori]

Charles Duffy wrote:
> Anthony Liguori wrote:
>> Daniel P. Berrange wrote:
>>> This isn't for snapshotting, so checkpointing of storage is 
>>> unneccessary.
>>> This is just straight save+restore, akin to hibernate-to-disk for a 
>>> physical machine.   
>>
>> This is a fundamentally broken concept.  You cannot just save the 
>> guest's memory state without saving the current state of the storage.
>
> Oh -- let me follow up on how this works in libvirt:
>
> The "save" and "restore" commands shut down and start the VM when 
> invoked; consequently, no storage changes are made while the VM is 
> down. Restoring more than once off the same "save" is not an intended 
> use case.

I see, that makes more sense then.

Regards,

Anthony Liguori

>
>

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Daniel P. Berrange]

On Tue, Nov 04, 2008 at 11:38:31PM -0600, Anthony Liguori wrote:
> Charles Duffy wrote:
> >Howdy.
> >
> >KVM's live migration support used to support an exec: protoco, 
> >allowing  either a completely arbitrary transport or a mechanism for 
> >storing system state to a separate file for later resurrection, being 
> >used in the latter by libvirt's qemu driver. I notice that this 
> >support no longer exists in the current codebase.
> >Would a patch reimplementing this support be welcome?
> 
> Absolutely.  I wasn't aware that libvirt used the exec: protocol.  I'm 
> somewhat surprised by that.  What did it use it for?

We used it to implement save-to-file, exec'ing 'dd'. I'd rather prefer
that we had a more explicit 'migrate to a file' capability rather than
the hack of execing dd.

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

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Avi Kivity]

Daniel P. Berrange wrote:
> We used it to implement save-to-file, exec'ing 'dd'. I'd rather prefer
> that we had a more explicit 'migrate to a file' capability rather than
> the hack of execing dd.
>   

(qemu) acceptfd blah
receives an file descriptor from the monitor using SCM_RIGHTS and 
assigns it to the tag 'blah'
(qemu) migrate fd:blah
migrates to the fd denominated by the tag 'blah'
(qemu) closefd blah

You can use literal numbers instead of tags to refer to file descriptors 
inherited from the environment:

(qemu) migrate fd:17

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Anthony Liguori]

Avi Kivity wrote:
> Daniel P. Berrange wrote:
>> We used it to implement save-to-file, exec'ing 'dd'. I'd rather prefer
>> that we had a more explicit 'migrate to a file' capability rather than
>> the hack of execing dd.
>>   
>
> (qemu) acceptfd blah
> receives an file descriptor from the monitor using SCM_RIGHTS and 
> assigns it to the tag 'blah'
> (qemu) migrate fd:blah
> migrates to the fd denominated by the tag 'blah'
> (qemu) closefd blah

This could be generally useful for other things too (like char device 
redirection).

I'm not sure how many people would really use it though, you're talking 
about serious unix-fu here.

Regards,

Anthony Liguori

> You can use literal numbers instead of tags to refer to file 
> descriptors inherited from the environment:
>
> (qemu) migrate fd:17
>

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Daniel P. Berrange]

On Wed, Nov 05, 2008 at 08:14:01AM -0600, Anthony Liguori wrote:
> Avi Kivity wrote:
> >Daniel P. Berrange wrote:
> >>We used it to implement save-to-file, exec'ing 'dd'. I'd rather prefer
> >>that we had a more explicit 'migrate to a file' capability rather than
> >>the hack of execing dd.
> >>  
> >
> >(qemu) acceptfd blah
> >receives an file descriptor from the monitor using SCM_RIGHTS and 
> >assigns it to the tag 'blah'
> >(qemu) migrate fd:blah
> >migrates to the fd denominated by the tag 'blah'
> >(qemu) closefd blah
> 
> This could be generally useful for other things too (like char device 
> redirection).
> 
> I'm not sure how many people would really use it though, you're talking 
> about serious unix-fu here.

It is useful from a security point of view - it means QEMU doesn't
need to be given permissions to create files, merely append to an
opened file handle.

On a related note, Avi pointed out to me that SCM_RIGHTS fd passing
would be important for NIC hotplug to allow parity with -net arg
on the command line. If the QEMU process is running unprivileged,
it will not have rights to create TAP devices & giving it a setuid()
network script is not desirable. The management app invoking QEMU
could open the TAP device, and do any setup before passing the FD
to the NIC hotplug command in the monitor. 

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

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Anthony Liguori]

Daniel P. Berrange wrote:
> It is useful from a security point of view - it means QEMU doesn't
> need to be given permissions to create files, merely append to an
> opened file handle.
>
> On a related note, Avi pointed out to me that SCM_RIGHTS fd passing
> would be important for NIC hotplug to allow parity with -net arg
> on the command line. If the QEMU process is running unprivileged,
> it will not have rights to create TAP devices & giving it a setuid()
> network script is not desirable. The management app invoking QEMU
> could open the TAP device, and do any setup before passing the FD
> to the NIC hotplug command in the monitor. 
>   

Yup.  If libvirt has a use case for it, then I'm more than happy to 
review patches.

I'm always looking for an excuse to use SCM_RIGHTS :-)

I think the monitor interface could use improvement.  I think it would 
look better as:

(qemu) receivefd /path/to/unix/socket
/* waits until it receives an fd on /path/to/unix/socket */
fd=10
(qemu) closefd 10

Then all of the existing uses of fd= can be preserved.

I like the idea of using a temporary socket because you don't have to 
rely on the monitor being on a unix socket.  This will be especially 
useful when we can support tunneling the monitor through VNC.

Regards,

Anthony Liguori

> Daniel
>   

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Daniel P. Berrange]

On Wed, Nov 05, 2008 at 09:19:02AM -0600, Anthony Liguori wrote:
> Daniel P. Berrange wrote:
> >It is useful from a security point of view - it means QEMU doesn't
> >need to be given permissions to create files, merely append to an
> >opened file handle.
> >
> >On a related note, Avi pointed out to me that SCM_RIGHTS fd passing
> >would be important for NIC hotplug to allow parity with -net arg
> >on the command line. If the QEMU process is running unprivileged,
> >it will not have rights to create TAP devices & giving it a setuid()
> >network script is not desirable. The management app invoking QEMU
> >could open the TAP device, and do any setup before passing the FD
> >to the NIC hotplug command in the monitor. 
> >  
> 
> Yup.  If libvirt has a use case for it, then I'm more than happy to 
> review patches.
> 
> I'm always looking for an excuse to use SCM_RIGHTS :-)
> 
> I think the monitor interface could use improvement.  I think it would 
> look better as:
> 
> (qemu) receivefd /path/to/unix/socket
> /* waits until it receives an fd on /path/to/unix/socket */
> fd=10
> (qemu) closefd 10
> 
> Then all of the existing uses of fd= can be preserved.
> 
> I like the idea of using a temporary socket because you don't have to 
> rely on the monitor being on a unix socket.  This will be especially 
> useful when we can support tunneling the monitor through VNC.

Yes, except that I would wouldn't want to pass "/path/to/unix/socket"
via the monitor - that allows any process which can access that path
to potentially open the socket & intercept the credentials. 

If I wasn't using a UNIX socket for the monitor already, then I'd
want to be able to pass a FD to a unix socket on the command line
so I know who's on the other end of it. 

eg, if i was using a hypothetical VNC server transport for the 
monitor, then perhaps allow

    --monitor vnc,scmrightsfd=7

Or, an explicit --scmrights arg for it

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

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Anthony Liguori]

Daniel P. Berrange wrote:
> On Wed, Nov 05, 2008 at 09:19:02AM -0600, Anthony Liguori wrote:
>   
>> Daniel P. Berrange wrote:
>>     
>>> It is useful from a security point of view - it means QEMU doesn't
>>> need to be given permissions to create files, merely append to an
>>> opened file handle.
>>>
>>> On a related note, Avi pointed out to me that SCM_RIGHTS fd passing
>>> would be important for NIC hotplug to allow parity with -net arg
>>> on the command line. If the QEMU process is running unprivileged,
>>> it will not have rights to create TAP devices & giving it a setuid()
>>> network script is not desirable. The management app invoking QEMU
>>> could open the TAP device, and do any setup before passing the FD
>>> to the NIC hotplug command in the monitor. 
>>>  
>>>       
>> Yup.  If libvirt has a use case for it, then I'm more than happy to 
>> review patches.
>>
>> I'm always looking for an excuse to use SCM_RIGHTS :-)
>>
>> I think the monitor interface could use improvement.  I think it would 
>> look better as:
>>
>> (qemu) receivefd /path/to/unix/socket
>> /* waits until it receives an fd on /path/to/unix/socket */
>> fd=10
>> (qemu) closefd 10
>>
>> Then all of the existing uses of fd= can be preserved.
>>
>> I like the idea of using a temporary socket because you don't have to 
>> rely on the monitor being on a unix socket.  This will be especially 
>> useful when we can support tunneling the monitor through VNC.
>>     
>
> Yes, except that I would wouldn't want to pass "/path/to/unix/socket"
> via the monitor - that allows any process which can access that path
> to potentially open the socket & intercept the credentials. 
>   

I don't really see this as a problem.

> If I wasn't using a UNIX socket for the monitor already, then I'd
> want to be able to pass a FD to a unix socket on the command line
> so I know who's on the other end of it. 
> eg, if i was using a hypothetical VNC server transport for the 
> monitor, then perhaps allow
>
>     --monitor vnc,scmrightsfd=7
>
> Or, an explicit --scmrights arg for it
>   

Yeah, the later sounds more reasonable, but I don't really know that I 
agree that the only safe thing to do is here pass an fd on exec.  Please 
describe an attack scenario where a user could access 
/path/to/unix/socket that isn't trusted?

Regards,

Anthony Liguori

> Daniel
>   

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Avi Kivity]

Anthony Liguori wrote:
>
> I think the monitor interface could use improvement.  I think it would 
> look better as:
>
> (qemu) receivefd /path/to/unix/socket
> /* waits until it receives an fd on /path/to/unix/socket */
> fd=10
> (qemu) closefd 10
>
> Then all of the existing uses of fd= can be preserved.
>

Ouch, my fd tags are much better as they don't require the user to read 
the fd number.

> I like the idea of using a temporary socket because you don't have to 
> rely on the monitor being on a unix socket.  This will be especially 
> useful when we can support tunneling the monitor through VNC.

I can't really see a user creating an fd and passing it to qemu through 
a unix domain socket.  This is for management apps.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] Live migration - exec: support to be reintroduced?

[Avi Kivity]

Anthony Liguori wrote:
>> (qemu) acceptfd blah
>> receives an file descriptor from the monitor using SCM_RIGHTS and 
>> assigns it to the tag 'blah'
>> (qemu) migrate fd:blah
>> migrates to the fd denominated by the tag 'blah'
>> (qemu) closefd blah
>
> This could be generally useful for other things too (like char device 
> redirection).
>
> I'm not sure how many people would really use it though, you're 
> talking about serious unix-fu here.

While SCM_RIGHTS does not feature on prime-time television, it's hardly 
complex.  I mean, compare with tty and sessions and group leaders and 
all that goo.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

[Qemu-devel] [PATCH] Re: Live migration - exec: support to be reintroduced?

[Charles Duffy]

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]

Anthony Liguori wrote:
> Charles Duffy wrote:
>> KVM's live migration support used to support an exec: protoco, 
>> allowing  either a completely arbitrary transport or a mechanism for 
>> storing system state to a separate file for later resurrection, being 
>> used in the latter by libvirt's qemu driver. I notice that this 
>> support no longer exists in the current codebase.
>> Would a patch reimplementing this support be welcome?
> 
> Absolutely.

Please see attached patch.

I'm new to the codebase (and a bit rusty on my C), so I expect some 
rework to be needed on this one; let me know what needs doing, and I'll 
try to update it as time permits.

[-- Attachment #2: qemu-live_migration_exec_support-r1.patch --]
[-- Type: text/x-patch, Size: 11185 bytes --]

diff --git a/Makefile.target b/Makefile.target
index 031ab45..d844d9c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -596,7 +596,7 @@ endif
 ifdef CONFIG_WIN32
 OBJS+=block-raw-win32.o
 else
-OBJS+=block-raw-posix.o
+OBJS+=block-raw-posix.o migration-exec.o
 endif
 
 LIBS+=-lz
diff --git a/hw/hw.h b/hw/hw.h
index 99d4b8d..eab7bb4 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -35,6 +35,8 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
                          QEMUFileRateLimit *rate_limit);
 QEMUFile *qemu_fopen(const char *filename, const char *mode);
 QEMUFile *qemu_fopen_socket(int fd);
+QEMUFile *qemu_popen(FILE *popen_file, const char *mode);
+QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
 void qemu_fflush(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
 void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
diff --git a/migration-exec.c b/migration-exec.c
new file mode 100644
index 0000000..2d04008
--- /dev/null
+++ b/migration-exec.c
@@ -0,0 +1,295 @@
+/*
+ * QEMU live migration
+ *
+ * Copyright IBM, Corp. 2008
+ * Copyright Dell MessageOne 2008
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *  Charles Duffy     <charles_duffy@messageone.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "qemu_socket.h"
+#include "migration.h"
+#include "qemu-char.h"
+#include "sysemu.h"
+#include "console.h"
+#include "buffered_file.h"
+#include "block.h"
+
+//#define DEBUG_MIGRATION_EXEC
+
+typedef struct FdMigrationState
+{
+    MigrationState mig_state;
+    QEMUFile *file, *popen_file;
+    int64_t bandwidth_limit;
+    int fd;
+    int detach;
+    int state;
+} FdMigrationState;
+
+#ifdef DEBUG_MIGRATION_EXEC
+#define dprintf(fmt, ...) \
+    do { printf("migration-exec: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define dprintf(fmt, ...) \
+    do { } while (0)
+#endif
+
+static void exec_cleanup(FdMigrationState *s)
+{
+    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+
+    if (s->file) {
+        dprintf("closing file\n");
+        qemu_fclose(s->file);
+    }
+
+    if (s->fd != -1)
+        close(s->fd);
+
+    /* Don't resume monitor until we've flushed all of the buffers */
+    if (s->detach == 2) {
+        monitor_resume();
+        s->detach = 0;
+    }
+
+    s->fd = -1;
+}
+
+static void exec_error(FdMigrationState *s)
+{
+    dprintf("setting error state\n");
+    s->state = MIG_STATE_ERROR;
+    exec_cleanup(s);
+}
+
+static void fd_put_notify(void *opaque)
+{
+    FdMigrationState *s = opaque;
+
+    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+    qemu_file_put_notify(s->file);
+}
+
+static ssize_t fd_put_buffer(void *opaque, const void *data, size_t size)
+{
+    FdMigrationState *s = opaque;
+    ssize_t ret;
+
+    do {
+        ret = write(s->fd, data, size);
+    } while (ret == -1 && (errno == EINTR || errno == EWOULDBLOCK));
+
+    if (ret == -1)
+        ret = -errno;
+
+    if (ret == -EAGAIN)
+        qemu_set_fd_handler2(s->fd, NULL, NULL, fd_put_notify, s);
+
+    return ret;
+}
+
+static int fd_close(void *opaque)
+{
+    FdMigrationState *s = opaque;
+    dprintf("fd_close\n");
+    if (s->popen_file) {
+        qemu_fclose(s->popen_file);
+        s->popen_file = NULL;
+        s->fd = -1;
+    }
+    return 0;
+}
+
+static void fd_wait_for_unfreeze(void *opaque)
+{
+    FdMigrationState *s = opaque;
+    int ret;
+
+    dprintf("wait for unfreeze\n");
+    if (s->state != MIG_STATE_ACTIVE)
+        return;
+
+    do {
+        fd_set wfds;
+
+        FD_ZERO(&wfds);
+        FD_SET(s->fd, &wfds);
+
+        ret = select(s->fd + 1, NULL, &wfds, NULL, NULL);
+    } while (ret == -1 && errno == EINTR);
+}
+
+static void fd_put_ready(void *opaque)
+{
+    FdMigrationState *s = opaque;
+
+    if (s->state != MIG_STATE_ACTIVE) {
+        dprintf("put_ready returning because of non-active state\n");
+        return;
+    }
+
+    dprintf("iterate\n");
+    if (qemu_savevm_state_iterate(s->file) == 1) {
+        dprintf("done iterating\n");
+        vm_stop(0);
+
+        bdrv_flush_all();
+        qemu_savevm_state_complete(s->file);
+        s->state = MIG_STATE_COMPLETED;
+        exec_cleanup(s);
+    }
+}
+
+static void exec_connect_migrate(FdMigrationState *s)
+{
+    int ret;
+
+    s->file = qemu_fopen_ops_buffered(s,
+                                      s->bandwidth_limit,
+                                      fd_put_buffer,
+                                      fd_put_ready,
+                                      fd_wait_for_unfreeze,
+                                      fd_close);
+
+    dprintf("beginning savevm\n");
+    ret = qemu_savevm_state_begin(s->file);
+    if (ret < 0) {
+        dprintf("failed, %d\n", ret);
+        exec_error(s);
+        return;
+    }
+
+    fd_put_ready(s);
+}
+
+static FdMigrationState *to_fms(MigrationState *mig_state)
+{
+    return container_of(mig_state, FdMigrationState, mig_state);
+}
+
+static int exec_get_status(MigrationState *mig_state)
+{
+    FdMigrationState *s = to_fms(mig_state);
+
+    return s->state;
+}
+
+static void exec_cancel(MigrationState *mig_state)
+{
+    FdMigrationState *s = to_fms(mig_state);
+
+    if (s->state != MIG_STATE_ACTIVE)
+        return;
+
+    dprintf("cancelling migration\n");
+
+    s->state = MIG_STATE_CANCELLED;
+
+    exec_cleanup(s);
+}
+
+static void exec_release(MigrationState *mig_state)
+{
+    FdMigrationState *s = to_fms(mig_state);
+
+    dprintf("releasing state\n");
+   
+    if (s->state == MIG_STATE_ACTIVE) {
+        s->state = MIG_STATE_CANCELLED;
+        exec_cleanup(s);
+    }
+    free(s);
+}
+
+MigrationState *exec_start_outgoing_migration(const char *command,
+                                             int64_t bandwidth_limit,
+                                             int async)
+{
+    FdMigrationState *s;
+    FILE *f;
+
+    s = qemu_mallocz(sizeof(*s));
+    if (s == NULL) {
+        dprintf("Unable to allocate FdMigrationState\n");
+        goto err;
+    }
+
+    f = popen(command, "w");
+    if (f == NULL) {
+        dprintf("Unable to popen exec target\n");
+        goto err_after_alloc;
+    }
+
+    s->fd = fileno(f);
+    if (s->fd == -1) {
+        dprintf("Unable to retrieve file descriptor for popen'd handle\n");
+        goto err_after_open;
+    }
+
+    if (fcntl(s->fd, F_SETFD, O_NONBLOCK) == -1) {
+        dprintf("Unable to set nonblocking mode on file descriptor\n");
+        goto err_after_open;
+    }
+
+    s->popen_file = qemu_popen(f, "w");
+
+    s->mig_state.cancel = exec_cancel;
+    s->mig_state.get_status = exec_get_status;
+    s->mig_state.release = exec_release;
+
+    s->state = MIG_STATE_ACTIVE;
+    s->detach = !async;
+    s->bandwidth_limit = bandwidth_limit;
+
+    if (s->detach == 1) {
+        dprintf("detaching from monitor\n");
+        monitor_suspend();
+        s->detach = 2;
+    }
+
+    exec_connect_migrate(s);
+    return &s->mig_state;
+
+err_after_open:
+    pclose(f);
+err_after_alloc:
+    qemu_free(s);
+err:
+    return NULL;
+}
+
+int exec_start_incoming_migration(const char *command)
+{
+    int ret;
+    QEMUFile *f;
+
+    dprintf("Attempting to start an incoming migration\n");
+    f = qemu_popen_cmd(command, "r");
+    if(f == NULL) {
+        dprintf("Unable to apply qemu wrapper to popen file\n");
+        return -errno;
+    }
+    vm_stop(0); /* just in case */
+    ret = qemu_loadvm_state(f);
+    if (ret < 0) {
+        fprintf(stderr, "load of migration failed\n");
+        goto err;
+    }
+    qemu_announce_self();
+    dprintf("successfully loaded vm state\n");
+    vm_start();
+    qemu_fclose(f);
+    return 0;
+
+err:
+    qemu_fclose(f);
+    return -errno;
+}
diff --git a/migration.c b/migration.c
index 9e6c437..fb4d3d3 100644
--- a/migration.c
+++ b/migration.c
@@ -26,6 +26,10 @@ void qemu_start_incoming_migration(const char *uri)
 
     if (strstart(uri, "tcp:", &p))
         tcp_start_incoming_migration(p);
+#if !defined(WIN32)
+    else if (strstart(uri, "exec:", &p))
+        exec_start_incoming_migration(p);
+#endif
     else
         fprintf(stderr, "unknown migration protocol: %s\n", uri);
 }
@@ -37,6 +41,10 @@ void do_migrate(int detach, const char *uri)
 
     if (strstart(uri, "tcp:", &p))
         s = tcp_start_outgoing_migration(p, max_throttle, detach);
+#if !defined(WIN32)
+    else if (strstart(uri, "exec:", &p))
+        s = exec_start_outgoing_migration(p, max_throttle, detach);
+#endif
     else
         term_printf("unknown migration protocol: %s\n", uri);
 
diff --git a/migration.h b/migration.h
index 9947f6a..5008323 100644
--- a/migration.h
+++ b/migration.h
@@ -39,6 +39,12 @@ void do_migrate_set_speed(const char *value);
 
 void do_info_migrate(void);
 
+int exec_start_incoming_migration(const char *host_port);
+
+MigrationState *exec_start_outgoing_migration(const char *host_port,
+					     int64_t bandwidth_limit,
+					     int detach);
+
 int tcp_start_incoming_migration(const char *host_port);
 
 MigrationState *tcp_start_outgoing_migration(const char *host_port,
diff --git a/vl.c b/vl.c
index 31dd3d7..b49e694 100644
--- a/vl.c
+++ b/vl.c
@@ -2816,6 +2816,12 @@ struct QEMUFile {
     int has_error;
 };
 
+typedef struct QEMUFilePopen
+{
+    FILE *popen_file;
+    QEMUFile *file;
+} QEMUFilePopen;
+
 typedef struct QEMUFileSocket
 {
     int fd;
@@ -2844,6 +2850,64 @@ static int socket_close(void *opaque)
     return 0;
 }
 
+static int popen_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
+{
+    QEMUFilePopen *s = opaque;
+    return fwrite(buf, 1, size, s->popen_file);
+}
+
+static int popen_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+    QEMUFilePopen *s = opaque;
+    return fread(buf, 1, size, s->popen_file);
+}
+
+static int popen_close(void *opaque)
+{
+    QEMUFilePopen *s = opaque;
+    pclose(s->popen_file);
+    qemu_free(s);
+    return 0;
+}
+
+QEMUFile *qemu_popen(FILE *popen_file, const char *mode)
+{
+    QEMUFilePopen *s;
+
+    if (popen_file == NULL || mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != 0) {
+        fprintf(stderr, "qemu_popen: Argument validity check failed\n");
+        return NULL;
+    }
+
+    s = qemu_mallocz(sizeof(QEMUFilePopen));
+    if (!s) {
+        fprintf(stderr, "qemu_popen: malloc failed\n");
+        return NULL;
+    }
+
+    s->popen_file = popen_file;
+
+    if(mode[0] == 'r') {
+        s->file = qemu_fopen_ops(s, NULL, popen_get_buffer, popen_close, NULL);
+    } else {
+        s->file = qemu_fopen_ops(s, popen_put_buffer, NULL, popen_close, NULL);
+    }
+    fprintf(stderr, "qemu_popen: returning result of qemu_fopen_ops\n");
+    return s->file;
+}
+
+QEMUFile *qemu_popen_cmd(const char *command, const char *mode)
+{
+    FILE *popen_file;
+
+    popen_file = popen(command, mode);
+    if(popen_file == NULL) {
+        return NULL;
+    }
+
+    return qemu_popen(popen_file, mode);
+}
+
 QEMUFile *qemu_fopen_socket(int fd)
 {
     QEMUFileSocket *s = qemu_mallocz(sizeof(QEMUFileSocket));

[Qemu-devel] Re: [PATCH] Re: Live migration - exec: support to be reintroduced? (r2)

[Charles Duffy]

[-- Attachment #1: Type: text/plain, Size: 109 bytes --]

The attached version refactors to reduce the amount of duplicate code 
between migrate-tcp and migrate-exec.

[-- Attachment #2: qemu-live_migration_exec_support-r2.patch --]
[-- Type: text/x-patch, Size: 19870 bytes --]

diff --git a/Makefile.target b/Makefile.target
index 031ab45..d844d9c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -596,7 +596,7 @@ endif
 ifdef CONFIG_WIN32
 OBJS+=block-raw-win32.o
 else
-OBJS+=block-raw-posix.o
+OBJS+=block-raw-posix.o migration-exec.o
 endif
 
 LIBS+=-lz
diff --git a/hw/hw.h b/hw/hw.h
index 99d4b8d..eab7bb4 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -35,6 +35,8 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
                          QEMUFileRateLimit *rate_limit);
 QEMUFile *qemu_fopen(const char *filename, const char *mode);
 QEMUFile *qemu_fopen_socket(int fd);
+QEMUFile *qemu_popen(FILE *popen_file, const char *mode);
+QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
 void qemu_fflush(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
 void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
diff --git a/migration-exec.c b/migration-exec.c
new file mode 100644
index 0000000..5d0b215
--- /dev/null
+++ b/migration-exec.c
@@ -0,0 +1,141 @@
+/*
+ * QEMU live migration
+ *
+ * Copyright IBM, Corp. 2008
+ * Copyright Dell MessageOne 2008
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *  Charles Duffy     <charles_duffy@messageone.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "qemu_socket.h"
+#include "migration.h"
+#include "qemu-char.h"
+#include "sysemu.h"
+#include "console.h"
+#include "buffered_file.h"
+#include "block.h"
+
+//#define DEBUG_MIGRATION_EXEC
+
+#ifdef DEBUG_MIGRATION_EXEC
+#define dprintf(fmt, ...) \
+    do { printf("migration-exec: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define dprintf(fmt, ...) \
+    do { } while (0)
+#endif
+
+static int file_errno(FdMigrationState *s)
+{
+    return errno;
+}
+
+static int file_write(FdMigrationState *s, const void * buf, size_t size)
+{
+    return write(s->fd, buf, size);
+}
+
+static int exec_close(FdMigrationState *s)
+{
+    dprintf("exec_close\n");
+    if (s->opaque) {
+        qemu_fclose(s->opaque);
+        s->opaque = NULL;
+        s->fd = -1;
+    }
+    return 0;
+}
+
+MigrationState *exec_start_outgoing_migration(const char *command,
+                                             int64_t bandwidth_limit,
+                                             int async)
+{
+    FdMigrationState *s;
+    FILE *f;
+
+    s = qemu_mallocz(sizeof(*s));
+    if (s == NULL) {
+        dprintf("Unable to allocate FdMigrationState\n");
+        goto err;
+    }
+
+    f = popen(command, "w");
+    if (f == NULL) {
+        dprintf("Unable to popen exec target\n");
+        goto err_after_alloc;
+    }
+
+    s->fd = fileno(f);
+    if (s->fd == -1) {
+        dprintf("Unable to retrieve file descriptor for popen'd handle\n");
+        goto err_after_open;
+    }
+
+    if (fcntl(s->fd, F_SETFD, O_NONBLOCK) == -1) {
+        dprintf("Unable to set nonblocking mode on file descriptor\n");
+        goto err_after_open;
+    }
+
+    s->opaque = qemu_popen(f, "w");
+
+    s->get_error = file_errno;
+    s->write = file_write;
+    s->mig_state.cancel = migrate_fd_cancel;
+    s->mig_state.get_status = migrate_fd_get_status;
+    s->mig_state.release = migrate_fd_release;
+
+    s->state = MIG_STATE_ACTIVE;
+    s->detach = !async;
+    s->bandwidth_limit = bandwidth_limit;
+
+    if (s->detach == 1) {
+        dprintf("detaching from monitor\n");
+        monitor_suspend();
+        s->detach = 2;
+    }
+
+    migrate_fd_connect(s);
+    return &s->mig_state;
+
+err_after_open:
+    pclose(f);
+err_after_alloc:
+    qemu_free(s);
+err:
+    return NULL;
+}
+
+int exec_start_incoming_migration(const char *command)
+{
+    int ret;
+    QEMUFile *f;
+
+    dprintf("Attempting to start an incoming migration\n");
+    f = qemu_popen_cmd(command, "r");
+    if(f == NULL) {
+        dprintf("Unable to apply qemu wrapper to popen file\n");
+        return -errno;
+    }
+    vm_stop(0); /* just in case */
+    ret = qemu_loadvm_state(f);
+    if (ret < 0) {
+        fprintf(stderr, "load of migration failed\n");
+        goto err;
+    }
+    qemu_announce_self();
+    dprintf("successfully loaded vm state\n");
+    vm_start();
+    qemu_fclose(f);
+    return 0;
+
+err:
+    qemu_fclose(f);
+    return -errno;
+}
diff --git a/migration-tcp.c b/migration-tcp.c
index f13dc0d..100644b 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -22,16 +22,6 @@
 
 //#define DEBUG_MIGRATION_TCP
 
-typedef struct FdMigrationState
-{
-    MigrationState mig_state;
-    QEMUFile *file;
-    int64_t bandwidth_limit;
-    int fd;
-    int detach;
-    int state;
-} FdMigrationState;
-
 #ifdef DEBUG_MIGRATION_TCP
 #define dprintf(fmt, ...) \
     do { printf("migration-tcp: " fmt, ## __VA_ARGS__); } while (0)
@@ -40,64 +30,19 @@ typedef struct FdMigrationState
     do { } while (0)
 #endif
 
-static void tcp_cleanup(FdMigrationState *s)
+static int socket_errno(FdMigrationState *s)
 {
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-
-    if (s->file) {
-        dprintf("closing file\n");
-        qemu_fclose(s->file);
-    }
-
-    if (s->fd != -1)
-        close(s->fd);
-
-    /* Don't resume monitor until we've flushed all of the buffers */
-    if (s->detach == 2) {
-        monitor_resume();
-        s->detach = 0;
-    }
-
-    s->fd = -1;
-}
-
-static void tcp_error(FdMigrationState *s)
-{
-    dprintf("setting error state\n");
-    s->state = MIG_STATE_ERROR;
-    tcp_cleanup(s);
-}
-
-static void fd_put_notify(void *opaque)
-{
-    FdMigrationState *s = opaque;
-
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-    qemu_file_put_notify(s->file);
+    return (s->get_error(s));
 }
 
-static ssize_t fd_put_buffer(void *opaque, const void *data, size_t size)
+static int socket_write(FdMigrationState *s, const void * buf, size_t size)
 {
-    FdMigrationState *s = opaque;
-    ssize_t ret;
-
-    do {
-        ret = send(s->fd, data, size, 0);
-    } while (ret == -1 && (socket_error() == EINTR || socket_error() == EWOULDBLOCK));
-
-    if (ret == -1)
-        ret = -socket_error();
-
-    if (ret == -EAGAIN)
-        qemu_set_fd_handler2(s->fd, NULL, NULL, fd_put_notify, s);
-
-    return ret;
+    return send(s->fd, buf, size, 0);
 }
 
-static int fd_close(void *opaque)
+static int tcp_close(FdMigrationState *s)
 {
-    FdMigrationState *s = opaque;
-    dprintf("fd_close\n");
+    dprintf("tcp_close\n");
     if (s->fd != -1) {
         close(s->fd);
         s->fd = -1;
@@ -105,67 +50,6 @@ static int fd_close(void *opaque)
     return 0;
 }
 
-static void fd_wait_for_unfreeze(void *opaque)
-{
-    FdMigrationState *s = opaque;
-    int ret;
-
-    dprintf("wait for unfreeze\n");
-    if (s->state != MIG_STATE_ACTIVE)
-        return;
-
-    do {
-        fd_set wfds;
-
-        FD_ZERO(&wfds);
-        FD_SET(s->fd, &wfds);
-
-        ret = select(s->fd + 1, NULL, &wfds, NULL, NULL);
-    } while (ret == -1 && socket_error() == EINTR);
-}
-
-static void fd_put_ready(void *opaque)
-{
-    FdMigrationState *s = opaque;
-
-    if (s->state != MIG_STATE_ACTIVE) {
-        dprintf("put_ready returning because of non-active state\n");
-        return;
-    }
-
-    dprintf("iterate\n");
-    if (qemu_savevm_state_iterate(s->file) == 1) {
-        dprintf("done iterating\n");
-        vm_stop(0);
-
-        bdrv_flush_all();
-        qemu_savevm_state_complete(s->file);
-        s->state = MIG_STATE_COMPLETED;
-        tcp_cleanup(s);
-    }
-}
-
-static void tcp_connect_migrate(FdMigrationState *s)
-{
-    int ret;
-
-    s->file = qemu_fopen_ops_buffered(s,
-                                      s->bandwidth_limit,
-                                      fd_put_buffer,
-                                      fd_put_ready,
-                                      fd_wait_for_unfreeze,
-                                      fd_close);
-
-    dprintf("beginning savevm\n");
-    ret = qemu_savevm_state_begin(s->file);
-    if (ret < 0) {
-        dprintf("failed, %d\n", ret);
-        tcp_error(s);
-        return;
-    }
-
-    fd_put_ready(s);
-}
 
 static void tcp_wait_for_connect(void *opaque)
 {
@@ -176,60 +60,21 @@ static void tcp_wait_for_connect(void *opaque)
     dprintf("connect completed\n");
     do {
         ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, &val, &valsize);
-    } while (ret == -1 && socket_error() == EINTR);
+    } while (ret == -1 && (s->get_error(s)) == EINTR);
 
     if (ret < 0) {
-        tcp_error(s);
+        migrate_fd_error(s);
         return;
     }
 
     qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
 
     if (val == 0)
-        tcp_connect_migrate(s);
+        migrate_fd_connect(s);
     else {
         dprintf("error connecting %d\n", val);
-        tcp_error(s);
-    }
-}
-
-static FdMigrationState *to_fms(MigrationState *mig_state)
-{
-    return container_of(mig_state, FdMigrationState, mig_state);
-}
-
-static int tcp_get_status(MigrationState *mig_state)
-{
-    FdMigrationState *s = to_fms(mig_state);
-
-    return s->state;
-}
-
-static void tcp_cancel(MigrationState *mig_state)
-{
-    FdMigrationState *s = to_fms(mig_state);
-
-    if (s->state != MIG_STATE_ACTIVE)
-        return;
-
-    dprintf("cancelling migration\n");
-
-    s->state = MIG_STATE_CANCELLED;
-
-    tcp_cleanup(s);
-}
-
-static void tcp_release(MigrationState *mig_state)
-{
-    FdMigrationState *s = to_fms(mig_state);
-
-    dprintf("releasing state\n");
-   
-    if (s->state == MIG_STATE_ACTIVE) {
-        s->state = MIG_STATE_CANCELLED;
-        tcp_cleanup(s);
+        migrate_fd_error(s);
     }
-    free(s);
 }
 
 MigrationState *tcp_start_outgoing_migration(const char *host_port,
@@ -247,9 +92,12 @@ MigrationState *tcp_start_outgoing_migration(const char *host_port,
     if (s == NULL)
         return NULL;
 
-    s->mig_state.cancel = tcp_cancel;
-    s->mig_state.get_status = tcp_get_status;
-    s->mig_state.release = tcp_release;
+    s->get_error = socket_errno;
+    s->write = socket_write;
+    s->close = tcp_close;
+    s->mig_state.cancel = migrate_fd_cancel;
+    s->mig_state.get_status = migrate_fd_get_status;
+    s->mig_state.release = migrate_fd_release;
 
     s->state = MIG_STATE_ACTIVE;
     s->detach = !async;
@@ -271,7 +119,7 @@ MigrationState *tcp_start_outgoing_migration(const char *host_port,
     do {
         ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
         if (ret == -1)
-            ret = -socket_error();
+            ret = -(s->get_error(s));
 
         if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
             qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
@@ -283,7 +131,7 @@ MigrationState *tcp_start_outgoing_migration(const char *host_port,
         qemu_free(s);
         return NULL;
     } else if (ret >= 0)
-        tcp_connect_migrate(s);
+        migrate_fd_connect(s);
 
     return &s->mig_state;
 }
diff --git a/migration.c b/migration.c
index 9e6c437..0a3ca1c 100644
--- a/migration.c
+++ b/migration.c
@@ -14,6 +14,19 @@
 #include "qemu-common.h"
 #include "migration.h"
 #include "console.h"
+#include "buffered_file.h"
+#include "sysemu.h"
+#include "block.h"
+
+//#define DEBUG_MIGRATION
+
+#ifdef DEBUG_MIGRATION
+#define dprintf(fmt, ...) \
+    do { printf("migration: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define dprintf(fmt, ...) \
+    do { } while (0)
+#endif
 
 /* Migration speed throttling */
 static uint32_t max_throttle = (32 << 20);
@@ -26,6 +39,10 @@ void qemu_start_incoming_migration(const char *uri)
 
     if (strstart(uri, "tcp:", &p))
         tcp_start_incoming_migration(p);
+#if !defined(WIN32)
+    else if (strstart(uri, "exec:", &p))
+        exec_start_incoming_migration(p);
+#endif
     else
         fprintf(stderr, "unknown migration protocol: %s\n", uri);
 }
@@ -37,6 +54,10 @@ void do_migrate(int detach, const char *uri)
 
     if (strstart(uri, "tcp:", &p))
         s = tcp_start_outgoing_migration(p, max_throttle, detach);
+#if !defined(WIN32)
+    else if (strstart(uri, "exec:", &p))
+        s = exec_start_outgoing_migration(p, max_throttle, detach);
+#endif
     else
         term_printf("unknown migration protocol: %s\n", uri);
 
@@ -101,3 +122,159 @@ void do_info_migrate(void)
     }
 }
 
+/* shared migration helpers */
+
+void migrate_fd_error(FdMigrationState *s)
+{
+    dprintf("setting error state\n");
+    s->state = MIG_STATE_ERROR;
+    migrate_fd_cleanup(s);
+}
+
+void migrate_fd_cleanup(FdMigrationState *s)
+{
+    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+
+    if (s->file) {
+        dprintf("closing file\n");
+        qemu_fclose(s->file);
+    }
+
+    if (s->fd != -1)
+        close(s->fd);
+
+    /* Don't resume monitor until we've flushed all of the buffers */
+    if (s->detach == 2) {
+        monitor_resume();
+        s->detach = 0;
+    }
+
+    s->fd = -1;
+}
+
+void migrate_fd_put_notify(void *opaque)
+{
+    FdMigrationState *s = opaque;
+
+    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+    qemu_file_put_notify(s->file);
+}
+
+ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
+{
+    FdMigrationState *s = opaque;
+    ssize_t ret;
+
+    do {
+        ret = s->write(s, data, size);
+    } while (ret == -1 && ((s->get_error(s)) == EINTR || (s->get_error(s)) == EWOULDBLOCK));
+
+    if (ret == -1)
+        ret = -(s->get_error(s));
+
+    if (ret == -EAGAIN)
+        qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+
+    return ret;
+}
+
+void migrate_fd_connect(FdMigrationState *s)
+{
+    int ret;
+
+    s->file = qemu_fopen_ops_buffered(s,
+                                      s->bandwidth_limit,
+                                      migrate_fd_put_buffer,
+                                      migrate_fd_put_ready,
+                                      migrate_fd_wait_for_unfreeze,
+                                      migrate_fd_close);
+
+    dprintf("beginning savevm\n");
+    ret = qemu_savevm_state_begin(s->file);
+    if (ret < 0) {
+        dprintf("failed, %d\n", ret);
+        migrate_fd_error(s);
+        return;
+    }
+
+    migrate_fd_put_ready(s);
+}
+
+void migrate_fd_put_ready(void *opaque)
+{
+    FdMigrationState *s = opaque;
+
+    if (s->state != MIG_STATE_ACTIVE) {
+        dprintf("put_ready returning because of non-active state\n");
+        return;
+    }
+
+    dprintf("iterate\n");
+    if (qemu_savevm_state_iterate(s->file) == 1) {
+        dprintf("done iterating\n");
+        vm_stop(0);
+
+        bdrv_flush_all();
+        qemu_savevm_state_complete(s->file);
+        s->state = MIG_STATE_COMPLETED;
+        migrate_fd_cleanup(s);
+    }
+}
+
+int migrate_fd_get_status(MigrationState *mig_state)
+{
+    FdMigrationState *s = migrate_to_fms(mig_state);
+    return s->state;
+}
+
+void migrate_fd_cancel(MigrationState *mig_state)
+{
+    FdMigrationState *s = migrate_to_fms(mig_state);
+
+    if (s->state != MIG_STATE_ACTIVE)
+        return;
+
+    dprintf("cancelling migration\n");
+
+    s->state = MIG_STATE_CANCELLED;
+
+    migrate_fd_cleanup(s);
+}
+
+void migrate_fd_release(MigrationState *mig_state)
+{
+    FdMigrationState *s = migrate_to_fms(mig_state);
+
+    dprintf("releasing state\n");
+   
+    if (s->state == MIG_STATE_ACTIVE) {
+        s->state = MIG_STATE_CANCELLED;
+        migrate_fd_cleanup(s);
+    }
+    free(s);
+}
+
+void migrate_fd_wait_for_unfreeze(void *opaque)
+{
+    FdMigrationState *s = opaque;
+    int ret;
+
+    dprintf("wait for unfreeze\n");
+    if (s->state != MIG_STATE_ACTIVE)
+        return;
+
+    do {
+        fd_set wfds;
+
+        FD_ZERO(&wfds);
+        FD_SET(s->fd, &wfds);
+
+        ret = select(s->fd + 1, NULL, &wfds, NULL, NULL);
+    } while (ret == -1 && (s->get_error(s)) == EINTR);
+}
+
+int migrate_fd_close(void *opaque)
+{
+    FdMigrationState *s = opaque;
+    return s->close(s);
+}
diff --git a/migration.h b/migration.h
index 9947f6a..953ec70 100644
--- a/migration.h
+++ b/migration.h
@@ -29,6 +29,22 @@ struct MigrationState
     void (*release)(MigrationState *s);
 };
 
+typedef struct FdMigrationState FdMigrationState;
+
+struct FdMigrationState
+{
+    MigrationState mig_state;
+    int64_t bandwidth_limit;
+    QEMUFile *file;
+    int fd;
+    int detach;
+    int state;
+    int (*get_error)(struct FdMigrationState*);
+    int (*close)(struct FdMigrationState*);
+    int (*write)(struct FdMigrationState*, const void *, size_t);
+    void *opaque;
+};
+
 void qemu_start_incoming_migration(const char *uri);
 
 void do_migrate(int detach, const char *uri);
@@ -39,11 +55,44 @@ void do_migrate_set_speed(const char *value);
 
 void do_info_migrate(void);
 
+int exec_start_incoming_migration(const char *host_port);
+
+MigrationState *exec_start_outgoing_migration(const char *host_port,
+					     int64_t bandwidth_limit,
+					     int detach);
+
 int tcp_start_incoming_migration(const char *host_port);
 
 MigrationState *tcp_start_outgoing_migration(const char *host_port,
 					     int64_t bandwidth_limit,
 					     int detach);
 
+void migrate_fd_error(FdMigrationState *s);
+
+void migrate_fd_cleanup(FdMigrationState *s);
+
+void migrate_fd_put_notify(void *opaque);
+
+ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size);
+
+void migrate_fd_connect(FdMigrationState *s);
+
+void migrate_fd_put_ready(void *opaque);
+
+int migrate_fd_get_status(MigrationState *mig_state);
+
+void migrate_fd_cancel(MigrationState *mig_state);
+
+void migrate_fd_release(MigrationState *mig_state);
+
+void migrate_fd_wait_for_unfreeze(void *opaque);
+
+int migrate_fd_close(void *opaque);
+
+static inline FdMigrationState *migrate_to_fms(MigrationState *mig_state)
+{
+    return container_of(mig_state, FdMigrationState, mig_state);
+}
+
 #endif
 
diff --git a/vl.c b/vl.c
index 31dd3d7..b49e694 100644
--- a/vl.c
+++ b/vl.c
@@ -2816,6 +2816,12 @@ struct QEMUFile {
     int has_error;
 };
 
+typedef struct QEMUFilePopen
+{
+    FILE *popen_file;
+    QEMUFile *file;
+} QEMUFilePopen;
+
 typedef struct QEMUFileSocket
 {
     int fd;
@@ -2844,6 +2850,64 @@ static int socket_close(void *opaque)
     return 0;
 }
 
+static int popen_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int size)
+{
+    QEMUFilePopen *s = opaque;
+    return fwrite(buf, 1, size, s->popen_file);
+}
+
+static int popen_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+    QEMUFilePopen *s = opaque;
+    return fread(buf, 1, size, s->popen_file);
+}
+
+static int popen_close(void *opaque)
+{
+    QEMUFilePopen *s = opaque;
+    pclose(s->popen_file);
+    qemu_free(s);
+    return 0;
+}
+
+QEMUFile *qemu_popen(FILE *popen_file, const char *mode)
+{
+    QEMUFilePopen *s;
+
+    if (popen_file == NULL || mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != 0) {
+        fprintf(stderr, "qemu_popen: Argument validity check failed\n");
+        return NULL;
+    }
+
+    s = qemu_mallocz(sizeof(QEMUFilePopen));
+    if (!s) {
+        fprintf(stderr, "qemu_popen: malloc failed\n");
+        return NULL;
+    }
+
+    s->popen_file = popen_file;
+
+    if(mode[0] == 'r') {
+        s->file = qemu_fopen_ops(s, NULL, popen_get_buffer, popen_close, NULL);
+    } else {
+        s->file = qemu_fopen_ops(s, popen_put_buffer, NULL, popen_close, NULL);
+    }
+    fprintf(stderr, "qemu_popen: returning result of qemu_fopen_ops\n");
+    return s->file;
+}
+
+QEMUFile *qemu_popen_cmd(const char *command, const char *mode)
+{
+    FILE *popen_file;
+
+    popen_file = popen(command, mode);
+    if(popen_file == NULL) {
+        return NULL;
+    }
+
+    return qemu_popen(popen_file, mode);
+}
+
 QEMUFile *qemu_fopen_socket(int fd)
 {
     QEMUFileSocket *s = qemu_mallocz(sizeof(QEMUFileSocket));

[Qemu-devel] Re: [PATCH] Re: Live migration - exec: support to be reintroduced? (r2)

[Charles Duffy]

Left out a suggested comment and Signed-off-by last time.

---

Reintroduce migrate-to-exec: support

KVM's live migration support included support for exec: URLs, allowing 
system state to be written or received via an arbitrary popen()ed 
subprocess. This provides a convenient way to pipe state through a 
compression algorithm or an arbitrary network transport on its way to 
its destination, and a convenient way to write state to disk; libvirt's 
qemu driver currently uses migration to exec: targets for this latter 
purpose.

This version of the patch refactors now-common code from migrate-tcp.c 
into migrate.c.

Signed-off-by: Charles Duffy <charles_duffy@messageone.com>

Re: [Qemu-devel] Re: [PATCH] Re: Live migration - exec: support to be reintroduced? (r2)

[Anthony Liguori]

Charles Duffy wrote:
> The attached version refactors to reduce the amount of duplicate code 
> between migrate-tcp and migrate-exec.

Applied.  Thanks.

Although I'd rather see a qemu_fopen_file() or something similar that 
took a FILE * instead of qemu_popen[_cmd]().  A follow up patch would be 
appreciated.

Regards,

Anthony Liguori