diff mbox

Allow saving QEMU libvirt state to a pipe

Message ID 33fc9eed-3b31-bb7d-aa35-595cd3ae04d6@knightpoint.com
State New
Headers show

Commit Message

Roy Keene Nov. 7, 2016, 3:19 p.m. UTC
On 11/07/2016 03:27 AM, Peter Krempa wrote:
> On Fri, Nov 04, 2016 at 15:20:44 -0500, Roy Keene wrote:

>> All,

>>

>>       Currently the "virsh save" command opens a file to save a domain's

>> XML and memory state, does the save, and then re-opens the file to

>> simulate seeking to the beginning to update the header to indicate that

>> the file is complete.

>>

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

>>

>> Attached is a patch that detects if the file being written to is not

>> seekable, and if so writes the completed header initially, then writes

>> the normal data, and avoids re-opening the file.

>>

>> This is useful to me for saving a VM state directly to Ceph RBD images

>> without having an intermediate file.

>>

>> I've tested this patch successfully (against v1.3.5) for both saving and

>> restoring a domain:

>>

>> # *( fifo="$(mktemp -u)"; mkfifo "${fifo}" && virsh save one-0 "${fifo}"

>> & cat "${fifo}" | rbd import - rbd/test1234 & wait; rm -f "${fifo}" )*

>>

>> Domain one-0 saved to /tmp/tmp.HK4hChiQqB

>>

>> #

>>

>> # *( fifo="$(mktemp -u)"; mkfifo "${fifo}" && rbd export rbd/test1234 -

>>   > "${fifo}" & virsh restore "${fifo}" & wait; rm -f "${fifo}" ) *

>> Exporting image: 100% complete...done.

>> Domain restored from /tmp/tmp.0YaUZ5Y2yT

>>

>> # *virsh list*

>>    Id    Name                           State

>> ----------------------------------------------------

>>    11    one-0                          running

>> #

>>

>> -- 

>> 	Roy Keene

>> 	Knight Point Systems, LLC

>> 	Service-Disabled Veteran-Owned Business

>> 	1775 Wiehle Avenue Suite 101 | Reston, VA 20190

>> 	c: 813-956-3808    f: 571-266-3106

>> 	www.knightpoint.com

>> 	DHS EAGLE II Prime Contractor: FC1 SDVOSB Track

>> 	GSA Schedule 70 SDVOSB: GS-35F-0646S

>> 	GSA MOBIS Schedule: GS-10F-0404Y

>> 	ISO 20000 / ISO 27001

>>

>> 	Notice: This e-mail message, including any attachments, is for the

>> 	sole use of the intended recipient(s) and may contain confidential

>> 	and privileged information.  Any unauthorized review,  copy,  use,

>> 	disclosure, or distribution is STRICTLY prohibited. If you are not

>> 	the intended recipient,  please contact the sender by reply e-mail

>> 	and destroy all copies of the original message.

> Trimming this on public postings might be a good idea.

This is currently required by my company's policy.  I'll work with them 
to try to come up with exceptions to this requirement.
>

>> diff --no-dereference -uNr libvirt-1.3.5.orig/src/qemu/qemu_driver.c libvirt-1.3.5-savepipe/src/qemu/qemu_driver.c

>> --- libvirt-1.3.5.orig/src/qemu/qemu_driver.c	2016-05-28 11:47:33.000000000 -0500

>> +++ libvirt-1.3.5-savepipe/src/qemu/qemu_driver.c	2016-11-04 10:54:15.160805261 -0500

> This is a very old version of libvirt.

It's 5 months old...
> Please rebase the changes against

> the git master since they can't be applied:

>

> Applying: Allow saving QEMU libvirt state to a pipe

> error: patch failed: src/qemu/qemu_driver.c:3124

> error: src/qemu/qemu_driver.c: patch does not apply

> Patch failed at 0001 Allow saving QEMU libvirt state to a pipe

>

> Additionally please generate the patches using git so that we have a

> proper commit message.


[rkeene@kps-rsk-laptop libvirt]$ patch -p1 < ~/devel/aurae/node/root/packages/libvirt/patches/libvirt-1.3.5-savepipe.diff
patching file src/qemu/qemu_driver.c
Hunk #1 succeeded at 3051 (offset -29 lines).
Hunk #2 succeeded at 3059 (offset -29 lines).
Hunk #3 succeeded at 3080 (offset -29 lines).
Hunk #4 succeeded at 3114 with fuzz 1 (offset -30 lines).
[rkeene@kps-rsk-laptop libvirt]$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
Changes not staged for commit:
   (use "git add <file>..." to update what will be committed)
   (use "git checkout -- <file>..." to discard changes in working directory)

         modified:   src/qemu/qemu_driver.c

no changes added to commit (use "git add" and/or "git commit -a")
[rkeene@kps-rsk-laptop libvirt]$

Attach is the output of "git diff" (2savepipe)
>> @@ -3080,6 +3080,7 @@

>>       virQEMUSaveHeader header;

>>       bool bypassSecurityDriver = false;

>>       bool needUnlink = false;

>> +    bool canReopen = true;

>>       int ret = -1;

>>       int fd = -1;

>>       int directFlag = 0;

>> @@ -3087,7 +3088,6 @@

>>       unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING;

>>   

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

>> @@ -3109,12 +3109,32 @@

>>       if (fd < 0)

>>           goto cleanup;

>>   

>> +    /*

>> +     * Determine if this file can be re-opened

>> +     *

>> +     * Right now we just try to seek into it, if that fails then that means we

>> +     * are probably not dealing with a regular file here and we probably

>> +     * cannot reopen it.

>> +     */

> Another option would be to pass a flag to the API, where the user would

> declare by using such flag that the output is valid only on success of

> the command and thus rewriting the header is not necessary.

I thought about that, but it was more work to pass that flag around... 
initially I just wanted to see if this was the only problem with 
streaming the save.
>

>> +    if (lseek(fd, 0, SEEK_SET) == ((off_t) -1)) {

>> +        canReopen = false;

>> +    }

> This will fail make syntax-check on current master since single line if

> statement bodies should not be put into braces.

I'll add a comment inside the braces to make it pass the syntax-check.  
I'm against unbraced expressions.
>

>> +

>>       if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, fd) < 0)

>>           goto cleanup;

>>   

>>       if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags)))

>>           goto cleanup;

>>   

>> +    /* Set the header magic */

>> +    if (canReopen) {

>> +        /* We will update the magic after the saving completes successfully */

>> +        memcpy(header.magic, QEMU_SAVE_PARTIAL, sizeof(header.magic));

>> +    } else {

>> +        /* If we cannot re-open the output, include the final magic here */

>> +        memcpy(header.magic, QEMU_SAVE_MAGIC, sizeof(header.magic));

> This is okay though, comments are counted as a line too.

>

>> +    }

>> +

>>       /* Write header to file, followed by XML */

>>       if (qemuDomainSaveHeader(fd, path, domXML, &header) < 0)

>>           goto cleanup;

> The rest looks reasonable. I'd slightly more prefer the use of the flag

> in comparison to auto detection.

Attached is a competing patch for the flag, I agree it's better 
(3savepipe).  It is untested.
> Thanks for your patch thoug.

>

> Peter


-- 
	Roy Keene
	Knight Point Systems, LLC
	Service-Disabled Veteran-Owned Business
	1775 Wiehle Avenue Suite 101 | Reston, VA 20190
	c: 813-956-3808    f: 571-266-3106
	www.knightpoint.com
	DHS EAGLE II Prime Contractor: FC1 SDVOSB Track
	GSA Schedule 70 SDVOSB: GS-35F-0646S
	GSA MOBIS Schedule: GS-10F-0404Y
	ISO 20000 / ISO 27001

	Notice: This e-mail message, including any attachments, is for the
	sole use of the intended recipient(s) and may contain confidential
	and privileged information.  Any unauthorized review,  copy,  use,
	disclosure, or distribution is STRICTLY prohibited. If you are not
	the intended recipient,  please contact the sender by reply e-mail
	and destroy all copies of the original message.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Pavel Hrdina Nov. 7, 2016, 4:05 p.m. UTC | #1
On Mon, Nov 07, 2016 at 09:19:23AM -0600, Roy Keene wrote:
> 

> 

> On 11/07/2016 03:27 AM, Peter Krempa wrote:

> > On Fri, Nov 04, 2016 at 15:20:44 -0500, Roy Keene wrote:

> >> All,

> >>

> >>       Currently the "virsh save" command opens a file to save a domain's

> >> XML and memory state, does the save, and then re-opens the file to

> >> simulate seeking to the beginning to update the header to indicate that

> >> the file is complete.

> >>

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

> >>

> >> Attached is a patch that detects if the file being written to is not

> >> seekable, and if so writes the completed header initially, then writes

> >> the normal data, and avoids re-opening the file.

> >>

> >> This is useful to me for saving a VM state directly to Ceph RBD images

> >> without having an intermediate file.

> >>

> >> I've tested this patch successfully (against v1.3.5) for both saving and

> >> restoring a domain:

> >>

> >> # *( fifo="$(mktemp -u)"; mkfifo "${fifo}" && virsh save one-0 "${fifo}"

> >> & cat "${fifo}" | rbd import - rbd/test1234 & wait; rm -f "${fifo}" )*

> >>

> >> Domain one-0 saved to /tmp/tmp.HK4hChiQqB

> >>

> >> #

> >>

> >> # *( fifo="$(mktemp -u)"; mkfifo "${fifo}" && rbd export rbd/test1234 -

> >>   > "${fifo}" & virsh restore "${fifo}" & wait; rm -f "${fifo}" ) *

> >> Exporting image: 100% complete...done.

> >> Domain restored from /tmp/tmp.0YaUZ5Y2yT

> >>

> >> # *virsh list*

> >>    Id    Name                           State

> >> ----------------------------------------------------

> >>    11    one-0                          running

> >> #

> >>

> >> -- 

> >> 	Roy Keene

> >> 	Knight Point Systems, LLC

> >> 	Service-Disabled Veteran-Owned Business

> >> 	1775 Wiehle Avenue Suite 101 | Reston, VA 20190

> >> 	c: 813-956-3808    f: 571-266-3106

> >> 	www.knightpoint.com

> >> 	DHS EAGLE II Prime Contractor: FC1 SDVOSB Track

> >> 	GSA Schedule 70 SDVOSB: GS-35F-0646S

> >> 	GSA MOBIS Schedule: GS-10F-0404Y

> >> 	ISO 20000 / ISO 27001

> >>

> >> 	Notice: This e-mail message, including any attachments, is for the

> >> 	sole use of the intended recipient(s) and may contain confidential

> >> 	and privileged information.  Any unauthorized review,  copy,  use,

> >> 	disclosure, or distribution is STRICTLY prohibited. If you are not

> >> 	the intended recipient,  please contact the sender by reply e-mail

> >> 	and destroy all copies of the original message.

> > Trimming this on public postings might be a good idea.

> This is currently required by my company's policy.  I'll work with them 

> to try to come up with exceptions to this requirement.


Hi, thanks for your patches.  That's probably OK, this can be part of the email
if attach the patches.  One more note, please use one empty line before and
after the inline comment while replying to make it easier to read.

> >

> >> diff --no-dereference -uNr libvirt-1.3.5.orig/src/qemu/qemu_driver.c libvirt-1.3.5-savepipe/src/qemu/qemu_driver.c

> >> --- libvirt-1.3.5.orig/src/qemu/qemu_driver.c	2016-05-28 11:47:33.000000000 -0500

> >> +++ libvirt-1.3.5-savepipe/src/qemu/qemu_driver.c	2016-11-04 10:54:15.160805261 -0500

> > This is a very old version of libvirt.

> It's 5 months old...

> > Please rebase the changes against

> > the git master since they can't be applied:

> >

> > Applying: Allow saving QEMU libvirt state to a pipe

> > error: patch failed: src/qemu/qemu_driver.c:3124

> > error: src/qemu/qemu_driver.c: patch does not apply

> > Patch failed at 0001 Allow saving QEMU libvirt state to a pipe

> >

> > Additionally please generate the patches using git so that we have a

> > proper commit message.

> 

> [rkeene@kps-rsk-laptop libvirt]$ patch -p1 < ~/devel/aurae/node/root/packages/libvirt/patches/libvirt-1.3.5-savepipe.diff

> patching file src/qemu/qemu_driver.c

> Hunk #1 succeeded at 3051 (offset -29 lines).

> Hunk #2 succeeded at 3059 (offset -29 lines).

> Hunk #3 succeeded at 3080 (offset -29 lines).

> Hunk #4 succeeded at 3114 with fuzz 1 (offset -30 lines).

> [rkeene@kps-rsk-laptop libvirt]$ git status

> On branch master

> Your branch is up-to-date with 'origin/master'.

> Changes not staged for commit:

>    (use "git add <file>..." to update what will be committed)

>    (use "git checkout -- <file>..." to discard changes in working directory)

> 

>          modified:   src/qemu/qemu_driver.c

> 

> no changes added to commit (use "git add" and/or "git commit -a")

> [rkeene@kps-rsk-laptop libvirt]$


This may apply in your case but still it's a bad workflow for upstream project
especially in case that the code is updated before someone applies the patches
for review.  If you cannot use "git send-email" for policy reasons that's OK,
but newer use "git diff > example.patch".  The proper command to use is "git
format-patch", for example:

You have a master branch and savepipe branch in your local git.  Always make
sure that you have updated git with latest changes using "git fetch origin" or
if you are on master branch "git pull".  As a next step rebase right before
sending the patches your new branch to the origin/master branch.

After those steps you can use "git format-patch origin/master" to get a proper
patch files with proper commit messages that are supposed to be send to the
upstream mailing list.

And one more note, if you send a first version of patches and you receive a
review, please send a new version with proper prefix [PATCH v2] as a new thread,
not as a reply to the original mail.

We all uses similar workflow and is easiest for both sides, for the author and
reviewers.

Pavel

> Attach is the output of "git diff" (2savepipe)

> >> @@ -3080,6 +3080,7 @@

> >>       virQEMUSaveHeader header;

> >>       bool bypassSecurityDriver = false;

> >>       bool needUnlink = false;

> >> +    bool canReopen = true;

> >>       int ret = -1;

> >>       int fd = -1;

> >>       int directFlag = 0;

> >> @@ -3087,7 +3088,6 @@

> >>       unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING;

> >>   

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

> >> @@ -3109,12 +3109,32 @@

> >>       if (fd < 0)

> >>           goto cleanup;

> >>   

> >> +    /*

> >> +     * Determine if this file can be re-opened

> >> +     *

> >> +     * Right now we just try to seek into it, if that fails then that means we

> >> +     * are probably not dealing with a regular file here and we probably

> >> +     * cannot reopen it.

> >> +     */

> > Another option would be to pass a flag to the API, where the user would

> > declare by using such flag that the output is valid only on success of

> > the command and thus rewriting the header is not necessary.

> I thought about that, but it was more work to pass that flag around... 

> initially I just wanted to see if this was the only problem with 

> streaming the save.

> >

> >> +    if (lseek(fd, 0, SEEK_SET) == ((off_t) -1)) {

> >> +        canReopen = false;

> >> +    }

> > This will fail make syntax-check on current master since single line if

> > statement bodies should not be put into braces.

> I'll add a comment inside the braces to make it pass the syntax-check.  

> I'm against unbraced expressions.

> >

> >> +

> >>       if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, fd) < 0)

> >>           goto cleanup;

> >>   

> >>       if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags)))

> >>           goto cleanup;

> >>   

> >> +    /* Set the header magic */

> >> +    if (canReopen) {

> >> +        /* We will update the magic after the saving completes successfully */

> >> +        memcpy(header.magic, QEMU_SAVE_PARTIAL, sizeof(header.magic));

> >> +    } else {

> >> +        /* If we cannot re-open the output, include the final magic here */

> >> +        memcpy(header.magic, QEMU_SAVE_MAGIC, sizeof(header.magic));

> > This is okay though, comments are counted as a line too.

> >

> >> +    }

> >> +

> >>       /* Write header to file, followed by XML */

> >>       if (qemuDomainSaveHeader(fd, path, domXML, &header) < 0)

> >>           goto cleanup;

> > The rest looks reasonable. I'd slightly more prefer the use of the flag

> > in comparison to auto detection.

> Attached is a competing patch for the flag, I agree it's better 

> (3savepipe).  It is untested.

> > Thanks for your patch thoug.

> >

> > Peter

> 

> -- 

> 	Roy Keene

> 	Knight Point Systems, LLC

> 	Service-Disabled Veteran-Owned Business

> 	1775 Wiehle Avenue Suite 101 | Reston, VA 20190

> 	c: 813-956-3808    f: 571-266-3106

> 	www.knightpoint.com

> 	DHS EAGLE II Prime Contractor: FC1 SDVOSB Track

> 	GSA Schedule 70 SDVOSB: GS-35F-0646S

> 	GSA MOBIS Schedule: GS-10F-0404Y

> 	ISO 20000 / ISO 27001

> 

> 	Notice: This e-mail message, including any attachments, is for the

> 	sole use of the intended recipient(s) and may contain confidential

> 	and privileged information.  Any unauthorized review,  copy,  use,

> 	disclosure, or distribution is STRICTLY prohibited. If you are not

> 	the intended recipient,  please contact the sender by reply e-mail

> 	and destroy all copies of the original message.

> 


> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c

> index 38c8414..9f5444c 100644

> --- a/src/qemu/qemu_driver.c

> +++ b/src/qemu/qemu_driver.c

> @@ -3051,6 +3051,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,

>      virQEMUSaveHeader header;

>      bool bypassSecurityDriver = false;

>      bool needUnlink = false;

> +    bool canReopen = true;

>      int ret = -1;

>      int fd = -1;

>      int directFlag = 0;

> @@ -3058,7 +3059,6 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,

>      unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING;

>  

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

> @@ -3080,12 +3080,32 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,

>      if (fd < 0)

>          goto cleanup;

>  

> +    /*

> +     * Determine if this file can be re-opened

> +     */

> +    if (lseek(fd, 0, SEEK_SET) == ((off_t) -1)) {

> +        /* Right now we just try to seek into it, if that fails then that means we

> +         * are probably not dealing with a regular file here and we probably

> +         * cannot reopen it.

> +         */

> +        canReopen = false;

> +    }

> +

>      if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, fd) < 0)

>          goto cleanup;

>  

>      if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags)))

>          goto cleanup;

>  

> +    /* Set the header magic */

> +    if (canReopen) {

> +        /* We will update the magic after the saving completes successfully */

> +        memcpy(header.magic, QEMU_SAVE_PARTIAL, sizeof(header.magic));

> +    } else {

> +        /* If we cannot re-open the output, include the final magic here */

> +        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;

> @@ -3094,28 +3114,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) {


> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h

> index 5f50660..b17615b 100644

> --- a/include/libvirt/libvirt-domain.h

> +++ b/include/libvirt/libvirt-domain.h

> @@ -1058,6 +1058,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 38c8414..261fab1 100644

> --- a/src/qemu/qemu_driver.c

> +++ b/src/qemu/qemu_driver.c

> @@ -3051,6 +3051,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,

>      virQEMUSaveHeader header;

>      bool bypassSecurityDriver = false;

>      bool needUnlink = false;

> +    bool canReopen = true;

>      int ret = -1;

>      int fd = -1;

>      int directFlag = 0;

> @@ -3058,7 +3059,6 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,

>      unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING;

>  

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

> @@ -3080,12 +3080,31 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,

>      if (fd < 0)

>          goto cleanup;

>  

> +    /*

> +     * Determine if the output is a pipe

> +     */

> +    if ((flags & VIR_DOMAIN_SAVE_PIPE)) {

> +        /*

> +         * If the output is a pipe, we cannot re-open it

> +         */

> +        canReopen = false;

> +    }

> +

>      if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, fd) < 0)

>          goto cleanup;

>  

>      if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags)))

>          goto cleanup;

>  

> +    /* Set the header magic */

> +    if (canReopen) {

> +        /* We will update the magic after the saving completes successfully */

> +        memcpy(header.magic, QEMU_SAVE_PARTIAL, sizeof(header.magic));

> +    } else {

> +        /* If we cannot re-open the output, include the final magic here */

> +        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;

> @@ -3094,28 +3113,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) {

> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c

> index 184f64d..32f188a 100644

> --- a/tools/virsh-domain.c

> +++ b/tools/virsh-domain.c

> @@ -4169,6 +4169,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")

> @@ -4205,6 +4209,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;





> --

> libvir-list mailing list

> libvir-list@redhat.com

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

Patch

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 5f50660..b17615b 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1058,6 +1058,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 38c8414..261fab1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3051,6 +3051,7 @@  qemuDomainSaveMemory(virQEMUDriverPtr driver,
     virQEMUSaveHeader header;
     bool bypassSecurityDriver = false;
     bool needUnlink = false;
+    bool canReopen = true;
     int ret = -1;
     int fd = -1;
     int directFlag = 0;
@@ -3058,7 +3059,6 @@  qemuDomainSaveMemory(virQEMUDriverPtr driver,
     unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING;
 
     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;
@@ -3080,12 +3080,31 @@  qemuDomainSaveMemory(virQEMUDriverPtr driver,
     if (fd < 0)
         goto cleanup;
 
+    /*
+     * Determine if the output is a pipe
+     */
+    if ((flags & VIR_DOMAIN_SAVE_PIPE)) {
+        /*
+         * If the output is a pipe, we cannot re-open it
+         */
+        canReopen = false;
+    }
+
     if (virSecurityManagerSetImageFDLabel(driver->securityManager, vm->def, fd) < 0)
         goto cleanup;
 
     if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags)))
         goto cleanup;
 
+    /* Set the header magic */
+    if (canReopen) {
+        /* We will update the magic after the saving completes successfully */
+        memcpy(header.magic, QEMU_SAVE_PARTIAL, sizeof(header.magic));
+    } else {
+        /* If we cannot re-open the output, include the final magic here */
+        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;
@@ -3094,28 +3113,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) {
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 184f64d..32f188a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4169,6 +4169,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")
@@ -4205,6 +4209,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;