Bug 1036449

Summary: after live snapshot, a "snapshot=on" block device becomes read-only
Product: Red Hat Enterprise Linux 7 Reporter: Sibiao Luo <sluo>
Component: qemu-kvmAssignee: Jeff Cody <jcody>
Status: CLOSED WONTFIX QA Contact: Virtualization Bugs <virt-bugs>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.0CC: acathrow, chayang, eblake, hhuang, jcody, juzhang, michen, pbonzini, qzhang, virt-maint, xfu
Target Milestone: rcKeywords: Reopened
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 1036450 (view as bug list) Environment:
Last Closed: 2014-01-28 20:05:09 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 1036450    

Description Sibiao Luo 2013-12-02 02:17:26 UTC
Description of problem:
when i just sepcified the data disk readonly=on and make live snapshot, but its snapshot file is readonly, it should be writeable.

Version-Release number of selected component (if applicable):
host info:
3.10.0-10.el7.x86_64
qemu-kvm-1.5.3-20.el7.x86_64
seabios-1.7.2.2-4.el7.x86_64
seabios-bin-1.7.2.2-4.el7.noarch

How reproducible:
100%

Steps to Reproduce:
1.boot a guest with a data disk specified 'readonly=on + snapshot=on'.
e.g1:...-drive file=/home/my-data-disk.qcow2,if=none,id=drive-data-disk,format=qcow2,cache=none,aio=native,werror=stop,rerror=stop,readonly=on,snapshot=on -device scsi-hd,drive=drive-data-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=1,id=data-disk
(qemu) info block
drive-data-disk: removable=0 io-status=ok file=/tmp/vl.ldLw3z backing_file=/home/my-data-disk.qcow2 *ro=0* drv=qcow2 encrypted=0 bps=0 bps_rd=0 bps_wr=0 iops=0 iops_rd=0 iops_wr=0
2.boot guest again sepcified the data disk 'readonly=on' and make live snapshot.
e.g2:...-drive file=/home/my-data-disk.qcow2,if=none,id=drive-data-disk,format=qcow2,cache=none,aio=native,werror=stop,rerror=stop,readonly=on -device scsi-hd,drive=drive-data-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=1,id=data-disk
(qemu) info block
drive-data-disk: removable=0 io-status=ok file=/home/my-data-disk.qcow2 *ro=1* drv=qcow2 encrypted=0 bps=0 bps_rd=0 bps_wr=0 iops=0 iops_rd=0 iops_wr=0
qemu) snapshot_blkdev drive-data-disk /home/snapshot
Formatting '/home/snapshot', fmt=qcow2 size=5368709120 backing_file='/home/my-data-disk.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536
(qemu) info block
drive-data-disk: removable=0 io-status=ok file=/home/snapshot backing_file=/home/my-data-disk.qcow2 *ro=1* drv=qcow2 encrypted=0 bps=0 bps_rd=0 bps_wr=0 iops=0 iops_rd=0 iops_wr=0

Actual results:
after step 1, its snapshot is writeable.
While after step 2, its snapshot is writeable.

Expected results:
after step 2, its snapshot file image should be writeable, like:
(qemu) info block
drive-data-disk: removable=0 io-status=ok file=/home/snapshot backing_file=/home/my-data-disk.qcow2 *ro=0* drv=qcow2 encrypted=0 bps=0 bps_rd=0 bps_wr=0 iops=0 iops_rd=0 iops_wr=0

Additional info:

Comment 1 Jeff Cody 2013-12-17 14:20:36 UTC
Specifying both readonly=on and snapshot=on is somewhat counterintuitive.  With snapshot=on, that does not deal with live snapshots, but tells QEMU to make a temporary difference file for the current session.  If the user then wants to save that data, the user can do a block commit from the HMP interface, which will commit the changes in the temporary difference file back into the image file.

The flag readonly=on will make the actual image file readonly, which it would also be in the case of snapshot=on.  So in reality, in both of those cases the image file remains read-only:

With readonly=on without snapshot=on:

[base]<----[top (r/o)]

With readonly=on with snapshot=on:

[base]<----[top (r/o)]<----[tmp (r/w)]


In both of those cases, the image file 'top' is not modified and remains r/o (unless a block commit is performed to push 'tmp' changes into 'top')

I am going to close the bug as "NOTABUG".  If you disagree, go ahead and reopen the bug.

Comment 2 Paolo Bonzini 2013-12-17 16:45:14 UTC
> In both of those cases, the image file 'top' is not modified and remains r/o
> (unless a block commit is performed to push 'tmp' changes into 'top')

But here can the VM write to tmp?

   [base]<----[top (r/o)]<----[tmp (r/w)]

If it can write, it should still be able to do that after a live-snapshot.  If it cannot, the guest should see the disk as readonly, and it should remain read-only after a live-snapshot (even if you bypass the guest driver using virtio-scsi and, as root, "sg_dd blk_sgio=1").

Comment 3 Jeff Cody 2013-12-17 18:20:32 UTC
(In reply to Paolo Bonzini from comment #2)
> > In both of those cases, the image file 'top' is not modified and remains r/o
> > (unless a block commit is performed to push 'tmp' changes into 'top')
> 
> But here can the VM write to tmp?
> 
>    [base]<----[top (r/o)]<----[tmp (r/w)]
> 

Yes.

> If it can write, it should still be able to do that after a live-snapshot. 

Do you mean a live snapshot on top of tmp?  I don't think you can do that.  Do you mean:

readonly=on
[base]<----[top (r/o)]<-----[snap1 (r/w)] is the expected behavior?

If so, that is not what happens.  Instead, this is what happens:

readonly=on
[base]<----[top (r/o)]<-----[snap1 (r/o)] 

I suppose I could see the argument for snap1 being r/w, as the idea of a new difference file when the drive is read-only is kind of odd.

> If it cannot, the guest should see the disk as readonly, and it should
> remain read-only after a live-snapshot (even if you bypass the guest driver
> using virtio-scsi and, as root, "sg_dd blk_sgio=1").

Comment 4 Paolo Bonzini 2013-12-18 12:20:19 UTC
> > If it can write, it should still be able to do that after a live-snapshot. 
> 
> Do you mean a live snapshot on top of tmp?  I don't think you can do that.

You can actually.  It doesn't make much sense alone, but you could follow it by a block-stream command to remove tmp from the chain.

So after you start the VM you have this:

   [base]<----[top (r/o)]<----[tmp (r/w)]

after you make a live snapshot you have this:

   [base]<----[top (r/o)]<----[tmp (r/w)]<----[snap1 (r/o)]

And the bug is that snap1 should be r/w *or* tmp should be r/o *or* readonly=on,snapshot=on should be forbidden.

Comment 5 Jeff Cody 2013-12-18 16:00:16 UTC
Given that, I think the most logical conclusion is to forbid readonly=on,snapshot=on as you mentioned.  It is either redundant (in both cases the drive image is preserved r/o), or illogical (a read-only ephemeral snapshot with no data makes no sense).

Reopening bug, and moving it to assigned - I'll submit a patch to prevent readonly=on,snapshot=on

Comment 6 Jeff Cody 2013-12-20 21:51:54 UTC
Eric,

If QEMU flagged an error on snapshot=on,readonly=on being used together as a drive argument, do you know if this would affect libvirt negatively?

Comment 7 Eric Blake 2013-12-20 22:26:45 UTC
(In reply to Jeff Cody from comment #6)
> Eric,
> 
> If QEMU flagged an error on snapshot=on,readonly=on being used together as a
> drive argument, do you know if this would affect libvirt negatively?

Libvirt doesn't use snapshot=on (probably never will - bug 1040068 says we want to avoid TMPDIR issues for sVirt reasons, and will instead have to implement our <transient> tag by pre-creating the snapshot before starting qemu).  So no, this would have no ill effects on libvirt.

Comment 8 Ademar Reis 2014-01-28 20:05:09 UTC
(In reply to Eric Blake from comment #7)
> (In reply to Jeff Cody from comment #6)
> > Eric,
> > 
> > If QEMU flagged an error on snapshot=on,readonly=on being used together as a
> > drive argument, do you know if this would affect libvirt negatively?
> 
> Libvirt doesn't use snapshot=on (probably never will - bug 1040068 says we
> want to avoid TMPDIR issues for sVirt reasons, and will instead have to
> implement our <transient> tag by pre-creating the snapshot before starting
> qemu).  So no, this would have no ill effects on libvirt.

Since this option is not used by libvirt, we can safely close it as WONTFIX in RHEL7, because we won't support direct usage of QEMU.

Jeff plans to fix it upstream though, as the patch appears to be simple.