Bug 1229255 - Fail to edit the guest's XML associated with a saved state file with virsh save-image-edit command
Summary: Fail to edit the guest's XML associated with a saved state file with virsh sa...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux Advanced Virtualization
Classification: Red Hat
Component: libvirt
Version: 8.2
Hardware: All
OS: Linux
medium
medium
Target Milestone: rc
: 8.3
Assignee: Andrea Bolognani
QA Contact: Yanqiu Zhang
URL:
Whiteboard:
: 1360119 (view as bug list)
Depends On:
Blocks: 1360119
TreeView+ depends on / blocked
 
Reported: 2015-06-08 10:33 UTC by zhenfeng wang
Modified: 2020-11-17 17:45 UTC (History)
17 users (show)

Fixed In Version: libvirt-6.3.0-1.el8
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1360119 (view as bug list)
Environment:
Last Closed: 2020-11-17 17:44:45 UTC
Type: Bug
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
xml of the vm (3.07 KB, text/plain)
2016-07-25 04:18 UTC, yisun
no flags Details

Description zhenfeng wang 2015-06-08 10:33:59 UTC
Description of problem:
Fail to edit the guest's XML associated with a saved state file with virsh save-image-edit command

Version-Release number:
kernel-3.10.0-244.ael7b.ppc64le
libvirt-1.2.16-1.el7.ppc64le
qemu-kvm-rhev-2.3.0-2.el7.ppc64le


How reproducible:
100%

Steps to Reproduce:
1.Start a guest
#virsh start virt-tests-vm1
#virsh dumpxml virt-tests-vm1
--
  <os>
    <type arch='ppc64le' machine='pseries-rhel7.1.0'>hvm</type>
    <boot dev='hd'/>
  </os>

2.Save the guest
#virsh save virt-tests-vm1 /tmp/vm1.save

3.Edit the save file with virsh save-image-edit command and change the boot dev from "hd" to "cdrom " in
the guest's xml, however will fail to save the guest's xml with the following error

# virsh save-image-edit /tmp/vm1.save
  <os>
    <type arch='ppc64le' machine='pseries-rhel7.1.0'>hvm</type>
    <boot dev='cdrom'/>  ===> //change the boot dev to cdrom, then save the guest's xml
  </os>
 
error: operation failed: new xml too large to fit in file
Failed. Try again? [y,n,f,?]:

4.It works well with x86_64 plantform

Actual results:
Fail to edit the guest's XML associated with a saved state file with virsh save-image-edit command

Expected results:
should succeed to operate it in ppc64LE plantform

Comment 2 Eric Blake 2015-06-08 16:00:00 UTC
When saving a domain's state to a file, libvirt takes the size of the XML, rounds it up to the nearest page size multiple, then concatenates the qemu migration stream on the end of that.  The complaint about new xml being too large to fit says that your old XML is just barely smaller than the page size multiple, and the changes you are making are enough to push it over the limit to not fit; which is a relatively rare problem.  Can you determine the size of the XML before and after your changes, and see if there is anything else that can be minimized to allow the image to fit?

Or maybe we have a real bug that is miscalculating sizes, and your new XML would fit if it weren't triggering a size calculation bug.

Comment 3 Daniel Berrangé 2015-06-08 16:05:26 UTC
(In reply to Eric Blake from comment #2)
> When saving a domain's state to a file, libvirt takes the size of the XML,
> rounds it up to the nearest page size multiple, then concatenates the qemu
> migration stream on the end of that.  The complaint about new xml being too
> large to fit says that your old XML is just barely smaller than the page
> size multiple, and the changes you are making are enough to push it over the
> limit to not fit; which is a relatively rare problem.  Can you determine the
> size of the XML before and after your changes, and see if there is anything
> else that can be minimized to allow the image to fit?

The libvirt padding logic isn't merely rounding to the page size multiple - it adds a 1024 byte fixed pad before rounding

    pad = 1024;
    pad += (QEMU_MONITOR_MIGRATE_TO_FILE_BS -
            ((offset + pad) % QEMU_MONITOR_MIGRATE_TO_FILE_BS));

So you should have quite a bit of spare room for a extra additions.

That, I do wonder if we're being too stingy here - saved state files contain entire guest RAM so are going to be multiple-MB in size, possibly even GB. As such, we could *easily* leave 64 kb of padding without a meaningful size increase in the state files, which is enough padding for any conceivable editing need.

Comment 4 zhenfeng wang 2015-10-14 07:36:07 UTC
Hi Andrea
I found a similar issue with this bug that fail to edit the guest's save file while save the guest with the --xml option in X86_64 arch, can you help check whether it's the same with this bug or not, if yes, then we should update the "hardware" for this bug and fix this issue in both ppc64le and x86_64, thanks

Addtional info for the above description:
I coulnd't reproduce the comment0's issue in this bug in x86_64

pkginfo
libvirt-1.2.17-13.el7.x86_64

steps
1.Prepare a normal running guest in host and save its xml while guest start
#virsh start rhel724
#virsh dumpxml rhel724
--
    <graphics type='spice' port='5960' autoport='no' listen='0.0.0.0'>
      <listen type='address' address='0.0.0.0'/>
      <image compression='off'/>
    </graphics>
#virsh dumpxml rhel724 >rhel724.xml

2.Save the guest with xml
#virsh save rhel724 /tmp/rhel724.save --xml rhel724.xml

3.Edit the save file, add extend the graphics element, then save it, will fail to save it and get the following error
#virsh save-image-edit /tmp/rhel724.save
--
    <graphics type='spice' port='5960' autoport='no' listen='0.0.0.0' passwd='123456'>
      <listen type='address' address='0.0.0.0'/>
      <image compression='off'/>
    </graphics>

:wq
error: operation failed: new xml too large to fit in file
Failed. Try again? [y,n,f,?]: 


4.if save guest without specifying --xml option, then the save file could be updated successfully

#virsh save rhel724 /tmp/rhel724.save
#virsh save-image-edit /tmp/rhle724.save

Comment 5 Andrea Bolognani 2015-10-15 15:42:00 UTC
Hi,

I'm still looking into the issue, but so far it looks like
the source is indeed the same.

In fact, can trigger the error when editing a save image
created without --xml on x86_64 by simply adding enough
data to the domain definition.

The only difference I've seen so far is that when using
--xml adding a single character is enough to trigger the
error, whereas when --xml is not used you can add much
more data.

I'll keep you updated.

Comment 9 yisun 2016-07-25 04:18:22 UTC
Created attachment 1183535 [details]
xml of the vm

Comment 10 yisun 2016-07-25 04:22:35 UTC
This is reproduced with:
libvirt-2.0.0-3.el7.x86_64


Scenario 1: save-image-edit

# virsh save avocado-vt-vm1 /tmp/vm.save

# virsh save-image-edit /tmp/vm.save 
(and change <boot dev='hd'/> to  <boot dev='cdrom'/>)

error: operation failed: new xml too large to fit in file
Failed. Try again? [y,n,f,?]: 


Scenario 2: save-image-define 
# virsh save-image-dumpxml /tmp/vm.save > /tmp/old_vm.xml

#vim /tmp/old_vm.xml 
(and change <boot dev='hd'/> to  <boot dev='cdrom'/>)


# virsh save-image-define /tmp/vm.save /tmp/old_vm.xml --running
error: Failed to update /tmp/vm.save
error: operation failed: new xml too large to fit in file




The vm's original xml is attached as Attachment #1183535 [details]

Comment 11 yisun 2016-07-27 02:45:06 UTC
*** Bug 1360119 has been marked as a duplicate of this bug. ***

Comment 12 yisun 2016-07-27 02:49:03 UTC
With same xml in Attachment #1183535 [details], it reproduced on libvirt 1.2.17-13 and  1.3.5-1, so it's not a x86 platform regression issue. Change the "Hardware" field from "ppc64le" to "ALL". 




 ============ libvirt-1.2.17-13.el7.x86_64 ============

[root@localhost 1.2.17-13.el7]# service libvirtd restart
Redirecting to /bin/systemctl restart  libvirtd.service
[root@localhost 1.2.17-13.el7]# rpm -qa | grep libvirt-1
libvirt-1.2.17-13.el7.x86_64
[root@localhost 1.2.17-13.el7]# rpm -qa | grep libvirt-1^C
[root@localhost 1.2.17-13.el7]# virsh save-image-edit /tmp/vm.save 
error: operation failed: new xml too large to fit in file
Failed. Try again? [y,n,f,?]: 
error: operation failed: new xml too large to fit in file

[root@localhost 1.2.17-13.el7]# virsh save-image-define /tmp/vm.save /tmp/old_vm.xml --running
error: Failed to update /tmp/vm.save
error: operation failed: new xml too large to fit in file


 ============ libvirt-1.3.5-1.el7.x86_64 ============
[root@localhost 1.3.5-1.el7]# rpm -qa | grep libvirt-1
libvirt-1.3.5-1.el7.x86_64
[root@localhost 1.3.5-1.el7]# service libvirtd restart
Redirecting to /bin/systemctl restart  libvirtd.service
[root@localhost 1.3.5-1.el7]# virsh save-image-edit /tmp/vm.save 
error: operation failed: new xml too large to fit in file
Failed. Try again? [y,n,f,?]: 
error: operation failed: new xml too large to fit in file

[root@localhost 1.3.5-1.el7]# vim /tmp/old_vm.xml 
[root@localhost 1.3.5-1.el7]# virsh save-image-define /tmp/vm.save /tmp/old_vm.xml --running
error: Failed to update /tmp/vm.save
error: operation failed: new xml too large to fit in file

Comment 15 smitterl 2019-09-11 11:11:45 UTC
This is reproduced with:
libvirt-4.5.0-24.3.module+el8.0.0+4084+cceb9f44
kernel-4.18.0-141.el8


Scenario 1: save-image-edit

# virsh save avocado-vt-vm1 /tmp/vm.save

# virsh save-image-edit /tmp/vm.save 
(and change <boot dev='hd'/> to  <boot dev='cdrom'/>)

error: operation failed: new xml too large to fit in file
Failed. Try again? [y,n,f,?]: 


Scenario 2: save-image-define 
# virsh save-image-dumpxml /tmp/vm.save > /tmp/old_vm.xml

#vim /tmp/old_vm.xml 
(and change <boot dev='hd'/> to  <boot dev='cdrom'/>)


# virsh save-image-define /tmp/vm.save /tmp/old_vm.xml --running
error: Failed to update /tmp/vm.save
error: operation failed: new xml too large to fit in file

Comment 16 Andrea Bolognani 2020-04-29 12:21:12 UTC
The fix has been merged upstream.

commit c4ccb0d0ce57264361dd2ca704f9037935f7f44d
Author: Daniel P. Berrangé <berrange>
Date:   Mon Apr 27 14:35:43 2020 +0100

    qemu: re-add padding to the saved state images
    
    In the past we added 1024 bytes of padding to saved state images so that
    users can run "virsh managedsave-edit $GUEST" and make XML changes which
    increase the size of the XML document. This padding was accidentally
    lost a while back
    
      commit 6b9b21db7079888a05d192b079e68290bdf14a76
      Author: Peter Krempa <pkrempa>
      Date:   Wed Feb 17 13:10:11 2016 +0100
    
        qemu: Remove unnecessary calculations in qemuDomainSaveMemory
    
    The original 1024 bytes was unreasonably stingy when we consider that
    the QEMU state is typically going to be many 100's of MB in size. Thus
    this adds 64 KB of padding after the XML which should cope with any
    plausible modifications a user will want to make.
    
      https://bugzilla.redhat.com/show_bug.cgi?id=1229255
    
    Reviewed-by: Eric Blake <eblake>
    Signed-off-by: Daniel P. Berrangé <berrange>

v6.3.0-rc1

Comment 19 smitterl 2020-05-12 08:56:27 UTC
Verified with
libvirt-6.3.0-1.module+el8.3.0+6478+69f490bb.s390x

Scenario 1: save-image-edit

# virsh save avocado-vt-vm1 /tmp/vm.save

# virsh save-image-edit /tmp/vm.save 
(and change <boot dev='hd'/> to  <boot dev='cdrom'/>)
==> State file /tmp/vm.save edited.

Scenario 2: save-image-define 
# virsh save-image-dumpxml /tmp/vm.save > /tmp/old_vm.xml

#vim /tmp/old_vm.xml 
(and change <boot dev='hd'/> to  <boot dev='cdrom'/>)

# virsh save-image-define /tmp/vm.save /tmp/old_vm.xml --running
State file /tmp/vm.save updated.

Comment 21 smitterl 2020-05-12 09:02:40 UTC
@yanqzhan Can you confirm we can set this to VERIFIED? Thank you.

Comment 22 Yanqiu Zhang 2020-05-13 08:28:04 UTC
Reproduce this bug on:
libvirt-daemon-6.0.0-19.virtcov.el8.x86_64
qemu-kvm-4.2.0-20.module+el8.2.1+6467+49dc3278.x86_64

Steps:
1.<boot dev='hd’/> TO <boot dev='cdrom'/>
(1)# virsh save-image-edit avocado.save1
error: operation failed: new xml too large to fit in file
Failed. Try again? [y,n,f,?]:

(2)# virsh save-image-define  avocado.save1 /tmp/avocado-save.xml
error: Failed to update avocado.save1
error: operation failed: new xml too large to fit in file

(3)# virsh managedsave-edit avocado-vt-vm1
error: operation failed: new xml too large to fit in file
Failed. Try again? [y,n,f,?]:

(4)# virsh managedsave-define avocado-vt-vm1 /tmp/avocado-msave.xml
error: Failed to update avocado-vt-vm1 XML configuration
error: operation failed: new xml too large to fit in file


Verify this bug on:
libvirt-daemon-6.3.0-1.module+el8.3.0+6478+69f490bb.x86_64
qemu-kvm-4.2.0-19.module+el8.3.0+6478+69f490bb.x86_64

1. <boot dev='hd’/> TO <boot dev='cdrom'/>
(1)# virsh save-image-edit /tmp/avocado.save1
State file /tmp/avocado.save1 edited.

# virsh restore /tmp/avocado.save1
Domain restored from /tmp/avocado.save1

# virsh dumpxml avocado-vt-vm1 |grep boot
    <boot dev='cdrom'/>

(2)# virsh save-image-define /tmp/avocado.save2 /tmp/avocado-save.xml
State file /tmp/avocado.save2 updated.

# virsh restore /tmp/avocado.save2
Domain restored from /tmp/avocado.save2

#  virsh dumpxml avocado-vt-vm1 |grep boot
    <boot dev='cdrom'/>

(3)# virsh managedsave-edit avocado-vt-vm1
Managed save image of Domain avocado-vt-vm1 XML configuration edited.

# virsh start avocado-vt-vm1
Domain avocado-vt-vm1 started

#  virsh dumpxml avocado-vt-vm1 |grep boot
    <boot dev='cdrom'/>

(4)
(# virsh managedsave-dumpxml avocado-vt-vm1 > /tmp/avocado-msave.xml
# vim /tmp/avocado-msave.xml)

# virsh managedsave-define avocado-vt-vm1 /tmp/avocado-msave.xml
Managed save state file of domain avocado-vt-vm1 updated.

# virsh start avocado-vt-vm1
Domain avocado-vt-vm1 started

# virsh dumpxml avocado-vt-vm1 |grep boot
    <boot dev='cdrom'/>

2. 64kb padding test
Prepare guest with ‘description’ in xml:
  <description>Some human readable</description>

Save-image-dumpxml the saved file to a new xml file, add 64*1024+ characters in it:
# cat /tmp/avocado-save2.xml|wc -c
6056
# cat /tmp/avocado-save_65537.xml  |wc -c
71593         <---6056+65537
# cat /tmp/avocado-save_65536.xml|wc -c
71592         <---6056+65536

(1)# virsh save-image-define /tmp/avocado.save2  /tmp/avocado-save_65537.xml
error: Failed to update /tmp/avocado.save2
error: operation failed: new xml too large to fit in file

# virsh save-image-define /tmp/avocado.save2  /tmp/avocado-save_65536.xml
State file /tmp/avocado.save2 updated.

# virsh dumpxml avocado-vt-vm1 |grep descrip
  <description>Some human readable description  1234567890123...012</description>

(2)# virsh managedsave-define avocado-vt-vm1  /tmp/avocado-save_65537.xml
error: Failed to update avocado-vt-vm1 XML configuration
error: operation failed: new xml too large to fit in file

# virsh managedsave-define avocado-vt-vm1  /tmp/avocado-save_65536.xml
Managed save state file of domain avocado-vt-vm1 updated.

# virsh start avocado-vt-vm1
Domain avocado-vt-vm1 started

(3)# virsh save-image-edit avocado.save
(Change description part as /tmp/avocado-save_65537.xml: ...0123</description>)
error: operation failed: new xml too large to fit in file
Failed. Try again? [y,n,f,?]: y
(Change description part as /tmp/avocado-save_65536.xml: ...012</description>)
State file avocado.save edited.

# virsh restore avocado.save
Domain restored from avocado.save

(4)# virsh managedsave-edit avocado-vt-vm1
(Change description part as /tmp/avocado-save_65537.xml: ...0123</description>)
error: operation failed: new xml too large to fit in file
Failed. Try again? [y,n,f,?]: y
(Change description part as /tmp/avocado-save_65536.xml: ...012</description>)
Managed save image of Domain avocado-vt-vm1 XML configuration edited.

# virsh start avocado-vt-vm1
Domain avocado-vt-vm1 started

# virsh dumpxml avocado-vt-vm1 |grep descrip
  <description>Some human readable description  12345678901...012</description>

3. Regression test for 0 added or reduce:
(1)# virsh save-image-edit avocado.save
(Do nothing)
Saved image avocado.save XML configuration not changed.

# virsh save-image-edit avocado.save
(Remove description part)
State file avocado.save edited.

# virsh restore avocado.save
Domain restored from avocado.save

# virsh dumpxml avocado-vt-vm1 |grep descrip
(nothing output)

(2)
(Use xml with no change)
# virsh save-image-define avocado.save  save.xml
State file avocado.save updated.

(Use xml which description part removed)
# virsh save-image-define avocado.save save-reduce.xml
State file avocado.save updated.

# virsh dumpxml avocado-vt-vm1 |grep descrip
(nothing output)

(3)# virsh managedsave-edit avocado-vt-vm1
Managed save image of domain avocado-vt-vm1 XML configuration not changed.

# virsh managedsave-edit avocado-vt-vm1
Managed save image of Domain avocado-vt-vm1 XML configuration edited.

# virsh start avocado-vt-vm1
Domain avocado-vt-vm1 started

# virsh dumpxml avocado-vt-vm1 |grep descrip

(4)# virsh managedsave-define avocado-vt-vm1  save.xml
Managed save state file of domain avocado-vt-vm1 updated.

# virsh managedsave-define avocado-vt-vm1  save-reduce.xml
Managed save state file of domain avocado-vt-vm1 updated.

# virsh start avocado-vt-vm1
Domain avocado-vt-vm1 started

# virsh dumpxml avocado-vt-vm1 |grep descrip

Comment 23 Yanqiu Zhang 2020-05-13 08:32:43 UTC
Thanks smitterl's testing on s390.
The test results are as expected, so mark this bug as verified per comment19, comment20, and comment22.

Comment 26 errata-xmlrpc 2020-11-17 17:44:45 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (virt:8.3 bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:5137


Note You need to log in before you can comment on or make changes to this bug.