[libvirt] [PATCH 0/2] Allow saving QEMU libvirt state to a pipe

This series introduce flag VIR_DOMAIN_SAVE_PIPE to enable command 'save' to write to PIPE. Base upon patches from Roy Keene <rkeene@knightpoint.com> with some fixes. Change from original patch: 1) Check whether the specified path is a PIPE. 2) Rebase on upstream. 3) Add doc for virsh command Chen Hanxiao (2): qemu: Allow saving QEMU libvirt state to a pipe virsh: introduce flage --pipe for save command include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 71 ++++++++++++++++++++++++++++++---------- tools/virsh-domain.c | 6 ++++ tools/virsh.pod | 6 +++- 4 files changed, 65 insertions(+), 19 deletions(-) -- 2.7.4

From: Chen Hanxiao <chenhanxiao@gmail.com> Base upon patches from Roy Keene <rkeene@knightpoint.com> Currently qemuDomainSaveMemory can save vm's config and memory to fd. It writes a magic QEMU_SAVE_PARTIAL firstly, then re-open it to change QEMU_SAVE_PARTIAL as QEMU_SAVE_MAGIC after a success write. For pipes this is not possible, attempting to re-open the pipe will not connect you to the same consumer. Seeking is also not possible on a pipe. This patch introduce VIR_DOMAIN_SAVE_PIPE. If set, write QEMU_SAVE_MAGIC directly. Try to write a regular file with VIR_DOMAIN_SAVE_PIPE is not supportted. This is useful to me for saving a VM state directly to Ceph RBD images without having an intermediate file. Cc: Roy Keene <rkeene@knightpoint.com> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 71 ++++++++++++++++++++++++++++++---------- 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a8435ab..c3e4c15 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1169,6 +1169,7 @@ typedef enum { VIR_DOMAIN_SAVE_BYPASS_CACHE = 1 << 0, /* Avoid file system cache pollution */ VIR_DOMAIN_SAVE_RUNNING = 1 << 1, /* Favor running over paused */ VIR_DOMAIN_SAVE_PAUSED = 1 << 2, /* Favor paused over running */ + VIR_DOMAIN_SAVE_PIPE = 1 << 3, /* Output is a pipe */ } virDomainSaveRestoreFlags; int virDomainSave (virDomainPtr domain, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3517aa2..58422ac 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3054,14 +3054,15 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, virQEMUSaveHeader header; bool bypassSecurityDriver = false; bool needUnlink = false; + bool canReopen = true; int ret = -1; int fd = -1; int directFlag = 0; virFileWrapperFdPtr wrapperFd = NULL; unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING; + struct stat statbuf; memset(&header, 0, sizeof(header)); - memcpy(header.magic, QEMU_SAVE_PARTIAL, sizeof(header.magic)); header.version = QEMU_SAVE_VERSION; header.was_running = was_running ? 1 : 0; header.compressed = compressed; @@ -3077,6 +3078,27 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, goto cleanup; } } + + /* + * Determine if this file is a PIPE, which could not be reopen. + */ + if (virFileExists(path)) { + fd = qemuOpenFile(driver, vm, path, O_RDONLY | O_NONBLOCK, NULL, NULL); + if (fd < 0) + goto cleanup; + if (fstat(fd, &statbuf) < 0) + goto cleanup; + if (S_ISFIFO(statbuf.st_mode)) { + if (flags & VIR_DOMAIN_SAVE_PIPE) { + canReopen = false; + } else { + virReportSystemError(EINVAL, _("%s is not PIPE"), path); + goto cleanup; + } + } + VIR_FORCE_CLOSE(fd); + } + fd = qemuOpenFile(driver, vm, path, O_WRONLY | O_TRUNC | O_CREAT | directFlag, &needUnlink, &bypassSecurityDriver); @@ -3089,6 +3111,16 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags))) goto cleanup; + /* Set the header magic. + * If the fd could be reopen, we'll update the magic after + * the saving completes successfully. + * Otherwise, include the final magic here, like pipe. + */ + if (canReopen) + memcpy(header.magic, QEMU_SAVE_PARTIAL, sizeof(header.magic)); + else + memcpy(header.magic, QEMU_SAVE_MAGIC, sizeof(header.magic)); + /* Write header to file, followed by XML */ if (qemuDomainSaveHeader(fd, path, domXML, &header) < 0) goto cleanup; @@ -3097,28 +3129,30 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, if (qemuMigrationToFile(driver, vm, fd, compressedpath, asyncJob) < 0) goto cleanup; - /* Touch up file header to mark image complete. */ + if (canReopen) { + /* Touch up file header to mark image complete. */ - /* Reopen the file to touch up the header, since we aren't set - * up to seek backwards on wrapperFd. The reopened fd will - * trigger a single page of file system cache pollution, but - * that's acceptable. */ - if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, _("unable to close %s"), path); - goto cleanup; - } + /* Reopen the file to touch up the header, since we aren't set + * up to seek backwards on wrapperFd. The reopened fd will + * trigger a single page of file system cache pollution, but + * that's acceptable. */ + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, _("unable to close %s"), path); + goto cleanup; + } - if (virFileWrapperFdClose(wrapperFd) < 0) - goto cleanup; + if (virFileWrapperFdClose(wrapperFd) < 0) + goto cleanup; - if ((fd = qemuOpenFile(driver, vm, path, O_WRONLY, NULL, NULL)) < 0) - goto cleanup; + if ((fd = qemuOpenFile(driver, vm, path, O_WRONLY, NULL, NULL)) < 0) + goto cleanup; - memcpy(header.magic, QEMU_SAVE_MAGIC, sizeof(header.magic)); + memcpy(header.magic, QEMU_SAVE_MAGIC, sizeof(header.magic)); - if (safewrite(fd, &header, sizeof(header)) != sizeof(header)) { - virReportSystemError(errno, _("unable to write %s"), path); - goto cleanup; + if (safewrite(fd, &header, sizeof(header)) != sizeof(header)) { + virReportSystemError(errno, _("unable to write %s"), path); + goto cleanup; + } } if (VIR_CLOSE(fd) < 0) { @@ -3345,6 +3379,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | VIR_DOMAIN_SAVE_RUNNING | + VIR_DOMAIN_SAVE_PIPE | VIR_DOMAIN_SAVE_PAUSED, -1); cfg = virQEMUDriverGetConfig(driver); -- 2.7.4

On Sat, Dec 03, 2016 at 17:45:47 +0800, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
Base upon patches from Roy Keene <rkeene@knightpoint.com>
Currently qemuDomainSaveMemory can save vm's config and memory to fd. It writes a magic QEMU_SAVE_PARTIAL firstly, then re-open it to change QEMU_SAVE_PARTIAL as QEMU_SAVE_MAGIC after a success write.
For pipes this is not possible, attempting to re-open the pipe will not connect you to the same consumer. Seeking is also not possible on a pipe.
This patch introduce VIR_DOMAIN_SAVE_PIPE. If set, write QEMU_SAVE_MAGIC directly. Try to write a regular file with VIR_DOMAIN_SAVE_PIPE is not supportted.
This is useful to me for saving a VM state directly to Ceph RBD images without having an intermediate file.
Cc: Roy Keene <rkeene@knightpoint.com> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 71 ++++++++++++++++++++++++++++++---------- 2 files changed, 54 insertions(+), 18 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a8435ab..c3e4c15 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1169,6 +1169,7 @@ typedef enum { VIR_DOMAIN_SAVE_BYPASS_CACHE = 1 << 0, /* Avoid file system cache pollution */ VIR_DOMAIN_SAVE_RUNNING = 1 << 1, /* Favor running over paused */ VIR_DOMAIN_SAVE_PAUSED = 1 << 2, /* Favor paused over running */ + VIR_DOMAIN_SAVE_PIPE = 1 << 3, /* Output is a pipe */
It doesn't have necessarily to be a pipe.
} virDomainSaveRestoreFlags;
int virDomainSave (virDomainPtr domain, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3517aa2..58422ac 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3054,14 +3054,15 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, virQEMUSaveHeader header; bool bypassSecurityDriver = false; bool needUnlink = false; + bool canReopen = true; int ret = -1; int fd = -1; int directFlag = 0; virFileWrapperFdPtr wrapperFd = NULL; unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING; + struct stat statbuf;
memset(&header, 0, sizeof(header)); - memcpy(header.magic, QEMU_SAVE_PARTIAL, sizeof(header.magic)); header.version = QEMU_SAVE_VERSION; header.was_running = was_running ? 1 : 0; header.compressed = compressed; @@ -3077,6 +3078,27 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, goto cleanup; } } + + /* + * Determine if this file is a PIPE, which could not be reopen. + */ + if (virFileExists(path)) { + fd = qemuOpenFile(driver, vm, path, O_RDONLY | O_NONBLOCK, NULL, NULL); + if (fd < 0) + goto cleanup; + if (fstat(fd, &statbuf) < 0) + goto cleanup; + if (S_ISFIFO(statbuf.st_mode)) {
You should not try to check this. If the user wishes to write the complete header right away, then we should obey it and not have to check prior to do so.
+ if (flags & VIR_DOMAIN_SAVE_PIPE) { + canReopen = false; + } else { + virReportSystemError(EINVAL, _("%s is not PIPE"), path); + goto cleanup; + } + } + VIR_FORCE_CLOSE(fd); + } + fd = qemuOpenFile(driver, vm, path, O_WRONLY | O_TRUNC | O_CREAT | directFlag, &needUnlink, &bypassSecurityDriver);

At 2016-12-08 20:08:11, "Peter Krempa" <pkrempa@redhat.com> wrote:
On Sat, Dec 03, 2016 at 17:45:47 +0800, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
Base upon patches from Roy Keene <rkeene@knightpoint.com>
Currently qemuDomainSaveMemory can save vm's config and memory to fd. It writes a magic QEMU_SAVE_PARTIAL firstly, then re-open it to change QEMU_SAVE_PARTIAL as QEMU_SAVE_MAGIC after a success write.
For pipes this is not possible, attempting to re-open the pipe will not connect you to the same consumer. Seeking is also not possible on a pipe.
This patch introduce VIR_DOMAIN_SAVE_PIPE. If set, write QEMU_SAVE_MAGIC directly. Try to write a regular file with VIR_DOMAIN_SAVE_PIPE is not supportted.
This is useful to me for saving a VM state directly to Ceph RBD images without having an intermediate file.
Cc: Roy Keene <rkeene@knightpoint.com> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 71 ++++++++++++++++++++++++++++++---------- 2 files changed, 54 insertions(+), 18 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a8435ab..c3e4c15 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1169,6 +1169,7 @@ typedef enum { VIR_DOMAIN_SAVE_BYPASS_CACHE = 1 << 0, /* Avoid file system cache pollution */ VIR_DOMAIN_SAVE_RUNNING = 1 << 1, /* Favor running over paused */ VIR_DOMAIN_SAVE_PAUSED = 1 << 2, /* Favor paused over running */ + VIR_DOMAIN_SAVE_PIPE = 1 << 3, /* Output is a pipe */
It doesn't have necessarily to be a pipe.
We need a flag to specify that we need to write QEMU_SAVE_MAGIC directly. Any suggestion for the name of this flag?
} virDomainSaveRestoreFlags;
int virDomainSave (virDomainPtr domain,
[snip]
+ + /* + * Determine if this file is a PIPE, which could not be reopen. + */ + if (virFileExists(path)) { + fd = qemuOpenFile(driver, vm, path, O_RDONLY | O_NONBLOCK, NULL, NULL); + if (fd < 0) + goto cleanup; + if (fstat(fd, &statbuf) < 0) + goto cleanup; + if (S_ISFIFO(statbuf.st_mode)) {
You should not try to check this. If the user wishes to write the complete header right away, then we should obey it and not have to check prior to do so.
My concern is that when we write to a pipe/fifo, if no one read it, we will hang. We should prevent from doing this only when we specify a flag. Regards, - Chen
+ if (flags & VIR_DOMAIN_SAVE_PIPE) { + canReopen = false; + } else { + virReportSystemError(EINVAL, _("%s is not PIPE"), path); + goto cleanup; + } + } + VIR_FORCE_CLOSE(fd); + } + fd = qemuOpenFile(driver, vm, path, O_WRONLY | O_TRUNC | O_CREAT | directFlag, &needUnlink, &bypassSecurityDriver);

From: Chen Hanxiao <chenhanxiao@gmail.com> Base upon patches from Roy Keene <rkeene@knightpoint.com> This patch introduces --pipe flag for save command. We could saving a VM state directly to Ceph RBD images without having an intermediate file. How to test: # fifo="$(mktemp -u)"; mkfifo "${fifo}" && virsh save --pipe cirros "${fifo}" & # cat "${fifo}" | rbd --id cinder import - rbd/test1234 & wait; rm -f "${fifo}" Cc: Roy Keene <rkeene@knightpoint.com> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- tools/virsh-domain.c | 6 ++++++ tools/virsh.pod | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e359bfc..6ac3edf 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4170,6 +4170,10 @@ static const vshCmdOptDef opts_save[] = { .type = VSH_OT_BOOL, .help = N_("set domain to be paused on restore") }, + {.name = "pipe", + .type = VSH_OT_BOOL, + .help = N_("the file being saved to is a pipe") + }, {.name = "verbose", .type = VSH_OT_BOOL, .help = N_("display the progress of save") @@ -4206,6 +4210,8 @@ doSave(void *opaque) flags |= VIR_DOMAIN_SAVE_RUNNING; if (vshCommandOptBool(cmd, "paused")) flags |= VIR_DOMAIN_SAVE_PAUSED; + if (vshCommandOptBool(cmd, "pipe")) + flags |= VIR_DOMAIN_SAVE_PIPE; if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0) goto out; diff --git a/tools/virsh.pod b/tools/virsh.pod index 247d235..8419db8 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1893,7 +1893,7 @@ have also reverted all storage volumes back to the same contents as when the state file was created. =item B<save> I<domain> I<state-file> [I<--bypass-cache>] [I<--xml> B<file>] -[{I<--running> | I<--paused>}] [I<--verbose>] +[{I<--running> | I<--paused>}] [I<--pipe>] [I<--verbose>] Saves a running domain (RAM, but not disk state) to a state file so that it can be restored @@ -1908,6 +1908,10 @@ with B<domjobabort> command (sent by another virsh instance). Another option is to send SIGINT (usually with C<Ctrl-C>) to the virsh process running B<save> command. I<--verbose> displays the progress of save. +Usually B<save> command will save the domain's state as a regular file. +Flag I<--pipe> could let B<save> dumps domain's state to a PIPE. +Try to write a PIPE without I<--pipe> would error out. + This is roughly equivalent to doing a hibernate on a running computer, with all the same limitations. Open network connections may be severed upon restore, as TCP timeouts may have expired. -- 2.7.4

At 2016-12-03 17:45:46, "Chen Hanxiao" <chen_han_xiao@126.com> wrote:
This series introduce flag VIR_DOMAIN_SAVE_PIPE to enable command 'save' to write to PIPE.
Base upon patches from Roy Keene <rkeene@knightpoint.com> with some fixes.
Change from original patch: 1) Check whether the specified path is a PIPE. 2) Rebase on upstream. 3) Add doc for virsh command
Chen Hanxiao (2): qemu: Allow saving QEMU libvirt state to a pipe virsh: introduce flage --pipe for save command
include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 71 ++++++++++++++++++++++++++++++---------- tools/virsh-domain.c | 6 ++++ tools/virsh.pod | 6 +++- 4 files changed, 65 insertions(+), 19 deletions(-)
ping Regards, - Chen
participants (2)
-
Chen Hanxiao
-
Peter Krempa